On Fri, Nov 8, 2019 at 11:17 AM Eduard-Mihai Burtescu <ed...@lyken.rs> wrote: > > On Fri, Nov 8, 2019, at 7:43 PM, Ian Lance Taylor wrote: > > On Fri, Nov 8, 2019 at 9:02 AM Eduard-Mihai Burtescu <ed...@lyken.rs> wrote: > > > > > > Ping #2 for https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01830.html > > > Original patch (without the early exit optimization): > > > https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01591.html > > > > Sorry for letting this slide. > > > > Do we need the CHECK_OR and ERROR_AND macros? Is there anything like > > those elsewhere in the libiberty or GCC sources? I would rather than > > have ordinary code than obscure macros. > > Good point, I was wondering about the macros but forgot to ask explicitly, > they're the least usual (for C) part of this code, they arose from porting > the demangler for the new format (most of which isn't even in the current > patch) from Rust, where we have facilities for conveniently propagating > errors, and I wanted to avoid using goto too much for this purpose. > > Looking at > https://gist.github.com/eddyb/c41a69378750a433767cf53fe2316768#file-rust-demangle-c > I can see: > * 5 uses of ERROR_AND > * 20 uses of CHECK_OR > * 7 of those are CHECK_OR (!rdm->errored, return ); which can just be if > (rdm->errored) return; > > So in the final code there'd be ~18 places that would need to set > rdm->errored = 1; (and then return or goto cleanup). > That's not that bad, I guess, and I'd welcome any suggestions for how to > clean up that. > > For this current patch, however, there's only 3 uses total, so the macros are > definitely overkill. > Assuming you'd want them removed, I took the liberty of doing that and here's > the fixed patch: > > 2019-10-22 Eduard-Mihai Burtescu <ed...@lyken.rs> > include/ChangeLog: > * demangle.h (rust_demangle_callback): Add. > libiberty/ChangeLog: > * cplus-dem.c (cplus_demangle): Use rust_demangle directly. > (rust_demangle): Remove. > * rust-demangle.c (is_prefixed_hash): Rename to > is_legacy_prefixed_hash. > (parse_lower_hex_nibble): Rename to decode_lower_hex_nibble. > (parse_legacy_escape): Rename to decode_legacy_escape. > (rust_is_mangled): Remove. > (struct rust_demangler): Add. > (peek): Add. > (next): Add. > (struct rust_mangled_ident): Add. > (parse_ident): Add. > (rust_demangle_sym): Remove. > (print_str): Add. > (PRINT): Add. > (print_ident): Add. > (rust_demangle_callback): Add. > (struct str_buf): Add. > (str_buf_reserve): Add. > (str_buf_append): Add. > (str_buf_demangle_callback): Add. > (rust_demangle): Add. > * rust-demangle.h: Remove.
This is OK. Thanks. Ian
