Skip to content

Commit

Permalink
[AST] [Modules] Handle full cases of DefaultArgStorage::setInherited
Browse files Browse the repository at this point in the history
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
  • Loading branch information
ChuanqiXu9 committed Jul 12, 2022
1 parent af0a26b commit 5791bcf
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 4 deletions.
13 changes: 11 additions & 2 deletions clang/include/clang/AST/DeclTemplate.h
Expand Up @@ -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"
Expand Down Expand Up @@ -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>()};
}
Expand Down
8 changes: 7 additions & 1 deletion clang/test/Modules/InheritDefaultArguments.cppm
Expand Up @@ -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>;
Expand All @@ -26,3 +29,6 @@ module;
#include "foo.h"
export module X;
import A;
void foo() {
Templ t(0);
}
2 changes: 1 addition & 1 deletion clang/test/Modules/Inputs/PR31469/textual.h
Expand Up @@ -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 {};
Expand Down

0 comments on commit 5791bcf

Please sign in to comment.