[PATCH] D50785: [clangd][tests] Add exit(EXIT_FAILURE) in case of JSON parsing failure in TestMode

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 16 01:37:49 PDT 2018


ilya-biryukov added a comment.

+1 to having a separate option for that. More generally, wouldn't we want to exit on more kinds errors in the future?
JSON parsing errors is something that popped up recently, but the option only for that looks too narrow. We might end up wanting more options like that in the future, e.g. `-exit-on-json-dispatch-error`, `-exit-on-parse-error`, ...

How about a more generic option that would force clangd to have non-zero error code whenever even a single error was emitted via `elog()`? 
I've checked the output of `./bin/llvm-lit -avv ../tools/clang/tools/extra/test/clangd/ | grep '\(^E\|PASS\)'` and there seems to be a few things that emit errors spuriously and should be fixed for that to work:

1. `E[10:19:11.985] Failed to get status for RootPath clangd: No such file or directory`.

Could be fixed by specifying a valid rootPath in all clangd tests.

2. `E[10:19:12.013] Could not build a preamble for file /clangd-test/main.cpp`.

IIUC, this happens on empty preambles, the fix is to not emit an error in that case. In general, whenever preamble is not built, this is probably due to changes in some headers and that case should be considered valid input for clangd, so arguably this shouldn't be an error in the first place.-

3. Some tests actually test for invalid inputs, those should probably expect an error code from clangd that runs into those errors.

That doesn't look like a long list, so it shouldn't be too hard. What are your thoughts on a more generic option like that?



================
Comment at: JSONRPCDispatcher.cpp:366
+        if (TestMode)
+          exit(EXIT_FAILURE);
       }
----------------
Alternative would be to run input parsing to completion, propagate if any error was encountered to main and exit with a predefined error code in that case.
Exiting prematurely might hide some important errors that we would otherwise catch even before seeing an error code from clangd, e.g. infinite loops in the input handling on invalid inputs, etc.



Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D50785





More information about the cfe-commits mailing list