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



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to