Negated scalar condition for matchers #335
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #335 +/- ##
==========================================
+ Coverage 84.61% 84.75% +0.14%
==========================================
Files 148 149 +1
Lines 7080 7152 +72
Branches 3271 3301 +30
==========================================
+ Hits 5991 6062 +71
+ Misses 411 409 -2
- Partials 678 681 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
BenchmarksBenchmark execution time: 2024-09-26 13:05:05 Comparing candidate commit b7fc7a5 in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 0 metrics, 0 unstable metrics. scenario:global-benchmark.random
|
Anilm3
force-pushed
the
anilm3/negated_scalar_condition
branch
from
888d062
to
1fe02c2
Compare
Anilm3
force-pushed
the
anilm3/negated_scalar_condition
branch
from
6ee2834
to
d37459d
Compare
Anilm3
force-pushed
the
anilm3/negated_scalar_condition
branch
from
574fc85
to
2944e23
Compare
Anilm3
force-pushed
the
anilm3/negated_scalar_condition
branch
from
2944e23
to
42fc3fb
Compare
| if constexpr (std::is_same_v<ResultType, bool>) { | ||
| return true; | ||
| } else { | ||
| return {{{{"input"sv, object_to_string(dst), address, it.get_current_path()}}, |
There was a problem hiding this comment.
difficult to follow. You clearly expect a certain type here, judging by the very specific constructor but it's not mentioned until later in the file. Maybe adding requires(std::is_same_v<ResultType, bool> || std::is_same_v<ResultType, std::optional<condition_match>>) .
Although, I'd very much prefer you don't have this where you build the condition_match in this template for the scalar_condition and for the negated condition you build it in the caller... seems that what you need is a boolean to indicate to this template if you should generate the match when res is true or false.
| @@ -92,4 +99,25 @@ template <> class equals<double> : public base_impl<equals<double>> { | |||
| friend class base_impl<equals<double>>; | |||
| }; | |||
|
|
|||
| template <> class equals<void> : public base_impl<equals<void>> { | |||
There was a problem hiding this comment.
kind of hacky. you could just change parse_matcher to take the names of the matchers as non-type template parameters instead of having these fake void types.
| conditions.emplace_back( | ||
| std::make_unique<scalar_negated_condition>(std::move(matcher), data_id, | ||
| std::move(arguments), std::string{raw_operator_name}, limits)); | ||
| } |
There was a problem hiding this comment.
fwict this could easily be refactored to remove the duplication while improving legibility
This PR introduces the ability to negate certain operators, using the
!as a prefix. The set of operators supported are the following:!match_regex,!phrase_match,!exact_match,!ip_matchand!equals. Negated operators work through the use of thescalar_negated_condition, which operates in a similar manner to thescalar_condition, however only producing a match if the exhaustive evaluation of the given address + key_path results in no match.Negated operators have the following restrictions:
lower_thanandgreater_than, as their negated version can be an operator in and of itself, e.g.greater_equalandlower_equal.Remaining work: