From b62ebbbfde94b3b9a1a051bc763db12f8d05bb73 Mon Sep 17 00:00:00 2001 From: Jan Svoboda Date: Mon, 6 Nov 2023 17:31:27 -0800 Subject: [PATCH] [clang][modules] Avoid modules diagnostics for `__has_include()` (#71450) After #70144 Clang started resolving module maps even for `__has_include()` expressions. This had the unintended consequence of emitting diagnostics around header misuse. These don't make sense if you actually don't bring contents of the header into the importer, so should be skipped for `__has_include()`. This patch moves emission of these diagnostics out of `Preprocessor::LookupFile()` up into `Preprocessor::LookupHeaderIncludeOrImport()`. (cherry picked from commit bdb309c5fd4030ae6ff9d5114e3532d45a98a183) --- clang/lib/Lex/PPDirectives.cpp | 36 +++++++++++--------- clang/test/Modules/has_include_non_modular.c | 14 ++++++++ 2 files changed, 34 insertions(+), 16 deletions(-) create mode 100644 clang/test/Modules/has_include_non_modular.c diff --git a/clang/lib/Lex/PPDirectives.cpp b/clang/lib/Lex/PPDirectives.cpp index 0ef3a85acf0c3..30c67870d13db 100644 --- a/clang/lib/Lex/PPDirectives.cpp +++ b/clang/lib/Lex/PPDirectives.cpp @@ -953,7 +953,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. @@ -1029,13 +1028,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; - } const FileEntry *CurFileEnt; // Otherwise, see if this is a subframework header. If so, this is relative @@ -1046,10 +1040,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; } } @@ -1061,10 +1051,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; } } @@ -2093,12 +2079,28 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport( const FileEntry *LookupFromFile, StringRef &LookupFilename, SmallVectorImpl &RelativePath, SmallVectorImpl &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)) @@ -2117,6 +2119,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, @@ -2147,6 +2150,7 @@ OptionalFileEntryRef Preprocessor::LookupHeaderIncludeOrImport( Callbacks ? &RelativePath : nullptr, &SuggestedModule, &IsMapped, /*IsFrameworkFound=*/nullptr); if (File) { + DiagnoseHeaderInclusion(*File); auto Hint = isAngled ? FixItHint::CreateReplacement( FilenameRange, "<" + TypoCorrectionName.str() + ">") 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 0000000000000..2e1a06bdc6e4a --- /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