[PATCH] D50119: Compiler support for P1144R0 "__is_trivially_relocatable(T)"

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.


================
Comment at: lib/AST/Type.cpp:2234
+bool QualType::isTriviallyRelocatableType(const ASTContext &Context) const {
+  QualType T = Context.getBaseElementType(*this);
+  if (T->isIncompleteType())
----------------
Quuxplusone wrote:
> erichkeane wrote:
> > Quuxplusone wrote:
> > > erichkeane wrote:
> > > > You likely want to canonicalize here.
> > > You mean `QualType T = Context.getBaseElementType(getCanonicalType());`?
> > > I can do that. For my own edification (and/or a test case), in what way does the current code fail?
> > More like: QualType T = Context.getBaseElementType(*this).getCanonicalType();
> > 
> > This desugars typedefs in some cases, which could make the below fail.  
> > 
> > Something like this: https://godbolt.org/z/-C-Onh
> > 
> > It MIGHT still pass here, but something else might canonicalize this along the way.
> > 
> > ADDITIONALLY, I wonder if this properly handles references?  I don't think getBaseElementType will de-reference. You might want to do that as well.
> > ADDITIONALLY, I wonder if this properly handles references? I don't think getBaseElementType will de-reference. You might want to do that as well.
> 
> I actually don't want references to be stripped here: `int&` should come out as "not trivially relocatable" because it is not an object type.
> https://p1144.godbolt.org/z/TbSsOA
Ah, I see.  For some reason I had it in my mind that you wanted to follow references.


Repository:
  rC Clang

https://reviews.llvm.org/D50119