[LLVM][Clang] Enable strict mode for getTrailingObjects (#144930)

Disallow calls to templated `getTrailingObjects` if there is a single
trailing type (strict mode). Add `getTrailingObjectsNonStrict` for cases
when it's not possible to know statically if there will be a single or
multiple trailing types (like in OpenMPClause.h) to bypass the struct
checks.

This will ensure that future users of TrailingObjects class do not
accidently use the templated `getTrailingObjects` when they have a
single trailing type.
This commit is contained in:
Rahul Joshi
2025-06-30 07:23:34 -07:00
committed by GitHub
parent b0ff473340
commit d1ba2692ee
4 changed files with 74 additions and 28 deletions

View File

@@ -295,7 +295,8 @@ protected:
/// Fetches list of variables associated with this clause.
MutableArrayRef<Expr *> getVarRefs() {
return static_cast<T *>(this)->template getTrailingObjects<Expr *>(NumVars);
return static_cast<T *>(this)->template getTrailingObjectsNonStrict<Expr *>(
NumVars);
}
/// Sets the list of variables for this clause.
@@ -334,8 +335,8 @@ public:
/// Fetches list of all variables in the clause.
ArrayRef<const Expr *> getVarRefs() const {
return static_cast<const T *>(this)->template getTrailingObjects<Expr *>(
NumVars);
return static_cast<const T *>(this)
->template getTrailingObjectsNonStrict<Expr *>(NumVars);
}
};
@@ -380,7 +381,7 @@ public:
MutableArrayRef<OpenMPDirectiveKind> getDirectiveKinds() {
return static_cast<T *>(this)
->template getTrailingObjects<OpenMPDirectiveKind>(NumKinds);
->template getTrailingObjectsNonStrict<OpenMPDirectiveKind>(NumKinds);
}
void setDirectiveKinds(ArrayRef<OpenMPDirectiveKind> DK) {
@@ -5921,7 +5922,8 @@ protected:
/// Get the unique declarations that are in the trailing objects of the
/// class.
MutableArrayRef<ValueDecl *> getUniqueDeclsRef() {
return static_cast<T *>(this)->template getTrailingObjects<ValueDecl *>(
return static_cast<T *>(this)
->template getTrailingObjectsNonStrict<ValueDecl *>(
NumUniqueDeclarations);
}
@@ -5929,7 +5931,8 @@ protected:
/// class.
ArrayRef<ValueDecl *> getUniqueDeclsRef() const {
return static_cast<const T *>(this)
->template getTrailingObjects<ValueDecl *>(NumUniqueDeclarations);
->template getTrailingObjectsNonStrict<ValueDecl *>(
NumUniqueDeclarations);
}
/// Set the unique declarations that are in the trailing objects of the
@@ -5943,15 +5946,15 @@ protected:
/// Get the number of lists per declaration that are in the trailing
/// objects of the class.
MutableArrayRef<unsigned> getDeclNumListsRef() {
return static_cast<T *>(this)->template getTrailingObjects<unsigned>(
NumUniqueDeclarations);
return static_cast<T *>(this)
->template getTrailingObjectsNonStrict<unsigned>(NumUniqueDeclarations);
}
/// Get the number of lists per declaration that are in the trailing
/// objects of the class.
ArrayRef<unsigned> getDeclNumListsRef() const {
return static_cast<const T *>(this)->template getTrailingObjects<unsigned>(
NumUniqueDeclarations);
return static_cast<const T *>(this)
->template getTrailingObjectsNonStrict<unsigned>(NumUniqueDeclarations);
}
/// Set the number of lists per declaration that are in the trailing
@@ -5966,7 +5969,8 @@ protected:
/// objects of the class. They are appended after the number of lists.
MutableArrayRef<unsigned> getComponentListSizesRef() {
return MutableArrayRef<unsigned>(
static_cast<T *>(this)->template getTrailingObjects<unsigned>() +
static_cast<T *>(this)
->template getTrailingObjectsNonStrict<unsigned>() +
NumUniqueDeclarations,
NumComponentLists);
}
@@ -5975,7 +5979,8 @@ protected:
/// objects of the class. They are appended after the number of lists.
ArrayRef<unsigned> getComponentListSizesRef() const {
return ArrayRef<unsigned>(
static_cast<const T *>(this)->template getTrailingObjects<unsigned>() +
static_cast<const T *>(this)
->template getTrailingObjectsNonStrict<unsigned>() +
NumUniqueDeclarations,
NumComponentLists);
}
@@ -5991,13 +5996,15 @@ protected:
/// Get the components that are in the trailing objects of the class.
MutableArrayRef<MappableComponent> getComponentsRef() {
return static_cast<T *>(this)
->template getTrailingObjects<MappableComponent>(NumComponents);
->template getTrailingObjectsNonStrict<MappableComponent>(
NumComponents);
}
/// Get the components that are in the trailing objects of the class.
ArrayRef<MappableComponent> getComponentsRef() const {
return static_cast<const T *>(this)
->template getTrailingObjects<MappableComponent>(NumComponents);
->template getTrailingObjectsNonStrict<MappableComponent>(
NumComponents);
}
/// Set the components that are in the trailing objects of the class.

View File

@@ -2020,7 +2020,8 @@ CXXBaseSpecifier **CastExpr::path_buffer() {
#define ABSTRACT_STMT(x)
#define CASTEXPR(Type, Base) \
case Stmt::Type##Class: \
return static_cast<Type *>(this)->getTrailingObjects<CXXBaseSpecifier *>();
return static_cast<Type *>(this) \
->getTrailingObjectsNonStrict<CXXBaseSpecifier *>();
#define STMT(Type, Base)
#include "clang/AST/StmtNodes.inc"
default:

View File

@@ -228,12 +228,18 @@ class TrailingObjects : private trailing_objects_internal::TrailingObjectsImpl<
using ParentType::getTrailingObjectsImpl;
// This function contains only a static_assert BaseTy is final. The
// static_assert must be in a function, and not at class-level
// because BaseTy isn't complete at class instantiation time, but
// will be by the time this function is instantiated.
static void verifyTrailingObjectsAssertions() {
template <bool Strict> static void verifyTrailingObjectsAssertions() {
// The static_assert for BaseTy must be in a function, and not at
// class-level because BaseTy isn't complete at class instantiation time,
// but will be by the time this function is instantiated.
static_assert(std::is_final<BaseTy>(), "BaseTy must be final.");
// Verify that templated getTrailingObjects() is used only with multiple
// trailing types. Use getTrailingObjectsNonStrict() which does not check
// this.
static_assert(!Strict || sizeof...(TrailingTys) > 1,
"Use templated getTrailingObjects() only when there are "
"multiple trailing types");
}
// These two methods are the base of the recursion for this method.
@@ -283,7 +289,7 @@ public:
/// (which must be one of those specified in the class template). The
/// array may have zero or more elements in it.
template <typename T> const T *getTrailingObjects() const {
verifyTrailingObjectsAssertions();
verifyTrailingObjectsAssertions<true>();
// Forwards to an impl function with overloads, since member
// function templates can't be specialized.
return this->getTrailingObjectsImpl(
@@ -295,7 +301,7 @@ public:
/// (which must be one of those specified in the class template). The
/// array may have zero or more elements in it.
template <typename T> T *getTrailingObjects() {
verifyTrailingObjectsAssertions();
verifyTrailingObjectsAssertions<true>();
// Forwards to an impl function with overloads, since member
// function templates can't be specialized.
return this->getTrailingObjectsImpl(
@@ -310,14 +316,20 @@ public:
static_assert(sizeof...(TrailingTys) == 1,
"Can use non-templated getTrailingObjects() only when there "
"is a single trailing type");
return getTrailingObjects<FirstTrailingType>();
verifyTrailingObjectsAssertions<false>();
return this->getTrailingObjectsImpl(
static_cast<const BaseTy *>(this),
TrailingObjectsBase::OverloadToken<FirstTrailingType>());
}
FirstTrailingType *getTrailingObjects() {
static_assert(sizeof...(TrailingTys) == 1,
"Can use non-templated getTrailingObjects() only when there "
"is a single trailing type");
return getTrailingObjects<FirstTrailingType>();
verifyTrailingObjectsAssertions<false>();
return this->getTrailingObjectsImpl(
static_cast<BaseTy *>(this),
TrailingObjectsBase::OverloadToken<FirstTrailingType>());
}
// Functions that return the trailing objects as ArrayRefs.
@@ -337,6 +349,31 @@ public:
return ArrayRef(getTrailingObjects(), N);
}
// Non-strict forms of templated `getTrailingObjects` that work with single
// trailing type.
template <typename T> const T *getTrailingObjectsNonStrict() const {
verifyTrailingObjectsAssertions<false>();
return this->getTrailingObjectsImpl(
static_cast<const BaseTy *>(this),
TrailingObjectsBase::OverloadToken<T>());
}
template <typename T> T *getTrailingObjectsNonStrict() {
verifyTrailingObjectsAssertions<false>();
return this->getTrailingObjectsImpl(
static_cast<BaseTy *>(this), TrailingObjectsBase::OverloadToken<T>());
}
template <typename T>
MutableArrayRef<T> getTrailingObjectsNonStrict(size_t N) {
return MutableArrayRef(getTrailingObjectsNonStrict<T>(), N);
}
template <typename T>
ArrayRef<T> getTrailingObjectsNonStrict(size_t N) const {
return ArrayRef(getTrailingObjectsNonStrict<T>(), N);
}
/// Returns the size of the trailing data, if an object were
/// allocated with the given counts (The counts are in the same order
/// as the template arguments). This does not include the size of the

View File

@@ -45,9 +45,10 @@ public:
template <typename... Ty>
using FixedSizeStorage = TrailingObjects::FixedSizeStorage<Ty...>;
using TrailingObjects::totalSizeToAlloc;
using TrailingObjects::additionalSizeToAlloc;
using TrailingObjects::getTrailingObjects;
using TrailingObjects::getTrailingObjectsNonStrict;
using TrailingObjects::totalSizeToAlloc;
};
// Here, there are two singular optional object types appended. Note
@@ -123,11 +124,11 @@ TEST(TrailingObjects, OneArg) {
EXPECT_EQ(Class1::totalSizeToAlloc<short>(3),
sizeof(Class1) + sizeof(short) * 3);
EXPECT_EQ(C->getTrailingObjects<short>(), reinterpret_cast<short *>(C + 1));
EXPECT_EQ(C->getTrailingObjects(), reinterpret_cast<short *>(C + 1));
EXPECT_EQ(C->get(0), 1);
EXPECT_EQ(C->get(2), 3);
EXPECT_EQ(C->getTrailingObjects(), C->getTrailingObjects<short>());
EXPECT_EQ(C->getTrailingObjects(), C->getTrailingObjectsNonStrict<short>());
delete C;
}