labath added a comment. In D131335#3726204 <https://reviews.llvm.org/D131335#3726204>, @Michael137 wrote:
> Thanks for taking the time to look at this > >> I would like to understand what (if any) are the issues with extending the >> approach #2 to cover non-member functions as well. For what it's worth, I >> don't think #2 is a hack. I might actually say that's "using DWARF as it was >> meant to be used" -- the mangled name is there for us to use (*). Before >> LLDB came around, I doubt anybody on the DWARF committee imagined people >> would try round-tripping the DWARF information back into a "regular" C++ >> compiler (and expecting it to recreate "perfect" mangled names for templates >> and all). > > There are no issues that I know of with this. My hesitation with this > approach was just that if we eventually fix the way we construct template > decls, we will need to make sure we continue attaching the labels correctly. > Maybe this isn't a big deal if we note it in a comment Yeah, if you end up working on this, and find that you need to have clang to something in this area, I can try to loop in some clang people to help. >> I don't mean to offend, but if anything, I would say that #1 is a hack. What >> it essentially does is pretend that abi tags don't exist. The way I >> understand it, the main reason for the introduction of abi tags was to >> enable one to have two versions of the same function (class, etc.) co-exist >> in the same binary/program. This essentially defeats that. I haven't looked >> at the implementation too closely, but I don't see how one could properly >> disambiguate two differently-tagged versions of the same functions using >> this approach. It may be a hack that we have to live with, but I am not >> convinced of that (yet). > > The way this patch allows distinguishing differently-tagged versions is by > checking the `Attrs` Node of the Itanium mangle tree. But you're right in > that this change does rely on a follow-up patch that attaches `AbiTagAttr` > nodes to the `FunctionDecl`s we generate, which we were planning on doing. > But I guess if we are going to attach labels to FunctionDecls anyway, we > could just attach the `AsmLabel` instead and thus cover all types of > attributes trivially. FWIW, I would be all for attaching the abi tags to the clang declarations if they would be in some easily accessible form (e.g. a DWARF attribute). Parsing them out of the mangled name is somewhat dubious, but I am not entirely against that, if it is necessary for some use case. However, even if we did that, I'd still say that attaching the asm attribute is a good idea. >> Regarding #3, yes, there are definite problems regarding the way we >> represent templates. The root cause is the fundamental mismatch between what >> clang wants (a completely accurate description of all objects "as if written >> in source") versus what DWARF provides (a description of the most important >> properties of entities that are present in the binary). Where the problem >> lies depends on your point of view. However, even if we come up with a >> better way to represent the debug information in AST, it's not clear to me >> why that would be incompatible with #2. Since we don't have the actual >> source code for the template function, we will never be able to provide a >> fully generic definition of it -- we will always need to deal with specific >> instantiations of that template. So what we just need is the attach a (asm) >> label to a specific template instantiation. I would hope that wouldn't be >> too much to ask of clang to support. > > This mismatch currently causes problems with unqualified lookups, especially > with import-std-module enabled, so it would be nice to fix eventually. We > don't need a fully generic definition, just the generic prototype to get the > mangled name correctly. But as you say, this can be done with an asm() label > with less trouble. > > I can open a separate diff with approach #2 and see from there. Cool. Thanks for doing that. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D131335/new/ https://reviews.llvm.org/D131335 _______________________________________________ lldb-commits mailing list lldb-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
