Use descriptive message if list initializer is incorrectly parenthesized.

[sepavloff]

Summary

If initializer contains parentheses around braced list where it is not allowed, as in
construct int({0}), clang issued message like `functional-style cast from 'void' to
'int' is not allowed`, which does not much help. Both gcc and msvc issue message
list-initializer for non-class type must not be parenthesized, which is more
descriptive. This change implements similar behavior for clang also.

[arphaman]

Thanks for working on this! I have a couple of comments:

include/clang/Basic/DiagnosticSemaKinds.td

1762 ↗

(On Diff #75270)

Wouldn't it be better if we had a diagnostic with the type information? So, instead of showing list-initializer for non-class type must not be parenthesized clang would show list-initializer for non-class type 'int' must not be parenthesized. What do you think?

As well as that, it seems that GCC issues a warning instead of an error for this diagnostic, so should this be a warning in clang as well?

include/clang/Sema/Sema.h

1802 ↗

(On Diff #75270)

I think you mean parenthesized instead of parenthzed here. As well as that, I think that it would be better to use a name without the not, something like canInitializeWithParenthesizedList.

Also, please add a comment that explains what this method does.

1803 ↗

(On Diff #75270)

Please add a blank line here to separate unrelated Act... methods from the new method.

lib/Sema/SemaDecl.cpp

9796 ↗

(On Diff #75270)

Slight formatting issue: please indent the three lines that start with '<<' using 4 extra spaces instead of 2 (just like you did in your changes for SemaExprCXX.cpp).

10110 ↗

(On Diff #75270)

If you change the name as I suggested to something like canInitializeWithParenthesizedList then the logic here would have to be inverted: TargetType()->isDependentType() || TargetType()->isRecordType() || TargetType()->getContainedAutoType(). Consequently, the call sites will have to be updated to include a ! before the call.

lib/Sema/SemaExprCXX.cpp

1229 ↗

(On Diff #75270)

Formatting issue: clang-format replaces

<< IList->getSourceRange()
<< FixItHint::CreateRemoval(LParenLoc)

with

<< IList->getSourceRange() << FixItHint::CreateRemoval(LParenLoc)

test/SemaCXX/cxx0x-initializer-scalars.cpp

96 ↗

(On Diff #75270)

Please add a test case for a pointer type as well, something like this should work:

int *x({0});

As well as that, please add a test for a dependent type.

[sepavloff]

Addressed reviewr's notes.

[sepavloff]

include/clang/Basic/DiagnosticSemaKinds.td

1762 ↗

(On Diff #75270)

Putting type name into the diagnostic message is a good idea, implemented.

As for making this message a warning instead of an error, I am in doubt.
Pros:

• it enhances GCC compatibility,
• wrong code may be easily fixed by compiler,

Cons:

• this is a violation of the standard,
• this extension does not add any benefits,
• wrong code can be easily fixed by user.

I chose to retain current clang behavior and reject questionable code. GCC patch that introduced this message explains using warning by some uncertainty, 5 years passed, I think the standard is stable in viewpoint on such usage.

lib/Sema/SemaExprCXX.cpp

1229 ↗

(On Diff #75270)

After insertion of type Ty this is no more the case.

[arphaman]

LGTM, I added Richard in case he has something to add.

I chose to retain current clang behavior and reject questionable code. GCC patch that introduced this message explains using warning by some uncertainty, 5 years passed, I think the standard is stable in viewpoint on such usage.

Thanks for the detailed explanation, I wasn't sure if GCC's was behaving according to the standard or not.

cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td

def note_in_reference_temporary_list_initializer : Note<
   "in initialization of temporary of type %0 created to "
   "list-initialize this reference">;
 def note_var_fixit_add_initialization : Note<
   "initialize the variable %0 to silence this warning">;
 def note_uninit_fixit_remove_cond : Note<
   "remove the %select{'%1' if its condition|condition if it}0 "
   "is always %select{false|true}2">;
 def err_init_incomplete_type : Error<"initialization of incomplete type %0">;
+def err_list_init_in_parens : Error<"list-initializer for non-class type %0 "
+  "must not be parenthesized">;
 
 def warn_unsequenced_mod_mod : Warning<
   "multiple unsequenced modifications to %0">, InGroup<Unsequenced>;
 def warn_unsequenced_mod_use : Warning<
   "unsequenced modification and access to %0">, InGroup<Unsequenced>;
 
 def err_temp_copy_no_viable : Error<
   "no viable constructor %select{copying variable|copying parameter|"

cfe/trunk/include/clang/Sema/Sema.h

public:
   void ActOnParamDefaultArgumentError(Decl *param, SourceLocation EqualLoc);
   bool SetParamDefaultArgument(ParmVarDecl *Param, Expr *DefaultArg,
                                SourceLocation EqualLoc);
 
   void AddInitializerToDecl(Decl *dcl, Expr *init, bool DirectInit,
                             bool TypeMayContainAuto);
   void ActOnUninitializedDecl(Decl *dcl, bool TypeMayContainAuto);
   void ActOnInitializerError(Decl *Dcl);
+  bool canInitializeWithParenthesizedList(QualType TargetType);
+
   void ActOnPureSpecifier(Decl *D, SourceLocation PureSpecLoc);
   void ActOnCXXForRangeDecl(Decl *D);
   StmtResult ActOnCXXForRangeIdentifier(Scope *S, SourceLocation IdentLoc,
                                         IdentifierInfo *Ident,
                                         ParsedAttributes &Attrs,
                                         SourceLocation AttrEnd);
   void SetDeclDeleted(Decl *dcl, SourceLocation DelLoc);
   void SetDeclDefaulted(Decl *dcl, SourceLocation DefaultLoc);

cfe/trunk/lib/Sema/SemaDecl.cpp

if (Result.isInvalid()) {
       return;
     }
     Init = Result.get();
   }
 
   // Perform the initialization.
   ParenListExpr *CXXDirectInit = dyn_cast<ParenListExpr>(Init);
   if (!VDecl->isInvalidDecl()) {
+    // Handle errors like: int a({0})
+    if (CXXDirectInit && CXXDirectInit->getNumExprs() == 1 &&
+        !canInitializeWithParenthesizedList(VDecl->getType()))
+      if (auto IList = dyn_cast<InitListExpr>(CXXDirectInit->getExpr(0))) {
+        Diag(VDecl->getLocation(), diag::err_list_init_in_parens)
+            << VDecl->getType() << CXXDirectInit->getSourceRange()
+            << FixItHint::CreateRemoval(CXXDirectInit->getLocStart())
+            << FixItHint::CreateRemoval(CXXDirectInit->getLocEnd());
+        Init = IList;
+        CXXDirectInit = nullptr;
+      }
+
     InitializedEntity Entity = InitializedEntity::InitializeVariable(VDecl);
     InitializationKind Kind =
         DirectInit
             ? CXXDirectInit
                   ? InitializationKind::CreateDirect(VDecl->getLocation(),
                                                      Init->getLocStart(),
                                                      Init->getLocEnd())
                   : InitializationKind::CreateDirectList(VDecl->getLocation())
if (RequireNonAbstractType(VD->getLocation(), Ty,
     VD->setInvalidDecl();
     return;
   }
 
   // Don't bother complaining about constructors or destructors,
   // though.
 }
 
+/// Checks if an object of the given type can be initialized with parenthesized
+/// init-list.
+///
+/// \param TargetType Type of object being initialized.
+///
+/// The function is used to detect wrong initializations, such as 'int({0})'.
+///
+bool Sema::canInitializeWithParenthesizedList(QualType TargetType) {
+  return TargetType->isDependentType() || TargetType->isRecordType() ||
+         TargetType->getContainedAutoType();
+}
+
 void Sema::ActOnUninitializedDecl(Decl *RealDecl,
                                   bool TypeMayContainAuto) {
   // If there is no declaration, there was an error parsing it. Just ignore it.
   if (!RealDecl)
     return;
 
   if (VarDecl *Var = dyn_cast<VarDecl>(RealDecl)) {
     QualType Type = Var->getType();

cfe/trunk/lib/Sema/SemaExprCXX.cpp

Sema::ActOnCXXTypeConstructExpr(ParsedType TypeRep,
   if (!TypeRep)
     return ExprError();
 
   TypeSourceInfo *TInfo;
   QualType Ty = GetTypeFromParser(TypeRep, &TInfo);
   if (!TInfo)
     TInfo = Context.getTrivialTypeSourceInfo(Ty, SourceLocation());
 
+  // Handle errors like: int({0})
+  if (exprs.size() == 1 && !canInitializeWithParenthesizedList(Ty) &&
+      LParenLoc.isValid() && RParenLoc.isValid())
+    if (auto IList = dyn_cast<InitListExpr>(exprs[0])) {
+      Diag(TInfo->getTypeLoc().getLocStart(), diag::err_list_init_in_parens)
+          << Ty << IList->getSourceRange()
+          << FixItHint::CreateRemoval(LParenLoc)
+          << FixItHint::CreateRemoval(RParenLoc);
+      LParenLoc = RParenLoc = SourceLocation();
+    }
+
   auto Result = BuildCXXTypeConstructExpr(TInfo, LParenLoc, exprs, RParenLoc);
   // Avoid creating a non-type-dependent expression that contains typos.
   // Non-type-dependent expressions are liable to be discarded without
   // checking for embedded typos.
   if (!Result.isInvalid() && Result.get()->isInstantiationDependent() &&
       !Result.get()->isTypeDependent())
     Result = CorrectDelayedTyposInExpr(Result.get());
   return Result;
Sema::ActOnCXXNew(SourceLocation StartLoc, bool UseGlobal,
   }
 
   TypeSourceInfo *TInfo = GetTypeForDeclarator(D, /*Scope=*/nullptr);
   QualType AllocType = TInfo->getType();
   if (D.isInvalidType())
     return ExprError();
 
   SourceRange DirectInitRange;
-  if (ParenListExpr *List = dyn_cast_or_null<ParenListExpr>(Initializer))
+  if (ParenListExpr *List = dyn_cast_or_null<ParenListExpr>(Initializer)) {
     DirectInitRange = List->getSourceRange();
+    // Handle errors like: new int a({0})
+    if (List->getNumExprs() == 1 &&
+        !canInitializeWithParenthesizedList(AllocType))
+      if (auto IList = dyn_cast<InitListExpr>(List->getExpr(0))) {
+        Diag(TInfo->getTypeLoc().getLocStart(), diag::err_list_init_in_parens)
+            << AllocType << List->getSourceRange()
+            << FixItHint::CreateRemoval(List->getLocStart())
+            << FixItHint::CreateRemoval(List->getLocEnd());
+        DirectInitRange = SourceRange();
+        Initializer = IList;
+      }
+  }
 
   return BuildCXXNew(SourceRange(StartLoc, D.getLocEnd()), UseGlobal,
                      PlacementLParen,
                      PlacementArgs,
                      PlacementRParen,
                      TypeIdParens,
                      AllocType,
                      TInfo,

cfe/trunk/test/SemaCXX/cxx0x-initializer-references.cpp

void overloading() {
 
     one h(const int&);
     two h(const A&);
 
     static_assert(sizeof(h({1, 2})) == sizeof(two), "bad overload resolution");
   }
 
   void edge_cases() {
-    // FIXME: very poor error message
-    int const &b({0}); // expected-error {{could not bind}}
+    int const &b({0}); // expected-error {{list-initializer for non-class type 'const int &' must not be parenthesized}}
+    const int (&arr)[3] ({1, 2, 3}); // expected-error {{list-initializer for non-class type 'const int (&)[3]' must not be parenthesized}}
   }
-
 }
 
 namespace PR12182 {
   void f(int const(&)[3]);
 
   void g() {
       f({1, 2});
   }

cfe/trunk/test/SemaCXX/cxx0x-initializer-scalars.cpp

void overloaded_call() {
 
     void emptylist(int);
     void emptylist(int, int, int);
     emptylist({});
     emptylist({}, {}, {});
   }
 
   void edge_cases() {
-    // FIXME: very poor error message
-    int a({0}); // expected-error {{cannot initialize}}
-    (void) int({0}); // expected-error {{functional-style cast}}
-    new int({0});  // expected-error {{cannot initialize}}
+    int a({0}); // expected-error {{list-initializer for non-class type 'int' must not be parenthesized}}
+    (void) int({0}); // expected-error {{list-initializer for non-class type 'int' must not be parenthesized}}
+    new int({0});  // expected-error {{list-initializer for non-class type 'int' must not be parenthesized}}
+
+    int *b({0});  // expected-error {{list-initializer for non-class type 'int *' must not be parenthesized}}
+    typedef int *intptr;
+    int *c = intptr({0});  // expected-error {{list-initializer for non-class type 'intptr' (aka 'int *') must not be parenthesized}}
+  }
+
+  template<typename T> void dependent_edge_cases() {
+    T a({0});
+    (void) T({0});
+    new T({0});
+
+    T *b({0});
+    typedef T *tptr;
+    T *c = tptr({0});
   }
 
   void default_argument(int i = {}) {
   }
   struct DefaultArgument {
     void default_argument(int i = {}) {
     }
   };