From f7cdab7d41fceeab806f7903eadf90aad384ffb2 Mon Sep 17 00:00:00 2001 From: Johannes Wolf Date: Tue, 10 Mar 2026 12:14:36 +0100 Subject: [PATCH 1/5] expr: Add Unique Identifier to Expressions --- include/simfil/expression.h | 15 ++++- include/simfil/parser.h | 6 ++ src/completion.cpp | 20 +++--- src/completion.h | 8 +-- src/expressions.cpp | 131 +++++++++++++++++++----------------- src/expressions.h | 75 +++++++++++---------- src/parser.cpp | 13 +++- src/simfil.cpp | 126 +++++++++++++++++++--------------- 8 files changed, 226 insertions(+), 168 deletions(-) diff --git a/include/simfil/expression.h b/include/simfil/expression.h index 01fb0ad0..93993426 100644 --- a/include/simfil/expression.h +++ b/include/simfil/expression.h @@ -19,6 +19,8 @@ class Expr { friend class AST; public: + using ExprId = std::uint32_t; + /** * Type of an expression. */ @@ -30,8 +32,12 @@ class Expr VALUE, }; - Expr() = default; - explicit Expr(const Token& token) + Expr() = delete; + explicit Expr(ExprId id) + : id_(id) + {} + explicit Expr(ExprId id, const Token& token) + : id_(id) { assert(token.end >= token.begin); sourceLocation_.offset = token.begin; @@ -41,6 +47,10 @@ class Expr virtual ~Expr() = default; /* Category */ + auto id() const -> ExprId + { + return id_; + } virtual auto type() const -> Type = 0; virtual auto constant() const -> bool { @@ -104,6 +114,7 @@ class Expr return ieval(ctx, value, result); } + ExprId id_; SourceLocation sourceLocation_; }; diff --git a/include/simfil/parser.h b/include/simfil/parser.h index 43fe4c2b..0999e500 100644 --- a/include/simfil/parser.h +++ b/include/simfil/parser.h @@ -59,6 +59,7 @@ class Parser }; struct Context { + Expr::ExprId id = 0; bool inPath = false; }; @@ -103,6 +104,11 @@ class Parser auto mode() const -> Mode; auto relaxed() const -> bool; + /** + * Get the next expression id. + */ + auto nextId() -> Expr::ExprId; + Context ctx; Environment* const env; std::unordered_map prefixParsers; diff --git a/src/completion.cpp b/src/completion.cpp index 21e46e68..9c3c06ac 100644 --- a/src/completion.cpp +++ b/src/completion.cpp @@ -125,8 +125,8 @@ auto completeWords(const simfil::Context& ctx, std::string_view prefix, simfil:: namespace simfil { -CompletionFieldOrWordExpr::CompletionFieldOrWordExpr(std::string prefix, Completion* comp, const Token& token, bool inPath) - : Expr(token) +CompletionFieldOrWordExpr::CompletionFieldOrWordExpr(ExprId id, std::string prefix, Completion* comp, const Token& token, bool inPath) + : Expr(id, token) , prefix_(std::move(prefix)) , comp_(comp) , inPath_(inPath) @@ -230,8 +230,9 @@ struct FindExpressionRange : ExprVisitor } -CompletionAndExpr::CompletionAndExpr(ExprPtr left, ExprPtr right, const Completion* comp) - : left_(std::move(left)) +CompletionAndExpr::CompletionAndExpr(ExprId id, ExprPtr left, ExprPtr right, const Completion* comp) + : Expr(id) + , left_(std::move(left)) , right_(std::move(right)) { FindExpressionRange leftRange; @@ -290,8 +291,9 @@ auto CompletionAndExpr::toString() const -> std::string return "(and ? ?)"; } -CompletionOrExpr::CompletionOrExpr(ExprPtr left, ExprPtr right, const Completion* comp) - : left_(std::move(left)) +CompletionOrExpr::CompletionOrExpr(ExprId id, ExprPtr left, ExprPtr right, const Completion* comp) + : Expr(id) + , left_(std::move(left)) , right_(std::move(right)) { FindExpressionRange leftRange; @@ -350,8 +352,8 @@ auto CompletionOrExpr::toString() const -> std::string return "(or ? ?)"; } -CompletionWordExpr::CompletionWordExpr(std::string prefix, Completion* comp, const Token& token) - : Expr(token) +CompletionWordExpr::CompletionWordExpr(ExprId id, std::string prefix, Completion* comp, const Token& token) + : Expr(id, token) , prefix_(std::move(prefix)) , comp_(comp) {} @@ -399,7 +401,7 @@ auto CompletionConstExpr::constant() const -> bool auto CompletionConstExpr::clone() const -> ExprPtr { - return std::make_unique(value_); + return std::make_unique(id(), value_); } auto CompletionConstExpr::ieval(Context ctx, const Value&, const ResultFn& res) -> tl::expected diff --git a/src/completion.h b/src/completion.h index 17eca844..114f5e1b 100644 --- a/src/completion.h +++ b/src/completion.h @@ -45,7 +45,7 @@ struct Completion class CompletionFieldOrWordExpr : public Expr { public: - CompletionFieldOrWordExpr(std::string prefix, Completion* comp, const Token& token, bool inPath); + CompletionFieldOrWordExpr(ExprId id, std::string prefix, Completion* comp, const Token& token, bool inPath); auto type() const -> Type override; auto ieval(Context ctx, const Value& value, const ResultFn& result) -> tl::expected override; @@ -61,7 +61,7 @@ class CompletionFieldOrWordExpr : public Expr class CompletionAndExpr : public Expr { public: - CompletionAndExpr(ExprPtr left, ExprPtr right, const Completion* comp); + CompletionAndExpr(ExprId id, ExprPtr left, ExprPtr right, const Completion* comp); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -75,7 +75,7 @@ class CompletionAndExpr : public Expr class CompletionOrExpr : public Expr { public: - CompletionOrExpr(ExprPtr left, ExprPtr right, const Completion* comp); + CompletionOrExpr(ExprId id, ExprPtr left, ExprPtr right, const Completion* comp); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -89,7 +89,7 @@ class CompletionOrExpr : public Expr class CompletionWordExpr : public Expr { public: - CompletionWordExpr(std::string prefix, Completion* comp, const Token& token); + CompletionWordExpr(ExprId id, std::string prefix, Completion* comp, const Token& token); auto type() const -> Type override; auto constant() const -> bool override; diff --git a/src/expressions.cpp b/src/expressions.cpp index ce3f03f8..f243096c 100644 --- a/src/expressions.cpp +++ b/src/expressions.cpp @@ -76,7 +76,9 @@ auto boolify(const Value& v) -> bool } -WildcardExpr::WildcardExpr() = default; +WildcardExpr::WildcardExpr(ExprId id) + : Expr(id) +{} auto WildcardExpr::type() const -> Type { @@ -132,7 +134,7 @@ auto WildcardExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) -> auto WildcardExpr::clone() const -> ExprPtr { - return std::make_unique(); + return std::make_unique(id()); } void WildcardExpr::accept(ExprVisitor& v) @@ -145,7 +147,9 @@ auto WildcardExpr::toString() const -> std::string return "**"s; } -AnyChildExpr::AnyChildExpr() = default; +AnyChildExpr::AnyChildExpr(ExprId id) + : Expr(id) +{} auto AnyChildExpr::type() const -> Type { @@ -178,7 +182,7 @@ auto AnyChildExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> auto AnyChildExpr::clone() const -> ExprPtr { - return std::make_unique(); + return std::make_unique(id()); } void AnyChildExpr::accept(ExprVisitor& v) @@ -191,15 +195,16 @@ auto AnyChildExpr::toString() const -> std::string return "*"s; } -FieldExpr::FieldExpr(std::string name) - : name_(std::move(name)) +FieldExpr::FieldExpr(ExprId id, std::string name) + : Expr(id) + , name_(std::move(name)) { if (name_ == "_") hits_ = 1; } -FieldExpr::FieldExpr(std::string name, const Token& token) - : Expr(token) +FieldExpr::FieldExpr(ExprId id, std::string name, const Token& token) + : Expr(id, token) , name_(std::move(name)) { if (name_ == "_") @@ -241,6 +246,8 @@ auto FieldExpr::ieval(Context ctx, Value&& val, const ResultFn& res) -> tl::expe /* Enter sub-node */ if (auto sub = val.node()->get(nameId_)) { + //if (ctx.diag) + // ctx.diag->get(id()).hits++; hits_++; return res(ctx, Value::field(*sub)); } @@ -252,7 +259,7 @@ auto FieldExpr::ieval(Context ctx, Value&& val, const ResultFn& res) -> tl::expe auto FieldExpr::clone() const -> ExprPtr { - return std::make_unique(name_); + return std::make_unique(id(), name_); } void FieldExpr::accept(ExprVisitor& v) @@ -265,12 +272,14 @@ auto FieldExpr::toString() const -> std::string return name_; } -MultiConstExpr::MultiConstExpr(const std::vector& vec) - : values_(vec) +MultiConstExpr::MultiConstExpr(ExprId id, const std::vector& vec) + : Expr(id) + , values_(vec) {} -MultiConstExpr::MultiConstExpr(std::vector&& vec) - : values_(std::move(vec)) +MultiConstExpr::MultiConstExpr(ExprId id, std::vector&& vec) + : Expr(id) + , values_(std::move(vec)) {} auto MultiConstExpr::type() const -> Type @@ -297,7 +306,7 @@ auto MultiConstExpr::ieval(Context ctx, const Value&, const ResultFn& res) -> tl auto MultiConstExpr::clone() const -> ExprPtr { - return std::make_unique(values_); + return std::make_unique(id(), values_); } void MultiConstExpr::accept(ExprVisitor& v) @@ -314,8 +323,9 @@ auto MultiConstExpr::toString() const -> std::string return fmt::format("{{{}}}", fmt::join(items, " ")); } -ConstExpr::ConstExpr(Value value) - : value_(std::move(value)) +ConstExpr::ConstExpr(ExprId id, Value value) + : Expr(id) + , value_(std::move(value)) {} auto ConstExpr::type() const -> Type @@ -335,7 +345,7 @@ auto ConstExpr::ieval(Context ctx, const Value&, const ResultFn& res) -> tl::exp auto ConstExpr::clone() const -> ExprPtr { - return std::make_unique(value_); + return std::make_unique(id(), value_); } void ConstExpr::accept(ExprVisitor& v) @@ -355,8 +365,9 @@ auto ConstExpr::value() const -> const Value& return value_; } -SubscriptExpr::SubscriptExpr(ExprPtr left, ExprPtr index) - : left_(std::move(left)) +SubscriptExpr::SubscriptExpr(ExprId id, ExprPtr left, ExprPtr index) + : Expr(id) + , left_(std::move(left)) , index_(std::move(index)) {} @@ -406,7 +417,7 @@ auto SubscriptExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) - auto SubscriptExpr::clone() const -> ExprPtr { - return std::make_unique(left_->clone(), index_->clone()); + return std::make_unique(id(), left_->clone(), index_->clone()); } void SubscriptExpr::accept(ExprVisitor& v) @@ -419,13 +430,9 @@ auto SubscriptExpr::toString() const -> std::string return fmt::format("(index {} {})", left_->toString(), index_->toString()); } -SubExpr::SubExpr(ExprPtr sub) - : left_(std::make_unique("_")) - , sub_(std::move(sub)) -{} - -SubExpr::SubExpr(ExprPtr left, ExprPtr sub) - : left_(std::move(left)) +SubExpr::SubExpr(ExprId id, ExprPtr left, ExprPtr sub) + : Expr(id) + , left_(std::move(left)) , sub_(std::move(sub)) {} @@ -468,7 +475,7 @@ auto SubExpr::toString() const -> std::string auto SubExpr::clone() const -> ExprPtr { - return std::make_unique(left_->clone(), sub_->clone()); + return std::make_unique(id(), left_->clone(), sub_->clone()); } void SubExpr::accept(ExprVisitor& v) @@ -476,8 +483,9 @@ void SubExpr::accept(ExprVisitor& v) v.visit(*this); } -AnyExpr::AnyExpr(std::vector args) - : args_(std::move(args)) +AnyExpr::AnyExpr(ExprId id, std::vector args) + : Expr(id) + , args_(std::move(args)) {} auto AnyExpr::type() const -> Type @@ -526,7 +534,7 @@ auto AnyExpr::clone() const -> ExprPtr return exp->clone(); }); - return std::make_unique(std::move(clonedArgs)); + return std::make_unique(id(), std::move(clonedArgs)); } auto AnyExpr::accept(ExprVisitor& v) -> void @@ -546,8 +554,9 @@ auto AnyExpr::toString() const -> std::string return fmt::format("(any {})", fmt::join(items, " ")); } -EachExpr::EachExpr(std::vector args) - : args_(std::move(args)) +EachExpr::EachExpr(ExprId id, std::vector args) + : Expr(id) + , args_(std::move(args)) {} auto EachExpr::type() const -> Type @@ -595,7 +604,7 @@ auto EachExpr::clone() const -> ExprPtr return exp->clone(); }); - return std::make_unique(std::move(clonedArgs)); + return std::make_unique(id(), std::move(clonedArgs)); } auto EachExpr::accept(ExprVisitor& v) -> void @@ -615,8 +624,9 @@ auto EachExpr::toString() const -> std::string return fmt::format("(each {})", fmt::join(items, " ")); } -CallExpression::CallExpression(std::string name, std::vector args) - : name_(std::move(name)) +CallExpression::CallExpression(ExprId id, std::string name, std::vector args) + : Expr(id) + , name_(std::move(name)) , args_(std::move(args)) {} @@ -659,7 +669,7 @@ auto CallExpression::clone() const -> ExprPtr return exp->clone(); }); - return std::make_unique(name_, std::move(clonedArgs)); + return std::make_unique(id(), name_, std::move(clonedArgs)); } void CallExpression::accept(ExprVisitor& v) @@ -679,13 +689,9 @@ auto CallExpression::toString() const -> std::string return fmt::format("({} {})", name_, fmt::join(items, " ")); } -PathExpr::PathExpr(ExprPtr right) - : left_(std::make_unique("_")) - , right_(std::move(right)) -{} - -PathExpr::PathExpr(ExprPtr left, ExprPtr right) - : left_(std::move(left)) +PathExpr::PathExpr(ExprId id, ExprPtr left, ExprPtr right) + : Expr(id) + , left_(std::move(left)) , right_(std::move(right)) { assert(left_.get()); @@ -731,7 +737,7 @@ auto PathExpr::ieval(Context ctx, Value&& val, const ResultFn& ores) -> tl::expe auto PathExpr::clone() const -> ExprPtr { - return std::make_unique(left_->clone(), right_->clone()); + return std::make_unique(id(), left_->clone(), right_->clone()); } void PathExpr::accept(ExprVisitor& v) @@ -744,8 +750,9 @@ auto PathExpr::toString() const -> std::string return fmt::format("(. {} {})", left_->toString(), right_->toString()); } -UnpackExpr::UnpackExpr(ExprPtr sub) - : sub_(std::move(sub)) +UnpackExpr::UnpackExpr(ExprId id, ExprPtr sub) + : Expr(id) + , sub_(std::move(sub)) {} auto UnpackExpr::type() const -> Type @@ -785,7 +792,7 @@ auto UnpackExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl auto UnpackExpr::clone() const -> ExprPtr { - return std::make_unique(sub_->clone()); + return std::make_unique(id(), sub_->clone()); } void UnpackExpr::accept(ExprVisitor& v) @@ -798,8 +805,9 @@ auto UnpackExpr::toString() const -> std::string return fmt::format("(... {})", sub_->toString()); } -UnaryWordOpExpr::UnaryWordOpExpr(std::string ident, ExprPtr left) - : ident_(std::move(ident)) +UnaryWordOpExpr::UnaryWordOpExpr(ExprId id, std::string ident, ExprPtr left) + : Expr(id) + , ident_(std::move(ident)) , left_(std::move(left)) {} @@ -833,7 +841,7 @@ void UnaryWordOpExpr::accept(ExprVisitor& v) auto UnaryWordOpExpr::clone() const -> ExprPtr { - return std::make_unique(ident_, left_->clone()); + return std::make_unique(id(), ident_, left_->clone()); } auto UnaryWordOpExpr::toString() const -> std::string @@ -841,8 +849,9 @@ auto UnaryWordOpExpr::toString() const -> std::string return fmt::format("({} {})", ident_, left_->toString()); } -BinaryWordOpExpr::BinaryWordOpExpr(std::string ident, ExprPtr left, ExprPtr right) - : ident_(std::move(ident)) +BinaryWordOpExpr::BinaryWordOpExpr(ExprId id, std::string ident, ExprPtr left, ExprPtr right) + : Expr(id) + , ident_(std::move(ident)) , left_(std::move(left)) , right_(std::move(right)) {} @@ -887,7 +896,7 @@ void BinaryWordOpExpr::accept(ExprVisitor& v) auto BinaryWordOpExpr::clone() const -> ExprPtr { - return std::make_unique(ident_, left_->clone(), right_->clone()); + return std::make_unique(id(), ident_, left_->clone(), right_->clone()); } auto BinaryWordOpExpr::toString() const -> std::string @@ -895,8 +904,9 @@ auto BinaryWordOpExpr::toString() const -> std::string return fmt::format("({} {} {})", ident_, left_->toString(), right_->toString()); } -AndExpr::AndExpr(ExprPtr left, ExprPtr right) - : left_(std::move(left)) +AndExpr::AndExpr(ExprId id, ExprPtr left, ExprPtr right) + : Expr(id) + , left_(std::move(left)) , right_(std::move(right)) { assert(left_.get()); @@ -935,7 +945,7 @@ void AndExpr::accept(ExprVisitor& v) auto AndExpr::clone() const -> ExprPtr { - return std::make_unique(left_->clone(), right_->clone()); + return std::make_unique(id(), left_->clone(), right_->clone()); } auto AndExpr::toString() const -> std::string @@ -943,8 +953,9 @@ auto AndExpr::toString() const -> std::string return fmt::format("(and {} {})", left_->toString(), right_->toString()); } -OrExpr::OrExpr(ExprPtr left, ExprPtr right) - : left_(std::move(left)) +OrExpr::OrExpr(ExprId id, ExprPtr left, ExprPtr right) + : Expr(id) + , left_(std::move(left)) , right_(std::move(right)) { assert(left_.get()); @@ -987,7 +998,7 @@ void OrExpr::accept(ExprVisitor& v) auto OrExpr::clone() const -> ExprPtr { - return std::make_unique(left_->clone(), right_->clone()); + return std::make_unique(id(), left_->clone(), right_->clone()); } auto OrExpr::toString() const -> std::string diff --git a/src/expressions.h b/src/expressions.h index 96b3ea2e..4b4f39ab 100644 --- a/src/expressions.h +++ b/src/expressions.h @@ -73,7 +73,7 @@ class ExprVisitor class WildcardExpr : public Expr { public: - WildcardExpr(); + explicit WildcardExpr(ExprId); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected override; @@ -88,7 +88,7 @@ class WildcardExpr : public Expr class AnyChildExpr : public Expr { public: - AnyChildExpr(); + explicit AnyChildExpr(ExprId); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -100,8 +100,8 @@ class AnyChildExpr : public Expr class FieldExpr : public Expr { public: - explicit FieldExpr(std::string name); - FieldExpr(std::string name, const Token& token); + FieldExpr(ExprId id, std::string name); + FieldExpr(ExprId id, std::string name, const Token& token); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -122,8 +122,9 @@ class MultiConstExpr : public Expr public: static constexpr size_t Limit = 10000; - explicit MultiConstExpr(const std::vector& vec); - explicit MultiConstExpr(std::vector&& vec); + MultiConstExpr() = delete; + MultiConstExpr(ExprId id, const std::vector& vec); + MultiConstExpr(ExprId id, std::vector&& vec); auto type() const -> Type override; auto constant() const -> bool override; @@ -138,12 +139,13 @@ class MultiConstExpr : public Expr class ConstExpr : public Expr { public: + ConstExpr() = delete; template - explicit ConstExpr(CType_&& value) - : value_(Value::make(std::forward(value))) + ConstExpr(ExprId id, CType_&& value) + : Expr(id) + , value_(Value::make(std::forward(value))) {} - - explicit ConstExpr(Value value); + ConstExpr(ExprId id, Value value); auto type() const -> Type override; auto constant() const -> bool override; @@ -161,7 +163,7 @@ class ConstExpr : public Expr class SubscriptExpr : public Expr { public: - SubscriptExpr(ExprPtr left, ExprPtr index); + SubscriptExpr(ExprId id, ExprPtr left, ExprPtr index); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected override; @@ -176,8 +178,7 @@ class SubscriptExpr : public Expr class SubExpr : public Expr { public: - explicit SubExpr(ExprPtr sub); - SubExpr(ExprPtr left, ExprPtr sub); + SubExpr(ExprId id, ExprPtr left, ExprPtr sub); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected override; @@ -192,7 +193,7 @@ class SubExpr : public Expr class AnyExpr : public Expr { public: - explicit AnyExpr(std::vector args); + AnyExpr(ExprId id, std::vector args); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -210,7 +211,7 @@ class AnyExpr : public Expr class EachExpr : public Expr { public: - explicit EachExpr(std::vector args); + EachExpr(ExprId id, std::vector args); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -228,7 +229,7 @@ class EachExpr : public Expr class CallExpression : public Expr { public: - CallExpression(std::string name, std::vector args); + CallExpression(ExprId id, std::string name, std::vector args); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -245,8 +246,7 @@ class CallExpression : public Expr class PathExpr : public Expr { public: - explicit PathExpr(ExprPtr right); - PathExpr(ExprPtr left, ExprPtr right); + PathExpr(ExprId id, ExprPtr left, ExprPtr right); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected override; @@ -269,7 +269,7 @@ class PathExpr : public Expr class UnpackExpr : public Expr { public: - explicit UnpackExpr(ExprPtr sub); + UnpackExpr(ExprId id, ExprPtr sub); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -287,8 +287,9 @@ template class UnaryExpr : public Expr { public: - explicit UnaryExpr(ExprPtr sub) - : sub_(std::move(sub)) + UnaryExpr(ExprId id, ExprPtr sub) + : Expr(id) + , sub_(std::move(sub)) {} auto type() const -> Type override @@ -314,7 +315,7 @@ class UnaryExpr : public Expr auto clone() const -> ExprPtr override { - return std::make_unique(sub_->clone()); + return std::make_unique(id(), sub_->clone()); } auto toString() const -> std::string override @@ -332,13 +333,14 @@ template class BinaryExpr : public Expr { public: - BinaryExpr(ExprPtr left, ExprPtr right) - : left_(std::move(left)) + BinaryExpr(ExprId id, ExprPtr left, ExprPtr right) + : Expr(id) + , left_(std::move(left)) , right_(std::move(right)) {} - BinaryExpr(const Token& token, ExprPtr left, ExprPtr right) - : Expr(token) + BinaryExpr(ExprId id, const Token& token, ExprPtr left, ExprPtr right) + : Expr(id, token) , left_(std::move(left)) , right_(std::move(right)) {} @@ -365,7 +367,7 @@ class BinaryExpr : public Expr auto clone() const -> ExprPtr override { - return std::make_unique(left_->clone(), right_->clone()); + return std::make_unique(id(), left_->clone(), right_->clone()); } void accept(ExprVisitor& v) override @@ -387,13 +389,14 @@ class BinaryExpr : public Expr class ComparisonExprBase : public Expr { public: - ComparisonExprBase(ExprPtr left, ExprPtr right) - : left_(std::move(left)) + ComparisonExprBase(ExprId id, ExprPtr left, ExprPtr right) + : Expr(id) + , left_(std::move(left)) , right_(std::move(right)) {} - ComparisonExprBase(const Token& token, ExprPtr left, ExprPtr right) - : Expr(token) + ComparisonExprBase(ExprId id, const Token& token, ExprPtr left, ExprPtr right) + : Expr(id, token) , left_(std::move(left)) , right_(std::move(right)) {} @@ -451,7 +454,7 @@ class ComparisonExpr : public ComparisonExprBase auto clone() const -> ExprPtr override { - return std::make_unique(left_->clone(), right_->clone()); + return std::make_unique(id(), left_->clone(), right_->clone()); } void accept(ExprVisitor& v) override @@ -506,7 +509,7 @@ class BinaryExpr : public ComparisonExpr Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -521,7 +524,7 @@ class UnaryWordOpExpr : public Expr class BinaryWordOpExpr : public Expr { public: - BinaryWordOpExpr(std::string ident, ExprPtr left, ExprPtr right); + BinaryWordOpExpr(ExprId id, std::string ident, ExprPtr left, ExprPtr right); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -536,7 +539,7 @@ class BinaryWordOpExpr : public Expr class AndExpr : public Expr { public: - AndExpr(ExprPtr left, ExprPtr right); + AndExpr(ExprId id, ExprPtr left, ExprPtr right); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; @@ -550,7 +553,7 @@ class AndExpr : public Expr class OrExpr : public Expr { public: - OrExpr(ExprPtr left, ExprPtr right); + OrExpr(ExprId id, ExprPtr left, ExprPtr right); auto type() const -> Type override; auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; diff --git a/src/parser.cpp b/src/parser.cpp index ab9fd132..94932529 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -19,6 +19,8 @@ namespace class NOOPExpr : public Expr { public: + using Expr::Expr; + auto type() const -> Type override { return Type::FIELD; @@ -31,7 +33,7 @@ class NOOPExpr : public Expr auto clone() const -> ExprPtr override { - return std::make_unique(); + return std::make_unique(id()); } void accept(ExprVisitor& v) override @@ -116,6 +118,11 @@ auto Parser::relaxed() const -> bool return mode_ == Mode::Relaxed; } +auto Parser::nextId() -> Expr::ExprId +{ + return ctx.id++; +} + auto Parser::parseInfix(expected left, int prec) -> expected { TRY_EXPECTED(left); @@ -125,7 +132,7 @@ auto Parser::parseInfix(expected left, int prec) -> expected(); + return std::make_unique(nextId()); return unexpected( Error::ParserError, @@ -174,7 +181,7 @@ auto Parser::parseTo(Token::Type type) -> expected if (!*expr) { if (relaxed()) - return std::make_unique(); + return std::make_unique(nextId()); return unexpected( Error::ParserError, diff --git a/src/simfil.cpp b/src/simfil.cpp index 5da371d1..db5a4c9c 100644 --- a/src/simfil.cpp +++ b/src/simfil.cpp @@ -149,10 +149,10 @@ static auto simplifyOrForward(Environment* env, expected expr) - env->warn("Expression is always "s + values[0].toString(), (*expr)->toString()); if (values.size() == 1) - return std::make_unique(std::move(values[0])); + return std::make_unique((*expr)->id(), std::move(values[0])); if (values.size() > 1) - return std::make_unique(std::vector(std::make_move_iterator(values.begin()), - std::make_move_iterator(values.end()))); + return std::make_unique((*expr)->id(), std::vector(std::make_move_iterator(values.begin()), + std::make_move_iterator(values.end()))); return expr; } @@ -174,11 +174,13 @@ class AndOrParser : public InfixParselet return right; if (t.type == Token::OP_AND) - return simplifyOrForward(p.env, std::make_unique(std::move(left), - std::move(*right))); + return simplifyOrForward(p.env, std::make_unique(p.nextId(), + std::move(left), + std::move(*right))); else if (t.type == Token::OP_OR) - return simplifyOrForward(p.env, std::make_unique(std::move(left), - std::move(*right))); + return simplifyOrForward(p.env, std::make_unique(p.nextId(), + std::move(left), + std::move(*right))); assert(0); return nullptr; } @@ -203,11 +205,13 @@ class CompletionAndOrParser : public InfixParselet return right; if (t.type == Token::OP_AND) - return simplifyOrForward(p.env, std::make_unique(std::move(left), - std::move(*right), comp_)); + return simplifyOrForward(p.env, std::make_unique(p.nextId(), + std::move(left), + std::move(*right), comp_)); else if (t.type == Token::OP_OR) - return simplifyOrForward(p.env, std::make_unique(std::move(left), - std::move(*right), comp_)); + return simplifyOrForward(p.env, std::make_unique(p.nextId(), + std::move(left), + std::move(*right), comp_)); assert(0); return nullptr; } @@ -227,7 +231,7 @@ class CastParser : public InfixParselet { auto type = p.consume(); if (type.type == Token::C_NULL) - return std::make_unique(Value::null()); + return std::make_unique(p.nextId(), Value::null()); if (type.type != Token::Type::WORD) return unexpected(Error::InvalidType, fmt::format("'as' expected typename got {}", type.toString())); @@ -235,17 +239,17 @@ class CastParser : public InfixParselet auto name = std::get(type.value); return simplifyOrForward(p.env, [&]() -> expected { if (name == strings::TypenameNull) - return std::make_unique(Value::null()); + return std::make_unique(p.nextId(), Value::null()); if (name == strings::TypenameBool) - return std::make_unique>(std::move(left)); + return std::make_unique>(p.nextId(), std::move(left)); if (name == strings::TypenameInt) - return std::make_unique>(std::move(left)); + return std::make_unique>(p.nextId(), std::move(left)); if (name == strings::TypenameFloat) - return std::make_unique>(std::move(left)); + return std::make_unique>(p.nextId(), std::move(left)); if (name == strings::TypenameString) - return std::make_unique>(std::move(left)); + return std::make_unique>(p.nextId(), std::move(left)); if (name == strings::TypenameBytes) - return std::make_unique>(std::move(left)); + return std::make_unique>(p.nextId(), std::move(left)); return unexpected(Error::InvalidType, fmt::format("Invalid type name for cast '{}'", name)); }()); @@ -273,7 +277,8 @@ class BinaryOpParser : public InfixParselet if (!right) return right; - return simplifyOrForward(p.env, std::make_unique>(t, + return simplifyOrForward(p.env, std::make_unique>(p.nextId(), + t, std::move(left), std::move(*right))); } @@ -298,7 +303,7 @@ class UnaryOpParser : public PrefixParselet if (!sub) return sub; - return simplifyOrForward(p.env, std::make_unique>(std::move(*sub))); + return simplifyOrForward(p.env, std::make_unique>(p.nextId(), std::move(*sub))); } }; @@ -310,7 +315,7 @@ class UnaryPostOpParser : public InfixParselet { auto parse(Parser& p, ExprPtr left, Token t) const -> expected override { - return p.parseInfix(simplifyOrForward(p.env, std::make_unique>(std::move(left))), 0); + return p.parseInfix(simplifyOrForward(p.env, std::make_unique>(p.nextId(), std::move(left))), 0); } auto precedence() const -> int override @@ -326,7 +331,7 @@ class UnpackOpParser : public InfixParselet { auto parse(Parser& p, ExprPtr left, Token t) const -> expected override { - return p.parseInfix(simplifyOrForward(p.env, std::make_unique(std::move(left))), 0); + return p.parseInfix(simplifyOrForward(p.env, std::make_unique(p.nextId(), std::move(left))), 0); } auto precedence() const -> int override @@ -348,12 +353,14 @@ class WordOpParser : public InfixParselet return right; if (*right) - return simplifyOrForward(p.env, std::make_unique(std::get(t.value), + return simplifyOrForward(p.env, std::make_unique(p.nextId(), + std::get(t.value), std::move(left), std::move(*right))); /* Parse as unary operator */ - return p.parseInfix(simplifyOrForward(p.env, std::make_unique(std::get(t.value), + return p.parseInfix(simplifyOrForward(p.env, std::make_unique(p.nextId(), + std::get(t.value), std::move(left))), 0); } @@ -373,7 +380,7 @@ class ScalarParser : public PrefixParselet { auto parse(Parser& p, Token t) const -> expected override { - return std::make_unique(std::get(t.value)); + return std::make_unique(p.nextId(), std::get(t.value)); } }; @@ -387,7 +394,7 @@ class RegExpParser : public PrefixParselet auto parse(Parser& p, Token t) const -> expected override { auto value = ReType::Type.make(std::get(t.value)); - return std::make_unique(std::move(value)); + return std::make_unique(p.nextId(), std::move(value)); } }; @@ -408,7 +415,7 @@ class ConstParser : public PrefixParselet auto parse(Parser& p, Token t) const -> expected override { - return std::make_unique(value_); + return std::make_unique(p.nextId(), value_); } Value value_; @@ -443,7 +450,10 @@ class SubscriptParser : public PrefixParselet, public InfixParselet if (!body) return body; - return simplifyOrForward(p.env, std::make_unique(std::make_unique("_"), + auto outerId = p.nextId(); + auto innerId = p.nextId(); + return simplifyOrForward(p.env, std::make_unique(outerId, + std::make_unique(innerId, "_"), std::move(*body))); } @@ -454,7 +464,8 @@ class SubscriptParser : public PrefixParselet, public InfixParselet if (!body) return body; - return simplifyOrForward(p.env, std::make_unique(std::move(left), + return simplifyOrForward(p.env, std::make_unique(p.nextId(), + std::move(left), std::move(*body))); } @@ -479,7 +490,11 @@ class SubSelectParser : public PrefixParselet, public InfixParselet * with the current node on the left. As "standalone" sub-selects are not useful. */ auto body = p.parseTo(Token::RBRACE); TRY_EXPECTED(body); - return simplifyOrForward(p.env, std::make_unique(std::make_unique("_"), + + auto outerId = p.nextId(); + auto innerId = p.nextId(); + return simplifyOrForward(p.env, std::make_unique(outerId, + std::make_unique(innerId, "_"), std::move(*body))); } @@ -488,7 +503,8 @@ class SubSelectParser : public PrefixParselet, public InfixParselet auto _ = scopedNotInPath(p); auto body = p.parseTo(Token::RBRACE); TRY_EXPECTED(body); - return simplifyOrForward(p.env, std::make_unique(std::move(left), + return simplifyOrForward(p.env, std::make_unique(p.nextId(), + std::move(left), std::move(*body))); } @@ -510,15 +526,15 @@ class WordParser : public PrefixParselet { /* Self */ if (t.type == Token::SELF) - return std::make_unique("_", t); + return std::make_unique(p.nextId(), "_", t); /* Any Child */ if (t.type == Token::OP_TIMES) - return std::make_unique(); + return std::make_unique(p.nextId()); /* Wildcard */ if (t.type == Token::WILDCARD) - return std::make_unique(); + return std::make_unique(p.nextId()); auto word = std::get(t.value); @@ -531,25 +547,25 @@ class WordParser : public PrefixParselet TRY_EXPECTED(arguments); if (word == "any") { - return simplifyOrForward(p.env, std::make_unique(std::move(*arguments))); + return simplifyOrForward(p.env, std::make_unique(p.nextId(), std::move(*arguments))); } else if (word == "each" || word == "all") { - return simplifyOrForward(p.env, std::make_unique(std::move(*arguments))); + return simplifyOrForward(p.env, std::make_unique(p.nextId(), std::move(*arguments))); } else { - return simplifyOrForward(p.env, std::make_unique(word, std::move(*arguments))); + return simplifyOrForward(p.env, std::make_unique(p.nextId(), word, std::move(*arguments))); } } else if (!p.ctx.inPath) { /* Parse Symbols (words in upper-case) */ if (isSymbolWord(word)) { - return std::make_unique(Value::make(std::move(word))); + return std::make_unique(p.nextId(), Value::make(std::move(word))); } /* Constant */ else if (auto constant = p.env->findConstant(word)) { - return std::make_unique(*constant); + return std::make_unique(p.nextId(), *constant); } } /* Single field name */ - return std::make_unique(std::move(word), t); + return std::make_unique(p.nextId(), std::move(word), t); } }; @@ -567,15 +583,15 @@ class CompletionWordParser : public WordParser { /* Self */ if (t.type == Token::SELF) - return std::make_unique("_"); + return std::make_unique(p.nextId(), "_"); /* Any Child */ if (t.type == Token::OP_TIMES) - return std::make_unique(); + return std::make_unique(p.nextId()); /* Wildcard */ if (t.type == Token::WILDCARD) - return std::make_unique(); + return std::make_unique(p.nextId()); auto word = std::get(t.value); @@ -591,26 +607,26 @@ class CompletionWordParser : public WordParser auto arguments = p.parseList(Token::RPAREN); TRY_EXPECTED(arguments); - return simplifyOrForward(p.env, std::make_unique(word, std::move(*arguments))); + return simplifyOrForward(p.env, std::make_unique(p.nextId(), word, std::move(*arguments))); } else if (!p.ctx.inPath) { /* Parse Symbols (words in upper-case) */ if (isSymbolWord(word)) { if (t.containsPoint(comp_->point)) { - return std::make_unique(word.substr(0, comp_->point - t.begin), comp_, t); + return std::make_unique(p.nextId(), word.substr(0, comp_->point - t.begin), comp_, t); } - return std::make_unique(Value::make(std::move(word))); + return std::make_unique(p.nextId(), Value::make(std::move(word))); } /* Constant */ else if (auto constant = p.env->findConstant(word)) { - return std::make_unique(*constant); + return std::make_unique(p.nextId(), *constant); } } /* Single field name */ if (t.containsPoint(comp_->point)) { - return std::make_unique(word.substr(0, comp_->point - t.begin), comp_, t, p.ctx.inPath); + return std::make_unique(p.nextId(), word.substr(0, comp_->point - t.begin), comp_, t, p.ctx.inPath); } - return std::make_unique(std::move(word)); + return std::make_unique(p.nextId(), std::move(word)); } Completion* comp_; @@ -637,7 +653,7 @@ class PathParser : public InfixParselet auto right = p.parsePrecedence(precedence()); TRY_EXPECTED(right); - return std::make_unique(std::move(left), std::move(*right)); + return std::make_unique(p.nextId(), std::move(left), std::move(*right)); } auto precedence() const -> int override @@ -668,10 +684,10 @@ class CompletionPathParser : public PathParser if (!*right) { Token expectedWord(Token::WORD, "", t.end, t.end); - right = std::make_unique("", comp_, expectedWord, p.ctx.inPath); + right = std::make_unique(p.nextId(), "", comp_, expectedWord, p.ctx.inPath); } - return std::make_unique(std::move(left), std::move(*right)); + return std::make_unique(p.nextId(), std::move(left), std::move(*right)); } Completion* comp_; @@ -803,8 +819,10 @@ auto compile(Environment& env, std::string_view query, bool any, bool autoWildca /* Expand a single value to `** == ` */ if (autoWildcard && *root && (*root)->constant()) { + auto outerId = p.nextId(); + auto innerId = p.nextId(); root = std::make_unique>( - std::make_unique(), std::move(*root)); + outerId, std::make_unique(innerId), std::move(*root)); } if (!*root) @@ -813,7 +831,7 @@ auto compile(Environment& env, std::string_view query, bool any, bool autoWildca if (any) { std::vector args; args.emplace_back(std::move(*root)); - return simplifyOrForward(p.env, std::make_unique(std::move(args))); + return simplifyOrForward(p.env, std::make_unique(p.nextId(), std::move(args))); } else { return root; } From 75c11e646fe7becbb59b7d058896ff50e13fb722 Mon Sep 17 00:00:00 2001 From: Johannes Wolf Date: Wed, 11 Mar 2026 00:32:25 +0100 Subject: [PATCH 2/5] diagnostics: Rework Diagnostics --- CMakeLists.txt | 2 + include/simfil/diagnostics.h | 93 ++++++++- include/simfil/environment.h | 5 +- include/simfil/expression-visitor.h | 77 +++++++ include/simfil/expression.h | 4 +- include/simfil/sourcelocation.h | 13 +- src/diagnostics.cpp | 312 +++++++++++----------------- src/environment.cpp | 4 +- src/expression-visitor.cpp | 182 ++++++++++++++++ src/expressions.cpp | 204 ++---------------- src/expressions.h | 108 ++-------- src/simfil.cpp | 19 +- test/diagnostics.cpp | 14 +- 13 files changed, 538 insertions(+), 499 deletions(-) create mode 100644 include/simfil/expression-visitor.h create mode 100644 src/expression-visitor.cpp diff --git a/CMakeLists.txt b/CMakeLists.txt index 8ae72b5c..f0db16a0 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -71,6 +71,7 @@ add_library(simfil ${LIBRARY_TYPE} src/value.cpp src/overlay.cpp src/exception-handler.cpp + src/expression-visitor.cpp src/model/model.cpp src/model/nodes.cpp src/model/string-pool.cpp) @@ -94,6 +95,7 @@ target_sources(simfil PUBLIC include/simfil/transient.h include/simfil/simfil.h include/simfil/exception-handler.h + include/simfil/expression-visitor.h include/simfil/model/arena.h include/simfil/model/column.h diff --git a/include/simfil/diagnostics.h b/include/simfil/diagnostics.h index 791fd6bc..95ad7c5d 100644 --- a/include/simfil/diagnostics.h +++ b/include/simfil/diagnostics.h @@ -2,12 +2,14 @@ #pragma once +#include "simfil/sourcelocation.h" #include "simfil/value.h" -#include "simfil/token.h" #include "simfil/error.h" +#include "simfil/expression.h" #include #include +#include #include #include #include @@ -16,7 +18,6 @@ namespace simfil { class AST; -class Expr; struct Environment; struct ModelNode; @@ -24,7 +25,23 @@ struct ModelNode; struct Diagnostics { public: - using ExprId = std::uint32_t; + struct FieldExprData + { + SourceLocation location; + std::uint32_t hits = 0; + std::uint32_t evaluations = 0; + std::string name; + }; + + + struct ComparisonExprData + { + SourceLocation location; + TypeFlags leftTypes; + TypeFlags rightTypes; + std::uint32_t falseResults = 0u; + std::uint32_t trueResults = 0u; + }; struct Message { @@ -42,6 +59,12 @@ struct Diagnostics Diagnostics(Diagnostics&&) noexcept; ~Diagnostics(); + /** + * Get diagnostics data for a single Expr. + */ + template + auto get(const Expr& expr) -> DiagnosticsDataType&; + /** * Append/merge another diagnostics object into this one. */ @@ -53,17 +76,23 @@ struct Diagnostics auto write(std::ostream& stream) const -> tl::expected; auto read(std::istream& stream) -> tl::expected; - struct Data; + /** ExprId to diagnostics data index mapping. */ + std::vector exprIndex_; + + /** FieldExpr diagnostics data. */ + std::vector fieldData_; + + /** ComparisonExpr diagnostics data. */ + std::vector comparisonData_; + private: friend auto eval(Environment&, const AST&, const ModelNode&, Diagnostics*) -> tl::expected, Error>; friend auto diagnostics(Environment& env, const AST& ast, const Diagnostics& diag) -> tl::expected, Error>; - std::unique_ptr data; - /** - * Collect diagnostics data from an AST. + * Build the exprIndex_ map for the AST. */ - auto collect(Expr& ast) -> void; + auto prepareIndizes(Expr& ast) -> void; /** * Build messages from this objecst diagnostics data. @@ -71,4 +100,52 @@ struct Diagnostics auto buildMessages(Environment& env, const AST& ast) const -> std::vector; }; +namespace detail +{ + +template +struct DiagnosticsStorage; + +template <> +struct DiagnosticsStorage +{ + static auto get(Diagnostics& diag) + { + return &diag.fieldData_; + } +}; + +template <> +struct DiagnosticsStorage +{ + static auto get(Diagnostics& diag) + { + return &diag.comparisonData_; + } +}; + +} + +/** + * Get typed diagnostics data for a single Expr. + */ +template +auto Diagnostics::get(const Expr& expr) -> DiagnosticsDataType& +{ + auto* data = detail::DiagnosticsStorage::get(*this); + + const auto id = expr.id(); + if (exprIndex_.size() <= id) [[unlikely]] { + exprIndex_.resize(id + 1u); + exprIndex_[id] = data->size(); + } + + const auto index = exprIndex_[id]; + if (data->size() <= index) { + data->resize(index + 1u); + } + + return (*data)[index]; +} + } diff --git a/include/simfil/environment.h b/include/simfil/environment.h index 4ed50461..03838728 100644 --- a/include/simfil/environment.h +++ b/include/simfil/environment.h @@ -21,6 +21,7 @@ namespace simfil class Expr; class Function; +class Diagnostics; struct ResultFn; struct Debug; @@ -138,6 +139,7 @@ struct Environment struct Context { Environment* const env; + Diagnostics* const diag; /* Current phase under which the evaluation * takes place. */ @@ -151,7 +153,8 @@ struct Context /* Timeout after which the evaluation should be canceled. */ std::optional> timeout; - Context(Environment* env, Phase = Phase::Evaluation); + Context() = delete; + Context(Environment* env, Diagnostics* diag, Phase = Phase::Evaluation); auto canceled() const -> bool { diff --git a/include/simfil/expression-visitor.h b/include/simfil/expression-visitor.h new file mode 100644 index 00000000..443f0da2 --- /dev/null +++ b/include/simfil/expression-visitor.h @@ -0,0 +1,77 @@ +// Copyright (c) Navigation Data Standard e.V. - See "LICENSE" file. + +#pragma once + +#include + +namespace simfil +{ + +class Expr; +class WildcardExpr; +class AnyChildExpr; +class MultiConstExpr; +class ConstExpr; +class SubscriptExpr; +class SubExpr; +class AnyExpr; +class EachExpr; +class CallExpression; +class UnpackExpr; +class UnaryWordOpExpr; +class BinaryWordOpExpr; +class FieldExpr; +class PathExpr; +class AndExpr; +class OrExpr; +struct OperatorEq; +struct OperatorNeq; +struct OperatorLt; +struct OperatorLtEq; +struct OperatorGt; +struct OperatorGtEq; +template class UnaryExpr; +template class BinaryExpr; + +/** + * Visitor base for visiting expressions recursively. + */ +class ExprVisitor +{ +public: + ExprVisitor(); + virtual ~ExprVisitor(); + + virtual void visit(Expr& expr); + virtual void visit(WildcardExpr& expr); + virtual void visit(AnyChildExpr& expr); + virtual void visit(MultiConstExpr& expr); + virtual void visit(ConstExpr& expr); + virtual void visit(SubscriptExpr& expr); + virtual void visit(SubExpr& expr); + virtual void visit(AnyExpr& expr); + virtual void visit(EachExpr& expr); + virtual void visit(CallExpression& expr); + virtual void visit(PathExpr& expr); + virtual void visit(FieldExpr& expr); + virtual void visit(UnpackExpr& expr); + virtual void visit(UnaryWordOpExpr& expr); + virtual void visit(BinaryWordOpExpr& expr); + virtual void visit(AndExpr& expr); + virtual void visit(OrExpr& expr); + virtual void visit(BinaryExpr& expr); + virtual void visit(BinaryExpr& expr); + virtual void visit(BinaryExpr& expr); + virtual void visit(BinaryExpr& expr); + virtual void visit(BinaryExpr& expr); + virtual void visit(BinaryExpr& expr); + +protected: + /* Returns the index of the current expression */ + std::size_t index() const; + +private: + std::size_t index_ = 0; +}; + +} diff --git a/include/simfil/expression.h b/include/simfil/expression.h index 93993426..4b71a602 100644 --- a/include/simfil/expression.h +++ b/include/simfil/expression.h @@ -5,7 +5,6 @@ #include "simfil/token.h" #include "simfil/value.h" #include "simfil/environment.h" -#include "simfil/diagnostics.h" #include "simfil/result.h" #include @@ -110,7 +109,8 @@ class Expr virtual auto ieval(Context ctx, const Value& value, const ResultFn& result) -> tl::expected = 0; /* Move-optimized evaluation implementation */ - virtual auto ieval(Context ctx, Value&& value, const ResultFn& result) -> tl::expected { + virtual auto ieval(Context ctx, Value&& value, const ResultFn& result) -> tl::expected + { return ieval(ctx, value, result); } diff --git a/include/simfil/sourcelocation.h b/include/simfil/sourcelocation.h index a3017501..e59d049c 100644 --- a/include/simfil/sourcelocation.h +++ b/include/simfil/sourcelocation.h @@ -1,26 +1,25 @@ #pragma once -#include +#include namespace simfil { struct SourceLocation { - size_t offset = 0; - size_t size = 0; + std::uint32_t offset = 0u; + std::uint32_t size = 0u; - SourceLocation() - : offset(0), size(0) - {} + SourceLocation() = default; - SourceLocation(size_t offset, size_t size) + SourceLocation(std::uint32_t offset, std::uint32_t size) : offset(offset), size(size) {} SourceLocation(const SourceLocation&) = default; auto operator==(const SourceLocation& o) const -> bool = default; + auto operator!=(const SourceLocation& o) const -> bool = default; }; } diff --git a/src/diagnostics.cpp b/src/diagnostics.cpp index 2929bb87..a6718c4a 100644 --- a/src/diagnostics.cpp +++ b/src/diagnostics.cpp @@ -1,37 +1,24 @@ // Copyright (c) Navigation Data Standard e.V. - See "LICENSE" file. #include "simfil/diagnostics.h" +#include "simfil/expression-visitor.h" #include "expressions.h" +#include "simfil/sourcelocation.h" -#include +#include +#include #include #include #include #include -#include #include +#include +#include namespace simfil { -struct Diagnostics::Data -{ - std::mutex mtx; - - // Number a FieldExpr field was found in the model - std::unordered_map fieldHits; - - // Comparison operator operand types - std::unordered_map> comparisonOperandTypes; - - [[nodiscard]] - auto lock() - { - return std::unique_lock(mtx); - } -}; - template void serialize(S& s, TypeFlags& flags) { @@ -39,44 +26,78 @@ void serialize(S& s, TypeFlags& flags) } template -void serialize(S& s, Diagnostics::Data& data) +void serialize(S& s, Diagnostics& data) { - s.ext(data.fieldHits, bitsery::ext::StdMap{std::numeric_limits::max()}, - [](S& s, Diagnostics::ExprId& key, uint32_t& value) { - s.value4b(key); - s.value4b(value); - }); - s.ext(data.comparisonOperandTypes, bitsery::ext::StdMap{std::numeric_limits::max()}, - [](S& s, Diagnostics::ExprId& key, std::tuple& value) { - s.value4b(key); - s.object(std::get<0>(value)); - s.object(std::get<1>(value)); - }); + s.container(data.exprIndex_, std::numeric_limits::max(), [](auto& s2, std::size_t& v) { + s2.value8b(v); + }); + s.container(data.fieldData_, std::numeric_limits::max(), [](auto& s2, Diagnostics::FieldExprData& data) { + s2.value4b(data.location.offset); + s2.value4b(data.location.size); + s2.value4b(data.hits); + s2.value4b(data.evaluations); + s2.text1b(data.name, 0xff); + }); + s.container(data.comparisonData_, std::numeric_limits::max(), [](auto& s2, Diagnostics::ComparisonExprData& data) { + s2.value4b(data.location.offset); + s2.value4b(data.location.size); + s2.object(data.leftTypes); + s2.object(data.rightTypes); + s2.value4b(data.trueResults); + s2.value4b(data.falseResults); + }); } -Diagnostics::Diagnostics() - : data(std::make_unique()) -{} +Diagnostics::Diagnostics() = default; Diagnostics::Diagnostics(Diagnostics&& other) noexcept - : data(std::move(other.data)) + : exprIndex_(std::move(other.exprIndex_)) + , fieldData_(std::move(other.fieldData_)) + , comparisonData_(std::move(other.comparisonData_)) {} Diagnostics::~Diagnostics() = default; Diagnostics& Diagnostics::append(const Diagnostics& other) { - auto otherLock = other.data->lock(); - auto lock = data->lock(); +#if !defined(NDEBUG) + if (!exprIndex_.empty()) + assert(std::ranges::equal(exprIndex_, other.exprIndex_)); +#endif + + // Either our indices are empty or equal to the rhs, so + // we can just copy them every time. + exprIndex_ = other.exprIndex_; - for (const auto& [key, value] : other.data->fieldHits) { - data->fieldHits[key] += value; + for (auto i = 0u; i < std::max(fieldData_.size(), other.fieldData_.size()); ++i) { + auto* ours = i < fieldData_.size() ? &fieldData_[i] : nullptr; + auto* theirs = i < other.fieldData_.size() ? &other.fieldData_[i] : nullptr; + + if (ours && theirs) { + assert(ours->name == theirs->name); + ours->hits += theirs->hits; + ours->evaluations += theirs->evaluations; + } + + if (!ours && theirs) { + fieldData_.emplace_back(*theirs); + } } - for (const auto& [key, value] : other.data->comparisonOperandTypes) { - auto& [left, right] = data->comparisonOperandTypes[key]; - left.set(std::get<0>(value)); - right.set(std::get<1>(value)); + for (auto i = 0u; i < std::max(comparisonData_.size(), other.comparisonData_.size()); ++i) { + auto* ours = i < comparisonData_.size() ? &comparisonData_[i] : nullptr; + auto* theirs = i < other.comparisonData_.size() ? &other.comparisonData_[i] : nullptr; + + if (ours && theirs) { + ours->leftTypes.set(theirs->leftTypes); + ours->rightTypes.set(theirs->rightTypes); + ours->trueResults += theirs->trueResults; + ours->falseResults += theirs->falseResults; + } + + if (!ours && theirs) { + comparisonData_.emplace_back(*theirs); + } } return *this; @@ -87,8 +108,8 @@ auto Diagnostics::write(std::ostream& stream) const -> tl::expected using OutputAdapter = bitsery::OutputStreamAdapter; bitsery::Serializer serializer{stream}; - auto lock = data->lock(); - serializer.object(*data); + //auto lock = data->lock(); + serializer.object(*this); if (!stream.good()) { return tl::unexpected(Error(Error::IOError, "Failed to write diagnostics data to stream")); @@ -102,8 +123,8 @@ auto Diagnostics::read(std::istream& stream) -> tl::expected using InputAdapter = bitsery::InputStreamAdapter; bitsery::Deserializer deserializer{stream}; - auto lock = data->lock(); - deserializer.object(*data); + //auto lock = data->lock(); + deserializer.object(*this); if (!stream.good()) { return tl::unexpected(Error(Error::IOError, "Failed to read diagnostics data from stream")); @@ -114,172 +135,80 @@ auto Diagnostics::read(std::istream& stream) -> tl::expected auto Diagnostics::buildMessages(Environment& env, const AST& ast) const -> std::vector { - struct Visitor : ExprVisitor - { - const AST& ast; - const Environment& env; - const Diagnostics::Data& diagnostics; - std::vector messages; - - Visitor(const AST& ast, const Environment& env, const Diagnostics::Data& diagnostics) - : ast(ast) - , env(env) - , diagnostics(diagnostics) - {} - - using ExprVisitor::visit; - - void visit(FieldExpr& e) override - { - ExprVisitor::visit(e); - - auto iter = diagnostics.fieldHits.find(index()); - if (iter == diagnostics.fieldHits.end()) - return; + std::vector messages; - if (iter->second > 0) - return; + auto addMessage = [&](SourceLocation loc, std::string text) { + Diagnostics::Message msg; + msg.message = std::move(text); + //msg.fix = std::move(fix); + msg.location = loc; - addMessage(fmt::format("No matches for field '{}'.", e.name_), e, {}); - } - - void visitComparisonOperator(ComparisonExprBase& e, bool expectedResult) - { - const auto [falseResults, trueResults] = e.resultCounts(); - if ((expectedResult && trueResults > 0) || (!expectedResult && falseResults > 0)) - return; - - auto iter = diagnostics.comparisonOperandTypes.find(index()); - if (iter == diagnostics.comparisonOperandTypes.end()) - return; - - auto [leftTypes, rightTypes] = iter->second; - if (leftTypes.test(ValueType::Int) || leftTypes.test(ValueType::Float)) { - leftTypes.set(ValueType::Int); - leftTypes.set(ValueType::Float); - } - if (rightTypes.test(ValueType::Int) || rightTypes.test(ValueType::Float)) { - rightTypes.set(ValueType::Int); - rightTypes.set(ValueType::Float); - } - - const auto intersection = leftTypes.flags & rightTypes.flags; - if (intersection.any()) - return; - - addMessage(fmt::format("All values compared to {}. Left hand types are {}, right hand types are {}.", - expectedResult ? "false" : "true", - fmt::join(leftTypes.typeNames(), "|"), - fmt::join(rightTypes.typeNames(), "|")), - e, {}); - } - - void visit(BinaryExpr& e) override - { - ExprVisitor::visit(e); - visitComparisonOperator(e, true); - } + messages.push_back(std::move(msg)); + }; - void visit(BinaryExpr& e) override - { - ExprVisitor::visit(e); - visitComparisonOperator(e, false); - } + for (const auto& data : fieldData_) { + if (data.hits == 0u) + addMessage(data.location, fmt::format("No matches for field '{}'", data.name)); + } - void visit(BinaryExpr& e) override - { - ExprVisitor::visit(e); - visitComparisonOperator(e, true); + for (const auto& data : comparisonData_) { + auto leftTypes = data.leftTypes; + if (leftTypes.test(ValueType::Int) || leftTypes.test(ValueType::Float)) { + leftTypes.set(ValueType::Int); + leftTypes.set(ValueType::Float); } - void visit(BinaryExpr& e) override - { - ExprVisitor::visit(e); - visitComparisonOperator(e, true); + auto rightTypes = data.rightTypes; + if (rightTypes.test(ValueType::Int) || rightTypes.test(ValueType::Float)) { + rightTypes.set(ValueType::Int); + rightTypes.set(ValueType::Float); } - void visit(BinaryExpr& e) override - { - ExprVisitor::visit(e); - visitComparisonOperator(e, false); - } + const auto intersection = leftTypes.flags & rightTypes.flags; + if (true || intersection.none()) { + const auto allTrue = data.trueResults > 0 && data.falseResults == 0; + const auto allFalse = data.falseResults > 0 && data.trueResults == 0; + const auto prefix = + allTrue ? "All values compared to true. " : + allFalse ? "All values compared to false. " : ""; - void visit(BinaryExpr& e) override - { - ExprVisitor::visit(e); - visitComparisonOperator(e, false); + addMessage(data.location, fmt::format("{}Left hand types are {}, right hand types are {}.", + prefix, + fmt::join(leftTypes.typeNames(), "|"), + fmt::join(rightTypes.typeNames(), "|"))); } + } - void addMessage(std::string text, const Expr& expr, std::optional fix) - { - Diagnostics::Message msg; - msg.message = std::move(text); - msg.location = expr.sourceLocation(); - msg.fix = std::move(fix); - - messages.push_back(std::move(msg)); - } - }; - - Visitor visitor(ast, env, *data); - - auto lock = data->lock(); - ast.expr().accept(visitor); - - return visitor.messages; + return messages; } -auto Diagnostics::collect(Expr& ast) -> void +auto Diagnostics::prepareIndizes(Expr& ast) -> void { - struct CollectDiagnostics : ExprVisitor + struct Visitor : ExprVisitor { - Diagnostics::Data& diagnostics; - bool suppressDiagnostics = false; + std::size_t fieldIndex_ = 0u; + std::size_t comparisonIndex_ = 0u; + std::vector indices_; - explicit CollectDiagnostics(Diagnostics::Data& diag) - : diagnostics(diag) - {} + Visitor() + { + indices_.reserve(32); + } using ExprVisitor::visit; auto visit(FieldExpr& e) -> void override { ExprVisitor::visit(e); - - if (e.evaluations_ > 0) - diagnostics.fieldHits[index()] += e.hits_; + if (e.id() >= indices_.size()) + indices_.resize(e.id() + 1); + indices_[e.id()] = fieldIndex_++; } auto visitComparisonOperator(const ComparisonExprBase& e) -> void { - const auto [thisLeft, thisRight] = e.operandTypes(); - if ((thisLeft.flags | thisRight.flags).any()) { - auto& [left, right] = diagnostics.comparisonOperandTypes[index()]; - left.set(thisLeft); - right.set(thisRight); - } - } - - auto visit(AnyExpr& e) -> void override { - ExprVisitor::visit(static_cast(e)); - - // Only provide diagnostic results for - // child expressions if none of them matched. - if (e.trueResults_ == 0 && e.falseResults_ > 0) { - for (const auto& arg : e.args_) - if (arg) - arg->accept(*this); - } - } - - auto visit(OrExpr& e) -> void override { - ExprVisitor::visit(static_cast(e)); - - // Suppress diagnostics of the right hand side - // expression if the left hand side matched and - // vice versa. - e.left_->accept(*this); - if (e.rightEvaluations_ > 0) - e.right_->accept(*this); + if (e.id() >= indices_.size()) + indices_.resize(e.id() + 1); + indices_[e.id()] = comparisonIndex_++; } auto visit(BinaryExpr& e) -> void override { @@ -313,10 +242,11 @@ auto Diagnostics::collect(Expr& ast) -> void } }; - CollectDiagnostics visitor(*data); - - auto lock = data->lock(); + Visitor visitor; ast.accept(visitor); + exprIndex_ = std::move(visitor.indices_); + fieldData_.reserve(visitor.fieldIndex_); + comparisonData_.reserve(visitor.comparisonIndex_); } } diff --git a/src/environment.cpp b/src/environment.cpp index 8230fb7b..88bbd07b 100644 --- a/src/environment.cpp +++ b/src/environment.cpp @@ -1,4 +1,5 @@ #include "simfil/environment.h" +#include "simfil/diagnostics.h" #include "simfil/function.h" namespace simfil @@ -58,8 +59,9 @@ auto Environment::strings() const -> std::shared_ptr { return stringPool; } -Context::Context(Environment* env, Context::Phase phase) +Context::Context(Environment* env, Diagnostics* diag, Context::Phase phase) : env(env) + , diag(diag) , phase(phase) {} diff --git a/src/expression-visitor.cpp b/src/expression-visitor.cpp new file mode 100644 index 00000000..fc02a644 --- /dev/null +++ b/src/expression-visitor.cpp @@ -0,0 +1,182 @@ +// Copyright (c) Navigation Data Standard e.V. - See "LICENSE" file. + +#include "simfil/expression-visitor.h" + +#include "expressions.h" + +namespace simfil +{ + +ExprVisitor::ExprVisitor() = default; +ExprVisitor::~ExprVisitor() = default; + +void ExprVisitor::visit(Expr& e) +{ + index_++; +} + +void ExprVisitor::visit(WildcardExpr& expr) +{ + visit(static_cast(expr)); +} + +void ExprVisitor::visit(AnyChildExpr& expr) +{ + visit(static_cast(expr)); +} + +void ExprVisitor::visit(MultiConstExpr& expr) +{ + visit(static_cast(expr)); +} + +void ExprVisitor::visit(ConstExpr& expr) +{ + visit(static_cast(expr)); +} + +void ExprVisitor::visit(SubscriptExpr& expr) +{ + visit(static_cast(expr)); + + if (expr.left_) + expr.left_->accept(*this); + if (expr.index_) + expr.index_->accept(*this); +} + +void ExprVisitor::visit(SubExpr& expr) +{ + visit(static_cast(expr)); + + if (expr.left_) + expr.left_->accept(*this); + if (expr.sub_) + expr.sub_->accept(*this); +} + +void ExprVisitor::visit(AnyExpr& expr) +{ + visit(static_cast(expr)); + + for (const auto& arg : expr.args_) + if (arg) + arg->accept(*this); +} + +void ExprVisitor::visit(EachExpr& expr) +{ + visit(static_cast(expr)); + + for (const auto& arg : expr.args_) + if (arg) + arg->accept(*this); +} + +void ExprVisitor::visit(CallExpression& expr) +{ + visit(static_cast(expr)); + + for (const auto& arg : expr.args_) + if (arg) + arg->accept(*this); +} + +void ExprVisitor::visit(PathExpr& expr) +{ + visit(static_cast(expr)); + + if (expr.left_) + expr.left_->accept(*this); + if (expr.right_) + expr.right_->accept(*this); +} + +void ExprVisitor::visit(FieldExpr& expr) +{ + visit(static_cast(expr)); +} + +void ExprVisitor::visit(UnpackExpr& expr) +{ + visit(static_cast(expr)); + + if (expr.sub_) + expr.sub_->accept(*this); +} + +void ExprVisitor::visit(UnaryWordOpExpr& expr) +{ + visit(static_cast(expr)); + + if (expr.left_) + expr.left_->accept(*this); +} + +void ExprVisitor::visit(BinaryWordOpExpr& expr) +{ + visit(static_cast(expr)); + + if (expr.left_) + expr.left_->accept(*this); + if (expr.right_) + expr.right_->accept(*this); +} + +void ExprVisitor::visit(AndExpr& expr) +{ + visit(static_cast(expr)); + + if (expr.left_) + expr.left_->accept(*this); + if (expr.right_) + expr.right_->accept(*this); +} + +void ExprVisitor::visit(OrExpr& expr) +{ + visit(static_cast(expr)); + + if (expr.left_) + expr.left_->accept(*this); + if (expr.right_) + expr.right_->accept(*this); +} + +void ExprVisitor::visit(BinaryExpr& e) +{ + visit(static_cast(e)); +} + +void ExprVisitor::visit(BinaryExpr& e) +{ + visit(static_cast(e)); +} + +void ExprVisitor::visit(BinaryExpr& e) +{ + visit(static_cast(e)); +} + +void ExprVisitor::visit(BinaryExpr& e) +{ + visit(static_cast(e)); +} + +void ExprVisitor::visit(BinaryExpr& e) +{ + visit(static_cast(e)); +} + +void ExprVisitor::visit(BinaryExpr& e) +{ + visit(static_cast(e)); +} + +auto ExprVisitor::index() const -> std::size_t +{ + return index_; +} + + +} diff --git a/src/expressions.cpp b/src/expressions.cpp index f243096c..6d63c6fb 100644 --- a/src/expressions.cpp +++ b/src/expressions.cpp @@ -5,6 +5,7 @@ #include "simfil/result.h" #include "simfil/value.h" #include "simfil/function.h" +#include "simfil/diagnostics.h" #include "fmt/core.h" #include "fmt/ranges.h" @@ -198,18 +199,12 @@ auto AnyChildExpr::toString() const -> std::string FieldExpr::FieldExpr(ExprId id, std::string name) : Expr(id) , name_(std::move(name)) -{ - if (name_ == "_") - hits_ = 1; -} +{} FieldExpr::FieldExpr(ExprId id, std::string name, const Token& token) : Expr(id, token) , name_(std::move(name)) -{ - if (name_ == "_") - hits_ = 1; -} +{} auto FieldExpr::type() const -> Type { @@ -223,15 +218,26 @@ auto FieldExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl: auto FieldExpr::ieval(Context ctx, Value&& val, const ResultFn& res) -> tl::expected { - if (ctx.phase != Context::Compilation) - evaluations_++; + Diagnostics::FieldExprData* diag = nullptr; + if (ctx.diag) + diag = &ctx.diag->get(*this); + + if (diag) { + diag->evaluations++; + diag->location = sourceLocation(); + if (diag->name.empty()) + diag->name = name_; + } if (val.isa(ValueType::Undef)) return res(ctx, std::move(val)); /* Special case: _ points to the current node */ - if (name_ == "_") + if (name_ == "_") { + if (diag) + diag->hits++; return res(ctx, std::move(val)); + } if (!val.node()) return res(ctx, Value::null()); @@ -246,9 +252,8 @@ auto FieldExpr::ieval(Context ctx, Value&& val, const ResultFn& res) -> tl::expe /* Enter sub-node */ if (auto sub = val.node()->get(nameId_)) { - //if (ctx.diag) - // ctx.diag->get(id()).hits++; - hits_++; + if (diag) + diag->hits++; return res(ctx, Value::field(*sub)); } @@ -975,15 +980,12 @@ auto OrExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::ex if (lval.isa(ValueType::Undef)) return res(ctx, lval); - ++leftEvaluations_; - auto v = UnaryOperatorDispatcher::dispatch(lval); TRY_EXPECTED(v); if (v->isa(ValueType::Bool)) if (v->template as()) return res(ctx, std::move(lval)); - ++rightEvaluations_; return right_->eval(ctx, val, LambdaResultFn([&](Context ctx, Value&& rval) { return res(ctx, std::move(rval)); })); @@ -1006,172 +1008,4 @@ auto OrExpr::toString() const -> std::string return fmt::format("(or {} {})", left_->toString(), right_->toString()); } -void ExprVisitor::visit(Expr& e) -{ - index_++; -} - -void ExprVisitor::visit(WildcardExpr& expr) -{ - visit(static_cast(expr)); -} - -void ExprVisitor::visit(AnyChildExpr& expr) -{ - visit(static_cast(expr)); -} - -void ExprVisitor::visit(MultiConstExpr& expr) -{ - visit(static_cast(expr)); -} - -void ExprVisitor::visit(ConstExpr& expr) -{ - visit(static_cast(expr)); -} - -void ExprVisitor::visit(SubscriptExpr& expr) -{ - visit(static_cast(expr)); - - if (expr.left_) - expr.left_->accept(*this); - if (expr.index_) - expr.index_->accept(*this); -} - -void ExprVisitor::visit(SubExpr& expr) -{ - visit(static_cast(expr)); - - if (expr.left_) - expr.left_->accept(*this); - if (expr.sub_) - expr.sub_->accept(*this); -} - -void ExprVisitor::visit(AnyExpr& expr) -{ - visit(static_cast(expr)); - - for (const auto& arg : expr.args_) - if (arg) - arg->accept(*this); -} - -void ExprVisitor::visit(EachExpr& expr) -{ - visit(static_cast(expr)); - - for (const auto& arg : expr.args_) - if (arg) - arg->accept(*this); -} - -void ExprVisitor::visit(CallExpression& expr) -{ - visit(static_cast(expr)); - - for (const auto& arg : expr.args_) - if (arg) - arg->accept(*this); -} - -void ExprVisitor::visit(PathExpr& expr) -{ - visit(static_cast(expr)); - - if (expr.left_) - expr.left_->accept(*this); - if (expr.right_) - expr.right_->accept(*this); -} - -void ExprVisitor::visit(FieldExpr& expr) -{ - visit(static_cast(expr)); -} - -void ExprVisitor::visit(UnpackExpr& expr) -{ - visit(static_cast(expr)); - - if (expr.sub_) - expr.sub_->accept(*this); -} - -void ExprVisitor::visit(UnaryWordOpExpr& expr) -{ - visit(static_cast(expr)); - - if (expr.left_) - expr.left_->accept(*this); -} - -void ExprVisitor::visit(BinaryWordOpExpr& expr) -{ - visit(static_cast(expr)); - - if (expr.left_) - expr.left_->accept(*this); - if (expr.right_) - expr.right_->accept(*this); -} - -void ExprVisitor::visit(AndExpr& expr) -{ - visit(static_cast(expr)); - - if (expr.left_) - expr.left_->accept(*this); - if (expr.right_) - expr.right_->accept(*this); -} - -void ExprVisitor::visit(OrExpr& expr) -{ - visit(static_cast(expr)); - - if (expr.left_) - expr.left_->accept(*this); - if (expr.right_) - expr.right_->accept(*this); -} - -void ExprVisitor::visit(BinaryExpr& e) -{ - visit(static_cast(e)); -} - -void ExprVisitor::visit(BinaryExpr& e) -{ - visit(static_cast(e)); -} - -void ExprVisitor::visit(BinaryExpr& e) -{ - visit(static_cast(e)); -} - -void ExprVisitor::visit(BinaryExpr& e) -{ - visit(static_cast(e)); -} - -void ExprVisitor::visit(BinaryExpr& e) -{ - visit(static_cast(e)); -} - -void ExprVisitor::visit(BinaryExpr& e) -{ - visit(static_cast(e)); -} - -auto ExprVisitor::index() const -> size_t -{ - return index_; -} - } diff --git a/src/expressions.h b/src/expressions.h index 4b4f39ab..5faf771a 100644 --- a/src/expressions.h +++ b/src/expressions.h @@ -3,6 +3,8 @@ #include "simfil/expression.h" #include "simfil/model/nodes.h" #include "simfil/operator.h" +#include "simfil/diagnostics.h" +#include "simfil/expression-visitor.h" #include #include @@ -10,66 +12,6 @@ namespace simfil { -class WildcardExpr; -class AnyChildExpr; -class MultiConstExpr; -class ConstExpr; -class SubscriptExpr; -class SubExpr; -class AnyExpr; -class EachExpr; -class CallExpression; -class UnpackExpr; -class UnaryWordOpExpr; -class BinaryWordOpExpr; -class FieldExpr; -class PathExpr; -class AndExpr; -class OrExpr; -template class UnaryExpr; -template class BinaryExpr; - -/** - * Visitor base for visiting expressions recursively. - */ -class ExprVisitor -{ -public: - virtual ~ExprVisitor() = default; - - virtual void visit(Expr& expr); - virtual void visit(WildcardExpr& expr); - virtual void visit(AnyChildExpr& expr); - virtual void visit(MultiConstExpr& expr); - virtual void visit(ConstExpr& expr); - virtual void visit(SubscriptExpr& expr); - virtual void visit(SubExpr& expr); - virtual void visit(AnyExpr& expr); - virtual void visit(EachExpr& expr); - virtual void visit(CallExpression& expr); - virtual void visit(PathExpr& expr); - virtual void visit(FieldExpr& expr); - virtual void visit(UnpackExpr& expr); - virtual void visit(UnaryWordOpExpr& expr); - virtual void visit(BinaryWordOpExpr& expr); - virtual void visit(AndExpr& expr); - virtual void visit(OrExpr& expr); - virtual void visit(BinaryExpr& expr); - virtual void visit(BinaryExpr& expr); - virtual void visit(BinaryExpr& expr); - virtual void visit(BinaryExpr& expr); - virtual void visit(BinaryExpr& expr); - virtual void visit(BinaryExpr& expr); - -protected: - /* Returns the index of the current expression */ - [[nodiscard]] - size_t index() const; - -private: - size_t index_ = 0; -}; - class WildcardExpr : public Expr { public: @@ -112,9 +54,6 @@ class FieldExpr : public Expr std::string name_; StringId nameId_ = {}; - - size_t hits_ = 0; - size_t evaluations_ = 0; }; class MultiConstExpr : public Expr @@ -389,6 +328,7 @@ class BinaryExpr : public Expr class ComparisonExprBase : public Expr { public: + ComparisonExprBase(ExprId id, ExprPtr left, ExprPtr right) : Expr(id) , left_(std::move(left)) @@ -406,21 +346,7 @@ class ComparisonExprBase : public Expr return Type::VALUE; } - auto operandTypes() const -> std::tuple - { - return {leftTypes_, rightTypes_}; - } - - auto resultCounts() const -> std::tuple - { - return {falseResults_, trueResults_}; - } - ExprPtr left_, right_; - TypeFlags leftTypes_; - TypeFlags rightTypes_; - uint32_t falseResults_ = 0; - uint32_t trueResults_ = 0; }; template @@ -431,20 +357,30 @@ class ComparisonExpr : public ComparisonExprBase auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override { - return left_->eval(ctx, val, LambdaResultFn([this, &res, &val](Context ctx, Value lv) { - leftTypes_.set(lv.type); - return right_->eval(ctx, val, LambdaResultFn([this, &res, &lv](Context ctx, Value rv) -> tl::expected { - rightTypes_.set(rv.type); + Diagnostics::ComparisonExprData* diag = nullptr; + if (ctx.diag) + diag = &ctx.diag->get(*this); + if (diag) { + diag->location = sourceLocation(); + } + + return left_->eval(ctx, val, LambdaResultFn([this, &res, &val, &diag](Context ctx, Value lv) { + if (diag) + diag->leftTypes.set(lv.type); + + return right_->eval(ctx, val, LambdaResultFn([this, &res, &lv, &diag](Context ctx, Value rv) -> tl::expected { + if (diag) + diag->rightTypes.set(rv.type); auto operatorResult = BinaryOperatorDispatcher::dispatch(std::move(lv), std::move(rv)); if (!operatorResult) return tl::unexpected(std::move(operatorResult.error())); - if (operatorResult->isa(ValueType::Bool)) { + if (diag && operatorResult->isa(ValueType::Bool)) { if (operatorResult->template as()) - ++trueResults_; + diag->trueResults++; else - ++falseResults_; + diag->falseResults++; } return res(ctx, std::move(operatorResult.value())); @@ -562,10 +498,6 @@ class OrExpr : public Expr auto toString() const -> std::string override; ExprPtr left_, right_; - - /* Runtime Data */ - uint32_t leftEvaluations_ = 0; - uint32_t rightEvaluations_ = 0; }; } diff --git a/src/simfil.cpp b/src/simfil.cpp index db5a4c9c..9e2e6c68 100644 --- a/src/simfil.cpp +++ b/src/simfil.cpp @@ -124,7 +124,7 @@ static auto simplifyOrForward(Environment* env, expected expr) - return nullptr; std::deque values; - auto stub = Context(env, Context::Phase::Compilation); + auto stub = Context(env, nullptr, Context::Phase::Compilation); auto res = (*expr)->eval(stub, Value::undef(), LambdaResultFn([&, n = 0](Context ctx, Value&& vv) mutable { n += 1; if ((n <= MultiConstExpr::Limit) && (!vv.isa(ValueType::Undef) || vv.nodePtr())) { @@ -888,7 +888,7 @@ auto complete(Environment& env, std::string_view query, size_t point, const Mode showComparisonWildcardHint = true; } - Context ctx(&env); + Context ctx(&env, nullptr); if (options.timeoutMs > 0) ctx.timeout = std::chrono::steady_clock::now() + std::chrono::milliseconds(options.timeoutMs); @@ -929,9 +929,14 @@ auto eval(Environment& env, const AST& ast, const ModelNode& node, Diagnostics* if (!node.model_) return unexpected(Error::NullModel, "ModelNode must have a model!"); - Context ctx(&env); + // For thread-safety we work on a local diagnostics object that gets merged + // into diag after query evaluation. + Diagnostics localDiag; + localDiag.prepareIndizes(ast.expr()); - auto mutableAST = ast.expr().clone(); + Context ctx(&env, &localDiag); + + auto mutableAST = &ast.expr(); // TODO: make const again! std::vector values; auto res = mutableAST->eval(ctx, Value::field(node), LambdaResultFn([&values](Context, Value&& value) { @@ -940,9 +945,9 @@ auto eval(Environment& env, const AST& ast, const ModelNode& node, Diagnostics* })); TRY_EXPECTED(res); - if (diag) { - diag->collect(*mutableAST); - } + // Merge diagnostics + if (diag) + diag->append(localDiag); return values; } diff --git a/test/diagnostics.cpp b/test/diagnostics.cpp index ec2e2580..cec4a87d 100644 --- a/test/diagnostics.cpp +++ b/test/diagnostics.cpp @@ -27,16 +27,12 @@ TEST_CASE("UnknownField", "[diag.unknown-field]") { EXPECT_DIAGNOSTIC_MESSAGE_CONTAINING("**.not_a_field > 0", "No matches for field"); } -TEST_CASE("ComparatorTypeMissmatch", "[diag.comparator-type-missmatch]") { - EXPECT_DIAGNOSTIC_MESSAGE_CONTAINING("['string'] > 123", "All values compared to true. Left hand types are string"); -} - -TEST_CASE("AnyUnknownField", "[diag.any-suppress-if-any]") { - EXPECT_N_DIAGNOSTIC_MESSAGES("any(**.not_a_field > 0 or **.number = 123)", 0); +TEST_CASE("UnknownFields", "[diag.unknown-fields]") { + EXPECT_N_DIAGNOSTIC_MESSAGES("**.not_a_field > 0 or **.not_b_field < 1 or **.not_c_field != 31", 6); } -TEST_CASE("OrShortCircuit", "[diag.suppress-short-circuitted-or]") { - EXPECT_N_DIAGNOSTIC_MESSAGES("**.number = 123 or **.not_a_field > 0", 0); +TEST_CASE("ComparatorTypeMissmatch", "[diag.comparator-type-missmatch]") { + EXPECT_DIAGNOSTIC_MESSAGE_CONTAINING("['string'] > 123", "All values compared to false. Left hand types are string"); } TEST_CASE("DiagnosticsSerialization", "[diag.serialization]") { @@ -46,7 +42,7 @@ TEST_CASE("DiagnosticsSerialization", "[diag.serialization]") { // Create two diagnostic messages. Diagnostics originalDiag; - auto ast = compile(env, "**.number == \"string\" or **.number == \"string\""); + auto ast = compile(env, R"(**.number == "string" or **.number == "string")"); REQUIRE(ast.has_value()); auto root = model.value()->root(0); REQUIRE(root); From a52ceeeeccf185ac096ddd52c89d0d4435cee423 Mon Sep 17 00:00:00 2001 From: Johannes Wolf Date: Wed, 11 Mar 2026 00:57:38 +0100 Subject: [PATCH 3/5] expr: Make eval Const Again Also remove the unused clone() function. --- include/simfil/diagnostics.h | 16 +-- include/simfil/expression-visitor.h | 48 ++++---- include/simfil/expression.h | 16 +-- include/simfil/sourcelocation.h | 1 - src/completion.cpp | 45 ++----- src/completion.h | 23 ++-- src/diagnostics.cpp | 22 ++-- src/expression-visitor.cpp | 90 +++++++------- src/expressions.cpp | 180 ++++++---------------------- src/expressions.h | 136 +++++++-------------- src/parser.cpp | 9 +- src/simfil.cpp | 6 +- test/simfil.cpp | 5 +- 13 files changed, 202 insertions(+), 395 deletions(-) diff --git a/include/simfil/diagnostics.h b/include/simfil/diagnostics.h index 95ad7c5d..5ebddc20 100644 --- a/include/simfil/diagnostics.h +++ b/include/simfil/diagnostics.h @@ -9,10 +9,9 @@ #include #include -#include #include #include -#include +#include namespace simfil { @@ -76,6 +75,11 @@ struct Diagnostics auto write(std::ostream& stream) const -> tl::expected; auto read(std::istream& stream) -> tl::expected; + /** + * Build the exprIndex_ map for the AST. + */ + auto prepareIndices(const Expr& ast) -> void; + /** ExprId to diagnostics data index mapping. */ std::vector exprIndex_; @@ -86,18 +90,14 @@ struct Diagnostics std::vector comparisonData_; private: - friend auto eval(Environment&, const AST&, const ModelNode&, Diagnostics*) -> tl::expected, Error>; friend auto diagnostics(Environment& env, const AST& ast, const Diagnostics& diag) -> tl::expected, Error>; - /** - * Build the exprIndex_ map for the AST. - */ - auto prepareIndizes(Expr& ast) -> void; - /** * Build messages from this objecst diagnostics data. */ auto buildMessages(Environment& env, const AST& ast) const -> std::vector; + + mutable std::mutex mtx_; }; namespace detail diff --git a/include/simfil/expression-visitor.h b/include/simfil/expression-visitor.h index 443f0da2..d01ccdfd 100644 --- a/include/simfil/expression-visitor.h +++ b/include/simfil/expression-visitor.h @@ -2,7 +2,7 @@ #pragma once -#include +#include namespace simfil { @@ -42,29 +42,29 @@ class ExprVisitor ExprVisitor(); virtual ~ExprVisitor(); - virtual void visit(Expr& expr); - virtual void visit(WildcardExpr& expr); - virtual void visit(AnyChildExpr& expr); - virtual void visit(MultiConstExpr& expr); - virtual void visit(ConstExpr& expr); - virtual void visit(SubscriptExpr& expr); - virtual void visit(SubExpr& expr); - virtual void visit(AnyExpr& expr); - virtual void visit(EachExpr& expr); - virtual void visit(CallExpression& expr); - virtual void visit(PathExpr& expr); - virtual void visit(FieldExpr& expr); - virtual void visit(UnpackExpr& expr); - virtual void visit(UnaryWordOpExpr& expr); - virtual void visit(BinaryWordOpExpr& expr); - virtual void visit(AndExpr& expr); - virtual void visit(OrExpr& expr); - virtual void visit(BinaryExpr& expr); - virtual void visit(BinaryExpr& expr); - virtual void visit(BinaryExpr& expr); - virtual void visit(BinaryExpr& expr); - virtual void visit(BinaryExpr& expr); - virtual void visit(BinaryExpr& expr); + virtual void visit(const Expr& expr); + virtual void visit(const WildcardExpr& expr); + virtual void visit(const AnyChildExpr& expr); + virtual void visit(const MultiConstExpr& expr); + virtual void visit(const ConstExpr& expr); + virtual void visit(const SubscriptExpr& expr); + virtual void visit(const SubExpr& expr); + virtual void visit(const AnyExpr& expr); + virtual void visit(const EachExpr& expr); + virtual void visit(const CallExpression& expr); + virtual void visit(const PathExpr& expr); + virtual void visit(const FieldExpr& expr); + virtual void visit(const UnpackExpr& expr); + virtual void visit(const UnaryWordOpExpr& expr); + virtual void visit(const BinaryWordOpExpr& expr); + virtual void visit(const AndExpr& expr); + virtual void visit(const OrExpr& expr); + virtual void visit(const BinaryExpr& expr); + virtual void visit(const BinaryExpr& expr); + virtual void visit(const BinaryExpr& expr); + virtual void visit(const BinaryExpr& expr); + virtual void visit(const BinaryExpr& expr); + virtual void visit(const BinaryExpr& expr); protected: /* Returns the index of the current expression */ diff --git a/include/simfil/expression.h b/include/simfil/expression.h index 4b71a602..39692f9d 100644 --- a/include/simfil/expression.h +++ b/include/simfil/expression.h @@ -59,7 +59,7 @@ class Expr /* Debug */ virtual auto toString() const -> std::string = 0; - auto eval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected + auto eval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { if (ctx.canceled()) return Result::Stop; @@ -75,7 +75,7 @@ class Expr return ieval(ctx, val, res); } - auto eval(Context ctx, Value&& val, const ResultFn& res) -> tl::expected + auto eval(Context ctx, Value&& val, const ResultFn& res) const -> tl::expected { if (ctx.canceled()) return Result::Stop; @@ -90,12 +90,8 @@ class Expr return ieval(ctx, std::move(val), res); } - /* Recursive clone */ - [[nodiscard]] - virtual auto clone() const -> std::unique_ptr = 0; - /* Accept expression visitor */ - virtual auto accept(ExprVisitor& v) -> void = 0; + virtual auto accept(ExprVisitor& v) const -> void = 0; /* Source location the expression got parsed from */ [[nodiscard]] @@ -106,10 +102,10 @@ class Expr private: /* Abstract evaluation implementation */ - virtual auto ieval(Context ctx, const Value& value, const ResultFn& result) -> tl::expected = 0; + virtual auto ieval(Context ctx, const Value& value, const ResultFn& result) const -> tl::expected = 0; /* Move-optimized evaluation implementation */ - virtual auto ieval(Context ctx, Value&& value, const ResultFn& result) -> tl::expected + virtual auto ieval(Context ctx, Value&& value, const ResultFn& result) const -> tl::expected { return ieval(ctx, value, result); } @@ -130,7 +126,7 @@ class AST ~AST(); - auto expr() const -> Expr& + auto expr() const -> const Expr& { return *expr_; } diff --git a/include/simfil/sourcelocation.h b/include/simfil/sourcelocation.h index e59d049c..aff7a581 100644 --- a/include/simfil/sourcelocation.h +++ b/include/simfil/sourcelocation.h @@ -19,7 +19,6 @@ struct SourceLocation SourceLocation(const SourceLocation&) = default; auto operator==(const SourceLocation& o) const -> bool = default; - auto operator!=(const SourceLocation& o) const -> bool = default; }; } diff --git a/src/completion.cpp b/src/completion.cpp index 9c3c06ac..83f3be14 100644 --- a/src/completion.cpp +++ b/src/completion.cpp @@ -137,7 +137,7 @@ auto CompletionFieldOrWordExpr::type() const -> Type return Type::FIELD; } -auto CompletionFieldOrWordExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto CompletionFieldOrWordExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { if (ctx.phase == Context::Phase::Compilation) return res(ctx, Value::undef()); @@ -191,12 +191,7 @@ auto CompletionFieldOrWordExpr::toString() const -> std::string return prefix_; } -auto CompletionFieldOrWordExpr::clone() const -> std::unique_ptr -{ - throw std::runtime_error("Cannot clone CompletionFieldOrWordExpr"); -} - -auto CompletionFieldOrWordExpr::accept(ExprVisitor& v) -> void +auto CompletionFieldOrWordExpr::accept(ExprVisitor& v) const -> void { v.visit(*this); } @@ -216,7 +211,7 @@ struct FindExpressionRange : ExprVisitor using ExprVisitor::visit; - void visit(Expr& expr) override + void visit(const Expr& expr) override { ExprVisitor::visit(expr); @@ -255,7 +250,7 @@ auto CompletionAndExpr::type() const -> Type return Type::VALUE; } -auto CompletionAndExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto CompletionAndExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { if (left_) (void)left_->eval(ctx, val, LambdaResultFn([](const Context&, const Value&) { @@ -270,16 +265,11 @@ auto CompletionAndExpr::ieval(Context ctx, const Value& val, const ResultFn& res return Result::Continue; } -void CompletionAndExpr::accept(ExprVisitor& v) +void CompletionAndExpr::accept(ExprVisitor& v) const { v.visit(*this); } -auto CompletionAndExpr::clone() const -> ExprPtr -{ - throw std::runtime_error("Cannot clone CompletionAndExpr"); -} - auto CompletionAndExpr::toString() const -> std::string { if (left_ && right_) @@ -316,7 +306,7 @@ auto CompletionOrExpr::type() const -> Type return Type::VALUE; } -auto CompletionOrExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto CompletionOrExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { if (left_) (void)left_->eval(ctx, val, LambdaResultFn([](const Context&, const Value&) { @@ -331,16 +321,11 @@ auto CompletionOrExpr::ieval(Context ctx, const Value& val, const ResultFn& res) return Result::Continue; } -void CompletionOrExpr::accept(ExprVisitor& v) +void CompletionOrExpr::accept(ExprVisitor& v) const { v.visit(*this); } -auto CompletionOrExpr::clone() const -> ExprPtr -{ - throw std::runtime_error("Cannot clone CompletionOrExpr"); -} - auto CompletionOrExpr::toString() const -> std::string { if (left_ && right_) @@ -368,7 +353,7 @@ auto CompletionWordExpr::constant() const -> bool return true; } -auto CompletionWordExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto CompletionWordExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { if (ctx.phase == Context::Phase::Compilation) return res(ctx, Value::undef()); @@ -384,12 +369,7 @@ auto CompletionWordExpr::toString() const -> std::string return prefix_; } -auto CompletionWordExpr::clone() const -> std::unique_ptr -{ - throw std::runtime_error("Cannot clone CompletionWordExpr"); -} - -auto CompletionWordExpr::accept(ExprVisitor& v) -> void +auto CompletionWordExpr::accept(ExprVisitor& v) const -> void { v.visit(*this); } @@ -399,12 +379,7 @@ auto CompletionConstExpr::constant() const -> bool return false; } -auto CompletionConstExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), value_); -} - -auto CompletionConstExpr::ieval(Context ctx, const Value&, const ResultFn& res) -> tl::expected +auto CompletionConstExpr::ieval(Context ctx, const Value&, const ResultFn& res) const -> tl::expected { if (ctx.phase == Context::Compilation) return res(ctx, Value::undef()); diff --git a/src/completion.h b/src/completion.h index 114f5e1b..8f9c51ba 100644 --- a/src/completion.h +++ b/src/completion.h @@ -48,9 +48,8 @@ class CompletionFieldOrWordExpr : public Expr CompletionFieldOrWordExpr(ExprId id, std::string prefix, Completion* comp, const Token& token, bool inPath); auto type() const -> Type override; - auto ieval(Context ctx, const Value& value, const ResultFn& result) -> tl::expected override; - auto clone() const -> std::unique_ptr override; - auto accept(ExprVisitor& v) -> void override; + auto ieval(Context ctx, const Value& value, const ResultFn& result) const -> tl::expected override; + auto accept(ExprVisitor& v) const -> void override; auto toString() const -> std::string override; std::string prefix_; @@ -64,9 +63,8 @@ class CompletionAndExpr : public Expr CompletionAndExpr(ExprId id, ExprPtr left, ExprPtr right, const Completion* comp); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - void accept(ExprVisitor& v) override; - auto clone() const -> ExprPtr override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; ExprPtr left_, right_; @@ -78,9 +76,8 @@ class CompletionOrExpr : public Expr CompletionOrExpr(ExprId id, ExprPtr left, ExprPtr right, const Completion* comp); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - void accept(ExprVisitor& v) override; - auto clone() const -> ExprPtr override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; ExprPtr left_, right_; @@ -93,9 +90,8 @@ class CompletionWordExpr : public Expr auto type() const -> Type override; auto constant() const -> bool override; - auto ieval(Context ctx, const Value& value, const ResultFn& result) -> tl::expected override; - auto clone() const -> std::unique_ptr override; - auto accept(ExprVisitor& v) -> void override; + auto ieval(Context ctx, const Value& value, const ResultFn& result) const -> tl::expected override; + auto accept(ExprVisitor& v) const -> void override; auto toString() const -> std::string override; std::string prefix_; @@ -112,8 +108,7 @@ class CompletionConstExpr : public ConstExpr using ConstExpr::ConstExpr; auto constant() const -> bool override; - auto clone() const -> ExprPtr override; - auto ieval(Context ctx, const Value&, const ResultFn& res) -> tl::expected override; + auto ieval(Context ctx, const Value&, const ResultFn& res) const -> tl::expected override; }; } diff --git a/src/diagnostics.cpp b/src/diagnostics.cpp index a6718c4a..c8f7d89e 100644 --- a/src/diagnostics.cpp +++ b/src/diagnostics.cpp @@ -108,7 +108,7 @@ auto Diagnostics::write(std::ostream& stream) const -> tl::expected using OutputAdapter = bitsery::OutputStreamAdapter; bitsery::Serializer serializer{stream}; - //auto lock = data->lock(); + std::unique_lock lock(mtx_); serializer.object(*this); if (!stream.good()) { @@ -123,7 +123,7 @@ auto Diagnostics::read(std::istream& stream) -> tl::expected using InputAdapter = bitsery::InputStreamAdapter; bitsery::Deserializer deserializer{stream}; - //auto lock = data->lock(); + std::unique_lock lock(mtx_); deserializer.object(*this); if (!stream.good()) { @@ -165,7 +165,7 @@ auto Diagnostics::buildMessages(Environment& env, const AST& ast) const -> std:: } const auto intersection = leftTypes.flags & rightTypes.flags; - if (true || intersection.none()) { + if (intersection.none()) { const auto allTrue = data.trueResults > 0 && data.falseResults == 0; const auto allFalse = data.falseResults > 0 && data.trueResults == 0; const auto prefix = @@ -182,7 +182,7 @@ auto Diagnostics::buildMessages(Environment& env, const AST& ast) const -> std:: return messages; } -auto Diagnostics::prepareIndizes(Expr& ast) -> void +auto Diagnostics::prepareIndices(const Expr& ast) -> void { struct Visitor : ExprVisitor { @@ -197,7 +197,7 @@ auto Diagnostics::prepareIndizes(Expr& ast) -> void using ExprVisitor::visit; - auto visit(FieldExpr& e) -> void override { + auto visit(const FieldExpr& e) -> void override { ExprVisitor::visit(e); if (e.id() >= indices_.size()) indices_.resize(e.id() + 1); @@ -211,32 +211,32 @@ auto Diagnostics::prepareIndizes(Expr& ast) -> void indices_[e.id()] = comparisonIndex_++; } - auto visit(BinaryExpr& e) -> void override { + auto visit(const BinaryExpr& e) -> void override { ExprVisitor::visit(e); visitComparisonOperator(e); } - auto visit(BinaryExpr& e) -> void override { + auto visit(const BinaryExpr& e) -> void override { ExprVisitor::visit(e); visitComparisonOperator(e); } - auto visit(BinaryExpr& e) -> void override { + auto visit(const BinaryExpr& e) -> void override { ExprVisitor::visit(e); visitComparisonOperator(e); } - auto visit(BinaryExpr& e) -> void override { + auto visit(const BinaryExpr& e) -> void override { ExprVisitor::visit(e); visitComparisonOperator(e); } - auto visit(BinaryExpr& e) -> void override { + auto visit(const BinaryExpr& e) -> void override { ExprVisitor::visit(e); visitComparisonOperator(e); } - auto visit(BinaryExpr& e) -> void override { + auto visit(const BinaryExpr& e) -> void override { ExprVisitor::visit(e); visitComparisonOperator(e); } diff --git a/src/expression-visitor.cpp b/src/expression-visitor.cpp index fc02a644..ec5a2acf 100644 --- a/src/expression-visitor.cpp +++ b/src/expression-visitor.cpp @@ -10,34 +10,34 @@ namespace simfil ExprVisitor::ExprVisitor() = default; ExprVisitor::~ExprVisitor() = default; -void ExprVisitor::visit(Expr& e) +void ExprVisitor::visit(const Expr& e) { index_++; } -void ExprVisitor::visit(WildcardExpr& expr) +void ExprVisitor::visit(const WildcardExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); } -void ExprVisitor::visit(AnyChildExpr& expr) +void ExprVisitor::visit(const AnyChildExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); } -void ExprVisitor::visit(MultiConstExpr& expr) +void ExprVisitor::visit(const MultiConstExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); } -void ExprVisitor::visit(ConstExpr& expr) +void ExprVisitor::visit(const ConstExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); } -void ExprVisitor::visit(SubscriptExpr& expr) +void ExprVisitor::visit(const SubscriptExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); if (expr.left_) expr.left_->accept(*this); @@ -45,9 +45,9 @@ void ExprVisitor::visit(SubscriptExpr& expr) expr.index_->accept(*this); } -void ExprVisitor::visit(SubExpr& expr) +void ExprVisitor::visit(const SubExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); if (expr.left_) expr.left_->accept(*this); @@ -55,36 +55,36 @@ void ExprVisitor::visit(SubExpr& expr) expr.sub_->accept(*this); } -void ExprVisitor::visit(AnyExpr& expr) +void ExprVisitor::visit(const AnyExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); for (const auto& arg : expr.args_) if (arg) arg->accept(*this); } -void ExprVisitor::visit(EachExpr& expr) +void ExprVisitor::visit(const EachExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); for (const auto& arg : expr.args_) if (arg) arg->accept(*this); } -void ExprVisitor::visit(CallExpression& expr) +void ExprVisitor::visit(const CallExpression& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); for (const auto& arg : expr.args_) if (arg) arg->accept(*this); } -void ExprVisitor::visit(PathExpr& expr) +void ExprVisitor::visit(const PathExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); if (expr.left_) expr.left_->accept(*this); @@ -92,30 +92,30 @@ void ExprVisitor::visit(PathExpr& expr) expr.right_->accept(*this); } -void ExprVisitor::visit(FieldExpr& expr) +void ExprVisitor::visit(const FieldExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); } -void ExprVisitor::visit(UnpackExpr& expr) +void ExprVisitor::visit(const UnpackExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); if (expr.sub_) expr.sub_->accept(*this); } -void ExprVisitor::visit(UnaryWordOpExpr& expr) +void ExprVisitor::visit(const UnaryWordOpExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); if (expr.left_) expr.left_->accept(*this); } -void ExprVisitor::visit(BinaryWordOpExpr& expr) +void ExprVisitor::visit(const BinaryWordOpExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); if (expr.left_) expr.left_->accept(*this); @@ -123,9 +123,9 @@ void ExprVisitor::visit(BinaryWordOpExpr& expr) expr.right_->accept(*this); } -void ExprVisitor::visit(AndExpr& expr) +void ExprVisitor::visit(const AndExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); if (expr.left_) expr.left_->accept(*this); @@ -133,9 +133,9 @@ void ExprVisitor::visit(AndExpr& expr) expr.right_->accept(*this); } -void ExprVisitor::visit(OrExpr& expr) +void ExprVisitor::visit(const OrExpr& expr) { - visit(static_cast(expr)); + visit(static_cast(expr)); if (expr.left_) expr.left_->accept(*this); @@ -143,34 +143,34 @@ void ExprVisitor::visit(OrExpr& expr) expr.right_->accept(*this); } -void ExprVisitor::visit(BinaryExpr& e) +void ExprVisitor::visit(const BinaryExpr& e) { - visit(static_cast(e)); + visit(static_cast(e)); } -void ExprVisitor::visit(BinaryExpr& e) +void ExprVisitor::visit(const BinaryExpr& e) { - visit(static_cast(e)); + visit(static_cast(e)); } -void ExprVisitor::visit(BinaryExpr& e) +void ExprVisitor::visit(const BinaryExpr& e) { - visit(static_cast(e)); + visit(static_cast(e)); } -void ExprVisitor::visit(BinaryExpr& e) +void ExprVisitor::visit(const BinaryExpr& e) { - visit(static_cast(e)); + visit(static_cast(e)); } -void ExprVisitor::visit(BinaryExpr& e) +void ExprVisitor::visit(const BinaryExpr& e) { - visit(static_cast(e)); + visit(static_cast(e)); } -void ExprVisitor::visit(BinaryExpr& e) +void ExprVisitor::visit(const BinaryExpr& e) { - visit(static_cast(e)); + visit(static_cast(e)); } auto ExprVisitor::index() const -> std::size_t diff --git a/src/expressions.cpp b/src/expressions.cpp index 6d63c6fb..82d77d2b 100644 --- a/src/expressions.cpp +++ b/src/expressions.cpp @@ -86,7 +86,7 @@ auto WildcardExpr::type() const -> Type return Type::PATH; } -auto WildcardExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected +auto WildcardExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) const -> tl::expected { if (ctx.phase == Context::Phase::Compilation) return ores(ctx, Value::undef()); @@ -133,12 +133,7 @@ auto WildcardExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) -> return r; } -auto WildcardExpr::clone() const -> ExprPtr -{ - return std::make_unique(id()); -} - -void WildcardExpr::accept(ExprVisitor& v) +void WildcardExpr::accept(ExprVisitor& v) const { v.visit(*this); } @@ -157,7 +152,7 @@ auto AnyChildExpr::type() const -> Type return Type::PATH; } -auto AnyChildExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto AnyChildExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { if (ctx.phase == Context::Phase::Compilation) return res(ctx, Value::undef()); @@ -181,12 +176,7 @@ auto AnyChildExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> return Result::Continue; } -auto AnyChildExpr::clone() const -> ExprPtr -{ - return std::make_unique(id()); -} - -void AnyChildExpr::accept(ExprVisitor& v) +void AnyChildExpr::accept(ExprVisitor& v) const { v.visit(*this); } @@ -211,12 +201,12 @@ auto FieldExpr::type() const -> Type return Type::FIELD; } -auto FieldExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto FieldExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { return ieval(ctx, Value{val}, res); } -auto FieldExpr::ieval(Context ctx, Value&& val, const ResultFn& res) -> tl::expected +auto FieldExpr::ieval(Context ctx, Value&& val, const ResultFn& res) const -> tl::expected { Diagnostics::FieldExprData* diag = nullptr; if (ctx.diag) @@ -262,12 +252,7 @@ auto FieldExpr::ieval(Context ctx, Value&& val, const ResultFn& res) -> tl::expe return res(ctx, Value::null()); } -auto FieldExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), name_); -} - -void FieldExpr::accept(ExprVisitor& v) +void FieldExpr::accept(ExprVisitor& v) const { v.visit(*this); } @@ -297,7 +282,7 @@ auto MultiConstExpr::constant() const -> bool return true; } -auto MultiConstExpr::ieval(Context ctx, const Value&, const ResultFn& res) -> tl::expected +auto MultiConstExpr::ieval(Context ctx, const Value&, const ResultFn& res) const -> tl::expected { for (const auto& v : values_) { auto r = res(ctx, v); @@ -309,12 +294,7 @@ auto MultiConstExpr::ieval(Context ctx, const Value&, const ResultFn& res) -> tl return Result::Continue; } -auto MultiConstExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), values_); -} - -void MultiConstExpr::accept(ExprVisitor& v) +void MultiConstExpr::accept(ExprVisitor& v) const { v.visit(*this); } @@ -343,17 +323,12 @@ auto ConstExpr::constant() const -> bool return true; } -auto ConstExpr::ieval(Context ctx, const Value&, const ResultFn& res) -> tl::expected +auto ConstExpr::ieval(Context ctx, const Value&, const ResultFn& res) const -> tl::expected { return res(ctx, value_); } -auto ConstExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), value_); -} - -void ConstExpr::accept(ExprVisitor& v) +void ConstExpr::accept(ExprVisitor& v) const { v.visit(*this); } @@ -381,7 +356,7 @@ auto SubscriptExpr::type() const -> Type return Type::SUBSCRIPT; } -auto SubscriptExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected +auto SubscriptExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) const -> tl::expected { auto res = CountedResultFn(ores, ctx); auto r = left_->eval(ctx, val, LambdaResultFn([this, &val, &res](Context ctx, const Value& lval) { @@ -420,12 +395,7 @@ auto SubscriptExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) - return r; } -auto SubscriptExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), left_->clone(), index_->clone()); -} - -void SubscriptExpr::accept(ExprVisitor& v) +void SubscriptExpr::accept(ExprVisitor& v) const { v.visit(*this); } @@ -446,12 +416,12 @@ auto SubExpr::type() const -> Type return Type::SUBEXPR; } -auto SubExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected +auto SubExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) const -> tl::expected { return ieval(ctx, Value{val}, ores); } -auto SubExpr::ieval(Context ctx, Value&& val, const ResultFn& ores) -> tl::expected +auto SubExpr::ieval(Context ctx, Value&& val, const ResultFn& ores) const -> tl::expected { /* Do not return null unless we have _no_ matching value. */ auto res = CountedResultFn(ores, ctx); @@ -478,12 +448,7 @@ auto SubExpr::toString() const -> std::string return fmt::format("(sub {} {})", left_->toString(), sub_->toString()); } -auto SubExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), left_->clone(), sub_->clone()); -} - -void SubExpr::accept(ExprVisitor& v) +void SubExpr::accept(ExprVisitor& v) const { v.visit(*this); } @@ -498,7 +463,7 @@ auto AnyExpr::type() const -> Type return Type::VALUE; } -auto AnyExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto AnyExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { auto subctx = ctx; auto result = false; /* At least one value is true */ @@ -524,25 +489,10 @@ auto AnyExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::e if (undef) return res(subctx, Value::undef()); - if (result) - ++trueResults_; - else - ++falseResults_; return res(subctx, Value::make(result)); } -auto AnyExpr::clone() const -> ExprPtr -{ - std::vector clonedArgs; - clonedArgs.resize(args_.size()); - std::transform(args_.cbegin(), args_.cend(), std::make_move_iterator(clonedArgs.begin()), [](const auto& exp) { - return exp->clone(); - }); - - return std::make_unique(id(), std::move(clonedArgs)); -} - -auto AnyExpr::accept(ExprVisitor& v) -> void +auto AnyExpr::accept(ExprVisitor& v) const -> void { v.visit(*this); } @@ -569,7 +519,7 @@ auto EachExpr::type() const -> Type return Type::VALUE; } -auto EachExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto EachExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { auto subctx = ctx; auto result = true; /* All values are true */ @@ -594,25 +544,10 @@ auto EachExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl:: if (undef) return res(subctx, Value::undef()); - if (result) - ++trueResults_; - else - ++falseResults_; return res(subctx, Value::make(result)); } -auto EachExpr::clone() const -> ExprPtr -{ - std::vector clonedArgs; - clonedArgs.resize(args_.size()); - std::transform(args_.cbegin(), args_.cend(), std::make_move_iterator(clonedArgs.begin()), [](const auto& exp) { - return exp->clone(); - }); - - return std::make_unique(id(), std::move(clonedArgs)); -} - -auto EachExpr::accept(ExprVisitor& v) -> void +auto EachExpr::accept(ExprVisitor& v) const -> void { v.visit(*this); } @@ -640,12 +575,12 @@ auto CallExpression::type() const -> Type return Type::VALUE; } -auto CallExpression::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto CallExpression::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { return ieval(ctx, Value{val}, res); } -auto CallExpression::ieval(Context ctx, Value&& val, const ResultFn& res) -> tl::expected +auto CallExpression::ieval(Context ctx, Value&& val, const ResultFn& res) const -> tl::expected { if (!fn_) [[unlikely]] { fn_ = ctx.env->findFunction(name_); @@ -666,18 +601,7 @@ auto CallExpression::ieval(Context ctx, Value&& val, const ResultFn& res) -> tl: return result; } -auto CallExpression::clone() const -> ExprPtr -{ - std::vector clonedArgs; - clonedArgs.resize(args_.size()); - std::transform(args_.cbegin(), args_.cend(), std::make_move_iterator(clonedArgs.begin()), [](const auto& exp) { - return exp->clone(); - }); - - return std::make_unique(id(), name_, std::move(clonedArgs)); -} - -void CallExpression::accept(ExprVisitor& v) +void CallExpression::accept(ExprVisitor& v) const { v.visit(*this); } @@ -708,12 +632,12 @@ auto PathExpr::type() const -> Type return Type::PATH; } -auto PathExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected +auto PathExpr::ieval(Context ctx, const Value& val, const ResultFn& ores) const -> tl::expected { return ieval(ctx, Value{val}, ores); } -auto PathExpr::ieval(Context ctx, Value&& val, const ResultFn& ores) -> tl::expected +auto PathExpr::ieval(Context ctx, Value&& val, const ResultFn& ores) const -> tl::expected { auto res = CountedResultFn(ores, ctx); @@ -724,8 +648,6 @@ auto PathExpr::ieval(Context ctx, Value&& val, const ResultFn& ores) -> tl::expe if (v.isa(ValueType::Null) && !v.node()) return Result::Continue; - ++hits_; - return right_->eval(ctx, std::move(v), LambdaResultFn([this, &res](Context ctx, Value&& vv) -> tl::expected { if (vv.isa(ValueType::Undef)) return Result::Continue; @@ -740,12 +662,7 @@ auto PathExpr::ieval(Context ctx, Value&& val, const ResultFn& ores) -> tl::expe return r; }; -auto PathExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), left_->clone(), right_->clone()); -} - -void PathExpr::accept(ExprVisitor& v) +void PathExpr::accept(ExprVisitor& v) const { v.visit(*this); } @@ -765,7 +682,7 @@ auto UnpackExpr::type() const -> Type return Type::VALUE; } -auto UnpackExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto UnpackExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { auto anyval = false; auto r = sub_->eval(ctx, val, LambdaResultFn([&res, &anyval](Context ctx, Value&& v) -> tl::expected { @@ -795,12 +712,7 @@ auto UnpackExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl return r; } -auto UnpackExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), sub_->clone()); -} - -void UnpackExpr::accept(ExprVisitor& v) +void UnpackExpr::accept(ExprVisitor& v) const { v.visit(*this); } @@ -821,7 +733,7 @@ auto UnaryWordOpExpr::type() const -> Type return Type::VALUE; } -auto UnaryWordOpExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto UnaryWordOpExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { return left_->eval(ctx, val, LambdaResultFn([this, &res](const Context& ctx, Value&& val) -> tl::expected { if (val.isa(ValueType::Undef)) @@ -839,16 +751,11 @@ auto UnaryWordOpExpr::ieval(Context ctx, const Value& val, const ResultFn& res) })); } -void UnaryWordOpExpr::accept(ExprVisitor& v) +void UnaryWordOpExpr::accept(ExprVisitor& v) const { v.visit(*this); } -auto UnaryWordOpExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), ident_, left_->clone()); -} - auto UnaryWordOpExpr::toString() const -> std::string { return fmt::format("({} {})", ident_, left_->toString()); @@ -866,7 +773,7 @@ auto BinaryWordOpExpr::type() const -> Type return Type::VALUE; } -auto BinaryWordOpExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto BinaryWordOpExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { return left_->eval(ctx, val, LambdaResultFn([this, &res, &val](const Context& ctx, const Value& lval) { return right_->eval(ctx, val, LambdaResultFn([this, &res, &lval](const Context& ctx, const Value& rval) -> tl::expected { @@ -894,16 +801,11 @@ auto BinaryWordOpExpr::ieval(Context ctx, const Value& val, const ResultFn& res) })); } -void BinaryWordOpExpr::accept(ExprVisitor& v) +void BinaryWordOpExpr::accept(ExprVisitor& v) const { v.visit(*this); } -auto BinaryWordOpExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), ident_, left_->clone(), right_->clone()); -} - auto BinaryWordOpExpr::toString() const -> std::string { return fmt::format("({} {} {})", ident_, left_->toString(), right_->toString()); @@ -923,7 +825,7 @@ auto AndExpr::type() const -> Type return Type::VALUE; } -auto AndExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto AndExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { /* Operator and behaves like in lua: * 'a and b' returns a if 'not a?' else b is returned */ @@ -943,16 +845,11 @@ auto AndExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::e })); } -void AndExpr::accept(ExprVisitor& v) +void AndExpr::accept(ExprVisitor& v) const { v.visit(*this); } -auto AndExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), left_->clone(), right_->clone()); -} - auto AndExpr::toString() const -> std::string { return fmt::format("(and {} {})", left_->toString(), right_->toString()); @@ -972,7 +869,7 @@ auto OrExpr::type() const -> Type return Type::VALUE; } -auto OrExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected +auto OrExpr::ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected { /* Operator or behaves like in lua: * 'a or b' returns a if 'a?' else b is returned */ @@ -993,16 +890,11 @@ auto OrExpr::ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::ex } -void OrExpr::accept(ExprVisitor& v) +void OrExpr::accept(ExprVisitor& v) const { v.visit(*this); } -auto OrExpr::clone() const -> ExprPtr -{ - return std::make_unique(id(), left_->clone(), right_->clone()); -} - auto OrExpr::toString() const -> std::string { return fmt::format("(or {} {})", left_->toString(), right_->toString()); diff --git a/src/expressions.h b/src/expressions.h index 5faf771a..c2f5e98e 100644 --- a/src/expressions.h +++ b/src/expressions.h @@ -18,9 +18,8 @@ class WildcardExpr : public Expr explicit WildcardExpr(ExprId); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value& val, const ResultFn& ores) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; }; @@ -33,9 +32,8 @@ class AnyChildExpr : public Expr explicit AnyChildExpr(ExprId); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; }; @@ -46,14 +44,13 @@ class FieldExpr : public Expr FieldExpr(ExprId id, std::string name, const Token& token); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - auto ieval(Context ctx, Value&& val, const ResultFn& res) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + auto ieval(Context ctx, Value&& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; std::string name_; - StringId nameId_ = {}; + mutable StringId nameId_ = {}; }; class MultiConstExpr : public Expr @@ -67,9 +64,8 @@ class MultiConstExpr : public Expr auto type() const -> Type override; auto constant() const -> bool override; - auto ieval(Context ctx, const Value&, const ResultFn& res) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value&, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; const std::vector values_; @@ -88,9 +84,8 @@ class ConstExpr : public Expr auto type() const -> Type override; auto constant() const -> bool override; - auto ieval(Context ctx, const Value&, const ResultFn& res) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value&, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; auto value() const -> const Value&; @@ -105,9 +100,8 @@ class SubscriptExpr : public Expr SubscriptExpr(ExprId id, ExprPtr left, ExprPtr index); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value& val, const ResultFn& ores) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; ExprPtr left_; @@ -120,11 +114,10 @@ class SubExpr : public Expr SubExpr(ExprId id, ExprPtr left, ExprPtr sub); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected override; - auto ieval(Context ctx, Value&& val, const ResultFn& ores) -> tl::expected override; + auto ieval(Context ctx, const Value& val, const ResultFn& ores) const -> tl::expected override; + auto ieval(Context ctx, Value&& val, const ResultFn& ores) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; ExprPtr left_, sub_; }; @@ -135,16 +128,11 @@ class AnyExpr : public Expr AnyExpr(ExprId id, std::vector args); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; std::vector args_; - - /* Runtime Data */ - std::uint32_t trueResults_ = 0; - std::uint32_t falseResults_ = 0; }; class EachExpr : public Expr @@ -153,16 +141,11 @@ class EachExpr : public Expr EachExpr(ExprId id, std::vector args); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; std::vector args_; - - /* Runtime Data */ - std::uint32_t trueResults_ = 0; - std::uint32_t falseResults_ = 0; }; class CallExpression : public Expr @@ -171,15 +154,14 @@ class CallExpression : public Expr CallExpression(ExprId id, std::string name, std::vector args); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - auto ieval(Context ctx, Value&& val, const ResultFn& res) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + auto ieval(Context ctx, Value&& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; std::string name_; std::vector args_; - const Function* fn_ = nullptr; + mutable const Function* fn_ = nullptr; }; class PathExpr : public Expr @@ -188,16 +170,12 @@ class PathExpr : public Expr PathExpr(ExprId id, ExprPtr left, ExprPtr right); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected override; - auto ieval(Context ctx, Value&& val, const ResultFn& ores) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value& val, const ResultFn& ores) const -> tl::expected override; + auto ieval(Context ctx, Value&& val, const ResultFn& ores) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; ExprPtr left_, right_; - - /* Evaluation data */ - uint32_t hits_ = 0; }; /** Calls `unpack` onto values of type Object. Forwards the value(s) otherwise. @@ -211,9 +189,8 @@ class UnpackExpr : public Expr UnpackExpr(ExprId id, ExprPtr sub); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - auto clone() const -> ExprPtr override; - void accept(ExprVisitor& v) override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; ExprPtr sub_; @@ -236,7 +213,7 @@ class UnaryExpr : public Expr return Type::VALUE; } - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override { return sub_->eval(ctx, val, LambdaResultFn([&](Context ctx, Value vv) -> tl::expected { auto v = UnaryOperatorDispatcher::dispatch(std::move(vv)); @@ -246,17 +223,12 @@ class UnaryExpr : public Expr })); } - void accept(ExprVisitor& v) override + void accept(ExprVisitor& v) const override { v.visit(*this); sub_->accept(v); } - auto clone() const -> ExprPtr override - { - return std::make_unique(id(), sub_->clone()); - } - auto toString() const -> std::string override { return "("s + Operator::name() + " "s + sub_->toString() + ")"s; @@ -289,13 +261,10 @@ class BinaryExpr : public Expr return Type::VALUE; } - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override { return left_->eval(ctx, val, LambdaResultFn([this, &res, &val](Context ctx, Value lv) { - leftTypes_.set(lv.type); return right_->eval(ctx, val, LambdaResultFn([this, &res, &lv](Context ctx, Value rv) -> tl::expected { - rightTypes_.set(rv.type); - auto v = BinaryOperatorDispatcher::dispatch(std::move(lv), std::move(rv)); if (!v) return tl::unexpected(std::move(v.error())); @@ -304,12 +273,7 @@ class BinaryExpr : public Expr })); } - auto clone() const -> ExprPtr override - { - return std::make_unique(id(), left_->clone(), right_->clone()); - } - - void accept(ExprVisitor& v) override + void accept(ExprVisitor& v) const override { v.visit(*this); left_->accept(v); @@ -322,7 +286,6 @@ class BinaryExpr : public Expr } ExprPtr left_, right_; - TypeFlags leftTypes_, rightTypes_; }; class ComparisonExprBase : public Expr @@ -355,7 +318,7 @@ class ComparisonExpr : public ComparisonExprBase public: using ComparisonExprBase::ComparisonExprBase; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override { Diagnostics::ComparisonExprData* diag = nullptr; if (ctx.diag) @@ -388,14 +351,9 @@ class ComparisonExpr : public ComparisonExprBase })); } - auto clone() const -> ExprPtr override - { - return std::make_unique(id(), left_->clone(), right_->clone()); - } - - void accept(ExprVisitor& v) override + void accept(ExprVisitor& v) const override { - v.visit(static_cast(*this)); + v.visit(static_cast(*this)); left_->accept(v); right_->accept(v); } @@ -448,9 +406,8 @@ class UnaryWordOpExpr : public Expr UnaryWordOpExpr(ExprId id, std::string ident, ExprPtr left); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - void accept(ExprVisitor& v) override; - auto clone() const -> ExprPtr override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; std::string ident_; @@ -463,9 +420,8 @@ class BinaryWordOpExpr : public Expr BinaryWordOpExpr(ExprId id, std::string ident, ExprPtr left, ExprPtr right); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - void accept(ExprVisitor& v) override; - auto clone() const -> ExprPtr override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; std::string ident_; @@ -478,9 +434,8 @@ class AndExpr : public Expr AndExpr(ExprId id, ExprPtr left, ExprPtr right); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - void accept(ExprVisitor& v) override; - auto clone() const -> ExprPtr override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; ExprPtr left_, right_; @@ -492,9 +447,8 @@ class OrExpr : public Expr OrExpr(ExprId id, ExprPtr left, ExprPtr right); auto type() const -> Type override; - auto ieval(Context ctx, const Value& val, const ResultFn& res) -> tl::expected override; - void accept(ExprVisitor& v) override; - auto clone() const -> ExprPtr override; + auto ieval(Context ctx, const Value& val, const ResultFn& res) const -> tl::expected override; + void accept(ExprVisitor& v) const override; auto toString() const -> std::string override; ExprPtr left_, right_; diff --git a/src/parser.cpp b/src/parser.cpp index 94932529..ca7be97d 100644 --- a/src/parser.cpp +++ b/src/parser.cpp @@ -26,17 +26,12 @@ class NOOPExpr : public Expr return Type::FIELD; } - auto ieval(Context ctx, const Value& val, const ResultFn& ores) -> tl::expected override + auto ieval(Context ctx, const Value& val, const ResultFn& ores) const -> tl::expected override { return Result::Stop; } - auto clone() const -> ExprPtr override - { - return std::make_unique(id()); - } - - void accept(ExprVisitor& v) override + void accept(ExprVisitor& v) const override { v.visit(*this); } diff --git a/src/simfil.cpp b/src/simfil.cpp index 9e2e6c68..226087cb 100644 --- a/src/simfil.cpp +++ b/src/simfil.cpp @@ -932,14 +932,12 @@ auto eval(Environment& env, const AST& ast, const ModelNode& node, Diagnostics* // For thread-safety we work on a local diagnostics object that gets merged // into diag after query evaluation. Diagnostics localDiag; - localDiag.prepareIndizes(ast.expr()); + localDiag.prepareIndices(ast.expr()); Context ctx(&env, &localDiag); - auto mutableAST = &ast.expr(); // TODO: make const again! - std::vector values; - auto res = mutableAST->eval(ctx, Value::field(node), LambdaResultFn([&values](Context, Value&& value) { + auto res = ast.expr().eval(ctx, Value::field(node), LambdaResultFn([&values](const Context&, Value&& value) { values.push_back(std::move(value)); return Result::Continue; })); diff --git a/test/simfil.cpp b/test/simfil.cpp index 66db4e09..7ec6511c 100644 --- a/test/simfil.cpp +++ b/test/simfil.cpp @@ -1,6 +1,7 @@ #include "simfil/simfil.h" #include "simfil/environment.h" #include "simfil/exception-handler.h" +#include "simfil/expression-visitor.h" #include "simfil/model/json.h" #include "simfil/value.h" #include "src/expressions.h" @@ -745,7 +746,9 @@ TEST_CASE("Visit AST", "[visit.ast]") { std::optional visitedFieldName; - auto visit(FieldExpr& expr) -> void override + using ExprVisitor::visit; + + auto visit(const FieldExpr& expr) -> void override { ExprVisitor::visit(expr); From c6e04e30627655f2bec30c54ad3145d657d48848 Mon Sep 17 00:00:00 2001 From: Johannes Wolf Date: Wed, 11 Mar 2026 15:23:01 +0100 Subject: [PATCH 4/5] diagnostics: Cursor Fixes --- include/simfil/diagnostics.h | 12 ++++++++++-- src/diagnostics.cpp | 20 +++++++++++++++----- src/expressions.h | 1 + 3 files changed, 26 insertions(+), 7 deletions(-) diff --git a/include/simfil/diagnostics.h b/include/simfil/diagnostics.h index 5ebddc20..269d2733 100644 --- a/include/simfil/diagnostics.h +++ b/include/simfil/diagnostics.h @@ -7,6 +7,7 @@ #include "simfil/error.h" #include "simfil/expression.h" +#include #include #include #include @@ -23,6 +24,7 @@ struct ModelNode; /** Query Diagnostics. */ struct Diagnostics { + static constexpr std::size_t InvalidIndex = std::numeric_limits::max(); public: struct FieldExprData { @@ -38,6 +40,7 @@ struct Diagnostics SourceLocation location; TypeFlags leftTypes; TypeFlags rightTypes; + std::uint32_t evaluations = 0u; std::uint32_t falseResults = 0u; std::uint32_t trueResults = 0u; }; @@ -136,11 +139,16 @@ auto Diagnostics::get(const Expr& expr) -> DiagnosticsDataType& const auto id = expr.id(); if (exprIndex_.size() <= id) [[unlikely]] { - exprIndex_.resize(id + 1u); + exprIndex_.resize(id + 1u, Diagnostics::InvalidIndex); exprIndex_[id] = data->size(); } - const auto index = exprIndex_[id]; + auto index = exprIndex_[id]; + if (index == Diagnostics::InvalidIndex) { + exprIndex_[id] = data->size(); + index = exprIndex_[id]; + } + if (data->size() <= index) { data->resize(index + 1u); } diff --git a/src/diagnostics.cpp b/src/diagnostics.cpp index c8f7d89e..e7e289cc 100644 --- a/src/diagnostics.cpp +++ b/src/diagnostics.cpp @@ -43,6 +43,7 @@ void serialize(S& s, Diagnostics& data) s2.value4b(data.location.size); s2.object(data.leftTypes); s2.object(data.rightTypes); + s2.value4b(data.evaluations); s2.value4b(data.trueResults); s2.value4b(data.falseResults); }); @@ -60,6 +61,9 @@ Diagnostics::~Diagnostics() = default; Diagnostics& Diagnostics::append(const Diagnostics& other) { + std::unique_lock lock1(mtx_); + std::unique_lock lock2(other.mtx_); + #if !defined(NDEBUG) if (!exprIndex_.empty()) assert(std::ranges::equal(exprIndex_, other.exprIndex_)); @@ -74,7 +78,10 @@ Diagnostics& Diagnostics::append(const Diagnostics& other) auto* theirs = i < other.fieldData_.size() ? &other.fieldData_[i] : nullptr; if (ours && theirs) { - assert(ours->name == theirs->name); + if (ours->name.empty()) + ours->name = theirs->name; + if (ours->location == SourceLocation{0, 0}) + ours->location = theirs->location; ours->hits += theirs->hits; ours->evaluations += theirs->evaluations; } @@ -89,8 +96,11 @@ Diagnostics& Diagnostics::append(const Diagnostics& other) auto* theirs = i < other.comparisonData_.size() ? &other.comparisonData_[i] : nullptr; if (ours && theirs) { + if (ours->location == SourceLocation{0, 0}) + ours->location = theirs->location; ours->leftTypes.set(theirs->leftTypes); ours->rightTypes.set(theirs->rightTypes); + ours->evaluations += theirs->evaluations; ours->trueResults += theirs->trueResults; ours->falseResults += theirs->falseResults; } @@ -147,7 +157,7 @@ auto Diagnostics::buildMessages(Environment& env, const AST& ast) const -> std:: }; for (const auto& data : fieldData_) { - if (data.hits == 0u) + if (data.hits == 0 && data.evaluations > 0) addMessage(data.location, fmt::format("No matches for field '{}'", data.name)); } @@ -165,7 +175,7 @@ auto Diagnostics::buildMessages(Environment& env, const AST& ast) const -> std:: } const auto intersection = leftTypes.flags & rightTypes.flags; - if (intersection.none()) { + if (intersection.none() && data.evaluations > 0) { const auto allTrue = data.trueResults > 0 && data.falseResults == 0; const auto allFalse = data.falseResults > 0 && data.trueResults == 0; const auto prefix = @@ -200,14 +210,14 @@ auto Diagnostics::prepareIndices(const Expr& ast) -> void auto visit(const FieldExpr& e) -> void override { ExprVisitor::visit(e); if (e.id() >= indices_.size()) - indices_.resize(e.id() + 1); + indices_.resize(e.id() + 1, Diagnostics::InvalidIndex); indices_[e.id()] = fieldIndex_++; } auto visitComparisonOperator(const ComparisonExprBase& e) -> void { if (e.id() >= indices_.size()) - indices_.resize(e.id() + 1); + indices_.resize(e.id() + 1, Diagnostics::InvalidIndex); indices_[e.id()] = comparisonIndex_++; } diff --git a/src/expressions.h b/src/expressions.h index c2f5e98e..c98257ec 100644 --- a/src/expressions.h +++ b/src/expressions.h @@ -325,6 +325,7 @@ class ComparisonExpr : public ComparisonExprBase diag = &ctx.diag->get(*this); if (diag) { diag->location = sourceLocation(); + diag->evaluations++; } return left_->eval(ctx, val, LambdaResultFn([this, &res, &val, &diag](Context ctx, Value lv) { From 33c42ac9ac5779b62886b567699600ce21186f5d Mon Sep 17 00:00:00 2001 From: Johannes Wolf Date: Fri, 13 Mar 2026 14:53:01 +0100 Subject: [PATCH 5/5] diagnostics: Remove Environment & AST Dependencies --- include/simfil/diagnostics.h | 10 +++++----- include/simfil/model/nodes.h | 2 +- include/simfil/simfil.h | 2 +- repl/repl.cpp | 2 +- src/diagnostics.cpp | 12 ++++++------ src/simfil.cpp | 4 ++-- test/common.cpp | 2 +- test/diagnostics.cpp | 4 ++-- 8 files changed, 19 insertions(+), 19 deletions(-) diff --git a/include/simfil/diagnostics.h b/include/simfil/diagnostics.h index 269d2733..8dca090d 100644 --- a/include/simfil/diagnostics.h +++ b/include/simfil/diagnostics.h @@ -22,9 +22,9 @@ struct Environment; struct ModelNode; /** Query Diagnostics. */ -struct Diagnostics +class Diagnostics { - static constexpr std::size_t InvalidIndex = std::numeric_limits::max(); + static constexpr std::uint32_t InvalidIndex = std::numeric_limits::max(); public: struct FieldExprData { @@ -84,7 +84,7 @@ struct Diagnostics auto prepareIndices(const Expr& ast) -> void; /** ExprId to diagnostics data index mapping. */ - std::vector exprIndex_; + std::vector exprIndex_; /** FieldExpr diagnostics data. */ std::vector fieldData_; @@ -93,12 +93,12 @@ struct Diagnostics std::vector comparisonData_; private: - friend auto diagnostics(Environment& env, const AST& ast, const Diagnostics& diag) -> tl::expected, Error>; + friend auto diagnostics(const Diagnostics& diag) -> tl::expected, Error>; /** * Build messages from this objecst diagnostics data. */ - auto buildMessages(Environment& env, const AST& ast) const -> std::vector; + auto buildMessages() const -> std::vector; mutable std::mutex mtx_; }; diff --git a/include/simfil/model/nodes.h b/include/simfil/model/nodes.h index a72af7ca..44da28e6 100644 --- a/include/simfil/model/nodes.h +++ b/include/simfil/model/nodes.h @@ -31,7 +31,7 @@ class ModelPool; class Model; struct ModelNode; struct Environment; -struct Diagnostics; +class Diagnostics; class AST; class Expr; diff --git a/include/simfil/simfil.h b/include/simfil/simfil.h index 14818446..2b98016d 100644 --- a/include/simfil/simfil.h +++ b/include/simfil/simfil.h @@ -52,7 +52,7 @@ auto eval(Environment& env, const AST& ast, ModelNode const& node, Diagnostics* * Param: * diag Diagnostics data filled by eval. */ -auto diagnostics(Environment& env, const AST& ast, const Diagnostics& diag) -> tl::expected, Error>; +auto diagnostics(const Diagnostics& diag) -> tl::expected, Error>; /** * Find completion candidates for an expression. diff --git a/repl/repl.cpp b/repl/repl.cpp index 6239a212..d74e6f89 100644 --- a/repl/repl.cpp +++ b/repl/repl.cpp @@ -323,7 +323,7 @@ int main(int argc, char *argv[]) std::cout << " " << v.toString() << "\n"; } - auto messages = simfil::diagnostics(env, **ast, diag); + auto messages = simfil::diagnostics(diag); if (!messages) { std::cerr << "Error: " << messages.error().message << "\n"; continue; diff --git a/src/diagnostics.cpp b/src/diagnostics.cpp index e7e289cc..5ddeccce 100644 --- a/src/diagnostics.cpp +++ b/src/diagnostics.cpp @@ -28,8 +28,8 @@ void serialize(S& s, TypeFlags& flags) template void serialize(S& s, Diagnostics& data) { - s.container(data.exprIndex_, std::numeric_limits::max(), [](auto& s2, std::size_t& v) { - s2.value8b(v); + s.container(data.exprIndex_, std::numeric_limits::max(), [](auto& s2, std::uint32_t& v) { + s2.value4b(v); }); s.container(data.fieldData_, std::numeric_limits::max(), [](auto& s2, Diagnostics::FieldExprData& data) { s2.value4b(data.location.offset); @@ -143,7 +143,7 @@ auto Diagnostics::read(std::istream& stream) -> tl::expected return {}; } -auto Diagnostics::buildMessages(Environment& env, const AST& ast) const -> std::vector +auto Diagnostics::buildMessages() const -> std::vector { std::vector messages; @@ -196,9 +196,9 @@ auto Diagnostics::prepareIndices(const Expr& ast) -> void { struct Visitor : ExprVisitor { - std::size_t fieldIndex_ = 0u; - std::size_t comparisonIndex_ = 0u; - std::vector indices_; + std::uint32_t fieldIndex_ = 0u; + std::uint32_t comparisonIndex_ = 0u; + std::vector indices_; Visitor() { diff --git a/src/simfil.cpp b/src/simfil.cpp index 226087cb..ae6f6738 100644 --- a/src/simfil.cpp +++ b/src/simfil.cpp @@ -950,9 +950,9 @@ auto eval(Environment& env, const AST& ast, const ModelNode& node, Diagnostics* return values; } -auto diagnostics(Environment& env, const AST& ast, const Diagnostics& diag) -> expected, Error> +auto diagnostics(const Diagnostics& diag) -> expected, Error> { - return diag.buildMessages(env, ast); + return diag.buildMessages(); } } diff --git a/test/common.cpp b/test/common.cpp index f2fc341d..ad45f200 100644 --- a/test/common.cpp +++ b/test/common.cpp @@ -104,5 +104,5 @@ auto GetDiagnosticMessages(std::string_view query) -> std::vector()); + return diagnostics(diag).value_or(std::vector()); } diff --git a/test/diagnostics.cpp b/test/diagnostics.cpp index cec4a87d..b35028cc 100644 --- a/test/diagnostics.cpp +++ b/test/diagnostics.cpp @@ -58,8 +58,8 @@ TEST_CASE("DiagnosticsSerialization", "[diag.serialization]") { REQUIRE(readResult.has_value()); // Both diagnostics objects must generate the exact same messages. - auto originalMessages = diagnostics(env, **ast, originalDiag); - auto deserializedMessages = diagnostics(env, **ast, deserializedDiag); + auto originalMessages = diagnostics(originalDiag); + auto deserializedMessages = diagnostics(deserializedDiag); REQUIRE(originalMessages.has_value()); REQUIRE(deserializedMessages.has_value());