dexonsmith added a subscriber: sammccall.
dexonsmith added inline comments.


================
Comment at: clang/include/clang/Frontend/CompilerInstance.h:738-740
+  createOutputFileImpl(StringRef OutputPath, llvm::sys::fs::OpenFlags Flags,
                        bool RemoveFileOnSignal, bool UseTemporary,
                        bool CreateMissingDirectories);
----------------
abhina.sreeskantharajan wrote:
> rnk wrote:
> > I think this is only going to be worth it if we can roll up all of these 
> > booleans into a new flags enum for compiler instance. It also prevents 
> > introducing a new use of FileSystem.h, which is an expensive header to 
> > include.
> Sorry, I don't think I completely understand your suggestion. Are you 
> proposing that we create a new enum just for CompilerInstance.h for the other 
> booleans like RemoveFileOnSignal, UseTemporary, CreateMissingDirectories? And 
> this new enum also contain text/binary flags so we don't need to use the enum 
> in FileSystem.h? 
> 
> For background, I need this specific change to set some more text files with 
> OF_Text because my old commit got reverted since it caused CRLF issues on 
> Windows https://reviews.llvm.org/D96363.
> 
Yes, I think that's what @rnk is suggesting, maybe along the lines of the 
OutputConfigFlag in the (languishing, I need to get back to it) 
OutputBackend/OutputManager proposal I have (see, e.g., 
https://reviews.llvm.org/D95501, which needs to be split up), or maybe as a 
struct with named members as @sammccall suggests there.

> It also prevents introducing a new use of FileSystem.h, which is an expensive 
> header to include.
Another option that'd be more isolated would be to split the OpenFlag enum 
(and/or its friends) out to another header in 
llvm/include/llvm/Support/FileSystem/ (say, .../FileSystem/FileSystemEnums.h), 
like was done for UniqueID.h.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D100872

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to