The unobufscation support for BCSymbolMaps was the last piece of code that hasn't been upstreamed yet. This patch contains a reworked version of the existing code and relevant tests.
Details
Diff Detail
- Repository
- rL LLVM
Event Timeline
| llvm/test/tools/dsymutil/ARM/obfuscated.test | ||
|---|---|---|
| 5 | nit: how about splitting those lines at 80 chars? RUN: ... \ RUN: -... | |
| llvm/tools/dsymutil/DwarfStreamer.cpp | ||
| 581 | comment should be on the declaration not the implementation, I believe. | |
| 597 | Any plans to implement this soon? | |
| llvm/tools/dsymutil/SymbolMap.cpp | ||
| 50 | s/that weird/cross reference to the MC code that explains why we do this/ ? | |
| 54 | this is in namespace llvm, does this need to be qualified? | |
| 68 | It's unfortunate that dsymutil on non-macOS will behave differently... but this is not a regression | |
| 69 | . | |
| 74 | why parent_path first and then ..? | |
| 93 | perhaps introduce a local var to make this more readable with 80 chars? | |
| 133 | perhaps use StringRef::consume() for premature optimization? | |
| llvm/tools/dsymutil/SymbolMap.h | ||
| 22 | Doxygen comment? | |
| 26 | ArrayRef or && or const& perhaps? | |
Feedback Adrian
| llvm/tools/dsymutil/DwarfStreamer.cpp | ||
|---|---|---|
| 597 | Soon, hopefully :-) | |
| llvm/tools/dsymutil/SymbolMap.cpp | ||
| 133 | I tried but this makes the code more complicated because we need to account for either the version number missing or the whole thing missing. | |
| llvm/tools/dsymutil/SymbolMap.h | ||
| 26 | Neither would improve performance as we're moving it into a member. I believe it's customary to take the argument by value in this case (although an r-value reference would be equivalent). | |
| llvm/tools/dsymutil/dsymutil.cpp | ||
|---|---|---|
| 515 | That comment doesn't carry a lot of information :-) | |
nit: how about splitting those lines at 80 chars?