teemperor added a comment. In D105168#3071152 <https://reviews.llvm.org/D105168#3071152>, @craig.topper wrote:
> In D105168#3069499 <https://reviews.llvm.org/D105168#3069499>, @teemperor > wrote: > >> This broke the modules build: >> >> While building module 'LLVM_Utils' imported from >> lvm/lib/Support/TargetParser.cpp:14: >> In file included from <module-includes>:209: >> llvm/include/llvm/Support/RISCVISAInfo.h:15:10: fatal error: could not >> build module 'LLVM_Option' >> #include "llvm/Option/ArgList.h" >> ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~ >> llvm/lib/Support/TargetParser.cpp:14:10: fatal error: could not build >> module 'LLVM_Utils' >> #include "llvm/Support/TargetParser.h" >> ~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >> >> `Support` headers can't include `Option` headers (as `Option` depends on >> `Support` already, so that would be a cyclic dependency). I believe folks >> here are in the US time zone, so I went ahead and replaced the header >> include with a forward decl to unbreak the bots (see >> de4d2f80b75e2a1e4b0ac5c25e20f20839633688 >> <https://reviews.llvm.org/rGde4d2f80b75e2a1e4b0ac5c25e20f20839633688> ) >> >> FWIW, I think there is a better place than `Support` for this new class. >> `Support` is a dependency of nearly every single LLVM component, but this >> class seems to be used by a handful of files. Could we maybe move this to >> some other/new part of LLVM (and make the few components that need it depend >> on that)? > > Thanks for the header fix. I think we also need to fix the library circular > dependency that still exists. The Support library should not use anything > from the Option library. Maybe we can partition the function some way that > moves the MakeArgStr back into the clang driver code. > > I don't know if we have a better library for this new class. I think Support > is the only common library between the clang driver and LLVM's MC layer. I > supposed we could create a new library, but that might be a hard sell for one > file. IIRC there are some other target-specific classes in Support for the same reason. Maybe we can make something such as `TargetSupport` or `SharedTarget`? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D105168/new/ https://reviews.llvm.org/D105168 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
