[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