Repository navigation
Replace string matcher with AST matcher - #527
Conversation
| v2.push(0); | ||
| v2.push(1); | ||
| v2.push(3); | ||
| { |
There was a problem hiding this comment.
The old string matcher was matching the push_back(const T&) version, which was wrong. The new matcher matches push_back(T&&), which is ok. I a future PR we can rewrite the body of the rules so that they don't generate useless temporaries. The ideal output is: v2.push(0)
| typename It, | ||
| typename T1 = typename std::remove_const< | ||
| typename std::iterator_traits<It>::value_type::first_type>::type, | ||
| typename T2 = typename std::iterator_traits<It>::value_type::second_type, |
There was a problem hiding this comment.
why do we need all of this?
There was a problem hiding this comment.
Is not ready for review yet
a329484 to
983c30d
Compare
|
@nunoplopes this is ready for review. But please take a look at #529 first |
| typedef BrotliDecoderErrorCode t5; | ||
|
|
||
| BrotliEncoderMode f1() { return BROTLI_MODE_FONT; } | ||
| BrotliEncoderMode f1(void) { return BROTLI_MODE_FONT; } |
There was a problem hiding this comment.
Replacing () with (void) is needed because C++ considers () prototype-less
| } | ||
|
|
||
| template <typename T1, typename T2> | ||
| std::map<T1, T2> f6(const std::map<T1, T2> &&o) { |
There was a problem hiding this comment.
const && is a wrong rule that was not caught in the old matcher. Same for vector, it had some wrong const && rules
Closes #447, depends on #529
Rules are matched on the AST instead of on printed strings. string_matcher and ir_src.json are deleted. cpp-rule-prepocessor is replaced by cpp-rule-indexer which walks the rules, compiles them, and creates an index (#451).
The bodies of the used rules are loaded in the TU to match against their usages.
A rule matches a usage if calling the rule with the operands of the usage would end up calling the same declaration as the usage. Concretely:
Member{class, member_name},Constructor{class},Unqualified{name}. In Member we keep: data members, static members, function members. In Unqualified we keep everything outside a class: free function, macro, enumerators, etc. Every loaded rule with the same key is a candidate.For example:
At load time: both f1 and f2 are stored under
Member{vector, push_back}.RuleRegistry::Search(expr)which reachesRuleRegistry::findExpr. The decl of the usage (vector<int>::push_back(int&&)) is also keyed underMember{vector, push_back}.Matchtakes the operands[v, 3]from the usage and gets:i.
f1(vector<T1>&, const T1&)->T1 = intii.
f2(vector<T1>&, T1&&)->T1 = inti. f1:
a0.push_back(a1)callspush_back(const int&). Does not match the usage decl.ii. f2:
a0.push_back(std::move(a1))callspush_back(int&&). It matches the usage decl.Type rules work the same. The usage type is deduced against the rule's alias with sugar preferred over the canonical type.
Not all rules have deducible arguments, for example
std::map<T1, T2>::const_iterator, in general every template argument that sits on the left of::is non-deducible. Such rules need changes so they become deducible:Both T1 and T2 originally sat in non-deducible contexts. The new syntax works like this:
Itwhich is always deducibleT1andT2are computed fromItusing iterator traits:T1 = It::value_type::first_typeandT2 = It::value_type::second_typeSameAsconstraints the rule only for const iterators ofstd::mapso that they don't conflict with the rules for non-cosnt iterators forstd::mapor other map typesPerformance
Some nice implications for rule writing:
T*,T**,T&,const T*, etc. We only write one rule for T and the converter takes care of the rest, seerules/builtin_types/src.candrules/iostream/src.cpprules/pair/src.cppvector<T, A>, rules forvector<T>are not needed. This drops a lot of useless rules inrules/vector/src.cppThis opens another new interesting opportunity: writing rules for multiple statements at once. Now that we match AST subtrees we can match an entire subtree at once and write more idiomatic rules. Also we can move some of the converter translations in rules: for exmaple binary operators which have a very complicated visitor today.
Next steps after this PR: