martong added a comment.

Thank you for this patch!

Could you please provide a lit test that ignites the over and over parsing 
behaviour? I think you need to create two files and the second one should 
contain parser error(s).



================
Comment at: clang/include/clang/CrossTU/CrossTranslationUnit.h:257
     llvm::Optional<InvocationListTy> InvocationList;
+    index_error_code InvocationListParsingError = index_error_code::no_error;
   };
----------------
I think it would be more consistent to make this an `llvm::Error`. I.e. we 
would store the result of the whole `lazyInitInvocationList` in this member 
variable. And as such, a better name could be `PreviousResult`. And this could 
be an `Optional` to indicate that we have never called `lazyInitInvocationList` 
before.
This means we would check in `lazyInitInvocationList` whether the 
`PreviousResult` is set and if yes is it an error. And if both conditions are 
true then return with the stashed error.


================
Comment at: clang/lib/CrossTU/CrossTranslationUnit.cpp:670
     return llvm::Error::success();
+  else if (index_error_code::no_error != InvocationListParsingError)
+    return llvm::make_error<IndexError>(InvocationListParsingError);
----------------
The `if` here is superfluous because the condition must be `true` always, 
otherwise we would have an `InvocationList`, wouldn't we?

Or we can have this condition, but then the `if (InvocatioList)` is not needed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101763

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

Reply via email to