[clang] 5791bcf - [AST] [Modules] Handle full cases of DefaultArgStorage::setInherited

[Chuanqi Xu via cfe-commits]

Author: Chuanqi Xu
Date: 2022-07-13T00:13:56+08:00
New Revision: 5791bcf9db0a3ec8bbce586dd99fce71fd773134

URL: https://github.com/llvm/llvm-project/commit/5791bcf9db0a3ec8bbce586dd99fce71fd773134
DIFF: https://github.com/llvm/llvm-project/commit/5791bcf9db0a3ec8bbce586dd99fce71fd773134.diff

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

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. 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.

Reviewed By: v.g.vassilev

Differential Revision: https://reviews.llvm.org/D128974

Added: 
    

Modified: 
    clang/include/clang/AST/DeclTemplate.h
    clang/test/Modules/InheritDefaultArguments.cppm
    clang/test/Modules/Inputs/PR31469/textual.h

Removed: 
    


################################################################################
diff  --git a/clang/include/clang/AST/DeclTemplate.h b/clang/include/clang/AST/DeclTemplate.h
index 3e4ccda73111a..725bb0bced9c5 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -15,6 +15,7 @@
 #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"
@@ -373,11 +374,19 @@ class DefaultArgStorage {
 
   /// 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>()};
   }

diff  --git a/clang/test/Modules/InheritDefaultArguments.cppm b/clang/test/Modules/InheritDefaultArguments.cppm
index bbd5ad4c96a56..0afb46319ff85 100644
--- a/clang/test/Modules/InheritDefaultArguments.cppm
+++ b/clang/test/Modules/InheritDefaultArguments.cppm
@@ -10,7 +10,10 @@ 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>;
@@ -26,3 +29,6 @@ module;
 #include "foo.h"
 export module X;
 import A;
+void foo() {
+    Templ t(0);
+}

diff  --git a/clang/test/Modules/Inputs/PR31469/textual.h b/clang/test/Modules/Inputs/PR31469/textual.h
index abdc662fb5ef8..3eba9be8431d9 100644
--- a/clang/test/Modules/Inputs/PR31469/textual.h
+++ b/clang/test/Modules/Inputs/PR31469/textual.h
@@ -4,7 +4,7 @@ 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 {};