I found this function somewhat hard to read and removed a few entirely redundant checks and converted it to early exits.
Details
Details
Diff Detail
Diff Detail
Event Timeline
Comment Actions
LGTM if the answer to both my questions is "yes".
| lldb/source/Utility/ArchSpec.cpp | ||
|---|---|---|
| 1399 | Are you sure that TripleOSWasSpecified() == user_specified_triple.getOS() != llvm::Triple::UnknownOS. Was the check in the original code redundant? Looking at the implementation bool TripleOSWasSpecified() const { return !m_triple.getOSName().empty(); }I guess this is identical? But when dealing with triples it's probably worth double checking. | |
| 1402 | Same for the vendor. | |
| lldb/source/Utility/ArchSpec.cpp | ||
|---|---|---|
| 1399 | Great question, yes I checked that. The code for both https://llvm.org/doxygen/Triple_8cpp_source.html#l00524 is a StringSwitch that uses Unknown as the Default case. | |
| lldb/source/Utility/ArchSpec.cpp | ||
|---|---|---|
| 1399 | Thanks for checking! | |
Are you sure that TripleOSWasSpecified() == user_specified_triple.getOS() != llvm::Triple::UnknownOS. Was the check in the original code redundant? Looking at the implementation
bool TripleOSWasSpecified() const { return !m_triple.getOSName().empty(); }I guess this is identical? But when dealing with triples it's probably worth double checking.