arphaman marked an inline comment as done. arphaman 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); ---------------- 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. 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
