rjmccall added a comment.
In https://reviews.llvm.org/D47627#1121970, @ebevhan wrote:
> 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.
Well, the documentation mismatch is worth fixing even if the code isn't. But I
think at best your use-case calls for weakening the assertion to be that any
existing address space isn't *different*, yeah.
Separately, I'm not sure that's really the right representation for a Harvard
architecture (which is what I assume you're trying to extend Clang to support);
I think you should probably just teach the compiler that function pointers are
different.
Repository:
rC Clang
https://reviews.llvm.org/D47627
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
