I tried to work around it.

which is weird because I didn't actually change that.

In D103575#2818340, @stella.stamenova wrote:

The Windows buildbot does not like signposts:

https://lab.llvm.org/buildbot/#/builders/83/builds/7271

Let me know if this goes into the right direction.

Address review feedback from @JDevlieghere.

I'm not sure I fully understand the suggestion:

I think I have addressed the review comments but I'd be happy to fix anything I missed

In D86813#2793958, @jmorse wrote:

lgtm

Cheers,

@jmorse @aprantl Sorry for the divergence, but regarding this tests reduction in terms of DI Metadata -- do you think that we can automate this process? I've written a small write-up/proposal for a utility for that purpose: https://github.com/djolertrk/llvm-metadataburn -- please let me know what you think.

Highly relevant to my interests, this'd save 5-10 minutes frequently fiddling with metadata, and it eases newcomers preparation / revision of patches. Yes please!

I just confirmed that passing an empty string for the end event looks even better in Instruments.

Impressive. I spot-checked a few instances and this generally looks great. Let's hope the test suite agrees!

Thanks! LGTM

Seems plausible.

Could you attach new debug info to one of the other functions in this file? Or is there only a single .resume function being generated for the entire test?

Add more comments

Add another comment and explicitly mark lambda captures.

Address review feedback.

If gracefully lowering rvalue references to references needs knowledge about the Clang typesystem (or depends on LLVM IR type uniquing to be efficient, as @dblaikie points out) then we should do it in the Clang frontend. As much as we can we should avoid encoding debug info format specific knowledge in the Clang frontend, but that's not always possible. I'm fine with taking this patch.

If the intended effect is that we run the debuginfo-tests against the just-built LLDB this change makes sense to me in a monorepo world. Conditional LGTM from me.

Looks good (with a testcase). It should be easy to adapt an existing testcase for this by artificially changing a suspend point's !dbg location to use a different file name.

This is good, but we should add a test for this. There are already one or two tests that are affected by this, so you can probably just modify one of them to use line 0.

<compiler-generated> is just something the Swift frontend picks as filename but LLVM shouldn't treat it as special.

I guess we could add least add a comment like this?

// By calling member_type instead of member_clang_type, the size comes from DWARF, which avoids premature record layout.

The change itself seems good to me, I wonder what we can do to prevent a similar mistake in the future.

Doing this in the backend SGTM, assuming all of @dblaikie's comments are addressed.

I only had time for a few superficial comments so far and this deserves an in-depth read, but I greatly appreciate the time and effert that went into putting this to paper!

I think this looks reasonable?

Sorry for the delay. LGTM assuming @dblaikie is happy too.

In D78203#2732842, @simoll wrote:

In D78203#2730888, @aprantl wrote:

Since it's already late on Friday I took the liberty to revert the commit so the bots can recover and you can investigate the failure at your own pace. Let me know if I can help with the fix!

Thanks for taking action! I'll look into how include/llvm/CodeGen/MachinePassRegistry.def actually works.. i suppose moving the entry from DUMMY_FUNCTION_PASS to FUNCTION_PASS should fix this (will reproduce and fix locally). Never had a revert, what is the usual procedure to recommit after revert? (update/new diff, review, or just commit with the fix?)

Since it's already late on Friday I took the liberty to revert the commit so the bots can recover and you can investigate the failure at your own pace. Let me know if I can help with the fix!

This commit may have broken the -DLLVM_ENABLE_MODULES=1 build. See http://green.lab.llvm.org/green/view/LLDB/job/lldb-cmake/31603/consoleFull#2136199809a1ca8a51-895e-46c6-af87-ce24fa4cd561

FAILED: lib/CodeGen/CMakeFiles/LLVMCodeGen.dir/ExpandVectorPredication.cpp.o 
/Users/buildslave/jenkins/workspace/lldb-cmake/host-compiler/bin/clang++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS -Ilib/CodeGen -I/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/lib/CodeGen -Iinclude -I/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include -Wdocumentation -fPIC -fvisibility-inlines-hidden -Werror=date-time -Werror=unguarded-availability-new -fmodules -fmodules-cache-path=/Users/buildslave/jenkins/workspace/lldb-cmake/lldb-build/module.cache -fcxx-modules -Xclang -fmodules-local-submodule-visibility -Wall -Wextra -Wno-unused-parameter -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion -fdiagnostics-color -O3 -DNDEBUG -isysroot /Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX10.15.sdk    -fno-exceptions -fno-rtti -UNDEBUG -std=c++14 -MD -MT lib/CodeGen/CMakeFiles/LLVMCodeGen.dir/ExpandVectorPredication.cpp.o -MF lib/CodeGen/CMakeFiles/LLVMCodeGen.dir/ExpandVectorPredication.cpp.o.d -o lib/CodeGen/CMakeFiles/LLVMCodeGen.dir/ExpandVectorPredication.cpp.o -c /Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/lib/CodeGen/ExpandVectorPredication.cpp
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/lib/CodeGen/ExpandVectorPredication.cpp:461:30: error: redefinition of 'run'
ExpandVectorPredicationPass::run(Function &F, FunctionAnalysisManager &AM) {
                             ^
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/CodeGen/MachinePassRegistry.def:106:1: note: previous definition is here
DUMMY_FUNCTION_PASS("expandvp", ExpandVectorPredicationPass, ())
^
/Users/buildslave/jenkins/workspace/lldb-cmake/llvm-project/llvm/include/llvm/CodeGen/CodeGenPassBuilder.h:68:23: note: expanded from macro 'DUMMY_FUNCTION_PASS'
    PreservedAnalyses run(Function &, FunctionAnalysisManager &) {             \
                      ^
1 error generated.

Functionally this LGTM. Would be curious to do git archeology to understand why the old code used this pattern.

I believe that this is purely a limitation of AsmPrinter and that we could probably change its behavior to emit different linkage names in different instances, but I have not looked at what various debuggers expect here and DWARF GCC emits for C++ coroutines. I was mostly focussed on getting the Swift case right, while not disturbing C++.

Ah, so that's more about whether the name was demangleable - but I thought there was also an issue when the DW_AT_linkage_name differed from the real mangled name of the symbol, even if both names could be demangled?

What's this claim ^ about/based on? What happens if it doesn't hold the original linkage name?

@lxfind wrote:

Why doesn't replaceUsesOfWith work? Could you double check that replaceUsesOfWith doesn't work for DbgValueInst?

In D99693#2687996, @dblaikie wrote:

This patch does not affect C++ coroutines, since the DW_AT_specification is expected to hold the (original) linkage name.

So does this mean that for C++ we'll have a DW_TAG_subprogram with a DW_AT_linkage_name that doesn't match the symbol for that code in the object's symbol table?

Thanks!