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

[Sam Elliott via Phabricator via llvm-commits]

lenary 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"
   }
----------------
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. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140420