[Sema] `setInvalidDecl` for error deduction declaration

[HerrCai0907]

Summary

Fixed: https://github.com/llvm/llvm-project/issues/62408
setInvalidDecl for invalid CXXDeductionGuideDecl to
avoid crashes during semantic analysis.

[HerrCai0907]

add release note

[shafik]

Thank you for the fix.

I have some questions, why don't we always call CheckDeductionGuideDeclarator when calling CXXDeductionGuideDecl::Create, I felt like perhaps CheckDeductionGuideDeclarator should be rolled into CXXDeductionGuideDecl::Create but then I noticed they are not paired up in other locations.

CC @rsmith

[rsmith]

I wonder if there are any cases where the "deduction guide" is so broken that we shouldn't be creating a CXXDeductionGuideDecl at all. For example, if there's no trailing return type, then the declaration is syntactically invalid (and looks more like a constructor than a deduction guide), and it's not clear to me whether creating a CXXDeductionGuideDecl really makes sense.

In D149516#4308203, @shafik wrote:

I have some questions, why don't we always call CheckDeductionGuideDeclarator when calling CXXDeductionGuideDecl::Create, I felt like perhaps CheckDeductionGuideDeclarator should be rolled into CXXDeductionGuideDecl::Create but then I noticed they are not paired up in other locations.

CXXDeductionGuideDecl::Create is the low-level AST primitive for creating a CXXDeductionGuideDecl. The responsibility for validating the declaration and issuing diagnostics doesn't belong at that level, it belongs in Sema. The other callers are:

• When Sema creates an implicit deduction guide from a constructor. In this case Sema knows it's valid by construction.
• When Sema instantiates a deduction guide. In this case Sema knows it's valid because the template was.

clang/lib/Sema/SemaDecl.cpp

9201–9202

This is reversed from the normal Clang convention (where true indicates that an error was produced).

9204

Use auto here.

clang/lib/Sema/SemaDeclCXX.cpp

11140–11143

Can we just return immediately here? I don't see a reason to keep going -- this error may well indicate that the developer didn't actually mean to declare a deduction guide in the first place, so the diagnostic on a function body below is likely to not be useful.

11194–11195

Should we return that we had an error after this one too?

[HerrCai0907]

refactor

[HerrCai0907]

clang/lib/Sema/SemaDeclCXX.cpp

11194–11195

if D.isFunctionDefinition(), we can still create valid deduction guide. I think keeping it can avoid lots of meaningless error later.

[HerrCai0907]

ping @rsmith

[Fznamznon]

clang/lib/Sema/SemaDeclCXX.cpp

11140–11143

NIT: You can also just do return Diag(...) here and in other places. This is equal to return true.

[HerrCai0907]

return Diag

[HerrCai0907]

ping

[HerrCai0907]

ping

[HerrCai0907]

This comment was removed by HerrCai0907.

[HerrCai0907]

@rsmith could you review it again?

[aaron.ballman]

LGTM aside from a request for a comment.

clang/lib/Sema/SemaDeclCXX.cpp

11194–11195

I think it's worth leaving that as a comment in the code so folks know it's done on purpose and why.

[HerrCai0907]

update comment

[dyung]

Hi @HerrCai0907, this change is failing on the PS4 linux and PS5 Windows bot. Can you take a look? I'm guessing these failures are likely due to the PS4/PS5 target defaulting to a different C/C++ standard than the rest of clang.

https://lab.llvm.org/buildbot/#/builders/139/builds/41248
https://lab.llvm.org/buildbot/#/builders/216/builds/21637

[TWeaver]

The build bot failures have been going for a while now so I've had to revert this until the author can address them, my apologies.

[HerrCai0907]

[HerrCai0907]

add std option

[HerrCai0907]

format

clang/docs/ReleaseNotes.rst

- Fix crash when handling undefined template partial specialization
   (`#61356 <https://github.com/llvm/llvm-project/issues/61356>`_)
 - Fix a crash caused by incorrectly evaluating constraints on an inheriting
   constructor declaration.
   (`#62361 <https://github.com/llvm/llvm-project/issues/62361>`_)
   (`#62362 <https://github.com/llvm/llvm-project/issues/62362>`_)
 - Fix crash when attempting to perform parenthesized initialization of an
   aggregate with a base class with only non-public constructors.
   (`#62296 <https://github.com/llvm/llvm-project/issues/62296>`_)
+- Fix crash when handling initialization candidates for invalid deduction guide.
+  (`#62408 <https://github.com/llvm/llvm-project/issues/62408>`_)
 
 Bug Fixes to Compiler Builtins
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 
 Bug Fixes to Attribute Support
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
 - Fixed a bug where attribute annotations on type specifiers (enums, classes,
   structs, unions, and scoped enums) were not properly ignored, resulting in

clang/include/clang/Sema/Sema.h

QualType CheckConstructorDeclarator(Declarator &D, QualType R,
                                       StorageClass& SC);
   void CheckConstructor(CXXConstructorDecl *Constructor);
   QualType CheckDestructorDeclarator(Declarator &D, QualType R,
                                      StorageClass& SC);
   bool CheckDestructor(CXXDestructorDecl *Destructor);
   void CheckConversionDeclarator(Declarator &D, QualType &R,
                                  StorageClass& SC);
   Decl *ActOnConversionDeclarator(CXXConversionDecl *Conversion);
-  void CheckDeductionGuideDeclarator(Declarator &D, QualType &R,
+  bool CheckDeductionGuideDeclarator(Declarator &D, QualType &R,
                                      StorageClass &SC);
   void CheckDeductionGuideTemplate(FunctionTemplateDecl *TD);
 
   void CheckExplicitlyDefaultedFunction(Scope *S, FunctionDecl *MD);
 
   bool CheckExplicitlyDefaultedSpecialMember(CXXMethodDecl *MD,
                                              CXXSpecialMember CSM,
                                              SourceLocation DefaultLoc);

clang/lib/Sema/SemaDecl.cpp

return CXXConversionDecl::Create(
         TInfo, SemaRef.getCurFPFeatures().isFPConstrained(), isInline,
         ExplicitSpecifier, ConstexprKind, SourceLocation(),
         TrailingRequiresClause);
 
   } else if (Name.getNameKind() == DeclarationName::CXXDeductionGuideName) {
     if (TrailingRequiresClause)
       SemaRef.Diag(TrailingRequiresClause->getBeginLoc(),
                    diag::err_trailing_requires_clause_on_deduction_guide)
           << TrailingRequiresClause->getSourceRange();
-    SemaRef.CheckDeductionGuideDeclarator(D, R, SC);
-
+    if (SemaRef.CheckDeductionGuideDeclarator(D, R, SC))
+      return nullptr;
     return CXXDeductionGuideDecl::Create(SemaRef.Context, DC, D.getBeginLoc(),
                                          ExplicitSpecifier, NameInfo, R, TInfo,
                                          D.getEndLoc());
   } else if (DC->isRecord()) {
     // If the name of the function is the same as the name of the record,
     // then this must be an invalid constructor that has a return type.
     // (The parser checks for a return type and makes the declarator a
     // constructor if it has no return type).
     if (Name.getAsIdentifierInfo() &&

clang/lib/Sema/SemaDeclCXX.cpp

struct BadSpecifierDiagnoser {
   Sema::SemaDiagnosticBuilder Diagnostic;
   std::string Specifiers;
 };
 }
 
 /// Check the validity of a declarator that we parsed for a deduction-guide.
 /// These aren't actually declarators in the grammar, so we need to check that
 /// the user didn't specify any pieces that are not part of the deduction-guide
-/// grammar.
-void Sema::CheckDeductionGuideDeclarator(Declarator &D, QualType &R,
+/// grammar. Return true on error.
+bool Sema::CheckDeductionGuideDeclarator(Declarator &D, QualType &R,
                                          StorageClass &SC) {
   TemplateName GuidedTemplate = D.getName().TemplateName.get().get();
   TemplateDecl *GuidedTemplateDecl = GuidedTemplate.getAsTemplateDecl();
   assert(GuidedTemplateDecl && "missing template decl for deduction guide");
 
   // C++ [temp.deduct.guide]p3:
   //   A deduction-gide shall be declared in the same scope as the
   //   corresponding class template.
if (DS.hasTypeSpecifier() || DS.getTypeQualifiers() ||
     Diagnoser.check(DS.getTypeSpecComplexLoc(), DS.getTypeSpecComplex());
     Diagnoser.check(DS.getTypeSpecSignLoc(), DS.getTypeSpecSign());
     Diagnoser.check(DS.getTypeSpecWidthLoc(), DS.getTypeSpecWidth());
     Diagnoser.check(DS.getTypeSpecTypeLoc(), DS.getTypeSpecType());
     DS.ClearTypeSpecType();
   }
 
   if (D.isInvalidType())
-    return;
+    return true;
 
   // Check the declarator is simple enough.
   bool FoundFunction = false;
   for (const DeclaratorChunk &Chunk : llvm::reverse(D.type_objects())) {
     if (Chunk.Kind == DeclaratorChunk::Paren)
       continue;
     if (Chunk.Kind != DeclaratorChunk::Function || FoundFunction) {
       Diag(D.getDeclSpec().getBeginLoc(),
            diag::err_deduction_guide_with_complex_decl)
           << D.getSourceRange();
       break;
     }
     if (!Chunk.Fun.hasTrailingReturnType()) {
       Diag(D.getName().getBeginLoc(),
            diag::err_deduction_guide_no_trailing_return_type);
-      break;
+      return true;
     }
 
     // Check that the return type is written as a specialization of
     // the template specified as the deduction-guide's name.
     // The template name may not be qualified. [temp.deduct.guide]
     ParsedType TrailingReturnType = Chunk.Fun.getTrailingReturnType();
     TypeSourceInfo *TSI = nullptr;
     QualType RetTy = GetTypeFromParser(TrailingReturnType, &TSI);
for (const DeclaratorChunk &Chunk : llvm::reverse(D.type_objects())) {
     }
 
     if (!AcceptableReturnType) {
       Diag(TSI->getTypeLoc().getBeginLoc(),
            diag::err_deduction_guide_bad_trailing_return_type)
           << GuidedTemplate << TSI->getType()
           << MightInstantiateToSpecialization
           << TSI->getTypeLoc().getSourceRange();
+      return true;
     }
 
     // Keep going to check that we don't have any inner declarator pieces (we
     // could still have a function returning a pointer to a function).
     FoundFunction = true;
   }
 
   if (D.isFunctionDefinition())
     Diag(D.getIdentifierLoc(), diag::err_deduction_guide_defines_function);
+  return false;
 }
 
 //===----------------------------------------------------------------------===//
 // Namespace Handling
 //===----------------------------------------------------------------------===//
 
 /// Diagnose a mismatch in 'inline' qualifiers when a namespace is
 /// reopened.

clang/test/CXX/expr/expr.prim/expr.prim.req/nested-requirement.cpp

 // https://reviews.llvm.org/D138914
 template <class a, a> struct b;
 template <bool c> using d = b<bool, c>;
 template <class a, class e> using f = d<__is_same(a, e)>;
 template <class a, class e>
 concept g = f<a, e>::h;
 template <class a, class e>
 concept i = g<e, a>;
-template <typename> class j { // expected-note {{candidate template ignored}}
+template <typename> class j {
   template <typename k>
   requires requires { requires i<j, k>; }
-  j(); // expected-note {{candidate template ignored}}
+  j();
 };
-template <> j(); // expected-error {{deduction guide declaration without trailing return type}} // expected-error {{no function template}}
+template <> j(); // expected-error {{deduction guide declaration without trailing return type}}
 }

clang/test/CXX/temp/temp.res/temp.local/p3.cpp

 
 namespace PR6717 {
   template <typename T>
   class WebVector {
   } // expected-error {{expected ';' after class}}
 
     WebVector(const WebVector<T>& other) { } // expected-error{{undeclared identifier 'T'}} \
                                                 precxx17-error{{a type specifier is required}} \
-                                                cxx17-error{{deduction guide declaration without trailing return type}} \
-                                                cxx17-error{{deduction guide cannot have a function definition}}
+                                                cxx17-error{{deduction guide declaration without trailing return type}}
 
   template <typename C>
   WebVector<T>& operator=(const C& other) { } // expected-error{{undeclared identifier 'T'}}
 }

clang/test/Parser/cxx1z-class-template-argument-deduction.cpp

 // deduction guide declaration.
 template<class> struct B; // expected-note 2{{template is declared here}}
 struct A1 {
   B() noexcept(false); // expected-error {{deduction guide must be declared in the same scope as template 'PR49735::B'}} \
                        // expected-error {{deduction guide declaration without trailing return type}}
 };
 
 struct A2 {
-  template <typename Ty> // expected-note {{non-deducible template parameter 'Ty'}}
+  template <typename Ty>
   B() noexcept(false); // expected-error {{deduction guide must be declared in the same scope as template 'PR49735::B'}} \
-                       // expected-error {{deduction guide template contains a template parameter that cannot be deduced}} \
                        // expected-error {{deduction guide declaration without trailing return type}}
 };
 
 }

clang/test/SemaCXX/invalid-deduction-guide-as-template-candidates.cpp

+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+template <class T> class Foo {}; // expected-note {{candidate template ignored: couldn't infer template argument 'T'}} \
+                                 // expected-note {{candidate function template not viable: requires 1 argument, but 0 were provided}}
+Foo(); // expected-error {{deduction guide declaration without trailing return type}}
+Foo vs; // expected-error {{no viable constructor or deduction guide for deduction of template arguments of 'Foo'}}