Skip to content

Commit 0d21436

Browse files
authored
[C++20] [Modules] Warn if we found #include <filename> in module purview (#69555)
Close #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. This is consistency with MSVC too: https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warnings-by-compiler-version?view=msvc-170#warnings-introduced-in-visual-studio-2022-version-170-compiler-version-1930
1 parent af50d6e commit 0d21436

File tree

3 files changed

+69
-0
lines changed

3 files changed

+69
-0
lines changed

clang/include/clang/Basic/DiagnosticLexKinds.td

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -922,6 +922,11 @@ def err_header_import_semi_in_macro : Error<
922922
def err_header_import_not_header_unit : Error<
923923
"header file %0 (aka '%1') cannot be imported because "
924924
"it is not known to be a header unit">;
925+
def warn_pp_include_angled_in_module_purview : Warning<
926+
"'#include <filename>' attaches the declarations to the named module '%0'"
927+
", which is not usually intended; consider moving that directive before "
928+
"the module declaration">,
929+
InGroup<DiagGroup<"include-angled-in-module-purview">>;
925930

926931
def warn_header_guard : Warning<
927932
"%0 is used as a header guard here, followed by #define of a different macro">,

clang/lib/Lex/PPDirectives.cpp

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2537,6 +2537,10 @@ Preprocessor::ImportAction Preprocessor::HandleHeaderIncludeOrImport(
25372537
return {ImportAction::None};
25382538
}
25392539

2540+
if (isAngled && isInNamedModule())
2541+
Diag(FilenameTok, diag::warn_pp_include_angled_in_module_purview)
2542+
<< getNamedModuleName();
2543+
25402544
// Look up the file, create a File ID for it.
25412545
SourceLocation IncludePos = FilenameTok.getLocation();
25422546
// If the filename string was the result of macro expansions, set the include
Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
// RUN: rm -rf %t
2+
// RUN: mkdir %t
3+
// RUN: split-file %s %t
4+
//
5+
// RUN: %clang_cc1 -std=c++20 %t/a.cppm -E -P -I%t -o %t/tmp 2>&1 | FileCheck %t/a.cppm
6+
// RUN: %clang_cc1 -std=c++20 %t/a.cppm -E -P -I%t -o - 2>&1 \
7+
// RUN: -Wno-include-angled-in-module-purview | FileCheck %t/a.cppm --check-prefix=CHECK-NO-WARN
8+
9+
//--- a.h
10+
// left empty
11+
12+
//--- b.h
13+
#include <stddef.h>
14+
// The headers not get included shouldn't be affected.
15+
#ifdef WHATEVER
16+
#include <stdint.h>
17+
#endif
18+
19+
//--- a.cppm
20+
module;
21+
#include <stddef.h>
22+
#include <a.h>
23+
#include <b.h>
24+
#include "a.h"
25+
#include "b.h"
26+
export module a;
27+
28+
#include <stddef.h>
29+
#include <a.h>
30+
#include <b.h>
31+
#include "a.h"
32+
#include "b.h"
33+
34+
// CHECK: a.cppm:9:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
35+
// CHECK: a.cppm:10:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
36+
// CHECK: a.cppm:11:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
37+
// CHECK: In file included from {{.*}}/a.cppm:11
38+
// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
39+
// CHECK: In file included from {{.*}}/a.cppm:13
40+
// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
41+
42+
module :private;
43+
#include <stddef.h>
44+
#include <a.h>
45+
#include <b.h>
46+
#include "a.h"
47+
#include "b.h"
48+
49+
// CHECK: a.cppm:24:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
50+
// CHECK: a.cppm:25:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
51+
// CHECK: a.cppm:26:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
52+
// CHECK: In file included from {{.*}}/a.cppm:26
53+
// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
54+
// CHECK: In file included from {{.*}}/a.cppm:28
55+
// CHECK-NEXT: b.h:1:10: warning: '#include <filename>' attaches the declarations to the named module 'a'
56+
57+
// We should have catched all warnings.
58+
// CHECK: 10 warnings generated.
59+
60+
// CHECK-NO-WARN-NOT: warning

0 commit comments

Comments
 (0)