Re: [PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).

[Richard Smith via cfe-commits]

On 1 June 2017 at 14:36, Tim Northover  wrote:

> On 26 May 2017 at 11:29, Richard Smith  wrote:
> > If we generally think that distinction is a good thing, then (because
> this
> > is a conforming extension) consistency weakly suggests that it should
> not be
> > controlled by GNU mode. But I don't find that argument decisive; the
> > important thing is that we don't enable non-conforming extensions by
> default
> > in non-GNU (and non-MS-compat) modes, not that GNU mode consists of
> /only/
> > non-conforming extensions.
>
> I'm pretty convinced by the conforming/non-conforming distinction and
> consistency argument. And think that the Microsoft way seems even
> better for the future.
>
> Making the libc++ test pass is pretty ugly, but I've managed to get it
> working by building with "-Werror=gnu-imaginary-constant".
>
> Marshall, I know this really isn't your preferred solution but can you
> stomach it if I also make sure we do the extra diagnostics so it's
> difficult to misuse?
>
> > Looking at the
> >
> >   std::complex x = 1.0if;
> >
> > case again, I think there's another problem here: we support an implicit
> > conversion from _Complex float to float in C++ (without even a warning).
> > This conversion is valid in C, but at least GCC disallows it in its C++
> > mode. We should probably at least warn on that.
>
> Definitely. I think the error from G++ is probably the right choice.
> I'll get cracking on that, it's a good idea regardless of what happens
> here.
>

Great, thanks, your intended direction makes sense to me.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).

[Tim Northover via cfe-commits]

On 26 May 2017 at 11:29, Richard Smith  wrote:
> If we generally think that distinction is a good thing, then (because this
> is a conforming extension) consistency weakly suggests that it should not be
> controlled by GNU mode. But I don't find that argument decisive; the
> important thing is that we don't enable non-conforming extensions by default
> in non-GNU (and non-MS-compat) modes, not that GNU mode consists of /only/
> non-conforming extensions.

I'm pretty convinced by the conforming/non-conforming distinction and
consistency argument. And think that the Microsoft way seems even
better for the future.

Making the libc++ test pass is pretty ugly, but I've managed to get it
working by building with "-Werror=gnu-imaginary-constant".

Marshall, I know this really isn't your preferred solution but can you
stomach it if I also make sure we do the extra diagnostics so it's
difficult to misuse?

> Looking at the
>
>   std::complex x = 1.0if;
>
> case again, I think there's another problem here: we support an implicit
> conversion from _Complex float to float in C++ (without even a warning).
> This conversion is valid in C, but at least GCC disallows it in its C++
> mode. We should probably at least warn on that.

Definitely. I think the error from G++ is probably the right choice.
I'll get cracking on that, it's a good idea regardless of what happens
here.

Cheers.

Tim.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).

[Richard Smith via cfe-commits]

On 26 May 2017 at 10:34, Tim Northover via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> On 24 May 2017 at 17:54, Richard Smith  wrote:
> > Yikes :-( Maybe libc++ *should* have that extension then. It sounds like
> > we'll silently get the wrong value for any attempt at std::complex /
> > _Complex interop right now:
>
> That sounds like a pretty good idea whatever we decide to do about the
> literals.
>
> But we do need to sort out what we're going to do here. I quite like
> the approach you suggested, and think it's an improvement worth
> putting in for gnu++ modes regardless.
>
> So the remaining question is what we should do for the
> standards-compliant C++ modes. We seem to have 3 plausible options:
>
> 1. Bar it everywhere. It's an extension.
> 2. Bar it for C++14 onwards, where the standard provides literals.
> 3. Allow it everywhere.
>
> Although 2 would be the most conservative option (it maintains all
> existing behaviour), it's my least favourite for the added complexity
> and quirkiness.
>
> My original inclination is to go with Marshall and pick 1, but it's a
> very weak preference and I know I tend to be a bit user-hostile in
> these situations.
>
> So, how do we resolve this. Does anyone else have opinions?


Generally I'd also side towards 1, but in this instance that would mean
we're removing a conforming extension that prior versions of clang
supported in C++98 and C++11 modes, with no way to get the extension back
other than turning on the full set of (non-conforming) GNU extensions.

I quite like the approach we take to MS extensions: we have two separate
flags for the conforming and non-conforming extensions. For GNU extensions,
we generally do the same, where the conforming extensions are enabled by
default (with no way to turn them off other than -pedantic-errors or
-Werror=gnu!) and the non-conforming extension controlled by -std=gnu* vs
-std=c* ("GNU mode"). (In the longer term, I'd like to replicate the MS
pattern for the GNU extensions, with -fgnu-extensions controlling the
conforming extensions and -fgnu-compatibility controlling the
non-conforming extensions.)

If we generally think that distinction is a good thing, then (because this
is a conforming extension) consistency weakly suggests that it should not
be controlled by GNU mode. But I don't find that argument decisive; the
important thing is that we don't enable non-conforming extensions by
default in non-GNU (and non-MS-compat) modes, not that GNU mode consists of
/only/ non-conforming extensions.

Given that we probably want to retain a way to enable this outside GNU
mode, if we go for 1, then we would want a separate flag to enable it, and
then the difference between 1 and 3 is whether it's a -f flag or a -W flag.
The latter is our normal choice for such conforming extensions.


Looking at the

  std::complex x = 1.0if;

case again, I think there's another problem here: we support an implicit
conversion from _Complex float to float in C++ (without even a warning).
This conversion is valid in C, but at least GCC disallows it in its C++
mode. We should probably at least warn on that.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).

[Tim Northover via cfe-commits]

On 24 May 2017 at 17:54, Richard Smith  wrote:
> Yikes :-( Maybe libc++ *should* have that extension then. It sounds like
> we'll silently get the wrong value for any attempt at std::complex /
> _Complex interop right now:

That sounds like a pretty good idea whatever we decide to do about the literals.

But we do need to sort out what we're going to do here. I quite like
the approach you suggested, and think it's an improvement worth
putting in for gnu++ modes regardless.

So the remaining question is what we should do for the
standards-compliant C++ modes. We seem to have 3 plausible options:

1. Bar it everywhere. It's an extension.
2. Bar it for C++14 onwards, where the standard provides literals.
3. Allow it everywhere.

Although 2 would be the most conservative option (it maintains all
existing behaviour), it's my least favourite for the added complexity
and quirkiness.

My original inclination is to go with Marshall and pick 1, but it's a
very weak preference and I know I tend to be a bit user-hostile in
these situations.

So, how do we resolve this. Does anyone else have opinions?

Tim.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).

[Tim Northover via cfe-commits]

On 24 May 2017 at 15:32, Marshall Clow via Phabricator
 wrote:
> More. Trying the above code on godbolt.org, gcc 6.1/6.2/6.3/7.1 all reject it 
> (with `-std=c++14` and `-std=c++1z`) with the error message:

This was a pretty explicit intent in Richard's proposal: treat
incoming code as charitably as possible, maybe dial it back if it
turns out we need more GCC compatibility for some reason.

Tim.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).

[Richard Smith via cfe-commits]

On 24 May 2017 3:19 pm, "Tim Northover"  wrote:

On 24 May 2017 at 15:06, Richard Smith  wrote:
> I think this is expected. Clang has an extension where it treats 1.0if as
a
> _Complex float if no operator""if is available;

Since it's breaking some bots, I've reverted my commit while we hash
this out. r303813.


Sounds good.

> libc++ has an extension
> where std::complex can be initialized from _Complex float. For the
> same reason, that code has historically worked in C++11 and C++98 modes.

Are you sure? It looks like it does an implicit cast to float
(discarding the imaginary part) and then calls the "complex(float,
float = 0.0)" constructor to me.


Yikes :-( Maybe libc++ *should* have that extension then. It sounds like
we'll silently get the wrong value for any attempt at std::complex /
_Complex interop right now:

_Complex float a;
std::complex b = a; // discards imaginary component

That seems pretty scary.

Cheers.

Tim.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

More. Trying the above code on godbolt.org, gcc 6.1/6.2/6.3/7.1 all reject it 
(with `-std=c++14` and `-std=c++1z`) with the error message:

> : In function 'int main()':
>  :4:30: error: unable to find numeric literal operator 'operator""if'
> 
>   { std::complex foo  = 1.0if; }
>^
> 
> :4:30: note: use -std=gnu++11 or -fext-numeric-literals to enable 
> more built-in suffixes
>  Compiler exited with result code 1




https://reviews.llvm.org/D33424



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).

[Tim Northover via cfe-commits]

On 24 May 2017 at 15:06, Richard Smith  wrote:
> I think this is expected. Clang has an extension where it treats 1.0if as a
> _Complex float if no operator""if is available;

Since it's breaking some bots, I've reverted my commit while we hash
this out. r303813.

> libc++ has an extension
> where std::complex can be initialized from _Complex float. For the
> same reason, that code has historically worked in C++11 and C++98 modes.

Are you sure? It looks like it does an implicit cast to float
(discarding the imaginary part) and then calls the "complex(float,
float = 0.0)" constructor to me.

Cheers.

Tim.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).

[Richard Smith via cfe-commits]

On 24 May 2017 at 14:35, Marshall Clow via Phabricator via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> mclow.lists added a comment.
>
> This broke a libc++ test.  The following is expected to fail to compile:
>
>   #include 
>   #include 
>
>   int main()
>   {
>   std::complex foo  = 1.0if;  // should fail w/conversion
> operator not found
>   }
>
> when build as C++1z


I think this is expected. Clang has an extension where it treats 1.0if as a
_Complex float if no operator""if is available; libc++ has an extension
where std::complex can be initialized from _Complex float. For the
same reason, that code has historically worked in C++11 and C++98 modes.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D33424: Lexer: allow imaginary constants in GNU mode (only).

[Marshall Clow via Phabricator via cfe-commits]

mclow.lists added a comment.

This broke a libc++ test.  The following is expected to fail to compile:

  #include 
  #include 
  
  int main()
  {
  std::complex foo  = 1.0if;  // should fail w/conversion operator 
not found
  }

when build as C++1z


https://reviews.llvm.org/D33424



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits