r269769 - [PCH] Fixed bug with preamble invalidation when overridden files change

[Nico Weber via cfe-commits]

Is it possible to write a test for this?

On Tue, May 17, 2016 at 10:34 AM, Cameron Desrochers via cfe-commits <
cfe-commits at lists.llvm.org> wrote:

> Author: cameron314
> Date: Tue May 17 09:34:53 2016
> New Revision: 269769
>
> URL: http://llvm.org/viewvc/llvm-project?rev=269769&view=rev
> Log:
> [PCH] Fixed bug with preamble invalidation when overridden files change
>
> When remapped files were changed, they would not always cause the
> preamble's PCH to be invalidated, because the remapped path didn't
> necessarily match the include path (e.g. slash direction -- this happens a
> lot on Windows). I fixed this by moving to a llvm::sys::fs::UniqueID-based
> map instead of comparing paths stringwise.
>
> Differential Revision: http://reviews.llvm.org/D20137
>
> Modified:
>     cfe/trunk/lib/Frontend/ASTUnit.cpp
>
> Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
> URL:
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=269769&r1=269768&r2=269769&view=diff
>
> ==============================================================================
> --- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
> +++ cfe/trunk/lib/Frontend/ASTUnit.cpp Tue May 17 09:34:53 2016
> @@ -1378,7 +1378,7 @@ ASTUnit::getMainBufferWithPrecompiledPre
>
>        // First, make a record of those files that have been overridden via
>        // remapping or unsaved_files.
> -      llvm::StringMap<PreambleFileHash> OverriddenFiles;
> +      std::map<llvm::sys::fs::UniqueID, PreambleFileHash> OverriddenFiles;
>        for (const auto &R : PreprocessorOpts.RemappedFiles) {
>          if (AnyFileChanged)
>            break;
> @@ -1391,24 +1391,38 @@ ASTUnit::getMainBufferWithPrecompiledPre
>            break;
>          }
>
> -        OverriddenFiles[R.first] = PreambleFileHash::createForFile(
> +        OverriddenFiles[Status.getUniqueID()] =
> PreambleFileHash::createForFile(
>              Status.getSize(),
> Status.getLastModificationTime().toEpochTime());
>        }
>
>        for (const auto &RB : PreprocessorOpts.RemappedFileBuffers) {
>          if (AnyFileChanged)
>            break;
> -        OverriddenFiles[RB.first] =
> +
> +        vfs::Status Status;
> +        if (FileMgr->getNoncachedStatValue(RB.first, Status)) {
> +          AnyFileChanged = true;
> +          break;
> +        }
> +
> +        OverriddenFiles[Status.getUniqueID()] =
>              PreambleFileHash::createForMemoryBuffer(RB.second);
>        }
>
>        // Check whether anything has changed.
> -      for (llvm::StringMap<PreambleFileHash>::iterator
> +      for (llvm::StringMap<PreambleFileHash>::iterator
>               F = FilesInPreamble.begin(), FEnd = FilesInPreamble.end();
>             !AnyFileChanged && F != FEnd;
>             ++F) {
> -        llvm::StringMap<PreambleFileHash>::iterator Overridden
> -          = OverriddenFiles.find(F->first());
> +        vfs::Status Status;
> +        if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
> +          // If we can't stat the file, assume that something horrible
> happened.
> +          AnyFileChanged = true;
> +          break;
> +        }
> +
> +        std::map<llvm::sys::fs::UniqueID, PreambleFileHash>::iterator
> Overridden
> +          = OverriddenFiles.find(Status.getUniqueID());
>          if (Overridden != OverriddenFiles.end()) {
>            // This file was remapped; check whether the newly-mapped file
>            // matches up with the previous mapping.
> @@ -1418,13 +1432,9 @@ ASTUnit::getMainBufferWithPrecompiledPre
>          }
>
>          // The file was not remapped; check whether it has changed on
> disk.
> -        vfs::Status Status;
> -        if (FileMgr->getNoncachedStatValue(F->first(), Status)) {
> -          // If we can't stat the file, assume that something horrible
> happened.
> -          AnyFileChanged = true;
> -        } else if (Status.getSize() != uint64_t(F->second.Size) ||
> -                   Status.getLastModificationTime().toEpochTime() !=
> -                       uint64_t(F->second.ModTime))
> +        if (Status.getSize() != uint64_t(F->second.Size) ||
> +            Status.getLastModificationTime().toEpochTime() !=
> +                uint64_t(F->second.ModTime))
>            AnyFileChanged = true;
>        }
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20160517/354e4599/attachment.html>