rsmith added inline comments.
================
Comment at: clang/lib/AST/ItaniumMangle.cpp:6031
+bool CXXNameMangler::mangleSubstitution(NestedNameSpecifier *NNS) {
+ NNS = Context.getASTContext().getCanonicalNestedNameSpecifier(NNS);
+ return mangleSubstitution(reinterpret_cast<uintptr_t>(NNS));
----------------
This seems a little error-prone to me: calling this on a type NNS would do the
wrong thing (those are supposed to share a substitution number with the type,
rather than have a substitution of their own).
We could handle the various cases here and dispatch to the right forms of
`mangleSubstitution` depending on the kind of NNS, but that code would all be
unreachable / untested. So maybe we should just make this assert that
`NNS->getKind() == NestedNameSpecifier::Identifier`. (And optionally we could
give this a more specific name, eg `mangleSubstitutionForIdentifierNNS`?)
================
Comment at: clang/test/CodeGenCXX/clang-abi-compat.cpp:160
+template <typename T> void test10(typename T::Y::a, typename T::Y::b) {}
+// PRE15: @_Z6test10I1XEvNT_1Y1aENS1_1Y1bE
+// V15: @_Z6test10I1XEvNT_1Y1aENS2_1bE
----------------
I think this test does not capture an important property for which we should
have test coverage: that we used to not register a substitution for `T::Y` (not
only that we used to not *use* a substitution for it). For example, this test
would capture that:
```
struct X {
struct Y {
using a = int;
using b = int;
};
};
template <typename T> void test10(typename T::Y::a, typename T::Y::b, float*,
float*) {}
template void test10<X>(int, int, float*, float*);
```
... where the numbering of `float*` depends on how many substitutions we
created for the earlier types.
(Put another way, this test covers only one of the two `if (!Clang14Compat)`
tests in ItaniumMangle.cpp, and we should cover both.)
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D122663/new/
https://reviews.llvm.org/D122663
_______________________________________________
cfe-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
