[clang] Extend -Wnonportable-include-path with a warning about trailing whitespace or dots (PR #96960)

2024-06-27 Thread Tobias Mayer via cfe-commits 

https://github.com/tobias-mayer created 
https://github.com/llvm/llvm-project/pull/96960

Resolves #96064

Extends `-Wnonportable-include-path` with a warning about trailing whitespace 
or dots in include paths. 

![Screenshot from 2024-06-27 
22-10-26](https://github.com/llvm/llvm-project/assets/56530704/9dbb7ddb-3f0e-4f8d-9eaf-dbb773baca27)


>From ec07df7ff56b21170c4a0d61b119426bf4dfeff7 Mon Sep 17 00:00:00 2001
From: Tobias Mayer 
Date: Thu, 27 Jun 2024 22:04:55 +0200
Subject: [PATCH] add warning about trailing whitespace or dots in include
 paths

---
 clang/include/clang/Basic/DiagnosticGroups.td   |  3 +++
 clang/include/clang/Basic/DiagnosticLexKinds.td |  6 +-
 clang/lib/Lex/PPDirectives.cpp  | 17 +
 .../nonportable-include-trailing-whitespace.c   | 11 +++
 4 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 
clang/test/Preprocessor/nonportable-include-trailing-whitespace.c

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index 9b37d4bd3205b..364a2387d3d50 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1535,3 +1535,6 @@ def BitIntExtension : DiagGroup<"bit-int-extension">;
 
 // Warnings about misuse of ExtractAPI options.
 def ExtractAPIMisuse : DiagGroup<"extractapi-misuse">;
+
+// Warnings about non-portable include paths
+def NonportablePath : DiagGroup<"nonportable-include-path">;
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td 
b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 12d7b8c0205ee..cf6e448d8fc36 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -361,7 +361,11 @@ class NonportablePath  : Warning<
   "non-portable path to file '%0'; specified path differs in case from file"
   " name on disk">;
 def pp_nonportable_path : NonportablePath,
-  InGroup>;
+  InGroup;
+def pp_nonportable_path_trailing_whitespace : Warning<
+  "non-portable path to file '%0'; specified path contains trailing"
+  "whitespace or dots">,
+  InGroup;
 def pp_nonportable_system_path : NonportablePath, DefaultIgnore,
   InGroup>;
 
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index a53540b12dee6..919e3edc18032 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2098,6 +2098,23 @@ OptionalFileEntryRef 
Preprocessor::LookupHeaderIncludeOrImport(
 const FileEntry *LookupFromFile, StringRef &LookupFilename,
 SmallVectorImpl &RelativePath, SmallVectorImpl &SearchPath,
 ModuleMap::KnownHeader &SuggestedModule, bool isAngled) {
+
+  // Check for trailing whitespace or dots in the include path.
+  // This must be done before looking up the file, as Windows will still
+  // find the file even if there are trailing dots or whitespace.
+  size_t TrailingPos = Filename.find_last_not_of(" .");
+  if (TrailingPos != StringRef::npos && TrailingPos < Filename.size() - 1) {
+StringRef TrimmedFilename = Filename.rtrim(" .");
+
+auto Hint = isAngled
+? FixItHint::CreateReplacement(
+  FilenameRange, "<" + TrimmedFilename.str() + ">")
+: FixItHint::CreateReplacement(
+  FilenameRange, "\"" + TrimmedFilename.str() + "\"");
+Diag(FilenameTok, diag::pp_nonportable_path_trailing_whitespace)
+<< Filename << TrimmedFilename << Hint;
+  }
+
   auto DiagnoseHeaderInclusion = [&](FileEntryRef FE) {
 if (LangOpts.AsmPreprocessor)
   return;
diff --git a/clang/test/Preprocessor/nonportable-include-trailing-whitespace.c 
b/clang/test/Preprocessor/nonportable-include-trailing-whitespace.c
new file mode 100644
index 0..6def70a59d2e1
--- /dev/null
+++ b/clang/test/Preprocessor/nonportable-include-trailing-whitespace.c
@@ -0,0 +1,11 @@
+// REQUIRES: case-insensitive-filesystem
+// RUN: %clang_cc1 -Wall %s
+
+#include "empty_file_to_include.h " // expected-warning {{non-portable path}}
+#include "empty_file_to_include.h." // expected-warning {{non-portable path}}
+#include "empty_file_to_include.h   " // expected-warning {{non-portable 
path}}
+#include "empty_file_to_include.h..." // expected-warning {{non-portable 
path}}
+#include "empty_file_to_include.h . . . " // expected-warning {{non-portable 
path}}
+#include "empty_file_to_include.h.. .. " // expected-warning {{non-portable 
path}}
+
+#include "empty_file_to_include.h" // No warning
\ No newline at end of file

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Extend -Wnonportable-include-path with a warning about trailing whitespace or dots (PR #96960)

2024-06-27 Thread Tobias Mayer via cfe-commits 

https://github.com/tobias-mayer updated 
https://github.com/llvm/llvm-project/pull/96960

>From 4118e37219390d8757720e62d0bdf058636d0273 Mon Sep 17 00:00:00 2001
From: Tobias Mayer 
Date: Thu, 27 Jun 2024 22:04:55 +0200
Subject: [PATCH] add warning about trailing whitespace or dots in include
 paths

---
 clang/include/clang/Basic/DiagnosticGroups.td   |  3 +++
 clang/include/clang/Basic/DiagnosticLexKinds.td |  6 +-
 clang/lib/Lex/PPDirectives.cpp  | 17 +
 .../nonportable-include-trailing-whitespace.c   | 11 +++
 4 files changed, 36 insertions(+), 1 deletion(-)
 create mode 100644 
clang/test/Preprocessor/nonportable-include-trailing-whitespace.c

diff --git a/clang/include/clang/Basic/DiagnosticGroups.td 
b/clang/include/clang/Basic/DiagnosticGroups.td
index de99cb46d3145..1823ebe7ed811 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -1543,3 +1543,6 @@ def ExtractAPIMisuse : DiagGroup<"extractapi-misuse">;
 // Warnings about using the non-standard extension having an explicit 
specialization
 // with a storage class specifier.
 def ExplicitSpecializationStorageClass : 
DiagGroup<"explicit-specialization-storage-class">;
+
+// Warnings about non-portable include paths
+def NonportablePath : DiagGroup<"nonportable-include-path">;
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td 
b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 12d7b8c0205ee..cf6e448d8fc36 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -361,7 +361,11 @@ class NonportablePath  : Warning<
   "non-portable path to file '%0'; specified path differs in case from file"
   " name on disk">;
 def pp_nonportable_path : NonportablePath,
-  InGroup>;
+  InGroup;
+def pp_nonportable_path_trailing_whitespace : Warning<
+  "non-portable path to file '%0'; specified path contains trailing"
+  "whitespace or dots">,
+  InGroup;
 def pp_nonportable_system_path : NonportablePath, DefaultIgnore,
   InGroup>;
 
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index a53540b12dee6..919e3edc18032 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2098,6 +2098,23 @@ OptionalFileEntryRef 
Preprocessor::LookupHeaderIncludeOrImport(
 const FileEntry *LookupFromFile, StringRef &LookupFilename,
 SmallVectorImpl &RelativePath, SmallVectorImpl &SearchPath,
 ModuleMap::KnownHeader &SuggestedModule, bool isAngled) {
+
+  // Check for trailing whitespace or dots in the include path.
+  // This must be done before looking up the file, as Windows will still
+  // find the file even if there are trailing dots or whitespace.
+  size_t TrailingPos = Filename.find_last_not_of(" .");
+  if (TrailingPos != StringRef::npos && TrailingPos < Filename.size() - 1) {
+StringRef TrimmedFilename = Filename.rtrim(" .");
+
+auto Hint = isAngled
+? FixItHint::CreateReplacement(
+  FilenameRange, "<" + TrimmedFilename.str() + ">")
+: FixItHint::CreateReplacement(
+  FilenameRange, "\"" + TrimmedFilename.str() + "\"");
+Diag(FilenameTok, diag::pp_nonportable_path_trailing_whitespace)
+<< Filename << TrimmedFilename << Hint;
+  }
+
   auto DiagnoseHeaderInclusion = [&](FileEntryRef FE) {
 if (LangOpts.AsmPreprocessor)
   return;
diff --git a/clang/test/Preprocessor/nonportable-include-trailing-whitespace.c 
b/clang/test/Preprocessor/nonportable-include-trailing-whitespace.c
new file mode 100644
index 0..6def70a59d2e1
--- /dev/null
+++ b/clang/test/Preprocessor/nonportable-include-trailing-whitespace.c
@@ -0,0 +1,11 @@
+// REQUIRES: case-insensitive-filesystem
+// RUN: %clang_cc1 -Wall %s
+
+#include "empty_file_to_include.h " // expected-warning {{non-portable path}}
+#include "empty_file_to_include.h." // expected-warning {{non-portable path}}
+#include "empty_file_to_include.h   " // expected-warning {{non-portable 
path}}
+#include "empty_file_to_include.h..." // expected-warning {{non-portable 
path}}
+#include "empty_file_to_include.h . . . " // expected-warning {{non-portable 
path}}
+#include "empty_file_to_include.h.. .. " // expected-warning {{non-portable 
path}}
+
+#include "empty_file_to_include.h" // No warning
\ No newline at end of file

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Extend -Wnonportable-include-path with a warning about trailing whitespace or dots (PR #96960)

2024-06-28 Thread Tobias Mayer via cfe-commits 


@@ -361,7 +361,11 @@ class NonportablePath  : Warning<
   "non-portable path to file '%0'; specified path differs in case from file"
   " name on disk">;
 def pp_nonportable_path : NonportablePath,
-  InGroup>;
+  InGroup;
+def pp_nonportable_path_trailing_whitespace : Warning<
+  "non-portable path to file '%0'; specified path contains trailing"
+  "whitespace or dots">,

tobias-mayer wrote:

Sounds good. How should we handle a case like ` #include "test.h.. .. "` where 
we have both? Should we add a third option complaining about both or just pick 
whatever is the last character?

https://github.com/llvm/llvm-project/pull/96960
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] Extend -Wnonportable-include-path with a warning about trailing whitespace or dots (PR #96960)

2024-06-28 Thread Tobias Mayer via cfe-commits 


@@ -0,0 +1,11 @@
+// REQUIRES: case-insensitive-filesystem

tobias-mayer wrote:

I wasn't sure about this but I think I could remove that.
The behavior should be as follows:
- Linux: You get the new warning and a file not found error.
- Windows: You only get the new warning as the file is still found.
- Mac: Warning + error just as on Linux. Case sensitivity shouldn't matter in 
both cases.

https://github.com/llvm/llvm-project/pull/96960
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits