================
Comment at: cpp11-migrate/Core/ReplacementsYaml.h:43
@@ +42,3 @@
+
+// ScalarTraits to read/write std::string objects.
+template <>
----------------
Doxygen-style comments.
================
Comment at: cpp11-migrate/Core/ReplacementsYaml.h:66
@@ +65,3 @@
+ Io.mapRequired("Length", Length);
+ Io.mapRequired("ReplacementText", ReplacementText);
+ }
----------------
I think the use of temp variables here will break YAML file **reading**. the
mapRequired() is forming a map between the key "X" and the address of a
variable. If you're providing a temp var, whatever it reads from the file will
be stored in some random location in memory and never make it to the
destination structure.
It occurs to me that since you're using an existing structure we can't really
change the interface of (tooling::Replacement) you might have to use
[[http://llvm.org/docs/YamlIO.html#normalization|normalization]].
I suggest you add a YAML file-reading unit test. All you have to do is ensure
the file parses and the resulting doc contains expected info.
================
Comment at: cpp11-migrate/Core/ReplacementsYaml.h:34
@@ +33,3 @@
+ unsigned int Length = R.getLength();
+ llvm::StringRef ReplacementText = R.getReplacementText();
+ Io.mapRequired("Offset", Offset);
----------------
Tareq A. Siraj wrote:
> Edwin Vane wrote:
> > I discovered this doesn't work for reading. Check out
> > `ScalarTraits<std::string>` in
> > https://github.com/revane/migmerge_git/blob/master/YAML.h.
> Did you mean to refactor the tooling::Replacement class to return a string
> instead or just changing this to std::string here?
See my comment regarding normalization.
http://llvm-reviews.chandlerc.com/D1142
BRANCH
write_replacements_to_disk
ARCANIST PROJECT
clang-tools-extra
_______________________________________________
cfe-commits mailing list
[email protected]
http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
