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

[Bruno Cardoso Lopes via cfe-commits]

Ping on the testcase :-)

On Tue, May 17, 2016 at 9:36 AM, Nico Weber via cfe-commits
<cfe-commits at lists.llvm.org> wrote:
> 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
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
>



-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc