[cfe-commits] r98690 - in /cfe/trunk: include/clang/Basic/SourceManager.h lib/Basic/SourceManager.cpp test/PCH/changed-files.c

Douglas Gregor dgregor at apple.com
Tue Mar 16 15:53:51 PDT 2010


Author: dgregor
Date: Tue Mar 16 17:53:51 2010
New Revision: 98690

URL: http://llvm.org/viewvc/llvm-project?rev=98690&view=rev
Log:
Teach SourceManager's content cache to keep track of whether its
buffer was invalid when it was created, and use that bit to always set
the "Invalid" flag according to whether the buffer is invalid. This
ensures that all accesses to an invalid buffer are marked invalid,
improving recovery.

Modified:
    cfe/trunk/include/clang/Basic/SourceManager.h
    cfe/trunk/lib/Basic/SourceManager.cpp
    cfe/trunk/test/PCH/changed-files.c

Modified: cfe/trunk/include/clang/Basic/SourceManager.h
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/SourceManager.h?rev=98690&r1=98689&r2=98690&view=diff
==============================================================================
--- cfe/trunk/include/clang/Basic/SourceManager.h (original)
+++ cfe/trunk/include/clang/Basic/SourceManager.h Tue Mar 16 17:53:51 2010
@@ -17,6 +17,7 @@
 #include "clang/Basic/SourceLocation.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/System/DataTypes.h"
+#include "llvm/ADT/PointerIntPair.h"
 #include "llvm/ADT/PointerUnion.h"
 #include "llvm/ADT/DenseMap.h"
 #include <vector>
@@ -54,7 +55,8 @@
   class ContentCache {
     /// Buffer - The actual buffer containing the characters from the input
     /// file.  This is owned by the ContentCache object.
-    mutable const llvm::MemoryBuffer *Buffer;
+    /// The bit indicates whether the buffer is invalid.
+    mutable llvm::PointerIntPair<const llvm::MemoryBuffer *, 1, bool> Buffer;
 
   public:
     /// Reference to the file entry.  This reference does not own
@@ -92,8 +94,9 @@
     unsigned getSizeBytesMapped() const;
 
     void setBuffer(const llvm::MemoryBuffer *B) {
-      assert(!Buffer && "MemoryBuffer already set.");
-      Buffer = B;
+      assert(!Buffer.getPointer() && "MemoryBuffer already set.");
+      Buffer.setPointer(B);
+      Buffer.setInt(false);
     }
 
     /// \brief Replace the existing buffer (which will be deleted)
@@ -101,17 +104,19 @@
     void replaceBuffer(const llvm::MemoryBuffer *B);
 
     ContentCache(const FileEntry *Ent = 0)
-      : Buffer(0), Entry(Ent), SourceLineCache(0), NumLines(0) {}
+      : Buffer(0, false), Entry(Ent), SourceLineCache(0), NumLines(0) {}
 
     ~ContentCache();
 
     /// The copy ctor does not allow copies where source object has either
     ///  a non-NULL Buffer or SourceLineCache.  Ownership of allocated memory
     ///  is not transfered, so this is a logical error.
-    ContentCache(const ContentCache &RHS) : Buffer(0), SourceLineCache(0) {
+    ContentCache(const ContentCache &RHS) 
+      : Buffer(0, false), SourceLineCache(0) 
+    {
       Entry = RHS.Entry;
 
-      assert (RHS.Buffer == 0 && RHS.SourceLineCache == 0
+      assert (RHS.Buffer.getPointer() == 0 && RHS.SourceLineCache == 0
               && "Passed ContentCache object cannot own a buffer.");
 
       NumLines = RHS.NumLines;

Modified: cfe/trunk/lib/Basic/SourceManager.cpp
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/SourceManager.cpp?rev=98690&r1=98689&r2=98690&view=diff
==============================================================================
--- cfe/trunk/lib/Basic/SourceManager.cpp (original)
+++ cfe/trunk/lib/Basic/SourceManager.cpp Tue Mar 16 17:53:51 2010
@@ -32,14 +32,14 @@
 //===----------------------------------------------------------------------===//
 
 ContentCache::~ContentCache() {
-  delete Buffer;
+  delete Buffer.getPointer();
 }
 
 /// getSizeBytesMapped - Returns the number of bytes actually mapped for
 ///  this ContentCache.  This can be 0 if the MemBuffer was not actually
 ///  instantiated.
 unsigned ContentCache::getSizeBytesMapped() const {
-  return Buffer ? Buffer->getBufferSize() : 0;
+  return Buffer.getPointer() ? Buffer.getPointer()->getBufferSize() : 0;
 }
 
 /// getSize - Returns the size of the content encapsulated by this ContentCache.
@@ -47,15 +47,16 @@
 ///  scratch buffer.  If the ContentCache encapsulates a source file, that
 ///  file is not lazily brought in from disk to satisfy this query.
 unsigned ContentCache::getSize() const {
-  return Buffer ? (unsigned) Buffer->getBufferSize()
-                : (unsigned) Entry->getSize();
+  return Buffer.getPointer() ? (unsigned) Buffer.getPointer()->getBufferSize()
+                             : (unsigned) Entry->getSize();
 }
 
 void ContentCache::replaceBuffer(const llvm::MemoryBuffer *B) {
-  assert(B != Buffer);
+  assert(B != Buffer.getPointer());
   
-  delete Buffer;
-  Buffer = B;
+  delete Buffer.getPointer();
+  Buffer.setPointer(B);
+  Buffer.setInt(false);
 }
 
 const llvm::MemoryBuffer *ContentCache::getBuffer(Diagnostic &Diag,
@@ -64,12 +65,13 @@
     *Invalid = false;
       
   // Lazily create the Buffer for ContentCaches that wrap files.
-  if (!Buffer && Entry) {
+  if (!Buffer.getPointer() && Entry) {
     std::string ErrorStr;
     struct stat FileInfo;
-    Buffer = MemoryBuffer::getFile(Entry->getName(), &ErrorStr,
-                                   Entry->getSize(), &FileInfo);
-
+    Buffer.setPointer(MemoryBuffer::getFile(Entry->getName(), &ErrorStr,
+                                            Entry->getSize(), &FileInfo));
+    Buffer.setInt(false);
+    
     // If we were unable to open the file, then we are in an inconsistent
     // situation where the content cache referenced a file which no longer
     // exists. Most likely, we were using a stat cache with an invalid entry but
@@ -80,16 +82,16 @@
     // currently handle returning a null entry here. Ideally we should detect
     // that we are in an inconsistent situation and error out as quickly as
     // possible.
-    if (!Buffer) {
+    if (!Buffer.getPointer()) {
       const llvm::StringRef FillStr("<<<MISSING SOURCE FILE>>>\n");
-      Buffer = MemoryBuffer::getNewMemBuffer(Entry->getSize(), "<invalid>");
-      char *Ptr = const_cast<char*>(Buffer->getBufferStart());
+      Buffer.setPointer(MemoryBuffer::getNewMemBuffer(Entry->getSize(), 
+                                                      "<invalid>"));
+      char *Ptr = const_cast<char*>(Buffer.getPointer()->getBufferStart());
       for (unsigned i = 0, e = Entry->getSize(); i != e; ++i)
         Ptr[i] = FillStr[i % FillStr.size()];
       Diag.Report(diag::err_cannot_open_file)
         << Entry->getName() << ErrorStr;
-      if (Invalid)
-        *Invalid = true;
+      Buffer.setInt(true);
     } else {
       // Check that the file's size and modification time is the same as 
       // in the file entry (which may have come from a stat cache).
@@ -97,17 +99,18 @@
         Diag.Report(diag::err_file_size_changed)
           << Entry->getName() << (unsigned)Entry->getSize() 
           << (unsigned)FileInfo.st_size;
-        if (Invalid)
-          *Invalid = true;
+        Buffer.setInt(true);
       } else if (FileInfo.st_mtime != Entry->getModificationTime()) {
         Diag.Report(diag::err_file_modified) << Entry->getName();
-        if (Invalid)
-          *Invalid = true;
+        Buffer.setInt(true);
       }
     }
   }
   
-  return Buffer;
+  if (Invalid)
+    *Invalid = Buffer.getInt();
+  
+  return Buffer.getPointer();
 }
 
 unsigned LineTableInfo::getLineTableFilenameID(const char *Ptr, unsigned Len) {

Modified: cfe/trunk/test/PCH/changed-files.c
URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/PCH/changed-files.c?rev=98690&r1=98689&r2=98690&view=diff
==============================================================================
--- cfe/trunk/test/PCH/changed-files.c (original)
+++ cfe/trunk/test/PCH/changed-files.c Tue Mar 16 17:53:51 2010
@@ -1,5 +1,6 @@
 const char *s0 = m0;
 int s1 = m1;
+const char *s2 = m0;
 
 // RUN: echo '#define m0 ""' > %t.h
 // RUN: %clang_cc1 -emit-pch -o %t.h.pch %t.h





More information about the cfe-commits mailing list