[PATCH] D34512: Add preliminary Cross Translation Unit support library

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added inline comments.


================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:35
+namespace {
+// FIXME: This class is will be removed after the transition to llvm::Error.
+class IndexErrorCategory : public std::error_category {
----------------
dcoughlin wrote:
> Is this FIXME still relevant? What will need to be transitioned to llvm::Error for it to be removed? Can you make the comment a bit more specific so that future maintainers will know when/how to address the FIXME?
Unfortunately, it is. IndexError needs to implement `convertToErrorCode` because it is pure virtual in the base class, and that requires the existence of this class. So this can only be removed, once `convertToErrorCode` is pruned globally from the codebase.  I think all of such classes are marked with similar FIXMEs. 


================
Comment at: lib/CrossTU/CrossTranslationUnit.cpp:194
+        handleAllErrors(IndexOrErr.takeError(), [&](const IndexError &IE) {
+          (bool)PropagatedErr;
+          PropagatedErr =
----------------
dcoughlin wrote:
> What is this cast for? Is it to suppress an analyzer dead store false positive? I thought we got all those!
The point was to avoid an assertion fail where an error was not checked. The reason, there is no really good way to propagate errors right now. But with this diagnostic emission moved to the client this is no longer required. 


https://reviews.llvm.org/D34512