rsmith added inline comments.
Herald added a project: All.
================
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);
----------------
What should happen if there's a qualifier difference other than a CVR
qualifier? Eg, address space or ObjC lifetime.
================
Comment at: clang/lib/AST/ASTContext.cpp:11578
+ default:
+ return X;
+ }
----------------
For `TemplateArgument::ArgKind::Declaration`, I think it'd make sense to take
the common type of the `getParamTypeForDecl`s.
================
Comment at: clang/lib/AST/ASTContext.cpp:11603
+ ? X->getQualifier()
+ : Ctx.getCanonicalNestedNameSpecifier(X->getQualifier());
+}
----------------
Might be worth adding a `FIXME` to keep as much common NNS sugar as we can?
================
Comment at: clang/lib/AST/ASTContext.cpp:11632-11633
+
+static QualType getCommonCanonicalType(ASTContext &Ctx, const Type *X,
+ const Type *Y) {
+ Type::TypeClass TC = X->getTypeClass();
----------------
I don't understand this function name: there's only one canonical
representation of a type, so this seems paradoxical. I think
`getCommonDesugaredType` is probably a clearer name for what this is doing.
================
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 {
----------------
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?
================
Comment at: clang/lib/AST/ASTContext.cpp:11861
+ *IY = cast<InjectedClassNameType>(Y);
+ assert(IX->getDecl() == IY->getDecl());
+ return Ctx.getInjectedClassNameType(
----------------
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.
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
