Author: Erich Keane Date: 2022-11-01T11:39:46-07:00 New Revision: ab4c22e2b735c839c667e08be303f7f8cef3ccf6
URL: https://github.com/llvm/llvm-project/commit/ab4c22e2b735c839c667e08be303f7f8cef3ccf6 DIFF: https://github.com/llvm/llvm-project/commit/ab4c22e2b735c839c667e08be303f7f8cef3ccf6.diff LOG: [Concepts] Improve diagnostics on a missing 'auto' keyword. As reported in https://github.com/llvm/llvm-project/issues/49192, we did a pretty poor job diagnosing cases where someone forgot 'auto', a nd is probably in the middle of a variable declaration. This patch makes us properly diagnose in cases where the next token is a reference, or CVR qualifier. Added: Modified: clang/docs/ReleaseNotes.rst clang/lib/Parse/ParseDecl.cpp clang/lib/Parse/ParseTentative.cpp clang/test/Parser/cxx2a-placeholder-type-constraint.cpp clang/test/SemaTemplate/concepts.cpp Removed: ################################################################################ diff --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst index aac52bfc6e8fa..488d4d6efb5d0 100644 --- a/clang/docs/ReleaseNotes.rst +++ b/clang/docs/ReleaseNotes.rst @@ -352,6 +352,8 @@ Improvements to Clang's diagnostics Fixes `Issue 57562 <https://github.com/llvm/llvm-project/issues/57562>`_. - Better error recovery for pack expansion of expressions. `Issue 58673 <https://github.com/llvm/llvm-project/issues/58673>`_. +- Better diagnostics when the user has missed `auto` in a declaration. + `Issue 49129 <https://github.com/llvm/llvm-project/issues/49129>`_. Non-comprehensive list of changes in this release ------------------------------------------------- diff --git a/clang/lib/Parse/ParseDecl.cpp b/clang/lib/Parse/ParseDecl.cpp index fd3589846a3d3..d7942fa8d82fa 100644 --- a/clang/lib/Parse/ParseDecl.cpp +++ b/clang/lib/Parse/ParseDecl.cpp @@ -3706,11 +3706,18 @@ void Parser::ParseDeclarationSpecifiers( if (TemplateId->Kind == TNK_Concept_template) { // If we've already diagnosed that this type-constraint has invalid - // arguemnts, drop it and just form 'auto' or 'decltype(auto)'. + // arguments, drop it and just form 'auto' or 'decltype(auto)'. if (TemplateId->hasInvalidArgs()) TemplateId = nullptr; - if (NextToken().is(tok::identifier)) { + // Any of the following tokens are likely the start of the user + // forgetting 'auto' or 'decltype(auto)', so diagnose. + // Note: if updating this list, please make sure we update + // isCXXDeclarationSpecifier's check for IsPlaceholderSpecifier to have + // a matching list. + if (NextToken().isOneOf(tok::identifier, tok::kw_const, + tok::kw_volatile, tok::kw_restrict, tok::amp, + tok::ampamp)) { Diag(Loc, diag::err_placeholder_expected_auto_or_decltype_auto) << FixItHint::CreateInsertion(NextToken().getLocation(), "auto"); // Attempt to continue as if 'auto' was placed here. diff --git a/clang/lib/Parse/ParseTentative.cpp b/clang/lib/Parse/ParseTentative.cpp index ca1602e638434..840b0cb3169da 100644 --- a/clang/lib/Parse/ParseTentative.cpp +++ b/clang/lib/Parse/ParseTentative.cpp @@ -1253,17 +1253,34 @@ Parser::TPResult Parser::isCXXDeclarationSpecifier(ImplicitTypenameContext AllowImplicitTypename, Parser::TPResult BracedCastResult, bool *InvalidAsDeclSpec) { - auto IsPlaceholderSpecifier = [&] (TemplateIdAnnotation *TemplateId, - int Lookahead) { + auto IsPlaceholderSpecifier = [&](TemplateIdAnnotation *TemplateId, + int Lookahead) { // We have a placeholder-constraint (we check for 'auto' or 'decltype' to // distinguish 'C<int>;' from 'C<int> auto c = 1;') return TemplateId->Kind == TNK_Concept_template && - GetLookAheadToken(Lookahead + 1).isOneOf(tok::kw_auto, tok::kw_decltype, - // If we have an identifier here, the user probably forgot the - // 'auto' in the placeholder constraint, e.g. 'C<int> x = 2;' - // This will be diagnosed nicely later, so disambiguate as a - // declaration. - tok::identifier); + (GetLookAheadToken(Lookahead + 1) + .isOneOf(tok::kw_auto, tok::kw_decltype, + // If we have an identifier here, the user probably + // forgot the 'auto' in the placeholder constraint, + // e.g. 'C<int> x = 2;' This will be diagnosed nicely + // later, so disambiguate as a declaration. + tok::identifier, + // CVR qualifierslikely the same situation for the + // user, so let this be diagnosed nicely later. We + // cannot handle references here, as `C<int> & Other` + // and `C<int> && Other` are both legal. + tok::kw_const, tok::kw_volatile, tok::kw_restrict) || + // While `C<int> && Other` is legal, doing so while not specifying a + // template argument is NOT, so see if we can fix up in that case at + // minimum. Concepts require at least 1 template parameter, so we + // can count on the argument count. + // FIXME: In the future, we migth be able to have SEMA look up the + // declaration for this concept, and see how many template + // parameters it has. If the concept isn't fully specified, it is + // possibly a situation where we want deduction, such as: + // `BinaryConcept<int> auto f = bar();` + (TemplateId->NumArgs == 0 && + GetLookAheadToken(Lookahead + 1).isOneOf(tok::amp, tok::ampamp))); }; switch (Tok.getKind()) { case tok::identifier: { diff --git a/clang/test/Parser/cxx2a-placeholder-type-constraint.cpp b/clang/test/Parser/cxx2a-placeholder-type-constraint.cpp index 64b09e3720599..ba1b1ea62c300 100644 --- a/clang/test/Parser/cxx2a-placeholder-type-constraint.cpp +++ b/clang/test/Parser/cxx2a-placeholder-type-constraint.cpp @@ -9,6 +9,7 @@ namespace ns { } int foo() { + int I; {ns::D auto a = 1;} {C auto a = 1;} {C<> auto a = 1;} @@ -28,8 +29,21 @@ int foo() { // expected-error@-1{{cannot form reference to 'decltype(auto)'}} {C a = 1;} // expected-error@-1{{expected 'auto' or 'decltype(auto)' after concept name}} + {C const a2 = 1;} + // expected-error@-1{{expected 'auto' or 'decltype(auto)' after concept name}} + {C &a3 = I;} + // expected-error@-1{{expected 'auto' or 'decltype(auto)' after concept name}} + {C &&a4 = 1;} + // expected-error@-1{{expected 'auto' or 'decltype(auto)' after concept name}} {C decltype a19 = 1;} // expected-error@-1{{expected '('}} {C decltype(1) a20 = 1;} // expected-error@-1{{expected 'auto' or 'decltype(auto)' after concept name}} } + +void foo1(C auto &a){} +void foo2(C const &a){} +// expected-error@-1{{expected 'auto' or 'decltype(auto)' after concept name}} +void foo3(C auto const &a){} +void foo4(const C &a){} +// expected-error@-1{{expected 'auto' or 'decltype(auto)' after concept name}} diff --git a/clang/test/SemaTemplate/concepts.cpp b/clang/test/SemaTemplate/concepts.cpp index 64e21c4a49627..8544635696331 100644 --- a/clang/test/SemaTemplate/concepts.cpp +++ b/clang/test/SemaTemplate/concepts.cpp @@ -4,7 +4,9 @@ namespace PR47043 { template<typename T> concept True = true; template<typename ...T> concept AllTrue1 = True<T>; // expected-error {{expression contains unexpanded parameter pack 'T'}} template<typename ...T> concept AllTrue2 = (True<T> && ...); + template<typename ...T> concept AllTrue3 = (bool)(True<T> & ...); static_assert(AllTrue2<int, float, char>); + static_assert(AllTrue3<int, float, char>); } namespace PR47025 { _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits