diff --git a/base/attribute_matcher.h b/base/attribute_matcher.h index f10448b0b..a243e56b8 100644 --- a/base/attribute_matcher.h +++ b/base/attribute_matcher.h @@ -118,6 +118,13 @@ class AttributeMatcherNode { return children_ != nullptr || wildcard_ != nullptr; } + // True if this is a partial match and the only child of this node is a + // wildcard and the wildcard has no children. + [[nodiscard]] + bool IsPartialWildcard() const { + return children_ == nullptr && wildcard_ != nullptr && wildcard_->IsFull(); + } + [[nodiscard]] bool IsFull() const { return !IsPartial(); @@ -229,6 +236,13 @@ class [[nodiscard]] AttributeMatch { return node_ != nullptr && node_->IsPartial(); } + // True if this is a partial match and the only possible next match is a + // wildcard and there is no next match after the wildcard. + [[nodiscard]] + bool IsPartialWildcard() const { + return node_ != nullptr && node_->IsPartialWildcard(); + } + [[nodiscard]] bool IsFull() const { return node_ != nullptr && node_->IsFull(); @@ -381,6 +395,12 @@ class AttributeMatcher { return !children_.empty(); } + friend void swap(AttributeMatcher& lhs, AttributeMatcher& rhs) { + using std::swap; + swap(lhs.children_, rhs.children_); + swap(lhs.attributes_, rhs.attributes_); + } + private: struct AttributeCollector; diff --git a/common/BUILD b/common/BUILD index 213a91035..0250a0b6a 100644 --- a/common/BUILD +++ b/common/BUILD @@ -756,8 +756,10 @@ cc_library( ":unknown", ":value_kind", "//base:attributes", + "//base:function_result", "//base:function_result_set", "//common/internal:byte_string", + "//common/internal:unknowns", "//eval/internal:cel_value_equal", "//eval/public:cel_value", "//eval/public:message_wrapper", diff --git a/common/internal/BUILD b/common/internal/BUILD index c3b92545d..7a877ac53 100644 --- a/common/internal/BUILD +++ b/common/internal/BUILD @@ -17,6 +17,24 @@ load("@rules_cc//cc:cc_test.bzl", "cc_test") package(default_visibility = ["//visibility:public"]) +cc_library( + name = "attribute_trail", + srcs = ["attribute_trail.cc"], + hdrs = ["attribute_trail.h"], + deps = [ + ":unknowns", + "//base:attribute_matcher", + "//base:attributes", + "//common:value", + "//runtime/internal:errors", + "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/container:btree", + "@com_google_absl//absl/log:absl_check", + "@com_google_absl//absl/strings:string_view", + "@com_google_absl//absl/types:span", + ], +) + cc_library( name = "unknowns", srcs = ["unknowns.cc"], diff --git a/common/internal/attribute_trail.cc b/common/internal/attribute_trail.cc new file mode 100644 index 000000000..71a230670 --- /dev/null +++ b/common/internal/attribute_trail.cc @@ -0,0 +1,167 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#include "common/internal/attribute_trail.h" + +#include +#include +#include + +#include "absl/base/optimization.h" +#include "absl/container/btree_set.h" +#include "absl/log/absl_check.h" +#include "common/internal/unknowns.h" +#include "common/value.h" + +namespace cel::common_internal { + +void UnknownAccumulator::Add(const UnknownSet& value) { + empty_ = false; + // Ugh, we have some tests which do evil things. We should fix them. + if (value.root == nullptr) { + ABSL_DCHECK(value.attributes == nullptr); + ABSL_DCHECK(value.functions == nullptr); + return; + } + if (ABSL_PREDICT_FALSE(value.root != tree_->Root())) { + Add(tree_->Graft(value)); + return; + } + if (value.attributes != nullptr) { + if (largest_unknown_attribute_set_ != nullptr) { + if (value.attributes->size > largest_unknown_attribute_set_->size) { + largest_unknown_attribute_set_ = value.attributes; + } + } else { + largest_unknown_attribute_set_ = value.attributes; + } + if (unknown_attribute_set_pair_[0] == nullptr) { + unknown_attribute_set_pair_[0] = value.attributes; + } else if (unknown_attribute_set_pair_[1] == nullptr) { + unknown_attribute_set_pair_[1] = value.attributes; + } else { + if (!added_unknown_attribute_set_pair_) { + Add(unknown_attribute_set_pair_[0]); + Add(unknown_attribute_set_pair_[1]); + added_unknown_attribute_set_pair_ = true; + } + Add(value.attributes); + } + } + if (value.functions != nullptr) { + if (largest_unknown_function_set_ != nullptr) { + if (value.functions->size > largest_unknown_function_set_->size) { + largest_unknown_function_set_ = value.functions; + } + } else { + largest_unknown_function_set_ = value.functions; + } + if (unknown_function_set_pair_[0] == nullptr) { + unknown_function_set_pair_[0] = value.functions; + } else if (unknown_function_set_pair_[1] == nullptr) { + unknown_function_set_pair_[1] = value.functions; + } else { + if (!added_unknown_function_set_pair_) { + Add(unknown_function_set_pair_[0]); + Add(unknown_function_set_pair_[1]); + added_unknown_function_set_pair_ = true; + } + Add(value.functions); + } + } +} + +std::optional UnknownAccumulator::Accumulate() && { + if (IsEmpty()) { + return std::nullopt; + } + const UnknownAttributeSet* unknown_attribute_set; + if (unknown_attributes_.empty()) { + if (unknown_attribute_set_pair_[0] == nullptr) { + ABSL_DCHECK(unknown_attribute_set_pair_[1] == nullptr); + unknown_attribute_set = nullptr; + } else if (unknown_attribute_set_pair_[1] == nullptr) { + unknown_attribute_set = unknown_attribute_set_pair_[0]; + } else { + // Two way merger can be done with no additional memory. + unknown_attribute_set = MergeUnknownAttributeSets( + unknown_attribute_set_pair_[0], unknown_attribute_set_pair_[1], + tree_->GetArena()); + } + } else { + if (largest_unknown_attribute_set_ != nullptr && + largest_unknown_attribute_set_->size == unknown_attributes_.size()) { + unknown_attribute_set = largest_unknown_attribute_set_; + } else { + UnknownAttributeSet* mutable_unknown_attribute_set = + UnknownAttributeSet::Allocate(unknown_attributes_.size(), + tree_->GetArena()); + mutable_unknown_attribute_set->max_depth = 0; + uint32_t i = 0; + for (UnknownAttributeNode* unknown_attribute : unknown_attributes_) { + mutable_unknown_attribute_set->nodes[i++] = unknown_attribute; + mutable_unknown_attribute_set->max_depth = + std::max(mutable_unknown_attribute_set->max_depth, + static_cast(unknown_attribute->depth())); + } + unknown_attribute_set = mutable_unknown_attribute_set; + } + unknown_attributes_.clear(); + } + const UnknownFunctionSet* unknown_function_set; + if (unknown_functions_.empty()) { + if (unknown_function_set_pair_[0] == nullptr) { + ABSL_DCHECK(unknown_function_set_pair_[1] == nullptr); + unknown_function_set = nullptr; + } else if (unknown_function_set_pair_[1] == nullptr) { + unknown_function_set = unknown_function_set_pair_[0]; + } else { + // Two way merger can be done with no additional memory. + unknown_function_set = MergeUnknownFunctionSets( + unknown_function_set_pair_[0], unknown_function_set_pair_[1], + tree_->GetArena()); + } + } else { + if (largest_unknown_function_set_ != nullptr && + largest_unknown_function_set_->size == unknown_functions_.size()) { + unknown_function_set = largest_unknown_function_set_; + } else { + UnknownFunctionSet* mutable_unknown_function_set = + UnknownFunctionSet::Allocate(unknown_functions_.size(), + tree_->GetArena()); + uint32_t i = 0; + for (UnknownFunctionNode* unknown_function : unknown_functions_) { + mutable_unknown_function_set->nodes[i++] = unknown_function; + } + unknown_function_set = mutable_unknown_function_set; + } + unknown_functions_.clear(); + } + return common_internal::MakeUnknownValue(tree_->Root(), unknown_attribute_set, + unknown_function_set); +} + +void UnknownAccumulator::Add(const UnknownAttributeSet* set) { + for (uint32_t i = 0; i < set->size; ++i) { + unknown_attributes_.insert(set->nodes[i]); + } +} + +void UnknownAccumulator::Add(const UnknownFunctionSet* set) { + for (uint32_t i = 0; i < set->size; ++i) { + unknown_functions_.insert(set->nodes[i]); + } +} + +} // namespace cel::common_internal diff --git a/common/internal/attribute_trail.h b/common/internal/attribute_trail.h new file mode 100644 index 000000000..c85e4a444 --- /dev/null +++ b/common/internal/attribute_trail.h @@ -0,0 +1,523 @@ +// Copyright 2026 Google LLC +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// https://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +// This header should ideally be moved to the runtime. Its here temporarily. + +#ifndef THIRD_PARTY_CEL_CPP_COMMON_INTERNAL_ATTRIBUTE_TRAIL_H_ +#define THIRD_PARTY_CEL_CPP_COMMON_INTERNAL_ATTRIBUTE_TRAIL_H_ + +#include +#include + +#include "absl/base/attributes.h" +#include "absl/base/macros.h" +#include "absl/base/optimization.h" +#include "absl/container/btree_set.h" +#include "absl/log/absl_check.h" +#include "absl/strings/string_view.h" +#include "absl/types/span.h" +#include "base/attribute.h" +#include "base/attribute_matcher.h" +#include "common/internal/unknowns.h" +#include "common/value.h" +#include "runtime/internal/errors.h" + +namespace cel::common_internal { + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline UnknownValue AttributeMatchToUnknownValue( + const AttributeMatcherNode* node, UnknownTree* tree) { + ABSL_DCHECK(!node->key().IsWildcard()); + ABSL_DCHECK(tree != nullptr); + UnknownAttributeNode* unknown = tree->Convert(node); + return common_internal::MakeUnknownValue(UnknownSet{ + .root = tree->Root(), + .attributes = + common_internal::CreateUnknownAttributeSet(unknown, tree->GetArena()), + .functions = nullptr, + }); +} + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline UnknownValue AttributeMatchToUnknownValue( + AttributeMatch match, UnknownTree* tree) { + return common_internal::AttributeMatchToUnknownValue( + common_internal::GetAttributeMatcherNode(match), tree); +} + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline UnknownValue AttributeMatchToUnknownValue( + const AttributeMatcherNode* node, const AttributeQualifierView& key, + UnknownTree* tree) { + ABSL_DCHECK(node->key().IsWildcard() || node->key() == key); + ABSL_DCHECK(tree != nullptr); + UnknownAttributeNode* unknown = tree->Convert(node, key); + return common_internal::MakeUnknownValue(UnknownSet{ + .root = tree->Root(), + .attributes = + common_internal::CreateUnknownAttributeSet(unknown, tree->GetArena()), + .functions = nullptr, + }); +} + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline UnknownValue AttributeMatchToUnknownValue( + AttributeMatch match, const AttributeQualifierView& key, + UnknownTree* tree) { + return common_internal::AttributeMatchToUnknownValue( + common_internal::GetAttributeMatcherNode(match), key, tree); +} + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline UnknownValue AttributeMatchToUnknownValue( + AttributeMatch match, absl::string_view key, UnknownTree* tree) { + return common_internal::AttributeMatchToUnknownValue( + common_internal::GetAttributeMatcherNode(match), + AttributeQualifierView::OfString(key), tree); +} + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline ErrorValue AttributeMatchToMissingValue( + const AttributeMatcherNode* node, const AttributeQualifierView& key, + UnknownTree* tree) { + ABSL_DCHECK(node->IsFull()); + ABSL_DCHECK(node->key().IsWildcard() || node->key() == key); + ABSL_DCHECK(tree != nullptr); + // We do not need to convert matcher. + return ErrorValue::From( + runtime_internal::CreateMissingAttributeError(tree->ToString(node, key)), + tree->GetArena()); +} + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline ErrorValue AttributeMatchToMissingValue( + AttributeMatch match, const AttributeQualifierView& key, + UnknownTree* tree) { + return common_internal::AttributeMatchToMissingValue( + common_internal::GetAttributeMatcherNode(match), key, tree); +} + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline ErrorValue AttributeMatchToMissingValue( + AttributeMatch match, absl::string_view key, UnknownTree* tree) { + return common_internal::AttributeMatchToMissingValue( + common_internal::GetAttributeMatcherNode(match), + AttributeQualifierView::OfString(key), tree); +} + +class AttributeTrail; +class UnknownAccumulator; + +// AttributeLeadMatch and AttributeTrail::Match make up the core of the +// unknown/known/missing attribute matching algorithm in the runtime. The former +// is used by identifier lookup and the later is used by selects and container +// accesses. + +[[nodiscard]] +bool AttributeLeadMatch(const AttributeMatcher& unknown_attributes, + const AttributeMatcher& known_attributes, + const AttributeMatcher& missing_attributes, + absl::string_view variable, Value& value, + AttributeTrail& trail, UnknownTree* unknown_tree); + +class AttributeTrail { + public: + enum MatchMode { + kFull = 0, + kPartial, + }; + + AttributeTrail() = default; + AttributeTrail(const AttributeTrail&) = default; + AttributeTrail& operator=(const AttributeTrail&) = default; + + template + [[nodiscard]] + bool Match(const AttributeQualifierView& qualifier, Value& value, + AttributeTrail& trail, UnknownTree* unknown_tree) const { + // !!!WARNING!!! + // trail can alias *this be careful when updating this code. + // !!!WARNING!!! + static_assert(MissingMode == MatchMode::kFull, + "we do not support partial matching for missing attributes"); + if (known_.IsNone()) { + // Known is not being used or the default outcome for the trail is + // unknown. + ABSL_DCHECK(!unknown_.IsFull()); + trail.unknown_ = unknown_.MatchQualifier(qualifier); + if (KnownUnknownMode == MatchMode::kPartial ? !trail.unknown_.IsNone() + : trail.unknown_.IsFull()) { + trail.missing_ = AttributeMatch(); + value = AttributeMatchToUnknownValue( + std::exchange(trail.unknown_, AttributeMatch()), qualifier, + unknown_tree); + return true; + } + } else { + // Known is partial or full. + if (unknown_.IsNone()) { + // Unknown is none, it does not matter what known is the outcome is + // known. + trail.known_ = trail.unknown_ = AttributeMatch(); + } else { + if (MatchSlow(qualifier, value, trail, + unknown_tree)) { + return true; + } + } + } + trail.missing_ = missing_.MatchQualifier(qualifier); + if (trail.missing_.IsFull()) { + trail.known_ = trail.unknown_ = AttributeMatch(); + value = AttributeMatchToMissingValue( + std::exchange(trail.missing_, AttributeMatch()), qualifier, + unknown_tree); + return true; + } + return false; + } + template + [[nodiscard]] + bool Match(const AttributeQualifierView& qualifier, Value& value, + UnknownTree* unknown_tree) { + return Match(qualifier, value, *this, + unknown_tree); + } + + ABSL_DEPRECATE_AND_INLINE() + const AttributeTrail& attribute() const { return *this; } + + [[nodiscard]] + bool IsNone() const { + return missing_.IsNone() && unknown_.IsNone(); + } + + [[nodiscard]] + bool IsFullMatch() const { + return missing_.IsFull() || (unknown_.IsFull() && known_.IsNone()); + } + + [[nodiscard]] + std::optional PartialUnknownMatch( + UnknownTree* unknown_tree) const { + ABSL_DCHECK(!known_.IsFull()); + if (!unknown_.IsNone() && !known_.IsPartialWildcard()) { + return AttributeMatchToUnknownValue(unknown_, unknown_tree); + } + return std::nullopt; + } + + private: + friend class UnknownAccumulator; + friend bool AttributeLeadMatch(const AttributeMatcher& unknown_attributes, + const AttributeMatcher& known_attributes, + const AttributeMatcher& missing_attributes, + absl::string_view variable, Value& value, + AttributeTrail& trail, + UnknownTree* unknown_tree); + + // Out of line remainder of the logic for Match to allow the compiler to more + // easily inline Match. + template + [[nodiscard]] + bool MatchSlow(const AttributeQualifierView& qualifier, Value& value, + AttributeTrail& trail, UnknownTree* unknown_tree) const { + static_assert(MissingMode == MatchMode::kFull, + "we do not support partial matching for missing attributes"); + // Unknown is partial or full. + if (known_.IsFull()) { + // The default from here on out is known. + trail.known_ = trail.unknown_ = AttributeMatch(); + } else { + // Partial known. + ABSL_DCHECK(known_.IsPartial()); + if (unknown_.IsFull()) { + // The default from here on out is unknown unless we get full known. + // To signal this we leave unknown as full and only match known. + trail.unknown_ = unknown_; + AttributeMatch prev_known = known_; + trail.known_ = known_.MatchQualifier(qualifier); + switch (trail.known_.GetType()) { + case AttributeMatch::Type::NONE: + // Result is unknown. + trail.missing_ = AttributeMatch(); + trail.unknown_ = AttributeMatch(); + value = AttributeMatchToUnknownValue(prev_known, unknown_tree); + return true; + case AttributeMatch::Type::PARTIAL: + // If we have a known attribute pattern `a.b.*` and we just + // advanced to `a.b`, we do not consider that partial unknown we + // consider that full known. + if (!trail.known_.IsPartialWildcard()) { + if (KnownUnknownMode != MatchMode::kPartial) { + break; + } + // Result is unknown. + trail.unknown_ = AttributeMatch(); + trail.missing_ = AttributeMatch(); + value = AttributeMatchToUnknownValue( + std::exchange(trail.known_, AttributeMatch()), qualifier, + unknown_tree); + return true; + } + [[fallthrough]]; + case AttributeMatch::Type::FULL: + // Result is known. + trail.known_ = trail.unknown_ = AttributeMatch(); + break; + } + } else { + ABSL_DCHECK(unknown_.IsPartial()); + // Partial known and unknown. + trail.known_ = known_.MatchQualifier(qualifier); + trail.unknown_ = unknown_.MatchQualifier(qualifier); + if (trail.known_.IsNone()) { + if (KnownUnknownMode == MatchMode::kPartial + ? !trail.unknown_.IsNone() + : trail.unknown_.IsFull()) { + trail.missing_ = AttributeMatch(); + value = AttributeMatchToUnknownValue( + std::exchange(trail.unknown_, AttributeMatch()), qualifier, + unknown_tree); + return true; + } + } else { + if (trail.unknown_.IsNone()) { + trail.known_ = AttributeMatch(); + } else if (trail.known_.IsFull() || + trail.known_.IsPartialWildcard()) { + trail.known_ = AttributeMatch(); + trail.unknown_ = AttributeMatch(); + } + } + } + } + return false; + } + + // Should only be called from when the unknown accumulator. + AttributeMatch GetNotFullUnknownMatch() const { + if (known_.IsPartialWildcard()) { + // If we have a known attribute pattern `a.b.*` and we just + // advanced to `a.b`, we do not consider that partial. + return AttributeMatch(); + } + ABSL_DCHECK(!unknown_.IsFull()); + return unknown_; + } + + // Due to the complicated logic required to correctly perform attribute + // matching with missing, known, and unknown attributes we keep all this an + // implementation detail and return only what the caller needs to see. + + // Current match in the attribute matcher used for missing attributes. + AttributeMatch missing_; + // Current match in the attribute matcher used for known attributes. + AttributeMatch known_; + // Current match in the attribute matcher used for unknown attributes. + AttributeMatch unknown_; +}; + +[[nodiscard]] +inline bool AttributeLeadMatch(const AttributeMatcher& unknown_attributes, + const AttributeMatcher& known_attributes, + const AttributeMatcher& missing_attributes, + absl::string_view variable, Value& value, + AttributeTrail& trail, + UnknownTree* unknown_tree) { + trail.known_ = known_attributes.MatchVariable(variable); + trail.unknown_ = unknown_attributes.MatchVariable(variable); + if (trail.known_.IsNone()) { + if (trail.unknown_.IsFull()) { + trail.missing_ = AttributeMatch(); + value = AttributeMatchToUnknownValue( + std::exchange(trail.unknown_, AttributeMatch()), variable, + unknown_tree); + return true; + } + } else { + if (trail.unknown_.IsNone()) { + trail.known_ = AttributeMatch(); + } else if (trail.known_.IsFull() || trail.known_.IsPartialWildcard()) { + trail.known_ = trail.unknown_ = AttributeMatch(); + } + } + trail.missing_ = missing_attributes.MatchVariable(variable); + if (trail.missing_.IsFull()) { + trail.known_ = trail.unknown_ = AttributeMatch(); + value = AttributeMatchToMissingValue( + std::exchange(trail.missing_, AttributeMatch()), variable, + unknown_tree); + return true; + } + return false; +} + +class UnknownAccumulator { + public: + UnknownAccumulator(const UnknownAccumulator&) = delete; + UnknownAccumulator(UnknownAccumulator&&) = delete; + UnknownAccumulator& operator=(const UnknownAccumulator&) = delete; + UnknownAccumulator& operator=(UnknownAccumulator&&) = delete; + + explicit UnknownAccumulator(UnknownTree* tree) : tree_(tree) { + ABSL_DCHECK(tree != nullptr); + } + + void Add(const UnknownValue& value) { + Add(common_internal::GetUnknownValueRep(value)); + } + + void Add(const UnknownSet& value); + + void Add(AttributeMatch match) { + ABSL_DCHECK(!match.IsNone()); + unknown_attributes_.insert( + tree_->Convert(common_internal::GetAttributeMatcherNode(match))); + empty_ = false; + } + + void MaybeAdd(AttributeMatch match) { + if (!match.IsNone()) { + Add(match); + } + } + + void MaybeAdd(const Value& value) { + if (auto unknown_value = value.AsUnknown(); unknown_value.has_value()) { + Add(*unknown_value); + } + } + + void MaybeAdd(const AttributeTrail& trail) { + MaybeAdd(trail.GetNotFullUnknownMatch()); + } + + void MaybeAdd(const Value& value, AttributeMatch match) { + if (auto unknown_value = value.AsUnknown(); unknown_value.has_value()) { + Add(*unknown_value); + } else { + MaybeAdd(match); + } + } + + void MaybeAdd(const Value& value, const AttributeTrail& trail) { + MaybeAdd(value, trail.GetNotFullUnknownMatch()); + } + + [[nodiscard]] + bool IsEmpty() const { + return empty_; + } + + [[nodiscard]] + std::optional Accumulate() &&; + + [[nodiscard]] + UnknownValue Build() && { + return std::move(*this).Accumulate().value_or(UnknownValue()); + } + + private: + void Add(const UnknownAttributeSet* set); + + void Add(const UnknownFunctionSet* set); + + UnknownTree* const tree_; + // BTrees used to perform merges when more than two unknown sets are + // encountered. Merges between two sets are performed without additional + // memory. + absl::btree_set unknown_attributes_; + absl::btree_set unknown_functions_; + const UnknownAttributeSet* largest_unknown_attribute_set_ = nullptr; + const UnknownFunctionSet* largest_unknown_function_set_ = nullptr; + const UnknownAttributeSet* unknown_attribute_set_pair_[2] = {nullptr, + nullptr}; + const UnknownFunctionSet* unknown_function_set_pair_[2] = {nullptr, nullptr}; + bool added_unknown_attribute_set_pair_ = false; + bool added_unknown_function_set_pair_ = false; + bool empty_ = true; +}; + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline std::optional +PartiallyIdentityAndMergeUnknowns(absl::Span values, + absl::Span attributes, + UnknownTree* tree) { + ABSL_DCHECK_EQ(values.size(), attributes.size()); + ABSL_DCHECK(tree != nullptr); + UnknownAccumulator accumulator(tree); + for (const Value& value : values) { + accumulator.MaybeAdd(value); + } + // There should not be any full unknown matches in attributes, so only inspect + // it if we want partials. + for (const AttributeTrail& attribute : attributes) { + accumulator.MaybeAdd(attribute); + } + return std::move(accumulator).Accumulate(); +} + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline std::optional +FullyIdentityAndMergeUnknowns(absl::Span values, + UnknownTree* tree) { + ABSL_DCHECK(tree != nullptr); + UnknownAccumulator accumulator(tree); + for (const Value& value : values) { + accumulator.MaybeAdd(value); + } + return std::move(accumulator).Accumulate(); +} + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline UnknownValue GraftUnknowns( + const UnknownValue& value, UnknownTree* tree) { + ABSL_DCHECK(tree != nullptr); + return common_internal::MakeUnknownValue( + tree->Graft(common_internal::GetUnknownValueRep(value))); +} + +[[nodiscard]] +ABSL_ATTRIBUTE_ALWAYS_INLINE inline Value GraftUnknowns(const Value& value, + UnknownTree* tree) { + ABSL_DCHECK(tree != nullptr); + if (auto unknown_value = value.AsUnknown(); + ABSL_PREDICT_FALSE(unknown_value.has_value())) { + return common_internal::GraftUnknowns(*unknown_value, tree); + } + return value; +} + +ABSL_ATTRIBUTE_ALWAYS_INLINE inline void GraftUnknownsAt(UnknownValue& value, + UnknownTree* tree) { + ABSL_DCHECK(tree != nullptr); + tree->GraftAt(common_internal::GetUnknownValueRep(value)); +} + +ABSL_ATTRIBUTE_ALWAYS_INLINE inline void GraftUnknownsAt(Value& value, + UnknownTree* tree) { + ABSL_DCHECK(tree != nullptr); + if (auto unknown_value = value.AsUnknown(); + ABSL_PREDICT_FALSE(unknown_value.has_value())) { + tree->GraftAt(common_internal::GetUnknownValueRep(*unknown_value)); + } +} + +} // namespace cel::common_internal + +#endif // THIRD_PARTY_CEL_CPP_COMMON_INTERNAL_ATTRIBUTE_TRAIL_H_ diff --git a/common/internal/unknowns.h b/common/internal/unknowns.h index b0bcc7dba..b785befb1 100644 --- a/common/internal/unknowns.h +++ b/common/internal/unknowns.h @@ -263,13 +263,6 @@ class UnknownTree { ABSL_DCHECK(arena != nullptr); } - void MaybeReset(google::protobuf::Arena* absl_nonnull arena) { - ABSL_DCHECK(arena != nullptr); - if (arena_ != arena) { - Reset(arena); - } - } - void Reset(google::protobuf::Arena* absl_nonnull arena) { ABSL_DCHECK(arena != nullptr); arena_ = arena; diff --git a/common/legacy_value.cc b/common/legacy_value.cc index 151d848af..0ff096ca5 100644 --- a/common/legacy_value.cc +++ b/common/legacy_value.cc @@ -1129,8 +1129,7 @@ absl::StatusOr LegacyValue( return common_internal::LegacyTrivialMapValue(arena, modern_value); case ValueKind::kUnknown: return CelValue::CreateUnknownSet(google::protobuf::Arena::Create( - arena, - common_internal::GetUnknown(modern_value.Get()))); + arena, common_internal::ToUnknown(modern_value.Get()))); case ValueKind::kType: return CelValue::CreateCelType( CelValue::CelTypeHolder(google::protobuf::Arena::Create( @@ -1232,7 +1231,7 @@ absl::StatusOr ToLegacyValue( return common_internal::LegacyTrivialMapValue(arena, value); case ValueKind::kUnknown: return CelValue::CreateUnknownSet(google::protobuf::Arena::Create( - arena, common_internal::GetUnknown(value.Get()))); + arena, common_internal::ToUnknown(value.Get()))); case ValueKind::kType: return CelValue::CreateCelType( CelValue::CelTypeHolder(google::protobuf::Arena::Create( diff --git a/common/values/unknown_value.cc b/common/values/unknown_value.cc index abfd66625..e59465df0 100644 --- a/common/values/unknown_value.cc +++ b/common/values/unknown_value.cc @@ -12,12 +12,23 @@ // See the License for the specific language governing permissions and // limitations under the License. -#include "absl/base/no_destructor.h" +#include +#include +#include +#include +#include +#include + #include "absl/base/nullability.h" #include "absl/log/absl_check.h" #include "absl/status/status.h" #include "absl/strings/str_cat.h" +#include "base/attribute.h" +#include "base/attribute_set.h" +#include "base/function_result.h" #include "base/function_result_set.h" +#include "common/internal/unknowns.h" +#include "common/unknown.h" #include "common/value.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" @@ -65,11 +76,88 @@ absl::Status UnknownValue::Equal( return absl::OkStatus(); } +[[nodiscard]] +AttributeSet UnknownValue::ToAttributeSet() const { + AttributeSet attribute_set; + if (rep_.attributes == nullptr || rep_.attributes->size == 0) { + return attribute_set; + } + std::vector path; + path.resize(rep_.attributes->max_depth - 1); + for (uint32_t i = 0; i < rep_.attributes->size; ++i) { + const common_internal::UnknownAttributeNode* leaf = + rep_.attributes->nodes[i]; + const size_t num_qualifiers = leaf->depth() - 1; + size_t j = num_qualifiers; + while (leaf->parent() != rep_.root->Attributes()) { + path[--j] = leaf; + leaf = leaf->parent(); + } + // leaf is now the ancestor which is a child of root_, also known as our + // variable name. + std::vector qualifiers; + qualifiers.reserve(num_qualifiers); + for (j = 0; j < num_qualifiers; ++j) { + qualifiers.push_back(AttributeQualifier(path[j]->key())); + } + attribute_set.Add( + Attribute(std::string(leaf->key().GetString()), std::move(qualifiers))); + } + return attribute_set; +} + +FunctionResultSet UnknownValue::ToFunctionResultSet() const { + FunctionResultSet function_set; + if (rep_.functions == nullptr || rep_.functions->size == 0) { + return function_set; + } + for (size_t i = 0; i < rep_.functions->size; ++i) { + function_set.Add(FunctionResult(rep_.functions->nodes[i]->key())); + } + return function_set; +} + namespace common_internal { -const FunctionResultSet& GetEmptyFunctionResults() { - static const absl::NoDestructor instance; - return *instance; +UnknownValue MakeUnknownValue(const Unknown& value, + google::protobuf::Arena* absl_nonnull arena) { + if (value.unknown_attributes().empty() && + value.unknown_function_results().empty()) { + return UnknownValue(); + } + common_internal::UnknownTree tree(arena); + common_internal::UnknownRoot* root = tree.Root(); + common_internal::UnknownAttributeSet* attributes = nullptr; + common_internal::UnknownFunctionSet* functions = nullptr; + if (!value.unknown_attributes().empty()) { + attributes = common_internal::UnknownAttributeSet::Allocate( + value.unknown_attributes().size(), arena); + attributes->size = static_cast(value.unknown_attributes().size()); + attributes->max_depth = 0; + size_t i = 0; + for (const auto& attribute : value.unknown_attributes()) { + common_internal::UnknownAttributeNode* node = root->Attributes()->Step( + &tree, AttributeQualifierView::OfString(attribute.variable_name())); + for (const auto& attribute_qualifier : attribute.qualifier_path()) { + node = node->Step(&tree, attribute_qualifier); + } + attributes->max_depth = + std::max(attributes->max_depth, static_cast(node->depth())); + attributes->nodes[i++] = node; + } + std::sort(attributes->nodes, attributes->nodes + attributes->size); + } + if (!value.unknown_function_results().empty()) { + functions = common_internal::UnknownFunctionSet::Allocate( + value.unknown_attributes().size(), arena); + functions->size = value.unknown_attributes().size(); + size_t i = 0; + for (const auto& function_result : value.unknown_function_results()) { + functions->nodes[i++] = root->SetFunction(&tree, function_result.name()); + } + std::sort(functions->nodes, functions->nodes + functions->size); + } + return MakeUnknownValue(root, attributes, functions); } } // namespace common_internal diff --git a/common/values/unknown_value.h b/common/values/unknown_value.h index dae2aa0b2..e9de497bc 100644 --- a/common/values/unknown_value.h +++ b/common/values/unknown_value.h @@ -20,13 +20,13 @@ #include #include -#include #include "absl/base/attributes.h" #include "absl/base/nullability.h" #include "absl/status/status.h" #include "absl/strings/string_view.h" -#include "base/function_result_set.h" +#include "base/attribute_set.h" +#include "common/internal/unknowns.h" #include "common/type.h" #include "common/unknown.h" #include "common/value_kind.h" @@ -43,12 +43,24 @@ class UnknownValue; namespace common_internal { [[nodiscard]] -UnknownValue MakeUnknownValue(Unknown value, google::protobuf::Arena* absl_nonnull arena); +UnknownValue MakeUnknownValue(const Unknown& value, + google::protobuf::Arena* absl_nonnull arena); [[nodiscard]] -Unknown GetUnknown(const UnknownValue& value); +UnknownValue MakeUnknownValue( + const common_internal::UnknownRoot* root, + const common_internal::UnknownAttributeSet* attributes, + const common_internal::UnknownFunctionSet* functions); [[nodiscard]] -const FunctionResultSet& GetUnknownFunctionResultSet( +Unknown ToUnknown(const UnknownValue& value); +using UnknownValueRep = UnknownSet; +[[nodiscard]] +UnknownValue MakeUnknownValue(const UnknownValueRep& rep); +[[nodiscard]] +const UnknownValueRep& GetUnknownValueRep( const UnknownValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND); +[[nodiscard]] +UnknownValueRep& GetUnknownValueRep( + UnknownValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND); } // namespace common_internal // `UnknownValue` represents values of the primitive `duration` type. @@ -90,32 +102,36 @@ class UnknownValue final : private common_internal::ValueMixin { bool IsZeroValue() const { return false; } [[nodiscard]] - AttributeSet ToAttributeSet() const { - return unknown_ != nullptr ? unknown_->unknown_attributes() - : AttributeSet(); - } + AttributeSet ToAttributeSet() const; - void swap(UnknownValue& other) noexcept { - using std::swap; - swap(unknown_, other.unknown_); - } + [[nodiscard]] + FunctionResultSet ToFunctionResultSet() const; private: friend UnknownValue common_internal::MakeUnknownValue( - Unknown value, google::protobuf::Arena* absl_nonnull arena); - friend Unknown common_internal::GetUnknown(const UnknownValue&); - friend const FunctionResultSet& common_internal::GetUnknownFunctionResultSet( - const UnknownValue& value); + const common_internal::UnknownRoot* root, + const common_internal::UnknownAttributeSet* attributes, + const common_internal::UnknownFunctionSet* functions); + friend Unknown common_internal::ToUnknown(const UnknownValue& value); + friend const common_internal::UnknownValueRep& + common_internal::GetUnknownValueRep(const UnknownValue& value); + friend common_internal::UnknownValueRep& common_internal::GetUnknownValueRep( + UnknownValue& value); + friend UnknownValue common_internal::MakeUnknownValue( + const common_internal::UnknownValueRep& rep); friend class common_internal::ValueMixin; - explicit UnknownValue(const Unknown* unknown) : unknown_(unknown) {} + using Rep = common_internal::UnknownValueRep; - const Unknown* unknown_ = nullptr; -}; + UnknownValue(const common_internal::UnknownRoot* root, + const common_internal::UnknownAttributeSet* attributes, + const common_internal::UnknownFunctionSet* functions) + : rep_{root, attributes, functions} {} -inline void swap(UnknownValue& lhs, UnknownValue& rhs) noexcept { - lhs.swap(rhs); -} + explicit UnknownValue(const Rep& rep) : rep_(rep) {} + + Rep rep_; +}; inline std::ostream& operator<<(std::ostream& out, const UnknownValue& value) { return out << value.DebugString(); @@ -126,21 +142,33 @@ namespace common_internal { const FunctionResultSet& GetEmptyFunctionResults(); [[nodiscard]] -inline UnknownValue MakeUnknownValue(Unknown value, - google::protobuf::Arena* absl_nonnull arena) { - return UnknownValue(google::protobuf::Arena::Create(arena, std::move(value))); +inline UnknownValue MakeUnknownValue( + const common_internal::UnknownRoot* root, + const common_internal::UnknownAttributeSet* attributes, + const common_internal::UnknownFunctionSet* functions) { + return UnknownValue(root, attributes, functions); +} + +[[nodiscard]] +inline Unknown ToUnknown(const UnknownValue& value) { + return Unknown(value.ToAttributeSet(), value.ToFunctionResultSet()); } [[nodiscard]] -inline Unknown GetUnknown(const UnknownValue& value) { - return value.unknown_ != nullptr ? *value.unknown_ : Unknown(); +inline UnknownValue MakeUnknownValue(const UnknownValueRep& rep) { + return UnknownValue(rep); } [[nodiscard]] -inline const FunctionResultSet& GetUnknownFunctionResultSet( +inline const UnknownValueRep& GetUnknownValueRep( const UnknownValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) { - return value.unknown_ != nullptr ? value.unknown_->unknown_function_results() - : GetEmptyFunctionResults(); + return value.rep_; +} + +[[nodiscard]] +inline UnknownValueRep& GetUnknownValueRep( + UnknownValue& value ABSL_ATTRIBUTE_LIFETIME_BOUND) { + return value.rep_; } } // namespace common_internal diff --git a/eval/compiler/instrumentation_test.cc b/eval/compiler/instrumentation_test.cc index 39edecf6a..cfd1c4fdc 100644 --- a/eval/compiler/instrumentation_test.cc +++ b/eval/compiler/instrumentation_test.cc @@ -192,6 +192,7 @@ TEST_F(InstrumentationTest, AndShortCircuit) { ASSERT_OK_AND_ASSIGN(auto value, plan.EvaluateWithCallback( activation, /*embedder_context=*/nullptr, EvaluationListener(), state)); + static_cast(value); EXPECT_THAT(expr_ids, ElementsAre(1, 2, 3)); @@ -200,6 +201,7 @@ TEST_F(InstrumentationTest, AndShortCircuit) { ASSERT_OK_AND_ASSIGN( value, plan.EvaluateWithCallback(activation, /*embedder_context=*/nullptr, EvaluationListener(), state)); + static_cast(value); EXPECT_THAT(expr_ids, ElementsAre(1, 2, 3, 1, 3)); } @@ -234,6 +236,7 @@ TEST_F(InstrumentationTest, OrShortCircuit) { ASSERT_OK_AND_ASSIGN(auto value, plan.EvaluateWithCallback( activation, /*embedder_context=*/nullptr, EvaluationListener(), state)); + static_cast(value); EXPECT_THAT(expr_ids, ElementsAre(1, 2, 3)); expr_ids.clear(); @@ -242,6 +245,7 @@ TEST_F(InstrumentationTest, OrShortCircuit) { ASSERT_OK_AND_ASSIGN( value, plan.EvaluateWithCallback(activation, /*embedder_context=*/nullptr, EvaluationListener(), state)); + static_cast(value); EXPECT_THAT(expr_ids, ElementsAre(1, 3)); } @@ -277,6 +281,7 @@ TEST_F(InstrumentationTest, Ternary) { ASSERT_OK_AND_ASSIGN(auto value, plan.EvaluateWithCallback( activation, /*embedder_context=*/nullptr, EvaluationListener(), state)); + static_cast(value); // AST // ?:() <2> @@ -290,6 +295,7 @@ TEST_F(InstrumentationTest, Ternary) { ASSERT_OK_AND_ASSIGN( value, plan.EvaluateWithCallback(activation, /*embedder_context=*/nullptr, EvaluationListener(), state)); + static_cast(value); EXPECT_THAT(expr_ids, ElementsAre(1, 4, 2)); expr_ids.clear(); diff --git a/eval/eval/BUILD b/eval/eval/BUILD index 4bb06c7b8..b43675981 100644 --- a/eval/eval/BUILD +++ b/eval/eval/BUILD @@ -56,11 +56,11 @@ cc_library( ], deps = [ ":attribute_trail", - ":attribute_utility", ":comprehension_slots", ":evaluator_stack", ":expression_step_logic", ":iterator_stack", + "//base:attribute_matcher", "//base:attributes", "//base:builtins", "//base:data", @@ -70,6 +70,8 @@ cc_library( "//common:optional_ref", "//common:value", "//common:value_kind", + "//common/internal:attribute_trail", + "//common/internal:unknowns", "//eval/internal:errors", "//internal:number", "//internal:status_macros", @@ -80,7 +82,6 @@ cc_library( "//runtime:function_provider", "//runtime:function_registry", "//runtime:runtime_options", - "//runtime/internal:activation_attribute_matcher_access", "//runtime/internal:errors", "//runtime/standard:equality_functions", "@com_google_absl//absl/base", @@ -132,14 +133,13 @@ cc_library( "comprehension_slots.h", ], deps = [ - ":attribute_trail", "//common:value", + "//common/internal:attribute_trail", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/base:no_destructor", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/container:fixed_array", "@com_google_absl//absl/log:absl_check", - "@com_google_absl//absl/types:optional", ], ) @@ -151,7 +151,6 @@ cc_test( deps = [ ":attribute_trail", ":comprehension_slots", - "//base:attributes", "//base:data", "//common:memory", "//common:value", @@ -168,8 +167,8 @@ cc_library( "evaluator_stack.h", ], deps = [ - ":attribute_trail", "//common:value", + "//common/internal:attribute_trail", "//internal:align", "//internal:new", "@com_google_absl//absl/base:core_headers", @@ -178,7 +177,6 @@ cc_library( "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/log:absl_log", "@com_google_absl//absl/meta:type_traits", - "@com_google_absl//absl/types:optional", "@com_google_absl//absl/types:span", ], ) @@ -189,6 +187,7 @@ cc_test( "evaluator_stack_test.cc", ], deps = [ + ":attribute_trail", ":evaluator_stack", "//base:attributes", "//common:value", @@ -222,7 +221,6 @@ cc_library( ], deps = [ ":attribute_trail", - ":attribute_utility", ":evaluator_core", ":expression_step_base", ":expression_step_logic", @@ -264,6 +262,17 @@ cc_library( name = "ident_step", deps = [ ":evaluator_core", + ":expression_step_base", + ":expression_step_logic", + "//base:attribute_matcher", + "//base:attributes", + "//common:value", + "//eval/internal:errors", + "//internal:status_macros", + "@com_google_absl//absl/base:nullability", + "@com_google_absl//absl/status", + "@com_google_absl//absl/status:statusor", + "@com_google_absl//absl/strings", ], ) @@ -287,6 +296,7 @@ cc_library( ":evaluator_core", ":expression_step_base", ":expression_step_logic", + "//base:attributes", "//common:legacy_value", "//common:type", "//common:value", @@ -295,7 +305,6 @@ cc_library( "//runtime:runtime_options", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/log:absl_check", - "@com_google_absl//absl/log:absl_log", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", @@ -304,6 +313,26 @@ cc_library( ], ) +cc_library( + name = "create_list_step", + srcs = [ + "create_list_step.cc", + ], + hdrs = [ + "create_list_step.h", + ], + deps = [ + ":evaluator_core", + "//common:optional_ref", + "//common:value", + "//common/internal:attribute_trail", + "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/log:absl_check", + "@com_google_absl//absl/status", + "@com_google_absl//absl/types:span", + ], +) + cc_library( name = "create_struct_step", srcs = [ @@ -339,12 +368,12 @@ cc_library( ":expression_step_base", ":expression_step_logic", "//common:value", + "//common/internal:attribute_trail", "//internal:status_macros", "@com_google_absl//absl/container:flat_hash_set", "@com_google_absl//absl/status", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", - "@com_google_absl//absl/types:optional", ], ) @@ -361,13 +390,14 @@ cc_test( "equality_steps_test.cc", ], deps = [ - ":attribute_trail", ":evaluator_core", ":expression_step_logic", "//base:attributes", + "//common:unknown", "//common:value", "//common:value_kind", "//common:value_testing", + "//common/internal:attribute_trail", "//internal:testing", "//internal:testing_descriptor_pool", "//internal:testing_message_factory", @@ -377,7 +407,6 @@ cc_test( "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/status", - "@com_google_absl//absl/status:status_matchers", "@com_google_protobuf//:protobuf", ], ) @@ -731,75 +760,15 @@ cc_test( cc_library( name = "attribute_trail", - srcs = ["attribute_trail.cc"], hdrs = ["attribute_trail.h"], deps = [ + "//base:attribute_matcher", "//base:attributes", - "@com_google_absl//absl/types:optional", - "@com_google_absl//absl/utility", - ], -) - -cc_test( - name = "attribute_trail_test", - size = "small", - srcs = [ - "attribute_trail_test.cc", - ], - deps = [ - ":attribute_trail", - "//eval/public:cel_attribute", - "//eval/public:cel_value", - "//internal:testing", - "@com_google_cel_spec//proto/cel/expr:syntax_cc_proto", - ], -) - -cc_library( - name = "attribute_utility", - srcs = ["attribute_utility.cc"], - hdrs = ["attribute_utility.h"], - deps = [ - ":attribute_trail", - "//base:attributes", - "//base:function_result", - "//base:function_result_set", - "//base/internal:unknown_set", - "//common:function_descriptor", - "//common:unknown", - "//common:value", - "//eval/internal:errors", - "//internal:status_macros", - "//runtime/internal:attribute_matcher", - "@com_google_absl//absl/base:nullability", - "@com_google_absl//absl/status:statusor", - "@com_google_absl//absl/types:optional", - "@com_google_absl//absl/types:span", - "@com_google_protobuf//:protobuf", - ], -) - -cc_test( - name = "attribute_utility_test", - size = "small", - srcs = [ - "attribute_utility_test.cc", - ], - deps = [ - ":attribute_trail", - ":attribute_utility", - "//base:attributes", - "//common:unknown", "//common:value", - "//eval/public:cel_attribute", - "//eval/public:cel_value", - "//eval/public:unknown_attribute_set", - "//eval/public:unknown_set", - "//internal:testing", - "//runtime/internal:attribute_matcher", - "@com_google_absl//absl/types:optional", - "@com_google_absl//absl/types:span", - "@com_google_protobuf//:protobuf", + "//common/internal:attribute_trail", + "//common/internal:unknowns", + "@com_google_absl//absl/base:core_headers", + "@com_google_absl//absl/strings:string_view", ], ) diff --git a/eval/eval/attribute_trail.cc b/eval/eval/attribute_trail.cc deleted file mode 100644 index 6b5db896e..000000000 --- a/eval/eval/attribute_trail.cc +++ /dev/null @@ -1,28 +0,0 @@ -#include "eval/eval/attribute_trail.h" - -#include -#include -#include -#include -#include - -#include "base/attribute.h" - -namespace google::api::expr::runtime { - -// Creates AttributeTrail with attribute path incremented by "qualifier". -AttributeTrail AttributeTrail::Step(cel::AttributeQualifier qualifier) const { - // Cannot continue void trail - if (empty()) return AttributeTrail(); - - std::vector qualifiers; - qualifiers.reserve(attribute_->qualifier_path().size() + 1); - std::copy_n(attribute_->qualifier_path().begin(), - attribute_->qualifier_path().size(), - std::back_inserter(qualifiers)); - qualifiers.push_back(std::move(qualifier)); - return AttributeTrail(cel::Attribute(std::string(attribute_->variable_name()), - std::move(qualifiers))); -} - -} // namespace google::api::expr::runtime diff --git a/eval/eval/attribute_trail.h b/eval/eval/attribute_trail.h index 838cc8875..bd76571cf 100644 --- a/eval/eval/attribute_trail.h +++ b/eval/eval/attribute_trail.h @@ -1,64 +1,15 @@ #ifndef THIRD_PARTY_CEL_CPP_EVAL_EVAL_ATTRIBUTE_TRAIL_H_ #define THIRD_PARTY_CEL_CPP_EVAL_EVAL_ATTRIBUTE_TRAIL_H_ -#include -#include - -#include "absl/types/optional.h" -#include "absl/utility/utility.h" -#include "base/attribute.h" +#include "base/attribute_matcher.h" +#include "common/internal/attribute_trail.h" namespace google::api::expr::runtime { -// AttributeTrail reflects current attribute path. -// It is functionally similar to cel::Attribute, yet intended to have better -// complexity on attribute path increment operations. -// TODO(issues/41) Current AttributeTrail implementation is equivalent to -// cel::Attribute - improve it. -// Intended to be used in conjunction with cel::Value, describing the attribute -// value originated from. -// Empty AttributeTrail denotes object with attribute path not defined -// or supported. -class AttributeTrail { - public: - AttributeTrail() : attribute_(absl::nullopt) {} - - explicit AttributeTrail(std::string variable_name) - : attribute_(absl::in_place, std::move(variable_name)) {} - - explicit AttributeTrail(cel::Attribute attribute) - : attribute_(std::move(attribute)) {} - - // NOLINTNEXTLINE(google-explicit-constructor) - AttributeTrail(absl::nullopt_t) : AttributeTrail() {} - - AttributeTrail(const AttributeTrail&) = default; - AttributeTrail& operator=(const AttributeTrail&) = default; - AttributeTrail(AttributeTrail&&) = default; - AttributeTrail& operator=(AttributeTrail&&) = default; - - AttributeTrail& operator=(absl::nullopt_t) { - attribute_.reset(); - return *this; - } - - // Creates AttributeTrail with attribute path incremented by "qualifier". - AttributeTrail Step(cel::AttributeQualifier qualifier) const; - - // Creates AttributeTrail with attribute path incremented by "qualifier". - AttributeTrail Step(const std::string* qualifier) const { - if (empty()) return AttributeTrail(); - return Step(cel::AttributeQualifier::OfString(*qualifier)); - } - - // Returns CelAttribute that corresponds to content of AttributeTrail. - const cel::Attribute& attribute() const { return attribute_.value(); } - - bool empty() const { return !attribute_.has_value(); } - - private: - absl::optional attribute_; -}; +using cel::AttributeMatch; +using cel::common_internal::AttributeLeadMatch; +using cel::common_internal::AttributeTrail; +using cel::common_internal::UnknownAccumulator; } // namespace google::api::expr::runtime diff --git a/eval/eval/attribute_trail_test.cc b/eval/eval/attribute_trail_test.cc deleted file mode 100644 index 3143b9ed4..000000000 --- a/eval/eval/attribute_trail_test.cc +++ /dev/null @@ -1,31 +0,0 @@ -#include "eval/eval/attribute_trail.h" - -#include - -#include "cel/expr/syntax.pb.h" -#include "eval/public/cel_attribute.h" -#include "eval/public/cel_value.h" -#include "internal/testing.h" - -namespace google::api::expr::runtime { - -// Attribute Trail behavior -TEST(AttributeTrailTest, AttributeTrailEmptyStep) { - std::string step = "step"; - CelValue step_value = CelValue::CreateString(&step); - AttributeTrail trail; - ASSERT_TRUE(trail.Step(&step).empty()); - ASSERT_TRUE(trail.Step(CreateCelAttributeQualifier(step_value)).empty()); -} - -TEST(AttributeTrailTest, AttributeTrailStep) { - std::string step = "step"; - CelValue step_value = CelValue::CreateString(&step); - - AttributeTrail trail = AttributeTrail("ident").Step(&step); - - ASSERT_EQ(trail.attribute(), - CelAttribute("ident", {CreateCelAttributeQualifier(step_value)})); -} - -} // namespace google::api::expr::runtime diff --git a/eval/eval/attribute_utility.cc b/eval/eval/attribute_utility.cc deleted file mode 100644 index b1e75111d..000000000 --- a/eval/eval/attribute_utility.cc +++ /dev/null @@ -1,271 +0,0 @@ -#include "eval/eval/attribute_utility.h" - -#include -#include -#include -#include - -#include "absl/status/statusor.h" -#include "absl/types/optional.h" -#include "absl/types/span.h" -#include "base/attribute.h" -#include "base/attribute_set.h" -#include "base/function_result.h" -#include "base/function_result_set.h" -#include "base/internal/unknown_set.h" -#include "common/function_descriptor.h" -#include "common/unknown.h" -#include "common/value.h" -#include "eval/eval/attribute_trail.h" -#include "eval/internal/errors.h" -#include "internal/status_macros.h" -#include "runtime/internal/attribute_matcher.h" -#include "google/protobuf/arena.h" - -namespace google::api::expr::runtime { - -using ::cel::Attribute; -using ::cel::AttributePattern; -using ::cel::AttributeSet; -using ::cel::ErrorValue; -using ::cel::FunctionResult; -using ::cel::FunctionResultSet; -using ::cel::UnknownValue; -using ::cel::Value; -using ::cel::base_internal::UnknownSet; -using ::cel::runtime_internal::AttributeMatcher; - -using Accumulator = AttributeUtility::Accumulator; -using MatchResult = AttributeMatcher::MatchResult; - -DefaultAttributeMatcher::DefaultAttributeMatcher( - absl::Span unknown_patterns, - absl::Span missing_patterns) - : unknown_patterns_(unknown_patterns), - missing_patterns_(missing_patterns) {} - -DefaultAttributeMatcher::DefaultAttributeMatcher() = default; - -AttributeMatcher::MatchResult MatchAgainstPatterns( - absl::Span patterns, const Attribute& attr) { - MatchResult result = MatchResult::NONE; - for (const auto& pattern : patterns) { - auto current_match = pattern.IsMatch(attr); - if (current_match == cel::AttributePattern::MatchType::FULL) { - return MatchResult::FULL; - } - if (current_match == cel::AttributePattern::MatchType::PARTIAL) { - result = MatchResult::PARTIAL; - } - } - return result; -} - -DefaultAttributeMatcher::MatchResult DefaultAttributeMatcher::CheckForUnknown( - const Attribute& attr) const { - return MatchAgainstPatterns(unknown_patterns_, attr); -} - -DefaultAttributeMatcher::MatchResult DefaultAttributeMatcher::CheckForMissing( - const Attribute& attr) const { - return MatchAgainstPatterns(missing_patterns_, attr); -} - -bool AttributeUtility::CheckForMissingAttribute( - const AttributeTrail& trail) const { - if (trail.empty()) { - return false; - } - // Missing attributes are only treated as errors if the attribute exactly - // matches (so no guard against passing partial state to a function as with - // unknowns). This was initially a design oversight, but is difficult to - // change now. - return matcher_->CheckForMissing(trail.attribute()) == - AttributeMatcher::MatchResult::FULL; -} - -// Checks whether particular corresponds to any patterns that define unknowns. -bool AttributeUtility::CheckForUnknown(const AttributeTrail& trail, - bool use_partial) const { - if (trail.empty()) { - return false; - } - MatchResult result = matcher_->CheckForUnknown(trail.attribute()); - - if (result == MatchResult::FULL || - (use_partial && result == MatchResult::PARTIAL)) { - return true; - } - return false; -} - -// Creates merged UnknownAttributeSet. -// Scans over the args collection, merges any UnknownSets found in -// it together with initial_set (if initial_set is not null). -// Returns pointer to merged set or nullptr, if there were no sets to merge. -absl::optional AttributeUtility::MergeUnknowns( - absl::Span args, google::protobuf::Arena* arena) const { - // Empty unknown value may be used as a sentinel in some tests so need to - // distinguish unset (nullopt) and empty(engaged empty value). - absl::optional result_set; - - for (const auto& value : args) { - if (!value->Is()) continue; - if (!result_set.has_value()) { - result_set.emplace(); - } - const auto& current_set = value.GetUnknown(); - - cel::base_internal::UnknownSetAccess::Add( - *result_set, - UnknownSet( - current_set.ToAttributeSet(), - cel::common_internal::GetUnknownFunctionResultSet(current_set))); - } - - if (!result_set.has_value()) { - return std::nullopt; - } - - return cel::common_internal::MakeUnknownValue( - cel::Unknown(result_set->unknown_attributes(), - result_set->unknown_function_results()), - arena); -} - -UnknownValue AttributeUtility::MergeUnknownValues(const UnknownValue& left, - const UnknownValue& right, - google::protobuf::Arena* arena) const { - // Empty unknown value may be used as a sentinel in some tests so need to - // distinguish unset (nullopt) and empty(engaged empty value). - AttributeSet attributes; - FunctionResultSet function_results; - attributes.Add(left.ToAttributeSet()); - function_results.Add(cel::common_internal::GetUnknownFunctionResultSet(left)); - attributes.Add(right.ToAttributeSet()); - function_results.Add( - cel::common_internal::GetUnknownFunctionResultSet(right)); - - return cel::common_internal::MakeUnknownValue( - cel::Unknown(std::move(attributes), std::move(function_results)), arena); -} - -// Creates merged UnknownAttributeSet. -// Scans over the args collection, determines if there matches to unknown -// patterns, merges attributes together with those from initial_set -// (if initial_set is not null). -// Returns pointer to merged set or nullptr, if there were no sets to merge. -AttributeSet AttributeUtility::CheckForUnknowns( - absl::Span args, bool use_partial) const { - AttributeSet attribute_set; - - for (const auto& trail : args) { - if (CheckForUnknown(trail, use_partial)) { - attribute_set.Add(trail.attribute()); - } - } - - return attribute_set; -} - -// Creates merged UnknownAttributeSet. -// Merges together attributes from UnknownAttributeSets found in the args -// collection, attributes from attr that match unknown pattern -// patterns, and attributes from initial_set -// (if initial_set is not null). -// Returns pointer to merged set or nullptr, if there were no sets to merge. -absl::optional AttributeUtility::IdentifyAndMergeUnknowns( - absl::Span args, absl::Span attrs, - bool use_partial, google::protobuf::Arena* arena) const { - absl::optional result_set; - - // Identify new unknowns by attribute patterns. - cel::AttributeSet attr_set = CheckForUnknowns(attrs, use_partial); - if (!attr_set.empty()) { - result_set.emplace(std::move(attr_set)); - } - - // merge down existing unknown sets - absl::optional arg_unknowns = MergeUnknowns(args, arena); - - if (!result_set.has_value()) { - // No new unknowns so no need to check for presence of existing unknowns -- - // just forward. - return arg_unknowns; - } - - if (arg_unknowns.has_value()) { - cel::base_internal::UnknownSetAccess::Add( - *result_set, cel::common_internal::GetUnknown(*arg_unknowns)); - } - - return cel::common_internal::MakeUnknownValue( - cel::Unknown(result_set->unknown_attributes(), - result_set->unknown_function_results()), - arena); -} - -UnknownValue AttributeUtility::CreateUnknownSet(cel::Attribute attr, - google::protobuf::Arena* arena) const { - return cel::common_internal::MakeUnknownValue( - cel::Unknown(AttributeSet({std::move(attr)})), arena); -} - -absl::StatusOr AttributeUtility::CreateMissingAttributeError( - const cel::Attribute& attr, google::protobuf::Arena* arena) const { - CEL_ASSIGN_OR_RETURN(std::string message, attr.AsString()); - return cel::ErrorValue::From( - cel::runtime_internal::CreateMissingAttributeError(message), arena); -} - -UnknownValue AttributeUtility::CreateUnknownSet( - const cel::FunctionDescriptor& fn_descriptor, int64_t expr_id, - absl::Span args, google::protobuf::Arena* arena) const { - return cel::common_internal::MakeUnknownValue( - cel::Unknown(FunctionResultSet(FunctionResult(fn_descriptor.name()))), - arena); -} - -void AttributeUtility::Add(Accumulator& a, const cel::UnknownValue& v) const { - a.attribute_set_.Add(v.ToAttributeSet()); - a.function_result_set_.Add( - cel::common_internal::GetUnknownFunctionResultSet(v)); -} - -void AttributeUtility::Add(Accumulator& a, const AttributeTrail& attr) const { - a.attribute_set_.Add(attr.attribute()); -} - -void Accumulator::Add(const UnknownValue& value) { - unknown_present_ = true; - parent_.Add(*this, value); -} - -void Accumulator::Add(const AttributeTrail& attr) { parent_.Add(*this, attr); } - -void Accumulator::MaybeAdd(const Value& v) { - if (v.IsUnknown()) { - Add(v.GetUnknown()); - } -} - -void Accumulator::MaybeAdd(const Value& v, const AttributeTrail& attr) { - if (v.IsUnknown()) { - Add(v.GetUnknown()); - } else if (parent_.CheckForUnknown(attr, /*use_partial=*/true)) { - Add(attr); - } -} - -bool Accumulator::IsEmpty() const { - return !unknown_present_ && attribute_set_.empty() && - function_result_set_.empty(); -} - -cel::UnknownValue Accumulator::Build(google::protobuf::Arena* arena) && { - return cel::common_internal::MakeUnknownValue( - cel::Unknown(std::move(attribute_set_), std::move(function_result_set_)), - arena); -} - -} // namespace google::api::expr::runtime diff --git a/eval/eval/attribute_utility.h b/eval/eval/attribute_utility.h deleted file mode 100644 index 251e6c7d2..000000000 --- a/eval/eval/attribute_utility.h +++ /dev/null @@ -1,182 +0,0 @@ -#ifndef THIRD_PARTY_CEL_CPP_EVAL_EVAL_UNKNOWNS_UTILITY_H_ -#define THIRD_PARTY_CEL_CPP_EVAL_EVAL_UNKNOWNS_UTILITY_H_ - -#include - -#include "absl/base/nullability.h" -#include "absl/status/statusor.h" -#include "absl/types/optional.h" -#include "absl/types/span.h" -#include "base/attribute.h" -#include "base/attribute_set.h" -#include "base/function_result_set.h" -#include "common/function_descriptor.h" -#include "common/value.h" -#include "eval/eval/attribute_trail.h" -#include "runtime/internal/attribute_matcher.h" -#include "google/protobuf/arena.h" - -namespace google::api::expr::runtime { - -// Default implementation of the attribute matcher. -// Scans the attribute trail against a list of unknown or missing patterns. -class DefaultAttributeMatcher : public cel::runtime_internal::AttributeMatcher { - private: - using MatchResult = cel::runtime_internal::AttributeMatcher::MatchResult; - - public: - DefaultAttributeMatcher( - absl::Span unknown_patterns, - absl::Span missing_patterns); - - DefaultAttributeMatcher(); - - MatchResult CheckForUnknown(const cel::Attribute& attr) const override; - MatchResult CheckForMissing(const cel::Attribute& attr) const override; - - private: - absl::Span unknown_patterns_; - absl::Span missing_patterns_; -}; - -// Helper class for handling unknowns and missing attribute logic. Provides -// helpers for merging unknown sets from arguments on the stack and for -// identifying unknown/missing attributes based on the patterns for a given -// Evaluation. -// Neither moveable nor copyable. -class AttributeUtility { - public: - class Accumulator { - public: - Accumulator(const Accumulator&) = delete; - Accumulator& operator=(const Accumulator&) = delete; - Accumulator(Accumulator&&) = delete; - Accumulator& operator=(Accumulator&&) = delete; - - // Add to the accumulated unknown attributes and functions. - void Add(const cel::UnknownValue& v); - void Add(const AttributeTrail& attr); - - // Add to the accumulated set of unknowns if value is UnknownValue. - void MaybeAdd(const cel::Value& v); - - // Add to the accumulated set of unknowns if value is UnknownValue or - // the attribute trail is (partially) unknown. This version prefers - // preserving an already present unknown value over a new one matching the - // attribute trail. - // - // Uses partial matching (a pattern matches the attribute or any - // sub-attribute). - void MaybeAdd(const cel::Value& v, const AttributeTrail& attr); - - bool IsEmpty() const; - - cel::UnknownValue Build(google::protobuf::Arena* arena) &&; - - private: - explicit Accumulator(const AttributeUtility& parent) - : parent_(parent), unknown_present_(false) {} - - friend class AttributeUtility; - const AttributeUtility& parent_; - - cel::AttributeSet attribute_set_; - cel::FunctionResultSet function_result_set_; - - // Some tests will use an empty unknown set as a sentinel. - // Preserve forwarding behavior. - bool unknown_present_; - }; - - AttributeUtility(absl::Span unknown_patterns, - absl::Span missing_patterns) - : default_matcher_(unknown_patterns, missing_patterns), - matcher_(&default_matcher_) {} - - explicit AttributeUtility( - const cel::runtime_internal::AttributeMatcher* absl_nonnull matcher) - : matcher_(matcher) {} - - AttributeUtility(const AttributeUtility&) = delete; - AttributeUtility& operator=(const AttributeUtility&) = delete; - AttributeUtility(AttributeUtility&&) = delete; - AttributeUtility& operator=(AttributeUtility&&) = delete; - - // Checks whether particular corresponds to any patterns that define missing - // attribute. - bool CheckForMissingAttribute(const AttributeTrail& trail) const; - - // Checks whether trail corresponds to any patterns that define unknowns. - bool CheckForUnknown(const AttributeTrail& trail, bool use_partial) const; - - // Checks whether trail corresponds to any patterns that identify - // unknowns. Only matches exactly (exact attribute match for self or parent). - bool CheckForUnknownExact(const AttributeTrail& trail) const { - return CheckForUnknown(trail, false); - } - - // Checks whether trail corresponds to any patterns that define unknowns. - // Matches if a parent or any descendant (select or index of) the attribute. - bool CheckForUnknownPartial(const AttributeTrail& trail) const { - return CheckForUnknown(trail, true); - } - - // Creates merged UnknownAttributeSet. - // Scans over the args collection, determines if there matches to unknown - // patterns and returns the (possibly empty) collection. - cel::AttributeSet CheckForUnknowns(absl::Span args, - bool use_partial) const; - - // Creates merged UnknownValue. - // Scans over the args collection, merges any UnknownValues found. - // Returns the merged UnknownValue or nullopt if not found. - absl::optional MergeUnknowns( - absl::Span args, google::protobuf::Arena* arena) const; - - // Creates a merged UnknownValue from two unknown values. - cel::UnknownValue MergeUnknownValues(const cel::UnknownValue& left, - const cel::UnknownValue& right, - google::protobuf::Arena* arena) const; - - // Creates merged UnknownValue. - // Merges together UnknownValues found in the args - // along with attributes from attr that match the configured unknown patterns - // Returns returns the merged UnknownValue if available or nullopt. - absl::optional IdentifyAndMergeUnknowns( - absl::Span args, absl::Span attrs, - bool use_partial, google::protobuf::Arena* arena) const; - - // Create an initial UnknownSet from a single attribute. - cel::UnknownValue CreateUnknownSet(cel::Attribute attr, - google::protobuf::Arena* arena) const; - - // Factory function for missing attribute errors. - absl::StatusOr CreateMissingAttributeError( - const cel::Attribute& attr, google::protobuf::Arena* arena) const; - - // Create an initial UnknownSet from a single missing function call. - cel::UnknownValue CreateUnknownSet( - const cel::FunctionDescriptor& fn_descriptor, int64_t expr_id, - absl::Span args, google::protobuf::Arena* arena) const; - - Accumulator CreateAccumulator() const ABSL_ATTRIBUTE_LIFETIME_BOUND { - return Accumulator(*this); - } - - void set_matcher( - const cel::runtime_internal::AttributeMatcher* absl_nonnull matcher) { - matcher_ = matcher; - } - - private: - // Workaround friend visibility. - void Add(Accumulator& a, const cel::UnknownValue& v) const; - void Add(Accumulator& a, const AttributeTrail& attr) const; - - DefaultAttributeMatcher default_matcher_; - const cel::runtime_internal::AttributeMatcher* absl_nonnull matcher_; -}; - -} // namespace google::api::expr::runtime - -#endif // THIRD_PARTY_CEL_CPP_EVAL_EVAL_UNKNOWNS_UTILITY_H_ diff --git a/eval/eval/attribute_utility_test.cc b/eval/eval/attribute_utility_test.cc deleted file mode 100644 index cf65bd8c7..000000000 --- a/eval/eval/attribute_utility_test.cc +++ /dev/null @@ -1,218 +0,0 @@ -#include "eval/eval/attribute_utility.h" - -#include -#include - -#include "absl/types/optional.h" -#include "absl/types/span.h" -#include "base/attribute.h" -#include "base/attribute_set.h" -#include "common/unknown.h" -#include "common/value.h" -#include "eval/eval/attribute_trail.h" -#include "eval/public/cel_attribute.h" -#include "eval/public/cel_value.h" -#include "eval/public/unknown_attribute_set.h" -#include "eval/public/unknown_set.h" -#include "internal/testing.h" -#include "runtime/internal/attribute_matcher.h" -#include "google/protobuf/arena.h" - -namespace google::api::expr::runtime { - -using ::cel::AttributeSet; - -using ::cel::UnknownValue; -using ::cel::Value; -using ::testing::Eq; -using ::testing::SizeIs; -using ::testing::UnorderedPointwise; - -class AttributeUtilityTest : public ::testing::Test { - public: - AttributeUtilityTest() = default; - - protected: - google::protobuf::Arena arena_; -}; - -absl::Span NoPatterns() { return {}; } - -TEST_F(AttributeUtilityTest, UnknownsUtilityCheckUnknowns) { - std::vector unknown_patterns = { - CelAttributePattern("unknown0", {CreateCelAttributeQualifierPattern( - CelValue::CreateInt64(1))}), - CelAttributePattern("unknown0", {CreateCelAttributeQualifierPattern( - CelValue::CreateInt64(2))}), - CelAttributePattern("unknown1", {}), - CelAttributePattern("unknown2", {}), - }; - - std::vector missing_attribute_patterns; - - AttributeUtility utility(unknown_patterns, missing_attribute_patterns); - // no match for void trail - ASSERT_FALSE(utility.CheckForUnknown(AttributeTrail(), true)); - ASSERT_FALSE(utility.CheckForUnknown(AttributeTrail(), false)); - - AttributeTrail unknown_trail0("unknown0"); - - { ASSERT_FALSE(utility.CheckForUnknown(unknown_trail0, false)); } - - { ASSERT_TRUE(utility.CheckForUnknown(unknown_trail0, true)); } - - { - ASSERT_TRUE(utility.CheckForUnknown( - unknown_trail0.Step( - CreateCelAttributeQualifier(CelValue::CreateInt64(1))), - false)); - } - - { - ASSERT_TRUE(utility.CheckForUnknown( - unknown_trail0.Step( - CreateCelAttributeQualifier(CelValue::CreateInt64(1))), - true)); - } -} - -TEST_F(AttributeUtilityTest, UnknownsUtilityMergeUnknownsFromValues) { - std::vector unknown_patterns; - - std::vector missing_attribute_patterns; - - CelAttribute attribute0("unknown0", {}); - CelAttribute attribute1("unknown1", {}); - - AttributeUtility utility(unknown_patterns, missing_attribute_patterns); - - UnknownValue unknown_set0 = cel::common_internal::MakeUnknownValue( - cel::Unknown(AttributeSet({attribute0})), &arena_); - UnknownValue unknown_set1 = cel::common_internal::MakeUnknownValue( - cel::Unknown(AttributeSet({attribute1})), &arena_); - - std::vector values = { - unknown_set0, - unknown_set1, - cel::BoolValue(true), - cel::IntValue(1), - }; - - absl::optional unknown_set = - utility.MergeUnknowns(values, &arena_); - ASSERT_TRUE(unknown_set.has_value()); - EXPECT_THAT((*unknown_set).ToAttributeSet(), - UnorderedPointwise( - Eq(), std::vector{attribute0, attribute1})); -} - -TEST_F(AttributeUtilityTest, UnknownsUtilityCheckForUnknownsFromAttributes) { - std::vector unknown_patterns = { - CelAttributePattern("unknown0", - {CelAttributeQualifierPattern::CreateWildcard()}), - }; - - std::vector missing_attribute_patterns; - - AttributeTrail trail0("unknown0"); - AttributeTrail trail1("unknown1"); - - CelAttribute attribute1("unknown1", {}); - UnknownSet unknown_set1(UnknownAttributeSet({attribute1})); - - AttributeUtility utility(unknown_patterns, missing_attribute_patterns); - - UnknownSet unknown_attr_set(utility.CheckForUnknowns( - { - AttributeTrail(), // To make sure we handle empty trail gracefully. - trail0.Step(CreateCelAttributeQualifier(CelValue::CreateInt64(1))), - trail0.Step(CreateCelAttributeQualifier(CelValue::CreateInt64(2))), - }, - false)); - - UnknownSet unknown_set(unknown_set1, unknown_attr_set); - - ASSERT_THAT(unknown_set.unknown_attributes(), SizeIs(3)); -} - -TEST_F(AttributeUtilityTest, UnknownsUtilityCheckForMissingAttributes) { - std::vector unknown_patterns; - - std::vector missing_attribute_patterns; - - AttributeTrail trail("destination"); - trail = - trail.Step(CreateCelAttributeQualifier(CelValue::CreateStringView("ip"))); - - AttributeUtility utility0(unknown_patterns, missing_attribute_patterns); - EXPECT_FALSE(utility0.CheckForMissingAttribute(trail)); - - missing_attribute_patterns.push_back(CelAttributePattern( - "destination", - {CreateCelAttributeQualifierPattern(CelValue::CreateStringView("ip"))})); - - AttributeUtility utility1(unknown_patterns, missing_attribute_patterns); - EXPECT_TRUE(utility1.CheckForMissingAttribute(trail)); -} - -TEST_F(AttributeUtilityTest, CreateUnknownSet) { - AttributeTrail trail("destination"); - trail = - trail.Step(CreateCelAttributeQualifier(CelValue::CreateStringView("ip"))); - - std::vector empty_patterns; - AttributeUtility utility(empty_patterns, empty_patterns); - - UnknownValue set = utility.CreateUnknownSet(trail.attribute(), &arena_); - ASSERT_THAT(set.ToAttributeSet(), SizeIs(1)); - ASSERT_OK_AND_ASSIGN(auto elem, set.ToAttributeSet().begin()->AsString()); - EXPECT_EQ(elem, "destination.ip"); -} - -class FakeMatcher : public cel::runtime_internal::AttributeMatcher { - private: - using MatchResult = cel::runtime_internal::AttributeMatcher::MatchResult; - - public: - MatchResult CheckForUnknown(const cel::Attribute& attr) const override { - std::string attr_str = attr.AsString().value_or(""); - if (attr_str == "device.foo") { - return MatchResult::FULL; - } else if (attr_str == "device") { - return MatchResult::PARTIAL; - } - return MatchResult::NONE; - } - - MatchResult CheckForMissing(const cel::Attribute& attr) const override { - std::string attr_str = attr.AsString().value_or(""); - - if (attr_str == "device2.foo") { - return MatchResult::FULL; - } else if (attr_str == "device2") { - return MatchResult::PARTIAL; - } - return MatchResult::NONE; - } -}; - -TEST_F(AttributeUtilityTest, CustomMatcher) { - AttributeTrail trail("device"); - - AttributeUtility utility(NoPatterns(), NoPatterns()); - FakeMatcher matcher; - utility.set_matcher(&matcher); - EXPECT_TRUE(utility.CheckForUnknownPartial(trail)); - EXPECT_FALSE(utility.CheckForUnknownExact(trail)); - - trail = trail.Step(cel::AttributeQualifier::OfString("foo")); - EXPECT_TRUE(utility.CheckForUnknownExact(trail)); - EXPECT_TRUE(utility.CheckForUnknownPartial(trail)); - - trail = AttributeTrail("device2"); - EXPECT_FALSE(utility.CheckForMissingAttribute(trail)); - trail = trail.Step(cel::AttributeQualifier::OfString("foo")); - EXPECT_TRUE(utility.CheckForMissingAttribute(trail)); -} - -} // namespace google::api::expr::runtime diff --git a/eval/eval/comprehension_slots.h b/eval/eval/comprehension_slots.h index 5a7ce21cc..d7e71431b 100644 --- a/eval/eval/comprehension_slots.h +++ b/eval/eval/comprehension_slots.h @@ -25,9 +25,8 @@ #include "absl/base/nullability.h" #include "absl/container/fixed_array.h" #include "absl/log/absl_check.h" -#include "absl/types/optional.h" +#include "common/internal/attribute_trail.h" #include "common/value.h" -#include "eval/eval/attribute_trail.h" namespace google::api::expr::runtime { @@ -51,13 +50,14 @@ class ComprehensionSlot final { return &value_; } - const AttributeTrail& attribute() const ABSL_ATTRIBUTE_LIFETIME_BOUND { + const cel::common_internal::AttributeTrail& attribute() const + ABSL_ATTRIBUTE_LIFETIME_BOUND { ABSL_DCHECK(Has()); return attribute_; } - AttributeTrail* absl_nonnull mutable_attribute() + cel::common_internal::AttributeTrail* absl_nonnull mutable_attribute() ABSL_ATTRIBUTE_LIFETIME_BOUND { ABSL_DCHECK(Has()); @@ -66,31 +66,35 @@ class ComprehensionSlot final { bool Has() const { return has_; } - void Set() { Set(cel::NullValue(), absl::nullopt); } + void Set() { Set(cel::NullValue(), cel::common_internal::AttributeTrail()); } template void Set(V&& value) { - Set(std::forward(value), absl::nullopt); + Set(std::forward(value), cel::common_internal::AttributeTrail()); } - template - void Set(V&& value, A&& attribute) { + template + void Set(V&& value, cel::common_internal::AttributeTrail attribute) { + ABSL_DCHECK(!attribute.IsFullMatch()) + << "Full matches should not be pushed onto the value stack, they " + "should be handled directly and converted to an unknown value or " + "missing error value"; value_ = std::forward(value); - attribute_ = std::forward(attribute); + attribute_ = attribute; has_ = true; } void Clear() { if (has_) { value_ = cel::NullValue(); - attribute_ = absl::nullopt; + attribute_ = cel::common_internal::AttributeTrail(); has_ = false; } } private: cel::Value value_; - AttributeTrail attribute_; + cel::common_internal::AttributeTrail attribute_; bool has_ = false; }; @@ -139,12 +143,13 @@ class ComprehensionSlots final { template void Set(size_t index, V&& value) { - Set(index, std::forward(value), absl::nullopt); + Set(index, std::forward(value), cel::common_internal::AttributeTrail()); } - template - void Set(size_t index, V&& value, A&& attribute) { - Get(index)->Set(std::forward(value), std::forward(attribute)); + template + void Set(size_t index, V&& value, + cel::common_internal::AttributeTrail attribute) { + Get(index)->Set(std::forward(value), attribute); } size_t size() const { return slots_.size(); } diff --git a/eval/eval/comprehension_slots_test.cc b/eval/eval/comprehension_slots_test.cc index 3dba2f80e..d3ee67d70 100644 --- a/eval/eval/comprehension_slots_test.cc +++ b/eval/eval/comprehension_slots_test.cc @@ -14,7 +14,6 @@ #include "eval/eval/comprehension_slots.h" -#include "base/attribute.h" #include "base/type_provider.h" #include "common/memory.h" #include "common/value.h" @@ -23,9 +22,6 @@ namespace google::api::expr::runtime { -using ::cel::Attribute; - -using ::absl_testing::IsOkAndHolds; using ::cel::MemoryManagerRef; using ::cel::StringValue; using ::cel::TypeProvider; @@ -38,8 +34,7 @@ TEST(ComprehensionSlots, Basic) { ComprehensionSlots::Slot* slot0 = slots.Get(0); EXPECT_FALSE(slot0->Has()); - slots.Set(0, cel::StringValue::WrapUnsafe("abcd"), - AttributeTrail(Attribute("fake_attr"))); + slots.Set(0, cel::StringValue::WrapUnsafe("abcd"), AttributeTrail()); ASSERT_TRUE(slot0->Has()); @@ -49,14 +44,10 @@ TEST(ComprehensionSlots, Basic) { })) << "value is 'abcd'"; - EXPECT_THAT(slot0->attribute().attribute().AsString(), - IsOkAndHolds("fake_attr")); - slots.ClearSlot(0); EXPECT_FALSE(slot0->Has()); - slots.Set(3, cel::StringValue::WrapUnsafe("abcd"), - AttributeTrail(Attribute("fake_attr"))); + slots.Set(3, cel::StringValue::WrapUnsafe("abcd"), AttributeTrail()); auto* slot3 = slots.Get(3); diff --git a/eval/eval/comprehension_step.cc b/eval/eval/comprehension_step.cc index 3532c1419..7a05043a7 100644 --- a/eval/eval/comprehension_step.cc +++ b/eval/eval/comprehension_step.cc @@ -28,7 +28,6 @@ enum class IterableKind { }; using ::cel::AttributeQualifier; -using ::cel::UnknownValue; using ::cel::Value; using ::cel::ValueIteratorPtr; using ::cel::ValueKind; @@ -67,10 +66,9 @@ void ComprehensionInitStep::Evaluate(ExecutionFrame* frame) const { if (frame->enable_unknowns() && top.IsMap()) { const AttributeTrail& top_attr = frame->value_stack().PeekAttribute(); - if (frame->attribute_utility().CheckForUnknownPartial(top_attr)) { - frame->value_stack().PopAndPush( - frame->attribute_utility().CreateUnknownSet(top_attr.attribute(), - frame->arena())); + if (auto unknown = top_attr.PartialUnknownMatch(frame->unknown_tree()); + unknown.has_value()) { + frame->value_stack().PopAndPush(*unknown); frame->JumpToOrAbort(error_jump_offset_); return; } @@ -163,12 +161,12 @@ void ComprehensionNextStep::Evaluate1(ExecutionFrame* frame) const { frame->Abort(std::move(inc_status)); return; } - *iter_slot->mutable_attribute() = frame->value_stack().PeekAttribute().Step( - AttributeQualifierFromValue(*key)); - if (frame->attribute_utility().CheckForUnknownExact( - iter_slot->attribute())) { - *iter_slot->mutable_value() = frame->attribute_utility().CreateUnknownSet( - iter_slot->attribute().attribute(), frame->arena()); + cel::AttributeQualifier qualifier = AttributeQualifierFromValue(*key); + if (qualifier) { + static_cast( + frame->value_stack().PeekAttribute().Match( + qualifier, *iter_slot->mutable_value(), + *iter_slot->mutable_attribute(), frame->unknown_tree())); } } else { absl::StatusOr ok = entry.iterator->Next1( @@ -240,14 +238,14 @@ void ComprehensionNextStep::Evaluate2(ExecutionFrame* frame) const { return; } if (frame->enable_unknowns()) { - *iter_slot->mutable_attribute() = *iter2_slot->mutable_attribute() = - frame->value_stack().PeekAttribute().Step( - AttributeQualifierFromValue(iter_slot->value())); - if (frame->attribute_utility().CheckForUnknownExact( - iter2_slot->attribute())) { - *iter2_slot->mutable_value() = - frame->attribute_utility().CreateUnknownSet( - iter2_slot->attribute().attribute(), frame->arena()); + cel::AttributeQualifier qualifier = + AttributeQualifierFromValue(iter_slot->value()); + if (qualifier) { + static_cast( + frame->value_stack().PeekAttribute().Match( + qualifier, *iter2_slot->mutable_value(), + *iter_slot->mutable_attribute(), frame->unknown_tree())); + *iter2_slot->mutable_attribute() = iter_slot->attribute(); } } } diff --git a/eval/eval/container_access_step.cc b/eval/eval/container_access_step.cc index 03fcb37d8..5e62cb7c6 100644 --- a/eval/eval/container_access_step.cc +++ b/eval/eval/container_access_step.cc @@ -2,6 +2,7 @@ #include #include +#include #include #include "absl/log/absl_check.h" @@ -16,7 +17,6 @@ #include "common/value.h" #include "common/value_kind.h" #include "eval/eval/attribute_trail.h" -#include "eval/eval/attribute_utility.h" #include "eval/eval/evaluator_core.h" #include "eval/eval/expression_step_base.h" #include "eval/eval/expression_step_logic.h" @@ -28,7 +28,6 @@ namespace google::api::expr::runtime { namespace { -using ::cel::AttributeQualifier; using ::cel::ErrorValue; using ::cel::IntValue; using ::cel::ListValue; @@ -56,7 +55,7 @@ absl::optional CelNumberFromValue(const Value& value) { } absl::Status CheckMapKeyType(const Value& key) { - ValueKind kind = key->kind(); + ValueKind kind = key.kind(); switch (kind) { case ValueKind::kString: case ValueKind::kInt64: @@ -69,19 +68,21 @@ absl::Status CheckMapKeyType(const Value& key) { } } -AttributeQualifier AttributeQualifierFromValue(const Value& v) { - switch (v->kind()) { +cel::AttributeQualifierView AttributeQualifierViewFromValue( + const Value& v, std::string& scratch) { + switch (v.kind()) { case ValueKind::kString: - return AttributeQualifier::OfString(v.GetString().ToString()); + return cel::AttributeQualifierView::OfString( + v.GetString().ToStringView(&scratch)); case ValueKind::kInt64: - return AttributeQualifier::OfInt(v.GetInt().NativeValue()); + return cel::AttributeQualifierView::OfInt(v.GetInt().NativeValue()); case ValueKind::kUint64: - return AttributeQualifier::OfUint(v.GetUint().NativeValue()); + return cel::AttributeQualifierView::OfUint(v.GetUint().NativeValue()); case ValueKind::kBool: - return AttributeQualifier::OfBool(v.GetBool().NativeValue()); + return cel::AttributeQualifierView::OfBool(v.GetBool().NativeValue()); default: // Non-matching qualifier. - return AttributeQualifier(); + return cel::AttributeQualifierView(); } } @@ -230,31 +231,32 @@ void PerformLookup(ExecutionFrameBase& frame, const Value& container, bool enable_optional_types, Value& result, AttributeTrail& trail) { if (frame.unknown_processing_enabled()) { - AttributeUtility::Accumulator unknowns = - frame.attribute_utility().CreateAccumulator(); + UnknownAccumulator unknowns(frame.unknown_tree()); unknowns.MaybeAdd(container); unknowns.MaybeAdd(key); if (!unknowns.IsEmpty()) { - result = std::move(unknowns).Build(frame.arena()); + result = std::move(unknowns).Build(); return; } - trail = container_trail.Step(AttributeQualifierFromValue(key)); - - if (frame.attribute_utility().CheckForUnknownExact(trail)) { - result = frame.attribute_utility().CreateUnknownSet(trail.attribute(), - frame.arena()); + std::string scratch; + cel::AttributeQualifierView qualifier = + AttributeQualifierViewFromValue(key, scratch); + if (qualifier && container_trail.Match( + qualifier, result, trail, frame.unknown_tree())) { return; } } if (container.IsError()) { result = container; + trail = AttributeTrail(); return; } if (key.IsError()) { result = key; + trail = AttributeTrail(); return; } diff --git a/eval/eval/container_access_step_test.cc b/eval/eval/container_access_step_test.cc index a298c28e3..1b6f5e67d 100644 --- a/eval/eval/container_access_step_test.cc +++ b/eval/eval/container_access_step_test.cc @@ -307,19 +307,6 @@ TEST_F(ContainerAccessStepTest, TestListIndexAccessUnknown) { ASSERT_TRUE(result.IsUnknownSet()); } -TEST_F(ContainerAccessStepTest, TestListUnknownKey) { - ContainerBackedListImpl cel_list({CelValue::CreateInt64(1), - CelValue::CreateInt64(2), - CelValue::CreateInt64(3)}); - - UnknownSet unknown_set; - CelValue result = - EvaluateAttribute(CelValue::CreateList(&cel_list), - CelValue::CreateUnknownSet(&unknown_set), true, true); - - ASSERT_TRUE(result.IsUnknownSet()); -} - TEST_F(ContainerAccessStepTest, TestMapInvalidKey) { const std::string kKey0 = "testkey0"; const std::string kKey1 = "testkey1"; @@ -339,31 +326,6 @@ TEST_F(ContainerAccessStepTest, TestMapInvalidKey) { HasSubstr("Invalid map key type: 'double'"))); } -TEST_F(ContainerAccessStepTest, TestMapUnknownKey) { - const std::string kKey0 = "testkey0"; - const std::string kKey1 = "testkey1"; - const std::string kKey2 = "testkey2"; - Struct cel_struct; - (*cel_struct.mutable_fields())[kKey0].set_string_value("value0"); - (*cel_struct.mutable_fields())[kKey1].set_string_value("value1"); - (*cel_struct.mutable_fields())[kKey2].set_string_value("value2"); - - UnknownSet unknown_set; - CelValue result = - EvaluateAttribute(CelProtoWrapper::CreateMessage(&cel_struct, &arena_), - CelValue::CreateUnknownSet(&unknown_set), true, true); - - ASSERT_TRUE(result.IsUnknownSet()); -} - -TEST_F(ContainerAccessStepTest, TestUnknownContainer) { - UnknownSet unknown_set; - CelValue result = EvaluateAttribute(CelValue::CreateUnknownSet(&unknown_set), - CelValue::CreateInt64(1), true, true); - - ASSERT_TRUE(result.IsUnknownSet()); -} - TEST_F(ContainerAccessStepTest, TestInvalidContainerType) { CelValue result = EvaluateAttribute(CelValue::CreateInt64(1), CelValue::CreateInt64(0), true, true); diff --git a/eval/eval/create_list_step.cc b/eval/eval/create_list_step.cc index d812486f2..1fb2bd7d1 100644 --- a/eval/eval/create_list_step.cc +++ b/eval/eval/create_list_step.cc @@ -8,12 +8,11 @@ #include "absl/container/flat_hash_set.h" #include "absl/log/absl_check.h" #include "absl/status/status.h" -#include "absl/types/optional.h" #include "absl/types/span.h" +#include "common/internal/attribute_trail.h" #include "common/optional_ref.h" #include "common/value.h" #include "common/values/list_value_builder.h" -#include "eval/eval/attribute_utility.h" #include "eval/eval/evaluator_core.h" namespace google::api::expr::runtime { @@ -22,7 +21,6 @@ namespace { using ::cel::ErrorValue; using ::cel::ListValueBuilderPtr; -using ::cel::UnknownValue; using ::cel::Value; using ::cel::common_internal::NewListValueBuilder; @@ -73,12 +71,11 @@ void EvaluateListStepImpl(const StepInfo& step, ExecutionFrame& frame) { } if (frame.enable_unknowns()) { - absl::optional unknown_set = - frame.attribute_utility().IdentifyAndMergeUnknowns( + if (auto unknown = cel::common_internal::PartiallyIdentityAndMergeUnknowns( args, frame.value_stack().GetAttributeSpan(num_elements), - /*use_partial=*/true, frame.arena()); - if (unknown_set.has_value()) { - frame.value_stack().PopAndPush(num_elements, std::move(*unknown_set)); + frame.unknown_tree()); + unknown.has_value()) { + frame.value_stack().PopAndPush(num_elements, *unknown); return; } } diff --git a/eval/eval/create_map_step.cc b/eval/eval/create_map_step.cc index 1b7d994a7..145589c32 100644 --- a/eval/eval/create_map_step.cc +++ b/eval/eval/create_map_step.cc @@ -23,7 +23,7 @@ #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" -#include "absl/types/optional.h" +#include "common/internal/attribute_trail.h" #include "common/value.h" #include "common/values/map_value_builder.h" #include "eval/eval/evaluator_core.h" @@ -37,7 +37,6 @@ namespace { using ::cel::ErrorValueReturn; using ::cel::MapValueBuilderPtr; -using ::cel::UnknownValue; using ::cel::Value; using ::cel::common_internal::NewMapValueBuilder; using ::cel::common_internal::NewMutableMapValue; @@ -70,12 +69,11 @@ absl::StatusOr CreateStructStepForMap::DoEvaluate( } if (frame->enable_unknowns()) { - absl::optional unknown_set = - frame->attribute_utility().IdentifyAndMergeUnknowns( - args, frame->value_stack().GetAttributeSpan(args.size()), true, - frame->arena()); - if (unknown_set.has_value()) { - return *unknown_set; + if (auto unknown = cel::common_internal::PartiallyIdentityAndMergeUnknowns( + args, frame->value_stack().GetAttributeSpan(args.size()), + frame->unknown_tree()); + unknown.has_value()) { + return *unknown; } } diff --git a/eval/eval/create_struct_step.cc b/eval/eval/create_struct_step.cc index 5f1aa3f2b..cdeabb000 100644 --- a/eval/eval/create_struct_step.cc +++ b/eval/eval/create_struct_step.cc @@ -38,7 +38,6 @@ namespace { using ::cel::ErrorValue; using ::cel::StructValueBuilderInterface; -using ::cel::UnknownValue; using ::cel::Value; // `CreateStruct` implementation for message/struct. @@ -74,12 +73,11 @@ absl::StatusOr CreateStructStepForStruct::DoEvaluate( } if (frame->enable_unknowns()) { - absl::optional unknown_set = - frame->attribute_utility().IdentifyAndMergeUnknowns( + if (auto unknown = cel::common_internal::PartiallyIdentityAndMergeUnknowns( args, frame->value_stack().GetAttributeSpan(entries_size), - /*use_partial=*/true, frame->arena()); - if (unknown_set.has_value()) { - return *unknown_set; + frame->unknown_tree()); + unknown.has_value()) { + return *unknown; } } diff --git a/eval/eval/equality_steps.cc b/eval/eval/equality_steps.cc index 5f3a6b58f..8e4646cf8 100644 --- a/eval/eval/equality_steps.cc +++ b/eval/eval/equality_steps.cc @@ -54,11 +54,12 @@ absl::StatusOr EvaluateEquality( } if (frame.unknown_processing_enabled()) { - auto accu = frame.attribute_utility().CreateAccumulator(); - accu.MaybeAdd(lhs, lhs_attr); - accu.MaybeAdd(rhs, rhs_attr); - if (!accu.IsEmpty()) { - return std::move(accu).Build(frame.arena()); + UnknownAccumulator accumulator(frame.unknown_tree()); + accumulator.MaybeAdd(lhs, lhs_attr); + accumulator.MaybeAdd(rhs, rhs_attr); + if (auto accumulated = std::move(accumulator).Accumulate(); + accumulated.has_value()) { + return *accumulated; } } @@ -142,11 +143,12 @@ absl::StatusOr EvaluateIn(ExecutionFrameBase& frame, const Value& item, } if (frame.unknown_processing_enabled()) { - auto accu = frame.attribute_utility().CreateAccumulator(); - accu.MaybeAdd(item, item_attr); - accu.MaybeAdd(container, container_attr); - if (!accu.IsEmpty()) { - return std::move(accu).Build(frame.arena()); + UnknownAccumulator accumulator(frame.unknown_tree()); + accumulator.MaybeAdd(item, item_attr); + accumulator.MaybeAdd(container, container_attr); + if (auto accumulated = std::move(accumulator).Accumulate(); + accumulated.has_value()) { + return *accumulated; } } if (container.IsList()) { diff --git a/eval/eval/equality_steps_test.cc b/eval/eval/equality_steps_test.cc index 4ebfe10ef..fe9982dc1 100644 --- a/eval/eval/equality_steps_test.cc +++ b/eval/eval/equality_steps_test.cc @@ -16,17 +16,17 @@ #include #include -#include #include "absl/base/nullability.h" #include "absl/log/absl_check.h" #include "absl/status/status.h" -#include "absl/status/status_matchers.h" #include "base/attribute.h" +#include "base/attribute_set.h" +#include "common/internal/attribute_trail.h" +#include "common/unknown.h" #include "common/value.h" #include "common/value_kind.h" #include "common/value_testing.h" -#include "eval/eval/attribute_trail.h" #include "eval/eval/evaluator_core.h" #include "eval/eval/expression_step_logic.h" #include "internal/testing.h" @@ -40,8 +40,6 @@ namespace google::api::expr::runtime { namespace { -using ::absl_testing::IsOk; -using ::cel::Attribute; using ::cel::DoubleValue; using ::cel::ErrorValue; using ::cel::IntValue; @@ -53,87 +51,25 @@ using ::cel::test::ValueKindIs; class ValueStep : public ExpressionStepLogic { public: - ValueStep(Value value, Attribute attr) - : ExpressionStepLogic(), - value_(std::move(value)), - attr_(std::move(attr)) {} explicit ValueStep(Value value) - : ExpressionStepLogic(), - value_(std::move(value)), - attr_() {} + : ExpressionStepLogic(), value_(std::move(value)) {} void Evaluate(ExecutionFrame* frame) const override { - frame->value_stack().Push(value_, attr_); + if (value_.IsUnknown()) { + frame->value_stack().Push(cel::common_internal::GraftUnknowns( + cel::common_internal::MakeUnknownValue( + cel::Unknown(cel::AttributeSet({cel::Attribute("foo")})), + frame->arena()), + frame->unknown_tree())); + } else { + frame->value_stack().Push(value_); + } } private: Value value_; - AttributeTrail attr_; }; -TEST(IterativeTest, PartialAttrUnknown) { - cel::Activation activation; - google::protobuf::Arena arena; - cel::RuntimeOptions opts; - opts.unknown_processing = cel::UnknownProcessingOptions::kAttributeOnly; - cel::runtime_internal::RuntimeTypeProvider type_provider( - cel::internal::GetTestingDescriptorPool()); - - FlatExpressionEvaluatorState state( - /*value_stack_size=*/5, - /*comprehension_slot_count=*/0, type_provider, - cel::internal::GetTestingDescriptorPool(), - cel::internal::GetTestingMessageFactory(), &arena); - - ExecutionPath steps; - steps.push_back(ExpressionStep::MakeGenericStep( - std::make_unique(IntValue(1), cel::Attribute("foo")))); - steps.push_back(ExpressionStep::MakeGenericStep( - std::make_unique(IntValue(2)))); - steps.push_back(ExpressionStep::MakeFastEqualStep()); - - ASSERT_THAT(activation.SetUnknownPatterns({cel::AttributePattern( - "foo", {cel::AttributeQualifierPattern::OfString("bar")})}), - IsOk()); - - ExecutionFrame frame(steps, activation, opts, state); - - ASSERT_OK_AND_ASSIGN(Value result, frame.Evaluate()); - - EXPECT_THAT(result, ValueKindIs(ValueKind::kUnknown)); -} - -TEST(IterativeTest, PartialAttrUnknownDisabled) { - cel::Activation activation; - google::protobuf::Arena arena; - cel::RuntimeOptions opts; - opts.unknown_processing = cel::UnknownProcessingOptions::kDisabled; - cel::runtime_internal::RuntimeTypeProvider type_provider( - cel::internal::GetTestingDescriptorPool()); - - FlatExpressionEvaluatorState state( - /*value_stack_size=*/5, - /*comprehension_slot_count=*/0, type_provider, - cel::internal::GetTestingDescriptorPool(), - cel::internal::GetTestingMessageFactory(), &arena); - - ExecutionPath steps; - steps.push_back(ExpressionStep::MakeGenericStep( - std::make_unique(IntValue(1), cel::Attribute("foo")))); - steps.push_back(ExpressionStep::MakeGenericStep( - std::make_unique(IntValue(2)))); - steps.push_back(ExpressionStep::MakeFastEqualStep()); - - ASSERT_THAT(activation.SetUnknownPatterns({cel::AttributePattern( - "foo", {cel::AttributeQualifierPattern::OfString("bar")})}), - IsOk()); - ExecutionFrame frame(steps, activation, opts, state); - - ASSERT_OK_AND_ASSIGN(Value result, frame.Evaluate()); - - EXPECT_THAT(result, BoolValueIs(false)); -} - enum class InputType { kInt1, kInt2, kDouble1, kList, kMap, kError, kUnknown }; enum class OutputType { kBoolTrue, kBoolFalse, kError, kUnknown }; diff --git a/eval/eval/evaluator_core.cc b/eval/eval/evaluator_core.cc index cbc47d136..ca3aae857 100644 --- a/eval/eval/evaluator_core.cc +++ b/eval/eval/evaluator_core.cc @@ -28,6 +28,7 @@ #include "absl/status/statusor.h" #include "absl/strings/str_cat.h" #include "absl/types/span.h" +#include "common/internal/attribute_trail.h" #include "common/value.h" #include "common/value_kind.h" #include "common/values/list_value_builder.h" @@ -136,12 +137,11 @@ void ExpressionStep::EvaluateMutableListAppendStep(ExecutionFrame& frame) { return; } if (frame.unknown_processing_enabled()) { - std::optional unknown_set = - frame.attribute_utility().IdentifyAndMergeUnknowns( + if (auto unknown = cel::common_internal::PartiallyIdentityAndMergeUnknowns( args, frame.value_stack().GetAttributeSpan(2), - /*use_partial=*/true, frame.arena()); - if (unknown_set.has_value()) { - frame.value_stack().PopAndPush(2, std::move(*unknown_set)); + frame.unknown_tree()); + unknown.has_value()) { + frame.value_stack().PopAndPush(2, *unknown); return; } } diff --git a/eval/eval/evaluator_core.h b/eval/eval/evaluator_core.h index e415487c3..a40476945 100644 --- a/eval/eval/evaluator_core.h +++ b/eval/eval/evaluator_core.h @@ -32,11 +32,12 @@ #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" #include "absl/types/span.h" +#include "base/attribute_matcher.h" #include "base/type_provider.h" +#include "common/internal/unknowns.h" #include "common/value.h" #include "common/value_kind.h" #include "common/values/list_value_builder.h" -#include "eval/eval/attribute_utility.h" #include "eval/eval/comprehension_slots.h" #include "eval/eval/comprehension_step.h" #include "eval/eval/create_list_step.h" @@ -49,7 +50,6 @@ #include "eval/eval/lazy_init_step.h" #include "eval/eval/logic_step.h" #include "runtime/activation_interface.h" -#include "runtime/internal/activation_attribute_matcher_access.h" #include "runtime/internal/errors.h" #include "runtime/runtime.h" #include "runtime/runtime_options.h" @@ -461,7 +461,8 @@ class FlatExpressionEvaluatorState { comprehension_slots_(comprehension_slot_count), type_provider_(type_provider), descriptor_pool_(descriptor_pool), - message_factory_(message_factory) {} + message_factory_(message_factory), + unknown_tree_() {} FlatExpressionEvaluatorState( size_t value_stack_size, size_t comprehension_slot_count, @@ -478,7 +479,8 @@ class FlatExpressionEvaluatorState { type_provider_(type_provider), descriptor_pool_(descriptor_pool), message_factory_(message_factory), - arena_(arena) {} + arena_(arena), + unknown_tree_(arena) {} void Reset(); @@ -506,10 +508,13 @@ class FlatExpressionEvaluatorState { return arena_; } + cel::common_internal::UnknownTree* unknown_tree() { return &unknown_tree_; } + void Rebind(google::protobuf::Arena* absl_nonnull arena, google::protobuf::MessageFactory* absl_nonnull message_factory) { arena_ = arena; message_factory_ = message_factory; + unknown_tree_.Reset(arena); } private: @@ -520,6 +525,7 @@ class FlatExpressionEvaluatorState { const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool_; google::protobuf::MessageFactory* absl_nullability_unknown message_factory_ = nullptr; google::protobuf::Arena* absl_nullability_unknown arena_ = nullptr; + cel::common_internal::UnknownTree unknown_tree_; }; // Context needed for evaluation. This is sufficient for supporting @@ -533,7 +539,8 @@ class ExecutionFrameBase { const cel::TypeProvider& type_provider, const google::protobuf::DescriptorPool* absl_nonnull descriptor_pool, google::protobuf::MessageFactory* absl_nonnull message_factory, - google::protobuf::Arena* absl_nonnull arena) + google::protobuf::Arena* absl_nonnull arena, + cel::common_internal::UnknownTree* unknown_tree) : activation_(&activation), callback_(), options_(&options), @@ -542,13 +549,18 @@ class ExecutionFrameBase { message_factory_(message_factory), arena_(arena), embedder_context_(nullptr), - attribute_utility_(options.unknown_processing != - cel::UnknownProcessingOptions::kDisabled - ? activation.GetUnknownAttributes() - : absl::Span(), - options.enable_missing_attribute_errors - ? activation.GetMissingAttributes() - : absl::Span()), + unknown_tree_(unknown_tree), + unknown_attributes_(options_->unknown_processing != + cel::UnknownProcessingOptions::kDisabled + ? activation.GetUnknownAttributeMatcher() + : cel::EmptyAttributeMatcher()), + known_attributes_(options_->unknown_processing != + cel::UnknownProcessingOptions::kDisabled + ? activation.GetKnownAttributeMatcher() + : cel::EmptyAttributeMatcher()), + missing_attributes_(options_->enable_missing_attribute_errors + ? activation.GetMissingAttributeMatcher() + : cel::EmptyAttributeMatcher()), slots_(&ComprehensionSlots::GetEmptyInstance()), max_iterations_(options.comprehension_max_iterations), iterations_(0), @@ -562,15 +574,7 @@ class ExecutionFrameBase { cel::UnknownProcessingOptions::kDisabled), unknown_function_results_enabled_( options_->unknown_processing == - cel::UnknownProcessingOptions::kAttributeAndFunction) { - if (unknown_processing_enabled()) { - if (auto matcher = cel::runtime_internal:: - ActivationAttributeMatcherAccess::GetAttributeMatcher(activation); - matcher != nullptr) { - attribute_utility_.set_matcher(matcher); - } - } - } + cel::UnknownProcessingOptions::kAttributeAndFunction) {} ExecutionFrameBase(const cel::ActivationInterface& activation, EvaluationListener callback, @@ -580,7 +584,8 @@ class ExecutionFrameBase { google::protobuf::MessageFactory* absl_nonnull message_factory, google::protobuf::Arena* absl_nonnull arena, const cel::EmbedderContext* absl_nullable embedder_context, - ComprehensionSlots& slots) + ComprehensionSlots& slots, + cel::common_internal::UnknownTree* unknown_tree) : activation_(&activation), callback_(std::move(callback)), options_(&options), @@ -589,13 +594,18 @@ class ExecutionFrameBase { message_factory_(message_factory), arena_(arena), embedder_context_(embedder_context), - attribute_utility_(options.unknown_processing != - cel::UnknownProcessingOptions::kDisabled - ? activation.GetUnknownAttributes() - : absl::Span(), - options.enable_missing_attribute_errors - ? activation.GetMissingAttributes() - : absl::Span()), + unknown_tree_(unknown_tree), + unknown_attributes_(options_->unknown_processing != + cel::UnknownProcessingOptions::kDisabled + ? activation.GetUnknownAttributeMatcher() + : cel::EmptyAttributeMatcher()), + known_attributes_(options_->unknown_processing != + cel::UnknownProcessingOptions::kDisabled + ? activation.GetKnownAttributeMatcher() + : cel::EmptyAttributeMatcher()), + missing_attributes_(options_->enable_missing_attribute_errors + ? activation.GetMissingAttributeMatcher() + : cel::EmptyAttributeMatcher()), slots_(&slots), max_iterations_(options.comprehension_max_iterations), iterations_(0), @@ -609,15 +619,7 @@ class ExecutionFrameBase { cel::UnknownProcessingOptions::kDisabled), unknown_function_results_enabled_( options_->unknown_processing == - cel::UnknownProcessingOptions::kAttributeAndFunction) { - if (unknown_processing_enabled()) { - if (auto matcher = cel::runtime_internal:: - ActivationAttributeMatcherAccess::GetAttributeMatcher(activation); - matcher != nullptr) { - attribute_utility_.set_matcher(matcher); - } - } - } + cel::UnknownProcessingOptions::kAttributeAndFunction) {} const cel::ActivationInterface& activation() const { return *activation_; } @@ -641,8 +643,8 @@ class ExecutionFrameBase { return embedder_context_; } - const AttributeUtility& attribute_utility() const { - return attribute_utility_; + cel::common_internal::UnknownTree* absl_nonnull unknown_tree() const { + return unknown_tree_; } bool attribute_tracking_enabled() const { @@ -661,6 +663,18 @@ class ExecutionFrameBase { return unknown_function_results_enabled_; } + const cel::AttributeMatcher& unknown_attributes() const { + return unknown_attributes_; + } + + const cel::AttributeMatcher& known_attributes() const { + return known_attributes_; + } + + const cel::AttributeMatcher& missing_attributes() const { + return missing_attributes_; + } + ComprehensionSlots& comprehension_slots() { return *slots_; } // Increment iterations and return an error if the iteration budget is @@ -688,7 +702,10 @@ class ExecutionFrameBase { google::protobuf::MessageFactory* absl_nonnull message_factory_; google::protobuf::Arena* absl_nonnull arena_; const cel::EmbedderContext* absl_nullable embedder_context_; - AttributeUtility attribute_utility_; + cel::common_internal::UnknownTree* absl_nonnull unknown_tree_; + const cel::AttributeMatcher& unknown_attributes_; + const cel::AttributeMatcher& known_attributes_; + const cel::AttributeMatcher& missing_attributes_; ComprehensionSlots* absl_nonnull slots_; const int max_iterations_; int iterations_; @@ -716,7 +733,8 @@ class ExecutionFrame : public ExecutionFrameBase { : ExecutionFrameBase(activation, std::move(callback), options, state.type_provider(), state.descriptor_pool(), state.message_factory(), state.arena(), - embedder_context, state.comprehension_slots()), + embedder_context, state.comprehension_slots(), + state.unknown_tree()), pc_(0UL), execution_path_(flat), value_stack_(&state.value_stack()), @@ -732,7 +750,8 @@ class ExecutionFrame : public ExecutionFrameBase { : ExecutionFrameBase(activation, std::move(callback), options, state.type_provider(), state.descriptor_pool(), state.message_factory(), state.arena(), - embedder_context, state.comprehension_slots()), + embedder_context, state.comprehension_slots(), + state.unknown_tree()), pc_(0UL), execution_path_(subexpressions[0]), value_stack_(&state.value_stack()), diff --git a/eval/eval/evaluator_stack.cc b/eval/eval/evaluator_stack.cc index 47c625dac..24da595e1 100644 --- a/eval/eval/evaluator_stack.cc +++ b/eval/eval/evaluator_stack.cc @@ -3,14 +3,12 @@ #include #include #include -#include -#include #include "absl/base/dynamic_annotations.h" #include "absl/base/nullability.h" #include "absl/log/absl_log.h" +#include "common/internal/attribute_trail.h" #include "common/value.h" -#include "eval/eval/attribute_trail.h" #include "internal/new.h" namespace google::api::expr::runtime { @@ -27,7 +25,8 @@ void EvaluatorStack::Grow() { void EvaluatorStack::Reserve(size_t size) { static_assert(alignof(cel::Value) <= __STDCPP_DEFAULT_NEW_ALIGNMENT__); - static_assert(alignof(AttributeTrail) <= __STDCPP_DEFAULT_NEW_ALIGNMENT__); + static_assert(alignof(cel::common_internal::AttributeTrail) <= + __STDCPP_DEFAULT_NEW_ALIGNMENT__); if (max_size_ >= size) { return; @@ -39,10 +38,12 @@ void EvaluatorStack::Reserve(size_t size) { reinterpret_cast(data); cel::Value* absl_nullability_unknown values = values_begin; - AttributeTrail* absl_nullability_unknown attributes_begin = - reinterpret_cast(reinterpret_cast(data) + - AttributesBytesOffset(size)); - AttributeTrail* absl_nullability_unknown attributes = attributes_begin; + cel::common_internal::AttributeTrail* absl_nullability_unknown + attributes_begin = + reinterpret_cast( + reinterpret_cast(data) + AttributesBytesOffset(size)); + cel::common_internal::AttributeTrail* absl_nullability_unknown attributes = + attributes_begin; if (max_size_ > 0) { const size_t n = this->size(); @@ -55,13 +56,9 @@ void EvaluatorStack::Reserve(size_t size) { attributes_begin + size, attributes + m); for (size_t i = 0; i < m; ++i) { - ::new (static_cast(values++)) - cel::Value(std::move(values_begin_[i])); - ::new (static_cast(attributes++)) - AttributeTrail(std::move(attributes_begin_[i])); + *values++ = values_begin_[i]; + *attributes++ = attributes_begin_[i]; } - std::destroy_n(values_begin_, n); - std::destroy_n(attributes_begin_, n); ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(values_begin_, values_begin_ + max_size_, values_, values_begin_ + max_size_); diff --git a/eval/eval/evaluator_stack.h b/eval/eval/evaluator_stack.h index e318f00ac..9ab0c84b3 100644 --- a/eval/eval/evaluator_stack.h +++ b/eval/eval/evaluator_stack.h @@ -3,7 +3,6 @@ #include #include -#include #include #include @@ -13,10 +12,9 @@ #include "absl/base/optimization.h" #include "absl/log/absl_check.h" #include "absl/meta/type_traits.h" -#include "absl/types/optional.h" #include "absl/types/span.h" +#include "common/internal/attribute_trail.h" #include "common/value.h" -#include "eval/eval/attribute_trail.h" #include "internal/align.h" #include "internal/new.h" @@ -34,9 +32,6 @@ class EvaluatorStack { ~EvaluatorStack() { if (max_size() > 0) { - const size_t n = size(); - std::destroy_n(values_begin_, n); - std::destroy_n(attributes_begin_, n); cel::internal::SizedDelete(data_, SizeBytes(max_size_)); } } @@ -97,10 +92,6 @@ class EvaluatorStack { // Dumps the entire stack state as is. void Clear() { if (max_size() > 0) { - const size_t n = size(); - std::destroy_n(values_begin_, n); - std::destroy_n(attributes_begin_, n); - ABSL_ANNOTATE_CONTIGUOUS_CONTAINER( values_begin_, values_begin_ + max_size_, values_, values_begin_); ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(attributes_begin_, @@ -124,10 +115,12 @@ class EvaluatorStack { // Gets the last size attribute trails of the stack. // Checking that stack has enough elements is caller's responsibility. // Please note that calls to Push may invalidate returned Span object. - absl::Span GetAttributeSpan(size_t size) const { + absl::Span GetAttributeSpan( + size_t size) const { ABSL_DCHECK(HasEnough(size)); - return absl::Span(attributes_ - size, size); + return absl::Span( + attributes_ - size, size); } // Peeks the last element of the stack. @@ -148,7 +141,7 @@ class EvaluatorStack { // Peeks the last element of the attribute stack. // Checking that stack is not empty is caller's responsibility. - const AttributeTrail& PeekAttribute() const { + const cel::common_internal::AttributeTrail& PeekAttribute() const { ABSL_DCHECK(HasEnough(1)); return *(attributes_ - 1); @@ -156,7 +149,7 @@ class EvaluatorStack { // Peeks the last element of the attribute stack. // Checking that stack is not empty is caller's responsibility. - AttributeTrail& PeekAttribute() { + cel::common_internal::AttributeTrail& PeekAttribute() { ABSL_DCHECK(HasEnough(1)); return *(attributes_ - 1); @@ -166,9 +159,7 @@ class EvaluatorStack { ABSL_DCHECK(!empty()); --values_; - values_->~Value(); --attributes_; - attributes_->~AttributeTrail(); ABSL_ANNOTATE_CONTIGUOUS_CONTAINER(values_begin_, values_begin_ + max_size_, values_ + 1, values_); @@ -187,12 +178,14 @@ class EvaluatorStack { } } - template , - std::is_convertible>>> - void Push(V&& value, A&& attribute) { + template >>> + void Push(V&& value, cel::common_internal::AttributeTrail attribute) { ABSL_DCHECK(!full()); + ABSL_DCHECK(!attribute.IsFullMatch()) + << "Full matches should not be pushed onto the value stack, they " + "should be handled directly and converted to an unknown value or " + "missing error value"; if (ABSL_PREDICT_FALSE(full())) { Grow(); @@ -204,9 +197,8 @@ class EvaluatorStack { attributes_begin_ + max_size_, attributes_, attributes_ + 1); - ::new (static_cast(values_++)) cel::Value(std::forward(value)); - ::new (static_cast(attributes_++)) - AttributeTrail(std::forward(attribute)); + *values_++ = std::forward(value); + *attributes_++ = attribute; } template (value), absl::nullopt); + Push(std::forward(value), cel::common_internal::AttributeTrail()); } + void Push() { Push(cel::Value(), cel::common_internal::AttributeTrail()); } + // Equivalent to `PopAndPush(1, ...)`. - template , - std::is_convertible>>> - void PopAndPush(V&& value, A&& attribute) { + template >>> + void PopAndPush(V&& value, cel::common_internal::AttributeTrail attribute) { ABSL_DCHECK(!empty()); + ABSL_DCHECK(!attribute.IsFullMatch()) + << "Full matches should not be pushed onto the value stack, they " + "should be handled directly and converted to an unknown value or " + "missing error value"; *(values_ - 1) = std::forward(value); - *(attributes_ - 1) = std::forward(attribute); + *(attributes_ - 1) = attribute; } // Equivalent to `PopAndPush(1, ...)`. @@ -235,38 +231,35 @@ class EvaluatorStack { void PopAndPush(V&& value) { ABSL_DCHECK(!empty()); - PopAndPush(std::forward(value), absl::nullopt); + PopAndPush(std::forward(value), cel::common_internal::AttributeTrail()); } // Equivalent to `Pop(n)` followed by `Push(...)`. Both `V` and `A` MUST NOT // be located on the stack. If this is the case, use SwapAndPop instead. - template , - std::is_convertible>>> - void PopAndPush(size_t n, V&& value, A&& attribute) { + template >>> + void PopAndPush(size_t n, V&& value, + cel::common_internal::AttributeTrail attribute) { if (n > 0) { + ABSL_DCHECK(!attribute.IsFullMatch()) + << "Full matches should not be pushed onto the value stack, they " + "should be handled directly and converted to an unknown value or " + "missing error value"; if constexpr (std::is_same_v>) { ABSL_DCHECK(&value < values_begin_ || &value >= values_begin_ + max_size_) << "Attmpting to push a value about to be popped, use PopAndSwap " "instead."; } - if constexpr (std::is_same_v>) { - ABSL_DCHECK(&attribute < attributes_begin_ || - &attribute >= attributes_begin_ + max_size_) - << "Attmpting to push an attribute about to be popped, use " - "PopAndSwap instead."; - } Pop(n - 1); ABSL_DCHECK(!empty()); *(values_ - 1) = std::forward(value); - *(attributes_ - 1) = std::forward(attribute); + *(attributes_ - 1) = attribute; } else { - Push(std::forward(value), std::forward(attribute)); + Push(std::forward(value), attribute); } } @@ -275,7 +268,8 @@ class EvaluatorStack { template >> void PopAndPush(size_t n, V&& value) { - PopAndPush(n, std::forward(value), absl::nullopt); + PopAndPush(n, std::forward(value), + cel::common_internal::AttributeTrail()); } // Given the top `n` the elements of the stack, swap the `i`th element with @@ -305,7 +299,8 @@ class EvaluatorStack { } static size_t SizeBytes(size_t size) { - return AttributesBytesOffset(size) + (sizeof(AttributeTrail) * size); + return AttributesBytesOffset(size) + + (sizeof(cel::common_internal::AttributeTrail) * size); } void Grow(); @@ -315,8 +310,10 @@ class EvaluatorStack { cel::Value* absl_nullability_unknown values_ = nullptr; cel::Value* absl_nullability_unknown values_begin_ = nullptr; - AttributeTrail* absl_nullability_unknown attributes_ = nullptr; - AttributeTrail* absl_nullability_unknown attributes_begin_ = nullptr; + cel::common_internal::AttributeTrail* absl_nullability_unknown attributes_ = + nullptr; + cel::common_internal::AttributeTrail* absl_nullability_unknown + attributes_begin_ = nullptr; cel::Value* absl_nullability_unknown values_end_ = nullptr; void* absl_nullability_unknown data_ = nullptr; size_t max_size_ = 0; diff --git a/eval/eval/evaluator_stack_test.cc b/eval/eval/evaluator_stack_test.cc index 9ce862d8a..2fcbc4ae7 100644 --- a/eval/eval/evaluator_stack_test.cc +++ b/eval/eval/evaluator_stack_test.cc @@ -2,6 +2,7 @@ #include "base/attribute.h" #include "common/value.h" +#include "eval/eval/attribute_trail.h" #include "internal/testing.h" namespace google::api::expr::runtime { @@ -14,21 +15,17 @@ TEST(EvaluatorStackTest, StackPushPop) { EvaluatorStack stack(10); stack.Push(cel::IntValue(1)); stack.Push(cel::IntValue(2), AttributeTrail()); - stack.Push(cel::IntValue(3), AttributeTrail("name")); + stack.Push(cel::IntValue(3), AttributeTrail()); ASSERT_EQ(stack.Peek().GetInt().NativeValue(), 3); - ASSERT_FALSE(stack.PeekAttribute().empty()); - ASSERT_EQ(stack.PeekAttribute().attribute(), attribute); stack.Pop(1); ASSERT_EQ(stack.Peek().GetInt().NativeValue(), 2); - ASSERT_TRUE(stack.PeekAttribute().empty()); stack.Pop(1); ASSERT_EQ(stack.Peek().GetInt().NativeValue(), 1); - ASSERT_TRUE(stack.PeekAttribute().empty()); } // Test that inner stacks within value stack retain the equality of their sizes. diff --git a/eval/eval/function_step.cc b/eval/eval/function_step.cc index 37763bc9e..fe00b9d8e 100644 --- a/eval/eval/function_step.cc +++ b/eval/eval/function_step.cc @@ -17,6 +17,8 @@ #include "absl/types/span.h" #include "common/expr.h" #include "common/function_descriptor.h" +#include "common/internal/attribute_trail.h" +#include "common/internal/unknowns.h" #include "common/kind.h" #include "common/value.h" #include "common/value_kind.h" @@ -111,14 +113,12 @@ std::vector CheckForPartialUnknowns( std::vector result; result.reserve(args.size()); for (size_t i = 0; i < args.size(); i++) { - const AttributeTrail& trail = attrs.subspan(i, 1)[0]; - - if (frame->attribute_utility().CheckForUnknown(trail, - /*use_partial=*/true)) { - result.push_back(frame->attribute_utility().CreateUnknownSet( - trail.attribute(), frame->arena())); + const AttributeTrail& trail = attrs[i]; + if (auto unknown = trail.PartialUnknownMatch(frame->unknown_tree()); + unknown.has_value()) { + result.push_back(*unknown); } else { - result.push_back(args.at(i)); + result.push_back(args[i]); } } @@ -161,8 +161,14 @@ inline absl::StatusOr Invoke( if (frame.unknown_function_results_enabled() && IsUnknownFunctionResultError(*result)) { - return frame.attribute_utility().CreateUnknownSet( - overload.descriptor, expr_id, args, frame.arena()); + return cel::common_internal::MakeUnknownValue( + cel::common_internal::UnknownValueRep{ + .root = frame.unknown_tree()->Root(), + .attributes = nullptr, + .functions = cel::common_internal::CreateUnknownFunctionSet( + frame.unknown_tree()->Root()->SetFunction( + frame.unknown_tree(), overload.descriptor.name()), + frame.unknown_tree()->GetArena())}); } return result; } @@ -183,10 +189,13 @@ Value NoOverloadResult(absl::string_view name, if (frame.unknown_processing_enabled()) { // Already converted partial unknowns to unknown sets so just merge. - absl::optional unknown_set = - frame.attribute_utility().MergeUnknowns(args, frame.arena()); - if (unknown_set.has_value()) { - return *unknown_set; + UnknownAccumulator accumulator(frame.unknown_tree()); + for (const auto& arg : args) { + accumulator.MaybeAdd(arg); + } + if (auto accumulated = std::move(accumulator).Accumulate(); + accumulated.has_value()) { + return *accumulated; } } @@ -357,6 +366,9 @@ void EvaluateFunctionStep(const Step* step, ExecutionFrame& frame) { frame.Abort(std::move(result).status()); return; } + if (frame.enable_unknowns()) { + cel::common_internal::GraftUnknownsAt(*result, frame.unknown_tree()); + } frame.value_stack().PopAndPush(step->num_arguments_, *std::move(result)); return; } diff --git a/eval/eval/ident_step.cc b/eval/eval/ident_step.cc index db1173101..ff76fd427 100644 --- a/eval/eval/ident_step.cc +++ b/eval/eval/ident_step.cc @@ -1,17 +1,17 @@ #include "eval/eval/ident_step.h" -#include #include -#include "absl/status/status.h" +#include "absl/base/attributes.h" +#include "absl/base/optimization.h" #include "absl/strings/str_cat.h" #include "absl/strings/string_view.h" +#include "common/internal/attribute_trail.h" #include "common/value.h" #include "eval/eval/attribute_trail.h" #include "eval/eval/evaluator_core.h" #include "eval/eval/expression_step_logic.h" #include "eval/internal/errors.h" -#include "internal/status_macros.h" namespace google::api::expr::runtime { @@ -20,40 +20,42 @@ namespace { using ::cel::Value; using ::cel::runtime_internal::CreateError; -absl::Status LookupIdent(absl::string_view name, ExecutionFrameBase& frame, - Value& result, AttributeTrail& attribute) { - if (frame.attribute_tracking_enabled()) { - attribute = AttributeTrail(std::string(name)); - if (frame.missing_attribute_errors_enabled() && - frame.attribute_utility().CheckForMissingAttribute(attribute)) { - CEL_ASSIGN_OR_RETURN( - result, frame.attribute_utility().CreateMissingAttributeError( - attribute.attribute(), frame.arena())); - return absl::OkStatus(); - } - if (frame.unknown_processing_enabled() && - frame.attribute_utility().CheckForUnknownExact(attribute)) { - result = frame.attribute_utility().CreateUnknownSet(attribute.attribute(), - frame.arena()); - return absl::OkStatus(); - } +ABSL_ATTRIBUTE_ALWAYS_INLINE inline void LookupIdent( + absl::string_view name, ExecutionFrame& frame, Value& result, + AttributeTrail& attribute) { + if (frame.attribute_tracking_enabled() && + (AttributeLeadMatch)(frame.unknown_attributes(), frame.known_attributes(), + frame.missing_attributes(), name, result, attribute, + frame.unknown_tree())) { + return; } - CEL_ASSIGN_OR_RETURN( - auto found, frame.activation().FindVariable(name, frame.descriptor_pool(), - frame.message_factory(), - frame.arena(), &result)); + auto status_or_found = frame.activation().FindVariable( + name, frame.descriptor_pool(), frame.message_factory(), frame.arena(), + &result); + if (ABSL_PREDICT_FALSE(!status_or_found.ok())) { + frame.Abort(std::move(status_or_found).status()); + return; + } + if (ABSL_PREDICT_FALSE(frame.attribute_tracking_enabled() && + result.IsUnknown())) { + // This really only happens in our own tests... + result = cel::common_internal::GraftUnknowns(result.GetUnknown(), + frame.unknown_tree()); + attribute = AttributeTrail(); + } - if (found) { - return absl::OkStatus(); + if (*status_or_found) { + return; } + if (frame.attribute_tracking_enabled()) { + attribute = AttributeTrail(); + } result = cel::ErrorValue::From( CreateError(absl::StrCat("No value with name \"", name, "\" found in Activation")), frame.arena()); - - return absl::OkStatus(); } } // namespace @@ -61,13 +63,8 @@ absl::Status LookupIdent(absl::string_view name, ExecutionFrameBase& frame, void EvaluateIdentifierStep(absl::string_view identifier, ExecutionFrame& frame) { frame.value_stack().Push(cel::NullValue()); - if (absl::Status status = - LookupIdent(identifier, frame, frame.value_stack().Peek(), - frame.value_stack().PeekAttribute()); - !status.ok()) { - frame.Abort(std::move(status)); - return; - } + LookupIdent(identifier, frame, frame.value_stack().Peek(), + frame.value_stack().PeekAttribute()); } } // namespace google::api::expr::runtime diff --git a/eval/eval/logic_step.cc b/eval/eval/logic_step.cc index cc0a42cf2..e6a158ab8 100644 --- a/eval/eval/logic_step.cc +++ b/eval/eval/logic_step.cc @@ -21,7 +21,6 @@ namespace { using ::cel::BoolValue; using ::cel::ErrorValue; -using ::cel::UnknownValue; using ::cel::Value; using ::cel::ValueKind; using ::cel::runtime_internal::CreateNoMatchingOverloadError; @@ -35,15 +34,6 @@ void EvaluateNotStep(ExecutionFrame& frame) { } const Value& operand = frame.value_stack().Peek(); - if (frame.unknown_processing_enabled()) { - const AttributeTrail& attribute_trail = frame.value_stack().PeekAttribute(); - if (frame.attribute_utility().CheckForUnknownPartial(attribute_trail)) { - frame.value_stack().PopAndPush(frame.attribute_utility().CreateUnknownSet( - attribute_trail.attribute(), frame.arena())); - return; - } - } - switch (operand.kind()) { case ValueKind::kBool: frame.value_stack().PopAndPush( @@ -122,10 +112,13 @@ void EvaluateBoolLogicStep(BoolLogicKind kind, size_t num_args, // error. if (frame.enable_unknowns()) { // Check if unknown? - std::optional unknown_set = - frame.attribute_utility().MergeUnknowns(args, frame.arena()); - if (unknown_set.has_value()) { - frame.value_stack().PopAndPush(num_args, *std::move(unknown_set)); + UnknownAccumulator accumulator(frame.unknown_tree()); + for (const auto& arg : args) { + accumulator.MaybeAdd(arg); + } + if (auto accumulated = std::move(accumulator).Accumulate(); + accumulated.has_value()) { + frame.value_stack().PopAndPush(num_args, *accumulated); return; } } diff --git a/eval/eval/select_step.cc b/eval/eval/select_step.cc index 297eefa2b..cde7093bb 100644 --- a/eval/eval/select_step.cc +++ b/eval/eval/select_step.cc @@ -1,17 +1,16 @@ #include "eval/eval/select_step.h" #include -#include #include #include #include "absl/base/nullability.h" #include "absl/log/absl_check.h" -#include "absl/log/absl_log.h" #include "absl/status/status.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" +#include "base/attribute.h" #include "common/legacy_value.h" #include "common/type.h" #include "common/value.h" @@ -48,32 +47,6 @@ absl::Status InvalidSelectTargetError() { "Applying SELECT to non-message type"); } -absl::optional CheckForMarkedAttributes(const AttributeTrail& trail, - ExecutionFrameBase& frame) { - if (frame.unknown_processing_enabled() && - frame.attribute_utility().CheckForUnknownExact(trail)) { - return frame.attribute_utility().CreateUnknownSet(trail.attribute(), - frame.arena()); - } - - if (frame.missing_attribute_errors_enabled() && - frame.attribute_utility().CheckForMissingAttribute(trail)) { - auto result = frame.attribute_utility().CreateMissingAttributeError( - trail.attribute(), frame.arena()); - - if (result.ok()) { - return std::move(result).value(); - } - // Invariant broken (an invalid CEL Attribute shouldn't match anything). - // Log and return a CelError. - ABSL_LOG(ERROR) << "Invalid attribute pattern matched select path: " - << result.status().ToString(); // NOLINT: OSS compatibility - return cel::ErrorValue::From(std::move(result).status(), frame.arena()); - } - - return std::nullopt; -} - absl::Status PerformHas(const Value& target, absl::string_view field, const StringValue& field_value, const google::protobuf::DescriptorPool* descriptor_pool, @@ -196,49 +169,41 @@ void SelectStep::Evaluate(ExecutionFrame* frame) const { return; } - const Value& arg = frame->value_stack().Peek(); - const AttributeTrail& trail = frame->value_stack().PeekAttribute(); + Value& arg_and_result = frame->value_stack().Peek(); - if (arg.IsUnknown() || arg.IsError()) { + if (arg_and_result.IsUnknown() || arg_and_result.IsError()) { // Bubble up unknowns and errors. return; } - AttributeTrail result_trail; - - // Handle unknown resolution. - if (frame->attribute_tracking_enabled()) { - result_trail = trail.Step(&field_); - } - absl::optional optional_arg; - if (enable_optional_types_ && arg.IsOptional()) { - optional_arg = arg.GetOptional(); + if (enable_optional_types_ && arg_and_result.IsOptional()) { + optional_arg = arg_and_result.GetOptional(); } - if (!(optional_arg || arg.IsMap() || arg.IsStruct())) { - frame->value_stack().PopAndPush( - cel::ErrorValue::From(InvalidSelectTargetError(), frame->arena()), - std::move(result_trail)); + if (!(optional_arg || arg_and_result.IsMap() || arg_and_result.IsStruct())) { + // Do not bother stepping the attribute trail if the target is incorrect, we + // would not have encountered it anyway. + arg_and_result = + cel::ErrorValue::From(InvalidSelectTargetError(), frame->arena()); return; } - absl::optional marked_attribute_check = - CheckForMarkedAttributes(result_trail, *frame); - if (marked_attribute_check.has_value()) { - frame->value_stack().PopAndPush(std::move(marked_attribute_check).value(), - std::move(result_trail)); + // Handle unknown resolution. + if (frame->attribute_tracking_enabled() && + frame->value_stack().PeekAttribute().Match( + cel::AttributeQualifierView::OfString(field_), arg_and_result, + frame->unknown_tree())) { return; } Value result; if (test_field_presence_) { - const Value* target = &arg; + const Value* target = &arg_and_result; if (optional_arg) { if (!optional_arg->HasValue()) { - frame->value_stack().PopAndPush(cel::BoolValue{false}, - std::move(result_trail)); + arg_and_result = cel::FalseValue(); return; } optional_arg->Value(&result); @@ -252,14 +217,13 @@ void SelectStep::Evaluate(ExecutionFrame* frame) const { frame->Abort(std::move(status)); return; } - frame->value_stack().PopAndPush(std::move(result), std::move(result_trail)); + arg_and_result = result; return; } if (optional_arg) { if (!optional_arg->HasValue()) { - frame->value_stack().PopAndPush(OptionalValue::None(), - std::move(result_trail)); + arg_and_result = OptionalValue::None(); return; } Value value; @@ -271,36 +235,26 @@ void SelectStep::Evaluate(ExecutionFrame* frame) const { if (!status.ok()) { result = ErrorValue::From(std::move(status), frame->arena()); } - frame->value_stack().PopAndPush(std::move(result), std::move(result_trail)); + arg_and_result = result; return; } - if (absl::Status status = - PerformGet(arg, field_, cel::StringValue::WrapUnsafe(field_), - unboxing_option_, frame->descriptor_pool(), - frame->message_factory(), frame->arena(), result); + if (absl::Status status = PerformGet( + arg_and_result, field_, cel::StringValue::WrapUnsafe(field_), + unboxing_option_, frame->descriptor_pool(), frame->message_factory(), + frame->arena(), result); !status.ok()) { frame->Abort(std::move(status)); return; } - frame->value_stack().PopAndPush(std::move(result), std::move(result_trail)); + arg_and_result = result; } bool CheckAttributeTrail(const std::string& field, ExecutionFrame* frame) { - if (!frame->attribute_tracking_enabled()) { - return false; - } - AttributeTrail& attr = frame->value_stack().PeekAttribute(); - attr = attr.Step(&field); - - absl::optional marked_attribute_check = - CheckForMarkedAttributes(attr, *frame); - if (marked_attribute_check.has_value()) { - frame->value_stack().Peek() = std::move(marked_attribute_check).value(); - return true; - } - - return false; + return frame->attribute_tracking_enabled() && + frame->value_stack().PeekAttribute().Match( + cel::AttributeQualifierView::OfString(field), + frame->value_stack().Peek(), frame->unknown_tree()); } bool SupportsCachedFieldDescriptor( diff --git a/eval/internal/BUILD b/eval/internal/BUILD index c168ce3fa..614ca48c4 100644 --- a/eval/internal/BUILD +++ b/eval/internal/BUILD @@ -84,20 +84,17 @@ cc_library( hdrs = ["adapter_activation_impl.h"], deps = [ ":interop", - "//base:attributes", + "//base:attribute_matcher", "//common:value", "//eval/public:base_activation", "//eval/public:cel_value", "//internal:status_macros", "//runtime:activation_interface", "//runtime:function_overload_reference", - "//runtime/internal:activation_attribute_matcher_access", - "//runtime/internal:attribute_matcher", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/types:optional", - "@com_google_absl//absl/types:span", "@com_google_protobuf//:protobuf", ], ) diff --git a/eval/internal/adapter_activation_impl.cc b/eval/internal/adapter_activation_impl.cc index c88fe8145..a0fcea02d 100644 --- a/eval/internal/adapter_activation_impl.cc +++ b/eval/internal/adapter_activation_impl.cc @@ -25,8 +25,6 @@ #include "eval/public/cel_value.h" #include "internal/status_macros.h" #include "runtime/function_overload_reference.h" -#include "runtime/internal/activation_attribute_matcher_access.h" -#include "runtime/internal/attribute_matcher.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/message.h" @@ -68,20 +66,4 @@ AdapterActivationImpl::FindFunctionOverloads(absl::string_view name) const return result; } -absl::Span AdapterActivationImpl::GetUnknownAttributes() - const { - return legacy_activation_.unknown_attribute_patterns(); -} - -absl::Span AdapterActivationImpl::GetMissingAttributes() - const { - return legacy_activation_.missing_attribute_patterns(); -} - -const runtime_internal::AttributeMatcher* absl_nullable -AdapterActivationImpl::GetAttributeMatcher() const { - return runtime_internal::ActivationAttributeMatcherAccess:: - GetAttributeMatcher(legacy_activation_); -} - } // namespace cel::interop_internal diff --git a/eval/internal/adapter_activation_impl.h b/eval/internal/adapter_activation_impl.h index ebf3156aa..89014cfd3 100644 --- a/eval/internal/adapter_activation_impl.h +++ b/eval/internal/adapter_activation_impl.h @@ -20,13 +20,11 @@ #include "absl/base/nullability.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" -#include "absl/types/span.h" -#include "base/attribute.h" +#include "base/attribute_matcher.h" #include "common/value.h" #include "eval/public/base_activation.h" #include "runtime/activation_interface.h" #include "runtime/function_overload_reference.h" -#include "runtime/internal/attribute_matcher.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/message.h" @@ -52,14 +50,19 @@ class AdapterActivationImpl : public ActivationInterface { std::vector FindFunctionOverloads( absl::string_view name) const override; - absl::Span GetUnknownAttributes() const override; + const AttributeMatcher& GetUnknownAttributeMatcher() const override { + return legacy_activation_.GetUnknownAttributeMatcher(); + } - absl::Span GetMissingAttributes() const override; + const AttributeMatcher& GetKnownAttributeMatcher() const override { + return legacy_activation_.GetKnownAttributeMatcher(); + } - private: - const runtime_internal::AttributeMatcher* absl_nullable GetAttributeMatcher() - const override; + const AttributeMatcher& GetMissingAttributeMatcher() const override { + return legacy_activation_.GetMissingAttributeMatcher(); + } + private: const google::api::expr::runtime::BaseActivation& legacy_activation_; }; diff --git a/eval/public/BUILD b/eval/public/BUILD index 7bb6aac0d..7f25a3b64 100644 --- a/eval/public/BUILD +++ b/eval/public/BUILD @@ -181,13 +181,15 @@ cc_library( ":cel_function", ":cel_value", ":cel_value_producer", - "//runtime/internal:attribute_matcher", + "//base:attribute_matcher", + "//internal:status_macros", "@com_google_absl//absl/base:core_headers", - "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/log:absl_check", "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/types:optional", + "@com_google_absl//absl/types:span", "@com_google_protobuf//:protobuf", ], ) @@ -725,14 +727,14 @@ cc_test( ], deps = [ ":activation", - ":cel_attribute", ":cel_function", - "//eval/eval:attribute_trail", - "//eval/eval:evaluator_core", - "//extensions/protobuf:memory_manager", - "//internal:status_macros", + ":cel_value", + ":cel_value_producer", "//internal:testing", - "//parser", + "@com_google_absl//absl/status", + "@com_google_absl//absl/strings:string_view", + "@com_google_absl//absl/types:span", + "@com_google_protobuf//:protobuf", ], ) @@ -1077,8 +1079,8 @@ cc_library( ":cel_attribute", ":cel_function", ":cel_value", - "//runtime/internal:attribute_matcher", - "@com_google_absl//absl/base:nullability", + "//base:attribute_matcher", + "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/strings", "@com_google_protobuf//:field_mask_cc_proto", ], diff --git a/eval/public/activation.h b/eval/public/activation.h index e9d7740af..183580bf3 100644 --- a/eval/public/activation.h +++ b/eval/public/activation.h @@ -7,17 +7,19 @@ #include #include "absl/base/attributes.h" -#include "absl/base/nullability.h" #include "absl/container/flat_hash_map.h" +#include "absl/log/absl_check.h" #include "absl/status/status.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" +#include "absl/types/span.h" +#include "base/attribute_matcher.h" #include "eval/public/base_activation.h" #include "eval/public/cel_attribute.h" #include "eval/public/cel_function.h" #include "eval/public/cel_value.h" #include "eval/public/cel_value_producer.h" -#include "runtime/internal/attribute_matcher.h" +#include "internal/status_macros.h" #include "google/protobuf/arena.h" namespace cel::runtime_internal { @@ -80,36 +82,65 @@ class Activation : public BaseActivation { // If a field access is found to match any of the provided patterns, the // result is treated as a missing attribute error. absl::Status SetMissingAttributePatterns( - std::vector missing_attribute_patterns) { - missing_attribute_patterns_ = std::move(missing_attribute_patterns); + absl::Span patterns) { + cel::AttributeMatcher matcher; + for (const auto& pattern : patterns) { + // This function used to return void, thus they could not fail. We use + // upsert because it just works most of the time, especially if the caller + // provided overlapped attribute patterns. Ideally we would use insert. + CEL_RETURN_IF_ERROR(matcher.UpsertAttribute(pattern)); + } + missing_attributes_ = std::move(matcher); return absl::OkStatus(); } - const std::vector& missing_attribute_patterns() - const override { - return missing_attribute_patterns_; - } - ABSL_DEPRECATED("Use SetUnknownAttributePatterns") void set_unknown_attribute_patterns( - std::vector unknown_attribute_patterns) { - SetUnknownAttributePatterns(std::move(unknown_attribute_patterns)) - .IgnoreError(); + absl::Span patterns) { + unknown_attributes_.ClearAttributes(); + absl::Status status = SetUnknownAttributePatterns(patterns); + ABSL_DCHECK_OK(status); + status.IgnoreError(); } // Sets the collection of attribute patterns that will be recognized as // "unknown" values during expression evaluation. absl::Status SetUnknownAttributePatterns( - std::vector unknown_attribute_patterns) { - unknown_attribute_patterns_ = std::move(unknown_attribute_patterns); + absl::Span patterns) { + cel::AttributeMatcher matcher; + for (const auto& pattern : patterns) { + // This function used to return void, thus they could not fail. We use + // upsert because it just works most of the time, especially if the caller + // provided overlapped attribute patterns. Ideally we would use insert. + CEL_RETURN_IF_ERROR(matcher.UpsertAttribute(pattern)); + } + unknown_attributes_ = std::move(matcher); + return absl::OkStatus(); + } + + absl::Status SetKnownAttributePatterns( + absl::Span patterns) { + cel::AttributeMatcher matcher; + for (const auto& pattern : patterns) { + // This function used to return void, thus they could not fail. We use + // upsert because it just works most of the time, especially if the caller + // provided overlapped attribute patterns. Ideally we would use insert. + CEL_RETURN_IF_ERROR(matcher.UpsertAttribute(pattern)); + } + known_attributes_ = std::move(matcher); return absl::OkStatus(); } - // Return the collection of attribute patterns that determine "unknown" - // values. - const std::vector& unknown_attribute_patterns() - const override { - return unknown_attribute_patterns_; + const cel::AttributeMatcher& GetUnknownAttributeMatcher() const final { + return unknown_attributes_; + } + + const cel::AttributeMatcher& GetKnownAttributeMatcher() const final { + return known_attributes_; + } + + const cel::AttributeMatcher& GetMissingAttributeMatcher() const final { + return missing_attributes_; } private: @@ -148,32 +179,13 @@ class Activation : public BaseActivation { friend class cel::runtime_internal::ActivationAttributeMatcherAccess; - void SetAttributeMatcher( - const cel::runtime_internal::AttributeMatcher* matcher) { - attribute_matcher_ = matcher; - } - - void SetAttributeMatcher( - std::unique_ptr matcher) { - owned_attribute_matcher_ = std::move(matcher); - attribute_matcher_ = owned_attribute_matcher_.get(); - } - - const cel::runtime_internal::AttributeMatcher* absl_nullable - GetAttributeMatcher() const override { - return attribute_matcher_; - } - absl::flat_hash_map value_map_; absl::flat_hash_map>> function_map_; - std::vector missing_attribute_patterns_; - std::vector unknown_attribute_patterns_; - - const cel::runtime_internal::AttributeMatcher* attribute_matcher_ = nullptr; - std::unique_ptr - owned_attribute_matcher_; + cel::AttributeMatcher unknown_attributes_; + cel::AttributeMatcher known_attributes_; + cel::AttributeMatcher missing_attributes_; }; } // namespace google::api::expr::runtime diff --git a/eval/public/activation_test.cc b/eval/public/activation_test.cc index 33a0dfafe..d628d5fa8 100644 --- a/eval/public/activation_test.cc +++ b/eval/public/activation_test.cc @@ -4,14 +4,14 @@ #include #include -#include "eval/eval/attribute_trail.h" -#include "eval/eval/ident_step.h" -#include "eval/public/cel_attribute.h" +#include "absl/status/status.h" +#include "absl/strings/string_view.h" +#include "absl/types/span.h" #include "eval/public/cel_function.h" -#include "extensions/protobuf/memory_manager.h" -#include "internal/status_macros.h" +#include "eval/public/cel_value.h" +#include "eval/public/cel_value_producer.h" #include "internal/testing.h" -#include "parser/parser.h" +#include "google/protobuf/arena.h" namespace google { namespace api { @@ -20,10 +20,7 @@ namespace runtime { namespace { -using ::absl_testing::IsOk; using ::absl_testing::StatusIs; -using ::cel::extensions::ProtoMemoryManager; -using ::cel::expr::Expr; using ::google::protobuf::Arena; using ::testing::ElementsAre; using ::testing::Eq; @@ -205,35 +202,6 @@ TEST(ActivationTest, CheckValueProducerClear) { EXPECT_EQ(1, activation.ClearCachedValues()); } -TEST(ActivationTest, ErrorPathTest) { - Activation activation; - - Expr expr; - auto* select_expr = expr.mutable_select_expr(); - select_expr->set_field("ip"); - - Expr* ident_expr = select_expr->mutable_operand(); - ident_expr->mutable_ident_expr()->set_name("destination"); - - const CelAttributePattern destination_ip_pattern( - "destination", - {CreateCelAttributeQualifierPattern(CelValue::CreateStringView("ip"))}); - - AttributeTrail trail("destination"); - trail = - trail.Step(CreateCelAttributeQualifier(CelValue::CreateStringView("ip"))); - - ASSERT_EQ(destination_ip_pattern.IsMatch(trail.attribute()), - CelAttributePattern::MatchType::FULL); - EXPECT_TRUE(activation.missing_attribute_patterns().empty()); - - ASSERT_THAT(activation.SetMissingAttributePatterns({destination_ip_pattern}), - IsOk()); - EXPECT_EQ( - activation.missing_attribute_patterns()[0].IsMatch(trail.attribute()), - CelAttributePattern::MatchType::FULL); -} - } // namespace } // namespace runtime diff --git a/eval/public/base_activation.h b/eval/public/base_activation.h index 7d9e0a51c..7a0c6b208 100644 --- a/eval/public/base_activation.h +++ b/eval/public/base_activation.h @@ -4,12 +4,12 @@ #include #include "google/protobuf/field_mask.pb.h" -#include "absl/base/nullability.h" +#include "absl/base/macros.h" #include "absl/strings/string_view.h" +#include "base/attribute_matcher.h" #include "eval/public/cel_attribute.h" #include "eval/public/cel_function.h" #include "eval/public/cel_value.h" -#include "runtime/internal/attribute_matcher.h" namespace cel::runtime_internal { class ActivationAttributeMatcherAccess; @@ -42,32 +42,31 @@ class BaseActivation { // Return the collection of attribute patterns that determine missing // attributes. - virtual const std::vector& missing_attribute_patterns() - const { - static const std::vector* empty = - new std::vector({}); - return *empty; + ABSL_DEPRECATE_AND_INLINE() + std::vector missing_attribute_patterns() const { + return GetMissingAttributeMatcher().GetAttributes(); } // Return the collection of attribute patterns that determine "unknown" // values. - virtual const std::vector& unknown_attribute_patterns() - const { - static const std::vector* empty = - new std::vector({}); - return *empty; + ABSL_DEPRECATE_AND_INLINE() + std::vector unknown_attribute_patterns() const { + return GetUnknownAttributeMatcher().GetAttributes(); } - virtual ~BaseActivation() = default; + virtual const cel::AttributeMatcher& GetUnknownAttributeMatcher() const { + return cel::EmptyAttributeMatcher(); + } - private: - friend class cel::runtime_internal::ActivationAttributeMatcherAccess; + virtual const cel::AttributeMatcher& GetKnownAttributeMatcher() const { + return cel::EmptyAttributeMatcher(); + } - // Internal getter for overriding the attribute matching behavior. - virtual const cel::runtime_internal::AttributeMatcher* absl_nullable - GetAttributeMatcher() const { - return nullptr; + virtual const cel::AttributeMatcher& GetMissingAttributeMatcher() const { + return cel::EmptyAttributeMatcher(); } + + virtual ~BaseActivation() = default; }; } // namespace google::api::expr::runtime diff --git a/eval/tests/BUILD b/eval/tests/BUILD index a9f081db0..46735296e 100644 --- a/eval/tests/BUILD +++ b/eval/tests/BUILD @@ -117,6 +117,8 @@ cc_test( ], deps = [ "//base:attributes", + "//common:function_descriptor", + "//common:kind", "//common:value", "//extensions:bindings_ext", "//extensions:comprehensions_v2_macros", @@ -129,6 +131,7 @@ cc_test( "//runtime", "//runtime:activation", "//runtime:constant_folding", + "//runtime:function", "//runtime:optional_types", "//runtime:reference_resolver", "//runtime:runtime_options", @@ -138,6 +141,7 @@ cc_test( "@com_google_absl//absl/status:status_matchers", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings:string_view", + "@com_google_absl//absl/types:span", "@com_google_protobuf//:protobuf", "@com_google_protobuf//:struct_cc_proto", ], @@ -273,8 +277,6 @@ cc_test( "//internal:status_macros", "//internal:testing", "//parser", - "//runtime/internal:activation_attribute_matcher_access", - "//runtime/internal:attribute_matcher", "@com_google_absl//absl/base:no_destructor", "@com_google_absl//absl/status", "@com_google_absl//absl/status:status_matchers", diff --git a/eval/tests/modern_unknown_test.cc b/eval/tests/modern_unknown_test.cc index 6575e45f3..f5596bcba 100644 --- a/eval/tests/modern_unknown_test.cc +++ b/eval/tests/modern_unknown_test.cc @@ -21,7 +21,10 @@ #include "absl/status/status_matchers.h" #include "absl/status/statusor.h" #include "absl/strings/string_view.h" +#include "absl/types/span.h" #include "base/attribute.h" +#include "common/function_descriptor.h" +#include "common/kind.h" #include "common/value.h" #include "extensions/bindings_ext.h" #include "extensions/comprehensions_v2_macros.h" @@ -33,6 +36,7 @@ #include "parser/parser_interface.h" #include "runtime/activation.h" #include "runtime/constant_folding.h" +#include "runtime/function.h" #include "runtime/optional_types.h" #include "runtime/reference_resolver.h" #include "runtime/runtime.h" @@ -47,6 +51,16 @@ namespace { using absl_testing::IsOk; using ::testing::UnorderedElementsAre; +// Function which just echos its input, this is to force the evaluator to handle +// partials. +class FullFunction final : public cel::Function { + public: + absl::StatusOr Invoke(absl::Span args, + const InvokeContext&) const override { + return args[0]; + } +}; + class ModernUnknownTest : public ::testing::Test { public: void SetUp() override { @@ -76,6 +90,9 @@ class ModernUnknownTest : public ::testing::Test { ABSL_CHECK_OK(cel::extensions::EnableConstantFolding( runtime_builder, cel::internal::GetTestingMessageFactory())); ABSL_CHECK_OK(cel::extensions::EnableOptionalTypes(runtime_builder)); + ABSL_CHECK_OK(runtime_builder.function_registry().Register( + cel::FunctionDescriptor("full", false, {cel::Kind::kAny}), + std::make_unique())); auto status_or_runtime = std::move(runtime_builder).Build(); ABSL_CHECK_OK(status_or_runtime); // Crash OK runtime_ = *std::move(status_or_runtime); @@ -500,4 +517,108 @@ TEST_F(ModernUnknownTest, ListVariableIndexWildcardQualifier) { cel::Attribute("var1", {cel::AttributeQualifier::OfInt(0)}))); } +TEST_F(ModernUnknownTest, FullKnownUnknown) { + auto program = CreateProgram(R"cel(full(a.b.c))cel"); + ASSERT_THAT(activation_.SetUnknownPatterns({cel::AttributePattern("a")}), + IsOk()); + ASSERT_THAT(activation_.SetKnownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::OfString("b")})}), + IsOk()); + BindJsonToActivation("a", R"json({"b": {"c": true}})json"); + ASSERT_OK_AND_ASSIGN(auto result, Evaluate(*program)); + ASSERT_TRUE(result.IsTrue()); +} + +TEST_F(ModernUnknownTest, FullKnownUnknownWildcard) { + auto program = CreateProgram(R"cel(full(a.b.c))cel"); + ASSERT_THAT(activation_.SetUnknownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::Wildcard()})}), + IsOk()); + ASSERT_THAT(activation_.SetKnownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::OfString("b")})}), + IsOk()); + BindJsonToActivation("a", R"json({"b": {"c": true}})json"); + ASSERT_OK_AND_ASSIGN(auto result, Evaluate(*program)); + ASSERT_TRUE(result.IsTrue()); +} + +TEST_F(ModernUnknownTest, FullKnownWildcardUnknown) { + auto program = CreateProgram(R"cel(full(a.b.c))cel"); + ASSERT_THAT(activation_.SetUnknownPatterns({cel::AttributePattern("a")}), + IsOk()); + ASSERT_THAT(activation_.SetKnownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::OfString("b"), + cel::AttributeQualifierPattern::Wildcard()})}), + IsOk()); + BindJsonToActivation("a", R"json({"b": {"c": true}})json"); + ASSERT_OK_AND_ASSIGN(auto result, Evaluate(*program)); + ASSERT_TRUE(result.IsTrue()); +} + +TEST_F(ModernUnknownTest, FullKnownWildcardUnknownWildcard) { + auto program = CreateProgram(R"cel(full(a.b.c))cel"); + ASSERT_THAT(activation_.SetUnknownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::Wildcard()})}), + IsOk()); + ASSERT_THAT(activation_.SetKnownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::OfString("b"), + cel::AttributeQualifierPattern::Wildcard()})}), + IsOk()); + BindJsonToActivation("a", R"json({"b": {"c": true}})json"); + ASSERT_OK_AND_ASSIGN(auto result, Evaluate(*program)); + ASSERT_TRUE(result.IsTrue()); +} + +TEST_F(ModernUnknownTest, PartialKnownUnknown) { + auto program = CreateProgram(R"cel(full(a.b))cel"); + ASSERT_THAT(activation_.SetUnknownPatterns({cel::AttributePattern("a")}), + IsOk()); + ASSERT_THAT(activation_.SetKnownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::OfString("b")})}), + IsOk()); + BindJsonToActivation("a", R"json({"b": true})json"); + ASSERT_OK_AND_ASSIGN(auto result, Evaluate(*program)); + ASSERT_TRUE(result.IsTrue()); +} + +TEST_F(ModernUnknownTest, PartialKnownUnknownWildcard) { + auto program = CreateProgram(R"cel(full(a.b))cel"); + ASSERT_THAT(activation_.SetUnknownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::Wildcard()})}), + IsOk()); + ASSERT_THAT(activation_.SetKnownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::OfString("b")})}), + IsOk()); + BindJsonToActivation("a", R"json({"b": true})json"); + ASSERT_OK_AND_ASSIGN(auto result, Evaluate(*program)); + ASSERT_TRUE(result.IsTrue()); +} + +TEST_F(ModernUnknownTest, PartialKnownWildcardUnknown) { + auto program = CreateProgram(R"cel(full(a.b))cel"); + ASSERT_THAT(activation_.SetUnknownPatterns({cel::AttributePattern("a")}), + IsOk()); + ASSERT_THAT(activation_.SetKnownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::OfString("b"), + cel::AttributeQualifierPattern::Wildcard()})}), + IsOk()); + BindJsonToActivation("a", R"json({"b": true})json"); + ASSERT_OK_AND_ASSIGN(auto result, Evaluate(*program)); + ASSERT_TRUE(result.IsTrue()); +} + +TEST_F(ModernUnknownTest, PartialKnownWildcardUnknownWildcard) { + auto program = CreateProgram(R"cel(full(a.b))cel"); + ASSERT_THAT(activation_.SetUnknownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::Wildcard()})}), + IsOk()); + ASSERT_THAT(activation_.SetKnownPatterns({cel::AttributePattern( + "a", {cel::AttributeQualifierPattern::OfString("b"), + cel::AttributeQualifierPattern::Wildcard()})}), + IsOk()); + BindJsonToActivation("a", R"json({"b": true})json"); + ASSERT_OK_AND_ASSIGN(auto result, Evaluate(*program)); + ASSERT_TRUE(result.IsTrue()); +} + } // namespace diff --git a/eval/tests/unknowns_end_to_end_test.cc b/eval/tests/unknowns_end_to_end_test.cc index eaa9c163a..cffeae5c6 100644 --- a/eval/tests/unknowns_end_to_end_test.cc +++ b/eval/tests/unknowns_end_to_end_test.cc @@ -18,6 +18,7 @@ #include "absl/strings/string_view.h" #include "absl/types/span.h" #include "base/attribute.h" +#include "base/attribute_set.h" #include "base/function_result.h" #include "eval/public/activation.h" #include "eval/public/builtin_func_registrar.h" @@ -33,8 +34,6 @@ #include "internal/status_macros.h" #include "internal/testing.h" #include "parser/parser.h" -#include "runtime/internal/activation_attribute_matcher_access.h" -#include "runtime/internal/attribute_matcher.h" #include "google/protobuf/arena.h" #include "google/protobuf/text_format.h" @@ -45,35 +44,11 @@ namespace runtime { namespace { using ::absl_testing::IsOk; -using ::cel::runtime_internal::ActivationAttributeMatcherAccess; using ::cel::expr::Expr; using ::cel::expr::ParsedExpr; using ::google::api::expr::parser::Parse; using ::google::protobuf::Arena; using ::testing::ElementsAre; -using ::testing::UnorderedElementsAre; - -absl::StatusOr MakeCelMap(absl::string_view expr, - google::protobuf::Arena* arena) { - static CelExpressionBuilder* builder = []() { - return CreateCelExpressionBuilder(InterpreterOptions()).release(); - }(); - static absl::NoDestructor activation; - - CEL_ASSIGN_OR_RETURN(ParsedExpr parsed_expr, Parse(expr)); - - CEL_ASSIGN_OR_RETURN(auto plan, - builder->CreateExpression(&parsed_expr.expr(), nullptr)); - absl::StatusOr result = plan->Evaluate(*activation, arena); - if (!result.ok()) { - return result.status(); - } - if (!result->IsMap()) { - return absl::FailedPreconditionError( - absl::StrCat("expression did not evaluate to a map: ", expr)); - } - return result; -} enum class FunctionResponse { kUnknown, kTrue, kFalse }; @@ -219,49 +194,6 @@ TEST_F(UnknownsTest, UnknownAttributesPruning) { EXPECT_TRUE(response.BoolOrDie()); } -class CustomMatcher : public cel::runtime_internal::AttributeMatcher { - public: - MatchResult CheckForUnknown(const cel::Attribute& attr) const override { - // Rendering to a string just for ease of testing. - std::string name = attr.AsString().value_or(""); - if (name == "var1") { - return MatchResult::PARTIAL; - } else if (name == "var1.foo") { - return MatchResult::FULL; - } - return MatchResult::NONE; - } -}; - -TEST_F(UnknownsTest, UnknownAttributesCustomMatcher) { - PrepareBuilder(UnknownProcessingOptions::kAttributeOnly); - - ASSERT_OK_AND_ASSIGN(auto var1, MakeCelMap("{'bar': 1}", &arena_)); - activation_.InsertValue("var1", var1); - CustomMatcher matcher; - ActivationAttributeMatcherAccess::SetAttributeMatcher(activation_, &matcher); - - ASSERT_THAT(activation_.InsertFunction(std::make_unique( - "F1", FunctionResponse::kTrue, CelValue::Type::kMap)), - IsOk()); - ASSERT_THAT(activation_.InsertFunction(std::make_unique( - "F2", FunctionResponse::kTrue)), - IsOk()); - - ASSERT_OK_AND_ASSIGN(ParsedExpr expr, - Parse("F1(var1) || var1.foo || var1.bar")); - auto plan = builder_->CreateExpression(&expr.expr(), nullptr); - ASSERT_THAT(plan, IsOk()); - - ASSERT_OK_AND_ASSIGN(CelValue response, - plan.value()->Evaluate(activation_, &arena_)); - - ASSERT_TRUE(response.IsUnknownSet()) << response.DebugString(); - EXPECT_THAT( - response.UnknownSetOrDie()->unknown_attributes(), - UnorderedElementsAre(AttributeIs("var1"), AttributeIs("var1.foo"))); -} - TEST_F(UnknownsTest, UnknownFunctionsWithoutOptionError) { PrepareBuilder(UnknownProcessingOptions::kAttributeOnly); activation_.InsertValue("var1", CelValue::CreateInt64(5)); @@ -765,7 +697,7 @@ TEST(UnknownsIterAttrTest, IterAttributeTrailMapKeyTypes) { Activation activation; Arena arena; - UnknownSet unknown_set; + UnknownSet unknown_set(cel::AttributeSet({cel::Attribute("foo")})); CelError error = absl::CancelledError(); std::vector> backing; @@ -809,7 +741,7 @@ TEST(UnknownsIterAttrTest, IterAttributeTrailMapKeyTypesShortcutted) { Activation activation; Arena arena; - UnknownSet unknown_set; + UnknownSet unknown_set(cel::AttributeSet({cel::Attribute("foo")})); CelError error = absl::CancelledError(); std::vector> backing; diff --git a/extensions/BUILD b/extensions/BUILD index 93b08e6da..8ed7fa4ba 100644 --- a/extensions/BUILD +++ b/extensions/BUILD @@ -332,9 +332,9 @@ cc_library( "//common:native_type", "//common:type", "//common:value", + "//common/internal:attribute_trail", "//eval/compiler:flat_expr_builder", "//eval/compiler:flat_expr_builder_extensions", - "//eval/eval:attribute_trail", "//eval/eval:evaluator_core", "//eval/eval:expression_step_base", "//internal:casts", diff --git a/extensions/select_optimization.cc b/extensions/select_optimization.cc index 02855dec1..c86e6ae52 100644 --- a/extensions/select_optimization.cc +++ b/extensions/select_optimization.cc @@ -42,13 +42,13 @@ #include "common/constant.h" #include "common/expr.h" #include "common/function_descriptor.h" +#include "common/internal/attribute_trail.h" #include "common/kind.h" #include "common/native_type.h" #include "common/type.h" #include "common/value.h" #include "eval/compiler/flat_expr_builder.h" #include "eval/compiler/flat_expr_builder_extensions.h" -#include "eval/eval/attribute_trail.h" #include "eval/eval/evaluator_core.h" #include "eval/eval/expression_step_base.h" #include "internal/casts.h" @@ -73,7 +73,7 @@ using ::cel::ConstantKind; using ::cel::Expr; using ::cel::ExprKind; using ::cel::SelectExpr; -using ::google::api::expr::runtime::AttributeTrail; +using ::cel::common_internal::AttributeTrail; using ::google::api::expr::runtime::ExecutionFrame; using ::google::api::expr::runtime::ExecutionFrameBase; using ::google::api::expr::runtime::ExpressionStep; @@ -584,8 +584,6 @@ class OptimizedSelectImpl { absl::StatusOr ApplySelect(ExecutionFrameBase& frame, const StructValue& struct_value) const; - AttributeTrail GetAttributeTrail(const AttributeTrail& operand_trail) const; - std::optional attribute() const { return attribute_; } const std::vector& qualifiers() const { @@ -600,37 +598,6 @@ class OptimizedSelectImpl { SelectOptimizationOptions options_; }; -// Check for unknowns or missing attributes. -absl::StatusOr> CheckForMarkedAttributes( - ExecutionFrameBase& frame, const AttributeTrail& attribute_trail) { - if (attribute_trail.empty()) { - return std::nullopt; - } - - if (frame.unknown_processing_enabled() && - frame.attribute_utility().CheckForUnknownExact(attribute_trail)) { - // Check if the inferred attribute is marked. Only matches if this attribute - // or a parent is marked unknown (use_partial = false). - // Partial matches (i.e. descendant of this attribute is marked) aren't - // considered yet in case another operation would select an unmarked - // descended attribute. - // - // TODO(uncreated-issue/51): this may return a more specific attribute than the - // declared pattern. Follow up will truncate the returned attribute to match - // the pattern. - return frame.attribute_utility().CreateUnknownSet( - attribute_trail.attribute(), frame.arena()); - } - - if (frame.missing_attribute_errors_enabled() && - frame.attribute_utility().CheckForMissingAttribute(attribute_trail)) { - return frame.attribute_utility().CreateMissingAttributeError( - attribute_trail.attribute(), frame.arena()); - } - - return std::nullopt; -} - absl::StatusOr OptimizedSelectImpl::ApplySelect( ExecutionFrameBase& frame, const StructValue& struct_value) const { auto value_or = @@ -661,21 +628,6 @@ absl::StatusOr OptimizedSelectImpl::ApplySelect( frame.arena()); } -AttributeTrail OptimizedSelectImpl::GetAttributeTrail( - const AttributeTrail& operand_trail) const { - if (operand_trail.empty()) { - return AttributeTrail(); - } - std::vector qualifiers = std::vector( - operand_trail.attribute().qualifier_path().begin(), - operand_trail.attribute().qualifier_path().end()); - qualifiers.reserve(qualifiers_.size() + qualifiers.size()); - absl::c_copy(qualifiers_, std::back_inserter(qualifiers)); - return AttributeTrail( - Attribute(std::string(operand_trail.attribute().variable_name()), - std::move(qualifiers))); -} - class StackMachineImpl : public ExpressionStepBase { public: StackMachineImpl(int expr_id, OptimizedSelectImpl impl) @@ -684,31 +636,18 @@ class StackMachineImpl : public ExpressionStepBase { void Evaluate(ExecutionFrame* frame) const override; private: - // Get the effective attribute for the optimized select expression. - // Assumes the operand is the top of stack if the attribute wasn't known at - // plan time. - AttributeTrail GetAttributeTrail(ExecutionFrame* frame) const; - OptimizedSelectImpl impl_; }; -AttributeTrail StackMachineImpl::GetAttributeTrail( - ExecutionFrame* frame) const { - const auto& attr = frame->value_stack().PeekAttribute(); - return impl_.GetAttributeTrail(attr); -} - void StackMachineImpl::Evaluate(ExecutionFrame* frame) const { - // Default empty. - AttributeTrail attribute_trail; // TODO(uncreated-issue/51): add support for variable qualifiers and string literal // variable names. - constexpr size_t kStackInputs = 1; // For now, we expect the operand to be top of stack. - const Value& operand = frame->value_stack().Peek(); + Value& operand = frame->value_stack().Peek(); + AttributeTrail& trail = frame->value_stack().PeekAttribute(); - if (operand->Is() || operand->Is()) { + if (operand.Is() || operand.Is()) { // Just forward the error which is already top of stack. return; } @@ -718,18 +657,11 @@ void StackMachineImpl::Evaluate(ExecutionFrame* frame) const { // When possible, this is computed at plan time based on the optimized // select arguments. // TODO(uncreated-issue/51): add support variable qualifiers - attribute_trail = GetAttributeTrail(frame); - absl::StatusOr> value = - CheckForMarkedAttributes(*frame, attribute_trail); - if (!value.ok()) { - frame->Abort(std::move(value).status()); - return; - } - if (value->has_value()) { - frame->value_stack().Pop(kStackInputs); - frame->value_stack().Push(std::move(*value).value(), - std::move(attribute_trail)); - return; + for (const auto& qualifier : impl_.qualifiers()) { + if (trail.Match(qualifier, operand, + frame->unknown_tree())) { + return; + } } } @@ -745,8 +677,7 @@ void StackMachineImpl::Evaluate(ExecutionFrame* frame) const { return; } - frame->value_stack().Pop(kStackInputs); - frame->value_stack().Push(*std::move(result), std::move(attribute_trail)); + operand = *result; } class SelectOptimizer : public ProgramOptimizer { diff --git a/extensions/select_optimization_test.cc b/extensions/select_optimization_test.cc index 619db4a07..3cbfffa07 100644 --- a/extensions/select_optimization_test.cc +++ b/extensions/select_optimization_test.cc @@ -1283,8 +1283,6 @@ INSTANTIATE_TEST_SUITE_P( "b", { AttributeQualifier::OfString("child"), AttributeQualifier::OfString("child"), - AttributeQualifier::OfString("payload"), - AttributeQualifier::OfString("single_int64"), })))); }, }, @@ -1371,8 +1369,7 @@ INSTANTIATE_TEST_SUITE_P( ASSERT_OK_AND_ASSIGN(Value result, got); ASSERT_TRUE(result->Is()) << result->DebugString(); EXPECT_THAT(result.GetError().NativeValue().message(), - HasSubstr("nested_test_all_types.child.payload." - "standalone_message")); + HasSubstr("nested_test_all_types.child")); }, }, { diff --git a/runtime/BUILD b/runtime/BUILD index d8ba3eb7d..c37f281ad 100644 --- a/runtime/BUILD +++ b/runtime/BUILD @@ -27,15 +27,14 @@ cc_library( hdrs = ["activation_interface.h"], deps = [ ":function_overload_reference", + "//base:attribute_matcher", "//base:attributes", "//common:value", "//internal:status_macros", - "//runtime/internal:attribute_matcher", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/status:statusor", "@com_google_absl//absl/strings", "@com_google_absl//absl/types:optional", - "@com_google_absl//absl/types:span", "@com_google_protobuf//:protobuf", ], ) @@ -68,11 +67,11 @@ cc_library( ":activation_interface", ":function", ":function_overload_reference", + "//base:attribute_matcher", "//base:attributes", "//common:function_descriptor", "//common:value", "//internal:status_macros", - "//runtime/internal:attribute_matcher", "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/base:nullability", "@com_google_absl//absl/container:flat_hash_map", diff --git a/runtime/activation.h b/runtime/activation.h index 0d647c32e..e05a0c295 100644 --- a/runtime/activation.h +++ b/runtime/activation.h @@ -30,12 +30,13 @@ #include "absl/types/optional.h" #include "absl/types/span.h" #include "base/attribute.h" +#include "base/attribute_matcher.h" #include "common/function_descriptor.h" #include "common/value.h" +#include "internal/status_macros.h" #include "runtime/activation_interface.h" #include "runtime/function.h" #include "runtime/function_overload_reference.h" -#include "runtime/internal/attribute_matcher.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/message.h" @@ -76,16 +77,6 @@ class Activation final : public ActivationInterface { std::vector FindFunctionOverloads( absl::string_view name) const override; - absl::Span GetUnknownAttributes() - const override { - return unknown_patterns_; - } - - absl::Span GetMissingAttributes() - const override { - return missing_patterns_; - } - // Bind a value to a named variable. // // Returns false if the entry for name was overwritten. @@ -98,23 +89,63 @@ class Activation final : public ActivationInterface { bool InsertOrAssignValueProvider(absl::string_view name, ValueProvider provider); - absl::Status AddUnknownPattern(cel::AttributePattern pattern) { - unknown_patterns_.push_back(std::move(pattern)); - return absl::OkStatus(); + absl::Status AddUnknownPattern(const cel::AttributePattern& pattern) { + // This function used to return void, thus they could not fail. We use + // upsert because it just works most of the time, especially if the caller + // provided overlapped attribute patterns. Ideally we would use insert. + return unknown_attributes_.UpsertAttribute(pattern); } - absl::Status SetUnknownPatterns(std::vector patterns) { - unknown_patterns_ = std::move(patterns); + absl::Status SetUnknownPatterns( + absl::Span patterns) { + AttributeMatcher matcher; + for (const auto& pattern : patterns) { + // This function used to return void, thus they could not fail. We use + // upsert because it just works most of the time, especially if the caller + // provided overlapped attribute patterns. Ideally we would use insert. + CEL_RETURN_IF_ERROR(matcher.UpsertAttribute(pattern)); + } + unknown_attributes_ = std::move(matcher); return absl::OkStatus(); } - absl::Status AddMissingPattern(cel::AttributePattern pattern) { - missing_patterns_.push_back(std::move(pattern)); + absl::Status AddMissingPattern(const cel::AttributePattern& pattern) { + // This function used to return void, thus they could not fail. We use + // upsert because it just works most of the time, especially if the caller + // provided overlapped attribute patterns. Ideally we would use insert. + return missing_attributes_.UpsertAttribute(pattern); + } + + absl::Status SetMissingPatterns( + absl::Span patterns) { + AttributeMatcher matcher; + for (const auto& pattern : patterns) { + // This function used to return void, thus they could not fail. We use + // upsert because it just works most of the time, especially if the caller + // provided overlapped attribute patterns. Ideally we would use insert. + CEL_RETURN_IF_ERROR(matcher.UpsertAttribute(pattern)); + } + missing_attributes_ = std::move(matcher); return absl::OkStatus(); } - absl::Status SetMissingPatterns(std::vector patterns) { - missing_patterns_ = std::move(patterns); + absl::Status AddKnownPattern(const cel::AttributePattern& pattern) { + // This function used to return void, thus they could not fail. We use + // upsert because it just works most of the time, especially if the caller + // provided overlapped attribute patterns. Ideally we would use insert. + return known_attributes_.UpsertAttribute(pattern); + } + + absl::Status SetKnownPatterns( + absl::Span patterns) { + AttributeMatcher matcher; + for (const auto& pattern : patterns) { + // This function used to return void, thus they could not fail. We use + // upsert because it just works most of the time, especially if the caller + // provided overlapped attribute patterns. Ideally we would use insert. + CEL_RETURN_IF_ERROR(matcher.UpsertAttribute(pattern)); + } + known_attributes_ = std::move(matcher); return absl::OkStatus(); } @@ -123,6 +154,18 @@ class Activation final : public ActivationInterface { bool InsertFunction(const cel::FunctionDescriptor& descriptor, std::unique_ptr impl); + const AttributeMatcher& GetUnknownAttributeMatcher() const final { + return unknown_attributes_; + } + + const AttributeMatcher& GetKnownAttributeMatcher() const final { + return known_attributes_; + } + + const AttributeMatcher& GetMissingAttributeMatcher() const final { + return missing_attributes_; + } + private: struct ValueEntry { // If provider is present, then access must be synchronized to maintain @@ -136,29 +179,13 @@ class Activation final : public ActivationInterface { std::unique_ptr implementation; }; - friend class runtime_internal::ActivationAttributeMatcherAccess; - - void SetAttributeMatcher(const runtime_internal::AttributeMatcher* matcher) { - attribute_matcher_ = matcher; - } - - void SetAttributeMatcher( - std::unique_ptr matcher) { - owned_attribute_matcher_ = std::move(matcher); - attribute_matcher_ = owned_attribute_matcher_.get(); - } - - const runtime_internal::AttributeMatcher* absl_nullable GetAttributeMatcher() - const override { - return attribute_matcher_; - } - friend void swap(Activation& a, Activation& b) { using std::swap; swap(a.values_, b.values_); swap(a.functions_, b.functions_); - swap(a.unknown_patterns_, b.unknown_patterns_); - swap(a.missing_patterns_, b.missing_patterns_); + swap(a.unknown_attributes_, b.unknown_attributes_); + swap(a.known_attributes_, b.known_attributes_); + swap(a.missing_attributes_, b.missing_attributes_); } // Internal getter for provided values. @@ -174,12 +201,9 @@ class Activation final : public ActivationInterface { mutable absl::Mutex mutex_; mutable absl::flat_hash_map values_; - std::vector unknown_patterns_; - std::vector missing_patterns_; - - const runtime_internal::AttributeMatcher* attribute_matcher_ = nullptr; - std::unique_ptr - owned_attribute_matcher_; + AttributeMatcher unknown_attributes_; + AttributeMatcher known_attributes_; + AttributeMatcher missing_attributes_; absl::flat_hash_map> functions_; }; diff --git a/runtime/activation_interface.h b/runtime/activation_interface.h index c589468de..890ea08f2 100644 --- a/runtime/activation_interface.h +++ b/runtime/activation_interface.h @@ -21,12 +21,11 @@ #include "absl/status/statusor.h" #include "absl/strings/string_view.h" #include "absl/types/optional.h" -#include "absl/types/span.h" #include "base/attribute.h" +#include "base/attribute_matcher.h" #include "common/value.h" #include "internal/status_macros.h" #include "runtime/function_overload_reference.h" -#include "runtime/internal/attribute_matcher.h" #include "google/protobuf/arena.h" #include "google/protobuf/descriptor.h" #include "google/protobuf/message.h" @@ -80,8 +79,9 @@ class ActivationInterface { // // The returned span must remain valid for the duration of any evaluation // using this this activation. - virtual absl::Span GetUnknownAttributes() - const = 0; + std::vector GetUnknownAttributes() const { + return GetUnknownAttributeMatcher().GetAttributes(); + } // Return a list of missing attribute patterns. // @@ -91,16 +91,20 @@ class ActivationInterface { // // The returned span must remain valid for the duration of any evaluation // using this activation. - virtual absl::Span GetMissingAttributes() - const = 0; + std::vector GetMissingAttributes() const { + return GetMissingAttributeMatcher().GetAttributes(); + } - private: - friend class runtime_internal::ActivationAttributeMatcherAccess; + virtual const AttributeMatcher& GetUnknownAttributeMatcher() const { + return EmptyAttributeMatcher(); + } + + virtual const cel::AttributeMatcher& GetKnownAttributeMatcher() const { + return EmptyAttributeMatcher(); + } - // Returns the attribute matcher for this activation. - virtual const runtime_internal::AttributeMatcher* absl_nullable - GetAttributeMatcher() const { - return nullptr; + virtual const AttributeMatcher& GetMissingAttributeMatcher() const { + return EmptyAttributeMatcher(); } }; diff --git a/runtime/internal/BUILD b/runtime/internal/BUILD index cbe1fd2fe..42a885958 100644 --- a/runtime/internal/BUILD +++ b/runtime/internal/BUILD @@ -226,21 +226,3 @@ cc_library( "@com_google_protobuf//:protobuf", ], ) - -cc_library( - name = "attribute_matcher", - hdrs = ["attribute_matcher.h"], - deps = ["//base:attributes"], -) - -cc_library( - name = "activation_attribute_matcher_access", - srcs = ["activation_attribute_matcher_access.cc"], - hdrs = ["activation_attribute_matcher_access.h"], - deps = [ - ":attribute_matcher", - "//eval/public:activation", - "//runtime:activation", - "@com_google_absl//absl/base:nullability", - ], -) diff --git a/runtime/internal/activation_attribute_matcher_access.cc b/runtime/internal/activation_attribute_matcher_access.cc deleted file mode 100644 index 7d358ba23..000000000 --- a/runtime/internal/activation_attribute_matcher_access.cc +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright 2025 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#include "runtime/internal/activation_attribute_matcher_access.h" - -#include -#include - -#include "absl/base/nullability.h" -#include "eval/public/activation.h" -#include "runtime/activation.h" -#include "runtime/internal/attribute_matcher.h" - -namespace cel::runtime_internal { - -void ActivationAttributeMatcherAccess::SetAttributeMatcher( - google::api::expr::runtime::Activation& activation, - const AttributeMatcher* matcher) { - activation.SetAttributeMatcher(matcher); -} - -void ActivationAttributeMatcherAccess::SetAttributeMatcher( - google::api::expr::runtime::Activation& activation, - std::unique_ptr matcher) { - activation.SetAttributeMatcher(std::move(matcher)); -} - -const AttributeMatcher* absl_nullable -ActivationAttributeMatcherAccess::GetAttributeMatcher( - const google::api::expr::runtime::BaseActivation& activation) { - return activation.GetAttributeMatcher(); -} - -void ActivationAttributeMatcherAccess::SetAttributeMatcher( - Activation& activation, const AttributeMatcher* matcher) { - activation.SetAttributeMatcher(matcher); -} - -void ActivationAttributeMatcherAccess::SetAttributeMatcher( - Activation& activation, std::unique_ptr matcher) { - activation.SetAttributeMatcher(std::move(matcher)); -} - -const AttributeMatcher* absl_nullable -ActivationAttributeMatcherAccess::GetAttributeMatcher( - const ActivationInterface& activation) { - return activation.GetAttributeMatcher(); -} - -} // namespace cel::runtime_internal diff --git a/runtime/internal/activation_attribute_matcher_access.h b/runtime/internal/activation_attribute_matcher_access.h deleted file mode 100644 index 2741be692..000000000 --- a/runtime/internal/activation_attribute_matcher_access.h +++ /dev/null @@ -1,60 +0,0 @@ -// Copyright 2025 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef THIRD_PARTY_CEL_CPP_RUNTIME_INTERNAL_ACTIVATION_MATCHER_ACCESS_H_ -#define THIRD_PARTY_CEL_CPP_RUNTIME_INTERNAL_ACTIVATION_MATCHER_ACCESS_H_ - -#include - -#include "absl/base/nullability.h" -#include "runtime/internal/attribute_matcher.h" - -namespace google::api::expr::runtime { -class Activation; -class BaseActivation; -} // namespace google::api::expr::runtime - -namespace cel { -class Activation; -class ActivationInterface; -} // namespace cel - -namespace cel::runtime_internal { - -class ActivationAttributeMatcherAccess { - public: - static void SetAttributeMatcher( - google::api::expr::runtime::Activation& activation, - const AttributeMatcher* matcher); - - static void SetAttributeMatcher( - google::api::expr::runtime::Activation& activation, - std::unique_ptr matcher); - - static const AttributeMatcher* absl_nullable GetAttributeMatcher( - const google::api::expr::runtime::BaseActivation& activation); - - static void SetAttributeMatcher(Activation& activation, - const AttributeMatcher* matcher); - - static void SetAttributeMatcher( - Activation& activation, std::unique_ptr matcher); - - static const AttributeMatcher* absl_nullable GetAttributeMatcher( - const ActivationInterface& activation); -}; - -} // namespace cel::runtime_internal - -#endif // THIRD_PARTY_CEL_CPP_RUNTIME_INTERNAL_ACTIVATION_MATCHER_ACCESS_H_ diff --git a/runtime/internal/attribute_matcher.h b/runtime/internal/attribute_matcher.h deleted file mode 100644 index a168b714c..000000000 --- a/runtime/internal/attribute_matcher.h +++ /dev/null @@ -1,48 +0,0 @@ -// Copyright 2025 Google LLC -// -// Licensed under the Apache License, Version 2.0 (the "License"); -// you may not use this file except in compliance with the License. -// You may obtain a copy of the License at -// -// https://www.apache.org/licenses/LICENSE-2.0 -// -// Unless required by applicable law or agreed to in writing, software -// distributed under the License is distributed on an "AS IS" BASIS, -// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -// See the License for the specific language governing permissions and -// limitations under the License. - -#ifndef THIRD_PARTY_CEL_CPP_RUNTIME_INTERNAL_ATTRIBUTE_MATCHER_H_ -#define THIRD_PARTY_CEL_CPP_RUNTIME_INTERNAL_ATTRIBUTE_MATCHER_H_ - -#include "base/attribute.h" - -namespace cel::runtime_internal { - -// Interface for matching unknown and missing attributes against the -// observed attribute trail at runtime. -class AttributeMatcher { - public: - using MatchResult = cel::AttributePattern::MatchType; - - virtual ~AttributeMatcher() = default; - - // Checks whether the attribute trail matches any unknown patterns. - // Used to identify and collect referenced unknowns in an UnknownValue. - virtual MatchResult CheckForUnknown(const Attribute& attr - [[maybe_unused]]) const { - return MatchResult::NONE; - }; - - // Checks whether the attribute trail matches any missing patterns. - // Used to identify missing attributes, and report an error if referenced - // directly. - virtual MatchResult CheckForMissing(const Attribute& attr - [[maybe_unused]]) const { - return MatchResult::NONE; - }; -}; - -} // namespace cel::runtime_internal - -#endif // THIRD_PARTY_CEL_CPP_RUNTIME_INTERNAL_ATTRIBUTE_MATCHER_H_ diff --git a/testing/testrunner/runner_lib_test.cc b/testing/testrunner/runner_lib_test.cc index 3b0e3f586..c34a1b1b0 100644 --- a/testing/testrunner/runner_lib_test.cc +++ b/testing/testrunner/runner_lib_test.cc @@ -679,16 +679,6 @@ class TestCustomActivation : public cel::ActivationInterface { absl::string_view name) const override { return {}; } - - absl::Span GetUnknownAttributes() - const override { - return {}; - } - - absl::Span GetMissingAttributes() - const override { - return {}; - } }; } // namespace