jarin added inline comments.
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp:3545
+
+DIEVector MergeBlockAbstractParameters(const DWARFDIE &block_die,
+ DIEVector &&variable_dies) {
----------------
labath wrote:
> In llvm, we prefer `static` functions over anonymous namespaces.
> Theoretically, you could keep the anonymous namespace around the using
> declaration (per
> <https://llvm.org/docs/CodingStandards.html#anonymous-namespaces>, as those
> can't use `static`), though I would actually probably prefer DIEArray type
> defined in DIERef.h over a custom type.
Changed to static function, DIEArray (interestingly, this file actually starts
with anonymous namespace, see line 121).
================
Comment at: lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h:403
+ const llvm::SmallVectorImpl<DWARFDIE> &variable_dies,
+ const lldb::addr_t func_low_pc);
----------------
labath wrote:
> We generally do not put a const qualifier on by-value arguments (it's pretty
> useless).
>
> (I see it's present on other functions too, but I don't know if they were
> introduced by you or you're just propagating them.)
Copy pasta, unfortunately.
================
Comment at:
lldb/test/API/functionalities/unused-inlined-parameters/TestUnusedInlinedParameters.py:17
+
+ # For the unused parameters, only check the types.
+ self.assertEqual("void *",
self.frame().FindVariable("unused1").GetType().GetName())
----------------
labath wrote:
> jarin wrote:
> > labath wrote:
> > > Maybe we could check something else as well... Do `GetDescription` or
> > > `str(value)` return something reasonable here?
> > Actually, the most important thing is the type here, so this was quite
> > deliberate.
> >
> > GetDescription returns `(void *) unused1 = <no location, value may have
> > been optimized out>\n\n`, but I am not sure if we can safely require that
> > all future versions/platforms optimize the parameter out.
> I don't feel strongly about it, but I would say that this function is so
> simple than any optimizer worthy of that name should be able to optimize
> those arguments away. I might replace printf with a `noinline`/`optnone`
> function though, to avoid any libc shenanigans.
I do not feel too strongly about this eiher.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D110571/new/
https://reviews.llvm.org/D110571
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
