[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-27 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith accepted this revision. dexonsmith added a comment. This revision is now accepted and ready to land. In https://reviews.llvm.org/D35159#823211, @erik.pilkington wrote: > Use LLVM naming conventions for all the new stuff in this patch. Thanks for renaming everything. LGTM! https://

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-16 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. In https://reviews.llvm.org/D35159#809724, @erik.pilkington wrote: > Rebase. I don't think the issue of purging underscores from this file/libcxx > should block this, if we want to discuss that cfe-dev would probably be the > place. I agree that it would be nice to c

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-12 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added a comment. +1 for moving this file to LLVM's internal style. Comment at: src/cxa_demangle.cpp:44 +class string_ref +{ dexonsmith wrote: > mehdi_amini wrote: > > dexonsmith wrote: > > > erik.pilkington wrote: > > > > mehdi_amini wrote: > > > > > I

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-12 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington added a comment. > Looks like this demangler's design is similar to my demangler for Microsoft > name mangling scheme (https://reviews.llvm.org/D34667). Is that a > coincidence? Both demanglers create AST, stringize it using > print_left/print_right (I named them write_pre/write

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-12 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a reviewer: mclow.lists. dexonsmith added a subscriber: mclow.lists. dexonsmith added a comment. +mclow.lists Comment at: src/cxa_demangle.cpp:44 +class string_ref +{ mehdi_amini wrote: > dexonsmith wrote: > > erik.pilkington wrote: > > > meh

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-12 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added a comment. In https://reviews.llvm.org/D35159#807075, @dexonsmith wrote: > This LGTM. Mehdi, do you have any other concerns? No other concern (haven't looked further for any either) https://reviews.llvm.org/D35159 ___ cfe-commi

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-12 Thread Rui Ueyama via Phabricator via cfe-commits
ruiu added a comment. Looks like this demangler's design is similar to my demangler for Microsoft name mangling scheme (https://reviews.llvm.org/D34667). Is that a coincidence? Both demanglers create AST, stringize it using print_left/print_right (I named them write_pre/write_post), and use cus

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-12 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments. Comment at: src/cxa_demangle.cpp:44 +class string_ref +{ dexonsmith wrote: > erik.pilkington wrote: > > mehdi_amini wrote: > > > If this is supposed to be *the* ultimate LLVM demangler, can we follow > > > LLVM coding standar

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-12 Thread Duncan P. N. Exon Smith via Phabricator via cfe-commits
dexonsmith added a comment. This LGTM. Mehdi, do you have any other concerns? Comment at: src/cxa_demangle.cpp:44 +class string_ref +{ erik.pilkington wrote: > mehdi_amini wrote: > > If this is supposed to be *the* ultimate LLVM demangler, can we follow LLVM

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-09 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments. Comment at: src/cxa_demangle.cpp:87 + +class stream +{ Doc (same for non trivial APIs) Comment at: src/cxa_demangle.cpp:125 + +typedef unsigned stream_position; + Doc Co

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-09 Thread Erik Pilkington via Phabricator via cfe-commits
erik.pilkington marked 4 inline comments as done. erik.pilkington added inline comments. Comment at: src/cxa_demangle.cpp:44 +class string_ref +{ mehdi_amini wrote: > If this is supposed to be *the* ultimate LLVM demangler, can we follow LLVM > coding standard

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-09 Thread Mehdi AMINI via Phabricator via cfe-commits
mehdi_amini added inline comments. Comment at: src/cxa_demangle.cpp:44 +class string_ref +{ If this is supposed to be *the* ultimate LLVM demangler, can we follow LLVM coding standard? https://reviews.llvm.org/D35159 __

[PATCH] D35159: [libcxxabi][demangler] Use an AST to represent the demangled name

2017-07-09 Thread Eric Fiselier via Phabricator via cfe-commits
EricWF added a comment. This patch causes `test_demangle.pass.cpp` to fail with UBSan. Standard Error: -- /home/eric/workspace/libcxxabi/src/cxa_demangle.cpp:113:44: runtime error: null pointer passed as argument 2, which is declared to never be null /usr/include/string.h:47:14: note: no