Skip to content

Change default behavior to drop function names from reference map. #1878

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
copybara-service wants to merge 1 commit into
base: master
Choose a base branch
from
+16 −7
Open

Change default behavior to drop function names from reference map. #1878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions checker/checker_options.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,16 @@ struct CheckerOptions {
// expressions that compound nesting e.g.
// type5(T)->type(type(type(type(type(T)))))); type5(type5(T)) -> type10(T)
int max_type_decl_nesting = 13;

// If true, the checker will include the resolved function name in the
// reference map for the function call expr.
//
// If false, the function name will be empty and implied by the overload id
// set. This matches the behavior in cel-go and cel-java.
//
// Temporary flag to allow rolling out the change. No functional changes to
// evaluation behavior in either mode.
bool enable_function_name_in_reference = false;
};

} // namespace cel
Expand Down
4 changes: 3 additions & 1 deletion checker/internal/type_checker_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1259,7 +1259,9 @@ class ResolveRewriter : public AstRewriterBase {
const FunctionDecl* decl = iter->second.decl;
const bool needs_rewrite = iter->second.namespace_rewrite;
auto& ast_ref = reference_map_[expr.id()];
ast_ref.set_name(decl->name());
if (options_.enable_function_name_in_reference) {
ast_ref.set_name(decl->name());
}
for (const auto& overload : decl->overloads()) {
ast_ref.mutable_overload_id().push_back(overload.id());
}
Expand Down
9 changes: 3 additions & 6 deletions checker/internal/type_checker_impl_test.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,23 +140,20 @@ MATCHER_P(IsVariableReference, var_name, "") {

MATCHER_P2(IsFunctionReference, fn_name, overloads, "") {
const Reference& reference = arg;
if (reference.name() != fn_name) {
*result_listener << "expected: " << fn_name
<< "\nactual: " << reference.name();
}

absl::flat_hash_set<std::string> got_overload_set(
reference.overload_id().begin(), reference.overload_id().end());
absl::flat_hash_set<std::string> want_overload_set(overloads.begin(),
overloads.end());

if (got_overload_set != want_overload_set) {
*result_listener << "expected overload_ids: "
*result_listener << "reference to " << fn_name << "\n"
<< "expected overload_ids: "
<< absl::StrJoin(want_overload_set, ",")
<< "\nactual: " << absl::StrJoin(got_overload_set, ",");
}

return reference.name() == fn_name && got_overload_set == want_overload_set;
return got_overload_set == want_overload_set;
}

absl::Status RegisterMinimalBuiltins(google::protobuf::Arena* absl_nonnull arena,
Expand Down