[clang][Sema] Simplify err_init_conversion_failed diagnostic message for const variables

[agentcooper]

Fixes #73399.

I initially tried to create a new EntityKind enum case to represent constants as it makes it easier to use Entity.getKind() with the diagnostic message. But in the end I've decided against it, as the new case needs to be handled in many places and feels error-prone.

[llvmbot]

@llvm/pr-subscribers-clang

Author: Artem Tyurin (agentcooper)

Changes

Fixes #73399.

I initially tried to create a new EntityKind enum case to represent constants as it makes it easier to use Entity.getKind() with the diagnostic message. But in the end I've decided against it, as the new case needs to be handled in many places and feels error-prone.

---

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

4 Files Affected:

• (modified) clang/include/clang/Basic/DiagnosticSemaKinds.td (+1-1)
• (modified) clang/lib/Sema/SemaInit.cpp (+16-6)
• (modified) clang/test/SemaCXX/constant-expression-cxx11.cpp (+1-1)
• (modified) clang/test/SemaCXX/err_init_conversion_failed.cpp (+11)

diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index b4dc4feee8e63a..1c9a69167c28d1 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -2204,7 +2204,7 @@ def err_destructor_template : Error<
 
 // C++ initialization
 def err_init_conversion_failed : Error<
-  "cannot initialize %select{a variable|a parameter|template parameter|"
+  "cannot initialize %select{a constant|a variable|a parameter|template parameter|"
   "return object|statement expression result|an "
   "exception object|a member subobject|an array element|a new value|a value|a "
   "base class|a constructor delegation|a vector element|a block element|a "
diff --git a/clang/lib/Sema/SemaInit.cpp b/clang/lib/Sema/SemaInit.cpp
index b6de06464cd6f3..c1846d9a80c8f2 100644
--- a/clang/lib/Sema/SemaInit.cpp
+++ b/clang/lib/Sema/SemaInit.cpp
@@ -9804,12 +9804,22 @@ bool InitializationSequence::Diagnose(Sema &S,
 
   case FK_ConversionFailed: {
     QualType FromType = OnlyArg->getType();
-    PartialDiagnostic PDiag = S.PDiag(diag::err_init_conversion_failed)
-      << (int)Entity.getKind()
-      << DestType
-      << OnlyArg->isLValue()
-      << FromType
-      << Args[0]->getSourceRange();
+
+    // NOTE: need to be in sync with err_init_conversion_failed
+    const auto TotalSpecialKinds = 1;
+
+    PartialDiagnostic PDiag = S.PDiag(diag::err_init_conversion_failed);
+    if (Entity.getKind() == InitializedEntity::EK_Variable &&
+        DestType.isConstQualified()) {
+      QualType NonConstDestType = DestType;
+      NonConstDestType.removeLocalConst();
+      PDiag << 0 /* a constant */
+            << NonConstDestType;
+    } else {
+      PDiag << (TotalSpecialKinds + (int)Entity.getKind()) << DestType;
+    }
+    PDiag << OnlyArg->isLValue() << FromType << Args[0]->getSourceRange();
+
     S.HandleFunctionTypeMismatch(PDiag, FromType, DestType);
     S.Diag(Kind.getLocation(), PDiag);
     emitBadConversionNotes(S, Entity, Args[0]);
diff --git a/clang/test/SemaCXX/constant-expression-cxx11.cpp b/clang/test/SemaCXX/constant-expression-cxx11.cpp
index 9e2ae07cbe4c9c..19ab0cc27cc015 100644
--- a/clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ b/clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -901,7 +901,7 @@ static_assert((Derived*)(Base*)pb1 == (Derived*)pok2, "");
 
 // Core issue 903: we do not perform constant evaluation when checking for a
 // null pointer in C++11. Just check for an integer literal with value 0.
-constexpr Base *nullB = 42 - 6 * 7; // expected-error {{cannot initialize a variable of type 'Base *const' with an rvalue of type 'int'}}
+constexpr Base *nullB = 42 - 6 * 7; // expected-error {{cannot initialize a constant of type 'Base *' with an rvalue of type 'int'}}
 constexpr Base *nullB1 = 0;
 static_assert((Bottom*)nullB == 0, "");
 static_assert((Derived*)nullB1 == 0, "");
diff --git a/clang/test/SemaCXX/err_init_conversion_failed.cpp b/clang/test/SemaCXX/err_init_conversion_failed.cpp
index e31f215b4528cb..359d840070f2c4 100644
--- a/clang/test/SemaCXX/err_init_conversion_failed.cpp
+++ b/clang/test/SemaCXX/err_init_conversion_failed.cpp
@@ -59,3 +59,14 @@ void test_15() {
   // expected-error-re@-1{{cannot initialize a member subobject of type 'void (template_test::S::*)(const int &){{( __attribute__\(\(thiscall\)\))?}}' with an rvalue of type 'void (template_test::S::*)(int){{( __attribute__\(\(thiscall\)\))?}}': type mismatch at 1st parameter ('const int &' vs 'int')}}
 }
 }
+
+void test_16() {
+  const int a = (void)0;
+  // expected-error@-1{{cannot initialize a constant of type 'int'}}
+
+  int* const c = (void)0;
+  // expected-error@-1{{cannot initialize a constant of type 'int *'}}
+
+  const int* b = (void)0;
+  // expected-error@-1{{cannot initialize a variable of type 'const int *'}}
+}

[agentcooper]

@cjdb Hey Christopher, I'm curious to hear your thoughts on this change.

[AaronBallman]

Thank you for working on this! I had an idea that might be an improved approach, but the general idea of the patch is great. I think this can likely land without a change to clang/docs/ReleaseNotes.rst because it's a minor wording improvement, but you can feel free to add a release note if you'd like.

[AaronBallman]

How about something along these lines (with comments)?

PartialDiagnostic PDiag = S.PDiag(diag::err_init_conversion_failed);
{
  llvm::SaveAndRestore _(DestType);
  unsigned EKind = (unsigned)Entity.getKind() + 1;
  if (Entity.getKind() == InitializedEntity::EK_Variable &&
        DestType.isConstQualified()) {
    EKind = 0;
    DestType.removeLocalConst();
  }

  PDiag << EKind << DestType << ...;
}
S.HandleFunctionTypeMismatch(PDiag, ...);

[halbi2]

Hello, I made patch #128614 that fixes a similar diagnostic with -Wsign-compare. But my patch is much simpler. I simply remove the cv qualifiers before printing. Why would such an approach not be appropriate in this case as well?