thakis updated this revision to Diff 261481.
thakis edited the summary of this revision.

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D79223/new/

https://reviews.llvm.org/D79223

Files:
  clang/lib/Lex/PPDirectives.cpp
  clang/test/Lexer/case-insensitive-include-ms.c
  clang/test/Lexer/case-insensitive-include-pr31836.sh
  clang/test/Lexer/case-insensitive-include.c
  llvm/include/llvm/Support/Path.h

Index: llvm/include/llvm/Support/Path.h
===================================================================
--- llvm/include/llvm/Support/Path.h
+++ llvm/include/llvm/Support/Path.h
@@ -47,7 +47,7 @@
 ///   foo/       => foo,.
 ///   /foo/bar   => /,foo,bar
 ///   ../        => ..,.
-///   C:\foo\bar => C:,/,foo,bar
+///   C:\foo\bar => C:,\,foo,bar
 /// @endcode
 class const_iterator
     : public iterator_facade_base<const_iterator, std::input_iterator_tag,
Index: clang/test/Lexer/case-insensitive-include.c
===================================================================
--- clang/test/Lexer/case-insensitive-include.c
+++ clang/test/Lexer/case-insensitive-include.c
@@ -16,11 +16,11 @@
 #include "Case-Insensitive-Include.h" // expected-warning {{non-portable path}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:38}:"\"case-insensitive-include.h\""
 
-#include "../Output/./case-insensitive-include.h"
-#include "../Output/./Case-Insensitive-Include.h" // expected-warning {{non-portable path}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"../Output/./case-insensitive-include.h\""
-#include "../output/./case-insensitive-include.h" // expected-warning {{non-portable path}}
-// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"../Output/./case-insensitive-include.h\""
+#include "../Output/.//case-insensitive-include.h"
+#include "../Output/.//Case-Insensitive-Include.h" // expected-warning {{non-portable path}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:51}:"\"../Output/.//case-insensitive-include.h\""
+#include "../output/.//case-insensitive-include.h" // expected-warning {{non-portable path}}
+// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:51}:"\"../Output/.//case-insensitive-include.h\""
 
 #include "apath/.././case-insensitive-include.h"
 #include "apath/.././Case-Insensitive-Include.h" // expected-warning {{non-portable path}}
Index: clang/test/Lexer/case-insensitive-include-pr31836.sh
===================================================================
--- clang/test/Lexer/case-insensitive-include-pr31836.sh
+++ clang/test/Lexer/case-insensitive-include-pr31836.sh
@@ -1,5 +1,4 @@
 // REQUIRES: case-insensitive-filesystem
-// UNSUPPORTED: system-windows
 
 // RUN: mkdir -p %t
 // RUN: touch %t/case-insensitive-include-pr31836.h
Index: clang/test/Lexer/case-insensitive-include-ms.c
===================================================================
--- clang/test/Lexer/case-insensitive-include-ms.c
+++ clang/test/Lexer/case-insensitive-include-ms.c
@@ -6,6 +6,8 @@
 // RUN: %clang_cc1 -fsyntax-only -fms-compatibility %s -include %s -I %t/Output -verify
 // RUN: %clang_cc1 -fsyntax-only -fms-compatibility -fdiagnostics-parseable-fixits %s -include %s -I %t/Output 2>&1 | FileCheck %s
 
+// FIXME: Add a test with repeated backslashes once clang can handle that
+// in ms-compat mode on non-Windows hosts.
 #include "..\Output\.\case-insensitive-include.h"
 #include "..\Output\.\Case-Insensitive-Include.h" // expected-warning {{non-portable path}}
 // CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:50}:"\"..\\Output\\.\\case-insensitive-include.h\""
Index: clang/lib/Lex/PPDirectives.cpp
===================================================================
--- clang/lib/Lex/PPDirectives.cpp
+++ clang/lib/Lex/PPDirectives.cpp
@@ -1918,14 +1918,18 @@
   SourceLocation FilenameLoc = FilenameTok.getLocation();
   StringRef LookupFilename = Filename;
 
-#ifndef _WIN32
+#ifdef _WIN32
+  bool BackslashIsSeparator = true;
+#else
   // Normalize slashes when compiling with -fms-extensions on non-Windows. This
   // is unnecessary on Windows since the filesystem there handles backslashes.
   SmallString<128> NormalizedPath;
+  bool BackslashIsSeparator = false;
   if (LangOpts.MicrosoftExt) {
     NormalizedPath = Filename.str();
     llvm::sys::path::native(NormalizedPath);
     LookupFilename = NormalizedPath;
+    BackslashIsSeparator = true;
   }
 #endif
 
@@ -2108,21 +2112,44 @@
       SmallString<128> Path;
       Path.reserve(Name.size()+2);
       Path.push_back(isAngled ? '<' : '"');
-      bool isLeadingSeparator = llvm::sys::path::is_absolute(Name);
+
+      const auto IsSep = [BackslashIsSeparator](char c) {
+        return c == '/' || (BackslashIsSeparator && c == '\\');
+      };
+
       for (auto Component : Components) {
-        if (isLeadingSeparator)
-          isLeadingSeparator = false;
-        else
+        // On POSIX, Components will contain a single '/' as first element
+        // exactly if Name is an absolute path.
+        // On Windows, it will contain "C:" followed by '\' for absolute paths.
+        // The drive letter is optional for absolute paths on Windows, but
+        // clang currently cannot process absolute paths in #include lines that
+        // don't have a drive.
+        // If the first entry in Components is a directory separator,
+        // then the code at the bottom of this loop that keeps the original
+        // directory separator style copies it. If the second entry is
+        // a directory separator (the C:\ case), then that separator already
+        // got copied when the C: was processed and we want to skip that entry.
+        if (!(Component.size() == 1 && IsSep(Component[0])))
           Path.append(Component);
-        // Append the separator the user used, or the close quote
-        Path.push_back(
-          Path.size() <= Filename.size() ? Filename[Path.size()-1] :
-            (isAngled ? '>' : '"'));
+        else if (!Path.empty())
+          continue;
+
+        // Append the separator(s) the user used, or the close quote
+        if (Path.size() > Filename.size()) {
+          Path.push_back(isAngled ? '>' : '"');
+          continue;
+        }
+        assert(IsSep(Filename[Path.size()-1]));
+        do
+          Path.push_back(Filename[Path.size()-1]);
+        while (Path.size() <= Filename.size() && IsSep(Filename[Path.size()-1]));
       }
-      // For user files and known standard headers, by default we issue a diagnostic.
-      // For other system headers, we don't. They can be controlled separately.
-      auto DiagId = (FileCharacter == SrcMgr::C_User || warnByDefaultOnWrongCase(Name)) ?
-          diag::pp_nonportable_path : diag::pp_nonportable_system_path;
+      // For user files and known standard headers, issue a diagnostic.
+      // For other system headers, don't. They can be controlled separately.
+      auto DiagId =
+          (FileCharacter == SrcMgr::C_User || warnByDefaultOnWrongCase(Name))
+              ? diag::pp_nonportable_path
+              : diag::pp_nonportable_system_path;
       Diag(FilenameTok, DiagId) << Path <<
         FixItHint::CreateReplacement(FilenameRange, Path);
     }
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to