r253398 - [modules] When a #include is mapped to a module import and appears somewhere
David Blaikie via cfe-commits
cfe-commits at lists.llvm.org
Tue Nov 17 15:40:52 PST 2015
On Tue, Nov 17, 2015 at 3:32 PM, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
> Author: rsmith
> Date: Tue Nov 17 17:32:01 2015
> New Revision: 253398
>
> URL: http://llvm.org/viewvc/llvm-project?rev=253398&view=rev
> Log:
> [modules] When a #include is mapped to a module import and appears
> somewhere
> other than the top level, we issue an error. This breaks a fair amount of
> C++
> code wrapping C libraries, where the C library is #included within a
> namespace
> / extern "C" combination, because the C library (probably) includes C++
> standard library headers which may be within modules.
>
> Without modules, this setup is harmless if (and *only* if) the
> corresponding
> standard library module was already included outside the namespace,
^ if that's the only place it's valid, is it that much more work to just
remove all such inclusions as pointless? If they ever trigger they'll do
bad things, yes?
(but I guess on a large enough codebase this fight is not worth fighting in
the effort to transition to modules? even if it's just deletions (code
review turnaround, etc))
> so
> downgrade the error to a default-error extension in that case, so that it
> can
> be selectively disabled for such misbehaving libraries.
>
> Modified:
> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> cfe/trunk/lib/Parse/ParseDecl.cpp
> cfe/trunk/lib/Parse/ParseDeclCXX.cpp
> cfe/trunk/lib/Parse/ParseStmt.cpp
> cfe/trunk/lib/Sema/SemaDecl.cpp
> cfe/trunk/test/Modules/auto-module-import.m
>
> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=253398&r1=253397&r2=253398&view=diff
>
> ==============================================================================
> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue Nov 17
> 17:32:01 2015
> @@ -7930,6 +7930,9 @@ def note_module_import_in_extern_c : Not
> "extern \"C\" language linkage specification begins here">;
> def err_module_import_not_at_top_level_fatal : Error<
> "import of module '%0' appears within %1">, DefaultFatal;
> +def ext_module_import_not_at_top_level_noop : ExtWarn<
> + "redundant #include of module '%0' appears within %1">, DefaultError,
> + InGroup<DiagGroup<"modules-import-nested-redundant">>;
> def note_module_import_not_at_top_level : Note<"%0 begins here">;
> def err_module_self_import : Error<
> "import of module '%0' appears within same top-level module '%1'">;
>
> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=253398&r1=253397&r2=253398&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Tue Nov 17 17:32:01 2015
> @@ -3589,8 +3589,8 @@ void Parser::ParseStructUnionBody(Source
> SmallVector<Decl *, 32> FieldDecls;
>
> // While we still have something to read, read the declarations in the
> struct.
> - while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof) &&
> - !tryParseMisplacedModuleImport()) {
> + while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
> + Tok.isNot(tok::eof)) {
> // Each iteration of this loop reads one struct-declaration.
>
> // Check for extraneous top-level semicolon.
>
> Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=253398&r1=253397&r2=253398&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Tue Nov 17 17:32:01 2015
> @@ -210,8 +210,8 @@ void Parser::ParseInnerNamespace(std::ve
> ParsedAttributes &attrs,
> BalancedDelimiterTracker &Tracker) {
> if (index == Ident.size()) {
> - while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof) &&
> - !tryParseMisplacedModuleImport()) {
> + while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
> + Tok.isNot(tok::eof)) {
> ParsedAttributesWithRange attrs(AttrFactory);
> MaybeParseCXX11Attributes(attrs);
> MaybeParseMicrosoftAttributes(attrs);
> @@ -3064,8 +3064,8 @@ void Parser::ParseCXXMemberSpecification
>
> if (TagDecl) {
> // While we still have something to read, read the
> member-declarations.
> - while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof) &&
> - !tryParseMisplacedModuleImport()) {
> + while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
> + Tok.isNot(tok::eof)) {
> // Each iteration of this loop reads one member-declaration.
> ParseCXXClassMemberDeclarationWithPragmas(
> CurAS, AccessAttrs, static_cast<DeclSpec::TST>(TagType),
> TagDecl);
>
> Modified: cfe/trunk/lib/Parse/ParseStmt.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseStmt.cpp?rev=253398&r1=253397&r2=253398&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Parse/ParseStmt.cpp (original)
> +++ cfe/trunk/lib/Parse/ParseStmt.cpp Tue Nov 17 17:32:01 2015
> @@ -948,8 +948,8 @@ StmtResult Parser::ParseCompoundStatemen
> Stmts.push_back(R.get());
> }
>
> - while (Tok.isNot(tok::r_brace) && Tok.isNot(tok::eof) &&
> - !tryParseMisplacedModuleImport()) {
> + while (!tryParseMisplacedModuleImport() && Tok.isNot(tok::r_brace) &&
> + Tok.isNot(tok::eof)) {
> if (Tok.is(tok::annot_pragma_unused)) {
> HandlePragmaUnused();
> continue;
>
> Modified: cfe/trunk/lib/Sema/SemaDecl.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDecl.cpp?rev=253398&r1=253397&r2=253398&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Sema/SemaDecl.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaDecl.cpp Tue Nov 17 17:32:01 2015
> @@ -14500,8 +14500,8 @@ Decl *Sema::ActOnFileScopeAsmDecl(Expr *
> }
>
> static void checkModuleImportContext(Sema &S, Module *M,
> - SourceLocation ImportLoc,
> - DeclContext *DC) {
> + SourceLocation ImportLoc,
> DeclContext *DC,
> + bool FromInclude = false) {
> SourceLocation ExternCLoc;
>
> if (auto *LSD = dyn_cast<LinkageSpecDecl>(DC)) {
> @@ -14520,7 +14520,9 @@ static void checkModuleImportContext(Sem
> DC = DC->getParent();
>
> if (!isa<TranslationUnitDecl>(DC)) {
> - S.Diag(ImportLoc, diag::err_module_import_not_at_top_level_fatal)
> + S.Diag(ImportLoc, (FromInclude && S.isModuleVisible(M))
> + ? diag::ext_module_import_not_at_top_level_noop
> + :
> diag::err_module_import_not_at_top_level_fatal)
> << M->getFullModuleName() << DC;
> S.Diag(cast<Decl>(DC)->getLocStart(),
> diag::note_module_import_not_at_top_level) << DC;
> @@ -14579,7 +14581,7 @@ DeclResult Sema::ActOnModuleImport(Sourc
> }
>
> void Sema::ActOnModuleInclude(SourceLocation DirectiveLoc, Module *Mod) {
> - checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext);
> + checkModuleImportContext(*this, Mod, DirectiveLoc, CurContext, true);
>
> // Determine whether we're in the #include buffer for a module. The
> #includes
> // in that buffer do not qualify as module imports; they're just an
>
> Modified: cfe/trunk/test/Modules/auto-module-import.m
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/auto-module-import.m?rev=253398&r1=253397&r2=253398&view=diff
>
> ==============================================================================
> --- cfe/trunk/test/Modules/auto-module-import.m (original)
> +++ cfe/trunk/test/Modules/auto-module-import.m Tue Nov 17 17:32:01 2015
> @@ -1,6 +1,7 @@
> // RUN: rm -rf %t
> // RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules
> -fimplicit-module-maps -F %S/Inputs %s -verify -DERRORS
> // RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules
> -fimplicit-module-maps -F %S/Inputs %s -verify
> +// RUN: %clang_cc1 -Wauto-import -fmodules-cache-path=%t -fmodules
> -fimplicit-module-maps -F %S/Inputs -xobjective-c++ %s -verify
> //
> // Test both with and without the declarations that refer to unimported
> // entities. For error recovery, those cases implicitly trigger an import.
> @@ -85,5 +86,16 @@ int getNotInModule() {
>
> void includeNotAtTopLevel() { // expected-note {{function
> 'includeNotAtTopLevel' begins here}}
> #include <NoUmbrella/A.h> // expected-warning {{treating #include as an
> import}} \
> - expected-error {{import of module
> 'NoUmbrella.A' appears within function 'includeNotAtTopLevel'}}
> + expected-error {{redundant #include of
> module 'NoUmbrella.A' appears within function 'includeNotAtTopLevel'}}
> }
> +
> +#ifdef __cplusplus
> +namespace NS { // expected-note {{begins here}}
> +#include <NoUmbrella/A.h> // expected-warning {{treating #include as an
> import}} \
> + expected-error {{redundant #include of
> module 'NoUmbrella.A' appears within namespace 'NS'}}
> +}
> +extern "C" { // expected-note {{begins here}}
> +#include <NoUmbrella/A.h> // expected-warning {{treating #include as an
> import}} \
> + expected-error {{import of C++ module
> 'NoUmbrella.A' appears within extern "C"}}
> +}
> +#endif
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20151117/49746b7e/attachment-0001.html>
More information about the cfe-commits
mailing list