Skip to content

Commit 689b3cc

Browse files
authored
[clang] Support header shadowing diagnostics in Clang header search (#162491)
When including a header file, multiple files with the same name may exist across different search paths, like:   |-- main.cpp   |-- **header.h**   |-- include   |  └── **header.h** The compiler usually picks the first match it finds (typically following MSVC rules for current/include-chain paths first, then regular -I paths), which may not be the user’s intended header. This silent behavior can lead to subtle runtime API mismatches or increase the cost of resolving errors such as “error: use of undeclared identifier”, especially in large projects. Therefore, this patch tries to provide a diagnostic message without changing the current header selection. It does this by performing an additional search for duplicate filenames across all search paths (both MSVC rules and standard paths). This informs the user about a potential "header shadowing" issue and clarifies which header path was actually used. Since header searching is much cheaper than file loading, the added overhead should be within an acceptable range -- assuming the diagnostic message is valuable.
1 parent 73036cf commit 689b3cc

File tree

6 files changed

+139
-0
lines changed

6 files changed

+139
-0
lines changed

clang/docs/ReleaseNotes.rst

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -449,6 +449,9 @@ Improvements to Clang's diagnostics
449449
comparison operators when mixed with bitwise operators in enum value initializers.
450450
This can be locally disabled by explicitly casting the initializer value.
451451

452+
- A new warning ``-Wshadow-header`` has been added to detect when a header file
453+
is found in multiple search directories (excluding system paths).
454+
452455
Improvements to Clang's time-trace
453456
----------------------------------
454457

clang/include/clang/Basic/DiagnosticGroups.td

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -790,6 +790,8 @@ def ShadowFieldInConstructor : DiagGroup<"shadow-field-in-constructor",
790790
def ShadowIvar : DiagGroup<"shadow-ivar">;
791791
def ShadowUncapturedLocal : DiagGroup<"shadow-uncaptured-local">;
792792

793+
def ShadowHeader : DiagGroup<"shadow-header">;
794+
793795
// -Wshadow-all is a catch-all for all shadowing. -Wshadow is just the
794796
// shadowing that we think is unsafe.
795797
def Shadow : DiagGroup<"shadow", [ShadowFieldInConstructorModified,

clang/include/clang/Basic/DiagnosticLexKinds.td

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -959,6 +959,10 @@ def warn_quoted_include_in_framework_header : Warning<
959959
def warn_framework_include_private_from_public : Warning<
960960
"public framework header includes private framework header '%0'"
961961
>, InGroup<FrameworkIncludePrivateFromPublic>;
962+
def warn_header_shadowing : Warning<
963+
"multiple candidates for header '%0' found; "
964+
"directory '%1' chosen, ignoring others including '%2'">,
965+
InGroup<ShadowHeader>, DefaultIgnore;
962966
def warn_deprecated_module_dot_map : Warning<
963967
"'%0' as a module map name is deprecated, rename it to %select{module.modulemap|module.private.modulemap}1%select{| in the 'Modules' directory of the framework}2">,
964968
InGroup<DeprecatedModuleDotMap>;

clang/include/clang/Lex/HeaderSearch.h

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -465,6 +465,12 @@ class HeaderSearch {
465465
ExternalSource = ES;
466466
}
467467

468+
void diagnoseHeaderShadowing(
469+
StringRef Filename, OptionalFileEntryRef FE, bool &DiagnosedShadowing,
470+
SourceLocation IncludeLoc, ConstSearchDirIterator FromDir,
471+
ArrayRef<std::pair<OptionalFileEntryRef, DirectoryEntryRef>> Includers,
472+
bool isAngled, int IncluderLoopIndex, ConstSearchDirIterator MainLoopIt);
473+
468474
/// Set the target information for the header search, if not
469475
/// already known.
470476
void setTarget(const TargetInfo &Target);

clang/lib/Lex/HeaderSearch.cpp

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -881,6 +881,66 @@ diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation IncludeLoc,
881881
<< IncludeFilename;
882882
}
883883

884+
void HeaderSearch::diagnoseHeaderShadowing(
885+
StringRef Filename, OptionalFileEntryRef FE, bool &DiagnosedShadowing,
886+
SourceLocation IncludeLoc, ConstSearchDirIterator FromDir,
887+
ArrayRef<std::pair<OptionalFileEntryRef, DirectoryEntryRef>> Includers,
888+
bool isAngled, int IncluderLoopIndex, ConstSearchDirIterator MainLoopIt) {
889+
890+
if (Diags.isIgnored(diag::warn_header_shadowing, IncludeLoc) ||
891+
DiagnosedShadowing)
892+
return;
893+
// Ignore diagnostics from system headers.
894+
if (MainLoopIt && MainLoopIt->isSystemHeaderDirectory())
895+
return;
896+
897+
DiagnosedShadowing = true;
898+
899+
// Indicates that file is first found in the includer's directory
900+
if (!MainLoopIt) {
901+
for (size_t i = IncluderLoopIndex + 1; i < Includers.size(); ++i) {
902+
const auto &IncluderAndDir = Includers[i];
903+
SmallString<1024> TmpDir = IncluderAndDir.second.getName();
904+
llvm::sys::path::append(TmpDir, Filename);
905+
if (auto File = getFileMgr().getFileRef(TmpDir, false, false)) {
906+
if (&File->getFileEntry() == *FE)
907+
continue;
908+
Diags.Report(IncludeLoc, diag::warn_header_shadowing)
909+
<< Filename << (*FE).getDir().getName()
910+
<< IncluderAndDir.second.getName();
911+
return;
912+
} else {
913+
llvm::errorToErrorCode(File.takeError());
914+
}
915+
}
916+
}
917+
918+
// Continue searching in the regular search paths
919+
ConstSearchDirIterator It =
920+
isAngled ? angled_dir_begin() : search_dir_begin();
921+
if (MainLoopIt) {
922+
It = std::next(MainLoopIt);
923+
} else if (FromDir) {
924+
It = FromDir;
925+
}
926+
for (; It != search_dir_end(); ++It) {
927+
// Suppress check for system headers, as duplicates are often intentional.
928+
if (It->getDirCharacteristic() != SrcMgr::C_User)
929+
continue;
930+
SmallString<1024> TmpPath = It->getName();
931+
llvm::sys::path::append(TmpPath, Filename);
932+
if (auto File = getFileMgr().getFileRef(TmpPath, false, false)) {
933+
if (&File->getFileEntry() == *FE)
934+
continue;
935+
Diags.Report(IncludeLoc, diag::warn_header_shadowing)
936+
<< Filename << (*FE).getDir().getName() << It->getName();
937+
return;
938+
} else {
939+
llvm::errorToErrorCode(File.takeError());
940+
}
941+
}
942+
}
943+
884944
/// LookupFile - Given a "foo" or \<foo> reference, look up the indicated file,
885945
/// return null on failure. isAngled indicates whether the file reference is
886946
/// for system \#include's or not (i.e. using <> instead of ""). Includers, if
@@ -930,6 +990,7 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
930990
// This is the header that MSVC's header search would have found.
931991
ModuleMap::KnownHeader MSSuggestedModule;
932992
OptionalFileEntryRef MSFE;
993+
bool DiagnosedShadowing = false;
933994

934995
// Check to see if the file is in the #includer's directory. This cannot be
935996
// based on CurDir, because each includer could be a #include of a
@@ -963,6 +1024,9 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
9631024
if (OptionalFileEntryRef FE = getFileAndSuggestModule(
9641025
TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
9651026
RequestingModule, SuggestedModule)) {
1027+
diagnoseHeaderShadowing(Filename, FE, DiagnosedShadowing, IncludeLoc,
1028+
FromDir, Includers, isAngled,
1029+
&IncluderAndDir - Includers.begin(), nullptr);
9661030
if (!Includer) {
9671031
assert(First && "only first includer can have no file");
9681032
return FE;
@@ -1097,6 +1161,9 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
10971161
if (!File)
10981162
continue;
10991163

1164+
diagnoseHeaderShadowing(Filename, File, DiagnosedShadowing, IncludeLoc,
1165+
FromDir, Includers, isAngled, -1, It);
1166+
11001167
CurDir = It;
11011168

11021169
IncludeNames[*File] = Filename;
Lines changed: 57 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,57 @@
1+
// RUN: rm -rf %t
2+
// RUN: split-file %s %t
3+
4+
/// Check that:
5+
/// - Quoted includes ("...") trigger the diagnostic.
6+
/// - System headers are ignored.
7+
/// - #include_next does not cause a duplicate warning.
8+
// RUN: %clang_cc1 -Wshadow-header -Eonly %t/main.c -I %t/include1 -I %t/include2 \
9+
// RUN: -isystem %t/system1 -isystem %t/system2 2>&1 | FileCheck %s --check-prefix=SHADOWING
10+
11+
// SHADOWING: {{.*}} warning: multiple candidates for header 'header.h' found; directory '{{.*}}include1' chosen, ignoring others including '{{.*}}include2' [-Wshadow-header]
12+
// SHADOWING: warning: include1/header.h included!
13+
// SHADOWING-NOT: {{.*}} warning: multiple candidates for header 'header.h' found; directory '{{.*}}include2' chosen, ignoring others including '{{.*}}include1' [-Wshadow-header]
14+
// SHADOWING: warning: include2/header.h included!
15+
// SHADOWING-NOT: {{.*}} warning: multiple candidates for header 'stdio.h' found; directory '{{.*}}system1' chosen, ignoring others including '{{.*}}system2' [-Wshadow-header]
16+
// SHADOWING: warning: system1/stdio.h included!
17+
18+
/// Check that the diagnostic is only performed once in MSVC compatibility mode.
19+
// RUN: %clang_cc1 -fms-compatibility -Wshadow-header -Eonly %t/t.c 2>&1 | FileCheck %s --check-prefix=SHADOWING-MS
20+
21+
// SHADOWING-MS: {{.*}} warning: multiple candidates for header 't3.h' found; directory '{{.*}}foo' chosen, ignoring others including '{{.*}}' [-Wshadow-header]
22+
// SHADOWING-MS-NOT: {{.*}} warning: multiple candidates for header 't3.h' found; directory '{{.*}}' chosen, ignoring others including '{{.*}}foo' [-Wshadow-header]
23+
// SHADOWING-MS: warning: Found foo/t3.h.
24+
25+
//--- main.c
26+
#include "header.h"
27+
#include <stdio.h>
28+
29+
//--- include1/header.h
30+
#warning include1/header.h included!
31+
#include_next "header.h"
32+
33+
//--- include2/header.h
34+
#warning include2/header.h included!
35+
36+
//--- system1/stdio.h
37+
#warning system1/stdio.h included!
38+
39+
//--- system2/stdio.h
40+
#warning system2/stdio.h included!
41+
42+
43+
/// Used to test when running in MSVC compatibility
44+
//--- t.c
45+
#include "foo/t1.h"
46+
47+
//--- foo/t1.h
48+
#include "bar/t2.h"
49+
50+
//--- foo/bar/t2.h
51+
#include "t3.h"
52+
53+
//--- foo/t3.h
54+
#warning Found foo/t3.h.
55+
56+
//--- t3.h
57+
#warning Found t3.h.

0 commit comments

Comments
 (0)