SherAndrei wrote:
> Will any of the new tests trigger the bug?
Yes, two of them do fail if no suggested changes are present. Feel free to
reproduce it yourself.
```bash
./tools/clang/unittests/Tooling/ToolingTests
--gtest_filter=*Transform*RewriteDescendantsApplyFirst*
```
yields
```diff
Note: Google Test filter = *Transform*RewriteDescendantsApplyFirst*
[==========] Running 3 tests from 1 test suite.
[----------] Global test environment set-up.
[----------] 3 tests from TransformerTest
[ RUN ] TransformerTest.RewriteDescendantsApplyFirstOrderedRuleUnrelated
/root/clang-llvm/llvm-project/clang/unittests/Tooling/TransformerTest.cpp:94:
Failure
Expected equality of these values:
format(Expected)
Which is: "int f(int x) {\n char y = 3;\n return 3;\n}"
format(Actual)
Which is: "int f(int x) {\n 3 y = 3;\n return 3;\n}"
With diff:
@@ -1,4 +1,4 @@
int f(int x) {
- char y = 3;
+ 3 y = 3;
return 3;
}
[ FAILED ] TransformerTest.RewriteDescendantsApplyFirstOrderedRuleUnrelated
(17 ms)
[ RUN ] TransformerTest.RewriteDescendantsApplyFirstOrderedRuleRelated
/root/clang-llvm/llvm-project/clang/unittests/Tooling/TransformerTest.cpp:94:
Failure
Expected equality of these values:
format(Expected)
Which is: "int f(int x) {\n int y = 3;\n return y;\n}"
format(Actual)
Which is: "int f(int x) {\n int y = y;\n return y;\n}"
With diff:
@@ -1,4 +1,4 @@
int f(int x) {
- int y = 3;
+ int y = y;
return y;
}
[ FAILED ] TransformerTest.RewriteDescendantsApplyFirstOrderedRuleRelated (14
ms)
[ RUN ]
TransformerTest.RewriteDescendantsApplyFirstOrderedRuleRelatedSwapped
[ OK ]
TransformerTest.RewriteDescendantsApplyFirstOrderedRuleRelatedSwapped (13 ms)
[----------] 3 tests from TransformerTest (45 ms total)
[----------] Global test environment tear-down
[==========] 3 tests from 1 test suite ran. (45 ms total)
[ PASSED ] 1 test.
[ FAILED ] 2 tests, listed below:
[ FAILED ] TransformerTest.RewriteDescendantsApplyFirstOrderedRuleUnrelated
[ FAILED ] TransformerTest.RewriteDescendantsApplyFirstOrderedRuleRelated
2 FAILED TESTS
```
> Can you clarify where the matchers associated with the nested rule will be
> shifted? As far as I can tell, the original match uses Tag0 but happens in a
> separate context (that is, MatchFinder) than the nested match so I don't see
> how they are interacting.
Yes, indeed the original match happens in separate context, but all contexts
share one `NodesMap`,
which values are overriden with a consecutive match ('clash'). Let's consider
test
`RewriteDescendantsApplyFirstOrderedRuleUnrelated` before suggested changes:
1. `Transformer::registerMatchers` calls to `taggedMatchers`: `FunctionDecl`
matcher, we'll call it FD, receives "Tag0",
2. then `ApplyRuleCallback::registerMatchers` calls to `taggedMatchers`: first
`DeclRefExpr` matcher, let's call it DRE1, receives "Tag0", second
`DeclRefExpr` matcher -- DRE2 -- receives "Tag1",
3. when DRE1 matches, `ApplyRuleCallback::run` calls to `findSelectedCase`: it
successfully finds "Tag0" which is bound to FD and not to expected DRE1.
Now let's consider same test with suggested changes
1. `Transformer::registerMatchers` binds FD to "Tag93825274205568",
2. `ApplyRuleCallback::registerMatchers` binds DRE1 to "Tag93825274195136",
DRE2 to "Tag93825274196288",
3. when DRE1 matches, `findSelectedCase` finds expected DRE1 by searching for
"Tag93825274195136"
Sorry for misleading you by using the word 'shifted' here. Nothing is shifted
here really. I do struggle to update description of the commit. What do you
think about next description of the commit:
```
[clang][transformer] Allow usage of applyFirst with rewriteDescendants
Previously,, `taggedMatchers` could apply same tag to two different matchers
(here, for enclosing matcher and for first from `applyFirst`).
We fix this by making sure that associated Tags are unique and deterministic as
they are intend to be.
```
https://github.com/llvm/llvm-project/pull/117658
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
