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" ) _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
