[PATCH] D22655: Avoid dsymutil calls to getFileNameByIndex
Frederic Riss via llvm-commits
llvm-commits at lists.llvm.org
Thu Jul 21 18:24:57 PDT 2016
friss accepted this revision.
friss added a comment.
This revision is now accepted and ready to land.
This LGTM if you agree with my comments. Thanks for the nice speed boost!
================
Comment at: include/llvm/DebugInfo/DWARF/DWARFDebugLine.h:191-192
@@ -190,1 +190,4 @@
+ bool hasFileNameByIndex(uint64_t FileIndex,
+ DILineInfoSpecifier::FileLineInfoKind Kind) const;
+
----------------
I know you modeled the name on the function below, but it reads strange to me. How about just
```
bool hasFileAtIndex(uint64_t FileIndex) const;
```
(ie also drop the Kind parameter, I don't think it makes sense to test it here.)
================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:630-633
@@ -631,3 +629,6 @@
+ FileLineInfoKind Kind) const {
if (FileIndex == 0 || FileIndex > Prologue.FileNames.size() ||
Kind == FileLineInfoKind::None)
return false;
+ return true;
+}
----------------
I like
```
return FileIndex != 0 && FileIndex <= Prologue.FileNames.size() ;
```
better (if you drop the Kind like I said above)
================
Comment at: lib/DebugInfo/DWARF/DWARFDebugLine.cpp:641
@@ +640,3 @@
+ std::string &Result) const {
+ if (!hasFileNameByIndex(FileIndex, Kind))
+ return false;
----------------
Then this becomes:
```
if (Kind == FileLineInfoKind::None || !hasFileAtIndex(FileIndex))
```
Repository:
rL LLVM
https://reviews.llvm.org/D22655
More information about the llvm-commits
mailing list