llvmbot wrote:
<!--LLVM PR SUMMARY COMMENT--> @llvm/pr-subscribers-clang Author: Rose Hudson (rosefromthedead) <details> <summary>Changes</summary> As part of -Wnonportable-include-path, warn when an include path contains a backslash, and emit a fixit that changes all backslashes to forward slashes. --- Full diff: https://github.com/llvm/llvm-project/pull/186770.diff 7 Files Affected: - (modified) clang/include/clang/Basic/DiagnosticLexKinds.td (+1-3) - (modified) clang/include/clang/Basic/SourceManager.h (+18) - (modified) clang/lib/InstallAPI/Frontend.cpp (+6-2) - (modified) clang/lib/Lex/PPDirectives.cpp (+33-2) - (added) clang/test/Lexer/backslash-include-win.c (+14) - (modified) clang/test/Lexer/case-insensitive-include-ms.c (+13-13) - (modified) clang/test/Lexer/case-insensitive-include-win.c (+3-2) ``````````diff diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td index 5eceeced311f2..17789a515d593 100644 --- a/clang/include/clang/Basic/DiagnosticLexKinds.td +++ b/clang/include/clang/Basic/DiagnosticLexKinds.td @@ -373,9 +373,7 @@ def ext_missing_whitespace_after_macro_name : ExtWarn< def warn_missing_whitespace_after_macro_name : Warning< "whitespace recommended after macro name">; -class NonportablePath : Warning< - "non-portable path to file '%0'; specified path differs in case from file" - " name on disk">; +class NonportablePath : Warning<"non-portable path to file '%0'; %1">; def pp_nonportable_path : NonportablePath, InGroup<DiagGroup<"nonportable-include-path">>; def pp_nonportable_system_path : NonportablePath, DefaultIgnore, diff --git a/clang/include/clang/Basic/SourceManager.h b/clang/include/clang/Basic/SourceManager.h index bc9e97863556d..e097e8c14d331 100644 --- a/clang/include/clang/Basic/SourceManager.h +++ b/clang/include/clang/Basic/SourceManager.h @@ -1526,6 +1526,24 @@ class SourceManager : public RefCountedBase<SourceManager> { return Filename == "<scratch space>"; } + /// Returns whether \p Loc is located in a <module-includes> file. + bool isWrittenInModuleIncludes(SourceLocation Loc) const { + PresumedLoc Presumed = getPresumedLoc(Loc); + if (Presumed.isInvalid()) + return false; + StringRef Filename(Presumed.getFilename()); + return Filename == "<module-includes>"; + } + + /// Returns whether \p Loc is located in a <extract-api-includes> file. + bool isWrittenInExtractAPIIncludes(SourceLocation Loc) const { + PresumedLoc Presumed = getPresumedLoc(Loc); + if (Presumed.isInvalid()) + return false; + StringRef Filename(Presumed.getFilename()); + return Filename == "<extract-api-includes>"; + } + /// Returns whether \p Loc is located in a built-in or command line source. bool isInPredefinedFile(SourceLocation Loc) const { PresumedLoc Presumed = getPresumedLoc(Loc); diff --git a/clang/lib/InstallAPI/Frontend.cpp b/clang/lib/InstallAPI/Frontend.cpp index cce0b19b50619..3168d6143e168 100644 --- a/clang/lib/InstallAPI/Frontend.cpp +++ b/clang/lib/InstallAPI/Frontend.cpp @@ -148,8 +148,12 @@ std::unique_ptr<MemoryBuffer> createInputBuffer(InstallAPIContext &Ctx) { OS << "#import "; if (H.useIncludeName()) OS << "<" << H.getIncludeName() << ">\n"; - else - OS << "\"" << H.getPath() << "\"\n"; + else { + // Don't trigger -Wnonportable-include-path + std::string Path = H.getPath().str(); + std::replace(Path.begin(), Path.end(), '\\', '/'); + OS << "\"" << Path << "\"\n"; + } Ctx.addKnownHeader(H); } diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 4a854c213926b..0fba46fd90fa0 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -2675,6 +2675,8 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( } #endif + std::optional<SmallString<128>> SuggestedPath; + bool HasCaseDifference = false; if (trySimplifyPath(Components, RealPathName, BackslashStyle)) { SmallString<128> Path; Path.reserve(Name.size()+2); @@ -2719,14 +2721,43 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport( Path = (Path.substr(0, 1) + "\\\\?\\" + Path.substr(1)).str(); #endif + SuggestedPath = std::make_optional(std::move(Path)); + HasCaseDifference = true; + } + + bool HasReplaceableBackslash = false; + if (Name.contains('\\')) { + if (!SuggestedPath.has_value()) { + SuggestedPath = std::make_optional(OriginalFilename); + } + std::replace(SuggestedPath->begin(), SuggestedPath->end(), '\\', '/'); + HasReplaceableBackslash = true; + } + + SourceLocation FilenameLoc = FilenameTok.getLocation(); + bool SuppressDiag = FilenameLoc.isMacroID() || + SourceMgr.isWrittenInBuiltinFile(FilenameLoc) || + SourceMgr.isWrittenInModuleIncludes(FilenameLoc) || + SourceMgr.isWrittenInExtractAPIIncludes(FilenameLoc); + if (SuggestedPath.has_value() && !SuppressDiag) { // 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); + auto Diagnostic = + Diag(FilenameTok, DiagId) + << SuggestedPath.value() + << FixItHint::CreateReplacement(FilenameRange, SuggestedPath.value()); + if (HasCaseDifference && HasReplaceableBackslash) { + Diagnostic << "specified path contains backslashes and differs in case " + "from file name on disk"; + } else if (HasReplaceableBackslash) { + Diagnostic << "specified path contains backslashes"; + } else if (HasCaseDifference) { + Diagnostic << "specified path differs in case from file name on disk"; + } } } diff --git a/clang/test/Lexer/backslash-include-win.c b/clang/test/Lexer/backslash-include-win.c new file mode 100644 index 0000000000000..e95098a06e1e4 --- /dev/null +++ b/clang/test/Lexer/backslash-include-win.c @@ -0,0 +1,14 @@ +// REQUIRES: system-windows +// RUN: mkdir -p %t/backslash +// RUN: cp %S/Inputs/case-insensitive-include.h %t/backslash/case-insensitive-include.h +// RUN: %clang -fsyntax-only -I%t %s 2>&1 | FileCheck %s + +#include "backslash\case-insensitive-include.h" +// CHECK: non-portable path to file +// CHECK: specified path contains backslashes +// CHECK: "backslash/case-insensitive-include.h" + +#include "backslash\CASE-insensitive-include.h" +// CHECK: non-portable path to file +// CHECK: specified path contains backslashes and differs in case from file name on disk +// CHECK: "backslash/case-insensitive-include.h" diff --git a/clang/test/Lexer/case-insensitive-include-ms.c b/clang/test/Lexer/case-insensitive-include-ms.c index f7af1fef8b4e6..8fa73010750d7 100644 --- a/clang/test/Lexer/case-insensitive-include-ms.c +++ b/clang/test/Lexer/case-insensitive-include-ms.c @@ -6,17 +6,17 @@ // 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 -#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]]:52}:"\"..\\\\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]]: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]]:52}:"\"..//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 "apath\..\.\case-insensitive-include.h" -#include "apath\..\.\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} -// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:49}:"\"apath\\..\\.\\case-insensitive-include.h\"" -#include "apath\\..\\.\\Case-Insensitive-Include.h" // expected-warning {{non-portable path}} -// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:52}:"\"apath\\\\..\\\\.\\\\case-insensitive-include.h\"" -#include "APath\..\.\case-insensitive-include.h" // For the sake of efficiency, this case is not diagnosed. :-( +#include "apath/.././case-insensitive-include.h" +#include "apath/.././Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:49}:"\"apath/.././case-insensitive-include.h\"" +#include "apath//..//.//Case-Insensitive-Include.h" // expected-warning {{non-portable path}} +// CHECK: fix-it:"{{.*}}":{[[@LINE-1]]:10-[[@LINE-1]]:52}:"\"apath//..//.//case-insensitive-include.h\"" +#include "APath/.././case-insensitive-include.h" // For the sake of efficiency, this case is not diagnosed. :-( diff --git a/clang/test/Lexer/case-insensitive-include-win.c b/clang/test/Lexer/case-insensitive-include-win.c index ed722feb3d994..9a8c3abf16b32 100644 --- a/clang/test/Lexer/case-insensitive-include-win.c +++ b/clang/test/Lexer/case-insensitive-include-win.c @@ -11,6 +11,7 @@ // REQUIRES: system-windows // RUN: mkdir -p %t.dir // RUN: touch %t.dir/foo.h -// RUN: not %clang_cl /FI \\?\%{t:real}.dir\FOO.h /WX -fsyntax-only %s 2>&1 | FileCheck %s +// RUN: echo #include "\"\\\\?\%{t:real}.dir\FOO.h\"" > %t.dir/test.c +// RUN: not %clang_cl /WX -fsyntax-only %t.dir\test.c 2>&1 | FileCheck %s -// CHECK: non-portable path to file '"\\?\ +// CHECK: non-portable path to file '"//?/ `````````` </details> https://github.com/llvm/llvm-project/pull/186770 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
