Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[RecursiveASTVisitor] Fix RecursiveASTVisitor (RAV) fails to visit the initializer of a bitfield #69557

Merged
merged 4 commits into from Oct 26, 2023

Conversation

xgupta
Copy link
Contributor

@xgupta xgupta commented Oct 19, 2023

The problem was introduced in commit 6b8e3c0 when the possibility of initialized bitfields was added, but the logic in RecursiveASTVisitor was not updated.

This fixes #64916.

Patch by Scott McPeak

@llvmbot llvmbot added clang Clang issues not falling into any other category clang:frontend Language frontend issues, e.g. anything involving "Sema" labels Oct 19, 2023
@llvmbot
Copy link
Collaborator

llvmbot commented Oct 19, 2023

@llvm/pr-subscribers-clang

Author: Shivam Gupta (xgupta)

Changes

This fixes #64916.

Patch by Scott McPeak


Full diff: https://github.com/llvm/llvm-project/pull/69557.diff

3 Files Affected:

  • (modified) clang/include/clang/AST/RecursiveASTVisitor.h (+1-1)
  • (modified) clang/unittests/Tooling/CMakeLists.txt (+1)
  • (added) clang/unittests/Tooling/RecursiveASTVisitorTests/BitfieldInitializer.cpp (+34)
diff --git a/clang/include/clang/AST/RecursiveASTVisitor.h b/clang/include/clang/AST/RecursiveASTVisitor.h
index 3dd23eb38eeabfc..53bc15e1b19f668 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -2103,7 +2103,7 @@ DEF_TRAVERSE_DECL(FieldDecl, {
   TRY_TO(TraverseDeclaratorHelper(D));
   if (D->isBitField())
     TRY_TO(TraverseStmt(D->getBitWidth()));
-  else if (D->hasInClassInitializer())
+  if (D->hasInClassInitializer())
     TRY_TO(TraverseStmt(D->getInClassInitializer()));
 })
 
diff --git a/clang/unittests/Tooling/CMakeLists.txt b/clang/unittests/Tooling/CMakeLists.txt
index 2fbe78e3fab7528..5a10a6b285390e9 100644
--- a/clang/unittests/Tooling/CMakeLists.txt
+++ b/clang/unittests/Tooling/CMakeLists.txt
@@ -25,6 +25,7 @@ add_clang_unittest(ToolingTests
   QualTypeNamesTest.cpp
   RangeSelectorTest.cpp
   RecursiveASTVisitorTests/Attr.cpp
+  RecursiveASTVisitorTests/BitfieldInitializer.cpp
   RecursiveASTVisitorTests/CallbacksLeaf.cpp
   RecursiveASTVisitorTests/CallbacksUnaryOperator.cpp
   RecursiveASTVisitorTests/CallbacksBinaryOperator.cpp
diff --git a/clang/unittests/Tooling/RecursiveASTVisitorTests/BitfieldInitializer.cpp b/clang/unittests/Tooling/RecursiveASTVisitorTests/BitfieldInitializer.cpp
new file mode 100644
index 000000000000000..676a491a43040ea
--- /dev/null
+++ b/clang/unittests/Tooling/RecursiveASTVisitorTests/BitfieldInitializer.cpp
@@ -0,0 +1,34 @@
+//===- unittest/Tooling/RecursiveASTVisitorTests/BitfieldInitializer.cpp -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===----------------------------------------------------------------------===//
+
+#include "TestVisitor.h"
+#include <string>
+
+using namespace clang;
+
+namespace {
+
+// Check to ensure that bitfield initializers are visited.
+class BitfieldInitializerVisitor : public ExpectedLocationVisitor<BitfieldInitializerVisitor> {
+public:
+  bool VisitIntegerLiteral(IntegerLiteral *IL) {
+    Match(std::to_string(IL->getValue().getSExtValue()), IL->getLocation());
+    return true;
+  }
+};
+
+TEST(RecursiveASTVisitor, BitfieldInitializerIsVisited) {
+  BitfieldInitializerVisitor Visitor;
+  Visitor.ExpectMatch("123", 2, 15); 
+  EXPECT_TRUE(Visitor.runOver(
+    "struct S {\n"
+    "  int x : 8 = 123;\n"
+    "};\n"));
+}
+
+} // end anonymous namespace

@github-actions
Copy link

github-actions bot commented Oct 19, 2023

✅ With the latest revision this PR passed the C/C++ code formatter.

@cor3ntin
Copy link
Contributor

Can you add a release note for this change? It looks good otherwise

@cor3ntin
Copy link
Contributor

I am not sure what happened here. It probably needs a rebase.

@ldionne ldionne removed request for a team October 25, 2023 22:12
Copy link
Contributor

@cor3ntin cor3ntin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks!

Co-authored-by: cor3ntin <corentinjabot@gmail.com>
@xgupta xgupta merged commit 897cc8a into llvm:main Oct 26, 2023
3 of 4 checks passed
@xgupta
Copy link
Contributor Author

xgupta commented Oct 26, 2023

Thanks @cor3ntin for the review!

zahiraam pushed a commit to zahiraam/llvm-project that referenced this pull request Oct 26, 2023
…e initializer of a bitfield (llvm#69557)

The problem was introduced in the commit
llvm@6b8e3c0
when the possibility of initialized bitfields was added, but the logic
in RecursiveASTVisitor was not updated. This PR fixed that.

This fixes llvm#64916.

Patch by Scott McPeak

---------

Co-authored-by: cor3ntin <corentinjabot@gmail.com>
@xgupta xgupta deleted the RecursiveASTVisitor branch November 21, 2023 03:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
clang:frontend Language frontend issues, e.g. anything involving "Sema" clang Clang issues not falling into any other category
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RecursiveASTVisitor (RAV) fails to visit the initializer of a bitfield
3 participants