[clang] d038383 - Make -Wnonportable-include-path ignore drive case on Windows.

Nico Weber via cfe-commits cfe-commits at lists.llvm.org
Thu May 7 12:54:31 PDT 2020


Author: Nico Weber
Date: 2020-05-07T15:54:09-04:00
New Revision: d03838343f2199580a1942eb353901add38af909

URL: https://github.com/llvm/llvm-project/commit/d03838343f2199580a1942eb353901add38af909
DIFF: https://github.com/llvm/llvm-project/commit/d03838343f2199580a1942eb353901add38af909.diff

LOG: Make -Wnonportable-include-path ignore drive case on Windows.

See PR45812 for motivation.

No explicit test since I couldn't figure out how to get the
current disk drive in lower case into a form in lit where I could
mkdir it and cd to it. But the change does have test coverage in
that I can remove the case normalization in lit, and tests failed
on several bots (and for me locally if in a pwd with a lower-case
drive) without that normalization prior to this change.

Differential Revision: https://reviews.llvm.org/D79531

Added: 
    clang/test/Lexer/case-insensitive-include-win.c

Modified: 
    clang/lib/Lex/PPDirectives.cpp
    llvm/cmake/modules/AddLLVM.cmake

Removed: 
    


################################################################################
diff  --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 8b1d1fb320e7..84c13b286e68 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2074,6 +2074,8 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
   if (Callbacks && !IsImportDecl) {
     // Notify the callback object that we've seen an inclusion directive.
     // FIXME: Use a 
diff erent callback for a pp-import?
+    // FIXME: Passes wrong filename if LookupHeaderIncludeOrImport() did
+    // typo correction.
     Callbacks->InclusionDirective(
         HashLoc, IncludeTok, LookupFilename, isAngled, FilenameRange,
         File ? &File->getFileEntry() : nullptr, SearchPath, RelativePath,
@@ -2100,10 +2102,41 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
       !IsMapped && !File->getFileEntry().tryGetRealPathName().empty();
 
   if (CheckIncludePathPortability) {
+    // FIXME: Looks at the wrong filename if we did typo correction.
     StringRef Name = LookupFilename;
+    StringRef NameWithoriginalSlashes = Filename;
+#if defined(_WIN32)
+    // Skip UNC prefix if present. (tryGetRealPathName() always
+    // returns a path with the prefix skipped.)
+    bool NameWasUNC = Name.consume_front("\\\\?\\");
+    NameWithoriginalSlashes.consume_front("\\\\?\\");
+#endif
     StringRef RealPathName = File->getFileEntry().tryGetRealPathName();
     SmallVector<StringRef, 16> Components(llvm::sys::path::begin(Name),
                                           llvm::sys::path::end(Name));
+#if defined(_WIN32)
+    // -Wnonportable-include-path is designed to diagnose includes using
+    // case even on systems with a case-insensitive file system.
+    // On Windows, RealPathName always starts with an upper-case drive
+    // letter for absolute paths, but Name might start with either
+    // case depending on if `cd c:\foo` or `cd C:\foo` was used in the shell.
+    // ("foo" will always have on-disk case, no matter which case was
+    // used in the cd command). To not emit this warning solely for
+    // the drive letter, whose case is dependent on if `cd` is used
+    // with upper- or lower-case drive letters, always consider the
+    // given drive letter case as correct for the purpose of this warning.
+    SmallString<128> FixedDriveRealPath;
+    if (llvm::sys::path::is_absolute(Name) &&
+        llvm::sys::path::is_absolute(RealPathName) &&
+        toLowercase(Name[0]) == toLowercase(RealPathName[0]) &&
+        isLowercase(Name[0]) != isLowercase(RealPathName[0])) {
+      assert(Components.size() >= 3 && "should have drive, backslash, name");
+      assert(Components[0].size() == 2 && "should start with drive");
+      assert(Components[0][1] == ':' && "should have colon");
+      FixedDriveRealPath = (Name.substr(0, 1) + RealPathName.substr(1)).str();
+      RealPathName = FixedDriveRealPath;
+    }
+#endif
 
     if (trySimplifyPath(Components, RealPathName)) {
       SmallString<128> Path;
@@ -2132,15 +2165,23 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
           continue;
 
         // Append the separator(s) the user used, or the close quote
-        if (Path.size() > Filename.size()) {
+        if (Path.size() > NameWithoriginalSlashes.size()) {
           Path.push_back(isAngled ? '>' : '"');
           continue;
         }
-        assert(IsSep(Filename[Path.size()-1]));
+        assert(IsSep(NameWithoriginalSlashes[Path.size()-1]));
         do
-          Path.push_back(Filename[Path.size()-1]);
-        while (Path.size() <= Filename.size() && IsSep(Filename[Path.size()-1]));
+          Path.push_back(NameWithoriginalSlashes[Path.size()-1]);
+        while (Path.size() <= NameWithoriginalSlashes.size() &&
+               IsSep(NameWithoriginalSlashes[Path.size()-1]));
       }
+
+#if defined(_WIN32)
+      // Restore UNC prefix if it was there.
+      if (NameWasUNC)
+        Path = (Path.substr(0, 1) + "\\\\?\\" + Path.substr(1)).str();
+#endif
+
       // For user files and known standard headers, issue a diagnostic.
       // For other system headers, don't. They can be controlled separately.
       auto DiagId =

diff  --git a/clang/test/Lexer/case-insensitive-include-win.c b/clang/test/Lexer/case-insensitive-include-win.c
new file mode 100644
index 000000000000..f88fa80ffa95
--- /dev/null
+++ b/clang/test/Lexer/case-insensitive-include-win.c
@@ -0,0 +1,10 @@
+// Most Microsoft-specific testing should go in case-insensitive-include-ms.c
+// This file should only include code that really needs a Windows host OS to
+// run.
+
+// REQUIRES: system-windows
+// RUN: mkdir -p %t.dir
+// RUN: touch %t.dir/foo.h
+// RUN: not %clang_cl /FI\\?\%t.dir\FOO.h /WX -Xclang -verify -fsyntax-only %s 2>&1 | FileCheck %s
+
+// CHECK: non-portable path to file '"\\?\

diff  --git a/llvm/cmake/modules/AddLLVM.cmake b/llvm/cmake/modules/AddLLVM.cmake
index b4bc15e16e8e..f40d99822d5e 100644
--- a/llvm/cmake/modules/AddLLVM.cmake
+++ b/llvm/cmake/modules/AddLLVM.cmake
@@ -1499,7 +1499,6 @@ string(CONCAT LLVM_LIT_PATH_FUNCTION
   "def path(p):\n"
   "    if not p: return ''\n"
   "    p = os.path.join(os.path.dirname(os.path.abspath(__file__)), p)\n"
-  "    if os.name == 'nt' and os.path.isabs(p): return p[0].upper() + p[1:]\n"
   "    return p\n"
   )
 


        


More information about the cfe-commits mailing list