awarzynski 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) ---------------- 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 :) 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
