Remove option for old non-unique id numbering for map/filter macros.

PiperOrigin-RevId: 595156573
google · Jan 2, 2024 · 6d4f299 · 6d4f299
1 parent 4dce3f7
commit 6d4f299
Show file tree

Hide file tree

Showing 6 changed files with 14 additions and 37 deletions.
diff --git 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
@@ -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
@@ -238,8 +238,7 @@ class ParserVisitor final : public CelBaseVisitor,
                 const int max_recursion_depth,
                 const std::vector<Macro>& 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<Macro>& 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<SourceFactory>(expression,
-                                          enable_unique_macro_numbering)),
+      sf_(std::make_shared<SourceFactory>(expression)),
       recursion_depth_(0),
       max_recursion_depth_(max_recursion_depth),
       add_macro_calls_(add_macro_calls),
@@ -1171,8 +1168,7 @@ absl::StatusOr<VerboseParsedExpr> 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
@@ -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
@@ -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<Expr()> 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<Expr()> 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
@@ -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<int32_t> line_offsets_;
   std::map<int64_t, Expr> macro_calls_;
-  bool unique_macro_ids_;
 };
 
 }  // namespace google::api::expr::parser