dnsampaio marked an inline comment as done.
dnsampaio added inline comments.


================
Comment at: clang/lib/Basic/SourceManager.cpp:587
+    Diag.Report(IncludePos, diag::err_include_too_large);
+    exit(1);
+  }
----------------
miyuki wrote:
> dnsampaio wrote:
> > miyuki wrote:
> > > dnsampaio wrote:
> > > > For debug builds, I could not find any other way to not reach an assert 
> > > > failure other than exiting here. Perhaps there is a more llvm specific 
> > > > way to die? llvm_unreachable() ?
> > > There is a similar error `err_file_too_large`. How is it handled? 
> > > `llvm_unreachable` is intended for code that is unreachable: it causes an 
> > > assertion failure in debug builds and undefined behavior in release 
> > > builds.
> > The `err_file_too_large` above in this file allows the function to finish 
> > and return further on. Here, we assert false before the message being 
> > printed.
> > If `llvm_unreachable` is undefined behavior, then I should stick to exit(1) 
> > ?
> I think you should return an invalid file ID (i.e. `return FileID();` and 
> propagate the error through the call stack. Clang can be used as a library, 
> so you can't just call exit(), this would terminate the user's program.
It will still reach an false assert in builds that enable them. But in release 
it linger on and ends with the correct warning.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70183



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

Reply via email to