Enforce module decl-use restrictions and private header restrictions in textual headers

Per the documentation, these restrictions were intended to apply to textual headers but previously this didn't work because we decided there was no requesting module when the `#include` was in a textual header.

A `-cc1` flag is provided to restore the old behavior for transitionary purposes.

Reviewed By: aaron.ballman

Differential Revision: https://reviews.llvm.org/D132779
This commit is contained in:
Richard Smith 2022-09-06 17:10:55 -07:00
parent 4f5147a7cf
commit a002063de3
8 changed files with 33 additions and 5 deletions

View File

@ -138,6 +138,13 @@ Non-comprehensive list of changes in this release
- It's now possible to set the crash diagnostics directory through
the environment variable ``CLANG_CRASH_DIAGNOSTICS_DIR``.
The ``-fcrash-diagnostics-dir`` flag takes precedence.
- When using header modules, inclusion of a private header and violations of
the `use-declaration rules
<https://clang.llvm.org/docs/Modules.html#use-declaration>`_ are now
diagnosed even when the includer is a textual header. This change can be
temporarily reversed with ``-Xclang
-fno-modules-validate-textual-header-includes``, but this flag will be
removed in a future Clang release.
New Compiler Flags
------------------

View File

@ -181,6 +181,7 @@ BENIGN_LANGOPT(PCHInstantiateTemplates, 1, 0, "instantiate templates while build
COMPATIBLE_LANGOPT(ModulesDeclUse , 1, 0, "require declaration of module uses")
BENIGN_LANGOPT(ModulesSearchAll , 1, 1, "searching even non-imported modules to find unresolved references")
COMPATIBLE_LANGOPT(ModulesStrictDeclUse, 1, 0, "requiring declaration of module uses and all headers to be in modules")
COMPATIBLE_LANGOPT(ModulesValidateTextualHeaderIncludes, 1, 1, "validation of textual header includes")
BENIGN_LANGOPT(ModulesErrorRecovery, 1, 1, "automatically importing modules as needed when performing error recovery")
BENIGN_LANGOPT(ImplicitModules, 1, 1, "building modules that are not specified via -fmodule-file")
COMPATIBLE_LANGOPT(ModulesLocalVisibility, 1, 0, "local submodule visibility")

View File

@ -2273,6 +2273,12 @@ defm modules_validate_system_headers : BoolOption<"f", "modules-validate-system-
HeaderSearchOpts<"ModulesValidateSystemHeaders">, DefaultFalse,
PosFlag<SetTrue, [CC1Option], "Validate the system headers that a module depends on when loading the module">,
NegFlag<SetFalse, [NoXarchOption]>>, Group<i_Group>;
def fno_modules_validate_textual_header_includes :
Flag<["-"], "fno-modules-validate-textual-header-includes">,
Group<f_Group>, Flags<[CC1Option, NoXarchOption]>,
MarshallingInfoNegativeFlag<LangOpts<"ModulesValidateTextualHeaderIncludes">>,
HelpText<"Do not enforce -fmodules-decluse and private header restrictions for textual headers. "
"This flag will be removed in a future Clang release.">;
def fvalidate_ast_input_files_content:
Flag <["-"], "fvalidate-ast-input-files-content">,

View File

@ -2551,7 +2551,7 @@ public:
/// Find the module that owns the source or header file that
/// \p Loc points to. If the location is in a file that was included
/// into a module, or is outside any module, returns nullptr.
Module *getModuleForLocation(SourceLocation Loc);
Module *getModuleForLocation(SourceLocation Loc, bool AllowTextual);
/// We want to produce a diagnostic at location IncLoc concerning an
/// unreachable effect at location MLoc (eg, where a desired entity was

View File

@ -856,7 +856,8 @@ void Preprocessor::SkipExcludedConditionalBlock(SourceLocation HashTokenLoc,
Tok.getLocation());
}
Module *Preprocessor::getModuleForLocation(SourceLocation Loc) {
Module *Preprocessor::getModuleForLocation(SourceLocation Loc,
bool AllowTextual) {
if (!SourceMgr.isInMainFile(Loc)) {
// Try to determine the module of the include directive.
// FIXME: Look into directly passing the FileEntry from LookupFile instead.
@ -864,7 +865,7 @@ Module *Preprocessor::getModuleForLocation(SourceLocation Loc) {
if (const FileEntry *EntryOfIncl = SourceMgr.getFileEntryForID(IDOfIncl)) {
// The include comes from an included file.
return HeaderInfo.getModuleMap()
.findModuleForHeader(EntryOfIncl)
.findModuleForHeader(EntryOfIncl, AllowTextual)
.getModule();
}
}
@ -879,7 +880,8 @@ Module *Preprocessor::getModuleForLocation(SourceLocation Loc) {
const FileEntry *
Preprocessor::getHeaderToIncludeForDiagnostics(SourceLocation IncLoc,
SourceLocation Loc) {
Module *IncM = getModuleForLocation(IncLoc);
Module *IncM = getModuleForLocation(
IncLoc, LangOpts.ModulesValidateTextualHeaderIncludes);
// Walk up through the include stack, looking through textual headers of M
// until we hit a non-textual header that we can #include. (We assume textual
@ -953,7 +955,8 @@ Optional<FileEntryRef> Preprocessor::LookupFile(
ConstSearchDirIterator CurDirLocal = nullptr;
ConstSearchDirIterator &CurDir = CurDirArg ? *CurDirArg : CurDirLocal;
Module *RequestingModule = getModuleForLocation(FilenameLoc);
Module *RequestingModule = getModuleForLocation(
FilenameLoc, LangOpts.ModulesValidateTextualHeaderIncludes);
bool RequestingModuleIsModuleInterface = !SourceMgr.isInMainFile(FilenameLoc);
// If the header lookup mechanism may be relative to the current inclusion

View File

@ -73,3 +73,7 @@ module XN {
module XS {
}
module Textual {
textual header "textual.h"
}

View File

@ -0,0 +1 @@
#include "a.h"

View File

@ -0,0 +1,6 @@
// RUN: rm -rf %t
// RUN: %clang_cc1 -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-decluse -fmodule-name=Textual -I %S/Inputs/declare-use %s -verify
// RUN: %clang_cc1 -fimplicit-module-maps -fmodules-cache-path=%t -fmodules-decluse -fmodule-name=Textual -I %S/Inputs/declare-use %s -fno-modules-validate-textual-header-includes
// expected-error@textual.h:* {{module Textual does not depend on a module exporting 'a.h'}}
#include "textual.h"