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

[Guillaume Papin]

  > Do you see any value in tests for the edge cases for -supports?

  I reduced it to only one transform in order to see if the algorithm that enables the transforms based on their version is correct.

  > Bad compiler names,

  Test added.

  > bad version numbers, version numbers with patch components?

  Tests added.

  > Also, do any of the tests above test the case when no transforms get selected?

  No, test added.

  > There should probably be a special message to the user in that case.

  The following message is already printed:

    $ cpp11-migrate -supports clang-2.0 /tmp/foo.cpp -- -std=c++11
    cpp11-migrate: No selected transforms

  I changed it to be different when `-supports` is used, now it says:

    $ cpp11-migrate -supports clang-2.0 /tmp/foo.cpp -- -std=c++11
    cpp11-migrate: No transforms supported


================
Comment at: cpp11-migrate/Core/Transform.h:76
@@ -74,1 +75,3 @@
+  /// \brief Whether all transforms should be enabled by default or not.
+  bool EnableAllTransformByDefault;
 };
----------------
Edwin Vane wrote:
> Transform -> Transforms.
Fixed.

================
Comment at: cpp11-migrate/Core/Transform.h:317
@@ +316,3 @@
+protected:
+  /// \brief Since when this transform is available.
+  ///
----------------
Edwin Vane wrote:
> I'd say "Since when the C++11 feature introduced by this transform has been available."
Fixed.

================
Comment at: cpp11-migrate/Core/Transform.cpp:143
@@ +142,3 @@
+  if (!MinorStr.empty() && MinorStr.getAsInteger(10, V.Minor))
+    return Version();
+  if (MajorStr.getAsInteger(10, V.Major))
----------------
Edwin Vane wrote:
> Do we care to gracefully handle the patch version #? (i.e. the Z in X.Y.Z) Right now this code looks like it will just fail out since MinorStr will be of the form Y.Z and won't be an integer.
I tried to be simple with the version handling.
The 'important' C++11 features required by the transforms seems to all be available since a version with only major or major.minor.
IMHO until a transform needs it we can stick with this one.


================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:125
@@ +124,3 @@
+        "Currently supports:\n"
+        "\tc++11, clang-VERSION, gcc-VERSION, icc-VERSION and msvc-VERSION\n"
+        "where VERSION is MAJOR[.MINOR], example:\n"
----------------
Edwin Vane wrote:
> Any reason to use \t instead of spaces?
> 
> I see the c++11 option is explained in the docs but it might benefit to have a small description. Something like:
> 
> Currently supports:
>    c++11 (turn on all transforms), clang-VERSION, ...
Explanation added for `c++11`.

I used `\t` because I used it in the same way for the `-format-style` option (which was partially taken from `clang-format`).
For reference here is the `-format-style` code:

      cl::desc("Coding style to use on the replacements, either a builtin style\n"
               "or a YAML config file (see: clang-format -dump-config).\n"
               "Currently supports 4 builtins style:\n"
               "  LLVM, Google, Chromium, Mozilla.\n"),

Sorry if this is not the right place to ask for an English correction but is "Currently supports 4 builtins style" correct?

================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:126
@@ +125,3 @@
+        "\tc++11, clang-VERSION, gcc-VERSION, icc-VERSION and msvc-VERSION\n"
+        "where VERSION is MAJOR[.MINOR], example:\n"
+        "\tcpp11-migrate -toward clang-3.1 -toward gcc-4.7 -toward icc-12.1 "
----------------
Edwin Vane wrote:
> I'd remove the example. This is something better left for the docs.
Okay I removed:

  where VERSION is MAJOR[.MINOR], example:
      cpp11-migrate -toward clang-3.1 -toward gcc-4.7 -toward icc-12.1 -toward msvc-11 <args...>

Not 100% sure if that's what you wanted (removing both line or not).

================
Comment at: cpp11-migrate/tool/Cpp11Migrate.cpp:128
@@ +127,3 @@
+        "\tcpp11-migrate -toward clang-3.1 -toward gcc-4.7 -toward icc-12.1 "
+        "-toward msvc-11 <args...>"));
+
----------------
Edwin Vane wrote:
> Still using -toward here.
Problem "fixed", example has been removed.

================
Comment at: docs/MigratorUsage.rst:118
@@ +117,3 @@
+
+  Select the platforms to support. The transforms will be selected automatically
+  to work on all selected platform.
----------------
Edwin Vane wrote:
> The transforms -> Transforms.
Done.

================
Comment at: docs/MigratorUsage.rst:119
@@ +118,3 @@
+  Select the platforms to support. The transforms will be selected automatically
+  to work on all selected platform.
+
----------------
Edwin Vane wrote:
> platform->platforms.
Fixed.

================
Comment at: docs/MigratorUsage.rst:126
@@ +125,3 @@
+
+  And four compilers are supported. The transforms are enabled according to this
+  table:
----------------
Edwin Vane wrote:
> Remove 'And'.
Done.

================
Comment at: docs/MigratorUsage.rst:139
@@ +138,3 @@
+
+  (1): if *-override-macros* is provided it's assumed that the macros is C++11
+  aware and the transform is enabled without regard to the supported compilers.
----------------
Edwin Vane wrote:
> is->are.
Fixed.

================
Comment at: docs/MigratorUsage.rst:142
@@ +141,3 @@
+
+  The version has to be provided like this `-supports [COMPILER]-[VERSION]`.
+
----------------
Edwin Vane wrote:
> I'd reword to:
> 
> The structure of the argument to the -supports option is <compiler>-<major ver>[.<minor ver>] where <compiler> is one of the compilers from the above table.
Done, thank you.

================
Comment at: docs/MigratorUsage.rst:146
@@ +145,3 @@
+
+  1. If we want to support `Clang >= 3`, `GCC >= 4.6` and `MSVC >= 11`:
+
----------------
Edwin Vane wrote:
> Reword:
> 
> 1. To suport ...
Done, thanks.

================
Comment at: docs/MigratorUsage.rst:153
@@ +152,3 @@
+
+  2. If we want to support `icc >= 12` and a C++11-aware macro exists for the
+     `override` identifier:
----------------
Edwin Vane wrote:
> 2. To support icc >= 12 while using a C++11-aware macro for the override virtual specifier:
Done, thanks!


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

BRANCH
  supports-cmd-2

ARCANIST PROJECT
  clang-tools-extra