[llvm] r186829 - Replace archive members in the old position.

Rafael Espindola rafael.espindola at gmail.com
Mon Jul 22 08:11:51 PDT 2013


Author: rafael
Date: Mon Jul 22 10:11:51 2013
New Revision: 186829

URL: http://llvm.org/viewvc/llvm-project?rev=186829&view=rev
Log:
Replace archive members in the old position.

This matches gnu archive behavior and since archive member order can change
which member is used, not changing the order on replacement looks like the
right thing to do.

This patch also refactors the logic for which archive member to keep and
whether to move it to a helper function (computeInsertAction). The
nesting in computeNewArchiveMembers was getting a bit confusing.

Modified:
    llvm/trunk/test/Object/archive-replace-pos.test
    llvm/trunk/tools/llvm-ar/llvm-ar.cpp

Modified: llvm/trunk/test/Object/archive-replace-pos.test
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/test/Object/archive-replace-pos.test?rev=186829&r1=186828&r2=186829&view=diff
==============================================================================
--- llvm/trunk/test/Object/archive-replace-pos.test (original)
+++ llvm/trunk/test/Object/archive-replace-pos.test Mon Jul 22 10:11:51 2013
@@ -25,3 +25,6 @@ RUN: llvm-ar t %t.a | FileCheck --check-
 CHECK3: .foo
 CHECK3-NEXT: .zed
 CHECK3-NEXT: .bar
+
+RUN: llvm-ar rc %t.a %t.zed
+RUN: llvm-ar t %t.a | FileCheck --check-prefix=CHECK3 %s

Modified: llvm/trunk/tools/llvm-ar/llvm-ar.cpp
URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/tools/llvm-ar/llvm-ar.cpp?rev=186829&r1=186828&r2=186829&view=diff
==============================================================================
--- llvm/trunk/tools/llvm-ar/llvm-ar.cpp (original)
+++ llvm/trunk/tools/llvm-ar/llvm-ar.cpp Mon Jul 22 10:11:51 2013
@@ -377,11 +377,11 @@ class NewArchiveIterator {
   bool IsNewMember;
   SmallString<16> MemberName;
   object::Archive::child_iterator OldI;
-  std::vector<std::string>::const_iterator NewI;
+  std::string NewFilename;
 
 public:
   NewArchiveIterator(object::Archive::child_iterator I, Twine Name);
-  NewArchiveIterator(std::vector<std::string>::const_iterator I, Twine Name);
+  NewArchiveIterator(std::string *I, Twine Name);
   NewArchiveIterator();
   bool isNewMember() const;
   object::Archive::child_iterator getOld() const;
@@ -398,9 +398,8 @@ NewArchiveIterator::NewArchiveIterator(o
   Name.toVector(MemberName);
 }
 
-NewArchiveIterator::NewArchiveIterator(
-    std::vector<std::string>::const_iterator I, Twine Name)
-    : IsNewMember(true), NewI(I) {
+NewArchiveIterator::NewArchiveIterator(std::string *NewFilename, Twine Name)
+    : IsNewMember(true), NewFilename(*NewFilename) {
   Name.toVector(MemberName);
 }
 
@@ -413,25 +412,24 @@ object::Archive::child_iterator NewArchi
 
 const char *NewArchiveIterator::getNew() const {
   assert(IsNewMember);
-  return NewI->c_str();
+  return NewFilename.c_str();
 }
 
 template <typename T>
 void addMember(std::vector<NewArchiveIterator> &Members,
                std::string &StringTable, T I, StringRef Name, int Pos = -1) {
+  if (Pos == -1) {
+    Pos = Members.size();
+    Members.resize(Pos + 1);
+  }
+
   if (Name.size() < 16) {
     NewArchiveIterator NI(I, Twine(Name) + "/");
-    if (Pos == -1)
-      Members.push_back(NI);
-    else
-      Members[Pos] = NI;
+    Members[Pos] = NI;
   } else {
     int MapIndex = StringTable.size();
     NewArchiveIterator NI(I, Twine("/") + Twine(MapIndex));
-    if (Pos == -1)
-      Members.push_back(NI);
-    else
-      Members[Pos] = NI;
+    Members[Pos] = NI;
     StringTable += Name;
     StringTable += "/\n";
   }
@@ -447,6 +445,60 @@ public:
 };
 }
 
+enum InsertAction {
+  IA_AddOldMember,
+  IA_AddNewMeber,
+  IA_Delete,
+  IA_MoveOldMember,
+  IA_MoveNewMember
+};
+
+static InsertAction
+computeInsertAction(ArchiveOperation Operation,
+                    object::Archive::child_iterator I, StringRef Name,
+                    std::vector<std::string>::iterator &Pos) {
+  if (Operation == QuickAppend || Members.empty())
+    return IA_AddOldMember;
+
+  std::vector<std::string>::iterator MI =
+      std::find_if(Members.begin(), Members.end(), HasName(Name));
+
+  if (MI == Members.end())
+    return IA_AddOldMember;
+
+  Pos = MI;
+
+  if (Operation == Delete)
+    return IA_Delete;
+
+  if (Operation == Move)
+    return IA_MoveOldMember;
+
+  if (Operation == ReplaceOrInsert) {
+    StringRef PosName = sys::path::filename(RelPos);
+    if (!OnlyUpdate) {
+      if (PosName.empty())
+        return IA_AddNewMeber;
+      return IA_MoveNewMember;
+    }
+
+    // We could try to optimize this to a fstat, but it is not a common
+    // operation.
+    sys::fs::file_status Status;
+    failIfError(sys::fs::status(*MI, Status));
+    if (Status.getLastModificationTime() < I->getLastModified()) {
+      if (PosName.empty())
+        return IA_AddOldMember;
+      return IA_MoveOldMember;
+    }
+
+    if (PosName.empty())
+      return IA_AddNewMeber;
+    return IA_MoveNewMember;
+  }
+  llvm_unreachable("No such operation");
+}
+
 // We have to walk this twice and computing it is not trivial, so creating an
 // explicit std::vector is actually fairly efficient.
 static std::vector<NewArchiveIterator>
@@ -471,28 +523,27 @@ computeNewArchiveMembers(ArchiveOperatio
         else
           InsertPos = Pos + 1;
       }
-      if (InsertPos == Pos)
-        Ret.resize(Ret.size() + Members.size());
-      if (Operation != QuickAppend && !Members.empty()) {
-        std::vector<std::string>::iterator MI =
-            std::find_if(Members.begin(), Members.end(), HasName(Name));
-        if (MI != Members.end()) {
-          if (Operation == Move) {
-            addMember(Moved, StringTable, I, Name);
-            continue;
-          }
-          if (Operation != ReplaceOrInsert || !OnlyUpdate)
-            continue;
-          // Ignore if the file if it is older than the member.
-          sys::fs::file_status Status;
-          failIfError(sys::fs::status(*MI, Status));
-          if (Status.getLastModificationTime() < I->getLastModified())
-            Members.erase(MI);
-          else
-            continue;
-        }
+
+      std::vector<std::string>::iterator MemberI = Members.end();
+      InsertAction Action = computeInsertAction(Operation, I, Name, MemberI);
+      switch (Action) {
+      case IA_AddOldMember:
+        addMember(Ret, StringTable, I, Name);
+        break;
+      case IA_AddNewMeber:
+        addMember(Ret, StringTable, &*MemberI, Name);
+        break;
+      case IA_Delete:
+        break;
+      case IA_MoveOldMember:
+        addMember(Moved, StringTable, I, Name);
+        break;
+      case IA_MoveNewMember:
+        addMember(Moved, StringTable, &*MemberI, Name);
+        break;
       }
-      addMember(Ret, StringTable, I, Name);
+      if (MemberI != Members.end())
+        Members.erase(MemberI);
     }
   }
 
@@ -502,27 +553,19 @@ computeNewArchiveMembers(ArchiveOperatio
   if (!RelPos.empty() && InsertPos == -1)
     fail("Insertion point not found");
 
-  if (Operation == Move) {
-    if (Members.size() != Moved.size())
-      fail("A member to be moved is not present in the archive");
-
-    if (RelPos.empty()) {
-      Ret.insert(Ret.end(), Moved.begin(), Moved.end());
-      return Ret;
-    }
-    assert(unsigned(InsertPos) <= Ret.size());
-    std::copy(Moved.begin(), Moved.end(), Ret.begin() + InsertPos);
-    return Ret;
-  }
+  if (RelPos.empty())
+    InsertPos = Ret.size();
+
+  assert(unsigned(InsertPos) <= Ret.size());
+  Ret.insert(Ret.begin() + InsertPos, Moved.begin(), Moved.end());
 
+  Ret.insert(Ret.begin() + InsertPos, Members.size(), NewArchiveIterator());
   int Pos = InsertPos;
   for (std::vector<std::string>::iterator I = Members.begin(),
-                                          E = Members.end();
-       I != E; ++I) {
+         E = Members.end();
+       I != E; ++I, ++Pos) {
     StringRef Name = sys::path::filename(*I);
-    addMember(Ret, StringTable, I, Name, Pos);
-    if (Pos != -1)
-      ++Pos;
+    addMember(Ret, StringTable, &*I, Name, Pos);
   }
 
   return Ret;





More information about the llvm-commits mailing list