thakis added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:75
+ for (llvm::sys::fs::directory_iterator DirIt(Directory, EC), DirEnd;
+ DirIt != DirEnd && !EC; DirIt.increment(EC)) {
+ if (!llvm::sys::fs::is_directory(DirIt->path()))
----------------
hans wrote:
> for being defensive, would it be safer to check !EC before comparing the
> iterators, in case DirIt is in a bad state because of an error?
Done (but this code is just moved up from below)
================
Comment at: clang/lib/Driver/ToolChains/MSVC.cpp:88
+
+ return !Highest.empty();
+}
----------------
hans wrote:
> Since the success of the function is directly tied to whether the string is
> empty, maybe just return the string directly to simplify the API?
>
> Oh I see, this comes from getWindows10SDKVersionFromPath(). Either way is
> fine I suppose.
Thanks, that's much nicer.
================
Comment at: clang/test/Driver/cl-sysroot.cpp:4
+
+// RUN: %clang_cl /winsysroot %t /c -- %t/foo.cpp
+// RUN: %clang_cl /vctoolsdir %t/VC/Tools/MSVC/14.26.28801 \
----------------
hans wrote:
> Driver/ tests don't usually run the actual compiler. Any reason not to just
> check the -### output?
I had that at first, but it felt a bit brittle to me. But sure, moved back to
that.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D95534/new/
https://reviews.llvm.org/D95534
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
