This patch is a pre-commit for https://reviews.llvm.org/D105257. It adds support for the macOS -> Mac Catalyst version mapping to the Darwin SDK Info class, alongside appropriate unit tests.
Details
Diff Detail
- Repository
- rG LLVM Github Monorepo
Event Timeline
This patch depends on a small NFC commit that moves DarwinSDKInfo over to lib/Basic from lib/Driver, which isn't up on phabricator.
Thanks for splitting this out! It mostly looks good, just a few things inline.
clang/lib/Basic/DarwinSDKInfo.cpp | ||
---|---|---|
27–28 | Your answer (https://reviews.llvm.org/D105257#inline-1007563) to @aaron.ballman's comment in the original patch was informative: this check is to avoid infinite recursion. That wasn't obvious to me either though. Might you add something to the comment for the benefit of other readers? | |
118 | Nit: I'd suggest skipping the "in the next commit" part. Instead, I'd suggest documenting in the commit message why this wasn't done here:
| |
clang/unittests/Basic/DarwinSDKinfoTest.cpp | ||
36–50 | Can you add a couple of comments saying what the EXPECTs are testing? (Maybe not each one individually, but at least groups by what they're testing (maybe exists, fallback to major, infinite loop, big minimum value, etc., but in full sentences)). Should there be a test that passes both minimum and maximum values? |
Your answer (https://reviews.llvm.org/D105257#inline-1007563) to @aaron.ballman's comment in the original patch was informative: this check is to avoid infinite recursion. That wasn't obvious to me either though. Might you add something to the comment for the benefit of other readers?