EricWF added inline comments.
================
Comment at: src/cxa_demangle.cpp:10
+// FIXME: (possibly) incomplete list of features that clang mangles that this
+// file does not yet support:
----------------
Awesome comment!
If your awesomeness knows no bound, I would love to add currently-failing tests
that demonstrate the FIXME.
Obviously that has no bearing on how this patch proceeds.
================
Comment at: src/cxa_demangle.cpp:16
+
#define _LIBCPP_NO_EXCEPTIONS
----------------
Urg...
Entirely unrelated to this patch, but manually defining `_LIBCPP_NO_EXCEPTIONS`
here smells awful.
================
Comment at: src/cxa_demangle.cpp:1848
+// <abi-tag> ::= B <positive length number> <identifier>
+
+const char*
----------------
I realize the unneeded blank line is part of the existing style... But I
fudging hate it.
If you're not opposed I would love to start removing the unneeded line wherever
it's
part of this changeset.
================
Comment at: src/cxa_demangle.cpp:4353
// ::= C3 # complete object allocating constructor
// extension ::= C5 # ?
// ::= D0 # deleting destructor
----------------
Does this comment need updating now that `<ctor-dtor-name>` now parses with an
optional ABI tag at the end?
================
Comment at: src/cxa_demangle.cpp:4408
// <unnamed-type-name> ::= Ut [ <nonnegative number> ] _
// ::= <closure-type-name>
//
----------------
Does this comment need to be updated to reflect that `<unnamed-type-name>` now
parses with an optional ABI tag after the `_` character?
================
Comment at: test/test_demangle.pass.cpp:29608
{"_Z4algoIJiiiEEvZ1gEUlT_E_", "void algo<int, int, int>(g::'lambda'(int,
int, int))"},
+
+ {"_Z1fB3foov", "f[abi:foo]()"},
----------------
Perhaps it would be better to replace this blank line with a comment stating
the tests below are for the ABI tag extension?
https://reviews.llvm.org/D40279
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
