This patch modernizes the GetSDKVersion API and hopefully prevents problems such as the ones discovered in D61218.
I decided to go for a vector because otherwise the implementation in ObjectFileMachO gets a bit awkward. But I have no problem with doing a VersionTuple instead and adding some manual translation to version tuple in ObjectFileMachO.
I think it would be better to use a VersionTuple, for consistency with other version uses. I don't see why the MachO implementation should be awkward because of that. You'd just need to replace the three push_back lines with m_sdk_version = VersionTuple(xxxx, yy, zz);. In fact, if at the same time you rewrite the GetSDKVersion into something like:
if (m_sdk_version.empty()) m_sdk_version = GetVersionFromLC_VERSION()); if (m_sdk_version.empty()) m_sdk_version = GetVersionFromLC_BUILD_VERSION()); return m_sdk_version;
that code will become at least five times more readable than it is now. :)
Awesome. I should have done that right away. This isn't super important, but I feel like just returning a vector (with potentially zero elements would be just as good) and the Optional doesn't add much value.
|211–212 ↗||(On Diff #206451)|
For my own education, why is this preferred over versions? Because of Optional's operator->?