[PATCH] D120874: [C++20] [Modules] Use '-' as the separator of partitions when searching in filesystems

[Alexandre Rames via Phabricator via cfe-commits]

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