Skip to content

Conversation

@jurahul
Copy link
Contributor

@jurahul jurahul commented May 21, 2025

ConstantDataFirstVal is 0, so getValueID() >= ConstantDataFirstVal leads to a compiler warning that the expression is always true. Replace such comparisons with a static_assert() to verify that ConstantDataFirstVal is 0, similar to the existing code in Value.h

`ConstantDataFirstVal` is 0, so `getValueID() >= ConstantDataFirstVal`
leads to a compiler warning that the expression is always true. Replace
such comparisons with a static_assert() to verify that `ConstantDataFirstVal`
is 0, similar to the existing code in Value.h
@jurahul
Copy link
Contributor Author

jurahul commented May 21, 2025

Seems to be triggered by 1ead4a8

@jurahul jurahul requested review from arsenm and nikic May 21, 2025 01:18
@jurahul jurahul marked this pull request as ready for review May 21, 2025 01:18
@llvmbot
Copy link
Member

llvmbot commented May 21, 2025

@llvm/pr-subscribers-llvm-ir

Author: Rahul Joshi (jurahul)

Changes

ConstantDataFirstVal is 0, so getValueID() >= ConstantDataFirstVal leads to a compiler warning that the expression is always true. Replace such comparisons with a static_assert() to verify that ConstantDataFirstVal is 0, similar to the existing code in Value.h


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

2 Files Affected:

  • (modified) llvm/include/llvm/IR/Constants.h (+3-2)
  • (modified) llvm/include/llvm/IR/Value.h (+5-3)
diff --git a/llvm/include/llvm/IR/Constants.h b/llvm/include/llvm/IR/Constants.h
index 7137c699cc992..6a01e8209071d 100644
--- a/llvm/include/llvm/IR/Constants.h
+++ b/llvm/include/llvm/IR/Constants.h
@@ -74,8 +74,9 @@ class ConstantData : public Constant {
 
   /// Methods to support type inquiry through isa, cast, and dyn_cast.
   static bool classof(const Value *V) {
-    return V->getValueID() >= ConstantDataFirstVal &&
-           V->getValueID() <= ConstantDataLastVal;
+    static_assert(Value::ConstantDataFirstVal == 0,
+                  "V->getValueID() >= Value::ConstantDataFirstVal");
+    return V->getValueID() <= ConstantDataLastVal;
   }
 };
 
diff --git a/llvm/include/llvm/IR/Value.h b/llvm/include/llvm/IR/Value.h
index c276899e673a3..999eeb3dad874 100644
--- a/llvm/include/llvm/IR/Value.h
+++ b/llvm/include/llvm/IR/Value.h
@@ -987,15 +987,17 @@ template <class Compare> void Value::sortUseList(Compare Cmp) {
 //
 template <> struct isa_impl<Constant, Value> {
   static inline bool doit(const Value &Val) {
-    static_assert(Value::ConstantFirstVal == 0, "Val.getValueID() >= Value::ConstantFirstVal");
+    static_assert(Value::ConstantFirstVal == 0,
+                  "Val.getValueID() >= Value::ConstantFirstVal");
     return Val.getValueID() <= Value::ConstantLastVal;
   }
 };
 
 template <> struct isa_impl<ConstantData, Value> {
   static inline bool doit(const Value &Val) {
-    return Val.getValueID() >= Value::ConstantDataFirstVal &&
-           Val.getValueID() <= Value::ConstantDataLastVal;
+    static_assert(Value::ConstantDataFirstVal == 0,
+                  "Val.getValueID() >= Value::ConstantDataFirstVal");
+    return Val.getValueID() <= Value::ConstantDataLastVal;
   }
 };
 

@jurahul
Copy link
Contributor Author

jurahul commented May 21, 2025

This caused build failures for our downstream build that had warning-as-error enabled. Particularly https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-4-c4296?view=msvc-170

@jurahul jurahul requested a review from justinfargnoli May 21, 2025 04:32
@jurahul jurahul merged commit fb627e3 into llvm:main May 21, 2025
15 checks passed
@jurahul jurahul deleted the ConstantDataFirstVal_static_assert branch May 21, 2025 13:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants