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

Guillaume Papin guillaume.papin at epitech.eu
Mon Jul 29 01:27:31 PDT 2013



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

================
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
----------------
Edwin Vane wrote:
> 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.
I made some minor changes to your suggestion and come up with:

In the sub-classed factory constructor, specify the earliest versions since the compilers in \c CompilerVersions are supporting the feature 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))
----------------
Edwin Vane wrote:
> Not important but for consistency and self documentation you could also just use .split('.') again like
> 
>   llvm::tie(MinorStr, Ignore) = MinorStr.split('.')
Done.

================
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"
----------------
Edwin Vane wrote:
> , here should be a .
Fixed.

================
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;
----------------
Edwin Vane wrote:
> Perhaps clearer to say: "no transforms available for specified compilers".
Done.


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

BRANCH
  supports-cmd-2

ARCANIST PROJECT
  clang-tools-extra



More information about the cfe-commits mailing list