steven_wu added inline comments.
================ 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: > > 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? > Hmm, we want to assume that version exists for future uses. I feel like the > current type captures the intention better. I think it is fine for current use. We can always change in the future. 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
