[PATCH] D125784: [llvm-debuginfo-analyzer] 09 - CodeView Reader
Carlos Alberto Enciso via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Oct 14 01:52:08 PDT 2022
CarlosAlbertoEnciso added inline comments.
================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewReader.cpp:422
+ Expected<InfoStream &> expectedInfo = Pdb.getPDBInfoStream();
+ if (expectedInfo && expectedInfo->getGuid() != TS.getGuid())
+ return createStringError(errc::invalid_argument, "signature_out_of_date");
----------------
probinson wrote:
> If we have to check `expectedInfo` anyway, let's not assume `getPDBInfoStream` succeeds.
Good suggestion. Changed.
================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:226
+ using LookupSet = std::set<StringRef>;
+ LookupSet DeductedScopes;
+ LookupSet UnresolvedScopes;
----------------
probinson wrote:
>
Changed.
================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:3404
+ break;
+ }
+ }
----------------
probinson wrote:
> Isn't this `for` loop (which does a `break` on the first iteration) equivalent to
> ```
> if (!Locations->empty())
> ParentLowPC = Locations->begin()->getLowerAddress();
> ```
Good finding. Replaced with
```
if (const LVLocations *Locations = Parent->getRanges()) {
if (!Locations->empty())
ParentLowPC = (*Locations->begin())->getLowerAddress();
}
```
================
Comment at: llvm/lib/DebugInfo/LogicalView/Readers/LVCodeViewVisitor.cpp:3496
+ SeenHighAddress = true;
+ break;
+ }
----------------
probinson wrote:
> `break` not needed here
Removed.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D125784/new/
https://reviews.llvm.org/D125784
More information about the llvm-commits
mailing list