[clang] Try to improve diagnostics about uninitialized constexpr variables

[tbaeder]

Summary

Consider:

constexpr int a;

the error message for this is currently:

error: default initialization of an object of const type 'const int'
constexpr int a;
              ^
                = 0

which makes very little sense to me. With this patch, the output is instead:

error: constexpr variable 'a' must be initialized by a constant expression
constexpr int a;
              ^
                = 0

which is much better. Tells the user exactly what is missing.

For

constexpr int a[];

before this patch, the output is:

error: definition of variable with array type needs an explicit size or an initializer
constexpr int a[];
              ^

The error message is not even true in this case, since _only_ passing a size won't work. It must be initialized by a constant expression, so now the error message is:

error: constexpr variable 'a' must be initialized by a constant expression
constexpr int a[];
              ^

and a fixit hint if a size was provided:

error: constexpr variable 'a' must be initialized by a constant expression
constexpr int a[2];
              ^
                   = {}

(not sure about that part)

[aaron.ballman]

Thanks for working on this, I like the direction it's heading! One question I'm kicking around my head is, should we do something similar for const variables?

const int i; // error: default initialization of an object of const type 'const int'

it sure seems like this could be similarly improved to say something along the lines of error: variable 'i' of const type must be initialized

Also, don't forget to add a release note.

clang/lib/Sema/SemaFixItUtils.cpp

208–210

I don't think this is a good change, consider:

int array[] = {};

zero-sized arrays are an extension in both C and C++, and the empty initializer is a GNU extension in C (at least until C2x).

clang/lib/Sema/SemaInit.cpp

8063

Why the check for a record type?

[tbaeder]

clang/lib/Sema/SemaInit.cpp

8063

For record types, the current error message would be:

default initialization of an object of const type 'const S3' without a user-provided default constructor

which gives more information than just "must be initialized by a constant expression".

[tbaeder]

clang/lib/Sema/SemaFixItUtils.cpp

208–210

The code in SemaInit.cpp is weird in that it only holds if the fixit isn't empty... so if that change does away, a constexpr int arr[2]; also doesn't use the new error message :(

[aaron.ballman]

clang/lib/Sema/SemaFixItUtils.cpp

208–210

Ugh, well that's rather unfortunate. But I'm not certain the improved diagnostic is worth the incorrect fix-it unless the array has specified bounds. Perhaps another way to address this is to work on the diagnostic behavior to be more like:

error: definition of constexpr variable with array type needs an explicit initializer
constexpr int a[];
              ^

Does that seem more plausible?

clang/lib/Sema/SemaInit.cpp

8063

Yeah, that is a better diagnostic, thanks! Too bad the diagnostic logic is separated like this though.

[tbaeder]

The new version also gets the new error message for constexpr int foo[2];.

[aaron.ballman]

LGTM, though you should add a release note.

clang/lib/Sema/SemaInit.cpp

8057

8063–8078

9477

Oh, neato, I see dyn_cast_or_null was deprecated...

clang/lib/Sema/SemaDecl.cpp

case VarDecl::TentativeDefinition:
       if (!Var->isInvalidDecl())
         TentativeDefinitions.push_back(Var);
       return;
     }
 
     // Provide a specific diagnostic for uninitialized variable
     // definitions with incomplete array type.
     if (Type->isIncompleteArrayType()) {
+      if (Var->isConstexpr())
+        Diag(Var->getLocation(), diag::err_constexpr_var_requires_const_init)
+            << Var;
+      else
         Diag(Var->getLocation(),
              diag::err_typecheck_incomplete_array_needs_initializer);
       Var->setInvalidDecl();
       return;
     }
 
     // Provide a specific diagnostic for uninitialized variable
     // definitions with reference type.
     if (Type->isReferenceType()) {
       Diag(Var->getLocation(), diag::err_reference_var_requires_init)

clang/lib/Sema/SemaFixItUtils.cpp

 Sema::getFixItZeroInitializerForType(QualType T, SourceLocation Loc) const {
   if (T->isScalarType()) {
     std::string s = getScalarZeroExpressionForType(*T, Loc, *this);
     if (!s.empty())
       s = " = " + s;
     return s;
   }
 
+  if (T->isArrayType()) {
+    return " = {}";
+  }
+
   const CXXRecordDecl *RD = T->getAsCXXRecordDecl();
   if (!RD || !RD->hasDefinition())
     return std::string();
   if (LangOpts.CPlusPlus11 && !RD->hasUserProvidedDefaultConstructor())
     return "{}";
   if (RD->isAggregate())
     return " = {}";
   return std::string();
 }
 
 std::string
 Sema::getFixItZeroLiteralForType(QualType T, SourceLocation Loc) const {
   return getScalarZeroExpressionForType(*T, Loc, *this);
 }

clang/lib/Sema/SemaInit.cpp

ExprResult InitializationSequence::Perform(Sema &S,
                                            const InitializationKind &Kind,
                                            MultiExprArg Args,
                                            QualType *ResultType) {
   if (Failed()) {
     Diagnose(S, Entity, Kind, Args);
     return ExprError();
   }
   if (!ZeroInitializationFixit.empty()) {
-    unsigned DiagID = diag::err_default_init_const;
-    if (Decl *D = Entity.getDecl())
-      if (S.getLangOpts().MSVCCompat && D->hasAttr<SelectAnyAttr>())
-        DiagID = diag::ext_default_init_const;
+    const Decl *D = Entity.getDecl();
+    const VarDecl *VD = dyn_cast_or_null<VarDecl>(D);
+    QualType DestType = Entity.getType();
 
     // The initialization would have succeeded with this fixit. Since the fixit
     // is on the error, we need to build a valid AST in this case, so this isn't
     // handled in the Failed() branch above.
-    QualType DestType = Entity.getType();
+    if (!DestType->getAs<RecordType>() && VD && VD->isConstexpr()) {
+      // Use a more useful diagnostic for constexpr variables.
+      S.Diag(Kind.getLocation(), diag::err_constexpr_var_requires_const_init)
+          << VD
+          << FixItHint::CreateInsertion(ZeroInitializationFixitLoc,
+                                        ZeroInitializationFixit);
+    } else {
+      unsigned DiagID = diag::err_default_init_const;
+      if (S.getLangOpts().MSVCCompat && D && D->hasAttr<SelectAnyAttr>())
+        DiagID = diag::ext_default_init_const;
+
       S.Diag(Kind.getLocation(), DiagID)
           << DestType << (bool)DestType->getAs<RecordType>()
           << FixItHint::CreateInsertion(ZeroInitializationFixitLoc,
                                         ZeroInitializationFixit);
     }
+  }
 
   if (getKind() == DependentSequence) {
     // If the declaration is a non-dependent, incomplete array type
     // that has an initializer, then its type will be completed once
     // the initializer is instantiated.
     if (ResultType && !Entity.getType()->isDependentType() &&
         Args.size() == 1) {
       QualType DeclType = Entity.getType();
if (Entity.getKind() == InitializedEntity::EK_Member &&
       S.Diag(Kind.getLocation(), diag::err_uninitialized_member_in_ctor)
         << (Constructor->getInheritedConstructor() ? 2 :
             Constructor->isImplicit() ? 1 : 0)
         << S.Context.getTypeDeclType(Constructor->getParent())
         << /*const=*/1
         << Entity.getName();
       S.Diag(Entity.getDecl()->getLocation(), diag::note_previous_decl)
         << Entity.getName();
     } else {
       S.Diag(Kind.getLocation(), diag::err_default_init_const)
           << DestType << (bool)DestType->getAs<RecordType>();
     }
     break;
 
   case FK_Incomplete:
     S.RequireCompleteType(Kind.getLocation(), FailedIncompleteType,
                           diag::err_init_incomplete_type);

clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p1.cpp

 void f2(constexpr int i) {} // expected-error {{function parameter cannot be constexpr}}
 // non-static member
 struct s2 {
   constexpr int mi1; // expected-error {{non-static data member cannot be constexpr; did you intend to make it const?}}
   static constexpr int mi2;
 #if __cplusplus <= 201402L && !defined(MS_ABI)
   // expected-error@-2 {{requires an initializer}}
 #else
-  // expected-error@-4 {{default initialization of an object of const}}
+  // expected-error@-4 {{constexpr variable 'mi2' must be initialized by a constant expression}}
 #endif
   mutable constexpr int mi3 = 3; // expected-error-re {{non-static data member cannot be constexpr{{$}}}} expected-error {{'mutable' and 'const' cannot be mixed}}
 };
 // typedef
 typedef constexpr int CI; // expected-error {{typedef cannot be constexpr}}
 // tag
 constexpr class C1 {}; // expected-error {{class cannot be marked constexpr}}
 constexpr struct S1 {}; // expected-error {{struct cannot be marked constexpr}}

clang/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p9.cpp

 constexpr int &c = i;
 extern int &c;
 
 constexpr int (*d)(int) = 0;
 extern int (*const d)(int);
 
 // A variable declaration which uses the constexpr specifier shall have an
 // initializer and shall be initialized by a constant expression.
-constexpr int ni1; // expected-error {{default initialization of an object of const type 'const int'}}
+constexpr int ni1; // expected-error {{constexpr variable 'ni1' must be initialized by a constant expression}}
 constexpr struct C { C(); } ni2; // expected-error {{cannot have non-literal type 'const struct C'}} expected-note 3{{has no constexpr constructors}}
 constexpr double &ni3; // expected-error {{declaration of reference variable 'ni3' requires an initializer}}
 
 constexpr int nc1 = i; // expected-error {{constexpr variable 'nc1' must be initialized by a constant expression}} expected-note {{read of non-const variable 'i' is not allowed in a constant expression}}
 constexpr C nc2 = C(); // expected-error {{cannot have non-literal type 'const C'}}
 int &f(); // expected-note 2{{declared here}}
 constexpr int &nc3 = f(); // expected-error {{constexpr variable 'nc3' must be initialized by a constant expression}} expected-note {{non-constexpr function 'f' cannot be used in a constant expression}}
 constexpr int nc4(i); // expected-error {{constexpr variable 'nc4' must be initialized by a constant expression}} expected-note {{read of non-const variable 'i' is not allowed in a constant expression}}

clang/test/SemaCXX/constant-expression-cxx11.cpp

 }
 template<typename T, size_t N> constexpr T *begin(T (&xs)[N]) { return xs; }
 template<typename T, size_t N> constexpr T *end(T (&xs)[N]) { return xs + N; }
 
 struct MemberZero {
   constexpr int zero() const { return 0; }
 };
 
+constexpr int arr[]; // expected-error {{constexpr variable 'arr' must be initialized by a constant expression}}
+constexpr int arr2[2]; // expected-error {{constexpr variable 'arr2' must be initialized by a constant expression}}
+constexpr int arr3[2] = {};
+
 namespace DerivedToVBaseCast {
 
   struct U { int n; };
   struct V : U { int n; };
   struct A : virtual V { int n; };
   struct Aa { int n; };
   struct B : virtual A, Aa {};
   struct C : virtual A, Aa {};

 }
 
 namespace ExternConstexpr {
   extern constexpr int n = 0;
   extern constexpr int m; // expected-error {{constexpr variable declaration must be a definition}}
   void f() {
     extern constexpr int i; // expected-error {{constexpr variable declaration must be a definition}}
     constexpr int j = 0;
-    constexpr int k; // expected-error {{default initialization of an object of const type}}
+    constexpr int k; // expected-error {{constexpr variable 'k' must be initialized by a constant expression}}
   }
 
   extern const int q;
   constexpr int g() { return q; } // expected-note {{outside its lifetime}}
   constexpr int q = g(); // expected-error {{constant expression}} expected-note {{in call}}
 
   extern int r; // expected-note {{here}}
   constexpr int h() { return r; } // expected-error {{never produces a constant}} expected-note {{read of non-const}}