arames added a comment. I was was asked to chime in to assess whether this patch could be a problem for the prebuilt-implicit clang modules workflow. No problem here. With prebuilt modules, the output file name has to be specified manually. So the mapping does not change the existing requirement of managing the file names to avoid collissions. Even in the future with implicit modules - as noted by others - `:` and `-` are not allowed in the clang module name in the `.modulemap`, so there cannot be any conflict.
A couple comments on the way. Take them with a grain of salt, since I don't much about how this is going to be used or what stage of the work this is. I see the mapping is only applied on the "prebuilt" code path, and not the "implicit" and "prebuilt implicit" code paths. I suppose at some point, module generation will be implicit. So any particular handling of `ModuleName` to be appended to the filesystem path will need to be done on different code paths. So I would already abstract this away in a helper. Without much of an informed opinion, I find @iains makes a valid point. Does this kind of mapping (semantic module name to name being used for filesystem search/storage) belong at a higher level ? Maybe that's to be answered or worked on later as well. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D120874/new/ https://reviews.llvm.org/D120874 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
