mizvekov added inline comments.
================
Comment at: clang/lib/AST/ASTContext.cpp:11551-11552
+ if (Unqualified && !Ctx.hasSameType(X, Y)) {
+ assert(Ctx.hasSameUnqualifiedType(X, Y));
+ auto XQuals = X.getCVRQualifiers(), YQuals = Y.getCVRQualifiers();
+ X.addFastQualifiers(YQuals & ~XQuals);
----------------
rsmith wrote:
> What should happen if there's a qualifier difference other than a CVR
> qualifier? Eg, address space or ObjC lifetime.
I have updated the `ASContext::getCommonSugaredType` to accept an `Unqualified`
parameter to handle this case.
We will strip qualifiers from each other until the types are the same.
================
Comment at: clang/lib/AST/ASTContext.cpp:11578
+ default:
+ return X;
+ }
----------------
rsmith wrote:
> For `TemplateArgument::ArgKind::Declaration`, I think it'd make sense to take
> the common type of the `getParamTypeForDecl`s.
Yeah, but this has been dead code since we went back to canonical template
parameter / arguments.
I have removed it and put some unreachables in place.
We will circle back around to add it with this suggestion later.
================
Comment at: clang/lib/AST/ASTContext.cpp:11790-11801
+ if (EPIX.ExceptionSpec.Exceptions.size() ==
+ EPIY.ExceptionSpec.Exceptions.size()) {
+ auto E = getCommonTypeArray(Ctx, EPIX.ExceptionSpec.Exceptions,
+ EPIY.ExceptionSpec.Exceptions);
+ EPIX.ExceptionSpec.Exceptions = E;
+ return Ctx.getFunctionType(R, P, EPIX);
+ } else {
----------------
rsmith wrote:
> This seems a bit suspicious: `getCommonTypeArray` assumes the two arrays
> contain the same types in the same order, but if the arrays can be different
> sizes, is it really correct to assume that they contain the same types if
> they're the same size?
>
> Can we make this work the same way that the conditional expression handling
> deals with differing lists of exception types?
Handled this with a new `ASTContext::mergeTypeLists` that does what the
conditional expression handling did, while getting the common sugar of types
when they occur in both lists.
================
Comment at: clang/lib/AST/ASTContext.cpp:11861
+ *IY = cast<InjectedClassNameType>(Y);
+ assert(IX->getDecl() == IY->getDecl());
+ return Ctx.getInjectedClassNameType(
----------------
rsmith wrote:
> I think this should probably be a `declaresSameEntity` check: we might be
> comparing injected class names from two different merged modules with two
> different declarations for the same class definition. The right declaration
> to use is the one that is chosen to be "the" definition.
Implemented this with a new getCommonDecl function, which for now will just
fall back to the canonical decl if they differ.
Wonder if there is a cheap way to determine which of the two decls was declared
earlier. Have to look this up, but would a simple pointer comparison be
guaranteed to work? Eg would the earliest declaration, since being created
earlier and allocated earlier, be guaranteed by design to have a lower (or
greater) pointer value?
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D111283/new/
https://reviews.llvm.org/D111283
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
