[LLVM][Support] Add getTrailingObjects() for single trailing type

[jurahul]

Add a specialization of getTrailingObjects() for a single trailing type. This is a common case and with the specialization you don't need to specify the single trailing type redundantly. Also add an overload for getTrailingObjects which takes size and returns an ArryaRef/MutableArrayRef as that's a common use case as well.

[llvmbot]

@llvm/pr-subscribers-llvm-support

Author: Rahul Joshi (jurahul)

Changes

Add a specialization of getTrailingObjects() for a single trailing type. This is a common case and with the specialization you don't need to specify the single trailing type redundantly. Also add an overload for getTrailingObjects which takes size and returns an ArryaRef/MutableArrayRef as that's a common use case as well.

---

Full diff: https://github.com/llvm/llvm-project/pull/138970.diff

2 Files Affected:

• (modified) llvm/include/llvm/Support/TrailingObjects.h (+36)
• (modified) llvm/unittests/Support/TrailingObjectsTest.cpp (+18-18)

diff --git a/llvm/include/llvm/Support/TrailingObjects.h b/llvm/include/llvm/Support/TrailingObjects.h
index 07cf08df45a6a..02c7ccf911aa8 100644
--- a/llvm/include/llvm/Support/TrailingObjects.h
+++ b/llvm/include/llvm/Support/TrailingObjects.h
@@ -46,6 +46,7 @@
 #ifndef LLVM_SUPPORT_TRAILINGOBJECTS_H
 #define LLVM_SUPPORT_TRAILINGOBJECTS_H
 
+#include "llvm/ADT/ArrayRef.h"
 #include "llvm/Support/Alignment.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/MathExtras.h"
@@ -301,6 +302,41 @@ class TrailingObjects : private trailing_objects_internal::TrailingObjectsImpl<
         static_cast<BaseTy *>(this), TrailingObjectsBase::OverloadToken<T>());
   }
 
+  // getTrailingObjects() specialization for a single trailing type.
+  using FirstTrailingType =
+      typename std::tuple_element_t<0, std::tuple<TrailingTys...>>;
+
+  const FirstTrailingType *getTrailingObjects() const {
+    static_assert(sizeof...(TrailingTys) == 1,
+                  "Can use non-templated getTrailingObjects() only when there "
+                  "is a single trailing type");
+    return getTrailingObjects<FirstTrailingType>();
+  }
+
+  FirstTrailingType *getTrailingObjects() {
+    static_assert(sizeof...(TrailingTys) == 1,
+                  "Can use non-templated getTrailingObjects() only when there "
+                  "is a single trailing type");
+    return getTrailingObjects<FirstTrailingType>();
+  }
+
+  // Functions that return the trailing objects as ArrayRefs.
+  template <typename T> MutableArrayRef<T> getTrailingObjects(size_t N) {
+    return {getTrailingObjects<T>(), N};
+  }
+
+  template <typename T> ArrayRef<T> getTrailingObjects(size_t N) const {
+    return {getTrailingObjects<T>(), N};
+  }
+
+  MutableArrayRef<FirstTrailingType> getTrailingObjects(size_t N) {
+    return {getTrailingObjects(), N};
+  }
+
+  ArrayRef<FirstTrailingType> getTrailingObjects(size_t N) const {
+    return {getTrailingObjects(), 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
diff --git a/llvm/unittests/Support/TrailingObjectsTest.cpp b/llvm/unittests/Support/TrailingObjectsTest.cpp
index e36979e75d7f7..c461301207db0 100644
--- a/llvm/unittests/Support/TrailingObjectsTest.cpp
+++ b/llvm/unittests/Support/TrailingObjectsTest.cpp
@@ -14,19 +14,19 @@
 using namespace llvm;
 
 namespace {
-// This class, beyond being used by the test case, a nice
-// demonstration of the intended usage of TrailingObjects, with a
-// single trailing array.
-class Class1 final : protected TrailingObjects<Class1, short> {
+// This class, beyond being used by the test case, a nice demonstration of the
+// intended usage of TrailingObjects, with a single trailing array.
+class Class1 final : private TrailingObjects<Class1, short> {
   friend TrailingObjects;
 
   unsigned NumShorts;
 
 protected:
-  size_t numTrailingObjects(OverloadToken<short>) const { return NumShorts; }
-
   Class1(ArrayRef<int> ShortArray) : NumShorts(ShortArray.size()) {
-    llvm::copy(ShortArray, getTrailingObjects<short>());
+    // This tests the non-templated getTrailingObjects() when using a single
+    // trailing type.
+    llvm::copy(ShortArray, getTrailingObjects());
+    EXPECT_EQ(getTrailingObjects(), getTrailingObjects<short>());
   }
 
 public:
@@ -36,7 +36,8 @@ class Class1 final : protected TrailingObjects<Class1, short> {
   }
   void operator delete(void *Ptr) { ::operator delete(Ptr); }
 
-  short get(unsigned Num) const { return getTrailingObjects<short>()[Num]; }
+  // This indexes into the ArrayRef<> returned by `getTrailingObjects`.
+  short get(unsigned Num) const { return getTrailingObjects(NumShorts)[Num]; }
 
   unsigned numShorts() const { return NumShorts; }
 
@@ -49,9 +50,9 @@ class Class1 final : protected TrailingObjects<Class1, short> {
   using TrailingObjects::getTrailingObjects;
 };
 
-// Here, there are two singular optional object types appended.  Note
-// that the alignment of Class2 is automatically increased to account
-// for the alignment requirements of the trailing objects.
+// Here, there are two singular optional object types appended. Note that the
+// alignment of Class2 is automatically increased to account for the alignment
+// requirements of the trailing objects.
 class Class2 final : protected TrailingObjects<Class2, double, short> {
   friend TrailingObjects;
 
@@ -172,10 +173,9 @@ TEST(TrailingObjects, TwoArg) {
   delete C2;
 }
 
-// This test class is not trying to be a usage demo, just asserting
-// that three args does actually work too (it's the same code as
-// handles the second arg, so it's basically covered by the above, but
-// just in case..)
+// This test class is not trying to be a usage demo, just asserting that three
+// args does actually work too (it's the same code as handles the second arg, so
+// it's basically covered by the above, but just in case..)
 class Class3 final : public TrailingObjects<Class3, double, short, bool> {
   friend TrailingObjects;
 
@@ -237,9 +237,9 @@ TEST(TrailingObjects, Realignment) {
 }
 }
 
-// Test the use of TrailingObjects with a template class. This
-// previously failed to compile due to a bug in MSVC's member access
-// control/lookup handling for OverloadToken.
+// Test the use of TrailingObjects with a template class. This previously failed
+// to compile due to a bug in MSVC's member access control/lookup handling for
+// OverloadToken.
 template <typename Derived>
 class Class5Tmpl : private llvm::TrailingObjects<Derived, float, int> {
   using TrailingObjects = typename llvm::TrailingObjects<Derived, float>;

[jurahul]

Thanks

[fmayer]

unrequest changes

[llvm-ci]

LLVM Buildbot has detected a new failure on builder llvm-clang-aarch64-darwin running on doug-worker-4 while building llvm at step 6 "test-build-unified-tree-check-all".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/190/builds/19745

Here is the relevant piece of the build log for the reference

Step 6 (test-build-unified-tree-check-all) failure: test (failure)
******************** TEST 'Clang-Unit :: ./AllClangUnitTests/14/48' FAILED ********************
Script(shard):
--
GTEST_OUTPUT=json:/Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/unittests/./AllClangUnitTests-Clang-Unit-94132-14-48.json GTEST_SHUFFLE=0 GTEST_TOTAL_SHARDS=48 GTEST_SHARD_INDEX=14 /Users/buildbot/buildbot-root/aarch64-darwin/build/tools/clang/unittests/./AllClangUnitTests
--

Note: This is test shard 15 of 48.
[==========] Running 510 tests from 105 test suites.
[----------] Global test environment set-up.
[----------] 1 test from MinimizeSourceToDependencyDirectivesTest
[ RUN      ] MinimizeSourceToDependencyDirectivesTest.DefineMultilineArgsStringize
[       OK ] MinimizeSourceToDependencyDirectivesTest.DefineMultilineArgsStringize (2 ms)
[----------] 1 test from MinimizeSourceToDependencyDirectivesTest (2 ms total)

[----------] 1 test from HeaderSearchTest
[ RUN      ] HeaderSearchTest.Dots
[       OK ] HeaderSearchTest.Dots (4 ms)
[----------] 1 test from HeaderSearchTest (4 ms total)

[----------] 1 test from ModuleDeclStateTest
[ RUN      ] ModuleDeclStateTest.NotModule
[       OK ] ModuleDeclStateTest.NotModule (4 ms)
[----------] 1 test from ModuleDeclStateTest (4 ms total)

[----------] 1 test from DxcModeTest
[ RUN      ] DxcModeTest.TargetProfileValidation
[       OK ] DxcModeTest.TargetProfileValidation (11 ms)
[----------] 1 test from DxcModeTest (11 ms total)

[----------] 1 test from MultilibTest
[ RUN      ] MultilibTest.SetPushback
[       OK ] MultilibTest.SetPushback (0 ms)
[----------] 1 test from MultilibTest (0 ms total)

[----------] 2 tests from ExprMutationAnalyzerTest
[ RUN      ] ExprMutationAnalyzerTest.CallUnresolved
[       OK ] ExprMutationAnalyzerTest.CallUnresolved (68 ms)
[ RUN      ] ExprMutationAnalyzerTest.RangeForNonArrayByConstRef
input.cc:1:103: warning: expression result unused [-Wunused-value]
    1 | struct V { const int* begin() const; const int* end() const; };void f() { V x; for (const int& e : x) e; }
      |                                                                                                       ^
[       OK ] ExprMutationAnalyzerTest.RangeForNonArrayByConstRef (3 ms)
[----------] 2 tests from ExprMutationAnalyzerTest (71 ms total)

[----------] 1 test from MacroExpansionContextTest
[ RUN      ] MacroExpansionContextTest.StringizingVariadicMacros
[       OK ] MacroExpansionContextTest.StringizingVariadicMacros (0 ms)
[----------] 1 test from MacroExpansionContextTest (0 ms total)

[----------] 1 test from EnvironmentTest
...