[PATCH] cpp11-migrate: Add '-supports' command line switch.

Edwin Vane edwin.vane at intel.com
Fri Jul 26 10:30:32 PDT 2013


  I think you ought to update the docs to explain the "all transforms enabled by default" behaviour. As part of this, be sure to indicate how explicitly specifying a transform behaves, especially if you do that **and** use `-for-compilers`.


================
Comment at: cpp11-migrate/Core/Transform.h:271
@@ -226,2 +270,3 @@
 ///
 /// Each transform should subclass it and implement the \c createTransform()
+/// method.
----------------
it -> this class. Remove 'the'.

================
Comment at: cpp11-migrate/Core/Transform.h:274
@@ +273,3 @@
+///
+/// Some constraints can informed about the compiler versions supported by the
+/// transform. These constraints should be set in the constructor as show in the
----------------
This paragraph is a bit weird. Does this suggested replacement capture what you wanted to say:

In the sub-classed factory constructor, specify the earliest version each compiler in \c CompilerVersions supported the feature to be introduced by the transform. See the example below.

================
Comment at: cpp11-migrate/Core/Transform.cpp:143
@@ +142,3 @@
+    // ignore version components after the minor
+    MinorStr = MinorStr.slice(0, MinorStr.find('.'));
+    if (MinorStr.getAsInteger(10, V.Minor))
----------------
Not important but for consistency and self documentation you could also just use .split('.') again like

  llvm::tie(MinorStr, Ignore) = MinorStr.split('.')

================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:128
@@ +127,3 @@
+    cl::desc("Select transforms targeting the intersection of\n"
+             "language features supported by the given compilers,\n"
+             "Takes a comma-seperated list of <compiler>-<version>.\n"
----------------
, here should be a .

================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:261
@@ -190,1 +260,3 @@
+          << argv[0]
+          << ": The given compilers can't benefit from any of the transforms\n";
     return 1;
----------------
Perhaps clearer to say: "no transforms available for specified compilers".


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



More information about the cfe-commits mailing list