agozillon marked an inline comment as done. agozillon added inline comments.
================ Comment at: flang/lib/Frontend/CompilerInvocation.cpp:730 + res.getLangOpts().OMPHostIRFile = arg->getValue(); + if (!llvm::sys::fs::exists(res.getLangOpts().OMPHostIRFile)) + diags.Report(clang::diag::err_drv_omp_host_ir_file_not_found) ---------------- awarzynski wrote: > agozillon wrote: > > awarzynski wrote: > > > I think that this is expecting a bit too much from `CompilerInvocation`. > > > Instead, whatever is going to use this file should complain if the file > > > does not exist (`CompilerInvocation` is merely a messenger here, and file > > > I/O can be quite expensive, hence my reservations). > > > > > > Is it possible to create a test for "invalid file path" wherever this > > > becomes relevant? > > I believe the follow up patch I have here which uses the file, emits an > > error if it can't find it: https://reviews.llvm.org/D148370 > > However, in this case it's an assert rather than more useful Diagnostics > > unfortunately. > > > > Although, this segment of code does currently just mimic what Clang does in > > it's own > > `CompilerInvocation`: > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/CompilerInvocation.cpp#L3883 > > > > > > So it depends if we wish to try to maintain the status quo for the shared > > argument across the compiler frontends! > > > > So whichever you prefer, I am happy to remove the check at this level and > > let the lowering handle the problem if it arises :-) but I do like sharing > > the commonality with Clang where possible. > Consistency with Clang is a good idea 👍🏻 Though I would appreciate a test > that demonstrates that this diagnostic is indeed issues when a non-existing > files is specified :) Thank you for pointing out a test was required! I've added one now :-) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D148038/new/ https://reviews.llvm.org/D148038 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
