[clang] [clang][modules] Avoid modules diagnostics for `__has_include()` (PR #71450)
Jan Svoboda via cfe-commits
cfe-commits at lists.llvm.org
Mon Nov 6 14:34:17 PST 2023
https://github.com/jansvoboda11 created https://github.com/llvm/llvm-project/pull/71450
After #70144 Clang started resolving module maps even for `__has_include()` expressions. The unintended consequence of this is that diagnostics related to module mis-use started trigerring. These diagnostics are clearly intended to catch actual usage of a header, so should not apply for `__has_include()` expressions that don't bring the contents of the header into the TU.
>From f66c121e698142892b4c59163cae38cd10fc1e0c Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 6 Nov 2023 13:05:23 -0800
Subject: [PATCH 1/2] Fix
---
clang/lib/Lex/PPDirectives.cpp | 36 +++++++++++++++++++---------------
1 file changed, 20 insertions(+), 16 deletions(-)
diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp
index d97a103833c2fa6..956e2276f25b710 100644
--- a/clang/lib/Lex/PPDirectives.cpp
+++ b/clang/lib/Lex/PPDirectives.cpp
@@ -960,7 +960,6 @@ OptionalFileEntryRef Preprocessor::LookupFile(
Module *RequestingModule = getModuleForLocation(
FilenameLoc, LangOpts.ModulesValidateTextualHeaderIncludes);
- bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
// If the header lookup mechanism may be relative to the current inclusion
// stack, record the parent #includes.
@@ -1041,13 +1040,8 @@ OptionalFileEntryRef Preprocessor::LookupFile(
Filename, FilenameLoc, isAngled, FromDir, &CurDir, Includers, SearchPath,
RelativePath, RequestingModule, SuggestedModule, IsMapped,
IsFrameworkFound, SkipCache, BuildSystemModule, OpenFile, CacheFailures);
- if (FE) {
- if (SuggestedModule && !LangOpts.AsmPreprocessor)
- HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
- RequestingModule, RequestingModuleIsModuleInterface, FilenameLoc,
- Filename, *FE);
+ if (FE)
return FE;
- }
OptionalFileEntryRef CurFileEnt;
// Otherwise, see if this is a subframework header. If so, this is relative
@@ -1058,10 +1052,6 @@ OptionalFileEntryRef Preprocessor::LookupFile(
if (OptionalFileEntryRef FE = HeaderInfo.LookupSubframeworkHeader(
Filename, *CurFileEnt, SearchPath, RelativePath, RequestingModule,
SuggestedModule)) {
- if (SuggestedModule && !LangOpts.AsmPreprocessor)
- HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
- RequestingModule, RequestingModuleIsModuleInterface, FilenameLoc,
- Filename, *FE);
return FE;
}
}
@@ -1073,10 +1063,6 @@ OptionalFileEntryRef Preprocessor::LookupFile(
if (OptionalFileEntryRef FE = HeaderInfo.LookupSubframeworkHeader(
Filename, *CurFileEnt, SearchPath, RelativePath,
RequestingModule, SuggestedModule)) {
- if (SuggestedModule && !LangOpts.AsmPreprocessor)
- HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
- RequestingModule, RequestingModuleIsModuleInterface,
- FilenameLoc, Filename, *FE);
return FE;
}
}
@@ -2027,12 +2013,28 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport(
const FileEntry *LookupFromFile, StringRef &LookupFilename,
SmallVectorImpl<char> &RelativePath, SmallVectorImpl<char> &SearchPath,
ModuleMap::KnownHeader &SuggestedModule, bool isAngled) {
+ auto DiagnoseHeaderInclusion = [&](FileEntryRef FE) {
+ if (LangOpts.AsmPreprocessor)
+ return;
+
+ Module *RequestingModule = getModuleForLocation(
+ FilenameLoc, LangOpts.ModulesValidateTextualHeaderIncludes);
+ bool RequestingModuleIsModuleInterface =
+ !SourceMgr.isInMainFile(FilenameLoc);
+
+ HeaderInfo.getModuleMap().diagnoseHeaderInclusion(
+ RequestingModule, RequestingModuleIsModuleInterface, FilenameLoc,
+ Filename, FE);
+ };
+
OptionalFileEntryRef File = LookupFile(
FilenameLoc, LookupFilename, isAngled, LookupFrom, LookupFromFile, CurDir,
Callbacks ? &SearchPath : nullptr, Callbacks ? &RelativePath : nullptr,
&SuggestedModule, &IsMapped, &IsFrameworkFound);
- if (File)
+ if (File) {
+ DiagnoseHeaderInclusion(*File);
return File;
+ }
// Give the clients a chance to silently skip this include.
if (Callbacks && Callbacks->FileNotFound(Filename))
@@ -2051,6 +2053,7 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport(
&SuggestedModule, &IsMapped,
/*IsFrameworkFound=*/nullptr);
if (File) {
+ DiagnoseHeaderInclusion(*File);
Diag(FilenameTok, diag::err_pp_file_not_found_angled_include_not_fatal)
<< Filename << IsImportDecl
<< FixItHint::CreateReplacement(FilenameRange,
@@ -2081,6 +2084,7 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport(
Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped,
/*IsFrameworkFound=*/nullptr);
if (File) {
+ DiagnoseHeaderInclusion(*File);
auto Hint =
isAngled ? FixItHint::CreateReplacement(
FilenameRange, "<" + TypoCorrectionName.str() + ">")
>From 961707929fe12ab78386450e04dfea541b08b9c1 Mon Sep 17 00:00:00 2001
From: Jan Svoboda <jan_svoboda at apple.com>
Date: Mon, 6 Nov 2023 13:05:26 -0800
Subject: [PATCH 2/2] Test
---
clang/test/Modules/has_include_non_modular.c | 14 ++++++++++++++
1 file changed, 14 insertions(+)
create mode 100644 clang/test/Modules/has_include_non_modular.c
diff --git a/clang/test/Modules/has_include_non_modular.c b/clang/test/Modules/has_include_non_modular.c
new file mode 100644
index 000000000000000..2e1a06bdc6e4a0d
--- /dev/null
+++ b/clang/test/Modules/has_include_non_modular.c
@@ -0,0 +1,14 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+
+//--- module.modulemap
+module Mod { header "mod.h" }
+//--- mod.h
+#if __has_include("textual.h")
+#endif
+//--- textual.h
+
+//--- tu.c
+#include "mod.h"
+
+// RUN: %clang -fsyntax-only %t/tu.c -fmodules -fmodules-cache-path=%t/cache -Werror=non-modular-include-in-module
More information about the cfe-commits
mailing list