[PATCH] D10272: Fix alignment issues in Clang.

Reid Kleckner rnk at google.com
Mon Jul 6 15:04:24 PDT 2015


rnk added a subscriber: rnk.

Most of actual fixes look good, but this adds an awful lot of static_asserts, and they are very far from the code implementing the trailing object array idiom. I chatted with Richard, and he suggested that wrap up the `reinterpret_cast<T>(this + 1)` idiom into something higher level. We could write a helper like this:

  namespace llvm {
  template <TrailingTy, LeadingTy>
  TrailingTy *getTrailingObjects(LeadingTy *Obj) {
    static_assert(llvm::AlignOf<LeadingTy>::Alignment >= llvm::AlignOf<TrailingTy>::Alignment,
                  "TrailingTy requires more alignment than LeadingTy provides");
    return reinterpret_cast<TrailingTy *>(Obj + 1);
  }
  }

The static_assert message ends up being less specific, but clang at least will print out the template instantiation backtrace, which should make it easy to figure out. This pattern should work so long as the trailing type is complete before the definition of the accessor, but the TemplateSpecializationType and TemplateArgument case is probably won't work. We should probably do the static_assert manually in the constructor in that case. The constructor also implements the trailing object array idiom, so again it keeps the assert closer to the relevant implementation.

What do you think?


================
Comment at: include/clang/AST/Type.h:3974-3980
@@ -3948,2 +3973,9 @@
 };
+// static_assert(llvm::AlignOf<TemplateSpecializationType>::Alignment >=
+// llvm::AlignOf<TemplateArgument>::Alignment, "Alignment is insufficient for
+// objects appended to TemplateSpecializationType");
+// static_assert(llvm::AlignOf<TemplateArgument>::Alignment >=
+// llvm::AlignOf<QualType>::Alignment, "Alignment is insufficient for objects
+// appended to TemplateSpecializationType");
+// ^ Moved after class TemplateArgument, as it is is forward declared here.
 
----------------
This still appears to be an issue?


http://reviews.llvm.org/D10272







More information about the cfe-commits mailing list