[RFC] [AST] [Modules] Handle full cases of DefaultArgStorage::setInherited

[ChuanqiXu]

Summary

There were two assertions in DefaultArgStorage::setInherited previously. It requires the DefaultArgument is either empty or an argument value. It would crash if it has a pointer refers to the previous declaration or contains a chain to the previous declaration.

But there are edge cases could hit them actually. One is InheritDefaultArguments.cppm that I found recently (there is another issue in the driver: https://github.com/llvm/llvm-project/issues/56327). Another one is pr31469.cpp, which was created fives years ago.

This patch tries to fix the two failures by handling full cases in DefaultArgStorage::setInherited.

This is guaranteed to not introduce any breaking change since it lives in the path we wouldn't touch before. And the added assertions for sameness should keep the correctness. But this is really fundamental, it would be better if someone else would like to take a look.

[ChuanqiXu]

Rebase

[ChuanqiXu]

Update tests.

[ChuanqiXu]

Rebase with main.

[v.g.vassilev]

LGTM!

clang/include/clang/AST/DeclTemplate.h

 /// Defines the C++ template declaration subclasses.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_AST_DECLTEMPLATE_H
 #define LLVM_CLANG_AST_DECLTEMPLATE_H
 
 #include "clang/AST/ASTConcept.h"
+#include "clang/AST/ASTContext.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclCXX.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/Redeclarable.h"
 #include "clang/AST/TemplateBase.h"
 #include "clang/AST/Type.h"
 #include "clang/Basic/LLVM.h"
public:
   /// Set the default argument.
   void set(ArgType Arg) {
     assert(!isSet() && "default argument already set");
     ValueOrInherited = Arg;
   }
 
   /// Set that the default argument was inherited from another parameter.
   void setInherited(const ASTContext &C, ParmDecl *InheritedFrom) {
-    assert(!isInherited() && "default argument already inherited");
     InheritedFrom = getParmOwningDefaultArg(InheritedFrom);
     if (!isSet())
       ValueOrInherited = InheritedFrom;
-    else
+    else if (auto *D = ValueOrInherited.template dyn_cast<ParmDecl *>()) {
+      assert(C.isSameDefaultTemplateArgument(D, InheritedFrom));
+      ValueOrInherited =
+          new (allocateDefaultArgStorageChain(C)) Chain{InheritedFrom, get()};
+    } else if (auto *Inherited =
+                   ValueOrInherited.template dyn_cast<Chain *>()) {
+      assert(C.isSameDefaultTemplateArgument(Inherited->PrevDeclWithDefaultArg,
+                                             InheritedFrom));
+      Inherited->PrevDeclWithDefaultArg = InheritedFrom;
+    } else
       ValueOrInherited = new (allocateDefaultArgStorageChain(C))
           Chain{InheritedFrom, ValueOrInherited.template get<ArgType>()};
   }
 
   /// Remove the default argument, even if it was inherited.
   void clear() {
     ValueOrInherited = ArgType();
   }

clang/test/Modules/InheritDefaultArguments.cppm

 // RUN: rm -rf %t
 // RUN: split-file %s %t
 // RUN: cd %t
 //
 // RUN: %clang_cc1 -std=c++20 %t/A.cppm -emit-module-interface -o %t/A.pcm
 // RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t -I%t %t/Use.cppm -verify -fsyntax-only
 
 //--- foo.h
 template <typename T, typename U = int>
 class Templ;
 
 template <typename T, typename U>
-class Templ {};
+class Templ {
+public:
+    Templ(T t) {}
+};
 
 template <typename T>
 Templ(T t) -> Templ<T, int>;
 
 //--- A.cppm
 module;
 #include "foo.h"
 export module A;
 
 //--- Use.cppm
 // expected-no-diagnostics
 module;
 #include "foo.h"
 export module X;
 import A;
+void foo() {
+    Templ t(0);
+}

clang/test/Modules/Inputs/PR31469/textual.h

 namespace A {
 inline
 namespace __1 {
   template <class _Tp> class allocator;
   template <class _Tp, class _Alloc = allocator<_Tp>> class list;
   template <class _VoidPtr> class __list_iterator {
-    //template <class> friend class list; // causes another crash in ASTDeclReader::attachPreviousDecl
+    template <class> friend class list;
     template <class, class> friend class list;
   };
   template <class _Tp, class _Alloc> class __list_imp {};
   template <class _Tp, class _Alloc> class list : __list_imp<_Tp, _Alloc> {
   public:
     list() {}
   };
   template <class _Tp> void f(list<_Tp>);
 }
 }