[PATCH] D140420: [Support] Update modulemap for TargetParser

Thu Jan 5 12:37:34 PST 2023

dblaikie added a comment.

In D140420#4027284 <https://reviews.llvm.org/D140420#4027284>, @lenary wrote:

> In D140420#4027204 <https://reviews.llvm.org/D140420#4027204>, @dblaikie wrote:
>
>> In D140420#4025376 <https://reviews.llvm.org/D140420#4025376>, @lenary wrote:
>>
>>> I noticed some duplicate lines in my previous update, corrected now.
>>>
>>> Adding @dblaikie as a reviewer as he also reported problems with the module build.
>>
>> Thanks! The modulemaps aren't an issue for us (Google) - we use the Bazel build files and explicit modules builds that don't use modulemaps.
>>
>> I'm looking at the BUILD files and I'm not entirely sure how they work in the face of this issue... but they seem to be working/seems we don't have an issue anymore...
>
> Sorry for the confusion about modules builds and modulemaps. I love that we have the same words for several different things, and so many separate build configurations.
>
> I think MaskRay fixed the Bazel build files, by making TargetParser just another part of the Support component (in rGbb22e3e8c006debb37efc04fbacb863290ec3484 <https://reviews.llvm.org/rGbb22e3e8c006debb37efc04fbacb863290ec3484>, though there is also now a `:TargetParser` component since rGc2a5f156d2977ff471934ebe6f143bc569fc4bc9 <https://reviews.llvm.org/rGc2a5f156d2977ff471934ebe6f143bc569fc4bc9> , so the CPP files are part of two components!). It would be good to do the equivalent of this patch there too, but I don't know how bazel modules cope with forwarding headers. I'm not untangling the bazel configuration, I am not an expert in it.

yeah, sorry, I just meant "seems we've got something working for the Bazel build files, which was mostly my concern, so I don't have opinions on the modulemaps".

I think the layering is important in general, but understand that forwarding header for backcompat during migration is useful - & probably the simplest fix is to classify the forwarding header as part of the TargetParser library, despite being in the Support directory, which I guess is what you're doing here - so seems like a reasonable compromise.

Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D140420/new/

https://reviews.llvm.org/D140420