[prev in list] [next in list] [prev in thread] [next in thread]
List: cfe-commits
Subject: Re: [PATCH] D11298: Convert a few classes over to use the new TrailingObjects helper.
From: Richard Smith <richard () metafoo ! co ! uk>
Date: 2015-08-03 20:17:06
Message-ID: d7868692b8167633322e1728826479fb () localhost ! localdomain
[Download RAW message or body]
rsmith added inline comments.
================
Comment at: include/clang/AST/Decl.h:3620
@@ +3619,3 @@
+ public DeclContext,
+ llvm::TrailingObjects<CapturedDecl, ImplicitParamDecl *> {
+protected:
----------------
I would prefer to see an explicit `private` access specifier here.
================
Comment at: include/clang/AST/Decl.h:3638-3642
@@ -3628,6 +3637,7 @@
ImplicitParamDecl **getParams() const {
- return reinterpret_cast<ImplicitParamDecl **>(
- const_cast<CapturedDecl *>(this) + 1);
+ // FIXME: doesn't seem like it should be using a const_cast.
+ return const_cast<CapturedDecl *>(this)
+ ->getTrailingObjects<ImplicitParamDecl *>();
}
----------------
The uses of this below appear to be const-correct; maybe add const and non-const \
overloads without the `const_cast`?
================
Comment at: include/clang/AST/Decl.h:3702
@@ -3691,2 +3701,3 @@
friend class ASTDeclWriter;
+ friend class TrailingObjects;
};
----------------
I predict one of our supported host compilers will choke on this.
*experiments*
That compiler is MSVC. It appears all our supported compilers provide enough C++11 to \
allow
friend TrailingObjects;
================
Comment at: include/clang/AST/DeclTemplate.h:47
@@ -45,2 +46,3 @@
/// derived classes.
-class LLVM_ALIGNAS(/*alignof(void*)*/ LLVM_PTR_SIZE) TemplateParameterList {
+class LLVM_ALIGNAS(/*alignof(void*)*/ LLVM_PTR_SIZE) TemplateParameterList final
+ : llvm::TrailingObjects<TemplateParameterList, NamedDecl *> {
----------------
Can you remove the LLVM_ALIGNAS?
================
Comment at: include/clang/AST/DeclTemplate.h:145
@@ -136,4 +144,3 @@
/// derived classes. Suitable for creating on the stack.
-template<size_t N>
-class FixedSizeTemplateParameterList : public TemplateParameterList {
- NamedDecl *Params[N];
+template <size_t N> class FixedSizeTemplateParameterListStorage {
+ char Mem[TemplateParameterList::totalSizeToAlloc<NamedDecl *>(N)];
----------------
This type is now underaligned.
================
Comment at: include/clang/AST/DeclTemplate.h:146
@@ -140,1 +145,3 @@
+template <size_t N> class FixedSizeTemplateParameterListStorage {
+ char Mem[TemplateParameterList::totalSizeToAlloc<NamedDecl *>(N)];
----------------
Do all of our supported compilers provide enough `constexpr` support for this to \
actually work? How would you feel about:
TemplateParameterList List;
NamedDecl *Params[N];
(Yeah, I don't like this either.)
================
Comment at: include/clang/AST/DeclTemplate.h:165
@@ -151,6 +164,3 @@
/// \brief The template argument list.
- ///
- /// The integer value will be non-zero to indicate that this
- /// template argument list does own the pointer.
- llvm::PointerIntPair<const TemplateArgument *, 1> Arguments;
+ const TemplateArgument *Arguments;
----------------
Please check in the removal of the `Owned` flag separately.
================
Comment at: include/clang/AST/DeclTemplate.h:1290-1302
@@ -1262,15 +1289,15 @@
/// pack.
QualType getExpansionType(unsigned I) const {
assert(I < NumExpandedTypes && "Out-of-range expansion type index");
- void * const *TypesAndInfos = reinterpret_cast<void * const*>(this + 1);
+ void *const *TypesAndInfos = getTrailingObjects<void *>();
return QualType::getFromOpaquePtr(TypesAndInfos[2*I]);
}
/// \brief Retrieve a particular expansion type source info within an
/// expanded parameter pack.
TypeSourceInfo *getExpansionTypeSourceInfo(unsigned I) const {
assert(I < NumExpandedTypes && "Out-of-range expansion type index");
- void * const *TypesAndInfos = reinterpret_cast<void * const*>(this + 1);
+ void *const *TypesAndInfos = getTrailingObjects<void *>();
return static_cast<TypeSourceInfo *>(TypesAndInfos[2*I+1]);
}
----------------
Wow, this is still pretty gross. Can we use `std::pair<QualType, TypeSourceInfo*>` as \
the trailing object type, or something morally equivalent?
================
Comment at: lib/AST/Decl.cpp:3129
@@ +3128,3 @@
+ void *Buffer = Context.Allocate(
+ totalSizeToAlloc<TemplateArgumentLoc, FunctionTemplateDecl *>(
+ TArgs.size(), Ts.size()));
----------------
Why do you need to specify the types here? Shouldn't they be implied by the type of \
the `TrailingObjects` base class?
================
Comment at: lib/AST/Decl.cpp:3896
@@ -3893,3 +3895,3 @@
unsigned NumParams) {
- return new (C, DC, NumParams * sizeof(ImplicitParamDecl *))
+ return new (C, DC, additionalSizeToAlloc<ImplicitParamDecl *>(NumParams))
CapturedDecl(DC, NumParams);
----------------
Likewise here.
http://reviews.llvm.org/D11298
_______________________________________________
cfe-commits mailing list
cfe-commits@cs.uiuc.edu
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic