Skip to content

Commit

Permalink
[clang][modules] Avoid modules diagnostics for __has_include() (llv…
Browse files Browse the repository at this point in the history
…m#71450)

After llvm#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()`.
  • Loading branch information
jansvoboda11 authored Nov 7, 2023

Verified

This commit was signed with the committer’s verified signature.
1 parent 160d483 commit bdb309c
Showing 2 changed files with 34 additions and 16 deletions.
36 changes: 20 additions & 16 deletions clang/lib/Lex/PPDirectives.cpp
Original file line number Diff line number Diff line change
@@ -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() + ">")
14 changes: 14 additions & 0 deletions clang/test/Modules/has_include_non_modular.c
Original file line number Diff line number Diff line change
@@ -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

0 comments on commit bdb309c

Please sign in to comment.