v.g.vassilev added inline comments.
================
Comment at: clang/include/clang/Basic/LangOptions.h:690-691
+
+ /// The translation unit is a partial translation unit, growing
incrementally.
+ TU_Partial
};
----------------
rsmith wrote:
> I don't think this is clear enough about the difference between `TU_Prefix`
> and `TU_Partial`. I think the difference is:
>
> * `TU_Prefix` is an incomplete prefix of a translation unit. Because it's not
> complete, we don't perform (most) finalization steps (eg, template
> instantiation) at the end.
> * `TU_Partial` is a complete translation unit that we might nonetheless
> incrementally extend later. Because it is complete (and we might want to
> generate code for it), we do perform template instantiation, but because it
> might be extended later, we don't warn if it declares or uses undefined
> internal-linkage symbols.
>
> I wonder if `TU_Incremental` would be a better name than `TU_Partial`.
Good point. Fixed.
================
Comment at: clang/include/clang/Interpreter/Interpreter.h:63
+ if (auto Err = ErrOrPTU.takeError())
return Err;
+ if (ErrOrPTU->TheModule)
----------------
sgraenitz wrote:
> `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.
Ha, that made the code much nicer looking! Thanks!!
================
Comment at: clang/lib/Interpreter/IncrementalParser.cpp:178
+ TranslationUnitDecl *PreviousTU = MostRecentTU->getPreviousDecl();
+ assert(PreviousTU);
+ TranslationUnitDecl *FirstTU = MostRecentTU->getFirstDecl();
----------------
sgraenitz wrote:
> Where does it point, if the very first TU fails? Maybe worth noting in the
> assert and/or adding a test case?
The very first TU contains the compiler builtins and it is created when the
ASTContext is created. Not having a PreviousTU would mean that we did not
initialize the CompilerInstance properly.
Added some description in the assert.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D104918/new/
https://reviews.llvm.org/D104918
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
