Oh boy, these are scary. Should LLDB fail harder when the utility expression fails, so we can detect these earlier?

I'm curious now why there is both count and max_class_infos and if the second is workaround for this bug? Anyway, this *looks* plausible!

Patch looks fine. Wording suggestion for the user-visible parts inside.

Thanks!

In D123319#3517966, @dblaikie wrote:

In D123319#3506745, @shafik wrote:

What does the linkage name do for your use case? Which cases are missing linkage names/where do they go missing?

I am happy to consider other approaches as well to solving lookup for local lambdas for D105564. Let me know what you think.

Why does the return type help perform lookup? What kind of lookup?

(again, my take is that "auto" return types probably shouldn't be described at all - we should just not describe functions where we don't know their return types because they're not very useful to know about (overload resolution, yes, but then you can't call them anyway) - but that's a broader argument to make/change to make)

Seems reasonable!

Do you need to add any special handling to CloneFunction.cpp or will it work correctly?

I think this looks reasonable. @JDevlieghere ?

More elegant would be to just add an API to TypeSystem to get the operator to access ivars.

This is good, but it also illustrates how the strings "->" and ".'" should actually come from the typesystem and not be hardcoded. We're just lucky that all languages have a "." operator.

Thanks, this looks really good! I have a couple of small comments inline.

Mechanically this looks fine, but I think the documentation could be improved. Could you add something to the comment of DwarfStringPoolEntryRef that explains what the two cases in the PointerUnion and their use-cases are?

I was able to update LLDB https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/44252/. We can leave this in.

It looks like this patch has broken 168 tests in LLDB: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/44239/changes#41d5033eb162cb92b684855166cabfa3983b74c6
I'm going to dig a little deeper, but I might ask you to revert this until we can figure out a solution.

I'm going to revert the patch now. This is not just breaking LLDb, but also clang itself on Darwin platforms. I think we need to be more careful to separate out the enabling of Clang C++ modules and C++20 modules. Either by having -fmodules-ts control the HaveModules flag, or by adding a way to explicitly turn them off. I'm happy to help with testing out any patches!

In any case, I would appreciate it if we could revert this patch until we found a solution!

Should we introduce a -fno-modules-ts option on top? Any better suggestions?

I think the problem might be that previously on Darwin -fcxx-modules was used to turn on C++ Clang modules (which was not implied by -fmodules) without turning of C++20 (ts) modules, and after this patch -fcxx-modules implies -fmodules-ts. Does that sound plausible?

https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/44167/

This patch broke a whole bunch tests in the LLDB testsuite. I'm trying to figure out what exactly the semantics of the change are, particularly on Darwin, where -fmodules doesn't imply -fcxx-modules.

Adding the debug-info group. I don't really know enough about deduction guides to comment on the implications, but if it fixes the crash it seems to be a strict improvement. Maybe wait another day to see if someone else has an opinion on this.

In D121100#3530683, @alok wrote:

GNU gdb is now modified to accept functions with linkage name.

commit 6f9b09edaee43ea34d34b1998fe7b844834f251a
Author: Alok Kumar Sharma <AlokKumar.Sharma@amd.com>
Date:   Sun May 22 21:46:06 2022 +0530

This is exciting!
I understand where it comes from, but I wanted to at least ask if it would make sense to give the tool a more descriptive and discoverable name than llvm-dva such as llvm-debuginfo-analyzer. I understand there's a tradeoff between having a short command and a descriptive name.

Sgtm.

This looks very good, thanks!

@shchenz Would you consider reverting your patch? Let me know if you have trouble reproducing the issue.

Another bot failure.
https://green.lab.llvm.org/green/view/Clang/job/clang-stage2-Rthinlto/5501/consoleFull#-125847874549ba4694-19c4-4d7e-bec5-911270d8a58c

It looks like this commit broke the -DLLVM_ENABLES_MODULES=1 build: https://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/43994/changes

How do we usually add tests for STL data structures?

That seemed to have done the trick.

I'm going to land this quickly to get the bots going again, but a thorough review would still be very much appreciated!

Note that this broke the LLDB data formatters.

I think this mostly looks good. I left comment about the test inline.

The debug info "upgrade" mechanism is unintuitive that I'm worried that this could subtly break intended behavior if it isn't tested.

I think agree with the idea of the patch. Can you please check that (1) there is an existing test that ensures warning is printed when not explicitly stripping debug info and (2) add a test for this case?

One last request: Could you add a Doxygen comment?

Thank you! This is equally useful and untestable :-)

Few nitpicks inside otherwise I'm happy.

I have some trivial suggestions inside, but otherwise, thanks!

Oh, it's a global variable that supposed to shadow the function parameter! Thanks for clarifying.

The new test looks good (I would replace the CHECK-NEXT with CHECK though).
It sounds like there was no objections for doing this for lambdas.

I would put it for example into the Fortran function information section and just add a source code example (if that makes sense) to explain under what cirumstances a frontend would want to put in a target function name.

Mechanically I think this looks fine, could you also update SourceLevelDebugging.rst?

For an example, if thread local variable is present in copyin clause (testcase attached with the

patch), parameter with same name is generated as parameter to artificial function. When user
inquires the thread Local variable, its debug info is hidden by the parameter. The debug info
for parameters (for thread local) must be suppressed.

At first glance this looks fine to me. @JDevlieghere ?