thakis added inline comments.
================ Comment at: clang/utils/TableGen/SveEmitter.cpp:32 #include "llvm/TableGen/Error.h" +#include "clang/Basic/AArch64SVETypeFlags.h" #include <string> ---------------- sdesmalen wrote: > thakis wrote: > > Including stuff from `clang/Basic` in clang/utils/TableGen is conceptually > > a layering violation: clang-tblgen is used to generate headers included in > > clang/Basic. In this case it happens to work, but it's because you're > > lucky, and it could break for subtle reasons if the TypeFlags header starts > > including some other header in Basic that happens to include something > > generated. > > > > Please restructure this so that the TableGen code doesn't need an include > > from Basic. > Thanks for pointing out! The only directory that seems to have common > includes between Clang TableGen/CodeGen is the llvm/Support directory, any > objection to me moving the header there? That seems like a strange place for this header. Maybe you can rework things so that you don't have to share a header between clang's tablegen and clang's Basic? No other tablegen output so far has needed that. (see e.g. the ` /// These must be kept in sync with the flags in utils/TableGen/NeonEmitter.h.` line in TargetBuiltins.h). If that isn't possible at all, I suppose you could put the .h file in clang/utils/TableGen and also make clang-tblgen write the .h file and use the written .h file in Basic. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D75470/new/ https://reviews.llvm.org/D75470 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
