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
I guess this is identical? But when dealing with triples it's probably worth double checking.