[PATCH] cpp11-migrate: Refactor for driver model of operation

Edwin Vane edwin.vane at intel.com
Thu Aug 29 12:47:03 PDT 2013



================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:192
@@ +191,3 @@
+    std::string &S = FileStates[FileName];
+    S.clear();
+    llvm::raw_string_ostream StringStream(S);
----------------
Tareq A. Siraj wrote:
> Is it necessary to take a reference and clearing the string vs. writing to a temp string and directly assigning to FileStates[FileName]?
No but it's more performant. What you're describing has an extra copy.

================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:191-195
@@ +190,7 @@
+        Rewrites.getSourceMgr().getFileEntryForID(BufferI->first)->getName();
+    std::string &S = FileStates[FileName];
+    S.clear();
+    llvm::raw_string_ostream StringStream(S);
+    BufferI->second.write(StringStream);
+    StringStream.flush();
+  }
----------------
Guillaume Papin wrote:
> Alternatively you can write something like:
> ```
> RewriteBuffer &RewriteBuf = BufferI->second;
> FileStates[FileName].assign(RewriteBuf.begin(), RewriteBuf.end());
> ```
Nice!

================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:403-408
@@ +402,8 @@
+    const TUReplacementsMap &ReplacementsMap = T->getAllReplacements();
+    const TranslationUnitReplacements &(
+        TUReplacementsMap::value_type::*getValue)() const =
+        &TUReplacementsMap::value_type::getValue;
+    std::transform(ReplacementsMap.begin(), ReplacementsMap.end(),
+                   std::back_inserter(AllReplacements),
+                   std::mem_fun_ref(getValue));
+
----------------
Guillaume Papin wrote:
> I feel like it's easier to understand when the member function is used directly where it's necessary. Not sure about the indentation but it seems like it fits 80 columns.
> 
> ```
>     std::transform(ReplacementsMap.begin(), ReplacementsMap.end(),
>                    std::back_inserter(AllReplacements),
>                    std::mem_fun_ref(&TUReplacementsMap::value_type::getValue));
> ```
I tried this already. The problem is what you're suggesting is ambiguous because there are two overloads of `getValue()`, one that's const and one that's not. The extra variable is there to disambiguate.


http://llvm-reviews.chandlerc.com/D1545



More information about the cfe-commits mailing list