From decf02e991a8712810db81b12023273b2f51cb7e Mon Sep 17 00:00:00 2001 From: "John F. Carr" Date: Fri, 8 Dec 2023 14:15:42 -0500 Subject: [PATCH 1/3] Do not process template arguments when checking for recursive hyperobjects --- clang/lib/Sema/SemaType.cpp | 31 ++----------------------------- clang/test/Cilk/223.cpp | 22 ++++++++++++++++++++++ 2 files changed, 24 insertions(+), 29 deletions(-) create mode 100644 clang/test/Cilk/223.cpp diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index e85b0711de68..0cff893f0c0d 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -1294,35 +1294,6 @@ static std::optional ContainsHyperobject(QualType Outer) { break; case Type::Record: { const RecordDecl *Decl = cast(T)->getDecl(); - // TODO: There must be a better way to do this. - // A hyperobject might sneak in without being explicitly - // declared in the template. - if (auto Spec = dyn_cast(Decl)) { - if (ClassTemplateDecl *Inner = Spec->getSpecializedTemplate()) - if (auto O = DeclContainsHyperobject(Inner->getTemplatedDecl())) - return O; - const TemplateArgumentList &Args = Spec->getTemplateArgs(); - for (unsigned I = 0; I < Args.size(); ++I) { - const TemplateArgument &Arg = Args.get(I); - switch (Arg.getKind()) { - case TemplateArgument::Declaration: - if (auto O = ContainsHyperobject(Arg.getAsDecl()->getType())) - return O; - break; - case TemplateArgument::Type: - if (auto O = ContainsHyperobject(Arg.getAsType())) - return O; - break; - case TemplateArgument::Integral: - case TemplateArgument::NullPtr: - case TemplateArgument::Null: - break; - default: - return diag::confusing_hyperobject; - } - } - return std::optional(); - } if (const RecordDecl *Def = Decl->getDefinition()) return DeclContainsHyperobject(Def); return diag::confusing_hyperobject; @@ -1362,6 +1333,8 @@ static std::optional ContainsHyperobject(QualType Outer) { } static std::optional DeclContainsHyperobject(const RecordDecl *Decl) { + if (Decl->isInvalidDecl()) + return std::optional(); for (const FieldDecl *FD : Decl->fields()) if (std::optional O = ContainsHyperobject(FD->getType())) return O; diff --git a/clang/test/Cilk/223.cpp b/clang/test/Cilk/223.cpp new file mode 100644 index 000000000000..7e7805c801f8 --- /dev/null +++ b/clang/test/Cilk/223.cpp @@ -0,0 +1,22 @@ +// RUN: %clang_cc1 %s -x c++ -O1 -fopencilk -verify -fsyntax-only +template +class A { }; + +class B { + A a; +}; + +void identity(void* view) {} +void reduce(void* l, void* r) {} + +B _Hyperobject(identity, reduce) b; + +template +class C { T _Hyperobject *field; }; +// expected-error@-1{{incomplete type 'D' may not be a hyperobject}} + +class D { // expected-note{{}}} + C a; // expected-note{{}}} +}; + +D _Hyperobject(identity, reduce) d; From 4fc29f4a7174c438a6d2f225754f52ca343eaff1 Mon Sep 17 00:00:00 2001 From: "John F. Carr" Date: Sat, 9 Dec 2023 16:35:02 -0500 Subject: [PATCH 2/3] If a HyperobjectType has an incomplete element type it must be marked as containing an error. --- clang/lib/AST/Type.cpp | 2 ++ clang/lib/Sema/SemaType.cpp | 16 +++++++++------- 2 files changed, 11 insertions(+), 7 deletions(-) diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp index 5b7463d8d403..d32e47372e40 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -3206,6 +3206,8 @@ HyperobjectType::HyperobjectType(QualType Element, QualType CanonicalPtr, : Type(Hyperobject, CanonicalPtr, Element->getDependence()), ElementType(Element), Identity(i), Reduce(r), IdentityID(ifn), ReduceID(rfn) { + if (Element->isIncompleteType()) // diagnosed in caller + addDependence(TypeDependence::Error); } bool HyperobjectType::hasCallbacks() const { diff --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp index 0cff893f0c0d..e4b9c77abacf 100644 --- a/clang/lib/Sema/SemaType.cpp +++ b/clang/lib/Sema/SemaType.cpp @@ -2442,20 +2442,22 @@ Expr *Sema::ValidateReducerCallback(Expr *E, unsigned NumArgs, QualType Sema::BuildHyperobjectType(QualType Element, Expr *Identity, Expr *Reduce, SourceLocation Loc) { - QualType Result = Element; + // This function must return a HyperobjectType with the given + // element type, otherwise the rest of the front end will get angry. + // Template instantiation is quite strict about preserving structure. + // The compiler will also get angry if the element type is incomplete + // and the HyperobjectType is not marked as containing an error. if (!RequireCompleteType(Loc, Element, CompleteTypeKind::Normal, diag::incomplete_hyperobject)) { - if (std::optional Code = ContainsHyperobject(Result)) - Diag(Loc, *Code) << Result; + if (std::optional Code = ContainsHyperobject(Element)) + Diag(Loc, *Code) << Element; } Identity = ValidateReducerCallback(Identity, 1, Loc); Reduce = ValidateReducerCallback(Reduce, 2, Loc); - // The result of this function must be HyperobjectType if it is called - // from C++ template instantiation when rebuilding an existing hyperobject - // type. - return Context.getHyperobjectType(Result, Identity, Reduce); + // The result will be marked erroneous if Element is incomplete. + return Context.getHyperobjectType(Element, Identity, Reduce); } /// Build a Read-only Pipe type. From df57b5564ef9f4e665d8296b1cffdf5d16e1b1d9 Mon Sep 17 00:00:00 2001 From: "John F. Carr" Date: Wed, 13 Dec 2023 13:27:19 -0500 Subject: [PATCH 3/3] HyperobjectType needs to copy dependence flags from parameters --- clang/lib/AST/Type.cpp | 3 +++ clang/test/Cilk/222.cpp | 26 ++++++++++++++++++++++++++ 2 files changed, 29 insertions(+) create mode 100644 clang/test/Cilk/222.cpp diff --git a/clang/lib/AST/Type.cpp b/clang/lib/AST/Type.cpp index d32e47372e40..00e0738c536b 100644 --- a/clang/lib/AST/Type.cpp +++ b/clang/lib/AST/Type.cpp @@ -3208,6 +3208,9 @@ HyperobjectType::HyperobjectType(QualType Element, QualType CanonicalPtr, IdentityID(ifn), ReduceID(rfn) { if (Element->isIncompleteType()) // diagnosed in caller addDependence(TypeDependence::Error); + addDependence(Element->getDependence()); + addDependence(toTypeDependence(i->getDependence())); + addDependence(toTypeDependence(r->getDependence())); } bool HyperobjectType::hasCallbacks() const { diff --git a/clang/test/Cilk/222.cpp b/clang/test/Cilk/222.cpp new file mode 100644 index 000000000000..8184b3e11f22 --- /dev/null +++ b/clang/test/Cilk/222.cpp @@ -0,0 +1,26 @@ +// RUN: %clang_cc1 %s -x c++ -fopencilk -emit-llvm -verify -mllvm -use-opencilk-runtime-bc=false -mllvm -debug-abi-calls -o /dev/null +#define DEPTH 3 +template struct R { + static void identity(void *); + static void reduce(void *, void *); + int get(int depth) { return depth <= 0 ? i : field.get(depth - 1); } +public: + R field; + // expected-note@-1{{in instantiation}} + // expected-note@-2{{in instantiation}} + // expected-note@-3{{in instantiation}} + int _Hyperobject(identity, reduce) i; + // expected-warning@-1{{reducer callbacks not implemented for structure members}} + // expected-warning@-2{{reducer callbacks not implemented for structure members}} + // expected-warning@-3{{reducer callbacks not implemented for structure members}} +}; + +template<> struct R<0> { int field; int get(int) { return field; } }; + +extern R r; + +int f() { return r.get(DEPTH / 2); } +// expected-note@-1{{in instantiation}} +// expected-note@-2{{in instantiation}} +// expected-note@-3{{in instantiation}} +