rsmith added inline comments.
================ Comment at: clang/test/CodeGenCXX/mangle.cpp:805 - // CHECK-LABEL: define weak_odr void @_ZN6test341fIiEEvDTstDTplcvT__EcvS1__EEE + // CHECK-LABEL: define weak_odr void @_ZN6test341fIiEEvm template void f<int>(decltype(sizeof(1))); ---------------- rsmith wrote: > sammccall wrote: > > sammccall wrote: > > > GCC mangles this as `_ZN6test341fIiEEvDTstDTplcvT__EcvS1__EEE`, so we're > > > breaking compat here :-\ > > > > > > And in fact we're just incorrect. > > > https://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling: > > > > > > > If the operand expression of decltype is not instantiation-dependent > > > > then the resulting type is encoded directly. > > > > > > Here, instantiation-dependent *is* explicitly defined as "type-dependent > > > or value-dependent or ...". And therefore these cases must not be > > > "encoded directly", including the motivating case (decltype(N) where N is > > > a typed constant whose initializer is value-dependent). > > > > > > We could consider not two but *three* types of decltypetypes: > > > - decltype(type-dependent), which is sugar for a canonical > > > DependentDeclTypeType > > > - decltype(instantiation-but-not-type-dependent), which is still sugar > > > for a concrete canonical type (per C++17) but mangles using the expr (per > > > cxxabi) > > > - decltype(non-dependent), which is sugar for a concrete canonical type > > > and mangles directly > > > > > > This only works if it's OK for mangling to depend on non-canonical type > > > details - I don't know whether this is the case. @rsmith - any hints here? > > Hmm, my naive reading of the mangle code matches what I described. > > > > The big comment in ItaniumMangle talks about related issues: > > https://github.com/llvm/llvm-project/blob/24238f09edb98b0f460aa41139874ae5d4e5cd8d/clang/lib/AST/ItaniumMangle.cpp#L2541-L2572 > > > > I don't really understand what's going on here, sorry. > Looks like we need the single-step-desugaring loop below the big comment you > quoted to stop when it hits a `decltype` type. That's presumably where we > step from the instantiation-dependent-but-not-type-dependent `decltype` node > to its desugared type. Also... the FIXME from that comment will apply here too. Given: ``` template<typename T> void f(decltype(sizeof(T)), decltype(sizeof(T))) {} template void f<int>(unsigned long, unsigned long); ``` we currently (correctly, as far as I can see) use a substitution for the second parameter type, but with the change here, I don't think we will do so any more. We could fix that by deduplicating `DecltypeType`s when they're instantiation dependent but not dependent; that's what we do for `ConstantArrayType`'s size expression, for example. If we do that, we'll need to store the actual expression on the `DecltypeTypeLoc`, since we can't rely on the `DecltypeType` having the right expression any more (only an expression that's equivalent to it). Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D87349/new/ https://reviews.llvm.org/D87349 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
