[NFC][DwarfDebug] Avoid default capturing when using lambdas

[djtodoro]

Summary

It is bad practice to capture by default (via [&] in this case) when using lambdas, so we should avoid that as much as possible.
This patch fixes that in the getForwardingRegsDefinedByMI from DwarfDebug module.

[dstenb]

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

710

Perhaps ForwardedRegWorklist even should be passed as a parameter? That could further show the intent of the function.

[djtodoro]

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

710

Makes sense to me, thanks!

[djtodoro]

-addressing comments

[dstenb]

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

710

Oh, and as TRI is a pointer maybe the reference should not be used here? Perhaps we should change the auto references to pointers at line 655-657 to stress that?

[djtodoro]

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

710

Hm, good point. We should avoid auto type deduction when the type is so obvious.
I'll add another small NFC patch refactoring that as well.

[djtodoro]

-rebasing

[dblaikie]

It is bad practice to capture by default (via [&] in this case) when using lambdas

I'd say quite the opposite - default reference capture should be used by default for any lambda that does not escape its context - same as any other code block. (I think this is more unambiguous when the lambda is never named, and just immediately passed into another function like `llvm::any_of```, and I'm not 100% on never enumerating captures when a lambda is named and maybe called several times in more surprising/distant places - but this lambda is called directly, and exactly once, which makes me question the use of a lambda here - I'd consider pulling it out as a separate named function, or just removing the lambda entirely & adding a comment block describing the purpose of the code inline)

Please revert this.

[djtodoro]

I'd consider pulling it out as a separate named function, or just removing the lambda entirely & adding a comment block describing the purpose of the code inline)

Thanks for the comment and explanation! A separate function makes sense to me.

I just reverted this.

[dblaikie]

In D79616#2030976, @djtodoro wrote:

I'd consider pulling it out as a separate named function, or just removing the lambda entirely & adding a comment block describing the purpose of the code inline)

Thanks for the comment and explanation! A separate function makes sense to me.

I just reverted this.

Thanks! Yeah, if you want to pull this out into a separate top level function I'd be fine with that - given it's only capturing one value, moving that to be a normal parameter doesn't seem like it'd make this significantly less legible, etc.

llvm/lib/CodeGen/AsmPrinter/DwarfDebug.cpp

static void collectCallSiteParameters(const MachineInstr *CallMI,
   // the describeLoadedValue(), we try to generate an entry value expression
   // for their call site value description, if the call is within the entry MBB.
   // TODO: Handle situations when call site parameter value can be described
   // as the entry value within basic blocks other than the first one.
   bool ShouldTryEmitEntryVals = MBB->getIterator() == MF->begin();
 
   // If the MI is an instruction defining one or more parameters' forwarding
   // registers, add those defines.
-  auto getForwardingRegsDefinedByMI = [&](const MachineInstr &MI,
-                                          SmallSetVector<unsigned, 4> &Defs) {
+  auto getForwardingRegsDefinedByMI =
+      [&TRI](const MachineInstr &MI, SmallSetVector<unsigned, 4> &Defs,
+             const FwdRegWorklist &ForwardedRegWorklist) {
         if (MI.isDebugInstr())
           return;
 
         for (const MachineOperand &MO : MI.operands()) {
           if (MO.isReg() && MO.isDef() &&
               Register::isPhysicalRegister(MO.getReg())) {
             for (auto FwdReg : ForwardedRegWorklist)
               if (TRI.regsOverlap(FwdReg.first, MO.getReg()))
                 Defs.insert(FwdReg.first);
           }
         }
       };
 
   // Search for a loading value in forwarding registers.
   for (; I != MBB->rend(); ++I) {
     // Skip bundle headers.
     if (I->isBundle())
       continue;
 
     // If the next instruction is a call we can not interpret parameter's
     // forwarding registers or we finished the interpretation of all parameters.
     if (I->isCall())
       return;
 
     if (ForwardedRegWorklist.empty())
       return;
 
     // Set of worklist registers that are defined by this instruction.
     SmallSetVector<unsigned, 4> FwdRegDefs;
 
-    getForwardingRegsDefinedByMI(*I, FwdRegDefs);
+    getForwardingRegsDefinedByMI(*I, FwdRegDefs, ForwardedRegWorklist);
     if (FwdRegDefs.empty())
       continue;
 
     for (auto ParamFwdReg : FwdRegDefs) {
       if (auto ParamValue = TII.describeLoadedValue(*I, ParamFwdReg)) {
         if (ParamValue->first.isImm()) {
           int64_t Val = ParamValue->first.getImm();
           finishCallSiteParams(Val, ParamValue->second,