[PATCH] D58749: [index-while-building] IndexRecordHasher

[Jan Korous via Phabricator via cfe-commits]

jkorous marked 8 inline comments as done.
jkorous added a comment.

Addressed some comments, going to update the diff.



================
Comment at: clang/lib/Index/IndexRecordHasher.cpp:291
+
+hash_code IndexRecordHasher::hashImpl(const Decl *D) {
+  return DeclHashVisitor(*this).Visit(D);
----------------
gribozavr wrote:
> hashImpl => cachedHashImpl
Honestly, I am not sure this would be better. I added a comment about this set of methods in the header file. Wouldn't mind renaming them but think `hashImpl` is actually quite accurate.


================
Comment at: clang/lib/Index/IndexRecordHasher.cpp:409
+
+  // Unhandled type.
+  return Hash;
----------------
gribozavr wrote:
> So what is the failure mode for unhandled types, what is the effect on the whole system?
Seems like just the `InitialHash` is returned at the moment.

I guess using llvm::Optional<hash_code> as a return type would be better. WDYT?


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

https://reviews.llvm.org/D58749