steven_wu accepted this revision. steven_wu added a comment. This revision is now accepted and ready to land.
Other than a small design choice commented inline, LGTM. ================ Comment at: include/clang/Driver/DarwinSDKInfo.h:36 +/// SDK has no SDKSettings.json, or a valid \c DarwinSDKInfo otherwise. +Expected<Optional<DarwinSDKInfo>> parseDarwinSDKInfo(llvm::vfs::FileSystem &VFS, + StringRef SDKRootPath); ---------------- arphaman wrote: > steven_wu wrote: > > Isn't parseSDKSettings enough? And it can just return > > Optional<VersionTuple>? > We will support other fields besides `VersionTuple` in the SDKSettings, so > that's why we have a structure. I feel like for this usage, it is better to return Expected<DarwinSDKInfo> with all the fields being Optional? Repository: rC Clang CHANGES SINCE LAST ACTION https://reviews.llvm.org/D55673/new/ https://reviews.llvm.org/D55673 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
