[PATCH] D47627: [ASTContext] Make getAddrSpaceQualType replace address spaces.

Bevin Hansson via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jun 4 23:49:57 PDT 2018


ebevhan added a comment.

There's actually a bit more to it than in these two patches. The complete diff to this function in our downstream Clang looks like this:

   QualType
   ASTContext::getAddrSpaceQualType(QualType T, unsigned AddressSpace) const {
  -  QualType CanT = getCanonicalType(T);
  -  if (CanT.getAddressSpace() == AddressSpace)
  +  if (T.getLocalQualifiers().getAddressSpace() == AddressSpace)
       return T;
   
     // If we are composing extended qualifiers together, merge together
     // into one ExtQuals node.
     QualifierCollector Quals;
     const Type *TypeNode = Quals.strip(T);
   
     // If this type already has an address space specified, it cannot get
     // another one.
  -  assert(!Quals.hasAddressSpace() &&
  -         "Type cannot be in multiple addr spaces!");
  -  Quals.addAddressSpace(AddressSpace);
  +  if (Quals.hasAddressSpace())
  +    Quals.removeAddressSpace();
  +  if (AddressSpace)
  +    Quals.addAddressSpace(AddressSpace);
   
     return getExtQualType(TypeNode, Quals);
   }

In our downstream Clang, functions have address spaces. The desired address space is applied in getFunctionType, using getAddrSpaceQualType. Due to how FunctionTypes are built, it's possible to end up with noncanonical FunctionType that doesn't have an address space whose canonical type does have one. For example, when building the noncanonical type `void (*)(void * const)`, we will first build its canonical type `void (_AS *)(void *)`. Since getAddrSpaceQualType looks at the canonical type to determine whether the address space is already applied, it's skipped and we end up with this discrepancy.

Now that I test it again, I can't seem to induce the assertion. I suspect I might just have changed it because the documentation said that was how it was supposed to work. Perhaps I should be submitting the upper diff instead? Though, considering that this cannot be reproduced in trunk maybe I simply shouldn't submit it at all.


Repository:
  rC Clang

https://reviews.llvm.org/D47627





More information about the cfe-commits mailing list