Skip to content

[clang][NFC] Fix the static assertion in 4797437 #120643

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

Merged
merged 1 commit into from
Dec 20, 2024

Conversation

ziqingluo-90
Copy link
Contributor

In the previous commit 4797437, I used llvm::isInt<NumStmtBits>(StmtClass::LAST##Class) to test if StmtClass is strictly bounded by the an unsigned integer of 'NumStmtBits'. That is incorrect as llvm::isInt tests for signed integers. This commit fixes it.

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

llvmbot commented Dec 19, 2024

@llvm/pr-subscribers-clang

Author: Ziqing Luo (ziqingluo-90)

Changes

In the previous commit 4797437, I used llvm::isInt&lt;NumStmtBits&gt;(StmtClass::LAST##Class) to test if StmtClass is strictly bounded by the an unsigned integer of 'NumStmtBits'. That is incorrect as llvm::isInt tests for signed integers. This commit fixes it.


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

1 Files Affected:

  • (modified) clang/include/clang/AST/Stmt.h (+4-4)
diff --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index 07cb63956aed0d..ef6824199b0af0 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -114,10 +114,10 @@ class alignas(void *) Stmt {
 #define STMT(CLASS, PARENT)
 #define STMT_RANGE(BASE, FIRST, LAST)
 #define LAST_STMT_RANGE(BASE, FIRST, LAST)                                     \
-  static_assert(                                                               \
-      llvm::isInt<NumStmtBits>(StmtClass::LAST##Class),                        \
-      "The number of 'StmtClass'es is strictly bounded under two to "          \
-      "the power of 'NumStmtBits'");
+  static_assert(0 <= StmtClass::LAST##Class &&                                 \
+                    StmtClass::LAST##Class < (INT64_C(1) << NumStmtBits),      \
+                "The number of 'StmtClass'es is strictly bound by a bitfield " \
+                "of width NumStmtBits");
 #define ABSTRACT_STMT(STMT)
 #include "clang/AST/StmtNodes.inc"
 

@ziqingluo-90 ziqingluo-90 force-pushed the ziqing/PR-141555357-2 branch 2 times, most recently from dd71be8 to d1b45d1 Compare December 19, 2024 22:30
Copy link

github-actions bot commented Dec 19, 2024

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

In the previous commit 4797437, I
used `llvm::isInt<NumStmtBits>(StmtClass::LAST##Class)` to test if
`StmtClass` is strictly bounded by the an unsigned integer of
'NumStmtBits'.  That is incorrect as `llvm::isInt` tests for signed
integers. This commit fixes it.
@jroelofs
Copy link
Contributor

may as well address @nikic's post-commit suggestion to move this out of the header, and into a source file, while you're here: #120341 (comment)

@ziqingluo-90 ziqingluo-90 merged commit 399c3a7 into llvm:main Dec 20, 2024
8 checks passed
@ziqingluo-90
Copy link
Contributor Author

#120341 (comment)

oops, I saw this after I hit the merge button. I will do it in a follow-up commit.

ziqingluo-90 added a commit that referenced this pull request Dec 20, 2024
@ziqingluo-90 ziqingluo-90 deleted the ziqing/PR-141555357-2 branch February 6, 2025 21:04
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.

3 participants