From 6d4f2991e3ca6d2963d182f3c3b7d70d2bff75a6 Mon Sep 17 00:00:00 2001 From: Jonathan Tatum Date: Tue, 2 Jan 2024 10:45:51 -0800 Subject: [PATCH] Remove option for old non-unique id numbering for map/filter macros. PiperOrigin-RevId: 595156573 --- parser/BUILD | 1 - parser/options.h | 3 --- parser/parser.cc | 12 ++++-------- parser/parser_test.cc | 1 - parser/source_factory.cc | 31 +++++++++---------------------- parser/source_factory.h | 3 +-- 6 files changed, 14 insertions(+), 37 deletions(-) diff --git a/parser/BUILD b/parser/BUILD index bdba32933..7af2c8d48 100644 --- a/parser/BUILD +++ b/parser/BUILD @@ -88,7 +88,6 @@ cc_library( "//parser/internal:cel_cc_parser", "@antlr4_runtimes//:cpp", "@com_google_absl//absl/container:flat_hash_set", - "@com_google_absl//absl/functional:any_invocable", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", "@com_google_absl//absl/types:optional", diff --git a/parser/options.h b/parser/options.h index 88ab2553c..230e16e18 100644 --- a/parser/options.h +++ b/parser/options.h @@ -47,9 +47,6 @@ struct ParserOptions final { // Enable support for optional syntax. bool enable_optional_syntax = false; - - // Enable unique numbering for builtin macro expr nodes. - bool enable_unique_macro_numbering = true; }; } // namespace cel diff --git a/parser/parser.cc b/parser/parser.cc index 3968a1b00..ac639a313 100644 --- a/parser/parser.cc +++ b/parser/parser.cc @@ -238,8 +238,7 @@ class ParserVisitor final : public CelBaseVisitor, const int max_recursion_depth, const std::vector& macros = {}, const bool add_macro_calls = false, - bool enable_optional_syntax = false, - bool enable_unique_macro_numbering = false); + bool enable_optional_syntax = false); ~ParserVisitor() override; antlrcpp::Any visit(antlr4::tree::ParseTree* tree) override; @@ -327,12 +326,10 @@ ParserVisitor::ParserVisitor(absl::string_view description, const int max_recursion_depth, const std::vector& macros, const bool add_macro_calls, - bool enable_optional_syntax, - bool enable_unique_macro_numbering) + bool enable_optional_syntax) : description_(description), expression_(expression), - sf_(std::make_shared(expression, - enable_unique_macro_numbering)), + sf_(std::make_shared(expression)), recursion_depth_(0), max_recursion_depth_(max_recursion_depth), add_macro_calls_(add_macro_calls), @@ -1171,8 +1168,7 @@ absl::StatusOr EnrichedParse( ExprRecursionListener listener(options.max_recursion_depth); ParserVisitor visitor(description, expression, options.max_recursion_depth, macros, options.add_macro_calls, - options.enable_optional_syntax, - options.enable_unique_macro_numbering); + options.enable_optional_syntax); lexer.removeErrorListeners(); parser.removeErrorListeners(); diff --git a/parser/parser_test.cc b/parser/parser_test.cc index 0a9146994..f3a147980 100644 --- a/parser/parser_test.cc +++ b/parser/parser_test.cc @@ -1375,7 +1375,6 @@ TEST_P(ExpressionTest, Parse) { ParserOptions options; if (!test_info.M.empty()) { options.add_macro_calls = true; - options.enable_unique_macro_numbering = true; } options.enable_optional_syntax = true; diff --git a/parser/source_factory.cc b/parser/source_factory.cc index e96828115..715b50a48 100644 --- a/parser/source_factory.cc +++ b/parser/source_factory.cc @@ -22,7 +22,6 @@ #include "google/protobuf/struct.pb.h" #include "absl/container/flat_hash_set.h" -#include "absl/functional/any_invocable.h" #include "absl/strings/str_format.h" #include "absl/strings/str_join.h" #include "absl/strings/str_split.h" @@ -49,9 +48,8 @@ const std::string& DefaultAccumulatorName() { } // namespace -SourceFactory::SourceFactory(absl::string_view expression, - bool unique_macro_ids) - : next_id_(1), num_errors_(0), unique_macro_ids_(unique_macro_ids) { +SourceFactory::SourceFactory(absl::string_view expression) + : next_id_(1), num_errors_(0) { CalcLineOffsets(expression); } @@ -385,15 +383,9 @@ Expr SourceFactory::NewFilterExprForMacro(int64_t macro_id, const Expr& target, const Expr& filter = args[1]; - absl::AnyInvocable accu_ident; - if (unique_macro_ids_) { - accu_ident = [this, macro_id]() { - return NewIdentForMacro(macro_id, DefaultAccumulatorName()); - }; - } else { - auto ident_expr = NewIdentForMacro(macro_id, DefaultAccumulatorName()); - accu_ident = [expr = std::move(ident_expr)]() { return expr; }; - } + auto accu_ident = [this, macro_id]() { + return NewIdentForMacro(macro_id, DefaultAccumulatorName()); + }; Expr init = NewListForMacro(macro_id, {}); Expr condition = NewLiteralBoolForMacro(macro_id, true); @@ -444,15 +436,10 @@ Expr SourceFactory::NewMapForMacro(int64_t macro_id, const Expr& target, fn = args[1]; } - absl::AnyInvocable accu_ident; - if (unique_macro_ids_) { - accu_ident = [this, macro_id]() { - return NewIdentForMacro(macro_id, DefaultAccumulatorName()); - }; - } else { - auto ident_expr = NewIdentForMacro(macro_id, DefaultAccumulatorName()); - accu_ident = [expr = std::move(ident_expr)]() { return expr; }; - } + auto accu_ident = [this, macro_id]() { + return NewIdentForMacro(macro_id, DefaultAccumulatorName()); + }; + Expr init = NewListForMacro(macro_id, {}); Expr condition = NewLiteralBoolForMacro(macro_id, true); Expr step = diff --git a/parser/source_factory.h b/parser/source_factory.h index 52279a638..d12bc9eea 100644 --- a/parser/source_factory.h +++ b/parser/source_factory.h @@ -81,7 +81,7 @@ class SourceFactory { QUANTIFIER_EXISTS_ONE }; - SourceFactory(absl::string_view expression, bool unique_macro_ids); + SourceFactory(absl::string_view expression); int64_t Id(const antlr4::Token* token); int64_t Id(antlr4::ParserRuleContext* ctx); @@ -183,7 +183,6 @@ class SourceFactory { int64_t num_errors_; std::vector line_offsets_; std::map macro_calls_; - bool unique_macro_ids_; }; } // namespace google::api::expr::parser