[clang] [Clang][Diagnostics] Update select uses in DiagnosticXKinds.td to use enum_select (PR #130868)

[Ayokunle Amodu via cfe-commits]

https://github.com/ayokunle321 updated 
https://github.com/llvm/llvm-project/pull/130868

>From dfc517be06531af965dc51b09a0f1ae7965e3e20 Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle...@users.noreply.github.com>
Date: Fri, 14 Mar 2025 15:13:05 -0600
Subject: [PATCH 1/4] revert changes in ASTKinds.td file

---
 clang/include/clang/Basic/DiagnosticASTKinds.td | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index ac53778339a20..9faa8eec56b40 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -1041,4 +1041,4 @@ def warn_unpacked_field
 def warn_unaligned_access : Warning<
   "field %1 within %0 is less aligned than %2 and is usually due to %0 being "
   "packed, which can lead to unaligned accesses">, InGroup, 
DefaultIgnore;
-}
+}
\ No newline at end of file

>From 9e08ddfb5931d06fa0e725ae7dea8de781489ab7 Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle...@users.noreply.github.com>
Date: Wed, 19 Mar 2025 00:12:11 -0600
Subject: [PATCH 2/4] refactor select in note_constexpr_invalid_cast

---
 clang/include/clang/Basic/DiagnosticASTKinds.td |  5 +++--
 clang/lib/AST/ByteCode/Interp.h |  6 +++---
 clang/lib/AST/ExprConstant.cpp  | 16 
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 9faa8eec56b40..7167f95e88296 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -11,8 +11,9 @@ let Component = "AST" in {
 // Constant expression diagnostics. These (and their users) belong in Sema.
 def note_expr_divide_by_zero : Note<"division by zero">;
 def note_constexpr_invalid_cast : Note<
-  "%select{reinterpret_cast|dynamic_cast|%select{this conversion|cast that"
-  " performs the conversions of a reinterpret_cast}1|cast from %1}0"
+  
"%enum_select{%Reinterpret{reinterpret_cast}|%Dynamic{dynamic_cast}|"
+  "%ThisCastOrReinterpret{%select{this conversion|cast that performs the 
conversions "
+  "of a reinterpret_cast}1}|%CastFrom{cast from %1}}0"
   " is not allowed in a constant expression"
   "%select{| in C++ standards before C++20||}0">;
 def note_constexpr_invalid_void_star_cast : Note<
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index f2ddeac99cd7e..8956a69c7124c 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2366,12 +2366,12 @@ static inline bool PtrPtrCast(InterpState &S, CodePtr 
OpPC, bool SrcIsVoidPtr) {
 } else if (!S.getLangOpts().CPlusPlus26) {
   const SourceInfo &E = S.Current->getSource(OpPC);
   S.CCEDiag(E, diag::note_constexpr_invalid_cast)
-  << 3 << "'void *'" << S.Current->getRange(OpPC);
+  << diag::CastKind::CastFrom << "'void *'" << 
S.Current->getRange(OpPC);
 }
   } else {
 const SourceInfo &E = S.Current->getSource(OpPC);
 S.CCEDiag(E, diag::note_constexpr_invalid_cast)
-<< 2 << S.getLangOpts().CPlusPlus << S.Current->getRange(OpPC);
+<< diag::CastKind::ThisCastOrReinterpret << S.getLangOpts().CPlusPlus 
<< S.Current->getRange(OpPC);
   }
 
   return true;
@@ -2736,7 +2736,7 @@ inline bool GetIntPtr(InterpState &S, CodePtr OpPC, const 
Descriptor *Desc) {
 
   if (Desc)
 S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_invalid_cast)
-<< 2 << S.getLangOpts().CPlusPlus;
+<< diag::CastKind::ThisCastOrReinterpret << S.getLangOpts().CPlusPlus;
 
   S.Stk.push(static_cast(IntVal), Desc);
   return true;
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index f8e8aaddbfdbd..9eb8c6717900b 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8103,12 +8103,12 @@ class ExprEvaluatorBase
   }
 
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E) {
-CCEDiag(E, diag::note_constexpr_invalid_cast) << 0;
+CCEDiag(E, diag::note_constexpr_invalid_cast) << 
diag::CastKind::Reinterpret;
 return static_cast(this)->VisitCastExpr(E);
   }
   bool VisitCXXDynamicCastExpr(const CXXDynamicCastExpr *E) {
 if (!Info.Ctx.getLangOpts().CPlusPlus20)
-  CCEDiag(E, diag::note_constexpr_invalid_cast) << 1;
+  CCEDiag(E, diag::note_constexpr_invalid_cast) << diag::CastKind::Dynamic;
 return static_cast(this)->VisitCastExpr(E);
   }
   bool VisitBuiltinBitCastExpr(const BuiltinBitCastExpr *E) {
@@ -8833,7 +8833,7 @@ class LValueExprEvaluator
 
 case CK_LValueBitCast:
   this->CCEDiag(E, diag::note_constexpr_invalid_cast)
-  << 2 << Info.Ctx.getLangOpts().CPlusPlus;
+  << diag::CastKind::ThisCastOrReinterpret << 
Info.Ctx.getLangOpts().CPlusPlus;
   if (!Visit(E->getSubExpr()))
 return 

[clang] [Clang][Diagnostics] Update select uses in DiagnosticXKinds.td to use enum_select (PR #130868)

[Ayokunle Amodu via cfe-commits]

ayokunle321 wrote:

@erichkeane Hey! So I was able to change one, could you please take a look.

Also for this case:

Instruction:
%select{classdesign|coclass|dependency|helper"
  "|helperclass|helps|instancesize|ownership|performance|security|superclass}1

Use:
https://github.com/user-attachments/assets/4a2433f9-058c-482c-a452-1f89e2cbb0c0";
 />

I was wondering if it was okay for a replacement since the names of the 
CommandTraits options are the same


https://github.com/llvm/llvm-project/pull/130868
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Diagnostics] Update select uses in DiagnosticXKinds.td to use enum_select (PR #130868)

[Ayokunle Amodu via cfe-commits]

https://github.com/ayokunle321 updated 
https://github.com/llvm/llvm-project/pull/130868

>From dfc517be06531af965dc51b09a0f1ae7965e3e20 Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle...@users.noreply.github.com>
Date: Fri, 14 Mar 2025 15:13:05 -0600
Subject: [PATCH 1/2] revert changes in ASTKinds.td file

---
 clang/include/clang/Basic/DiagnosticASTKinds.td | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index ac53778339a20..9faa8eec56b40 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -1041,4 +1041,4 @@ def warn_unpacked_field
 def warn_unaligned_access : Warning<
   "field %1 within %0 is less aligned than %2 and is usually due to %0 being "
   "packed, which can lead to unaligned accesses">, InGroup, 
DefaultIgnore;
-}
+}
\ No newline at end of file

>From 9e08ddfb5931d06fa0e725ae7dea8de781489ab7 Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle...@users.noreply.github.com>
Date: Wed, 19 Mar 2025 00:12:11 -0600
Subject: [PATCH 2/2] refactor select in note_constexpr_invalid_cast

---
 clang/include/clang/Basic/DiagnosticASTKinds.td |  5 +++--
 clang/lib/AST/ByteCode/Interp.h |  6 +++---
 clang/lib/AST/ExprConstant.cpp  | 16 
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 9faa8eec56b40..7167f95e88296 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -11,8 +11,9 @@ let Component = "AST" in {
 // Constant expression diagnostics. These (and their users) belong in Sema.
 def note_expr_divide_by_zero : Note<"division by zero">;
 def note_constexpr_invalid_cast : Note<
-  "%select{reinterpret_cast|dynamic_cast|%select{this conversion|cast that"
-  " performs the conversions of a reinterpret_cast}1|cast from %1}0"
+  
"%enum_select{%Reinterpret{reinterpret_cast}|%Dynamic{dynamic_cast}|"
+  "%ThisCastOrReinterpret{%select{this conversion|cast that performs the 
conversions "
+  "of a reinterpret_cast}1}|%CastFrom{cast from %1}}0"
   " is not allowed in a constant expression"
   "%select{| in C++ standards before C++20||}0">;
 def note_constexpr_invalid_void_star_cast : Note<
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index f2ddeac99cd7e..8956a69c7124c 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2366,12 +2366,12 @@ static inline bool PtrPtrCast(InterpState &S, CodePtr 
OpPC, bool SrcIsVoidPtr) {
 } else if (!S.getLangOpts().CPlusPlus26) {
   const SourceInfo &E = S.Current->getSource(OpPC);
   S.CCEDiag(E, diag::note_constexpr_invalid_cast)
-  << 3 << "'void *'" << S.Current->getRange(OpPC);
+  << diag::CastKind::CastFrom << "'void *'" << 
S.Current->getRange(OpPC);
 }
   } else {
 const SourceInfo &E = S.Current->getSource(OpPC);
 S.CCEDiag(E, diag::note_constexpr_invalid_cast)
-<< 2 << S.getLangOpts().CPlusPlus << S.Current->getRange(OpPC);
+<< diag::CastKind::ThisCastOrReinterpret << S.getLangOpts().CPlusPlus 
<< S.Current->getRange(OpPC);
   }
 
   return true;
@@ -2736,7 +2736,7 @@ inline bool GetIntPtr(InterpState &S, CodePtr OpPC, const 
Descriptor *Desc) {
 
   if (Desc)
 S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_invalid_cast)
-<< 2 << S.getLangOpts().CPlusPlus;
+<< diag::CastKind::ThisCastOrReinterpret << S.getLangOpts().CPlusPlus;
 
   S.Stk.push(static_cast(IntVal), Desc);
   return true;
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index f8e8aaddbfdbd..9eb8c6717900b 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8103,12 +8103,12 @@ class ExprEvaluatorBase
   }
 
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E) {
-CCEDiag(E, diag::note_constexpr_invalid_cast) << 0;
+CCEDiag(E, diag::note_constexpr_invalid_cast) << 
diag::CastKind::Reinterpret;
 return static_cast(this)->VisitCastExpr(E);
   }
   bool VisitCXXDynamicCastExpr(const CXXDynamicCastExpr *E) {
 if (!Info.Ctx.getLangOpts().CPlusPlus20)
-  CCEDiag(E, diag::note_constexpr_invalid_cast) << 1;
+  CCEDiag(E, diag::note_constexpr_invalid_cast) << diag::CastKind::Dynamic;
 return static_cast(this)->VisitCastExpr(E);
   }
   bool VisitBuiltinBitCastExpr(const BuiltinBitCastExpr *E) {
@@ -8833,7 +8833,7 @@ class LValueExprEvaluator
 
 case CK_LValueBitCast:
   this->CCEDiag(E, diag::note_constexpr_invalid_cast)
-  << 2 << Info.Ctx.getLangOpts().CPlusPlus;
+  << diag::CastKind::ThisCastOrReinterpret << 
Info.Ctx.getLangOpts().CPlusPlus;
   if (!Visit(E->getSubExpr()))
 return 

[clang] [Clang][Diagnostics] Update select uses in DiagnosticXKinds.td to use enum_select (PR #130868)

[Ayokunle Amodu via cfe-commits]

https://github.com/ayokunle321 updated 
https://github.com/llvm/llvm-project/pull/130868

>From dfc517be06531af965dc51b09a0f1ae7965e3e20 Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle...@users.noreply.github.com>
Date: Fri, 14 Mar 2025 15:13:05 -0600
Subject: [PATCH 1/6] revert changes in ASTKinds.td file

---
 clang/include/clang/Basic/DiagnosticASTKinds.td | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index ac53778339a20..9faa8eec56b40 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -1041,4 +1041,4 @@ def warn_unpacked_field
 def warn_unaligned_access : Warning<
   "field %1 within %0 is less aligned than %2 and is usually due to %0 being "
   "packed, which can lead to unaligned accesses">, InGroup, 
DefaultIgnore;
-}
+}
\ No newline at end of file

>From 9e08ddfb5931d06fa0e725ae7dea8de781489ab7 Mon Sep 17 00:00:00 2001
From: Ayokunle Amodu <121697771+ayokunle...@users.noreply.github.com>
Date: Wed, 19 Mar 2025 00:12:11 -0600
Subject: [PATCH 2/6] refactor select in note_constexpr_invalid_cast

---
 clang/include/clang/Basic/DiagnosticASTKinds.td |  5 +++--
 clang/lib/AST/ByteCode/Interp.h |  6 +++---
 clang/lib/AST/ExprConstant.cpp  | 16 
 3 files changed, 14 insertions(+), 13 deletions(-)

diff --git a/clang/include/clang/Basic/DiagnosticASTKinds.td 
b/clang/include/clang/Basic/DiagnosticASTKinds.td
index 9faa8eec56b40..7167f95e88296 100644
--- a/clang/include/clang/Basic/DiagnosticASTKinds.td
+++ b/clang/include/clang/Basic/DiagnosticASTKinds.td
@@ -11,8 +11,9 @@ let Component = "AST" in {
 // Constant expression diagnostics. These (and their users) belong in Sema.
 def note_expr_divide_by_zero : Note<"division by zero">;
 def note_constexpr_invalid_cast : Note<
-  "%select{reinterpret_cast|dynamic_cast|%select{this conversion|cast that"
-  " performs the conversions of a reinterpret_cast}1|cast from %1}0"
+  
"%enum_select{%Reinterpret{reinterpret_cast}|%Dynamic{dynamic_cast}|"
+  "%ThisCastOrReinterpret{%select{this conversion|cast that performs the 
conversions "
+  "of a reinterpret_cast}1}|%CastFrom{cast from %1}}0"
   " is not allowed in a constant expression"
   "%select{| in C++ standards before C++20||}0">;
 def note_constexpr_invalid_void_star_cast : Note<
diff --git a/clang/lib/AST/ByteCode/Interp.h b/clang/lib/AST/ByteCode/Interp.h
index f2ddeac99cd7e..8956a69c7124c 100644
--- a/clang/lib/AST/ByteCode/Interp.h
+++ b/clang/lib/AST/ByteCode/Interp.h
@@ -2366,12 +2366,12 @@ static inline bool PtrPtrCast(InterpState &S, CodePtr 
OpPC, bool SrcIsVoidPtr) {
 } else if (!S.getLangOpts().CPlusPlus26) {
   const SourceInfo &E = S.Current->getSource(OpPC);
   S.CCEDiag(E, diag::note_constexpr_invalid_cast)
-  << 3 << "'void *'" << S.Current->getRange(OpPC);
+  << diag::CastKind::CastFrom << "'void *'" << 
S.Current->getRange(OpPC);
 }
   } else {
 const SourceInfo &E = S.Current->getSource(OpPC);
 S.CCEDiag(E, diag::note_constexpr_invalid_cast)
-<< 2 << S.getLangOpts().CPlusPlus << S.Current->getRange(OpPC);
+<< diag::CastKind::ThisCastOrReinterpret << S.getLangOpts().CPlusPlus 
<< S.Current->getRange(OpPC);
   }
 
   return true;
@@ -2736,7 +2736,7 @@ inline bool GetIntPtr(InterpState &S, CodePtr OpPC, const 
Descriptor *Desc) {
 
   if (Desc)
 S.CCEDiag(S.Current->getSource(OpPC), diag::note_constexpr_invalid_cast)
-<< 2 << S.getLangOpts().CPlusPlus;
+<< diag::CastKind::ThisCastOrReinterpret << S.getLangOpts().CPlusPlus;
 
   S.Stk.push(static_cast(IntVal), Desc);
   return true;
diff --git a/clang/lib/AST/ExprConstant.cpp b/clang/lib/AST/ExprConstant.cpp
index f8e8aaddbfdbd..9eb8c6717900b 100644
--- a/clang/lib/AST/ExprConstant.cpp
+++ b/clang/lib/AST/ExprConstant.cpp
@@ -8103,12 +8103,12 @@ class ExprEvaluatorBase
   }
 
   bool VisitCXXReinterpretCastExpr(const CXXReinterpretCastExpr *E) {
-CCEDiag(E, diag::note_constexpr_invalid_cast) << 0;
+CCEDiag(E, diag::note_constexpr_invalid_cast) << 
diag::CastKind::Reinterpret;
 return static_cast(this)->VisitCastExpr(E);
   }
   bool VisitCXXDynamicCastExpr(const CXXDynamicCastExpr *E) {
 if (!Info.Ctx.getLangOpts().CPlusPlus20)
-  CCEDiag(E, diag::note_constexpr_invalid_cast) << 1;
+  CCEDiag(E, diag::note_constexpr_invalid_cast) << diag::CastKind::Dynamic;
 return static_cast(this)->VisitCastExpr(E);
   }
   bool VisitBuiltinBitCastExpr(const BuiltinBitCastExpr *E) {
@@ -8833,7 +8833,7 @@ class LValueExprEvaluator
 
 case CK_LValueBitCast:
   this->CCEDiag(E, diag::note_constexpr_invalid_cast)
-  << 2 << Info.Ctx.getLangOpts().CPlusPlus;
+  << diag::CastKind::ThisCastOrReinterpret << 
Info.Ctx.getLangOpts().CPlusPlus;
   if (!Visit(E->getSubExpr()))
 return 

[clang] [Clang][Diagnostics] Update select uses in DiagnosticXKinds.td to use enum_select (PR #130868)

[Ayokunle Amodu via cfe-commits]

ayokunle321 wrote:

@erichkeane thanks a lot for the clarification. I think I have a hang of it 
now- I was kinda confused before. Now, my approach is to look through 
DiagnosticXKinds.td files -> check for a `select` with a significant number of 
items -> check its diagnostic ID -> grep the lib directory with this ID and 
find the uses of the diagnostic -> see if it’s up for replacement (caller has a 
lot of magic numbers/an enum made just for it). 

Now I wanted to ask - what do you consider a lot - 3 or more uses with magic 
numbers? 

For a `select` with a significant number of items, I’m thinking of going with 4 
or more items as it seems reasonable. I was also looking through the codebase 
using this heuristic exposed a lot more for replacement (rather than 5 which I 
had in mind before). 

Also could you just brief me through the process of a change. I know the .td 
files are used to generate header files. So do I have to change the select to 
an enum_select first in the .td files and then build and then reference the 
generated enum option in the caller (e.g. diag::MemClassWork::AliasDecl) and 
then rebuild. The thing about this is that the build would probably fail the 
first time as the callers will still make use of the magic numbers. I also 
found an enum declared just for a select that was already in a header file. For 
this case, I just remove this declaration right? And then replace the select. I 
guess I’m just confused about the whole header generation process - where the 
new enums would live since the relevant header files already exist before a 
build (e.g Sema.h, Decl.h) and since we’re making changes to .td files which 
are the header generators. Kinda new to clang stuff so forgive me for these 
questions 😅

https://github.com/llvm/llvm-project/pull/130868
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Diagnostics] Update select uses in DiagnosticXKinds.td to use enum_select (PR #130868)

[Ayokunle Amodu via cfe-commits]

ayokunle321 wrote:

Okay cool. I'll change the option's name but for the .td file, I don't think it 
had a newline at the end initially (just checked the main repo). I could easily 
add it but just wanted to clarify. 

And yeah we can merge this one for now and when I'm able to find more I'll make 
another PR for them.

https://github.com/llvm/llvm-project/pull/130868
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][diagnostics] Update note_constexpr_invalid_cast to use enum_select and adjust its uses (PR #130868)

[Ayokunle Amodu via cfe-commits]

https://github.com/ayokunle321 edited 
https://github.com/llvm/llvm-project/pull/130868
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang][diagnostics] Update note_constexpr_invalid_cast to use enum_select and adjust its uses (PR #130868)

[Ayokunle Amodu via cfe-commits]

ayokunle321 wrote:

@erichkeane CI is happy now! For future patches, I was wondering if to separate 
them like this so reviews could be easier or I should just do it all in one PR.

https://github.com/llvm/llvm-project/pull/130868
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits