[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