aaron.ballman added inline comments.
================ Comment at: clang/lib/AST/ASTContext.cpp:12910-12916 + // FIXME:: is this assumption correct or do we need to do work here to find + // the common type sugar regarding the stripped qualifiers if only one side + // is unqual? + assert(cast<TypeOfType>(X)->isUnqual() == cast<TypeOfType>(Y)->isUnqual() && + "typeof vs typeof_unqual mismatch?"); + return Ctx.getTypeOfType(Ctx.getQualifiedType(Underlying), + cast<TypeOfType>(X)->isUnqual()); ---------------- mizvekov wrote: > mizvekov wrote: > > erichkeane wrote: > > > I'm unfamiliar with this function, but I would expect you MIGHT need to? > > > If only because they are the same AST node. Should 'unqual' version be > > > its own node? I'm on the fence, as it is a LOT of code to do so, but > > > also ends up being simpler in many places. > > A qualified and an unqualified typeof could have the same underlying type, > > so this assert can trip. > > > > I think what makes most sense is to unify them to a qualified typeof in > > case they differ, as that holds the underlying type unchanged: > > ``` > > return Ctx.getTypeOfType(Ctx.getQualifiedType(Underlying), > > cast<TypeOfType>(X)->isUnqual() && > > cast<TypeOfType>(Y)->isUnqual()); > > ``` > By the way, one thing to notice is the confusion regarding what is the > underlying type of this node, the result of `desugar()` or the result of > `getUnderlyingType()`? > > On my previous post, I meant the former. > > Maybe this is a good reason to rename `getUnderlyingType()` to > `getUnmodifiedType()` or something similar? FWIW: I still have to address these comments in the patch, but I'll do that tomorrow. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D134286/new/ https://reviews.llvm.org/D134286 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
