r206027 - Add -fmodules-strict-decluse to check that all headers are in modules

Richard Smith richard at metafoo.co.uk
Mon Apr 14 17:47:08 PDT 2014


On Mon, Apr 14, 2014 at 11:53 AM, Ben Langmuir <blangmuir at apple.com> wrote:

> To be clear, my patch does not involve decluse, so not quite the same.
>

There's no reason for this patch to be restricted to decluse (just as
there's no reason for yours to be restricted to framework modules in the
long term). The underlying intent is the same, and it doesn't make sense to
have two different implementations of it.

Also (by way of review), I think this patch doesn’t handle umbrella
> directories correctly, since it only looks for known headers.
>

Hmm, if we're getting that part wrong, we'll be getting it wrong for the
decluse checking in general. Nice catch =)


> On Apr 14, 2014, at 11:42 AM, Richard Smith <richard at metafoo.co.uk> wrote:
>
> Why is this a -f option rather than a diagnostic? Why is it coupled to
> -fmodules-decluse? See also Ben Langmuir's patch that does the same thing
> for framework modules (also, oddly, with a language option).
>
>
> On Fri, Apr 11, 2014 at 4:47 AM, Daniel Jasper <djasper at google.com> wrote:
>
>> Author: djasper
>> Date: Fri Apr 11 06:47:45 2014
>> New Revision: 206027
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=206027&view=rev
>> Log:
>> Add -fmodules-strict-decluse to check that all headers are in modules
>>
>> Review: http://reviews.llvm.org/D3335
>>
>> Added:
>>     cfe/trunk/test/Modules/strict-decluse.cpp
>> Modified:
>>     cfe/trunk/include/clang/Basic/LangOptions.def
>>     cfe/trunk/include/clang/Driver/Options.td
>>     cfe/trunk/lib/Driver/Tools.cpp
>>     cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>>     cfe/trunk/lib/Lex/ModuleMap.cpp
>>
>> Modified: cfe/trunk/include/clang/Basic/LangOptions.def
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/LangOptions.def?rev=206027&r1=206026&r2=206027&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/LangOptions.def (original)
>> +++ cfe/trunk/include/clang/Basic/LangOptions.def Fri Apr 11 06:47:45 2014
>> @@ -96,6 +96,7 @@ LANGOPT(MathErrno         , 1, 1, "errno
>>  BENIGN_LANGOPT(HeinousExtensions , 1, 0, "Extensions that we really
>> don't like and may be ripped out at any time")
>>  LANGOPT(Modules           , 1, 0, "modules extension to C")
>>  LANGOPT(ModulesDeclUse    , 1, 0, "require declaration of module uses")
>> +LANGOPT(ModulesStrictDeclUse, 1, 0, "require declaration of module uses
>> and all headers to be in modules")
>>  LANGOPT(Optimize          , 1, 0, "__OPTIMIZE__ predefined macro")
>>  LANGOPT(OptimizeSize      , 1, 0, "__OPTIMIZE_SIZE__ predefined macro")
>>  LANGOPT(Static            , 1, 0, "__STATIC__ predefined macro (as
>> opposed to __DYNAMIC__)")
>>
>> Modified: cfe/trunk/include/clang/Driver/Options.td
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=206027&r1=206026&r2=206027&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Driver/Options.td (original)
>> +++ cfe/trunk/include/clang/Driver/Options.td Fri Apr 11 06:47:45 2014
>> @@ -607,6 +607,9 @@ def fmodules_ignore_macro : Joined<["-"]
>>  def fmodules_decluse : Flag <["-"], "fmodules-decluse">, Group<f_Group>,
>>    Flags<[DriverOption,CC1Option]>,
>>    HelpText<"Require declaration of modules used within a module">;
>> +def fmodules_strict_decluse : Flag <["-"], "fmodules-strict-decluse">,
>> Group<f_Group>,
>> +  Flags<[DriverOption,CC1Option]>,
>> +  HelpText<"Like -fmodules-decluse but requires all headers to be in
>> modules">;
>>  def fretain_comments_from_system_headers : Flag<["-"],
>> "fretain-comments-from-system-headers">, Group<f_Group>, Flags<[CC1Option]>;
>>
>>  def fmudflapth : Flag<["-"], "fmudflapth">, Group<f_Group>;
>> @@ -666,6 +669,8 @@ def fno_module_maps : Flag <["-"], "fno-
>>    Flags<[DriverOption]>;
>>  def fno_modules_decluse : Flag <["-"], "fno-modules-decluse">,
>> Group<f_Group>,
>>    Flags<[DriverOption]>;
>> +def fno_modules_strict_decluse : Flag <["-"],
>> "fno-strict-modules-decluse">, Group<f_Group>,
>> +  Flags<[DriverOption]>;
>>  def fno_ms_extensions : Flag<["-"], "fno-ms-extensions">, Group<f_Group>;
>>  def fno_ms_compatibility : Flag<["-"], "fno-ms-compatibility">,
>> Group<f_Group>;
>>  def fno_delayed_template_parsing : Flag<["-"],
>> "fno-delayed-template-parsing">, Group<f_Group>;
>>
>> Modified: cfe/trunk/lib/Driver/Tools.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Tools.cpp?rev=206027&r1=206026&r2=206027&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Driver/Tools.cpp (original)
>> +++ cfe/trunk/lib/Driver/Tools.cpp Fri Apr 11 06:47:45 2014
>> @@ -3469,6 +3469,14 @@ void Clang::ConstructJob(Compilation &C,
>>      CmdArgs.push_back("-fmodules-decluse");
>>    }
>>
>> +  // -fmodules-strict-decluse is like -fmodule-decluse, but also checks
>> that
>> +  // all #included headers are part of modules.
>> +  if (Args.hasFlag(options::OPT_fmodules_strict_decluse,
>> +                   options::OPT_fno_modules_strict_decluse,
>> +                   false)) {
>> +    CmdArgs.push_back("-fmodules-strict-decluse");
>> +  }
>> +
>>    // -fmodule-name specifies the module that is currently being built (or
>>    // used for header checking by -fmodule-maps).
>>    if (Arg *A = Args.getLastArg(options::OPT_fmodule_name)) {
>>
>> Modified: cfe/trunk/lib/Frontend/CompilerInvocation.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInvocation.cpp?rev=206027&r1=206026&r2=206027&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Frontend/CompilerInvocation.cpp (original)
>> +++ cfe/trunk/lib/Frontend/CompilerInvocation.cpp Fri Apr 11 06:47:45 2014
>> @@ -1343,7 +1343,9 @@ static void ParseLangArgs(LangOptions &O
>>    Opts.Blocks = Args.hasArg(OPT_fblocks);
>>    Opts.BlocksRuntimeOptional = Args.hasArg(OPT_fblocks_runtime_optional);
>>    Opts.Modules = Args.hasArg(OPT_fmodules);
>> -  Opts.ModulesDeclUse = Args.hasArg(OPT_fmodules_decluse);
>> +  Opts.ModulesStrictDeclUse = Args.hasArg(OPT_fmodules_strict_decluse);
>> +  Opts.ModulesDeclUse =
>> +      Args.hasArg(OPT_fmodules_decluse) || Opts.ModulesStrictDeclUse;
>>    Opts.CharIsSigned = Opts.OpenCL || !Args.hasArg(OPT_fno_signed_char);
>>    Opts.WChar = Opts.CPlusPlus && !Args.hasArg(OPT_fno_wchar);
>>    Opts.ShortWChar = Args.hasFlag(OPT_fshort_wchar, OPT_fno_short_wchar,
>> false);
>>
>> Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=206027&r1=206026&r2=206027&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
>> +++ cfe/trunk/lib/Lex/ModuleMap.cpp Fri Apr 11 06:47:45 2014
>> @@ -243,8 +243,12 @@ void ModuleMap::diagnoseHeaderInclusion(
>>      resolveUses(RequestingModule, /*Complain=*/false);
>>
>>    HeadersMap::iterator Known = findKnownHeader(File);
>> -  if (Known == Headers.end())
>> +  if (Known == Headers.end()) {
>> +    if (LangOpts.ModulesStrictDeclUse)
>> +      Diags.Report(FilenameLoc, diag::error_undeclared_use_of_module)
>> +          << RequestingModule->getFullModuleName() << Filename;
>>      return;
>> +  }
>>
>>    Module *Private = NULL;
>>    Module *NotUsed = NULL;
>>
>> Added: cfe/trunk/test/Modules/strict-decluse.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/strict-decluse.cpp?rev=206027&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/strict-decluse.cpp (added)
>> +++ cfe/trunk/test/Modules/strict-decluse.cpp Fri Apr 11 06:47:45 2014
>> @@ -0,0 +1,9 @@
>> +// RUN: rm -rf %t
>> +// RUN: %clang_cc1 -fmodule-maps -fmodules-cache-path=%t
>> -fmodules-strict-decluse -fmodule-name=XG -I %S/Inputs/declare-use %s
>> -verify
>> +
>> +#include "g.h"
>> +#include "e.h"
>> +#include "f.h" // expected-error {{module XG does not depend on a module
>> exporting 'f.h'}}
>> +#include "i.h" // expected-error {{module XG does not depend on a module
>> exporting 'i.h'}}
>> +
>> +const int g2 = g1 + e + f + aux_i;
>>
>>
>> _______________________________________________
>> cfe-commits mailing list
>> cfe-commits at cs.uiuc.edu
>> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>>
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20140414/dc435e43/attachment.html>


More information about the cfe-commits mailing list