Rakete1111 added a comment.
Thanks for working on this! :)
================
Comment at: clang/lib/Parse/ParseDecl.cpp:3533
+ if (ExplicitExpr.isInvalid()) {
+ Diag(ParenLoc, diag::note_explicit_bool_breaking_change_cxx2a)
+ << FixItHint::CreateReplacement(
----------------
This is a useful diagnostic but you'll need to fix (a lot) of false positives:
```
template <typename T> struct Foo { explicit(T{} +) Foo(); };
```
gets me:
```
main.cpp:1:50: error: expected expression
template <typename T> struct Foo { explicit(T{} +) Foo(); };
^
main.cpp:1:44: note: this expression is parsed as explicit(bool) since C++2a
template <typename T> struct Foo { explicit(T{} +) Foo(); };
~~~~~~~~^
explicit(true)
```
Fixit hints should only be used when it is 100% the right fix that works,
always.
================
Comment at: clang/lib/Parse/ParseDecl.cpp:3534
+ Diag(ParenLoc, diag::note_explicit_bool_breaking_change_cxx2a)
+ << FixItHint::CreateReplacement(
+ SourceRange(ExplicitLoc, ExplicitLoc.getLocWithOffset(
----------------
FixIt Hints also need tests :)
================
Comment at: clang/lib/Sema/DeclSpec.cpp:952
+ assert((ExplicitState != ESF_unresolved || ExplicitExpr) &&
+ "ExplicitExpr can't be null if the state is ESF_resolved_false or "
+ "ESF_unresolved");
----------------
The comment seems to explain something else than the code.
================
Comment at: clang/lib/Sema/DeclSpec.cpp:956
// intended.
- if (FS_explicit_specified) {
+ // TODO : it is unclear how to handle multiple explicit(bool) specifiers.
+ if (hasExplicitSpecifier()) {
----------------
We accept `explicit explicit`, but we really shouldn't accept
`explicit(some-expr) explicit(some-other-expr)`. That would just be confusing.
`explicit explicit(some-expr)` is also borderline.
================
Comment at: clang/lib/Sema/DeclSpec.cpp:1316
FixItHint Hint = FixItHint::CreateRemoval(SCLoc);
S.Diag(SCLoc, diag::err_friend_decl_spec)
<< Keyword << Hint;
----------------
Please change this warning so that the whole *explicit-specifier* is underlined
(or even change the text for conditional explicit):
```
main.cpp:1:36: error: 'explicit' can only be applied to a constructor or
conversion function
template <typename T> struct Foo { explicit(false) void foo(); };
^~~~~~~~
1 error generated.
```
This will also fix the FixIt Hint.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:8641
// C++0x explicit conversion operators.
- if (DS.isExplicitSpecified())
+ if (DS.hasExplicitSpecifier())
Diag(DS.getExplicitSpecLoc(),
----------------
You should amend the diagnostic, because as of right now `explicit(true)` is
being diagnosed as C++11 feature.
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:10870
+ ExprResult Converted =
+ CheckBooleanCondition(Loc, ExplicitExpr, /*isConstexpr=*/true);
+ if (Converted.isInvalid())
----------------
This assumes that we are in a [constexpr] if, leading to errors like this:
```
int a;
template <typename T> struct Foo { explicit(a) Foo(); };
```
```
main.cpp:2:45: error: constexpr if condition is not a constant expression
template <typename T> struct Foo { explicit(a) Foo(); };
^
```
================
Comment at: clang/lib/Sema/SemaDeclCXX.cpp:10876-10881
+ return Converted;
+ }
+
+ llvm::APSInt Result;
+ Converted = VerifyIntegerConstantExpression(
+ Converted.get(), &Result, diag::err_noexcept_needs_constant_expression,
----------------
Wrong diagnostic.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:10359
+ default:
+ llvm_unreachable("invalide Decl");
+ }
----------------
typo.
================
Comment at: clang/lib/Sema/SemaOverload.cpp:10361
+ }
+ assert(ESI.getPointer() && "null expression should be handled before");
+ S.Diag(Cand->Function->getLocation(),
----------------
This fires for an instantiation dependent (but not value dependent) explicit
specifier that is invalid:
```
template <typename T> struct Foo { explicit((T{}, false)) Foo(int); };
int main() { Foo<void> f = 1; }
```
================
Comment at: clang/test/SemaCXX/cxx2a-compat.cpp:46
+#if __cplusplus <= 201703L
+// expected-warning@-3 {{this expression would be parsed as explicit(bool) in
C++2a}}
+#if defined(__cpp_conditional_explicit)
----------------
would -> will
================
Comment at: clang/test/SemaCXX/cxx2a-compat.cpp:51
+#else
+// expected-error@-8 {{does not refer to a value}}
+// expected-error@-9 {{expected member name or ';'}}
----------------
A fixit hint for this would be great. Also it would be nice if there was a
nicer error message.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60934/new/
https://reviews.llvm.org/D60934
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
