[clang] [clang] Support header shadowing diagnostics in Clang header search (PR #162491)
Jinjie Huang via cfe-commits
cfe-commits at lists.llvm.org
Wed Oct 8 08:04:55 PDT 2025
https://github.com/Jinjie-Huang updated https://github.com/llvm/llvm-project/pull/162491
>From 2663a533bafda57b8fe69968911117a4adab64e1 Mon Sep 17 00:00:00 2001
From: huangjinjie <huangjinjie at bytedance.com>
Date: Wed, 8 Oct 2025 23:04:29 +0800
Subject: [PATCH] support header shadowing detection
---
clang/include/clang/Basic/DiagnosticGroups.td | 2 +
.../include/clang/Basic/DiagnosticLexKinds.td | 4 +
clang/include/clang/Lex/DirectoryLookup.h | 2 +-
clang/include/clang/Lex/HeaderSearch.h | 11 +-
clang/lib/Lex/HeaderSearch.cpp | 165 ++++++++++++------
clang/test/Preprocessor/header-shadowing.c | 14 ++
6 files changed, 137 insertions(+), 61 deletions(-)
create mode 100644 clang/test/Preprocessor/header-shadowing.c
diff --git a/clang/include/clang/Basic/DiagnosticGroups.td b/clang/include/clang/Basic/DiagnosticGroups.td
index 0c994e0b5ca4d..b08adc59c9a8a 100644
--- a/clang/include/clang/Basic/DiagnosticGroups.td
+++ b/clang/include/clang/Basic/DiagnosticGroups.td
@@ -789,6 +789,8 @@ def ShadowFieldInConstructor : DiagGroup<"shadow-field-in-constructor",
def ShadowIvar : DiagGroup<"shadow-ivar">;
def ShadowUncapturedLocal : DiagGroup<"shadow-uncaptured-local">;
+def HeaderShadowing : DiagGroup<"header-shadowing">;
+
// -Wshadow-all is a catch-all for all shadowing. -Wshadow is just the
// shadowing that we think is unsafe.
def Shadow : DiagGroup<"shadow", [ShadowFieldInConstructorModified,
diff --git a/clang/include/clang/Basic/DiagnosticLexKinds.td b/clang/include/clang/Basic/DiagnosticLexKinds.td
index c7fe6e1db6d1f..8057e0e95e187 100644
--- a/clang/include/clang/Basic/DiagnosticLexKinds.td
+++ b/clang/include/clang/Basic/DiagnosticLexKinds.td
@@ -951,6 +951,10 @@ def warn_quoted_include_in_framework_header : Warning<
def warn_framework_include_private_from_public : Warning<
"public framework header includes private framework header '%0'"
>, InGroup<FrameworkIncludePrivateFromPublic>;
+def warn_header_shadowed
+ : Warning<"multiple candidates for header '%0' found; "
+ "using the one from '%1', shadowed by '%2'">,
+ InGroup<HeaderShadowing>, DefaultIgnore;
def warn_deprecated_module_dot_map : Warning<
"'%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">,
InGroup<DeprecatedModuleDotMap>;
diff --git a/clang/include/clang/Lex/DirectoryLookup.h b/clang/include/clang/Lex/DirectoryLookup.h
index bb703dfad2b28..45c7360385a73 100644
--- a/clang/include/clang/Lex/DirectoryLookup.h
+++ b/clang/include/clang/Lex/DirectoryLookup.h
@@ -181,7 +181,7 @@ class DirectoryLookup {
ModuleMap::KnownHeader *SuggestedModule,
bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound,
bool &IsInHeaderMap, SmallVectorImpl<char> &MappedName,
- bool OpenFile = true) const;
+ bool NeedSuggest, bool OpenFile = true) const;
private:
OptionalFileEntryRef DoFrameworkLookup(
diff --git a/clang/include/clang/Lex/HeaderSearch.h b/clang/include/clang/Lex/HeaderSearch.h
index 850aea41c4c3b..83f6e11cd6ddc 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -809,12 +809,11 @@ class HeaderSearch {
/// Look up the file with the specified name and determine its owning
/// module.
- OptionalFileEntryRef
- getFileAndSuggestModule(StringRef FileName, SourceLocation IncludeLoc,
- const DirectoryEntry *Dir, bool IsSystemHeaderDir,
- Module *RequestingModule,
- ModuleMap::KnownHeader *SuggestedModule,
- bool OpenFile = true, bool CacheFailures = true);
+ OptionalFileEntryRef getFileAndSuggestModule(
+ StringRef FileName, SourceLocation IncludeLoc, const DirectoryEntry *Dir,
+ bool IsSystemHeaderDir, Module *RequestingModule,
+ ModuleMap::KnownHeader *SuggestedModule, bool NeedSuggest,
+ bool OpenFile = true, bool CacheFailures = true);
/// Cache the result of a successful lookup at the given include location
/// using the search path at \c HitIt.
diff --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 238c5e2f2d9a5..35c461a8bb6c9 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -444,8 +444,8 @@ StringRef DirectoryLookup::getName() const {
OptionalFileEntryRef HeaderSearch::getFileAndSuggestModule(
StringRef FileName, SourceLocation IncludeLoc, const DirectoryEntry *Dir,
bool IsSystemHeaderDir, Module *RequestingModule,
- ModuleMap::KnownHeader *SuggestedModule, bool OpenFile /*=true*/,
- bool CacheFailures /*=true*/) {
+ ModuleMap::KnownHeader *SuggestedModule, bool NeedSuggest,
+ bool OpenFile /*=true*/, bool CacheFailures /*=true*/) {
// If we have a module map that might map this header, load it and
// check whether we'll have a suggestion for a module.
auto File = getFileMgr().getFileRef(FileName, OpenFile, CacheFailures);
@@ -462,6 +462,9 @@ OptionalFileEntryRef HeaderSearch::getFileAndSuggestModule(
return std::nullopt;
}
+ if (!NeedSuggest)
+ return *File;
+
// If there is a module that corresponds to this header, suggest it.
if (!findUsableModuleForHeader(
*File, Dir ? Dir : File->getFileEntry().getDir(), RequestingModule,
@@ -478,7 +481,7 @@ OptionalFileEntryRef DirectoryLookup::LookupFile(
SmallVectorImpl<char> *SearchPath, SmallVectorImpl<char> *RelativePath,
Module *RequestingModule, ModuleMap::KnownHeader *SuggestedModule,
bool &InUserSpecifiedSystemFramework, bool &IsFrameworkFound,
- bool &IsInHeaderMap, SmallVectorImpl<char> &MappedName,
+ bool &IsInHeaderMap, SmallVectorImpl<char> &MappedName, bool NeedSuggest,
bool OpenFile) const {
InUserSpecifiedSystemFramework = false;
IsInHeaderMap = false;
@@ -501,7 +504,7 @@ OptionalFileEntryRef DirectoryLookup::LookupFile(
return HS.getFileAndSuggestModule(
TmpDir, IncludeLoc, getDir(), isSystemHeaderDirectory(),
- RequestingModule, SuggestedModule, OpenFile);
+ RequestingModule, SuggestedModule, NeedSuggest, OpenFile);
}
if (isFramework())
@@ -881,6 +884,35 @@ diagnoseFrameworkInclude(DiagnosticsEngine &Diags, SourceLocation IncludeLoc,
<< IncludeFilename;
}
+/// Return true if a shadow has been detected and the caller should
+/// stop and return the first-found file and module, false otherwise.
+static bool checkAndStoreCandidate(
+ ModuleMap::KnownHeader *SuggestedModule, OptionalFileEntryRef CandidateFile,
+ StringRef CandidateDir, DiagnosticsEngine &Diags, StringRef Filename,
+ SourceLocation IncludeLoc, ModuleMap::KnownHeader &FirstModule,
+ OptionalFileEntryRef &FirstHeader, SmallString<1024> &FirstDir) {
+ if (!FirstHeader) {
+ // Found the first candidate
+ FirstHeader = CandidateFile;
+ FirstDir = CandidateDir;
+ if (SuggestedModule)
+ FirstModule = *SuggestedModule;
+ return false;
+ }
+
+ if (FirstDir != CandidateDir) {
+ // Found a second candidate from a different directory
+ Diags.Report(IncludeLoc, diag::warn_header_shadowed)
+ << Filename << FirstDir << CandidateDir;
+ if (SuggestedModule)
+ *SuggestedModule = FirstModule;
+ return true;
+ }
+
+ // Found a candidate from the same directory as the first one
+ return false;
+}
+
/// LookupFile - Given a "foo" or \<foo> reference, look up the indicated file,
/// return null on failure. isAngled indicates whether the file reference is
/// for system \#include's or not (i.e. using <> instead of ""). Includers, if
@@ -923,13 +955,18 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
// Otherwise, just return the file.
return getFileAndSuggestModule(Filename, IncludeLoc, nullptr,
/*IsSystemHeaderDir*/ false,
- RequestingModule, SuggestedModule, OpenFile,
- CacheFailures);
+ RequestingModule, SuggestedModule, true,
+ OpenFile, CacheFailures);
}
// This is the header that MSVC's header search would have found.
+ bool First = true;
+ bool NeedSuggest = true;
ModuleMap::KnownHeader MSSuggestedModule;
OptionalFileEntryRef MSFE;
+ ModuleMap::KnownHeader FirstModule;
+ OptionalFileEntryRef FirstHeader;
+ SmallString<1024> FirstDir;
// Check to see if the file is in the #includer's directory. This cannot be
// based on CurDir, because each includer could be a #include of a
@@ -938,7 +975,6 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
// headers.
if (!Includers.empty() && !isAngled) {
SmallString<1024> TmpDir;
- bool First = true;
for (const auto &IncluderAndDir : Includers) {
OptionalFileEntryRef Includer = IncluderAndDir.first;
@@ -962,10 +998,15 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
}();
if (OptionalFileEntryRef FE = getFileAndSuggestModule(
TmpDir, IncludeLoc, IncluderAndDir.second, IncluderIsSystemHeader,
- RequestingModule, SuggestedModule)) {
+ RequestingModule, SuggestedModule, NeedSuggest)) {
+ NeedSuggest = false;
+
if (!Includer) {
assert(First && "only first includer can have no file");
- return FE;
+ checkAndStoreCandidate(
+ SuggestedModule, FE, IncluderAndDir.second.getName(), Diags,
+ Filename, IncludeLoc, FirstModule, FirstHeader, FirstDir);
+ break;
}
// Leave CurDir unset.
@@ -994,22 +1035,28 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
diagnoseFrameworkInclude(Diags, IncludeLoc,
IncluderAndDir.second.getName(), Filename,
*FE);
- return FE;
+ checkAndStoreCandidate(
+ SuggestedModule, FE, IncluderAndDir.second.getName(), Diags,
+ Filename, IncludeLoc, FirstModule, FirstHeader, FirstDir);
+ break;
}
// Otherwise, we found the path via MSVC header search rules. If
// -Wmsvc-include is enabled, we have to keep searching to see if we
// would've found this header in -I or -isystem directories.
- if (Diags.isIgnored(diag::ext_pp_include_search_ms, IncludeLoc)) {
- return FE;
- } else {
- MSFE = FE;
- if (SuggestedModule) {
- MSSuggestedModule = *SuggestedModule;
- *SuggestedModule = ModuleMap::KnownHeader();
- }
+ if (checkAndStoreCandidate(
+ SuggestedModule, FE, IncluderAndDir.second.getName(), Diags,
+ Filename, IncludeLoc, FirstModule, FirstHeader, FirstDir)) {
+ // Found mutiple candidates via MSVC rules
+ if (Diags.isIgnored(diag::ext_pp_include_search_ms, IncludeLoc))
+ return FirstHeader;
break;
}
+ MSFE = FE;
+ if (SuggestedModule) {
+ MSSuggestedModule = *SuggestedModule;
+ *SuggestedModule = ModuleMap::KnownHeader();
+ }
}
First = false;
}
@@ -1069,6 +1116,7 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
}
SmallString<64> MappedName;
+ First = true;
// Check each directory in sequence to see if it contains this file.
for (; It != search_dir_end(); ++It) {
@@ -1078,7 +1126,7 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
OptionalFileEntryRef File = It->LookupFile(
Filename, *this, IncludeLoc, SearchPath, RelativePath, RequestingModule,
SuggestedModule, InUserSpecifiedSystemFramework, IsFrameworkFoundInDir,
- IsInHeaderMap, MappedName, OpenFile);
+ IsInHeaderMap, MappedName, NeedSuggest, OpenFile);
if (!MappedName.empty()) {
assert(IsInHeaderMap && "MappedName should come from a header map");
CacheLookup.MappedName =
@@ -1097,53 +1145,62 @@ OptionalFileEntryRef HeaderSearch::LookupFile(
if (!File)
continue;
- CurDir = It;
-
- IncludeNames[*File] = Filename;
-
- // This file is a system header or C++ unfriendly if the dir is.
- HeaderFileInfo &HFI = getFileInfo(*File);
- HFI.DirInfo = CurDir->getDirCharacteristic();
-
- // If the directory characteristic is User but this framework was
- // user-specified to be treated as a system framework, promote the
- // characteristic.
- if (HFI.DirInfo == SrcMgr::C_User && InUserSpecifiedSystemFramework)
- HFI.DirInfo = SrcMgr::C_System;
-
- // If the filename matches a known system header prefix, override
- // whether the file is a system header.
- for (unsigned j = SystemHeaderPrefixes.size(); j; --j) {
- if (Filename.starts_with(SystemHeaderPrefixes[j - 1].first)) {
- HFI.DirInfo = SystemHeaderPrefixes[j-1].second ? SrcMgr::C_System
- : SrcMgr::C_User;
- break;
+ if (First) {
+ First = false;
+ NeedSuggest = false;
+ CurDir = It;
+ IncludeNames[*File] = Filename;
+
+ // This file is a system header or C++ unfriendly if the dir is.
+ HeaderFileInfo &HFI = getFileInfo(*File);
+ HFI.DirInfo = CurDir->getDirCharacteristic();
+
+ // If the directory characteristic is User but this framework was
+ // user-specified to be treated as a system framework, promote the
+ // characteristic.
+ if (HFI.DirInfo == SrcMgr::C_User && InUserSpecifiedSystemFramework)
+ HFI.DirInfo = SrcMgr::C_System;
+
+ // If the filename matches a known system header prefix, override
+ // whether the file is a system header.
+ for (unsigned j = SystemHeaderPrefixes.size(); j; --j) {
+ if (Filename.starts_with(SystemHeaderPrefixes[j - 1].first)) {
+ HFI.DirInfo = SystemHeaderPrefixes[j - 1].second ? SrcMgr::C_System
+ : SrcMgr::C_User;
+ break;
+ }
}
- }
- if (checkMSVCHeaderSearch(Diags, MSFE, &File->getFileEntry(), IncludeLoc)) {
- if (SuggestedModule)
+ if (checkMSVCHeaderSearch(Diags, MSFE, &File->getFileEntry(),
+ IncludeLoc) &&
+ SuggestedModule)
*SuggestedModule = MSSuggestedModule;
- return MSFE;
- }
- bool FoundByHeaderMap = !IsMapped ? false : *IsMapped;
- if (!Includers.empty())
- diagnoseFrameworkInclude(Diags, IncludeLoc,
- Includers.front().second.getName(), Filename,
- *File, isAngled, FoundByHeaderMap);
+ bool FoundByHeaderMap = !IsMapped ? false : *IsMapped;
+ if (!Includers.empty())
+ diagnoseFrameworkInclude(Diags, IncludeLoc,
+ Includers.front().second.getName(), Filename,
+ *File, isAngled, FoundByHeaderMap);
- // Remember this location for the next lookup we do.
- cacheLookupSuccess(CacheLookup, It, IncludeLoc);
- return File;
+ // Remember this location for the next lookup we do.
+ cacheLookupSuccess(CacheLookup, It, IncludeLoc);
+ }
+
+ if (checkAndStoreCandidate(SuggestedModule, File, It->getName(), Diags,
+ Filename, IncludeLoc, FirstModule, FirstHeader,
+ FirstDir))
+ return FirstHeader;
}
- if (checkMSVCHeaderSearch(Diags, MSFE, nullptr, IncludeLoc)) {
+ if (First && checkMSVCHeaderSearch(Diags, MSFE, nullptr, IncludeLoc)) {
if (SuggestedModule)
*SuggestedModule = MSSuggestedModule;
return MSFE;
}
+ if (FirstHeader)
+ return FirstHeader;
+
// Otherwise, didn't find it. Remember we didn't find this.
CacheLookup.HitIt = search_dir_end();
return std::nullopt;
diff --git a/clang/test/Preprocessor/header-shadowing.c b/clang/test/Preprocessor/header-shadowing.c
new file mode 100644
index 0000000000000..08b3c3e3f8db7
--- /dev/null
+++ b/clang/test/Preprocessor/header-shadowing.c
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+// RUN: %clang_cc1 -Wheader-shadowing -Eonly %t/main.c -I %t/include 2>&1 | FileCheck %s --check-prefix=SHADOWING
+// SHADOWING: {{.*}} warning: multiple candidates for header 'header.h' found;
+
+//--- main.c
+#include "header.h"
+
+//--- include/header.h
+#pragma once
+
+//--- header.h
+#pragma once
More information about the cfe-commits
mailing list