When generate_default_umbrella_header is set to False, still add statements around extern double {module_name}VersionNumber; to umbrella header so that clang can resolve the situation when module_name is the same for two different modules (where their path is diff).
When generate_default_umbrella_header is set to False, use extern instead of FOUNDATION_EXPORT since FOUNDATION_EXPORT is extern and trying to define it in else clause will cause warning: ambiguous expansion of macro 'FOUNDATION_EXPORT' error.
Why this change:
The original PR that introduced the flag generate_default_umbrella_header essentially wants to not import UIKit/Foundation to restrict on what to import by default, which is a good thing. However, it kinda overkill by also not including the statements around version number and version string. Which apparently affects how clang resolves module mapping (from a module name to a path) and in our set up, leading to error similar to:
path/to/alice.modulemap:2:21: error: umbrella for module 'bob' already covers this directory
umbrella header "foo-umbrella.h"
^
For our case, the umbrella header name for both alice and bob modules are foo, which is totally fine since their path is different. But for some reason when trying to compile the code in a remote execution service such as buildfarm, above error shows up. I cannot reproduce it with local or sandboxed mode.
Now looking back, the theory is that when there is clash on umbrealla header name, it will use the module's version string/number to figure out which is which? I don't really have any other theory for now. I cannot find such reference to VersionNumber/VersionString that is meaningful. Nor can I setup a test case here since it only break remote execution.
This does not revert the original PR in anyway, I still get to implicitly import Foundation/UIKit with this change.
Tests
No change on testing since this flag was not tested anywhere for now. I can add this flag to some random testing targets in this PR though but tests so far are still all passing with this turned off.
What changed:
extern double {module_name}VersionNumber;to umbrella header so that clang can resolve the situation when module_name is the same for two different modules (where their path is diff).externinstead ofFOUNDATION_EXPORTsinceFOUNDATION_EXPORTisexternand trying to define it inelseclause will causewarning: ambiguous expansion of macro 'FOUNDATION_EXPORT'error.Why this change:
The original PR that introduced the flag
generate_default_umbrella_headeressentially wants to not import UIKit/Foundation to restrict on what to import by default, which is a good thing. However, it kinda overkill by also not including the statements around version number and version string. Which apparently affects how clang resolves module mapping (from a module name to a path) and in our set up, leading to error similar to:For reference, this is originated from
diag::err_mmap_umbrella_clasherror in https://github.com/llvm/llvm-projectFor our case, the umbrella header name for both alice and bob modules are
foo, which is totally fine since their path is different. But for some reason when trying to compile the code in a remote execution service such as buildfarm, above error shows up. I cannot reproduce it withlocalorsandboxedmode.Now looking back, the theory is that when there is clash on umbrealla header name, it will use the module's version string/number to figure out which is which? I don't really have any other theory for now. I cannot find such reference to
VersionNumber/VersionStringthat is meaningful. Nor can I setup a test case here since it only break remote execution.This does not revert the original PR in anyway, I still get to implicitly import Foundation/UIKit with this change.
Tests
No change on testing since this flag was not tested anywhere for now. I can add this flag to some random testing targets in this PR though but tests so far are still all passing with this turned off.