r244912 - [Modules] Add Darwin-specific compatibility module map parsing hacks

Sean Silva via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 13 17:00:07 PDT 2015


On Thu, Aug 13, 2015 at 4:47 PM, Ben Langmuir <blangmuir at apple.com> wrote:

> Yep, that should be it!
>

Great!

-- Sean Silva


>
> On Aug 13, 2015, at 4:45 PM, Sean Silva <chisophugis at gmail.com> wrote:
>
> This was the last thing blocking http://reviews.llvm.org/D10423 on your
> side, right?
>
> -- Sean Silva
>
> On Thu, Aug 13, 2015 at 10:13 AM, Ben Langmuir via cfe-commits <
> cfe-commits at lists.llvm.org> wrote:
>
>> Author: benlangmuir
>> Date: Thu Aug 13 12:13:33 2015
>> New Revision: 244912
>>
>> URL: http://llvm.org/viewvc/llvm-project?rev=244912&view=rev
>> Log:
>> [Modules] Add Darwin-specific compatibility module map parsing hacks
>>
>> This preserves backwards compatibility for two hacks in the Darwin
>> system module map files:
>>
>> 1. The use of 'requires excluded' to make headers non-modular, which
>> should really be mapped to 'textual' now that we have this feature.
>>
>> 2. Silently removes a bogus cplusplus requirement from IOKit.avc.
>>
>> Once we start diagnosing missing requirements and headers on
>> auto-imports these would have broken compatibility with existing Darwin
>> SDKs.
>>
>> Added:
>>     cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h
>>     cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/
>>     cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h
>>     cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m
>> Modified:
>>     cfe/trunk/include/clang/Basic/Module.h
>>     cfe/trunk/lib/Basic/Module.cpp
>>     cfe/trunk/lib/Lex/ModuleMap.cpp
>>     cfe/trunk/test/Modules/Inputs/System/usr/include/module.map
>>
>> Modified: cfe/trunk/include/clang/Basic/Module.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Module.h?rev=244912&r1=244911&r2=244912&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/include/clang/Basic/Module.h (original)
>> +++ cfe/trunk/include/clang/Basic/Module.h Thu Aug 13 12:13:33 2015
>> @@ -356,6 +356,12 @@ public:
>>    /// its top-level module.
>>    std::string getFullModuleName() const;
>>
>> +  /// \brief Whether the full name of this module is equal to joining
>> +  /// \p nameParts with "."s.
>> +  ///
>> +  /// This is more efficient than getFullModuleName().
>> +  bool fullModuleNameIs(ArrayRef<StringRef> nameParts) const;
>> +
>>    /// \brief Retrieve the top-level module for this (sub)module, which
>> may
>>    /// be this module.
>>    Module *getTopLevelModule() {
>>
>> Modified: cfe/trunk/lib/Basic/Module.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=244912&r1=244911&r2=244912&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Basic/Module.cpp (original)
>> +++ cfe/trunk/lib/Basic/Module.cpp Thu Aug 13 12:13:33 2015
>> @@ -139,6 +139,15 @@ std::string Module::getFullModuleName()
>>    return Result;
>>  }
>>
>> +bool Module::fullModuleNameIs(ArrayRef<StringRef> nameParts) const {
>> +  for (const Module *M = this; M; M = M->Parent) {
>> +    if (nameParts.empty() || M->Name != nameParts.back())
>> +      return false;
>> +    nameParts = nameParts.drop_back();
>> +  }
>> +  return nameParts.empty();
>> +}
>> +
>>  Module::DirectoryName Module::getUmbrellaDir() const {
>>    if (Header U = getUmbrellaHeader())
>>      return {"", U.Entry->getDir()};
>>
>> Modified: cfe/trunk/lib/Lex/ModuleMap.cpp
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/ModuleMap.cpp?rev=244912&r1=244911&r2=244912&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/lib/Lex/ModuleMap.cpp (original)
>> +++ cfe/trunk/lib/Lex/ModuleMap.cpp Thu Aug 13 12:13:33 2015
>> @@ -1015,7 +1015,17 @@ namespace clang {
>>
>>      /// \brief The active module.
>>      Module *ActiveModule;
>> -
>> +
>> +    /// \brief Whether a module uses the 'requires excluded' hack to
>> mark its
>> +    /// contents as 'textual'.
>> +    ///
>> +    /// On older Darwin SDK versions, 'requires excluded' is used to
>> mark the
>> +    /// contents of the Darwin.C.excluded (assert.h) and Tcl.Private
>> modules as
>> +    /// non-modular headers.  For backwards compatibility, we continue to
>> +    /// support this idiom for just these modules, and map the headers to
>> +    /// 'textual' to match the original intent.
>> +    llvm::SmallPtrSet<Module *, 2> UsesRequiresExcludedHack;
>> +
>>      /// \brief Consume the current token and return its location.
>>      SourceLocation consumeToken();
>>
>> @@ -1570,6 +1580,38 @@ void ModuleMapParser::parseExternModuleD
>>              : File->getDir(), ExternLoc);
>>  }
>>
>> +/// Whether to add the requirement \p Feature to the module \p M.
>> +///
>> +/// This preserves backwards compatibility for two hacks in the Darwin
>> system
>> +/// module map files:
>> +///
>> +/// 1. The use of 'requires excluded' to make headers non-modular, which
>> +///    should really be mapped to 'textual' now that we have this
>> feature.  We
>> +///    drop the 'excluded' requirement, and set \p
>> IsRequiresExcludedHack to
>> +///    true.  Later, this bit will be used to map all the headers inside
>> this
>> +///    module to 'textual'.
>> +///
>> +///    This affects Darwin.C.excluded (for assert.h) and Tcl.Private.
>> +///
>> +/// 2. Removes a bogus cplusplus requirement from IOKit.avc.  This
>> requirement
>> +///    was never correct and causes issues now that we check it, so drop
>> it.
>> +static bool shouldAddRequirement(Module *M, StringRef Feature,
>> +                                 bool &IsRequiresExcludedHack) {
>> +  static const StringRef DarwinCExcluded[] = {"Darwin", "C", "excluded"};
>> +  static const StringRef TclPrivate[] = {"Tcl", "Private"};
>> +  static const StringRef IOKitAVC[] = {"IOKit", "avc"};
>> +
>> +  if (Feature == "excluded" && (M->fullModuleNameIs(DarwinCExcluded) ||
>> +                                M->fullModuleNameIs(TclPrivate))) {
>> +    IsRequiresExcludedHack = true;
>> +    return false;
>> +  } else if (Feature == "cplusplus" && M->fullModuleNameIs(IOKitAVC)) {
>> +    return false;
>> +  }
>> +
>> +  return true;
>> +}
>> +
>>  /// \brief Parse a requires declaration.
>>  ///
>>  ///   requires-declaration:
>> @@ -1605,9 +1647,18 @@ void ModuleMapParser::parseRequiresDecl(
>>      std::string Feature = Tok.getString();
>>      consumeToken();
>>
>> -    // Add this feature.
>> -    ActiveModule->addRequirement(Feature, RequiredState,
>> -                                 Map.LangOpts, *Map.Target);
>> +    bool IsRequiresExcludedHack = false;
>> +    bool ShouldAddRequirement =
>> +        shouldAddRequirement(ActiveModule, Feature,
>> IsRequiresExcludedHack);
>> +
>> +    if (IsRequiresExcludedHack)
>> +      UsesRequiresExcludedHack.insert(ActiveModule);
>> +
>> +    if (ShouldAddRequirement) {
>> +      // Add this feature.
>> +      ActiveModule->addRequirement(Feature, RequiredState, Map.LangOpts,
>> +                                   *Map.Target);
>> +    }
>>
>>      if (!Tok.is <http://tok.is>(MMToken::Comma))
>>        break;
>> @@ -1657,9 +1708,16 @@ void ModuleMapParser::parseHeaderDecl(MM
>>        consumeToken();
>>      }
>>    }
>> +
>>    if (LeadingToken == MMToken::TextualKeyword)
>>      Role = ModuleMap::ModuleHeaderRole(Role | ModuleMap::TextualHeader);
>>
>> +  if (UsesRequiresExcludedHack.count(ActiveModule)) {
>> +    // Mark this header 'textual' (see doc comment for
>> +    // Module::UsesRequiresExcludedHack).
>> +    Role = ModuleMap::ModuleHeaderRole(Role | ModuleMap::TextualHeader);
>> +  }
>> +
>>    if (LeadingToken != MMToken::HeaderKeyword) {
>>      if (!Tok.is <http://tok.is>(MMToken::HeaderKeyword)) {
>>        Diags.Report(Tok.getLocation(), diag::err_mmap_expected_header)
>> @@ -1838,14 +1896,40 @@ void ModuleMapParser::parseUmbrellaDirDe
>>      HadError = true;
>>      return;
>>    }
>> -
>> +
>> +  if (UsesRequiresExcludedHack.count(ActiveModule)) {
>> +    // Mark this header 'textual' (see doc comment for
>> +    // ModuleMapParser::UsesRequiresExcludedHack). Although iterating
>> over the
>> +    // directory is relatively expensive, in practice this only applies
>> to the
>> +    // uncommonly used Tcl module on Darwin platforms.
>> +    std::error_code EC;
>> +    SmallVector<Module::Header, 6> Headers;
>> +    for (llvm::sys::fs::recursive_directory_iterator I(Dir->getName(),
>> EC), E;
>> +         I != E && !EC; I.increment(EC)) {
>> +      if (const FileEntry *FE =
>> SourceMgr.getFileManager().getFile(I->path())) {
>> +
>> +        Module::Header Header = {I->path(), FE};
>> +        Headers.push_back(std::move(Header));
>> +      }
>> +    }
>> +
>> +    // Sort header paths so that the pcm doesn't depend on iteration
>> order.
>> +    llvm::array_pod_sort(Headers.begin(), Headers.end(),
>> +                         [](const Module::Header *A, const
>> Module::Header *B) {
>> +                           return
>> A->NameAsWritten.compare(B->NameAsWritten);
>> +                         });
>> +    for (auto &Header : Headers)
>> +      Map.addHeader(ActiveModule, std::move(Header),
>> ModuleMap::TextualHeader);
>> +    return;
>> +  }
>> +
>>    if (Module *OwningModule = Map.UmbrellaDirs[Dir]) {
>>      Diags.Report(UmbrellaLoc, diag::err_mmap_umbrella_clash)
>>        << OwningModule->getFullModuleName();
>>      HadError = true;
>>      return;
>> -  }
>> -
>> +  }
>> +
>>    // Record this umbrella directory.
>>    Map.setUmbrellaDir(ActiveModule, Dir, DirName);
>>  }
>>
>> Added: cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h?rev=244912&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h (added)
>> +++ cfe/trunk/test/Modules/Inputs/System/usr/include/assert.h Thu Aug 13
>> 12:13:33 2015
>> @@ -0,0 +1,2 @@
>> +// assert.h
>> +#define DARWIN_C_EXCLUDED 1
>>
>> Modified: cfe/trunk/test/Modules/Inputs/System/usr/include/module.map
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/module.map?rev=244912&r1=244911&r2=244912&view=diff
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/System/usr/include/module.map (original)
>> +++ cfe/trunk/test/Modules/Inputs/System/usr/include/module.map Thu Aug
>> 13 12:13:33 2015
>> @@ -30,3 +30,25 @@ module uses_other_constants {
>>    header "uses_other_constants.h"
>>    export *
>>  }
>> +
>> +module Darwin {
>> +  module C {
>> +    module excluded {
>> +      requires excluded
>> +      header "assert.h"
>> +    }
>> +  }
>> +}
>> +
>> +module Tcl {
>> +  module Private {
>> +    requires excluded
>> +    umbrella ""
>> +  }
>> +}
>> +
>> +module IOKit {
>> +  module avc {
>> +    requires cplusplus
>> +  }
>> +}
>>
>> Added:
>> cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h?rev=244912&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h
>> (added)
>> +++ cfe/trunk/test/Modules/Inputs/System/usr/include/tcl-private/header.h
>> Thu Aug 13 12:13:33 2015
>> @@ -0,0 +1,2 @@
>> +// tcl-private/header.h
>> +#define TCL_PRIVATE 1
>>
>> Added: cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m
>> URL:
>> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m?rev=244912&view=auto
>>
>> ==============================================================================
>> --- cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m (added)
>> +++ cfe/trunk/test/Modules/darwin_specific_modulemap_hacks.m Thu Aug 13
>> 12:13:33 2015
>> @@ -0,0 +1,22 @@
>> +// RUN: rm -rf %t
>> +// RUN: %clang_cc1 -fmodules -fmodules-cache-path=%t
>> -fimplicit-module-maps -isystem %S/Inputs/System/usr/include -triple
>> x86_64-apple-darwin10 %s -verify -fsyntax-only
>> +// expected-no-diagnostics
>> +
>> + at import Darwin.C.excluded; // no error, header is implicitly 'textual'
>> + at import Tcl.Private;       // no error, header is implicitly 'textual'
>> + at import IOKit.avc;         // no error, cplusplus requirement removed
>> +
>> +#if defined(DARWIN_C_EXCLUDED)
>> +#error assert.h should be textual
>> +#elif defined(TCL_PRIVATE)
>> +#error tcl-private/header.h should be textual
>> +#endif
>> +
>> +#import <assert.h>
>> +#import <tcl-private/header.h>
>> +
>> +#if !defined(DARWIN_C_EXCLUDED)
>> +#error assert.h missing
>> +#elif !defined(TCL_PRIVATE)
>> +#error tcl-private/header.h missing
>> +#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/20150813/888ee7ce/attachment-0001.html>


More information about the cfe-commits mailing list