[PATCH] D158678: [NFC][AsmPrinter] Use std::visit in constructVariableDIEImpl

Fri Aug 25 12:41:33 PDT 2023

scott.linder added inline comments.

================
Comment at: llvm/lib/CodeGen/AsmPrinter/DwarfCompileUnit.cpp:776
+      makeVisitor(
+          [=, &DV](const Loc::Single &Single) {
+            const DbgValueLoc *DVal = &Single.getValueLoc();
----------------
fdeazeve wrote:
> What do you think of implementing each of the visitors as a private member of `DwarfCompileUnit`?
> I personally think this function could benefit from being split into different functions for readability, but this is a larger refactor.
I definitely considered it, and agree it is worthwhile! 

The whole series was an attempt to keep the changes gradual and understandable, and to leave open the option to drop things piecemeal. I try to assume the original intent  has some rationale I am not considering, as that is often the case anyway (a lot of smart, dedicated people have contributed here) and I always worry that I'm changing things just to change them.

Hearing you come to the same conclusion makes me confident it is an improvement, though, so I will post a follow up!

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D158678/new/

https://reviews.llvm.org/D158678