ilya-biryukov wrote: > To better understand the motivation, I have two questions: > > * Why do you give Clang module maps describing modules that are already > described in PCM files? Unless the module map describes some other top-level > module that's yet to be built, the information it provides is redundant. Is > this impossible to detect in your build system?
It's possible, but much more complicated to do at build system level and it would be much easier to solve this in the compiler. I want to be very clear that we also commit to support this flag while we are using it, and to remove it when we stop doing so. So if it causes any issues, we're there to help and take on the maintenance burden. The build system is Bazel, the code is available upstream too, btw. Although the specific code that runs modules is not enabled by default, and, AFAIK, Google internally is the only user of that. Definitely the only user that cares about the scale that causes these issues. > * Why do you need to wait for 64-bit source locations to be the default > upstream? Is the concern that it's not a well-tested configuration right now > and you don't want to shoulder the burden of qualifying that mode? The biggest concern that we have for 64bit source location is performance costs. We expect some of our compile actions to time out or run out of memory after the change (because the added overhead is significant), so we would like to budget for it and make sure we get a chance to test it in advance of the upstream defaults actually changing. All of that takes time, so this change is intended to give us a way out the corner that we're in until 64 bit source locations are in. > This patch looks good to me, but I think it'd be worth pursuing making this > the default behavior, i.e. investigating the test failures and at least > understanding what exactly is going on. I can take another look, but my vague memory from the investigation I made last time I checked, is that there were some module map shadowing semantics that we could not support unless we read the module map before loading the PCM. We do not use that internally and rely on the fact that each module name is uniquely identified by its module map, hence we can use the flag without giving it too much thought. I will need to refresh my memory, but I believe the conclusion I had at the time was that this shadowing is fundamentally incompatible with loading the module maps later than PCMs, because it affects which PCMs need to loaded. (The conclusions are a little rushed, I may misremember, will take another look next week). That was another reason why I felt that having this as a `-cc1` flag is the only reasonable option, otherwise the interface just gets way too complicated. https://github.com/llvm/llvm-project/pull/88893 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
