[clang] ab4c22e - [Concepts] Improve diagnostics on a missing 'auto' keyword.

Erich Keane via cfe-commits cfe-commits at lists.llvm.org
Tue Nov 1 11:39:53 PDT 2022


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 at -1{{cannot form reference to 'decltype(auto)'}}
   {C a = 1;}
   // expected-error at -1{{expected 'auto' or 'decltype(auto)' after concept name}}
+  {C const a2 = 1;}
+  // expected-error at -1{{expected 'auto' or 'decltype(auto)' after concept name}}
+  {C &a3 = I;}
+  // expected-error at -1{{expected 'auto' or 'decltype(auto)' after concept name}}
+  {C &&a4 = 1;}
+  // expected-error at -1{{expected 'auto' or 'decltype(auto)' after concept name}}
   {C decltype a19 = 1;}
   // expected-error at -1{{expected '('}}
   {C decltype(1) a20 = 1;}
   // expected-error at -1{{expected 'auto' or 'decltype(auto)' after concept name}}
 }
+
+void foo1(C auto &a){}
+void foo2(C const &a){}
+// expected-error at -1{{expected 'auto' or 'decltype(auto)' after concept name}}
+void foo3(C auto const &a){}
+void foo4(const C &a){}
+// expected-error at -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 {


        


More information about the cfe-commits mailing list