dblaikie added inline comments.
================ Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:3659 // Use llvm function name. - Name = Fn->getName(); + if (Fn->getName().startswith("___Z")) + LinkageName = Fn->getName(); ---------------- shafik wrote: > dblaikie wrote: > > shafik wrote: > > > dblaikie wrote: > > > > shafik wrote: > > > > > dblaikie wrote: > > > > > > aprantl wrote: > > > > > > > aprantl wrote: > > > > > > > > Could you please add a comment that Clang Blocks are generated > > > > > > > > as raw llvm::Functions but do have a mangled name and that is > > > > > > > > handling this case? Otherwise this would look suspicious. > > > > > > > Should *all* raw LLVM functions have their name as the linkage > > > > > > > name? Perhaps a raw LLVM function should only have a linkage name > > > > > > > and no human-readable name? > > > > > > Seems plausible to me - do we have any data on other types of > > > > > > functions that hit this codepath? > > > > > So it was not obvious to me what other cases would this branch so I > > > > > added an assert and ran `check-clang` and from that I saw four cases > > > > > that ended up here: > > > > > > > > > > `GenerateCapturedStmtFunction` > > > > > `GenerateOpenMPCapturedStmtFunction` > > > > > `GenerateBlockFunction` > > > > > `generateDestroyHelper` > > > > > > > > > > It is not obvious to me we want to alter the behavior of any of the > > > > > other cases. > > > > Could you show any small source examples & their corresponding DWARF & > > > > how that DWARF would change? (what names are we using, what names would > > > > we end up using/what sort of things are they naming) > > > Ok, I understand the objections to special casing like this. We ended up > > > setting both the `Name` and `LinkageName` unconditionally in this branch > > > because not setting the name for subroutines end up with us generating > > > `DW_TAG_subprogram` without a `DW_AT_name` which is not valid. We > > > discovered this when running the LLDB test suite. > > If we need to have a name, and these are mangled names - were they mangled > > /from/ something & have an unmangled name we should be using, then? > > > > llvm-cxxfilt demangles the example/test as "invocation function for block > > in f(void (int) block_pointer)" - perhaps we should name this "invocation > > function"? & let the scope of the DIE communicate the rest of the > > information about this thing (like the "operator()" for a lambda is just > > "operator()")? > If we take the example from the test I added it demangles to: > > ``` > invocation function for block in f(void (int) block_pointer) > ``` > > There is no usable "short" name there, it is a complex description of the > block and what wraps it. In general from the LLDB perspective we would want a > name to set a breakpoint or display it during a back trace. In this case we > care about displaying this properly in a back trace and it would not be > reasonable to use such a name to set a breakpoint. > How's that compare to lambdas, for example? CHANGES SINCE LAST ACTION https://reviews.llvm.org/D73282/new/ https://reviews.llvm.org/D73282 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits