================
Comment at: cpp11-migrate/Core/FileOverrides.cpp:99
@@ +98,3 @@
+ I != E; ++I) {
+ const Replacement &R = *I;
+ if (R.getFilePath() == MainFileName)
----------------
I'm not sure this temp variable really helps readability at all. I'd just use
*I and I-> below.
================
Comment at: cpp11-migrate/Core/ReplacementsYaml.h:34
@@ +33,3 @@
+ unsigned int Length = R.getLength();
+ llvm::StringRef ReplacementText = R.getReplacementText();
+ Io.mapRequired("Offset", Offset);
----------------
I discovered this doesn't work for reading. Check out
`ScalarTraits<std::string>` in
https://github.com/revane/migmerge_git/blob/master/YAML.h.
================
Comment at: cpp11-migrate/Core/Replacement.h:1
@@ +1,2 @@
+//===-- Core/Replacement.h --------------------------------------*- C++
-*-===//
+//
----------------
Since this file contains stuff only used for YAML, can it be merged with
ReplacementsYaml.h?
================
Comment at: cpp11-migrate/Core/FileOverrides.h:63
@@ +62,3 @@
+ /// \brief getter for TransformReplacementsDoc.
+ TransformDocument &getTransformReplacementsDoc() {
+ return TransformReplacementsDoc;
----------------
I'm not sure how you feel about this but here's a suggestion. To try and
maintain semantics, how about we get rid of the non-const iterator providers in
FileOverrides, mark this member function `const` and use this impl instead:
return const_cast<HeaderOverride*>(this)->TransformReplacementsDoc
I think it's safe here because we know the returned ref is only used for
writing out to file and thus won't be modified.
================
Comment at: test/cpp11-migrate/HeaderReplacements/main.cpp:6
@@ +5,3 @@
+// RUN: mkdir -p %t/Test
+// RUN: cp %p/main.cpp %p/common.cpp %p/common.h %t/Test
+// RUN: cpp11-migrate -loop-convert -headers -include=%t/Test %t/Test/main.cpp
%t/Test/common.cpp --
----------------
Use %S instead of %p for consistency. We have other tests that use it already.
================
Comment at: test/cpp11-migrate/HeaderReplacements/main.cpp:8
@@ +7,3 @@
+// RUN: cpp11-migrate -loop-convert -headers -include=%t/Test %t/Test/main.cpp
%t/Test/common.cpp --
+// RUN: ls %t/Test/main.cpp_common.h_*.yaml | wc -l | grep 1
+// RUN: ls %t/Test/common.cpp_common.h_*.yaml | wc -l | grep 1
----------------
Could you add a comment explaning what these commands are doing? Especially the
seds.
================
Comment at: test/cpp11-migrate/HeaderReplacements/common.cpp:3
@@ +2,3 @@
+// in main.cpp
+// RUN: ls
+
----------------
Looking at lit source it looks like you can just say `// END.`
http://llvm-reviews.chandlerc.com/D1142
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
