[PATCH] D19135: [sanitizers] Teach the internal demangler about Swift names

[Anna Zaks via llvm-commits]

zaks.anna added inline comments.

================
Comment at: lib/sanitizer_common/tests/sanitizer_symbolizer_test.cc:59
@@ +58,3 @@
+TEST(Symbolizer, DemangleCXXAndSwift) {
+  // The Swift name will only be demangled if the Swift runtime is linked in.
+  EXPECT_STREQ("_TtSd", DemangleCXXAndSwift("_TtSd"));
----------------
aizatsky wrote:
> zaks.anna wrote:
> > aizatsky wrote:
> > > Let's have two separate test cases: DemangleCXX and DemangleSwift.
> > > 
> > > Let's also make the comment a statement.
> > > // Swift names are not demangled in default llvm build because Swift runtime is not linked in.
> > > Let's have two separate test cases: DemangleCXX and DemangleSwift.
> > It's important to test DemangleCXXAndSwift specifically since it's the only API that others will call and testing it ensures that the chaining works. (DemangleCXX and DemangleSwift are not exposed in the header and should not be.)
> I mean this:
> 
>   TEST(Symbolizer, DemangleCXX) {
>     EXPECT_STREQ("f1(char*, int)", DemangleCXXAndSwift("_Z2f1Pci"));
>     EXPECT_STREQ("foo", DemangleCXXAndSwift("foo"));
>     EXPECT_STREQ("", DemangleCXXAndSwift(""));
>   }
> 
>   TEST(Symbolizer, DemangleSwift) {
>     // ...
>     EXPECT_STREQ("_TtSd", DemangleCXXAndSwift("_TtSd"));
>   }
But the following are not testing either Swift or CXX:
  EXPECT_STREQ("foo", DemangleCXXAndSwift("foo"));
  EXPECT_STREQ("", DemangleCXXAndSwift(""));

The point of this whole test is to check that the combined function works. We are not testing CXX or Swift detangling.. I can change it but I do not think that would be best.


http://reviews.llvm.org/D19135