On Thu, Jan 19, 2017 at 1:27 PM, Richard Smith <rich...@metafoo.co.uk> wrote:
> On 19 Jan 2017 11:16 am, "Manman Ren via cfe-commits" < > cfe-commits@lists.llvm.org> wrote: > > Author: mren > Date: Thu Jan 19 13:05:55 2017 > New Revision: 292508 > > URL: http://llvm.org/viewvc/llvm-project?rev=292508&view=rev > Log: > Module: Improve diagnostic message when cxx modules are disabled and > @import is used in Objective CXX. > > rdar://problem/19399671 > > Added: > cfe/trunk/test/Modules/check-syntax.mm > Modified: > cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td > cfe/trunk/lib/Parse/ParseObjc.cpp > > Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ > Basic/DiagnosticParseKinds.td?rev=292508&r1=292507&r2=292508&view=diff > ============================================================ > ================== > --- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original) > +++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Thu Jan 19 > 13:05:55 2017 > @@ -243,7 +243,10 @@ def err_expected_property_name : Error<" > > def err_unexpected_at : Error<"unexpected '@' in program">; > def err_atimport : Error< > -"use of '@import' when modules are disabled">; > + "use of '@import' when modules are disabled">; > +def err_atimport_cxx : Error< > + "use of '@import' when C++ modules are disabled, consider using > fmodules " > + "and fcxx-modules">; > > > Please add a hyphen before fmodules. There's also no reason to suggest > -fcxx-modules; modules support for C++ is controlled by -fmodules nowadays. > > I also don't think it makes sense to have different diagnostics for C and > C++ mode; we should suggest the -fmodules flag in either case. > Hi Richard, This is an internal patch that I am upstreaming to open source. Internally at Apple, modules support for C++ is off by default, users need to use -fcxx-modules to turn it on. So when they use -fmodules without -fcxx-modules, they will get a confusing error saying modules are disabled. Open source, the testing case in this commit uses driver option -fno-cxx-modules, in this case, it makes sense to call out c++ modules so users know to remove -fno-cxx-modules. Let me know your thoughts, Manman > > def err_invalid_reference_qualifier_application : Error< > "'%0' qualifier may not be applied to a reference">; > > Modified: cfe/trunk/lib/Parse/ParseObjc.cpp > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Pars > eObjc.cpp?rev=292508&r1=292507&r2=292508&view=diff > ============================================================ > ================== > --- cfe/trunk/lib/Parse/ParseObjc.cpp (original) > +++ cfe/trunk/lib/Parse/ParseObjc.cpp Thu Jan 19 13:05:55 2017 > @@ -83,7 +83,10 @@ Parser::DeclGroupPtrTy Parser::ParseObjC > case tok::objc_import: > if (getLangOpts().Modules || getLangOpts().DebuggerSupport) > return ParseModuleImport(AtLoc); > - Diag(AtLoc, diag::err_atimport); > + if (getLangOpts().CPlusPlus) > + Diag(AtLoc, diag::err_atimport_cxx); > + else > + Diag(AtLoc, diag::err_atimport); > SkipUntil(tok::semi); > return Actions.ConvertDeclToDeclGroup(nullptr); > default: > > Added: cfe/trunk/test/Modules/check-syntax.mm > URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/ > check-syntax.mm?rev=292508&view=auto > ============================================================ > ================== > --- cfe/trunk/test/Modules/check-syntax.mm (added) > +++ cfe/trunk/test/Modules/check-syntax.mm Thu Jan 19 13:05:55 2017 > @@ -0,0 +1,5 @@ > +// RUN: not %clang -fmodules -fno-cxx-modules -fsyntax-only %s 2>&1 | > FileCheck %s > +// rdar://19399671 > + > +// CHECK: use of '@import' when C++ modules are disabled > +@import Foundation; > > > _______________________________________________ > 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
