[cfe-commits] r143115 - in /cfe/trunk: include/clang/Frontend/ASTUnit.h lib/Frontend/ASTUnit.cpp test/Index/crash-recovery-code-complete.c test/Index/crash-recovery-reparse.c

Argyrios Kyrtzidis akyrtzi at gmail.com
Fri Oct 28 21:07:34 PDT 2011


On Oct 27, 2011, at 10:55 AM, Ted Kremenek wrote:

> Author: kremenek
> Date: Thu Oct 27 12:55:18 2011
> New Revision: 143115
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=143115&view=rev
> Log:
> Move ASTUnit's handling of temporary files and the preamble file into a lazily-created static DenseMap.  This DenseMap is cleared (and the files erased) via an atexit routine in the case an ASTUnit is not destroyed.  Fixes <rdar://problem/10293367>.

How about storing the preamble PCH in memory in the corresponding ASTUnit instead ?
There doesn't seem to be any benefits in creating a file for it, plus we save on the I/O and the worry of cleaning up behind us.

-Argyrios

> 
> Modified:
>    cfe/trunk/include/clang/Frontend/ASTUnit.h
>    cfe/trunk/lib/Frontend/ASTUnit.cpp
>    cfe/trunk/test/Index/crash-recovery-code-complete.c
>    cfe/trunk/test/Index/crash-recovery-reparse.c
> 
> Modified: cfe/trunk/include/clang/Frontend/ASTUnit.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Frontend/ASTUnit.h?rev=143115&r1=143114&r2=143115&view=diff
> ==============================================================================
> --- cfe/trunk/include/clang/Frontend/ASTUnit.h (original)
> +++ cfe/trunk/include/clang/Frontend/ASTUnit.h Thu Oct 27 12:55:18 2011
> @@ -147,10 +147,6 @@
>   /// the next.
>   unsigned NumStoredDiagnosticsFromDriver;
> 
> -  /// \brief Temporary files that should be removed when the ASTUnit is 
> -  /// destroyed.
> -  SmallVector<llvm::sys::Path, 4> TemporaryFiles;
> -  
>   /// \brief Counter that determines when we want to try building a
>   /// precompiled preamble.
>   ///
> @@ -161,10 +157,7 @@
>   /// building the precompiled preamble fails, we won't try again for
>   /// some number of calls.
>   unsigned PreambleRebuildCounter;
> -  
> -  /// \brief The file in which the precompiled preamble is stored.
> -  std::string PreambleFile;
> -  
> +
> public:
>   class PreambleData {
>     const FileEntry *File;
> @@ -468,9 +461,7 @@
>   /// \brief Add a temporary file that the ASTUnit depends on.
>   ///
>   /// This file will be erased when the ASTUnit is destroyed.
> -  void addTemporaryFile(const llvm::sys::Path &TempFile) {
> -    TemporaryFiles.push_back(TempFile);
> -  }
> +  void addTemporaryFile(const llvm::sys::Path &TempFile);
> 
>   bool getOnlyLocalDecls() const { return OnlyLocalDecls; }
> 
> 
> Modified: cfe/trunk/lib/Frontend/ASTUnit.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/ASTUnit.cpp?rev=143115&r1=143114&r2=143115&view=diff
> ==============================================================================
> --- cfe/trunk/lib/Frontend/ASTUnit.cpp (original)
> +++ cfe/trunk/lib/Frontend/ASTUnit.cpp Thu Oct 27 12:55:18 2011
> @@ -81,6 +81,102 @@
>       }
>     }
>   };
> +  
> +  struct OnDiskData {
> +    /// \brief The file in which the precompiled preamble is stored.
> +    std::string PreambleFile;
> +
> +    /// \brief Temporary files that should be removed when the ASTUnit is 
> +    /// destroyed.
> +    SmallVector<llvm::sys::Path, 4> TemporaryFiles;
> +    
> +    /// \brief Erase temporary files.
> +    void CleanTemporaryFiles();
> +
> +    /// \brief Erase the preamble file.
> +    void CleanPreambleFile();
> +
> +    /// \brief Erase temporary files and the preamble file.
> +    void Cleanup();
> +  };
> +}
> +
> +static void cleanupOnDiskMapAtExit(void);
> +
> +typedef llvm::DenseMap<const ASTUnit *, OnDiskData *> OnDiskDataMap;
> +static OnDiskDataMap &getOnDiskDataMap() {
> +  static OnDiskDataMap M;
> +  static bool hasRegisteredAtExit = false;
> +  if (!hasRegisteredAtExit) {
> +    hasRegisteredAtExit = true;
> +    atexit(cleanupOnDiskMapAtExit);
> +  }
> +  return M;
> +}
> +
> +static void cleanupOnDiskMapAtExit(void) {
> +  OnDiskDataMap &M = getOnDiskDataMap();
> +  for (OnDiskDataMap::iterator I = M.begin(), E = M.end(); I != E; ++I) {
> +    // We don't worry about freeing the memory associated with OnDiskDataMap.
> +    // All we care about is erasing stale files.
> +    I->second->Cleanup();
> +  }
> +}
> +
> +static OnDiskData &getOnDiskData(const ASTUnit *AU) {
> +  OnDiskDataMap &M = getOnDiskDataMap();
> +  OnDiskData *&D = M[AU];
> +  if (!D)
> +    D = new OnDiskData();
> +  return *D;
> +}
> +
> +static void erasePreambleFile(const ASTUnit *AU) {
> +  getOnDiskData(AU).CleanPreambleFile();
> +}
> +
> +static void removeOnDiskEntry(const ASTUnit *AU) {
> +  OnDiskDataMap &M = getOnDiskDataMap();
> +  OnDiskDataMap::iterator I = M.find(AU);
> +  if (I != M.end()) {
> +    I->second->Cleanup();
> +    delete I->second;
> +    M.erase(AU);
> +  }
> +}
> +
> +static void setPreambleFile(const ASTUnit *AU, llvm::StringRef preambleFile) {
> +  getOnDiskData(AU).PreambleFile = preambleFile;
> +}
> +
> +static const std::string &getPreambleFile(const ASTUnit *AU) {
> +  return getOnDiskData(AU).PreambleFile;  
> +}
> +
> +void OnDiskData::CleanTemporaryFiles() {
> +  for (unsigned I = 0, N = TemporaryFiles.size(); I != N; ++I)
> +    TemporaryFiles[I].eraseFromDisk();
> +  TemporaryFiles.clear(); 
> +}
> +
> +void OnDiskData::CleanPreambleFile() {
> +  if (!PreambleFile.empty()) {
> +    llvm::sys::Path(PreambleFile).eraseFromDisk();
> +    PreambleFile.clear();
> +  }
> +}
> +
> +void OnDiskData::Cleanup() {
> +  CleanTemporaryFiles();
> +  CleanPreambleFile();
> +}
> +
> +void ASTUnit::CleanTemporaryFiles() {
> +  getOnDiskData(this).CleanTemporaryFiles();
> +}
> +
> +void ASTUnit::addTemporaryFile(const llvm::sys::Path &TempFile) {
> +  getOnDiskData(this).TemporaryFiles.push_back(TempFile);
> }
> 
> /// \brief After failing to build a precompiled preamble (due to
> @@ -114,10 +210,9 @@
> }
> 
> ASTUnit::~ASTUnit() {
> -  CleanTemporaryFiles();
> -  if (!PreambleFile.empty())
> -    llvm::sys::Path(PreambleFile).eraseFromDisk();
> -  
> +  // Clean up the temporary files and the preamble file.
> +  removeOnDiskEntry(this);
> +
>   // Free the buffers associated with remapped files. We are required to
>   // perform this operation here because we explicitly request that the
>   // compiler instance *not* free these buffers for each invocation of the
> @@ -143,12 +238,6 @@
>   }    
> }
> 
> -void ASTUnit::CleanTemporaryFiles() {
> -  for (unsigned I = 0, N = TemporaryFiles.size(); I != N; ++I)
> -    TemporaryFiles[I].eraseFromDisk();
> -  TemporaryFiles.clear();
> -}
> -
> /// \brief Determine the set of code-completion contexts in which this 
> /// declaration should be shown.
> static unsigned getDeclShowContexts(NamedDecl *ND,
> @@ -939,7 +1028,7 @@
>     PreprocessorOpts.PrecompiledPreambleBytes.first = Preamble.size();
>     PreprocessorOpts.PrecompiledPreambleBytes.second
>                                                     = PreambleEndsAtStartOfLine;
> -    PreprocessorOpts.ImplicitPCHInclude = PreambleFile;
> +    PreprocessorOpts.ImplicitPCHInclude = getPreambleFile(this);
>     PreprocessorOpts.DisablePCHValidation = true;
> 
>     // The stored diagnostic has the old source manager in it; update
> @@ -971,7 +1060,7 @@
>     goto error;
> 
>   if (OverrideMainBuffer) {
> -    std::string ModName = PreambleFile;
> +    std::string ModName = getPreambleFile(this);
>     TranslateStoredDiagnostics(Clang->getModuleManager(), ModName,
>                                getSourceManager(), PreambleDiagnostics,
>                                StoredDiagnostics);
> @@ -1172,10 +1261,7 @@
>     // We couldn't find a preamble in the main source. Clear out the current
>     // preamble, if we have one. It's obviously no good any more.
>     Preamble.clear();
> -    if (!PreambleFile.empty()) {
> -      llvm::sys::Path(PreambleFile).eraseFromDisk();
> -      PreambleFile.clear();
> -    }
> +    erasePreambleFile(this);
> 
>     // The next time we actually see a preamble, precompile it.
>     PreambleRebuildCounter = 1;
> @@ -1281,7 +1367,7 @@
>     // We can't reuse the previously-computed preamble. Build a new one.
>     Preamble.clear();
>     PreambleDiagnostics.clear();
> -    llvm::sys::Path(PreambleFile).eraseFromDisk();
> +    erasePreambleFile(this);
>     PreambleRebuildCounter = 1;
>   } else if (!AllowRebuild) {
>     // We aren't allowed to rebuild the precompiled preamble; just
> @@ -1439,7 +1525,7 @@
>   StoredDiagnostics.erase(stored_diag_afterDriver_begin(), stored_diag_end());
> 
>   // Keep track of the preamble we precompiled.
> -  PreambleFile = FrontendOpts.OutputFile;
> +  setPreambleFile(this, FrontendOpts.OutputFile);
>   NumWarningsInPreamble = getDiagnostics().getNumWarnings();
> 
>   // Keep track of all of the files that the source manager knows about,
> @@ -1802,7 +1888,7 @@
>   // If we have a preamble file lying around, or if we might try to
>   // build a precompiled preamble, do so now.
>   llvm::MemoryBuffer *OverrideMainBuffer = 0;
> -  if (!PreambleFile.empty() || PreambleRebuildCounter > 0)
> +  if (!getPreambleFile(this).empty() || PreambleRebuildCounter > 0)
>     OverrideMainBuffer = getMainBufferWithPrecompiledPreamble(*Invocation);
> 
>   // Clear out the diagnostics state.
> @@ -2173,7 +2259,7 @@
>   // point is within the main file, after the end of the precompiled
>   // preamble.
>   llvm::MemoryBuffer *OverrideMainBuffer = 0;
> -  if (!PreambleFile.empty()) {
> +  if (!getPreambleFile(this).empty()) {
>     using llvm::sys::FileStatus;
>     llvm::sys::PathWithStatus CompleteFilePath(File);
>     llvm::sys::PathWithStatus MainPath(OriginalSourceFile);
> @@ -2197,7 +2283,7 @@
>     PreprocessorOpts.PrecompiledPreambleBytes.first = Preamble.size();
>     PreprocessorOpts.PrecompiledPreambleBytes.second
>                                                     = PreambleEndsAtStartOfLine;
> -    PreprocessorOpts.ImplicitPCHInclude = PreambleFile;
> +    PreprocessorOpts.ImplicitPCHInclude = getPreambleFile(this);
>     PreprocessorOpts.DisablePCHValidation = true;
> 
>     OwnedBuffers.push_back(OverrideMainBuffer);
> @@ -2214,7 +2300,7 @@
>   if (Act->BeginSourceFile(*Clang.get(), Clang->getFrontendOpts().Inputs[0].second,
>                            Clang->getFrontendOpts().Inputs[0].first)) {
>     if (OverrideMainBuffer) {
> -      std::string ModName = PreambleFile;
> +      std::string ModName = getPreambleFile(this);
>       TranslateStoredDiagnostics(Clang->getModuleManager(), ModName,
>                                  getSourceManager(), PreambleDiagnostics,
>                                  StoredDiagnostics);
> 
> Modified: cfe/trunk/test/Index/crash-recovery-code-complete.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/crash-recovery-code-complete.c?rev=143115&r1=143114&r2=143115&view=diff
> ==============================================================================
> --- cfe/trunk/test/Index/crash-recovery-code-complete.c (original)
> +++ cfe/trunk/test/Index/crash-recovery-code-complete.c Thu Oct 27 12:55:18 2011
> @@ -3,7 +3,7 @@
> // RUN:   "-remap-file=%s;%S/Inputs/crash-recovery-code-complete-remap.c" \
> // RUN:   %s 2> %t.err
> // RUN: FileCheck < %t.err -check-prefix=CHECK-CODE-COMPLETE-CRASH %s
> -// RUN: rm %t-preamble.pch
> +// RUN: test ! -e %t-preamble.pch
> // CHECK-CODE-COMPLETE-CRASH: Unable to perform code completion!
> //
> // REQUIRES: crash-recovery
> 
> Modified: cfe/trunk/test/Index/crash-recovery-reparse.c
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Index/crash-recovery-reparse.c?rev=143115&r1=143114&r2=143115&view=diff
> ==============================================================================
> --- cfe/trunk/test/Index/crash-recovery-reparse.c (original)
> +++ cfe/trunk/test/Index/crash-recovery-reparse.c Thu Oct 27 12:55:18 2011
> @@ -3,7 +3,7 @@
> // RUN:   -remap-file="%s;%S/Inputs/crash-recovery-reparse-remap.c" \
> // RUN:   %s 2> %t.err
> // RUN: FileCheck < %t.err -check-prefix=CHECK-REPARSE-SOURCE-CRASH %s
> -// RUN: rm %t-preamble.pch
> +// RUN: test ! -e $t-preamble.pch
> // CHECK-REPARSE-SOURCE-CRASH: Unable to reparse translation unit
> //
> // REQUIRES: crash-recovery
> 
> 
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits





More information about the cfe-commits mailing list