[clang-tools-extra] [clang] [clang][NFC] Refactor `CXXNewExpr::InitializationStyle` (re-land) (PR #71417)

Balazs Benics via cfe-commits cfe-commits at lists.llvm.org
Wed Jan 17 23:12:55 PST 2024


steakhal wrote:

The commit _"[clang][NFC] Refactor `CXXNewExpr::InitializationStyle` (re-land) (#71417)"_ https://github.com/llvm/llvm-project/commit/e929f0694aeb5f8cdbd2369db6189d28bb6fbcf3 appears to be a functional change, as it has a side effect on the following test code:

```diff
diff --git a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
index 8f0dd5602307..59a1a673dd99 100644
--- a/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
+++ b/clang/unittests/ASTMatchers/ASTMatchersNodeTest.cpp
@@ -2738,6 +2738,35 @@ TEST(MatchFinderAPI, MatchesDynamic) {
   EXPECT_EQ(MethodNode, GlobalMethodNode);
 }
 
+TEST(MatchFinderAPI, InvalidParenOrBraceRange) {
+  StringRef SourceCode = R"cpp(
+struct Base {
+  Base(int x, int y, int z);
+};
+class Derived : public Base {
+public:
+  Derived();
+  explicit Derived(int x);
+};
+void top() {
+  const auto *p = new Derived;
+}
+  )cpp";
+
+  // master: getInitializationStyle() == CXXNewExpr::NoInit
+
+  auto Matcher = cxxNewExpr(has(cxxConstructExpr())).bind("target");
+  auto AstUnit = tooling::buildASTFromCode(SourceCode);
+  auto Matches = matchDynamic(Matcher, AstUnit->getASTContext());
+  const auto &SM = AstUnit->getSourceManager();
+  ASSERT_EQ(Matches.size(), 1u);
+  ASSERT_EQ(Matches[0].getMap().size(), 1u);
+  const auto *NewExpr = Matches[0].getNodeAs<CXXNewExpr>("target");
+  ASSERT_TRUE(NewExpr != nullptr);
+  int Actual = (int)NewExpr->getInitializationStyle();
+  ASSERT_EQ(0, Actual);
+}
+
 static std::vector<TestClangConfig> allTestClangConfigs() {
   std::vector<TestClangConfig> all_configs;
   for (TestLanguage lang : {Lang_C89, Lang_C99, Lang_CXX03, Lang_CXX11,
```

The assertion `ASSERT_EQ(0, Actual)` would pass on the commit prior to e929f0694aeb5f8cdbd2369db6189d28bb6fbcf3, and break with it, and report that the `Actual` is `1`.

Reproducer instructions after applying the patch:
```bash
ninja -C build/rel ASTMatchersTests
./build/rel/tools/clang/unittests/ASTMatchers/ASTMatchersTests --gtest_filter=MatchFinderAPI.InvalidParenOrBraceRange
```

I'd qualify this as a regression, by looking at that the commit was supposed to be an NFC.
Could you please confirm @Endilll?

https://github.com/llvm/llvm-project/pull/71417


More information about the cfe-commits mailing list