[PATCH] D67678: PR17164: Change clang's default behavior from -flax-vector-conversions=all to -flax-vector-conversions=integer.

[Kristof Beyls via Phabricator via cfe-commits]

kristof.beyls added inline comments.


================
Comment at: docs/ReleaseNotes.rst:84
+  In a future release of Clang, we intend to change the default to
+  ``-fno-lax-vector-conversions``.
+
----------------
rsmith wrote:
> efriedma wrote:
> > kristof.beyls wrote:
> > > efriedma wrote:
> > > > rsmith wrote:
> > > > > efriedma wrote:
> > > > > > And if you want to allow your code to build with both clang-9 and clang-10, you have to do version detection in your build scripts?
> > > > > I guess you'd detect whether the compiler supports `-flax-vector-conversions=all`, and pass that if so, and otherwise pass `-flax-vector-conversions`. Well, either that or you fix your code to not rely on lax vector conversions between int and float vectors. If your code builds with GCC, you did that already (they never supported lax conversions between float and int vectors, as far as I can tell).
> > > > > 
> > > > > Do you have a preferred alternative?
> > > > All the alternatives are terrible in their own way:
> > > > 
> > > > 1. This status quo, which breaks compatibility with previous versions of clang
> > > > 2. We could make -flax-vector-conversions mean the same thing as earlier versions of clang.  So the flag wouldn't have the same meaning as gcc's flag.
> > > > 3. Some mix of the previous options: we could wait until there are one or two released versions that support -flax-vector-conversions=all , then change the meaning of -flax-vector-conversions.  But I have no idea how we would decide on a timeline.
> > > > 
> > > > I ran into this issue recently updating our compiler.  That said, the code in question was only using the implicit conversion in a couple places by accident, so it was easy enough to just fix the source.
> > > Maybe adding an entry in the release notes about this change could help with making option 1 slightly more palatable?
> > > My guess is that option 1 is the right one for the long term (compatibility between gcc and clang so code is more portable between both compilers).
> > It probably makes sense to call out the behavior change to -flax-vector-conversions in the release notes, yes.
> @kristof.beyls Are you looking for more changes to the release notes in addition to what's already in this change? If so, what would you like to see?
@rsmith I'm afraid I reacted to the review comments above and completely missed you had already added an entry to the release notes! My apologies.
I think it might still take some time for someone getting a build error who then goes through the release notes to easily spot that it's the change to lax vector conversions that's making their build fail.
However, I can't think of a much better way to describe this in the release notes so that a developer would spot this more easily, unless we'd put a source code example of something that now fails by default that didn't before. Putting source code examples in the release notes for all changes might make the release notes too long/complex?


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D67678/new/

https://reviews.llvm.org/D67678