[PATCH] D104918: [clang-repl] Implement partial translation units and error recovery.

[Stefan Gränitz via Phabricator via cfe-commits]

sgraenitz added a comment.

A few minor notes



================
Comment at: clang/include/clang/Interpreter/Interpreter.h:63
+    if (auto Err = ErrOrPTU.takeError())
       return Err;
+    if (ErrOrPTU->TheModule)
----------------
`ErrorOr<T>` has different semantics and is still in use, so the name could be confusing. Idiomatic usage for `Expected<T>` would rather be like:
```
auto PTU = Parse(Code);
if (!PTU)
  return PTU.takeError();
```

The patch didn't introduce it, but it might be a good chance for improvement.


================
Comment at: clang/lib/Interpreter/IncrementalParser.cpp:178
+    TranslationUnitDecl *PreviousTU = MostRecentTU->getPreviousDecl();
+    assert(PreviousTU);
+    TranslationUnitDecl *FirstTU = MostRecentTU->getFirstDecl();
----------------
Where does it point, if the very first TU fails? Maybe worth noting in the assert and/or adding a test case?


================
Comment at: clang/lib/Interpreter/IncrementalParser.cpp:263
+  if (auto Err = ErrOrPTU.takeError())
     return std::move(Err);
 
----------------
```
auto PTU = ParseOrWrapTopLevelDecl();
if (!PTU)
  return PTU.takeError();
```


Repository:
  rC Clang

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

https://reviews.llvm.org/D104918