[PATCH] D41005: Reuse preamble even if an unsaved file does not exist

[Nikolai Kosjar via Phabricator via cfe-commits]

nik added inline comments.


================
Comment at: include/clang/Frontend/ASTUnit.h:196
+  /// \brief Counter indicating how often the preamble was build in total.
+  unsigned PreambleCounter;
+
----------------
ilya-biryukov wrote:
> nik wrote:
> > Any better name for this one? Otherwise I would suggest renaming PreambleRebuildCounter to PreambleRebuildCountdownCounter to make them more distinct.
> +1, names PreambleRebuildCounter and PreambleCounter are too similar.
> 
OK, will address in the next patch set / diff.


================
Comment at: lib/Frontend/PrecompiledPreamble.cpp:461
 
     std::map<llvm::sys::fs::UniqueID, PreambleFileHash>::iterator Overridden =
         OverriddenFiles.find(Status.getUniqueID());
----------------
ilya-biryukov wrote:
> Will anything fail if we remove the map from `UniqueID` to hashes of overriden files and the corresponding checks?
> 
> We should either document why having `UniqueID`-based checks is important or remove the code doing those checks.
> Will anything fail if we remove the map from UniqueID to hashes of overriden files and the corresponding checks?

Hmm, I would need to remove/replace it and run the tests.

I see these reasons for UniqueID:
* It's already there :)
* Normalization of file paths is not necessary.
* It potentially can deal with hard links, though I'm not sure whether this is real world use case at all.

> We should either document why having UniqueID-based checks is important or remove the code doing those checks.

Agree.



Repository:
  rC Clang

https://reviews.llvm.org/D41005