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

[Steven Wu via Phabricator via llvm-commits]

steven_wu added inline comments.


================
Comment at: llvm/include/llvm/module.modulemap:403
+    // Exclude this; it is a forwarding header to TargetParser.
+    exclude header "ADT/Triple.h"
   }
----------------
lenary wrote:
> steven_wu wrote:
> > lenary wrote:
> > > steven_wu wrote:
> > > > I am not sure if this exclude is necessary. It is ok for ADT to depend on TargetParser, correct?
> > > > 
> > > > We should either clean up all the include through ADT, or maybe just leave this. Same for all the TargetParser header in Support.
> > > I got build issues until I added this. The `llvm/ADT/Triple.h` file now just forwards to `llvm/TargetParser/Triple.h`, so without this, you get a circular dependency of LLVM_Support -> TargetParser -> LLVM_Support
> > But isn't Triple.h module dependency: ADT -> TargetParser? I don't see an edge goes back.
> > 
> > For the rest of the TargetParser headers, that could be a problem. What prevents us from just rewrite all the includes to use the new location and remove those forwarding headers? Just excluding here without including them in a different module still means they cannot be used in a module build.
> As I understand it, without the exclude line, if something outside ADT or Targetparser includes on "llvm/ADT/*" then it's classed as pulling in the ADT module. In the case of "llvm/ADT/Triple.h", this pulls in TargetParser (because it is a forwarding include to "llvm/TargetParser/Triple.h", which is classed as being in TargetParser, because it is in "llvm/TargetParser/*"). Then when TargetParser relies on something else in ADT, you get a circular dependency. 
> 
> This exclude line says "don't consider this header part of ADT" (because it isn't, any more). I guess I'm missing that I need to add the forwarding headers to the new TargetParser module, even though they're in the "wrong" directory. 
> 
> The forwarding headers are staying for at least LLVM 16, to make compatibility easier, especially for downstream users. I intend to rewrite the in-tree uses of the forwarding headers soon, but I'm sick right now and won't be back online until after the holiday period. 
I won't be worry too much about compatibility considering how many things are being rewritten now that almost nothing will be compatible~

As far as I care, as long as the module bots stay in good shape I am good. @aprantl what do you think?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140420