[PATCH] D51910: [Modules] Add platform feature to requires clause

This revision was automatically updated to reflect the committed changes. Closed by commit rL342499: [Modules] Add platform and environment features to requires clause (authored by bruno, committed by ). Herald added a subscriber: llvm-commits. Changed prior to commit:

[PATCH] D51910: [Modules] Add platform feature to requires clause

This revision was automatically updated to reflect the committed changes. Closed by commit rC342499: [Modules] Add platform and environment features to requires clause (authored by bruno, committed by ). Changed prior to commit: https://reviews.llvm.org/D51910?vs=165861=165998#toc Repository:

[PATCH] D51910: [Modules] Add platform feature to requires clause

rsmith accepted this revision. rsmith added inline comments. This revision is now accepted and ready to land. Comment at: lib/Basic/Module.cpp:81 + // 2, Environment + // 3. Platform-Environment + if (Platform == Feature || Target.getTriple().getOSName() == Feature ||

[PATCH] D51910: [Modules] Add platform feature to requires clause

bruno updated this revision to Diff 165861. bruno added a comment. Update patch after review. https://reviews.llvm.org/D51910 Files: docs/Modules.rst lib/Basic/Module.cpp test/Modules/target-platform-features.m Index: test/Modules/target-platform-features.m

[PATCH] D51910: [Modules] Add platform feature to requires clause

bruno added inline comments. Comment at: docs/Modules.rst:476-477 + +*platform-environment* + A platform-environment variant (e.g. ``linux-gnueabi``, ``windows-msvc``) is available. rsmith wrote: > What is the reason to allow these to be combined into a

[PATCH] D51910: [Modules] Add platform feature to requires clause

rsmith added inline comments. Comment at: docs/Modules.rst:476-477 + +*platform-environment* + A platform-environment variant (e.g. ``linux-gnueabi``, ``windows-msvc``) is available. What is the reason to allow these to be combined into a single feature name

[PATCH] D51910: [Modules] Add platform feature to requires clause

aprantl added inline comments. Comment at: lib/Basic/Module.cpp:101 + // variant (2), the simulator is hardcoded as part of the platform name. Both + // forms above should match "iossimulator" and "ios-simulator" features. + if (Target.getTriple().isOSDarwin() &&

[PATCH] D51910: [Modules] Add platform feature to requires clause

aprantl added inline comments. Comment at: docs/Modules.rst:470 +*platform variant* + A specific os/platform variant (e.g. ``ios``, ``macos``, ``android``, ``win32``, ``linux``, etc) is available. aprantl wrote: > Does this work with platforms+environment

[PATCH] D51910: [Modules] Add platform feature to requires clause

bruno updated this revision to Diff 165612. bruno added a comment. Addressed Adrian's review. Added support to consider the environment and well the combination platform-environment. @aprantl also handled multiple variants of simulator combinations. https://reviews.llvm.org/D51910 Files:

[PATCH] D51910: [Modules] Add platform feature to requires clause

aprantl added inline comments. Comment at: docs/Modules.rst:470 +*platform variant* + A specific os/platform variant (e.g. ``ios``, ``macos``, ``android``, ``win32``, ``linux``, etc) is available. Does this work with platforms+environment combinations, such

[PATCH] D51910: [Modules] Add platform feature to requires clause

bruno created this revision. bruno added reviewers: rsmith, v.g.vassilev, aprantl. Herald added subscribers: dexonsmith, srhines. Allows module map writers to add build requirements based on platform/os. Useful when target features and language dialects aren't enough to conditionalize building