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

[Guillaume Papin]

  > FYI, when I read your description of the option, it seemed like what it did was choose transforms that have been available in cpp11-migrate since clang-3.0 (which is completely different from what the flag actually does).
  > The -supports flag is not very self-documenting; can you think of a name that would be self-documenting? TBH I can't think of a good, short name for this; maybe -filter-transforms-for-compilers=clang-3.0,gcc-4.8,icc-12.1? (I'm not very happy with that name though)

  Thanks for the feedback.

  For the description of the option I will try to come up with something better when everyone will agree on a good name. I already tried different names and I have some difficulties as well. Before `-supports` the option was `-toward`.
  I also thought about `-targets`, `-platform/-P`, `-to`. Longer and more explicit names are welcomed but I couldn't come with one either (yet).

  A question about your suggestion, do you think this:

    cpp11-migrate -filter-transforms-for-compilers=clang-3.0,gcc-4.8,icc-12.1

  is better than this:

    cpp11-migrate -filter-transforms-for-compilers clang-3.0 -filter-transforms-for-compilers gcc-4.8 -filter-transforms-for-compilers gcc-4.8 icc-12.1

  Well I agree with an option of this size it doesn't look good but I find it a bit weird to give different arguments separated by a comma. It's more common to repeat the option like when adding directories to the header search paths:

    clang -I foo/ -I bar/ # not clang -Ifoo,bar

  Maybe the option can have a long and quite explicit name, as well as a one-letter alias easier to type?


================
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:
> 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.
Okay, version information restored, I didn't change the VERSION requirements even if it is more permissive now.

================
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:
> 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.
I decided to be 'strict' because I don't know how some compiler versions works. I do not have MSVC or ICC in my possession right now, if MSVC release candidates versions look like 11rc1 this won't be handled for example.

I changed the function to ignore what's after the minor. I tried to make the error message a bit more helpful as well, now it prints:

  $ cpp11-migrate -supports clang-potato foo.cpp -- 
  cpp11-migrate: clang-potato: invalid version, please use "<major>[.<minor>]" instead of "potato"

I created a test case in case we plan on improving the class later on. But I would rather have a more explicit message than supporting all kinds of versions the compilers can produce.

I know Emacs Lisp has built-in functionality for handling versions and it can become a bit convoluted quickly looking at what it handles:

  1.0pre2   1.0.7.5   22.8beta3   0.9alpha1   6.9.30Beta

Something with `pre` will be considered an inferior version than the non pre version for example (1.0pre2 < 1.0).

================
Comment at: docs/MigratorUsage.rst:121-123
@@ +120,5 @@
+
+  There is a special platform **c++11** that enables all transforms.
+
+  ``cpp11-migrate -supports c++11 <args..>``
+
----------------
Sean Silva wrote:
> Is there ever a situation where `-supports c++11` would alter behavior? If not, then why have it?
Yes because by default no transforms are enabled, so this switch enables them all.

But now I agree that by default cpp11-migrate should probably have everything enabled by default, so you can say:

 cpp11-migrate file.cpp --

And the file is migrated to C++11 as one can expect from the C++11 Migrator.

Someone opened an issue about this (https://cpp11-migrate.atlassian.net/browse/CM-97) and I don't know if everyone agree with this so I didn't try to change the old behavior in this patch.

Another reason for this flag is that I think an addition like `c++14` can be appreciated someday so it made sense to me to have `c++11`.

================
Comment at: docs/MigratorUsage.rst:116-119
@@ -115,1 +115,6 @@
 
+.. option:: -supports <platform>
+
+  Select the platforms to support. Transforms will be selected automatically to
+  work on all selected platforms.
+
----------------
Sean Silva wrote:
> This description is not very clear. How about "Select transforms targeting the intersection of language features supported by the given compilers".
Fixed, thanks.
I should take more time to write proper sentences and more useful to users not familiar with the migrator.


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

BRANCH
  supports-cmd-2

ARCANIST PROJECT
  clang-tools-extra