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

Edwin Vane edwin.vane at intel.com
Wed Jul 24 12:38:09 PDT 2013



================
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))
----------------
Guillaume Papin wrote:
> 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.
> 
It's not a case of need but rather users specifying something like gcc-4.6.4 because that's what they see gcc --version output. Right now the migrator just outputs an error "invalid version 4.6.4" which isn't too helpful. We should just ignore any version components after the first two.

================
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"
----------------
Guillaume Papin wrote:
> 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?
You're right: should be "currently supports 4 builtin styles:". However, regarding CM-103 I think we shouldn't explicitly list the styles here and just refer to clang-format.

================
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 "
----------------
Guillaume Papin wrote:
> 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).
No. Just remove the example. The format of VERSION is important.


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

BRANCH
  supports-cmd-2

ARCANIST PROJECT
  clang-tools-extra



More information about the cfe-commits mailing list