r209922 - Invalidate the file system cache entries for files that may rebuild

[Argyrios Kyrtzidis]

LGTM!

> On May 30, 2014, at 2:20 PM, Ben Langmuir <blangmuir at apple.com> wrote:
> 
> Author: benlangmuir
> Date: Fri May 30 16:20:54 2014
> New Revision: 209922
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=209922&view=rev
> Log:
> Invalidate the file system cache entries for files that may rebuild
> 
> This reapplies r209910 with a fix for the assertion failures hit on the
> buildbots.
> 
> original commit message:
> I thought we could get away without this, but it means that the
> FileEntry objects actually refer to the wrong files, since pcms are not
> updated inplace, they are atomically renamed into place after compiling
> a module.
> 
> So we are close to the original behaviour of invalidating the cache for
> all modules being removed, but now we should only invalidate the ones
> that depend on whichever module failed to load.
> 
> Unfortunately I haven't come up with a new test that didn't require
> a race between parallel invocations of clang.
> 
> <rdar://problem/17038180>
> 
> Modified:
>    cfe/trunk/lib/Serialization/ModuleManager.cpp
> 
> Modified: cfe/trunk/lib/Serialization/ModuleManager.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ModuleManager.cpp?rev=209922&r1=209921&r2=209922&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Serialization/ModuleManager.cpp (original)
> +++ cfe/trunk/lib/Serialization/ModuleManager.cpp Fri May 30 16:20:54 2014
> @@ -135,19 +135,31 @@ ModuleManager::addModule(StringRef FileN
>   return NewModule? NewlyLoaded : AlreadyLoaded;
> }
> 
> +static void getModuleFileAncestors(
> +    ModuleFile *F,
> +    llvm::SmallPtrSetImpl<ModuleFile *> &Ancestors) {
> +  Ancestors.insert(F);
> +  for (ModuleFile *Importer : F->ImportedBy)
> +    getModuleFileAncestors(Importer, Ancestors);
> +}
> +
> void ModuleManager::removeModules(ModuleIterator first, ModuleIterator last,
>                                   ModuleMap *modMap) {
>   if (first == last)
>     return;
> 
> -  // The first file entry is about to be rebuilt (or there was an error), so
> -  // there should be no references to it. Remove it from the cache to close it,
> -  // as Windows doesn't seem to allow renaming over an open file.
> -  FileMgr.invalidateCache((*first)->File);
> -
>   // Collect the set of module file pointers that we'll be removing.
>   llvm::SmallPtrSet<ModuleFile *, 4> victimSet(first, last);
> 
> +  // The last module file caused the load failure, so it and its ancestors in
> +  // the module dependency tree will be rebuilt (or there was an error), so
> +  // there should be no references to them. Collect the files to remove from
> +  // the cache below, since rebuilding them will create new files at the old
> +  // locations.
> +  llvm::SmallPtrSet<ModuleFile *, 4> Ancestors;
> +  getModuleFileAncestors(*(last-1), Ancestors);
> +  assert(Ancestors.count(*first) && "non-dependent module loaded");
> +
>   // Remove any references to the now-destroyed modules.
>   for (unsigned i = 0, n = Chain.size(); i != n; ++i) {
>     Chain[i]->ImportedBy.remove_if([&](ModuleFile *MF) {
> @@ -165,6 +177,10 @@ void ModuleManager::removeModules(Module
>         mod->setASTFile(nullptr);
>       }
>     }
> +
> +    if (Ancestors.count(*victim))
> +      FileMgr.invalidateCache((*victim)->File);
> +
>     delete *victim;
>   }
> 
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits