On Tue, Nov 14, 2017 at 1:31 PM, Nathan Sidwell <nat...@acm.org> wrote: > This patch addresses c++/82836 & c++/82737. The root cause was a bad > assumption I made when moving the mangling alias machinery to its own hash > table. > > I had thought that once we SET_DECL_ASSEMBLER_NAME, it never becomes unset > (or changed). That is false. There are paths in the compiler that set it > back to zero, and at least one path where we remangled because of a bad > assumption about the templatedness of a friend. > > Previously, we placed mangling aliases in the global namespace mapping an > identifier (the mangled name) to the decl. Resetting the assembler name > didn't break this map -- but may have led to unneeded aliases, I guess. > > Moving the alias machinery to its own hash-map allowed me to make namespace > hashes a simple hash, keyed by DECL_NAME (rather than a hash-map from > identifier->decl). > > Finally converting the alias hash-map to a hash-table keyed by > DECL_ASSEMBLER_NAME shrunk that table too. But exposed this problem. > > I did contemplate reverting that change, but tried this first, and it seems > good. > > 1) rename the args of COPY_DECL_ASSEMBLER_NAME from DECL1 & DECL2 to the > more semantic SRC_DECL and DST_DECL. Same for COPY_DECL_RTL. These macros > smell rather memcpy-like, but the args are the wrong way round, so the more > clues the better. (My comment in 82737 was misled by this.) > > 2) change SET_DECL_ASSEMBLER_NAME to a function call, similar to > DECL_ASSEMBLER_NAME. > > 3) have that call forward to a new lang hook, override_decl_assembler_name, > if it is changing the name. (set_decl_assembler_name already having been > taken.) > > 4) the new lang hook default simply stores the new name via > DECL_ASSEMBLER_NAME_RAW. > > 5) the C++ FE overrides this. If the current name is in the alias map and > maps to this decl, we take it out of the map. Then set the new name. > > 6) excitingly, mangle_decl can be called with a non-null > DECL_ASSEMBLER_NAME, so that function's use of SET_DECL_ASSEMBLER_NAME works > just fine. > > booted on all languages. > > ok?
Looks reasonable apart from + /* Overwrite the DECL_ASSEMBLER_NAME for a node. The name is being + changed (including to or from NULL_TREE). */ which suggests the default implementation of set_decl_assembler_name would call this hook (which it doesn't). Any particular reason? Maybe just document (including to NULL_TREE), thus exclude from NULL_TREE? Thanks, Richard. > > nathan > -- > Nathan Sidwell
