[PATCH] cpp11-migrate: Write header replacements to disk

Tareq A. Siraj tareq.a.siraj at intel.com
Tue Jul 16 07:47:30 PDT 2013


  As for ordering of ordering of transforms, the AC says:
  **//Replacements are written in the order they were generated by transforms as the translation unit moves through the transform pipeline.//**
  which should be satisfied by this patch. If we need to group the transforms, we really should do it in a separate story. Please have a look at the [[https://cpp11-migrate.atlassian.net/browse/CM-87|ACs]].


================
Comment at: cpp11-migrate/Core/Replacement.h:22
@@ +21,3 @@
+/// \brief A replacement struct to store the transform ID and the replacement.
+struct MigratorReplacement {
+  llvm::SmallString<16> TransformID;
----------------
Edwin Vane wrote:
> Why do we need a whole new type? This is basically just Replacement with a string.
Because you need to pass the whole structure to the YAML output stream and Replacements in clang tooling has no notion of transform ids. Look at the specialized MappingTraits for the structure we want to serialize.

================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:210
@@ -208,3 +209,3 @@
     // written to disk for testing purposes.
-    for (HeaderOverrides::const_iterator HeaderI = Overrides.headers_begin(),
+    for (HeaderOverrides::iterator HeaderI = Overrides.headers_begin(),
                                          HeaderE = Overrides.headers_end();
----------------
Edwin Vane wrote:
> Tareq A. Siraj wrote:
> > Guillaume Papin wrote:
> > > Is this non-const iterator really necessary? Seems to me that the HeaderOverrides is not really modified, just read.
> > > 
> > Writing vector<MigratorReplacements> to YAML requires the vector to be non-const (not entirely sure why they did it like that) and thus the HeaderOverrides need to be non-const.
> Can we fix the yaml-writing code?
Not without major refactoring. The internals of the macro `LLVM_YAML_IS_SEQUENCE_VECTOR` has an `element()` which require the vector to be non-const (it allocates an element if it doesn't exist). The docs also mention this: http://llvm.org/docs/YamlIO.html#sequence


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



More information about the cfe-commits mailing list