arphaman added a comment.
Thanks for working on this! I have a couple of comments:
================
Comment at: include/clang/Basic/DiagnosticSemaKinds.td:1762
def err_init_incomplete_type : Error<"initialization of incomplete type %0">;
+def err_list_init_in_parens : Error<"list-initializer for non-class type "
+ "must not be parenthesized">;
----------------
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?
================
Comment at: include/clang/Sema/Sema.h:1802
void ActOnInitializerError(Decl *Dcl);
+ bool cannotBeInitializededByParenthzedList(QualType TargetType);
void ActOnPureSpecifier(Decl *D, SourceLocation PureSpecLoc);
----------------
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.
================
Comment at: include/clang/Sema/Sema.h:1803
+ bool cannotBeInitializededByParenthzedList(QualType TargetType);
void ActOnPureSpecifier(Decl *D, SourceLocation PureSpecLoc);
void ActOnCXXForRangeDecl(Decl *D);
----------------
Please add a blank line here to separate unrelated `Act...` methods from the
new method.
================
Comment at: lib/Sema/SemaDecl.cpp:9796
+ Diag(VDecl->getLocation(), diag::err_list_init_in_parens)
+ << CXXDirectInit->getSourceRange()
+ << FixItHint::CreateRemoval(CXXDirectInit->getLocStart())
----------------
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).
================
Comment at: lib/Sema/SemaDecl.cpp:10110
+bool Sema::cannotBeInitializededByParenthzedList(QualType TargetType) {
+ return !TargetType->isDependentType() && !TargetType->isRecordType() &&
+ !TargetType->getContainedAutoType();
----------------
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.
================
Comment at: lib/Sema/SemaExprCXX.cpp:1229
+ Diag(TInfo->getTypeLoc().getLocStart(), diag::err_list_init_in_parens)
+ << IList->getSourceRange()
+ << FixItHint::CreateRemoval(LParenLoc)
----------------
Formatting issue: clang-format replaces
```
<< IList->getSourceRange()
<< FixItHint::CreateRemoval(LParenLoc)
```
with
```
<< IList->getSourceRange() << FixItHint::CreateRemoval(LParenLoc)
```
================
Comment at: test/SemaCXX/cxx0x-initializer-scalars.cpp:96
+ (void) int({0}); // expected-error {{list-initializer for non-class type
must not be parenthesized}}
+ new int({0}); // expected-error {{list-initializer for non-class type
must not be parenthesized}}
}
----------------
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.
https://reviews.llvm.org/D25816
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
