[PATCH] D70183: Detect source location overflow due includes

[Diogo N. Sampaio via Phabricator via cfe-commits]

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