[clang] Avoid warnings about enum mismatch in ternary expressions. NFC.

[mstorsjo]

This avoids the following kind of warning when built with GCC:

../../clang/lib/Sema/SemaStmtAttr.cpp: In function ‘clang::Attr* ProcessStmtAttribute(clang::Sema&, clang::Stmt*, const clang::ParsedAttr&, clang::SourceRange)’:
../../clang/lib/Sema/SemaStmtAttr.cpp:677:30: warning: enumerated mismatch in conditional expression: ‘clang::diag::<unnamed enum>’ vs ‘clang::diag::<unnamed enum>’ [-Wenum-compare]
  676 |       S.Diag(A.getLoc(), A.isRegularKeywordAttribute()
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  677 |                              ? diag::err_keyword_not_supported_on_targe
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  678 |                              : diag::warn_unhandled_ms_attribute_ignore )
      |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

These enums are non-overlapping, but due they are defined in different enum scopes due to how they are generated with tablegen.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Martin Storsjö (mstorsjo)

Changes

This avoids the following kind of warning when built with GCC:

../../clang/lib/Sema/SemaStmtAttr.cpp: In function ‘clang::Attr* ProcessStmtAttribute(clang::Sema&, clang::Stmt*, const clang::ParsedAttr&, clang::SourceRange)’:
../../clang/lib/Sema/SemaStmtAttr.cpp:677:30: warning: enumerated mismatch in conditional expression: ‘clang::diag::<unnamed enum>’ vs ‘clang::diag::<unnamed enum>’ [-Wenum-compare]
  676 |       S.Diag(A.getLoc(), A.isRegularKeywordAttribute()
      |                          ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  677 |                              ? diag::err_keyword_not_supported_on_targe
      |                              ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
  678 |                              : diag::warn_unhandled_ms_attribute_ignore )
      |                              ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

These enums are non-overlapping, but due they are defined in different enum scopes due to how they are generated with tablegen.

---

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

2 Files Affected:

• (modified) clang/lib/Sema/SemaDeclAttr.cpp (+4-2)
• (modified) clang/lib/Sema/SemaStmtAttr.cpp (+4-2)

diff --git a/clang/lib/Sema/SemaDeclAttr.cpp b/clang/lib/Sema/SemaDeclAttr.cpp
index 44906456f3371..7b3dc80e38913 100644
--- a/clang/lib/Sema/SemaDeclAttr.cpp
+++ b/clang/lib/Sema/SemaDeclAttr.cpp
@@ -6837,8 +6837,10 @@ ProcessDeclAttribute(Sema &S, Scope *scope, Decl *D, const ParsedAttr &AL,
       !AL.existsInTarget(S.Context.getTargetInfo())) {
     if (AL.isRegularKeywordAttribute() || AL.isDeclspecAttribute()) {
       S.Diag(AL.getLoc(), AL.isRegularKeywordAttribute()
-                              ? diag::err_keyword_not_supported_on_target
-                              : diag::warn_unhandled_ms_attribute_ignored)
+                              ? static_cast<unsigned>(
+                                    diag::err_keyword_not_supported_on_target)
+                              : static_cast<unsigned>(
+                                    diag::warn_unhandled_ms_attribute_ignored))
           << AL.getAttrName() << AL.getRange();
     } else {
       S.DiagnoseUnknownAttribute(AL);
diff --git a/clang/lib/Sema/SemaStmtAttr.cpp b/clang/lib/Sema/SemaStmtAttr.cpp
index 77aa7164d4555..8cac790e969fa 100644
--- a/clang/lib/Sema/SemaStmtAttr.cpp
+++ b/clang/lib/Sema/SemaStmtAttr.cpp
@@ -674,8 +674,10 @@ static Attr *ProcessStmtAttribute(Sema &S, Stmt *St, const ParsedAttr &A,
          A.existsInTarget(*Aux)))) {
     if (A.isRegularKeywordAttribute() || A.isDeclspecAttribute()) {
       S.Diag(A.getLoc(), A.isRegularKeywordAttribute()
-                             ? diag::err_keyword_not_supported_on_target
-                             : diag::warn_unhandled_ms_attribute_ignored)
+                             ? static_cast<unsigned>(
+                                   diag::err_keyword_not_supported_on_target)
+                             : static_cast<unsigned>(
+                                   diag::warn_unhandled_ms_attribute_ignored))
           << A << A.getRange();
     } else {
       S.DiagnoseUnknownAttribute(A);

[mstorsjo]

This is also not very pretty, but again, this is a mostly reasonable warning - unfortunately our diagnostics aren't all in one large enum for (presumably) technical reasons, even if they are forced to be non-overlapping.

[cor3ntin]

I think it might be cleaner in that case to have two different if statements (one for isRegularKeywordAttribute and one for isDeclspecAttribute)

[brunodf-snps]

@mstorsjo It seems you are (by coincidence) looking into warnings of a gcc build at the same time as I was. There was a third PR about a gcc warning: #158635

[erichkeane]

I think I agree with corentin, just breaking this up into 2 branches (an if (A.isRegularKeywordAttribute()) and else if (A.isDeclspecAttribute()) is probably better. As it is, we're basically checking the regular keyword 2x.

Else, this patch is fine for me.

[mstorsjo]

I think it might be cleaner in that case to have two different if statements (one for isRegularKeywordAttribute and one for isDeclspecAttribute)

Sounds reasonable, thanks - I updated the PR with such a change.