[PATCH] D37696: [dwarfdump] Add DWARF verifiers for address ranges
David Blaikie via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Mon Sep 11 14:33:21 PDT 2017
dblaikie added a comment.
Does this handle the interesting cases (test cases are a bit verbose, so I admit I haven't looked through them carefully) that came up in the original review:
Nested functions (a function inside a function - where the inner function shouldn't be contained within the ranges of the outer function (I find this weird, but apparently this is what Adrian & Paul & others think is right, and I can sort of see it - LLVM doesn't produce anything like this))?
Maybe that was the only interesting one...
================
Comment at: include/llvm/DebugInfo/DWARF/DWARFVerifier.h:46
+ DieRangeInfo() = default;
+ DieRangeInfo(DWARFDie Die) : Die(Die), Ranges(), Children() {}
+
----------------
Drop "Ranges(), Children()" from here - they're the default
================
Comment at: include/llvm/DebugInfo/DWARF/DWARFVerifier.h:50
+ DieRangeInfo(std::vector<DWARFAddressRange> Ranges)
+ : Die(), Ranges(Ranges), Children() {}
+
----------------
Drop the "Die()" and "Children()" from here - they're the default anyway?
================
Comment at: include/llvm/DebugInfo/DWARF/DWARFVerifier.h:50
+ DieRangeInfo(std::vector<DWARFAddressRange> Ranges)
+ : Die(), Ranges(Ranges), Children() {}
+
----------------
dblaikie wrote:
> Drop the "Die()" and "Children()" from here - they're the default anyway?
use "Ranges(std::move(Ranges))" to avoid an unnecessary copy of the Ranges vector
================
Comment at: lib/DebugInfo/DWARF/DWARFVerifier.cpp:35-36
+ if (Pos->intersects(R))
+ return Pos;
+ else if (Pos != Begin) {
+ auto Iter = Pos - 1;
----------------
drop else after return
Repository:
rL LLVM
https://reviews.llvm.org/D37696
More information about the llvm-commits
mailing list