Skip to content

Commit 540086d

Browse files
authored
Refcounted-based internal value (#62)
* Refcounted-based internal value * Fix build on gcc * Fix build with gcc * Fix build with gcc * Fix tests * Fix build on clang * Fix build * Fix .travis.yml
1 parent e368425 commit 540086d

12 files changed

+260
-48
lines changed

.travis.yml

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -60,11 +60,11 @@ matrix:
6060
before_install:
6161
- date -u
6262
- uname -a
63-
- sudo add-apt-repository -y ppa:samuel-bachmann/boost
64-
- sudo apt-get update -qq
63+
# - sudo add-apt-repository -y ppa:samuel-bachmann/boost
64+
# - sudo apt-get update -qq
6565

6666
install:
67-
- sudo apt-get install libboost1.60-all-dev
67+
# - sudo apt-get install libboost1.60-all-dev
6868

6969
script:
7070
- if [[ "${COMPILER}" != "" ]]; then export CXX=${COMPILER}; fi

src/expression_evaluator.cpp

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99
#include <boost/container/small_vector.hpp>
1010

1111
#include <cmath>
12+
#include <stack>
1213

1314
namespace jinja2
1415
{
@@ -55,7 +56,18 @@ InternalValue ValueRefExpression::Evaluate(RenderContext& values)
5556

5657
InternalValue SubscriptExpression::Evaluate(RenderContext& values)
5758
{
58-
return Subscript(m_value->Evaluate(values), m_subscriptExpr->Evaluate(values));
59+
InternalValue cur = m_value->Evaluate(values);
60+
61+
for (auto idx : m_subscriptExprs)
62+
{
63+
auto subscript = idx->Evaluate(values);
64+
auto newVal = Subscript(cur, subscript);
65+
if (cur.ShouldExtendLifetime())
66+
newVal.SetParentData(cur);
67+
std::swap(newVal, cur);
68+
}
69+
70+
return cur;
5971
}
6072

6173
InternalValue UnaryExpression::Evaluate(RenderContext& values)
@@ -252,7 +264,7 @@ InternalValue CallExpression::Evaluate(RenderContext& values)
252264
void CallExpression::Render(OutStream& stream, RenderContext& values)
253265
{
254266
auto fnVal = m_valueRef->Evaluate(values);
255-
Callable* callable = GetIf<Callable>(&fnVal);
267+
const Callable* callable = GetIf<Callable>(&fnVal);
256268
if (callable == nullptr)
257269
{
258270
fnVal = Subscript(fnVal, std::string("operator()"));
@@ -325,6 +337,7 @@ InternalValue CallExpression::CallGlobalRange(RenderContext& values)
325337
{
326338
return m_start + m_step * idx;
327339
}
340+
bool ShouldExtendLifetime() const override {return false;}
328341

329342
private:
330343
int64_t m_start;

src/expression_evaluator.h

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -100,15 +100,19 @@ class ValueRefExpression : public Expression
100100
class SubscriptExpression : public Expression
101101
{
102102
public:
103-
SubscriptExpression(ExpressionEvaluatorPtr<Expression> value, ExpressionEvaluatorPtr<Expression> subscriptExpr)
103+
SubscriptExpression(ExpressionEvaluatorPtr<Expression> value)
104104
: m_value(value)
105-
, m_subscriptExpr(subscriptExpr)
106105
{
107106
}
108107
InternalValue Evaluate(RenderContext& values) override;
108+
void AddIndex(ExpressionEvaluatorPtr<Expression> value)
109+
{
110+
m_subscriptExprs.push_back(value);
111+
}
112+
109113
private:
110114
ExpressionEvaluatorPtr<Expression> m_value;
111-
ExpressionEvaluatorPtr<Expression> m_subscriptExpr;
115+
std::vector<ExpressionEvaluatorPtr<Expression>> m_subscriptExprs;
112116
};
113117

114118
class ConstantExpression : public Expression

src/expression_parser.cpp

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -471,6 +471,7 @@ ExpressionParser::ParseResult<CallParams> ExpressionParser::ParseCallParams(LexS
471471

472472
ExpressionParser::ParseResult<ExpressionEvaluatorPtr<Expression>> ExpressionParser::ParseSubscript(LexScanner& lexer, ExpressionEvaluatorPtr<Expression> valueRef)
473473
{
474+
ExpressionEvaluatorPtr<SubscriptExpression> result = std::make_shared<SubscriptExpression>(valueRef);
474475
for (Token tok = lexer.NextToken(); tok.type == '.' || tok.type == '['; tok = lexer.NextToken())
475476
{
476477
ParseResult<ExpressionEvaluatorPtr<Expression>> indexExpr;
@@ -496,12 +497,12 @@ ExpressionParser::ParseResult<ExpressionEvaluatorPtr<Expression>> ExpressionPars
496497
return MakeParseError(ErrorCode::ExpectedSquareBracket, lexer.PeekNextToken());
497498
}
498499

499-
valueRef = std::make_shared<SubscriptExpression>(valueRef, *indexExpr);
500+
result->AddIndex(*indexExpr);
500501
}
501502

502503
lexer.ReturnToken();
503504

504-
return valueRef;
505+
return result;
505506
}
506507

507508
ExpressionParser::ParseResult<ExpressionEvaluatorPtr<ExpressionFilter>> ExpressionParser::ParseFilterExpression(LexScanner& lexer)

src/filters.cpp

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -256,7 +256,14 @@ InternalValue DictSort::Filter(const InternalValue& baseVal, RenderContext& cont
256256
else
257257
std::sort(tempVector.begin(), tempVector.end(), [comparator](auto& l, auto& r) {return comparator(l, r);});
258258

259-
InternalValueList resultList(tempVector.begin(), tempVector.end());
259+
InternalValueList resultList;
260+
for (auto& tmpVal : tempVector)
261+
{
262+
auto resultVal = InternalValue(std::move(tmpVal));
263+
if (baseVal.ShouldExtendLifetime())
264+
resultVal.SetParentData(baseVal);
265+
resultList.push_back(std::move(resultVal));
266+
}
260267

261268
return InternalValue(ListAdapter::CreateAdapter(std::move(resultList)));
262269
}
@@ -879,6 +886,7 @@ struct ValueConverterImpl : visitors::BaseVisitor<>
879886

880887
size_t GetSize() const override {return m_str->size();}
881888
InternalValue GetValueByIndex(int64_t idx) const override {return InternalValue(m_str->substr(static_cast<size_t>(idx), 1));}
889+
bool ShouldExtendLifetime() const override {return false;}
882890

883891
const string* m_str;
884892
};
@@ -892,6 +900,7 @@ struct ValueConverterImpl : visitors::BaseVisitor<>
892900

893901
size_t GetSize() const override {return m_map->GetSize();}
894902
InternalValue GetValueByIndex(int64_t idx) const override {return m_map->GetValueByIndex(idx);}
903+
bool ShouldExtendLifetime() const override {return false;}
895904

896905
const MapAdapter* m_map;
897906
};
@@ -988,7 +997,7 @@ struct ValueConverterImpl : visitors::BaseVisitor<>
988997
params.mode = ValueConverter::ToIntMode;
989998
params.base = static_cast<int64_t>(10);
990999
InternalValue intVal = Apply<ValueConverterImpl>(val, params);
991-
T* result = GetIf<int64_t>(&intVal);
1000+
const T* result = GetIf<int64_t>(&intVal);
9921001
if (result == nullptr)
9931002
return defValue;
9941003

@@ -1006,7 +1015,11 @@ InternalValue ValueConverter::Filter(const InternalValue& baseVal, RenderContext
10061015
params.base = GetArgumentValue("base", context);
10071016
params.prec = GetArgumentValue("precision", context);
10081017
params.roundMethod = GetArgumentValue("method", context);
1009-
return Apply<ValueConverterImpl>(baseVal, params);
1018+
auto result = Apply<ValueConverterImpl>(baseVal, params);
1019+
if (baseVal.ShouldExtendLifetime())
1020+
result.SetParentData(baseVal);
1021+
1022+
return result;
10101023
}
10111024
} // filters
10121025
} // jinja2

src/internal_value.cpp

Lines changed: 32 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ class ByRef
149149

150150
const T& Get() const {return *m_val;}
151151
T& Get() {return *const_cast<T*>(m_val);}
152+
bool ShouldExtendLifetime() const {return false;}
152153
private:
153154
const T* m_val;
154155
};
@@ -166,10 +167,29 @@ class ByVal
166167

167168
const T& Get() const {return m_val;}
168169
T& Get() {return m_val;}
170+
bool ShouldExtendLifetime() const {return false;}
169171
private:
170172
T m_val;
171173
};
172174

175+
template<typename T>
176+
class BySharedVal
177+
{
178+
public:
179+
BySharedVal(T&& val)
180+
: m_val(std::make_shared<T>(std::move(val)))
181+
{}
182+
~BySharedVal()
183+
{
184+
}
185+
186+
const T& Get() const {return *m_val;}
187+
T& Get() {return *m_val;}
188+
bool ShouldExtendLifetime() const {return true;}
189+
private:
190+
std::shared_ptr<T> m_val;
191+
};
192+
173193
template<template<typename> class Holder>
174194
class GenericListAdapter : public IListAccessor
175195
{
@@ -183,6 +203,7 @@ class GenericListAdapter : public IListAccessor
183203
const auto& val = m_values.Get().GetValueByIndex(idx);
184204
return visit(visitors::InputValueConvertor(true), val.data()).get();
185205
}
206+
bool ShouldExtendLifetime() const override {return m_values.ShouldExtendLifetime();}
186207
private:
187208
Holder<GenericList> m_values;
188209
};
@@ -200,6 +221,7 @@ class ValuesListAdapter : public IListAccessor
200221
const auto& val = m_values.Get()[idx];
201222
return visit(visitors::InputValueConvertor(false), val.data()).get();
202223
}
224+
bool ShouldExtendLifetime() const override {return m_values.ShouldExtendLifetime();}
203225
private:
204226
Holder<ValuesList> m_values;
205227
};
@@ -214,6 +236,7 @@ ListAdapter ListAdapter::CreateAdapter(InternalValueList&& values)
214236

215237
size_t GetSize() const override {return m_values.size();}
216238
InternalValue GetValueByIndex(int64_t idx) const override {return m_values[static_cast<size_t>(idx)];}
239+
bool ShouldExtendLifetime() const override {return false;}
217240
private:
218241
InternalValueList m_values;
219242
};
@@ -233,12 +256,12 @@ ListAdapter ListAdapter::CreateAdapter(const ValuesList& values)
233256

234257
ListAdapter ListAdapter::CreateAdapter(GenericList&& values)
235258
{
236-
return ListAdapter([accessor = GenericListAdapter<ByVal>(std::move(values))]() {return &accessor;});
259+
return ListAdapter([accessor = GenericListAdapter<BySharedVal>(std::move(values))]() {return &accessor;});
237260
}
238261

239262
ListAdapter ListAdapter::CreateAdapter(ValuesList&& values)
240263
{
241-
return ListAdapter([accessor = ValuesListAdapter<ByVal>(std::move(values))]() {return &accessor;});
264+
return ListAdapter([accessor = ValuesListAdapter<BySharedVal>(std::move(values))]() {return &accessor;});
242265
}
243266

244267
template<template<typename> class Holder>
@@ -253,6 +276,7 @@ class SubscriptedListAdapter : public IListAccessor
253276
{
254277
return Subscript(m_values.Get().GetValueByIndex(idx), m_subscript);
255278
}
279+
bool ShouldExtendLifetime() const override {return m_values.ShouldExtendLifetime();}
256280
private:
257281
Holder<ListAdapter> m_values;
258282
InternalValue m_subscript;
@@ -264,7 +288,7 @@ ListAdapter ListAdapter::ToSubscriptedList(const InternalValue& subscript, bool
264288
return ListAdapter([accessor = SubscriptedListAdapter<ByRef>(*this, subscript)]() {return &accessor;});
265289

266290
ListAdapter tmp(*this);
267-
return ListAdapter([accessor = SubscriptedListAdapter<ByVal>(std::move(tmp), subscript)]() {return &accessor;});
291+
return ListAdapter([accessor = SubscriptedListAdapter<BySharedVal>(std::move(tmp), subscript)]() {return &accessor;});
268292
}
269293

270294
InternalValueList ListAdapter::ToValueList() const
@@ -325,6 +349,7 @@ class InternalValueMapAdapter : public IMapAccessor
325349
}
326350
return false;
327351
}
352+
bool ShouldExtendLifetime() const override {return m_values.ShouldExtendLifetime();}
328353
private:
329354
Holder<InternalValueMap> m_values;
330355
};
@@ -380,7 +405,7 @@ class GenericMapAdapter : public IMapAccessor
380405
{
381406
return m_values.Get().GetKeys();
382407
}
383-
408+
bool ShouldExtendLifetime() const override {return m_values.ShouldExtendLifetime();}
384409

385410
private:
386411
Holder<GenericMap> m_values;
@@ -428,6 +453,7 @@ class ValuesMapAdapter : public IMapAccessor
428453

429454
return result;
430455
}
456+
bool ShouldExtendLifetime() const override {return m_values.ShouldExtendLifetime();}
431457
private:
432458
Holder<ValuesMap> m_values;
433459
};
@@ -450,7 +476,7 @@ MapAdapter MapAdapter::CreateAdapter(const GenericMap& values)
450476

451477
MapAdapter MapAdapter::CreateAdapter(GenericMap&& values)
452478
{
453-
return MapAdapter([accessor = GenericMapAdapter<ByVal>(std::move(values))]() mutable {return &accessor;});
479+
return MapAdapter([accessor = GenericMapAdapter<BySharedVal>(std::move(values))]() mutable {return &accessor;});
454480
}
455481

456482
MapAdapter MapAdapter::CreateAdapter(const ValuesMap& values)
@@ -460,7 +486,7 @@ MapAdapter MapAdapter::CreateAdapter(const ValuesMap& values)
460486

461487
MapAdapter MapAdapter::CreateAdapter(ValuesMap&& values)
462488
{
463-
return MapAdapter([accessor = ValuesMapAdapter<ByVal>(std::move(values))]() mutable {return &accessor;});
489+
return MapAdapter([accessor = ValuesMapAdapter<BySharedVal>(std::move(values))]() mutable {return &accessor;});
464490
}
465491

466492
} // jinja2

0 commit comments

Comments
 (0)