[PATCH] D96033: [clang-repl] Land initial infrastructure for incremental parsing

[Vassil Vassilev via Phabricator via cfe-commits]

v.g.vassilev added inline comments.


================
Comment at: clang/lib/Interpreter/Interpreter.cpp:93
+                                   "Initialization failed. "
+                                   "Unable to create diagnostics engine");
+
----------------
sgraenitz wrote:
> It looks like `clang-repl` always dumps errors to stdout currently. This is fine for the interactive use case, but it will be impractical for input/output tests. As a result unit tests e.g. dump:
> ```
> Note: Google Test filter = InterpreterTest.Errors
> [==========] Running 1 test from 1 test case.
> [----------] Global test environment set-up.
> [----------] 1 test from InterpreterTest
> [ RUN      ] InterpreterTest.Errors
> In file included from <built-in>:0:
> input_line_0:1:1: error: unknown type name 'intentional_error'
> intentional_error v1 = 42; 
> ^
> [       OK ] InterpreterTest.Errors (9024 ms)
> [----------] 1 test from InterpreterTest (9024 ms total)
> 
> [----------] Global test environment tear-down
> [==========] 1 test from 1 test case ran. (9025 ms total)
> [  PASSED  ] 1 test.
> ```
> 
> It would be useful to have an option for streaming diagnostics to an in-memory buffer (and maybe append them to returned llvm::Error instances in the future). Instead of `createDiagnostics()` you could pass a TextDiagnosticPrinter via `setDiagnostics()` here to accomplish it.
> 
> Not insisting on having it in this review, but it would be a good follow-up task at least.
I should have addressed it now I get:

```
[==========] Running 5 tests from 2 test cases.
[----------] Global test environment set-up.
[----------] 1 test from IncrementalProcessing
[ RUN      ] IncrementalProcessing.EmitCXXGlobalInitFunc
[       OK ] IncrementalProcessing.EmitCXXGlobalInitFunc (17 ms)
[----------] 1 test from IncrementalProcessing (17 ms total)

[----------] 4 tests from InterpreterTest
[ RUN      ] InterpreterTest.Sanity
[       OK ] InterpreterTest.Sanity (8 ms)
[ RUN      ] InterpreterTest.IncrementalInputTopLevelDecls
[       OK ] InterpreterTest.IncrementalInputTopLevelDecls (9 ms)
[ RUN      ] InterpreterTest.Errors
[       OK ] InterpreterTest.Errors (91 ms)
[ RUN      ] InterpreterTest.DeclsAndStatements
[       OK ] InterpreterTest.DeclsAndStatements (8 ms)
[----------] 4 tests from InterpreterTest (116 ms total)

[----------] Global test environment tear-down
[==========] 5 tests from 2 test cases ran. (133 ms total)
[  PASSED  ] 5 tests.
```


================
Comment at: clang/test/Interpreter/execute.c:9
+
+struct S { float f = 1.0; S *m = nullptr;} s;
+auto r2 = printf("S[f=%f, m=%p]\n", s.f, s.m);
----------------
sgraenitz wrote:
> *nit* this should be a cpp file right? Otherwise, you should write `struct S *m = nullptr;` here. Also, C doesn't understand the `extern "C"` above :)
Thanks! It is C++ indeed and I have changed the file extension.


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

https://reviews.llvm.org/D96033