On Tue, Nov 17, 2015 at 3:32 PM, Richard Smith via cfe-commits < cfe-commits@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@lists.llvm.org > http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits >
_______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
