https://github.com/Fznamznon updated https://github.com/llvm/llvm-project/pull/192957
>From cd2a8069daacb37f781a36c5bd21029de6427cc8 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Mon, 20 Apr 2026 04:59:25 -0700 Subject: [PATCH 01/16] [clang][SYCL] Diagnose reference kernel arguments Per SYCL 2020 spec: Reference types are not trivially copyable, so they may not be passed as kernel parameters. This PR adds infrastructure for kernel object visiting and implements diagnostics for reference kernel parameters. The infrastructure will be also used for other kernel parameter restrictions and functional code transformations that will be done in separate PRs. --- clang/lib/Sema/SemaSYCL.cpp | 278 ++++++++++++++++++ .../SemaSYCL/sycl-kernel-arg-restrictions.cpp | 85 ++++++ 2 files changed, 363 insertions(+) create mode 100644 clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 112a6e4416df2..0d22676aced29 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -294,6 +294,268 @@ void SemaSYCL::CheckSYCLExternalFunctionDecl(FunctionDecl *FD) { } } +namespace { +/// A special visitor to visit subobjects within a type, i.e. fields of a +/// class or elements of an array. Useful for SYCl because in SYCL kernels are +/// defined via lambda expressions or named callable objects and kernel +/// parameters are fields of these. These visitors will be used for diagnosing +/// invalid kernel arugments as well as for functional transformations. +class SubobjectVisitor { + ASTContext &Ctx; + + // These enable handler execution only when previous Handlers succeed. + template <typename... Tn> + bool handleField(FieldDecl *FD, QualType FDTy, Tn &&...tn) { + bool result = true; + (void)std::initializer_list<int>{(result = result && tn(FD, FDTy), 0)...}; + return result; + } + template <typename... Tn> + bool handleField(const CXXBaseSpecifier &BD, QualType BDTy, Tn &&...tn) { + bool result = true; + std::initializer_list<int>{(result = result && tn(BD, BDTy), 0)...}; + return result; + } + +#define KF_FOR_EACH(FUNC, Item, Qt) \ + handleField(Item, Qt, ([&](FieldDecl *FD, QualType FDTy) { \ + return Handlers.FUNC(FD, FDTy); \ + })...) + + // Parent contains the FieldDecl or CXXBaseSpecifier that was used to enter + // the Wrapper structure that we're currently visiting. Owner is the parent + // type (which doesn't exist in cases where it is a FieldDecl in the + // 'root'), and Wrapper is the current struct being unwrapped. + template <typename ParentTy, typename... HandlerTys> + void visitComplexRecord(const CXXRecordDecl *Owner, ParentTy &Parent, + const CXXRecordDecl *Wrapper, QualType RecordTy, + HandlerTys &...Handlers) { + (void)std::initializer_list<int>{ + (Handlers.enterStruct(Owner, Parent, RecordTy), 0)...}; + visitRecordHelper(Wrapper, Wrapper->bases(), Handlers...); + visitRecordHelper(Wrapper, Wrapper->fields(), Handlers...); + (void)std::initializer_list<int>{ + (Handlers.leaveStruct(Owner, Parent, RecordTy), 0)...}; + } + + template <typename... HandlerTys> + void visitArray(const CXXRecordDecl *Owner, FieldDecl *Field, + QualType ArrayTy, HandlerTys &...Handlers) { + // TODO add support for simple array visiting, i.e. without entering array + // elements. + visitComplexArray(Owner, Field, ArrayTy, Handlers...); + } + + template <typename ParentTy, typename... HandlerTys> + void visitRecord(const CXXRecordDecl *Owner, ParentTy &Parent, + const CXXRecordDecl *Wrapper, QualType RecordTy, + HandlerTys &...Handlers) { + // TODO add support for simple record visiting, i.e. without entering record + // fields. + visitComplexRecord(Owner, Parent, Wrapper, RecordTy, Handlers...); + } + + template <typename... HandlerTys> + void visitRecordHelper(const CXXRecordDecl *Owner, + clang::CXXRecordDecl::base_class_const_range Range, + HandlerTys &...Handlers) { + for (const auto &Base : Range) { + QualType BaseTy = Base.getType(); + visitRecord(Owner, Base, BaseTy->getAsCXXRecordDecl(), BaseTy, + Handlers...); + } + } + + template <typename... HandlerTys> + void visitRecordHelper(const CXXRecordDecl *Owner, RecordDecl::field_range, + HandlerTys &...Handlers) { + visitRecordFields(Owner, Handlers...); + } + + template <typename... HandlerTys> + void visitArrayElementImpl(const CXXRecordDecl *Owner, FieldDecl *ArrayField, + QualType ElementTy, uint64_t Index, + HandlerTys &...Handlers) { + visitField(Owner, ArrayField, ElementTy, Handlers...); + } + + template <typename... HandlerTys> + void visitNthArrayElement(const CXXRecordDecl *Owner, FieldDecl *ArrayField, + QualType ElementTy, uint64_t Index, + HandlerTys &...Handlers) { + visitArrayElementImpl(Owner, ArrayField, ElementTy, Index, Handlers...); + } + + template <typename... HandlerTys> + void visitComplexArray(const CXXRecordDecl *Owner, FieldDecl *Field, + QualType ArrayTy, HandlerTys &...Handlers) { + // Array workflow is: + // handleArrayType + // enterArray + // visitField (same as before, note that The FieldDecl is the of array + // itself, not the element) + // ... repeat per element, opt-out for duplicates. + // leaveArray + + if (!KF_FOR_EACH(handleArrayType, Field, ArrayTy)) + return; + + const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(ArrayTy); + assert(CAT && "Should only be called on constant-size array."); + QualType ET = CAT->getElementType(); + uint64_t ElemCount = CAT->getSize().getZExtValue(); + + (void)std::initializer_list<int>{ + (Handlers.enterArray(Field, ArrayTy, ET), 0)...}; + + for (uint64_t Index = 0; Index < ElemCount; ++Index) + visitNthArrayElement(Owner, Field, ET, Index, Handlers...); + + (void)std::initializer_list<int>{ + (Handlers.leaveArray(Field, ArrayTy, ET), 0)...}; + } + + template <typename... HandlerTys> + void visitField(const CXXRecordDecl *Owner, FieldDecl *Field, + QualType FieldTy, HandlerTys &...Handlers) { + if (FieldTy->isStructureOrClassType()) { + if (KF_FOR_EACH(handleStructType, Field, FieldTy)) { + CXXRecordDecl *RD = FieldTy->getAsCXXRecordDecl(); + visitRecord(Owner, Field, RD, FieldTy, Handlers...); + } + } else if (FieldTy->isUnionType()) + KF_FOR_EACH(handleUnionType, Field, FieldTy); + else if (FieldTy->isReferenceType()) + KF_FOR_EACH(handleReferenceType, Field, FieldTy); + else if (FieldTy->isPointerType()) + KF_FOR_EACH(handlePointerType, Field, FieldTy); + else if (FieldTy->isArrayType()) + visitArray(Owner, Field, FieldTy, Handlers...); + else if (FieldTy->isScalarType() || FieldTy->isVectorType()) + KF_FOR_EACH(handleScalarType, Field, FieldTy); + else + KF_FOR_EACH(handleOtherType, Field, FieldTy); + } + +public: + SubobjectVisitor(ASTContext &C) : Ctx(C) {} + + template <typename... HandlerTys> + void visitRecordBases(const CXXRecordDecl *KernelFunctor, + HandlerTys &...Handlers) { + visitRecordHelper(KernelFunctor, KernelFunctor->bases(), Handlers...); + } + + template <typename... HandlerTys> + void visitRecordFields(const CXXRecordDecl *Owner, HandlerTys &...Handlers) { + for (const auto Field : Owner->fields()) + visitField(Owner, Field, Field->getType(), Handlers...); + } + +#undef KF_FOR_EACH +}; + +class SyclKernelFieldHandlerBase { +public: + virtual bool handleStructType(FieldDecl *, QualType) { return true; } + virtual bool handleUnionType(FieldDecl *, QualType) { return true; } + virtual bool handleReferenceType(FieldDecl *, QualType) { return true; } + virtual bool handlePointerType(FieldDecl *, QualType) { return true; } + virtual bool handleArrayType(FieldDecl *, QualType) { return true; } + virtual bool handleScalarType(FieldDecl *, QualType) { return true; } + // Most handlers shouldn't be handling this, just the field checker. + virtual bool handleOtherType(FieldDecl *, QualType) { return true; } + + virtual bool enterStruct(const CXXRecordDecl *, FieldDecl *, QualType) { + return true; + } + virtual bool leaveStruct(const CXXRecordDecl *, FieldDecl *, QualType) { + return true; + } + virtual bool enterStruct(const CXXRecordDecl *, const CXXBaseSpecifier &, + QualType) { + return true; + } + virtual bool leaveStruct(const CXXRecordDecl *, const CXXBaseSpecifier &, + QualType) { + return true; + } + // The following are used for stepping through array elements. + virtual bool enterArray(FieldDecl *, QualType, QualType) { return true; } + virtual bool leaveArray(FieldDecl *, QualType, QualType) { return true; } + + virtual ~SyclKernelFieldHandlerBase() = default; +}; + +// A class to act as the direct base for all the SYCL Kernel related +// tasks that contains a reference to Sema (and potentially any other +// universally required data). +class SyclKernelFieldHandler : public SyclKernelFieldHandlerBase { +protected: + SemaSYCL &SemaSYCLRef; + SyclKernelFieldHandler(SemaSYCL &S) : SemaSYCLRef(S) {} +}; + +// A type to check the validity of all of the argument types. +class SyclKernelFieldChecker : public SyclKernelFieldHandler { + bool IsInvalid = false; + + bool checkArrayType(QualType Ty, SourceLocation Loc) { + if (Ty->isArrayType()) { + if (const auto *CAT = + SemaSYCLRef.getASTContext().getAsConstantArrayType(Ty)) { + QualType ET = CAT->getElementType(); + return checkArrayType(ET, Loc); + } + return SemaSYCLRef.Diag(Loc, diag::err_bad_kernel_param_type) << Ty; + } + return false; + } + +public: + /// Constructor for the SyclKernelFieldChecker + /// \param S The SemaSYCL reference used for diagnostics and context. + explicit SyclKernelFieldChecker(SemaSYCL &S) : SyclKernelFieldHandler(S) {} + bool isValid() { return !IsInvalid; } + + bool handleReferenceType(FieldDecl *FD, QualType FieldTy) final { + SemaSYCLRef.Diag(FD->getLocation(), diag::err_bad_kernel_param_type) + << FieldTy; + IsInvalid = true; + return isValid(); + } + + bool handlePointerType(FieldDecl *FD, QualType FieldTy) final { + while (FieldTy->isAnyPointerType()) { + FieldTy = QualType{FieldTy->getPointeeOrArrayElementType(), 0}; + if (FieldTy->isVariableArrayType()) { + SemaSYCLRef.Diag(FD->getLocation(), diag::err_bad_kernel_param_type) + << FieldTy; + IsInvalid = true; + break; + } + } + return isValid(); + } + + bool handleOtherType(FieldDecl *FD, QualType FieldTy) final { + SemaSYCLRef.Diag(FD->getLocation(), diag::err_bad_kernel_param_type) + << FieldTy; + IsInvalid = true; + return isValid(); + } + + bool handleStructType(FieldDecl *, QualType FieldTy) final { + return isValid(); + } + + bool handleArrayType(FieldDecl *FD, QualType FieldTy) final { + IsInvalid |= checkArrayType(FieldTy, FD->getLocation()); + return isValid(); + } +}; +} // namespace + void SemaSYCL::CheckSYCLEntryPointFunctionDecl(FunctionDecl *FD) { // Ensure that all attributes present on the declaration are consistent // and warn about any redundant ones. @@ -665,6 +927,20 @@ OutlinedFunctionDecl *BuildSYCLKernelEntryPointOutline(Sema &SemaRef, return OFD; } +bool verifyKernelArguments(FunctionDecl *FD, SemaSYCL &SemaSYCLRef) { + SyclKernelFieldChecker FieldChecker(SemaSYCLRef); + SubobjectVisitor Visitor{SemaSYCLRef.getASTContext()}; + for (auto Param : FD->parameters()) { + if (Param->getType()->isRecordType()) { + const CXXRecordDecl *ObjRecord = Param->getType()->getAsCXXRecordDecl(); + assert(ObjRecord && "object is expected"); + Visitor.visitRecordBases(ObjRecord, FieldChecker); + Visitor.visitRecordFields(ObjRecord, FieldChecker); + } + } + return !FieldChecker.isValid(); +} + } // unnamed namespace StmtResult SemaSYCL::BuildSYCLKernelCallStmt(FunctionDecl *FD, @@ -690,6 +966,8 @@ StmtResult SemaSYCL::BuildSYCLKernelCallStmt(FunctionDecl *FD, getASTContext().getSYCLKernelInfo(SKEPAttr->getKernelName()); assert(declaresSameEntity(SKI.getKernelEntryPointDecl(), FD) && "SYCL kernel name conflict"); + if (verifyKernelArguments(FD, *this)) + return StmtError(); // Build the outline of the synthesized device entry point function. OutlinedFunctionDecl *OFD = diff --git a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp b/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp new file mode 100644 index 0000000000000..6025f89ff12c4 --- /dev/null +++ b/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp @@ -0,0 +1,85 @@ +// RUN: %clang_cc1 -triple x86_64-linux-gnu -std=c++17 -fsyntax-only -Wno-vla-cxx-extension -fsycl-is-host -verify %s +// RUN: %clang_cc1 -triple spirv64 -std=c++17 -fsyntax-only -Wno-vla-cxx-extension -fsycl-is-device -verify %s + +// A unique kernel name type is required for each declared kernel entry point. +template<int, int = 0> struct KN; + +// A generic kernel launch function. +template<typename KNT, typename... Ts> +void sycl_kernel_launch(const char *, Ts...) {} + +// Kernel entry point template definition. +template<typename KNT, typename T> +[[clang::sycl_kernel_entry_point(KNT)]] +void kernel_single_task(T) {} + +struct Empty {}; + +struct S { + int a; + int &b; //expected-error 2{{'int &' cannot be used as the type of a kernel parameter}} +}; + +void fooarr(int (&arr)[5]) { +} + +template <typename T> class Callable { + T data; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} +public: + Callable(T d) : data(d) {} + void operator()() { + } +}; + +void refCases(int AS) { + int p = 0; + double q = 0; + float s = 0; + kernel_single_task<class KN<1>>( // expected-note {{requested here}} + [ + // expected-error@+1 {{'int &' cannot be used as the type of a kernel parameter}} + &p, q, + // expected-error@+1 {{'float &' cannot be used as the type of a kernel parameter}} + &s] { + (void)q; + (void)p; + (void)s; + }); + + auto L = [&]() { (void)p;}; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} + S Str {p, p}; + kernel_single_task<class KN<2>>( // expected-note {{requested here}} + [=] { + (void)L; + (void)Str; // no error because fail for L already + }); + + kernel_single_task<class KN<3>>( // expected-note {{requested here}} + [=] { + (void)Str; + }); + + S arr[2] = {Str, Str}; + kernel_single_task<class KN<4>>( // expected-note {{requested here}} + [=] { + (void)arr; + }); + int arr1[AS]; + kernel_single_task<class KN<5>>( // expected-note {{requested here}} + [&] { + (void)arr1; // expected-error {{'int (&)[AS]' cannot be used as the type of a kernel parameter}} + }); + auto a = &arr1; + kernel_single_task<class KN<6>>( // expected-note {{requested here}} + [=] { + (void)a; // expected-error {{'int[AS]' cannot be used as the type of a kernel parameter}} + }); + int arrayints[5] = {0}; + kernel_single_task<class KN<7>>( // expected-note {{requested here}} + [&] { + fooarr(arrayints); // expected-error {{'int (&)[5]' cannot be used as the type of a kernel parameter}} + }); + kernel_single_task<class KN<8>>(Callable<int&>{p}); // expected-note {{requested here}} + kernel_single_task<class KN<9>>(Callable<int>{p}); +} + >From 6a0361a7069999d7032b83ed2748ca8636ac59e9 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Fri, 8 May 2026 08:52:01 -0700 Subject: [PATCH 02/16] Redo the visitor as CRTP, put it outside of SemaSYCL --- clang/include/clang/AST/SubobjectVisitor.h | 140 ++++++++ clang/lib/Sema/SemaSYCL.cpp | 307 +++--------------- .../SemaSYCL/sycl-kernel-arg-restrictions.cpp | 33 +- 3 files changed, 196 insertions(+), 284 deletions(-) create mode 100644 clang/include/clang/AST/SubobjectVisitor.h diff --git a/clang/include/clang/AST/SubobjectVisitor.h b/clang/include/clang/AST/SubobjectVisitor.h new file mode 100644 index 0000000000000..ee9b70d6710db --- /dev/null +++ b/clang/include/clang/AST/SubobjectVisitor.h @@ -0,0 +1,140 @@ +//===---------- SubobjectVisitor.h - Subobject Visitor ----------*- C++ -*-===// +// +// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions. +// See https://llvm.org/LICENSE.txt for license information. +// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception +// +//===----------------------------------------------------------------------===// +// +// This file defines the SubobjectVisitor interface, which recursively +// traverses subobjects within a type. +// +//===----------------------------------------------------------------------===// + +#ifndef LLVM_CLANG_AST_SUBOBJECTVISITOR_H +#define LLVM_CLANG_AST_SUBOBJECTVISITOR_H + +#include "clang/AST/Type.h" + +namespace clang { +template <typename Derived> class SubobjectVisitor { + ASTContext &Ctx; + + public: + SubobjectVisitor(ASTContext &Ctx) : Ctx(Ctx) {} + /// Return a reference to the derived class. + Derived &getDerived() { return *static_cast<Derived *>(this); } + + bool enterRecord(CXXRecordDecl *Record, FieldDecl *Parent) { + return getDerived().visitRecord(Record, Parent); + } + + bool leaveRecord(CXXRecordDecl *Record, FieldDecl *Parent) { + return true; + } + + bool enterUnion(CXXRecordDecl *Record, FieldDecl *Parent) { + return getDerived().visitUnion(Record, Parent); + } + + bool enterArray(QualType ArrayTy, FieldDecl *Parent) { + return getDerived().visitArrayTy(ArrayTy, Parent); + } + + bool visitRecord(CXXRecordDecl *Record, FieldDecl *Parent) { + return true; + } + + bool visitUnion(CXXRecordDecl *Record, FieldDecl *Parent) { + return true; + } + + bool visitReferenceType(QualType Ty, FieldDecl *Parent) { + return true; + } + + bool visitPointerType(QualType Ty, FieldDecl *Parent) { + return true; + } + + bool visitScalarType(QualType Ty, FieldDecl *Parent) { + return true; + } + + bool visitOtherType(QualType Ty, FieldDecl *Parent) { + return true; + } + + bool visitArrayTy(QualType Ty, FieldDecl *Parent) { + return true; + } + + bool traverseRecord(CXXRecordDecl *Record) { + FieldDecl* Parent = nullptr; + getDerived().traverseRecord(Record, Parent); + return true; + } + + bool traverseRecord(CXXRecordDecl *Record, FieldDecl *Parent) { + getDerived().enterRecord(Record, Parent); + + for (const auto &Base : Record->bases()) { + QualType BaseTy = Base.getType(); + getDerived().traverseType(BaseTy, Parent); + } + for (const auto Field : Record->fields()) { + QualType FieldTy = Field->getType(); + getDerived().traverseType(FieldTy, Field); + } + + getDerived().leaveRecord(Record, Parent); + return true; + } + + bool traverseUnion(CXXRecordDecl *Record, FieldDecl *Parent) { + getDerived().enterUnion(Record, Parent); + + for (const auto Field : Record->fields()) { + QualType FieldTy = Field->getType(); + getDerived().traverseType(FieldTy, Field); + } + return true; + } + + bool traverseArray(QualType Ty, FieldDecl *Parent) { + getDerived().enterArray(Ty, Parent); + const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(Ty); + if (!CAT) + return true; + + QualType ET = CAT->getElementType(); + uint64_t ElemCount = CAT->getSize().getZExtValue(); + for (uint64_t Index = 0; Index < ElemCount; ++Index) + getDerived().traverseType(ET, Parent); + + return true; + } + + bool traverseType(QualType Ty, FieldDecl *Parent) { + if (Ty->isStructureOrClassType()) { + CXXRecordDecl *RD = Ty->getAsCXXRecordDecl(); + getDerived().traverseRecord(RD, Parent); + } else if (Ty->isUnionType()) { + CXXRecordDecl *RD = Ty->getAsCXXRecordDecl(); + getDerived().traverseUnion(RD, Parent); + } else if (Ty->isReferenceType()) + getDerived().visitReferenceType(Ty, Parent); + else if (Ty->isPointerType()) + getDerived().visitPointerType(Ty, Parent); + else if (Ty->isArrayType()) + getDerived().traverseArray(Ty, Parent); + else if (Ty->isScalarType() || Ty->isVectorType()) + getDerived().visitScalarType(Ty, Parent); + else + getDerived().visitOtherType(Ty, Parent); + return true; + } + +}; +} // end namespace clang +#endif // LLVM_CLANG_AST_SUBOBJECTVISITOR diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 0d22676aced29..ce16f25fb34f9 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -11,6 +11,8 @@ #include "clang/Sema/SemaSYCL.h" #include "TreeTransform.h" #include "clang/AST/Mangle.h" +#include "clang/AST/RecursiveASTVisitor.h" +#include "clang/AST/SubobjectVisitor.h" #include "clang/AST/SYCLKernelInfo.h" #include "clang/AST/StmtSYCL.h" #include "clang/AST/TypeOrdering.h" @@ -294,268 +296,6 @@ void SemaSYCL::CheckSYCLExternalFunctionDecl(FunctionDecl *FD) { } } -namespace { -/// A special visitor to visit subobjects within a type, i.e. fields of a -/// class or elements of an array. Useful for SYCl because in SYCL kernels are -/// defined via lambda expressions or named callable objects and kernel -/// parameters are fields of these. These visitors will be used for diagnosing -/// invalid kernel arugments as well as for functional transformations. -class SubobjectVisitor { - ASTContext &Ctx; - - // These enable handler execution only when previous Handlers succeed. - template <typename... Tn> - bool handleField(FieldDecl *FD, QualType FDTy, Tn &&...tn) { - bool result = true; - (void)std::initializer_list<int>{(result = result && tn(FD, FDTy), 0)...}; - return result; - } - template <typename... Tn> - bool handleField(const CXXBaseSpecifier &BD, QualType BDTy, Tn &&...tn) { - bool result = true; - std::initializer_list<int>{(result = result && tn(BD, BDTy), 0)...}; - return result; - } - -#define KF_FOR_EACH(FUNC, Item, Qt) \ - handleField(Item, Qt, ([&](FieldDecl *FD, QualType FDTy) { \ - return Handlers.FUNC(FD, FDTy); \ - })...) - - // Parent contains the FieldDecl or CXXBaseSpecifier that was used to enter - // the Wrapper structure that we're currently visiting. Owner is the parent - // type (which doesn't exist in cases where it is a FieldDecl in the - // 'root'), and Wrapper is the current struct being unwrapped. - template <typename ParentTy, typename... HandlerTys> - void visitComplexRecord(const CXXRecordDecl *Owner, ParentTy &Parent, - const CXXRecordDecl *Wrapper, QualType RecordTy, - HandlerTys &...Handlers) { - (void)std::initializer_list<int>{ - (Handlers.enterStruct(Owner, Parent, RecordTy), 0)...}; - visitRecordHelper(Wrapper, Wrapper->bases(), Handlers...); - visitRecordHelper(Wrapper, Wrapper->fields(), Handlers...); - (void)std::initializer_list<int>{ - (Handlers.leaveStruct(Owner, Parent, RecordTy), 0)...}; - } - - template <typename... HandlerTys> - void visitArray(const CXXRecordDecl *Owner, FieldDecl *Field, - QualType ArrayTy, HandlerTys &...Handlers) { - // TODO add support for simple array visiting, i.e. without entering array - // elements. - visitComplexArray(Owner, Field, ArrayTy, Handlers...); - } - - template <typename ParentTy, typename... HandlerTys> - void visitRecord(const CXXRecordDecl *Owner, ParentTy &Parent, - const CXXRecordDecl *Wrapper, QualType RecordTy, - HandlerTys &...Handlers) { - // TODO add support for simple record visiting, i.e. without entering record - // fields. - visitComplexRecord(Owner, Parent, Wrapper, RecordTy, Handlers...); - } - - template <typename... HandlerTys> - void visitRecordHelper(const CXXRecordDecl *Owner, - clang::CXXRecordDecl::base_class_const_range Range, - HandlerTys &...Handlers) { - for (const auto &Base : Range) { - QualType BaseTy = Base.getType(); - visitRecord(Owner, Base, BaseTy->getAsCXXRecordDecl(), BaseTy, - Handlers...); - } - } - - template <typename... HandlerTys> - void visitRecordHelper(const CXXRecordDecl *Owner, RecordDecl::field_range, - HandlerTys &...Handlers) { - visitRecordFields(Owner, Handlers...); - } - - template <typename... HandlerTys> - void visitArrayElementImpl(const CXXRecordDecl *Owner, FieldDecl *ArrayField, - QualType ElementTy, uint64_t Index, - HandlerTys &...Handlers) { - visitField(Owner, ArrayField, ElementTy, Handlers...); - } - - template <typename... HandlerTys> - void visitNthArrayElement(const CXXRecordDecl *Owner, FieldDecl *ArrayField, - QualType ElementTy, uint64_t Index, - HandlerTys &...Handlers) { - visitArrayElementImpl(Owner, ArrayField, ElementTy, Index, Handlers...); - } - - template <typename... HandlerTys> - void visitComplexArray(const CXXRecordDecl *Owner, FieldDecl *Field, - QualType ArrayTy, HandlerTys &...Handlers) { - // Array workflow is: - // handleArrayType - // enterArray - // visitField (same as before, note that The FieldDecl is the of array - // itself, not the element) - // ... repeat per element, opt-out for duplicates. - // leaveArray - - if (!KF_FOR_EACH(handleArrayType, Field, ArrayTy)) - return; - - const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(ArrayTy); - assert(CAT && "Should only be called on constant-size array."); - QualType ET = CAT->getElementType(); - uint64_t ElemCount = CAT->getSize().getZExtValue(); - - (void)std::initializer_list<int>{ - (Handlers.enterArray(Field, ArrayTy, ET), 0)...}; - - for (uint64_t Index = 0; Index < ElemCount; ++Index) - visitNthArrayElement(Owner, Field, ET, Index, Handlers...); - - (void)std::initializer_list<int>{ - (Handlers.leaveArray(Field, ArrayTy, ET), 0)...}; - } - - template <typename... HandlerTys> - void visitField(const CXXRecordDecl *Owner, FieldDecl *Field, - QualType FieldTy, HandlerTys &...Handlers) { - if (FieldTy->isStructureOrClassType()) { - if (KF_FOR_EACH(handleStructType, Field, FieldTy)) { - CXXRecordDecl *RD = FieldTy->getAsCXXRecordDecl(); - visitRecord(Owner, Field, RD, FieldTy, Handlers...); - } - } else if (FieldTy->isUnionType()) - KF_FOR_EACH(handleUnionType, Field, FieldTy); - else if (FieldTy->isReferenceType()) - KF_FOR_EACH(handleReferenceType, Field, FieldTy); - else if (FieldTy->isPointerType()) - KF_FOR_EACH(handlePointerType, Field, FieldTy); - else if (FieldTy->isArrayType()) - visitArray(Owner, Field, FieldTy, Handlers...); - else if (FieldTy->isScalarType() || FieldTy->isVectorType()) - KF_FOR_EACH(handleScalarType, Field, FieldTy); - else - KF_FOR_EACH(handleOtherType, Field, FieldTy); - } - -public: - SubobjectVisitor(ASTContext &C) : Ctx(C) {} - - template <typename... HandlerTys> - void visitRecordBases(const CXXRecordDecl *KernelFunctor, - HandlerTys &...Handlers) { - visitRecordHelper(KernelFunctor, KernelFunctor->bases(), Handlers...); - } - - template <typename... HandlerTys> - void visitRecordFields(const CXXRecordDecl *Owner, HandlerTys &...Handlers) { - for (const auto Field : Owner->fields()) - visitField(Owner, Field, Field->getType(), Handlers...); - } - -#undef KF_FOR_EACH -}; - -class SyclKernelFieldHandlerBase { -public: - virtual bool handleStructType(FieldDecl *, QualType) { return true; } - virtual bool handleUnionType(FieldDecl *, QualType) { return true; } - virtual bool handleReferenceType(FieldDecl *, QualType) { return true; } - virtual bool handlePointerType(FieldDecl *, QualType) { return true; } - virtual bool handleArrayType(FieldDecl *, QualType) { return true; } - virtual bool handleScalarType(FieldDecl *, QualType) { return true; } - // Most handlers shouldn't be handling this, just the field checker. - virtual bool handleOtherType(FieldDecl *, QualType) { return true; } - - virtual bool enterStruct(const CXXRecordDecl *, FieldDecl *, QualType) { - return true; - } - virtual bool leaveStruct(const CXXRecordDecl *, FieldDecl *, QualType) { - return true; - } - virtual bool enterStruct(const CXXRecordDecl *, const CXXBaseSpecifier &, - QualType) { - return true; - } - virtual bool leaveStruct(const CXXRecordDecl *, const CXXBaseSpecifier &, - QualType) { - return true; - } - // The following are used for stepping through array elements. - virtual bool enterArray(FieldDecl *, QualType, QualType) { return true; } - virtual bool leaveArray(FieldDecl *, QualType, QualType) { return true; } - - virtual ~SyclKernelFieldHandlerBase() = default; -}; - -// A class to act as the direct base for all the SYCL Kernel related -// tasks that contains a reference to Sema (and potentially any other -// universally required data). -class SyclKernelFieldHandler : public SyclKernelFieldHandlerBase { -protected: - SemaSYCL &SemaSYCLRef; - SyclKernelFieldHandler(SemaSYCL &S) : SemaSYCLRef(S) {} -}; - -// A type to check the validity of all of the argument types. -class SyclKernelFieldChecker : public SyclKernelFieldHandler { - bool IsInvalid = false; - - bool checkArrayType(QualType Ty, SourceLocation Loc) { - if (Ty->isArrayType()) { - if (const auto *CAT = - SemaSYCLRef.getASTContext().getAsConstantArrayType(Ty)) { - QualType ET = CAT->getElementType(); - return checkArrayType(ET, Loc); - } - return SemaSYCLRef.Diag(Loc, diag::err_bad_kernel_param_type) << Ty; - } - return false; - } - -public: - /// Constructor for the SyclKernelFieldChecker - /// \param S The SemaSYCL reference used for diagnostics and context. - explicit SyclKernelFieldChecker(SemaSYCL &S) : SyclKernelFieldHandler(S) {} - bool isValid() { return !IsInvalid; } - - bool handleReferenceType(FieldDecl *FD, QualType FieldTy) final { - SemaSYCLRef.Diag(FD->getLocation(), diag::err_bad_kernel_param_type) - << FieldTy; - IsInvalid = true; - return isValid(); - } - - bool handlePointerType(FieldDecl *FD, QualType FieldTy) final { - while (FieldTy->isAnyPointerType()) { - FieldTy = QualType{FieldTy->getPointeeOrArrayElementType(), 0}; - if (FieldTy->isVariableArrayType()) { - SemaSYCLRef.Diag(FD->getLocation(), diag::err_bad_kernel_param_type) - << FieldTy; - IsInvalid = true; - break; - } - } - return isValid(); - } - - bool handleOtherType(FieldDecl *FD, QualType FieldTy) final { - SemaSYCLRef.Diag(FD->getLocation(), diag::err_bad_kernel_param_type) - << FieldTy; - IsInvalid = true; - return isValid(); - } - - bool handleStructType(FieldDecl *, QualType FieldTy) final { - return isValid(); - } - - bool handleArrayType(FieldDecl *FD, QualType FieldTy) final { - IsInvalid |= checkArrayType(FieldTy, FD->getLocation()); - return isValid(); - } -}; -} // namespace - void SemaSYCL::CheckSYCLEntryPointFunctionDecl(FunctionDecl *FD) { // Ensure that all attributes present on the declaration are consistent // and warn about any redundant ones. @@ -927,18 +667,49 @@ OutlinedFunctionDecl *BuildSYCLKernelEntryPointOutline(Sema &SemaRef, return OFD; } +class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { + SemaSYCL &SemaSYCLRef; + bool IsValid = true; + SmallVector<const CXXRecordDecl *, 4> History; + +public: + KernelArgsChecker(SemaSYCL &SR, SourceLocation Loc) + : SubobjectVisitor<KernelArgsChecker>(SR.getASTContext()), + SemaSYCLRef(SR) {} + bool enterRecord(CXXRecordDecl *Record, FieldDecl *Parent) { + History.push_back(Record); + return getDerived().visitRecord(Record, Parent); + } + + bool visitReferenceType(QualType Ty, FieldDecl *Parent) { + SemaSYCLRef.Diag(Parent->getLocation(), diag::err_bad_kernel_param_type) + << Ty; + for (auto &ParentRecord : History) + SemaSYCLRef.Diag(ParentRecord->getLocation(), + diag::note_within_field_of_type) + << ParentRecord; + IsValid = false; + return true; + } + + bool leaveRecord(CXXRecordDecl *Record, FieldDecl *Parent) { + History.pop_back(); + return true; + } + + bool isInvalid() { return !IsValid; } +}; + bool verifyKernelArguments(FunctionDecl *FD, SemaSYCL &SemaSYCLRef) { - SyclKernelFieldChecker FieldChecker(SemaSYCLRef); - SubobjectVisitor Visitor{SemaSYCLRef.getASTContext()}; + KernelArgsChecker KAC(SemaSYCLRef, FD->getLocation()); for (auto Param : FD->parameters()) { if (Param->getType()->isRecordType()) { - const CXXRecordDecl *ObjRecord = Param->getType()->getAsCXXRecordDecl(); + CXXRecordDecl *ObjRecord = Param->getType()->getAsCXXRecordDecl(); assert(ObjRecord && "object is expected"); - Visitor.visitRecordBases(ObjRecord, FieldChecker); - Visitor.visitRecordFields(ObjRecord, FieldChecker); + KAC.traverseRecord(ObjRecord); } } - return !FieldChecker.isValid(); + return KAC.isInvalid(); } } // unnamed namespace diff --git a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp b/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp index 6025f89ff12c4..2df4dadaf293a 100644 --- a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp +++ b/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp @@ -13,17 +13,15 @@ template<typename KNT, typename T> [[clang::sycl_kernel_entry_point(KNT)]] void kernel_single_task(T) {} -struct Empty {}; - -struct S { +struct S { // expected-note 4{{within field of type 'S' declared here}} int a; - int &b; //expected-error 2{{'int &' cannot be used as the type of a kernel parameter}} + int &b; //expected-error 4{{'int &' cannot be used as the type of a kernel parameter}} }; void fooarr(int (&arr)[5]) { } -template <typename T> class Callable { +template <typename T> class Callable { // expected-note {{within field of type 'Callable<int &>' declared here}} T data; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} public: Callable(T d) : data(d) {} @@ -31,12 +29,18 @@ template <typename T> class Callable { } }; +class Derived1 : Callable<int> { // expected-note {{within field of type 'Derived1' declared here}} + int &a; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} +public: + Derived1(int d, int &b) : Callable<int>(d), a(b) {} +}; + void refCases(int AS) { int p = 0; double q = 0; float s = 0; kernel_single_task<class KN<1>>( // expected-note {{requested here}} - [ + [ // expected-note2{{within field of type}} // expected-error@+1 {{'int &' cannot be used as the type of a kernel parameter}} &p, q, // expected-error@+1 {{'float &' cannot be used as the type of a kernel parameter}} @@ -47,39 +51,36 @@ void refCases(int AS) { }); auto L = [&]() { (void)p;}; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} + // expected-note@-1 {{within field of type}} S Str {p, p}; kernel_single_task<class KN<2>>( // expected-note {{requested here}} - [=] { + [=] { // expected-note 2{{within field of type}} (void)L; (void)Str; // no error because fail for L already }); kernel_single_task<class KN<3>>( // expected-note {{requested here}} - [=] { + [=] { // // expected-note {{within field of type}} (void)Str; }); S arr[2] = {Str, Str}; kernel_single_task<class KN<4>>( // expected-note {{requested here}} - [=] { + [=] { // expected-note 2{{within field of type}} (void)arr; }); int arr1[AS]; kernel_single_task<class KN<5>>( // expected-note {{requested here}} - [&] { + [&] { // expected-note {{within field of type}} (void)arr1; // expected-error {{'int (&)[AS]' cannot be used as the type of a kernel parameter}} }); - auto a = &arr1; - kernel_single_task<class KN<6>>( // expected-note {{requested here}} - [=] { - (void)a; // expected-error {{'int[AS]' cannot be used as the type of a kernel parameter}} - }); int arrayints[5] = {0}; kernel_single_task<class KN<7>>( // expected-note {{requested here}} - [&] { + [&] { // expected-note {{within field of type}} fooarr(arrayints); // expected-error {{'int (&)[5]' cannot be used as the type of a kernel parameter}} }); kernel_single_task<class KN<8>>(Callable<int&>{p}); // expected-note {{requested here}} kernel_single_task<class KN<9>>(Callable<int>{p}); + kernel_single_task<class KN<10>>(Derived1{p, p}); // expected-note {{requested here}} } >From b6c566d9a91fd94644e12db383ccf7dc6a3eb23f Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Wed, 20 May 2026 10:29:50 -0700 Subject: [PATCH 03/16] WIP, test doesn't pass yet --- clang/include/clang/AST/SubobjectVisitor.h | 181 ++++++++---------- .../clang/Basic/DiagnosticSemaKinds.td | 2 + clang/lib/Sema/SemaSYCL.cpp | 50 +++-- 3 files changed, 122 insertions(+), 111 deletions(-) diff --git a/clang/include/clang/AST/SubobjectVisitor.h b/clang/include/clang/AST/SubobjectVisitor.h index ee9b70d6710db..62c8e6ec65c02 100644 --- a/clang/include/clang/AST/SubobjectVisitor.h +++ b/clang/include/clang/AST/SubobjectVisitor.h @@ -17,124 +17,113 @@ #include "clang/AST/Type.h" namespace clang { -template <typename Derived> class SubobjectVisitor { - ASTContext &Ctx; - public: - SubobjectVisitor(ASTContext &Ctx) : Ctx(Ctx) {} - /// Return a reference to the derived class. - Derived &getDerived() { return *static_cast<Derived *>(this); } - - bool enterRecord(CXXRecordDecl *Record, FieldDecl *Parent) { - return getDerived().visitRecord(Record, Parent); - } +#define DISPATCH(CLASS) \ + return static_cast<Derived *>(this)->visit##CLASS( \ + static_cast<const CLASS *>(T)) - bool leaveRecord(CXXRecordDecl *Record, FieldDecl *Parent) { - return true; - } +template <template <typename> class Ptr, typename Derived> +class SubobjectVisitorBase { + ASTContext &Ctx; + template <typename Class> using ptr_t = typename Ptr<Class>::type; + template <typename Class> + using non_ptr_t = typename std::remove_pointer<ptr_t<Class>>::type; - bool enterUnion(CXXRecordDecl *Record, FieldDecl *Parent) { - return getDerived().visitUnion(Record, Parent); - } +public: + SubobjectVisitorBase(ASTContext &Ctx) : Ctx(Ctx) {} - bool enterArray(QualType ArrayTy, FieldDecl *Parent) { - return getDerived().visitArrayTy(ArrayTy, Parent); - } - - bool visitRecord(CXXRecordDecl *Record, FieldDecl *Parent) { - return true; - } + /// Return a reference to the derived class. + Derived &getDerived() { return *static_cast<Derived *>(this); } - bool visitUnion(CXXRecordDecl *Record, FieldDecl *Parent) { - return true; - } + void visit(QualType QT) { + // If the type is an array, visit its element type. Separate traversal of + // arrays is not needed because the array will be encountered as a + // FieldDecl. - bool visitReferenceType(QualType Ty, FieldDecl *Parent) { - return true; - } + if (QT->isArrayType()) { + QualType ElTy = + cast<ConstantArrayType>(Ctx.getAsArrayType(QT))->getElementType(); + visit(ElTy); + return; + } - bool visitPointerType(QualType Ty, FieldDecl *Parent) { - return true; - } + if (ptr_t<RecordDecl> RD = QT->getAsRecordDecl()) { + traverseRecord(RD); + return; + } - bool visitScalarType(QualType Ty, FieldDecl *Parent) { - return true; + visitGenericType(QT.getTypePtr()); } - bool visitOtherType(QualType Ty, FieldDecl *Parent) { - return true; + void traverseRecord(ptr_t<RecordDecl> RD) { + if (ptr_t<CXXRecordDecl> CRD = dyn_cast<CXXRecordDecl>(RD)) { + for (non_ptr_t<CXXBaseSpecifier> BS : CRD->bases()) { + if (!static_cast<Derived *>(this)->visitBaseSpecifierPre(BS)) + continue; + visit(BS.getType()); + static_cast<Derived *>(this)->visitBaseSpecifierPost(BS); + } + } + for (ptr_t<FieldDecl> FD : RD->fields()) { + if (!static_cast<Derived *>(this)->visitFieldDeclPre(FD)) + continue; + visit(FD->getType()); + static_cast<Derived *>(this)->visitFieldDeclPost(FD); + } } - bool visitArrayTy(QualType Ty, FieldDecl *Parent) { - return true; - } + // Default base class specifier pre-order visitor. + bool visitBaseSpecifierPre(non_ptr_t<CXXBaseSpecifier> BS) { return true; } - bool traverseRecord(CXXRecordDecl *Record) { - FieldDecl* Parent = nullptr; - getDerived().traverseRecord(Record, Parent); - return true; - } + // Default base class specifier post-order visitor. + void visitBaseSpecifierPost(non_ptr_t<CXXBaseSpecifier> BS) {} - bool traverseRecord(CXXRecordDecl *Record, FieldDecl *Parent) { - getDerived().enterRecord(Record, Parent); + // Default field pre-order visitor. + bool visitFieldDeclPre(ptr_t<FieldDecl> FD) { return true; } - for (const auto &Base : Record->bases()) { - QualType BaseTy = Base.getType(); - getDerived().traverseType(BaseTy, Parent); + // Default field post-order visitor. + void visitFieldDeclPost(ptr_t<FieldDecl> FD) {} + bool visitGenericType(const Type *T) { + // Top switch stmt: dispatch to VisitFooType for each FooType. + switch (T->getTypeClass()) { +#define ABSTRACT_TYPE(CLASS, PARENT) +#define TYPE(CLASS, PARENT) \ + case Type::CLASS: \ + DISPATCH(CLASS##Type); +#include "clang/AST/TypeNodes.inc" } - for (const auto Field : Record->fields()) { - QualType FieldTy = Field->getType(); - getDerived().traverseType(FieldTy, Field); - } - - getDerived().leaveRecord(Record, Parent); - return true; + llvm_unreachable("Unknown type class!"); } - bool traverseUnion(CXXRecordDecl *Record, FieldDecl *Parent) { - getDerived().enterUnion(Record, Parent); + // If the implementation chooses not to implement a certain visit method, fall + // back on superclass. +#define TYPE(CLASS, PARENT) \ + bool visit##CLASS##Type(const CLASS##Type *T) { DISPATCH(PARENT); } +#include "clang/AST/TypeNodes.inc" - for (const auto Field : Record->fields()) { - QualType FieldTy = Field->getType(); - getDerived().traverseType(FieldTy, Field); - } - return true; - } - - bool traverseArray(QualType Ty, FieldDecl *Parent) { - getDerived().enterArray(Ty, Parent); - const ConstantArrayType *CAT = Ctx.getAsConstantArrayType(Ty); - if (!CAT) - return true; + /// Method called if \c ImpClass doesn't provide specific handler + /// for some type class. + bool visitType(const Type *) { return true; } +}; - QualType ET = CAT->getElementType(); - uint64_t ElemCount = CAT->getSize().getZExtValue(); - for (uint64_t Index = 0; Index < ElemCount; ++Index) - getDerived().traverseType(ET, Parent); +template <typename Derived> +class SubobjectVisitor + : public SubobjectVisitorBase<std::add_pointer, Derived> { +public: + SubobjectVisitor(ASTContext &Ctx) + : SubobjectVisitorBase<std::add_pointer, Derived>(Ctx) {} +}; - return true; - } +template <typename Derived> +class ConstSubobjectVisitor + : public SubobjectVisitorBase<llvm::make_const_ptr, Derived> { +public: + ConstSubobjectVisitor(ASTContext &Ctx) + : SubobjectVisitorBase<std::add_pointer, Derived>(Ctx) {} +}; - bool traverseType(QualType Ty, FieldDecl *Parent) { - if (Ty->isStructureOrClassType()) { - CXXRecordDecl *RD = Ty->getAsCXXRecordDecl(); - getDerived().traverseRecord(RD, Parent); - } else if (Ty->isUnionType()) { - CXXRecordDecl *RD = Ty->getAsCXXRecordDecl(); - getDerived().traverseUnion(RD, Parent); - } else if (Ty->isReferenceType()) - getDerived().visitReferenceType(Ty, Parent); - else if (Ty->isPointerType()) - getDerived().visitPointerType(Ty, Parent); - else if (Ty->isArrayType()) - getDerived().traverseArray(Ty, Parent); - else if (Ty->isScalarType() || Ty->isVectorType()) - getDerived().visitScalarType(Ty, Parent); - else - getDerived().visitOtherType(Ty, Parent); - return true; - } +#undef DISPATCH -}; } // end namespace clang + #endif // LLVM_CLANG_AST_SUBOBJECTVISITOR diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 9cd3a5b2fc049..0f05e798b92b3 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11804,6 +11804,8 @@ def err_record_with_pointers_kernel_param : Error< "%select{struct|union}0 kernel parameters may not contain pointers">; def note_within_field_of_type : Note< "within field of type %0 declared here">; +def note_within_base_of_type : Note< + "within base of type %1 declared here">; def note_illegal_field_declared_here : Note< "field of illegal %select{type|pointer type}0 %1 declared here">; def err_opencl_type_struct_or_union_field : Error< diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index ce16f25fb34f9..469bdef168509 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -11,7 +11,6 @@ #include "clang/Sema/SemaSYCL.h" #include "TreeTransform.h" #include "clang/AST/Mangle.h" -#include "clang/AST/RecursiveASTVisitor.h" #include "clang/AST/SubobjectVisitor.h" #include "clang/AST/SYCLKernelInfo.h" #include "clang/AST/StmtSYCL.h" @@ -670,33 +669,54 @@ OutlinedFunctionDecl *BuildSYCLKernelEntryPointOutline(Sema &SemaRef, class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { SemaSYCL &SemaSYCLRef; bool IsValid = true; - SmallVector<const CXXRecordDecl *, 4> History; + using SubobjectAccess = llvm::PointerUnion<CXXBaseSpecifier *, FieldDecl *>; + SmallVector<SubobjectAccess, 4> SubobjectAccessPath; public: KernelArgsChecker(SemaSYCL &SR, SourceLocation Loc) : SubobjectVisitor<KernelArgsChecker>(SR.getASTContext()), SemaSYCLRef(SR) {} - bool enterRecord(CXXRecordDecl *Record, FieldDecl *Parent) { - History.push_back(Record); - return getDerived().visitRecord(Record, Parent); + + bool visitBaseSpecifierPre(CXXBaseSpecifier BS) { + SubobjectAccessPath.push_back(&BS); + return true; } - bool visitReferenceType(QualType Ty, FieldDecl *Parent) { - SemaSYCLRef.Diag(Parent->getLocation(), diag::err_bad_kernel_param_type) - << Ty; - for (auto &ParentRecord : History) - SemaSYCLRef.Diag(ParentRecord->getLocation(), - diag::note_within_field_of_type) - << ParentRecord; - IsValid = false; + bool visitFieldDeclPre(FieldDecl *FD) { + SubobjectAccessPath.push_back(FD); return true; } - bool leaveRecord(CXXRecordDecl *Record, FieldDecl *Parent) { - History.pop_back(); + bool visitReferenceType(const ReferenceType *RT) { + + // Reference cannot be a base, so just assume we came via a FieldDecl. + auto *DirectParent = SubobjectAccessPath.back().get<FieldDecl *>(); + SemaSYCLRef.Diag(DirectParent->getLocation(), + diag::err_bad_kernel_param_type) + << DirectParent->getType(); + + for (auto Parent : SubobjectAccessPath) { + if (auto *FD = Parent.dyn_cast<FieldDecl *>()) { + SemaSYCLRef.Diag(FD->getParent()->getLocation(), + diag::note_within_field_of_type) + << FD->getParent(); + } else { + auto *BS = Parent.get<CXXBaseSpecifier *>(); + CXXRecordDecl *RD = BS->getType()->getAsCXXRecordDecl(); + assert(RD); + SemaSYCLRef.Diag(RD->getLocation(), diag::note_within_base_of_type) + << RD; + } + } + IsValid = false; return true; } + void visitFieldDeclPost(FieldDecl *FD) { SubobjectAccessPath.pop_back(); } + void visitBaseSpecifierPost(CXXBaseSpecifier BS) { + SubobjectAccessPath.pop_back(); + } + bool isInvalid() { return !IsValid; } }; >From b06133dc57b14e534928c0c3598690706fa09c4f Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Thu, 21 May 2026 04:27:29 -0700 Subject: [PATCH 04/16] Make the test pass --- clang/include/clang/AST/SubobjectVisitor.h | 2 +- clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/clang/include/clang/AST/SubobjectVisitor.h b/clang/include/clang/AST/SubobjectVisitor.h index 62c8e6ec65c02..0bf70c2a8b6af 100644 --- a/clang/include/clang/AST/SubobjectVisitor.h +++ b/clang/include/clang/AST/SubobjectVisitor.h @@ -52,7 +52,7 @@ class SubobjectVisitorBase { return; } - visitGenericType(QT.getTypePtr()); + visitGenericType(QT.getCanonicalType().getTypePtr()); } void traverseRecord(ptr_t<RecordDecl> RD) { diff --git a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp b/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp index 2df4dadaf293a..f727b416953f0 100644 --- a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp +++ b/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp @@ -13,9 +13,9 @@ template<typename KNT, typename T> [[clang::sycl_kernel_entry_point(KNT)]] void kernel_single_task(T) {} -struct S { // expected-note 4{{within field of type 'S' declared here}} +struct S { // expected-note 3{{within field of type 'S' declared here}} int a; - int &b; //expected-error 4{{'int &' cannot be used as the type of a kernel parameter}} + int &b; //expected-error 3{{'int &' cannot be used as the type of a kernel parameter}} }; void fooarr(int (&arr)[5]) { @@ -52,7 +52,7 @@ void refCases(int AS) { auto L = [&]() { (void)p;}; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} // expected-note@-1 {{within field of type}} - S Str {p, p}; + S Str {p, p}; kernel_single_task<class KN<2>>( // expected-note {{requested here}} [=] { // expected-note 2{{within field of type}} (void)L; @@ -66,7 +66,7 @@ void refCases(int AS) { S arr[2] = {Str, Str}; kernel_single_task<class KN<4>>( // expected-note {{requested here}} - [=] { // expected-note 2{{within field of type}} + [=] { // expected-note {{within field of type}} (void)arr; }); int arr1[AS]; >From 90a7172e4845d63c0216b16f8f95ea5f21694e28 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Thu, 21 May 2026 06:03:23 -0700 Subject: [PATCH 05/16] Fix base class history --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 2 +- clang/lib/Sema/SemaSYCL.cpp | 2 +- clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp | 11 +++++++++-- 3 files changed, 11 insertions(+), 4 deletions(-) diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index 0f05e798b92b3..7d70827341f93 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -11805,7 +11805,7 @@ def err_record_with_pointers_kernel_param : Error< def note_within_field_of_type : Note< "within field of type %0 declared here">; def note_within_base_of_type : Note< - "within base of type %1 declared here">; + "within base of type %0 declared here">; def note_illegal_field_declared_here : Note< "field of illegal %select{type|pointer type}0 %1 declared here">; def err_opencl_type_struct_or_union_field : Error< diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 469bdef168509..388a471a7d9e5 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -704,7 +704,7 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { auto *BS = Parent.get<CXXBaseSpecifier *>(); CXXRecordDecl *RD = BS->getType()->getAsCXXRecordDecl(); assert(RD); - SemaSYCLRef.Diag(RD->getLocation(), diag::note_within_base_of_type) + SemaSYCLRef.Diag(BS->getBeginLoc(), diag::note_within_base_of_type) << RD; } } diff --git a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp b/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp index f727b416953f0..47ca74d1642a3 100644 --- a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp +++ b/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp @@ -21,8 +21,8 @@ struct S { // expected-note 3{{within field of type 'S' declared here}} void fooarr(int (&arr)[5]) { } -template <typename T> class Callable { // expected-note {{within field of type 'Callable<int &>' declared here}} - T data; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} +template <typename T> class Callable { // expected-note 2{{within field of type 'Callable<int &>' declared here}} + T data; // expected-error 2{{'int &' cannot be used as the type of a kernel parameter}} public: Callable(T d) : data(d) {} void operator()() { @@ -35,6 +35,12 @@ class Derived1 : Callable<int> { // expected-note {{within field of type 'Derive Derived1(int d, int &b) : Callable<int>(d), a(b) {} }; +class Derived2 : Callable<int&> { // expected-note {{within base of type 'Callable<int &>' declared here}} + int a; +public: + Derived2(int d, int &b) : Callable<int&>(b), a(d) {} +}; + void refCases(int AS) { int p = 0; double q = 0; @@ -82,5 +88,6 @@ void refCases(int AS) { kernel_single_task<class KN<8>>(Callable<int&>{p}); // expected-note {{requested here}} kernel_single_task<class KN<9>>(Callable<int>{p}); kernel_single_task<class KN<10>>(Derived1{p, p}); // expected-note {{requested here}} + kernel_single_task<class KN<11>>(Derived2{p, p}); // expected-note {{requested here}} } >From afc70df320b3051473b507c678cf8d785cbe2c9e Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Thu, 21 May 2026 06:08:44 -0700 Subject: [PATCH 06/16] Fix the warning --- clang/lib/Sema/SemaSYCL.cpp | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 388a471a7d9e5..503fb9e33f27b 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -688,9 +688,8 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { } bool visitReferenceType(const ReferenceType *RT) { - // Reference cannot be a base, so just assume we came via a FieldDecl. - auto *DirectParent = SubobjectAccessPath.back().get<FieldDecl *>(); + auto *DirectParent = cast<FieldDecl *>(SubobjectAccessPath.back()); SemaSYCLRef.Diag(DirectParent->getLocation(), diag::err_bad_kernel_param_type) << DirectParent->getType(); @@ -701,7 +700,7 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { diag::note_within_field_of_type) << FD->getParent(); } else { - auto *BS = Parent.get<CXXBaseSpecifier *>(); + auto *BS = cast<CXXBaseSpecifier *>(Parent); CXXRecordDecl *RD = BS->getType()->getAsCXXRecordDecl(); assert(RD); SemaSYCLRef.Diag(BS->getBeginLoc(), diag::note_within_base_of_type) >From e60ade531053ae0b3662e8fb6edb741e5ac902a9 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Thu, 21 May 2026 06:15:25 -0700 Subject: [PATCH 07/16] Visit, not traverse --- clang/lib/Sema/SemaSYCL.cpp | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 503fb9e33f27b..6d2b3447868ce 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -722,11 +722,8 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { bool verifyKernelArguments(FunctionDecl *FD, SemaSYCL &SemaSYCLRef) { KernelArgsChecker KAC(SemaSYCLRef, FD->getLocation()); for (auto Param : FD->parameters()) { - if (Param->getType()->isRecordType()) { - CXXRecordDecl *ObjRecord = Param->getType()->getAsCXXRecordDecl(); - assert(ObjRecord && "object is expected"); - KAC.traverseRecord(ObjRecord); - } + if (Param->getType()->isRecordType()) + KAC.visit(Param->getType()); } return KAC.isInvalid(); } >From aa45cd19fded6e5298b288bee6e217cef2ce6649 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Thu, 21 May 2026 06:16:08 -0700 Subject: [PATCH 08/16] Make format happy --- clang/lib/Sema/SemaSYCL.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 6d2b3447868ce..c4a925baa710f 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -11,9 +11,9 @@ #include "clang/Sema/SemaSYCL.h" #include "TreeTransform.h" #include "clang/AST/Mangle.h" -#include "clang/AST/SubobjectVisitor.h" #include "clang/AST/SYCLKernelInfo.h" #include "clang/AST/StmtSYCL.h" +#include "clang/AST/SubobjectVisitor.h" #include "clang/AST/TypeOrdering.h" #include "clang/Basic/Diagnostic.h" #include "clang/Sema/Attr.h" >From 74f2460cc0da5379ddec79d1624f0305da600803 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Fri, 22 May 2026 07:06:29 -0700 Subject: [PATCH 09/16] Use getDerived(), eliminate non_ptr_t --- clang/include/clang/AST/SubobjectVisitor.h | 27 ++++++++++------------ clang/lib/Sema/SemaSYCL.cpp | 6 ++--- 2 files changed, 15 insertions(+), 18 deletions(-) diff --git a/clang/include/clang/AST/SubobjectVisitor.h b/clang/include/clang/AST/SubobjectVisitor.h index 0bf70c2a8b6af..22ad184b70b52 100644 --- a/clang/include/clang/AST/SubobjectVisitor.h +++ b/clang/include/clang/AST/SubobjectVisitor.h @@ -26,8 +26,6 @@ template <template <typename> class Ptr, typename Derived> class SubobjectVisitorBase { ASTContext &Ctx; template <typename Class> using ptr_t = typename Ptr<Class>::type; - template <typename Class> - using non_ptr_t = typename std::remove_pointer<ptr_t<Class>>::type; public: SubobjectVisitorBase(ASTContext &Ctx) : Ctx(Ctx) {} @@ -43,40 +41,39 @@ class SubobjectVisitorBase { if (QT->isArrayType()) { QualType ElTy = cast<ConstantArrayType>(Ctx.getAsArrayType(QT))->getElementType(); - visit(ElTy); + getDerived().visit(ElTy); return; } if (ptr_t<RecordDecl> RD = QT->getAsRecordDecl()) { - traverseRecord(RD); + getDerived().traverseRecord(RD); return; } - - visitGenericType(QT.getCanonicalType().getTypePtr()); + getDerived().visitGenericType(QT.getCanonicalType().getTypePtr()); } void traverseRecord(ptr_t<RecordDecl> RD) { if (ptr_t<CXXRecordDecl> CRD = dyn_cast<CXXRecordDecl>(RD)) { - for (non_ptr_t<CXXBaseSpecifier> BS : CRD->bases()) { - if (!static_cast<Derived *>(this)->visitBaseSpecifierPre(BS)) + for (CXXBaseSpecifier& BS : CRD->bases()) { + if (!getDerived().visitBaseSpecifierPre(&BS)) continue; - visit(BS.getType()); - static_cast<Derived *>(this)->visitBaseSpecifierPost(BS); + getDerived().visit(BS.getType()); + getDerived().visitBaseSpecifierPost(&BS); } } for (ptr_t<FieldDecl> FD : RD->fields()) { - if (!static_cast<Derived *>(this)->visitFieldDeclPre(FD)) + if (!getDerived().visitFieldDeclPre(FD)) continue; - visit(FD->getType()); - static_cast<Derived *>(this)->visitFieldDeclPost(FD); + getDerived().visit(FD->getType()); + getDerived().visitFieldDeclPost(FD); } } // Default base class specifier pre-order visitor. - bool visitBaseSpecifierPre(non_ptr_t<CXXBaseSpecifier> BS) { return true; } + bool visitBaseSpecifierPre(ptr_t<CXXBaseSpecifier> BS) { return true; } // Default base class specifier post-order visitor. - void visitBaseSpecifierPost(non_ptr_t<CXXBaseSpecifier> BS) {} + void visitBaseSpecifierPost(ptr_t<CXXBaseSpecifier> BS) {} // Default field pre-order visitor. bool visitFieldDeclPre(ptr_t<FieldDecl> FD) { return true; } diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index c4a925baa710f..7c56bc82e3c10 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -677,8 +677,8 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { : SubobjectVisitor<KernelArgsChecker>(SR.getASTContext()), SemaSYCLRef(SR) {} - bool visitBaseSpecifierPre(CXXBaseSpecifier BS) { - SubobjectAccessPath.push_back(&BS); + bool visitBaseSpecifierPre(CXXBaseSpecifier *BS) { + SubobjectAccessPath.push_back(BS); return true; } @@ -712,7 +712,7 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { } void visitFieldDeclPost(FieldDecl *FD) { SubobjectAccessPath.pop_back(); } - void visitBaseSpecifierPost(CXXBaseSpecifier BS) { + void visitBaseSpecifierPost(CXXBaseSpecifier *BS) { SubobjectAccessPath.pop_back(); } >From 4622ad836a7f88859b3c6e5bfca0d5fc8892ca5e Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Fri, 22 May 2026 09:20:49 -0700 Subject: [PATCH 10/16] Remove "concrete type" visitation --- clang/include/clang/AST/SubobjectVisitor.h | 28 ------- clang/lib/Sema/SemaSYCL.cpp | 95 +++++++++++++++------- 2 files changed, 64 insertions(+), 59 deletions(-) diff --git a/clang/include/clang/AST/SubobjectVisitor.h b/clang/include/clang/AST/SubobjectVisitor.h index 22ad184b70b52..116ecc6692eb9 100644 --- a/clang/include/clang/AST/SubobjectVisitor.h +++ b/clang/include/clang/AST/SubobjectVisitor.h @@ -18,10 +18,6 @@ namespace clang { -#define DISPATCH(CLASS) \ - return static_cast<Derived *>(this)->visit##CLASS( \ - static_cast<const CLASS *>(T)) - template <template <typename> class Ptr, typename Derived> class SubobjectVisitorBase { ASTContext &Ctx; @@ -49,7 +45,6 @@ class SubobjectVisitorBase { getDerived().traverseRecord(RD); return; } - getDerived().visitGenericType(QT.getCanonicalType().getTypePtr()); } void traverseRecord(ptr_t<RecordDecl> RD) { @@ -80,27 +75,6 @@ class SubobjectVisitorBase { // Default field post-order visitor. void visitFieldDeclPost(ptr_t<FieldDecl> FD) {} - bool visitGenericType(const Type *T) { - // Top switch stmt: dispatch to VisitFooType for each FooType. - switch (T->getTypeClass()) { -#define ABSTRACT_TYPE(CLASS, PARENT) -#define TYPE(CLASS, PARENT) \ - case Type::CLASS: \ - DISPATCH(CLASS##Type); -#include "clang/AST/TypeNodes.inc" - } - llvm_unreachable("Unknown type class!"); - } - - // If the implementation chooses not to implement a certain visit method, fall - // back on superclass. -#define TYPE(CLASS, PARENT) \ - bool visit##CLASS##Type(const CLASS##Type *T) { DISPATCH(PARENT); } -#include "clang/AST/TypeNodes.inc" - - /// Method called if \c ImpClass doesn't provide specific handler - /// for some type class. - bool visitType(const Type *) { return true; } }; template <typename Derived> @@ -119,8 +93,6 @@ class ConstSubobjectVisitor : SubobjectVisitorBase<std::add_pointer, Derived>(Ctx) {} }; -#undef DISPATCH - } // end namespace clang #endif // LLVM_CLANG_AST_SUBOBJECTVISITOR diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 7c56bc82e3c10..c9eadc26d41bf 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -669,51 +669,86 @@ OutlinedFunctionDecl *BuildSYCLKernelEntryPointOutline(Sema &SemaRef, class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { SemaSYCL &SemaSYCLRef; bool IsValid = true; - using SubobjectAccess = llvm::PointerUnion<CXXBaseSpecifier *, FieldDecl *>; - SmallVector<SubobjectAccess, 4> SubobjectAccessPath; + using ObjectAccess = + llvm::PointerUnion<ParmVarDecl *, CXXBaseSpecifier *, FieldDecl *>; + SmallVector<ObjectAccess, 4> ObjectAccessPath; + + void emitObjectAccessPathNotes() { + for (auto Parent : ObjectAccessPath) { + if (auto *FD = Parent.dyn_cast<FieldDecl *>()) { + SemaSYCLRef.Diag(FD->getParent()->getLocation(), + diag::note_within_field_of_type) + << FD->getParent(); + } else if (auto *BS = Parent.dyn_cast<CXXBaseSpecifier *>()) { + CXXRecordDecl *RD = BS->getType()->getAsCXXRecordDecl(); + assert(RD); + SemaSYCLRef.Diag(BS->getBeginLoc(), diag::note_within_base_of_type) + << RD; + } else { + // Nothing to emit for ParmVarDecl since its location just points to + // skep-attributed function template. + assert(isa<ParmVarDecl *>(Parent)); + } + } + } public: KernelArgsChecker(SemaSYCL &SR, SourceLocation Loc) : SubobjectVisitor<KernelArgsChecker>(SR.getASTContext()), SemaSYCLRef(SR) {} + void checkParameter(ParmVarDecl *PVD) { + ObjectAccessPath.push_back(PVD); + // Check the immediate type of the parameter. + if (checkType(PVD->getType())) { + // If type checking wasn't short circuited, visit subobjects to check + // them. + visit(PVD->getType()); + } + ObjectAccessPath.pop_back(); + // We either visited everything or we stopped visiting due to invalid kernel + // argument type. + assert(ObjectAccessPath.empty()); + } + bool visitBaseSpecifierPre(CXXBaseSpecifier *BS) { - SubobjectAccessPath.push_back(BS); - return true; + ObjectAccessPath.push_back(BS); + return checkType(BS->getType()); } bool visitFieldDeclPre(FieldDecl *FD) { - SubobjectAccessPath.push_back(FD); - return true; + ObjectAccessPath.push_back(FD); + return checkType(FD->getType()); } - bool visitReferenceType(const ReferenceType *RT) { - // Reference cannot be a base, so just assume we came via a FieldDecl. - auto *DirectParent = cast<FieldDecl *>(SubobjectAccessPath.back()); - SemaSYCLRef.Diag(DirectParent->getLocation(), - diag::err_bad_kernel_param_type) - << DirectParent->getType(); - - for (auto Parent : SubobjectAccessPath) { - if (auto *FD = Parent.dyn_cast<FieldDecl *>()) { - SemaSYCLRef.Diag(FD->getParent()->getLocation(), - diag::note_within_field_of_type) - << FD->getParent(); - } else { - auto *BS = cast<CXXBaseSpecifier *>(Parent); - CXXRecordDecl *RD = BS->getType()->getAsCXXRecordDecl(); - assert(RD); - SemaSYCLRef.Diag(BS->getBeginLoc(), diag::note_within_base_of_type) - << RD; + // Returns true if subobjects should be visited and false otherwise. + bool checkType(QualType Ty) { + if (Ty->isReferenceType()) { + auto DirectParent = ObjectAccessPath.back(); + // Reference cannot be a base, so just assume we came via a FieldDecl. + if (isa<ParmVarDecl *>(DirectParent)) { + // If reference is a kernel argument, there is nothing to do. We allow + // references in direct kernel arguments for better performance of the + // host code and we eliminate them when building actual kernel. + return true; } + + auto *DirectFieldParent = cast<FieldDecl *>(DirectParent); + SemaSYCLRef.Diag(DirectFieldParent->getLocation(), + diag::err_bad_kernel_param_type) + << DirectFieldParent->getType(); + emitObjectAccessPathNotes(); + + IsValid = false; + ObjectAccessPath.pop_back(); + return false; } - IsValid = false; return true; } - void visitFieldDeclPost(FieldDecl *FD) { SubobjectAccessPath.pop_back(); } + void visitFieldDeclPost(FieldDecl *FD) { ObjectAccessPath.pop_back(); } void visitBaseSpecifierPost(CXXBaseSpecifier *BS) { - SubobjectAccessPath.pop_back(); + ObjectAccessPath.pop_back(); } bool isInvalid() { return !IsValid; } @@ -721,10 +756,8 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { bool verifyKernelArguments(FunctionDecl *FD, SemaSYCL &SemaSYCLRef) { KernelArgsChecker KAC(SemaSYCLRef, FD->getLocation()); - for (auto Param : FD->parameters()) { - if (Param->getType()->isRecordType()) - KAC.visit(Param->getType()); - } + for (auto Param : FD->parameters()) + KAC.checkParameter(Param); return KAC.isInvalid(); } >From 46db74989e52a7646bf9714cfa9782b3b5609265 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Tue, 26 May 2026 02:43:22 -0700 Subject: [PATCH 11/16] Fix constvisitor --- clang/include/clang/AST/SubobjectVisitor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/AST/SubobjectVisitor.h b/clang/include/clang/AST/SubobjectVisitor.h index 116ecc6692eb9..d56d905454a5d 100644 --- a/clang/include/clang/AST/SubobjectVisitor.h +++ b/clang/include/clang/AST/SubobjectVisitor.h @@ -90,7 +90,7 @@ class ConstSubobjectVisitor : public SubobjectVisitorBase<llvm::make_const_ptr, Derived> { public: ConstSubobjectVisitor(ASTContext &Ctx) - : SubobjectVisitorBase<std::add_pointer, Derived>(Ctx) {} + : SubobjectVisitorBase<llvm::make_const_ptr, Derived>(Ctx) {} }; } // end namespace clang >From dc7b5a9f41c62f767f49b28ff1226b649104a220 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Tue, 26 May 2026 02:54:56 -0700 Subject: [PATCH 12/16] Enter post visitation even if pre visitation failed --- clang/include/clang/AST/SubobjectVisitor.h | 10 ++++------ clang/lib/Sema/SemaSYCL.cpp | 3 --- 2 files changed, 4 insertions(+), 9 deletions(-) diff --git a/clang/include/clang/AST/SubobjectVisitor.h b/clang/include/clang/AST/SubobjectVisitor.h index d56d905454a5d..bb36b8e217aec 100644 --- a/clang/include/clang/AST/SubobjectVisitor.h +++ b/clang/include/clang/AST/SubobjectVisitor.h @@ -50,16 +50,14 @@ class SubobjectVisitorBase { void traverseRecord(ptr_t<RecordDecl> RD) { if (ptr_t<CXXRecordDecl> CRD = dyn_cast<CXXRecordDecl>(RD)) { for (CXXBaseSpecifier& BS : CRD->bases()) { - if (!getDerived().visitBaseSpecifierPre(&BS)) - continue; - getDerived().visit(BS.getType()); + if (getDerived().visitBaseSpecifierPre(&BS)) + getDerived().visit(BS.getType()); getDerived().visitBaseSpecifierPost(&BS); } } for (ptr_t<FieldDecl> FD : RD->fields()) { - if (!getDerived().visitFieldDeclPre(FD)) - continue; - getDerived().visit(FD->getType()); + if (getDerived().visitFieldDeclPre(FD)) + getDerived().visit(FD->getType()); getDerived().visitFieldDeclPost(FD); } } diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index c9eadc26d41bf..58ab6537a462d 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -706,8 +706,6 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { visit(PVD->getType()); } ObjectAccessPath.pop_back(); - // We either visited everything or we stopped visiting due to invalid kernel - // argument type. assert(ObjectAccessPath.empty()); } @@ -740,7 +738,6 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { emitObjectAccessPathNotes(); IsValid = false; - ObjectAccessPath.pop_back(); return false; } return true; >From 627848eb6f7ed5267f881db4ac07cf2d8f1a7f33 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Tue, 26 May 2026 03:06:26 -0700 Subject: [PATCH 13/16] Const it --- clang/include/clang/AST/SubobjectVisitor.h | 4 +++- clang/lib/Sema/SemaSYCL.cpp | 27 +++++++++++----------- 2 files changed, 17 insertions(+), 14 deletions(-) diff --git a/clang/include/clang/AST/SubobjectVisitor.h b/clang/include/clang/AST/SubobjectVisitor.h index bb36b8e217aec..ffd14b95d7dec 100644 --- a/clang/include/clang/AST/SubobjectVisitor.h +++ b/clang/include/clang/AST/SubobjectVisitor.h @@ -22,6 +22,8 @@ template <template <typename> class Ptr, typename Derived> class SubobjectVisitorBase { ASTContext &Ctx; template <typename Class> using ptr_t = typename Ptr<Class>::type; + template <typename Class> + using non_ptr_t = typename std::remove_pointer<ptr_t<Class>>::type; public: SubobjectVisitorBase(ASTContext &Ctx) : Ctx(Ctx) {} @@ -49,7 +51,7 @@ class SubobjectVisitorBase { void traverseRecord(ptr_t<RecordDecl> RD) { if (ptr_t<CXXRecordDecl> CRD = dyn_cast<CXXRecordDecl>(RD)) { - for (CXXBaseSpecifier& BS : CRD->bases()) { + for (non_ptr_t<CXXBaseSpecifier&> BS : CRD->bases()) { if (getDerived().visitBaseSpecifierPre(&BS)) getDerived().visit(BS.getType()); getDerived().visitBaseSpecifierPost(&BS); diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index 58ab6537a462d..a39c29cd45798 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -666,20 +666,21 @@ OutlinedFunctionDecl *BuildSYCLKernelEntryPointOutline(Sema &SemaRef, return OFD; } -class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { +class KernelArgsChecker : public ConstSubobjectVisitor<KernelArgsChecker> { SemaSYCL &SemaSYCLRef; bool IsValid = true; using ObjectAccess = - llvm::PointerUnion<ParmVarDecl *, CXXBaseSpecifier *, FieldDecl *>; + llvm::PointerUnion<const ParmVarDecl *, const CXXBaseSpecifier *, + const FieldDecl *>; SmallVector<ObjectAccess, 4> ObjectAccessPath; void emitObjectAccessPathNotes() { for (auto Parent : ObjectAccessPath) { - if (auto *FD = Parent.dyn_cast<FieldDecl *>()) { + if (auto *FD = Parent.dyn_cast<const FieldDecl *>()) { SemaSYCLRef.Diag(FD->getParent()->getLocation(), diag::note_within_field_of_type) << FD->getParent(); - } else if (auto *BS = Parent.dyn_cast<CXXBaseSpecifier *>()) { + } else if (auto *BS = Parent.dyn_cast<const CXXBaseSpecifier *>()) { CXXRecordDecl *RD = BS->getType()->getAsCXXRecordDecl(); assert(RD); SemaSYCLRef.Diag(BS->getBeginLoc(), diag::note_within_base_of_type) @@ -687,17 +688,17 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { } else { // Nothing to emit for ParmVarDecl since its location just points to // skep-attributed function template. - assert(isa<ParmVarDecl *>(Parent)); + assert(isa<const ParmVarDecl *>(Parent)); } } } public: KernelArgsChecker(SemaSYCL &SR, SourceLocation Loc) - : SubobjectVisitor<KernelArgsChecker>(SR.getASTContext()), + : ConstSubobjectVisitor<KernelArgsChecker>(SR.getASTContext()), SemaSYCLRef(SR) {} - void checkParameter(ParmVarDecl *PVD) { + void checkParameter(const ParmVarDecl *PVD) { ObjectAccessPath.push_back(PVD); // Check the immediate type of the parameter. if (checkType(PVD->getType())) { @@ -709,12 +710,12 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { assert(ObjectAccessPath.empty()); } - bool visitBaseSpecifierPre(CXXBaseSpecifier *BS) { + bool visitBaseSpecifierPre(const CXXBaseSpecifier *BS) { ObjectAccessPath.push_back(BS); return checkType(BS->getType()); } - bool visitFieldDeclPre(FieldDecl *FD) { + bool visitFieldDeclPre(const FieldDecl *FD) { ObjectAccessPath.push_back(FD); return checkType(FD->getType()); } @@ -724,14 +725,14 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { if (Ty->isReferenceType()) { auto DirectParent = ObjectAccessPath.back(); // Reference cannot be a base, so just assume we came via a FieldDecl. - if (isa<ParmVarDecl *>(DirectParent)) { + if (isa<const ParmVarDecl *>(DirectParent)) { // If reference is a kernel argument, there is nothing to do. We allow // references in direct kernel arguments for better performance of the // host code and we eliminate them when building actual kernel. return true; } - auto *DirectFieldParent = cast<FieldDecl *>(DirectParent); + auto *DirectFieldParent = cast<const FieldDecl *>(DirectParent); SemaSYCLRef.Diag(DirectFieldParent->getLocation(), diag::err_bad_kernel_param_type) << DirectFieldParent->getType(); @@ -743,8 +744,8 @@ class KernelArgsChecker : public SubobjectVisitor<KernelArgsChecker> { return true; } - void visitFieldDeclPost(FieldDecl *FD) { ObjectAccessPath.pop_back(); } - void visitBaseSpecifierPost(CXXBaseSpecifier *BS) { + void visitFieldDeclPost(const FieldDecl *FD) { ObjectAccessPath.pop_back(); } + void visitBaseSpecifierPost(const CXXBaseSpecifier *BS) { ObjectAccessPath.pop_back(); } >From 3730ffce79e368b43c39ee037634c2dd30d9d7da Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Tue, 26 May 2026 06:16:08 -0700 Subject: [PATCH 14/16] Organize the test --- .../SemaSYCL/sycl-kernel-arg-restrictions.cpp | 89 ++++++++++++------- 1 file changed, 59 insertions(+), 30 deletions(-) diff --git a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp b/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp index 47ca74d1642a3..2f7681de9c74e 100644 --- a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp +++ b/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp @@ -13,35 +13,9 @@ template<typename KNT, typename T> [[clang::sycl_kernel_entry_point(KNT)]] void kernel_single_task(T) {} -struct S { // expected-note 3{{within field of type 'S' declared here}} - int a; - int &b; //expected-error 3{{'int &' cannot be used as the type of a kernel parameter}} -}; - -void fooarr(int (&arr)[5]) { -} - -template <typename T> class Callable { // expected-note 2{{within field of type 'Callable<int &>' declared here}} - T data; // expected-error 2{{'int &' cannot be used as the type of a kernel parameter}} -public: - Callable(T d) : data(d) {} - void operator()() { - } -}; - -class Derived1 : Callable<int> { // expected-note {{within field of type 'Derived1' declared here}} - int &a; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} -public: - Derived1(int d, int &b) : Callable<int>(d), a(b) {} -}; - -class Derived2 : Callable<int&> { // expected-note {{within base of type 'Callable<int &>' declared here}} - int a; -public: - Derived2(int d, int &b) : Callable<int&>(b), a(d) {} -}; - -void refCases(int AS) { +// Check that reference captures of kernel that defined as lambda are diagnosed. +namespace badref1 { +void test() { int p = 0; double q = 0; float s = 0; @@ -55,8 +29,19 @@ void refCases(int AS) { (void)p; (void)s; }); +} +} // namespace badref1 + +// Check reference kernel arguments witin structs or lambdas; +namespace badref2 { +struct S { // expected-note 2{{within field of type 'S' declared here}} + int a; + int &b; //expected-error 2{{'int &' cannot be used as the type of a kernel parameter}} +}; - auto L = [&]() { (void)p;}; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} +void test() { + int p = 0; + auto L = [&]() { (void)p;}; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} // expected-note@-1 {{within field of type}} S Str {p, p}; kernel_single_task<class KN<2>>( // expected-note {{requested here}} @@ -70,6 +55,22 @@ void refCases(int AS) { (void)Str; }); +} +} // namespace badref2 + +// Check references within array kernel arguments. +namespace badref3 { +struct S { // expected-note {{within field of type 'S' declared here}} + int a; + int &b; //expected-error {{'int &' cannot be used as the type of a kernel parameter}} +}; + +void fooarr(int (&arr)[5]) { +} + +void test(int AS) { + int p = 0; + S Str {p, p}; S arr[2] = {Str, Str}; kernel_single_task<class KN<4>>( // expected-note {{requested here}} [=] { // expected-note {{within field of type}} @@ -85,9 +86,37 @@ void refCases(int AS) { [&] { // expected-note {{within field of type}} fooarr(arrayints); // expected-error {{'int (&)[5]' cannot be used as the type of a kernel parameter}} }); +} +} // namespace badref3 + +// Check callable objects containing references. +namespace badref4 { +template <typename T> class Callable { // expected-note 2{{within field of type 'Callable<int &>' declared here}} + T data; // expected-error 2{{'int &' cannot be used as the type of a kernel parameter}} +public: + Callable(T d) : data(d) {} + void operator()() { + } +}; + +class Derived1 : Callable<int> { // expected-note {{within field of type 'Derived1' declared here}} + int &a; // expected-error {{'int &' cannot be used as the type of a kernel parameter}} +public: + Derived1(int d, int &b) : Callable<int>(d), a(b) {} +}; + +class Derived2 : Callable<int&> { // expected-note {{within base of type 'Callable<int &>' declared here}} + int a; +public: + Derived2(int d, int &b) : Callable<int&>(b), a(d) {} +}; + +void test(int AS) { + int p = 0; kernel_single_task<class KN<8>>(Callable<int&>{p}); // expected-note {{requested here}} kernel_single_task<class KN<9>>(Callable<int>{p}); kernel_single_task<class KN<10>>(Derived1{p, p}); // expected-note {{requested here}} kernel_single_task<class KN<11>>(Derived2{p, p}); // expected-note {{requested here}} } +} // namespace badref4 >From e2394464623225cec70d8684f85ddacd8997e5ec Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Wed, 27 May 2026 07:53:08 -0700 Subject: [PATCH 15/16] Make format happy --- clang/include/clang/AST/SubobjectVisitor.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/clang/include/clang/AST/SubobjectVisitor.h b/clang/include/clang/AST/SubobjectVisitor.h index ffd14b95d7dec..fdf1c5c681356 100644 --- a/clang/include/clang/AST/SubobjectVisitor.h +++ b/clang/include/clang/AST/SubobjectVisitor.h @@ -51,7 +51,7 @@ class SubobjectVisitorBase { void traverseRecord(ptr_t<RecordDecl> RD) { if (ptr_t<CXXRecordDecl> CRD = dyn_cast<CXXRecordDecl>(RD)) { - for (non_ptr_t<CXXBaseSpecifier&> BS : CRD->bases()) { + for (non_ptr_t<CXXBaseSpecifier &> BS : CRD->bases()) { if (getDerived().visitBaseSpecifierPre(&BS)) getDerived().visit(BS.getType()); getDerived().visitBaseSpecifierPost(&BS); >From c3d8438e5f43d454835753a1026cf170970ee433 Mon Sep 17 00:00:00 2001 From: "Podchishchaeva, Mariya" <[email protected]> Date: Thu, 28 May 2026 07:02:07 -0700 Subject: [PATCH 16/16] Arguments -> parameters --- clang/lib/Sema/SemaSYCL.cpp | 16 ++++++++-------- ...ns.cpp => sycl-kernel-param-restrictions.cpp} | 4 ++-- 2 files changed, 10 insertions(+), 10 deletions(-) rename clang/test/SemaSYCL/{sycl-kernel-arg-restrictions.cpp => sycl-kernel-param-restrictions.cpp} (97%) diff --git a/clang/lib/Sema/SemaSYCL.cpp b/clang/lib/Sema/SemaSYCL.cpp index a39c29cd45798..5e966bc7103d7 100644 --- a/clang/lib/Sema/SemaSYCL.cpp +++ b/clang/lib/Sema/SemaSYCL.cpp @@ -666,7 +666,7 @@ OutlinedFunctionDecl *BuildSYCLKernelEntryPointOutline(Sema &SemaRef, return OFD; } -class KernelArgsChecker : public ConstSubobjectVisitor<KernelArgsChecker> { +class KernelParamsChecker : public ConstSubobjectVisitor<KernelParamsChecker> { SemaSYCL &SemaSYCLRef; bool IsValid = true; using ObjectAccess = @@ -694,8 +694,8 @@ class KernelArgsChecker : public ConstSubobjectVisitor<KernelArgsChecker> { } public: - KernelArgsChecker(SemaSYCL &SR, SourceLocation Loc) - : ConstSubobjectVisitor<KernelArgsChecker>(SR.getASTContext()), + KernelParamsChecker(SemaSYCL &SR, SourceLocation Loc) + : ConstSubobjectVisitor<KernelParamsChecker>(SR.getASTContext()), SemaSYCLRef(SR) {} void checkParameter(const ParmVarDecl *PVD) { @@ -726,8 +726,8 @@ class KernelArgsChecker : public ConstSubobjectVisitor<KernelArgsChecker> { auto DirectParent = ObjectAccessPath.back(); // Reference cannot be a base, so just assume we came via a FieldDecl. if (isa<const ParmVarDecl *>(DirectParent)) { - // If reference is a kernel argument, there is nothing to do. We allow - // references in direct kernel arguments for better performance of the + // If reference is a kernel parameter, there is nothing to do. We allow + // references in direct kernel parameters for better performance of the // host code and we eliminate them when building actual kernel. return true; } @@ -752,8 +752,8 @@ class KernelArgsChecker : public ConstSubobjectVisitor<KernelArgsChecker> { bool isInvalid() { return !IsValid; } }; -bool verifyKernelArguments(FunctionDecl *FD, SemaSYCL &SemaSYCLRef) { - KernelArgsChecker KAC(SemaSYCLRef, FD->getLocation()); +bool verifyKernelParams(FunctionDecl *FD, SemaSYCL &SemaSYCLRef) { + KernelParamsChecker KAC(SemaSYCLRef, FD->getLocation()); for (auto Param : FD->parameters()) KAC.checkParameter(Param); return KAC.isInvalid(); @@ -784,7 +784,7 @@ StmtResult SemaSYCL::BuildSYCLKernelCallStmt(FunctionDecl *FD, getASTContext().getSYCLKernelInfo(SKEPAttr->getKernelName()); assert(declaresSameEntity(SKI.getKernelEntryPointDecl(), FD) && "SYCL kernel name conflict"); - if (verifyKernelArguments(FD, *this)) + if (verifyKernelParams(FD, *this)) return StmtError(); // Build the outline of the synthesized device entry point function. diff --git a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp b/clang/test/SemaSYCL/sycl-kernel-param-restrictions.cpp similarity index 97% rename from clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp rename to clang/test/SemaSYCL/sycl-kernel-param-restrictions.cpp index 2f7681de9c74e..5d549af450f32 100644 --- a/clang/test/SemaSYCL/sycl-kernel-arg-restrictions.cpp +++ b/clang/test/SemaSYCL/sycl-kernel-param-restrictions.cpp @@ -32,7 +32,7 @@ void test() { } } // namespace badref1 -// Check reference kernel arguments witin structs or lambdas; +// Check reference kernel parameters witin structs or lambdas; namespace badref2 { struct S { // expected-note 2{{within field of type 'S' declared here}} int a; @@ -58,7 +58,7 @@ void test() { } } // namespace badref2 -// Check references within array kernel arguments. +// Check references within array kernel parameters. namespace badref3 { struct S { // expected-note {{within field of type 'S' declared here}} int a; _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
