[clang] [C++20] [Modules] Warn if we found #include <filename> in module purview (PR #69555)
Chuanqi Xu via cfe-commits
cfe-commits at lists.llvm.org
Thu Oct 19 01:31:33 PDT 2023
https://github.com/ChuanqiXu9 updated https://github.com/llvm/llvm-project/pull/69555
>From 86663a35a7af039f9440af2cc1896e8b4cf33310 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Thu, 19 Oct 2023 11:28:01 +0800
Subject: [PATCH 1/2] [C++20] [Modules] Warn if we found #include <filename> in
module purview
Address https://github.com/llvm/llvm-project/issues/68615.
It is generally wrong to include <filename> in the module purview.
Although there are cases to include files in the module purview,
generally these use cases should include files by quotes instead of by
angles. Here we think the files got included by angles are the system
headers.
---
.../include/clang/Basic/DiagnosticLexKinds.td | 4 ++
clang/lib/Lex/PPDirectives.cpp | 3 +
.../include-in-module-purview.cppm | 60 +++++++++++++++++++
3 files changed, 67 insertions(+)
create mode 100644 clang/test/Preprocessor/include-in-module-purview.cppm
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 940cca67368492f..5d96bcb9359951f 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -922,6 +922,10 @@ def err_header_import_semi_in_macro : Error<
def err_header_import_not_header_unit : Error<
"header file %0 (aka '%1') cannot be imported because "
"it is not known to be a header unit">;
+def warn_pp_include_angled_in_module_purview : Warning<
+ "'#include <filename>' in the module purview appears to be erroneous; "
+ "consider moving that directive before the module declaration">,
+ InGroup<DiagGroup<"include-angled-in-module-purview">>;
def warn_header_guard : Warning<
"%0 is used as a header guard here, followed by #define of a different macro">,
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 2892d4b777846ff..243f7a729681ce9 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2530,6 +2530,9 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
return {ImportAction::None};
}
+ if (isAngled && isInNamedModule())
+ Diag(FilenameTok, diag::warn_pp_include_angled_in_module_purview);
+
// Look up the file, create a File ID for it.
SourceLocation IncludePos = FilenameTok.getLocation();
// If the filename string was the result of macro expansions, set the include
diff --git a/clang/test/Preprocessor/include-in-module-purview.cppm b/clang/test/Preprocessor/include-in-module-purview.cppm
new file mode 100644
index 000000000000000..9ad66953f7dc31d
--- /dev/null
+++ b/clang/test/Preprocessor/include-in-module-purview.cppm
@@ -0,0 +1,60 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -E -P -I%t -o %t/tmp 2>&1 | FileCheck %t/a.cppm
+// RUN: %clang_cc1 -std=c++20 %t/a.cppm -E -P -I%t -o - 2>&1 \
+// RUN: -Wno-include-angled-in-module-purview | FileCheck %t/a.cppm --check-prefix=CHECK-NO-WARN
+
+//--- a.h
+// left empty
+
+//--- b.h
+#include <stddef.h>
+// The headers not get included shouldn't be affected.
+#ifdef WHATEVER
+#include <stdint.h>
+#endif
+
+//--- a.cppm
+module;
+#include <stddef.h>
+#include <a.h>
+#include <b.h>
+#include "a.h"
+#include "b.h"
+export module a;
+
+#include <stddef.h>
+#include <a.h>
+#include <b.h>
+#include "a.h"
+#include "b.h"
+
+// CHECK: a.cppm:9:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK: a.cppm:10:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK: a.cppm:11:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK: In file included from {{.*}}/a.cppm:11
+// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK: In file included from {{.*}}/a.cppm:13
+// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+
+module :private;
+#include <stddef.h>
+#include <a.h>
+#include <b.h>
+#include "a.h"
+#include "b.h"
+
+// CHECK: a.cppm:24:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK: a.cppm:25:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK: a.cppm:26:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK: In file included from {{.*}}/a.cppm:26
+// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK: In file included from {{.*}}/a.cppm:28
+// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+
+// We should have catched all warnings.
+// CHECK: 10 warnings generated.
+
+// CHECK-NO-WARN-NOT: warning
>From 49ead327fec98f5276a9667414668a05fb03c705 Mon Sep 17 00:00:00 2001
From: Chuanqi Xu <yedeng.yd at linux.alibaba.com>
Date: Thu, 19 Oct 2023 16:25:34 +0800
Subject: [PATCH 2/2] Address comments.
---
.../include/clang/Basic/DiagnosticLexKinds.td | 5 +++--
clang/lib/Lex/PPDirectives.cpp | 3 ++-
.../include-in-module-purview.cppm | 20 +++++++++----------
3 files changed, 15 insertions(+), 13 deletions(-)
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index 5d96bcb9359951f..564ca48cc32ac55 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -923,8 +923,9 @@ def err_header_import_not_header_unit : Error<
"header file %0 (aka '%1') cannot be imported because "
"it is not known to be a header unit">;
def warn_pp_include_angled_in_module_purview : Warning<
- "'#include <filename>' in the module purview appears to be erroneous; "
- "consider moving that directive before the module declaration">,
+ "'#include <filename>' attaches the declarations to the named module '%0'"
+ ", which is not usually intended; consider moving that directive before "
+ "the module declaration">,
InGroup<DiagGroup<"include-angled-in-module-purview">>;
def warn_header_guard : Warning<
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index 243f7a729681ce9..93b05c3443e5550 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -2531,7 +2531,8 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
}
if (isAngled && isInNamedModule())
- Diag(FilenameTok, diag::warn_pp_include_angled_in_module_purview);
+ Diag(FilenameTok, diag::warn_pp_include_angled_in_module_purview)
+ << getNamedModuleName();
// Look up the file, create a File ID for it.
SourceLocation IncludePos = FilenameTok.getLocation();
diff --git a/clang/test/Preprocessor/include-in-module-purview.cppm b/clang/test/Preprocessor/include-in-module-purview.cppm
index 9ad66953f7dc31d..0a080112b43277c 100644
--- a/clang/test/Preprocessor/include-in-module-purview.cppm
+++ b/clang/test/Preprocessor/include-in-module-purview.cppm
@@ -31,13 +31,13 @@ export module a;
#include "a.h"
#include "b.h"
-// CHECK: a.cppm:9:10: warning: '#include <filename>' in the module purview appears to be erroneous;
-// CHECK: a.cppm:10:10: warning: '#include <filename>' in the module purview appears to be erroneous;
-// CHECK: a.cppm:11:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK: a.cppm:9:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
+// CHECK: a.cppm:10:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
+// CHECK: a.cppm:11:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
// CHECK: In file included from {{.*}}/a.cppm:11
-// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
// CHECK: In file included from {{.*}}/a.cppm:13
-// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
module :private;
#include <stddef.h>
@@ -46,13 +46,13 @@ module :private;
#include "a.h"
#include "b.h"
-// CHECK: a.cppm:24:10: warning: '#include <filename>' in the module purview appears to be erroneous;
-// CHECK: a.cppm:25:10: warning: '#include <filename>' in the module purview appears to be erroneous;
-// CHECK: a.cppm:26:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK: a.cppm:24:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
+// CHECK: a.cppm:25:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
+// CHECK: a.cppm:26:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
// CHECK: In file included from {{.*}}/a.cppm:26
-// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
// CHECK: In file included from {{.*}}/a.cppm:28
-// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' in the module purview appears to be erroneous;
+// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
// We should have catched all warnings.
// CHECK: 10 warnings generated.
More information about the cfe-commits
mailing list