[PATCH] D80606: [libTooling][NFC] Demo bug introduced in D72534.

2020-05-27 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits 
ymandel added a comment.

In D80606#2057136 , @steveire wrote:

> It might make sense for now (in order to unblock you) to make the 
> `Transformer` library explicitly require the `AsIs` mode. I am not so 
> familiar with the transformer library, but I think you can do that by adding 
> `traverse(AsIs, ...)` in `Transformer::registerMatchers`.
>
> The real fix is probably in adjusting `transformer::detail::buildMatchers`. I 
> can look into that in a few hours.


Thanks for the suggestion - I agree that changing `registerMatchers` would be a 
short term fix, but I think its better to just fix `buildMatchers` directly. 
I'm working on a patch now, so need to look into it (though thanks for the 
offer!)


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80606/new/

https://reviews.llvm.org/D80606



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80606: [libTooling][NFC] Demo bug introduced in D72534.

2020-05-27 Thread Stephen Kelly via Phabricator via cfe-commits 
steveire added a comment.

It might make sense for now (in order to unblock you) to make the `Transformer` 
library explicitly require the `AsIs` mode. I am not so familiar with the 
transformer library, but I think you can do that by adding `traverse(AsIs, 
...)` in `Transformer::registerMatchers`.

The real fix is probably in adjusting `transformer::detail::buildMatchers`. I 
can look into that in a few hours.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D80606/new/

https://reviews.llvm.org/D80606



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D80606: [libTooling][NFC] Demo bug introduced in D72534.

2020-05-26 Thread Yitzhak Mandelbaum via Phabricator via cfe-commits 
ymandel created this revision.
ymandel added reviewers: hokein, gribozavr.
Herald added a project: clang.

DO NOT PUSH. This patch includes two new tests that demo a bug introduced into
Transformer by https://reviews.llvm.org/D72534. This patch is intended only as a
demonstration of the problem.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D80606

Files:
  clang/unittests/Tooling/TransformerTest.cpp


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -571,6 +571,54 @@
   testRule(Rule, Input, Expected);
 }
 
+// Demonstrates bug in applyFirst caused by
+// https://reviews.llvm.org/D72534. This passes before and fails after.
+TEST_F(TransformerTest, OrderedRuleUnrelatedIgnored) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 = makeRule(castExpr(hasSourceExpression(callExpr())),
+   changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
+// BAD FIX: This attempt by the API client to fix the problem fails.
+TEST_F(TransformerTest, OrderedRuleUnrelatedIgnoredBadFix) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 = makeRule(
+  traverse(clang::TK_AsIs, castExpr(hasSourceExpression(callExpr(,
+  changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
 //
 // Negative tests (where we expect no transformation to occur).
 //


Index: clang/unittests/Tooling/TransformerTest.cpp
===
--- clang/unittests/Tooling/TransformerTest.cpp
+++ clang/unittests/Tooling/TransformerTest.cpp
@@ -571,6 +571,54 @@
   testRule(Rule, Input, Expected);
 }
 
+// Demonstrates bug in applyFirst caused by
+// https://reviews.llvm.org/D72534. This passes before and fails after.
+TEST_F(TransformerTest, OrderedRuleUnrelatedIgnored) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 = makeRule(castExpr(hasSourceExpression(callExpr())),
+   changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
+// BAD FIX: This attempt by the API client to fix the problem fails.
+TEST_F(TransformerTest, OrderedRuleUnrelatedIgnoredBadFix) {
+  std::string Input = R"cc(
+void f1();
+int f2();
+void call_f1() { f1(); }
+float call_f2() { return f2(); }
+  )cc";
+  std::string Expected = R"cc(
+void f1();
+int f2();
+void call_f1() { REPLACE_F1; }
+float call_f2() { return REPLACE_F2; }
+  )cc";
+
+  RewriteRule ReplaceF1 =
+  makeRule(callExpr(callee(functionDecl(hasName("f1",
+   changeTo(cat("REPLACE_F1")));
+  RewriteRule ReplaceF2 = makeRule(
+  traverse(clang::TK_AsIs, castExpr(hasSourceExpression(callExpr(,
+  changeTo(cat("REPLACE_F2")));
+  testRule(applyFirst({ReplaceF1, ReplaceF2}), Input, Expected);
+}
+
 //
 // Negative tests (where we expect no transformation to occur).
 //
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits