oToToT added inline comments.
================ Comment at: clang/test/AST/ast-dump-array-json.cpp:6 + +// CHECK: "mangledName": "_ZN1A4TestE", +// CHECK: "mangledName": "_ZN1A4TestE", ---------------- rsmith wrote: > oToToT wrote: > > yaxunl wrote: > > > I am not sure if this test tests the code you change since the mangled > > > variable name does not encode type. > > I think this test my code. Or, at least, this test will trigger a runtime > > error in the latest clang without this patch when mangling `A::Test`. > > > > Also, maybe I could try construct a test to mangle variable with element > > type if needed. > > > > WDYT? > I would like to see a test that verifies the `A<type>_` mangling is produced. Ah, you're right! After some investigation, I found that this `CHECK` in the test doesn't really check my patch. This test only ensure that clang won't get runtime error when handling this file. However, I can't find any clever way to construct a test like this to ensure the output of `CXXNameMangler::mangleType(const DependentSizedArrayType *)`. (It is called here only for checking ABI tags, so I guess no output will be displayed). I will try replace this test with a unittest to check the output. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D99407/new/ https://reviews.llvm.org/D99407 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
