Re: [RFC] Increase libstdc++ line length to 100(?) columns

[Richard Biener via Gcc]

On Fri, Nov 27, 2020 at 3:48 AM Liu Hao via Gcc  wrote:
>
> 在 2020/11/27 上午7:50, Jonathan Wakely via Gcc 写道:
> > I've touched on the subject a few times, e.g.
> > https://gcc.gnu.org/pipermail/gcc/2019-December/230993.html
> > and https://gcc.gnu.org/pipermail/gcc/2019-December/231013.html
> >
> > Libstdc++ code is indented by 2 columns for the enclosing namespace,
> > usually another two for being in a template, and is full of __
> > prefixes for reserved names. On top of that, modern C++ declarations
> > are *noisy* (template head, requires-clause, noexcept-specifier, often
> > 'constexpr' or 'inline' and 'explicit', and maybe some attributes.
> >
> > All that gets hard to fit in 80 columns without compromising
> > readability with line breaks in unnatural places.
> >
>
> I think I want to vote +1 for this. On my 1920x1080 laptop screen with an 
> 11pt monospace font, 100
> colons allows me to open two terminals side by side, while still providing 3 
> colons for line
> numbers. On a larger desktop screen with a 10pt font it'd be 132 colomns, but 
> more often I find
> lines longer than 110 characters hard to read, so I agree with 100 (but I 
> suggest making it a
> 'recommended limit' instead of a 'hard limit' anyway).
>
>
> There was a small fragment of code in 
> :
>
> >   if (present)
> > ptr
> >   = gfc_build_conditional_assign_expr (block, 
> > present,
> >ptr, 
> > nullarg);
>
> Why not change this to:
>
> >   if (present)
> > ptr = gfc_build_conditional_assign_expr (
> > block, present, ptr, nullarg);
> >
>
> I think it looks balanced and way more comfortable, and doesn't waste much 
> leading space.

Other places use

   if (present)
 ptr = gfc_build_conditional_assign_expr
  (block, present, ptr, nullarg);

I prefer the ( on the next line.  The argument list can be two spaces
indented from
the function name or "right justified" (I think the latter looks
visually better).

Richard.

>
>
>
> --
> Best regards,
> LH_Mouse
>

Re: [RFC] Increase libstdc++ line length to 100(?) columns

[Ville Voutilainen via Gcc]

On Fri, 27 Nov 2020 at 10:16, Richard Biener via Libstdc++
 wrote:
> > Why not change this to:
> >
> > >   if (present)
> > > ptr = gfc_build_conditional_assign_expr (
> > > block, present, ptr, nullarg);
> > >
> >
> > I think it looks balanced and way more comfortable, and doesn't waste much 
> > leading space.
>
> Other places use
>
>if (present)
>  ptr = gfc_build_conditional_assign_expr
>   (block, present, ptr, nullarg);
>
> I prefer the ( on the next line.  The argument list can be two spaces
> indented from
> the function name or "right justified" (I think the latter looks
> visually better).

I find it easier to grok the code when the opening paren is on the
first line, I know instantly that I'm
looking at a function call. That sort of style probably fits better
the C++ code in libstdc++ than the code
in gcc, because in libstdc++ code we don't have a space before the
argument list.

Re: [RFC] Increase libstdc++ line length to 100(?) columns

[Liu Hao via Gcc]

在 2020/11/27 下午4:14, Richard Biener 写道:
> 
> I prefer the ( on the next line.  The argument list can be two spaces
> indented from
> the function name or "right justified" (I think the latter looks
> visually better).
> 

The right justification thing looks reasonable. For example, I think this

```c++
basic_cow_string(const basic_cow_string& other)
  noexcept
  : 
m_sth(allocator_traits::select_on_container_copy_construction(
other.m_sth.as_allocator()))
  { this->assign(other);  }
```

looks better than

```c++
basic_cow_string(const basic_cow_string& other)
  noexcept
  : 
m_sth(allocator_traits::select_on_container_copy_construction(
  other.m_sth.as_allocator()))
  { this->assign(other);  }
```

In the former fragment, indention of the 4th line is probably arbitrary. The 
only purpose is that,
if there wasn't a line break, the 3rd line would exceed a given length limit.

As you can see, qualified names in C++ can grow up to ~100 characters quite 
frequently. This may
deteriorate when `typename` and `template` are sometimes required. I don't 
think there is
practically a set of rules which governs all cases. So, if something looks 
better, go for it, and
that's why I think a (suggested) 100-char limit is better than the conventional 
80-char limit, which
forces another line break in front of `select_on_container_copy_construction`.


-- 
Best regards,
LH_Mouse



signature.asc
Description: OpenPGP digital signature

Re: [RFC] Increase libstdc++ line length to 100(?) columns

[Ville Voutilainen via Gcc]

On Fri, 27 Nov 2020 at 11:54, Liu Hao via Libstdc++
 wrote:
> As you can see, qualified names in C++ can grow up to ~100 characters quite 
> frequently. This may
> deteriorate when `typename` and `template` are sometimes required. I don't 
> think there is
> practically a set of rules which governs all cases. So, if something looks 
> better, go for it, and
> that's why I think a (suggested) 100-char limit is better than the 
> conventional 80-char limit, which
> forces another line break in front of `select_on_container_copy_construction`.

I do have a general question/thought/rumination here, though.
Shouldn't the paren-style and the line length
of libstdc++ be mainly decided by those who develop and maintain it?
It's already not written in the same style
as gcc is, so tweaking that different style to better suit the need of
a template-heavy C++ library perhaps
should be more or less a slam dunk? :)

Despite my sizable contributions to libstd++, I don't have a
particularly strong opinion here. Except that,
my somewhat strong opinion is "let's give Jonathan what he wants,
because it helps his work". ;)

Re: [RFC] Increase libstdc++ line length to 100(?) columns

[Allan Sandfeld Jensen]

On Freitag, 27. November 2020 00:50:57 CET Jonathan Wakely via Gcc wrote:
> I've touched on the subject a few times, e.g.
> https://gcc.gnu.org/pipermail/gcc/2019-December/230993.html
> and https://gcc.gnu.org/pipermail/gcc/2019-December/231013.html
> 
> Libstdc++ code is indented by 2 columns for the enclosing namespace,
> usually another two for being in a template, and is full of __
> prefixes for reserved names. On top of that, modern C++ declarations
> are *noisy* (template head, requires-clause, noexcept-specifier, often
> 'constexpr' or 'inline' and 'explicit', and maybe some attributes.
> 
> All that gets hard to fit in 80 columns without compromising
> readability with line breaks in unnatural places.
> 
> Does anybody object to raising the line length for libstdc++ code
> (not the rest of GCC) to 100 columns?
> 
If you _do_ change it. I would suggest changing it to 120, which is next 
common step for a lot of C++ projects.

Often also with an allowance for overruns if that makes the code cleaner.

'Allan

Re: unnormal Intel 80-bit long doubles and isnanl

[Florian Weimer via Gcc]

* Joseph Myers:

> glibc effectively treats them as unspecified behavior - we don't expect 
> them to produce any particular meaningful function return value (this 
> includes the possibility that such an invalid encoding might be returned 
> by a function given such an encoding as input), but if they result in 
> buffer overflows, infinite loops or similar, that's fixed as a bug.

I think the last part (the “bug”) is new.  I welcome a consensus along
those lines.  I just want to highlight this aspect.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: unnormal Intel 80-bit long doubles and isnanl

[Siddhesh Poyarekar]

On 11/27/20 5:01 PM, Florian Weimer wrote:

I think the last part (the “bug”) is new.  I welcome a consensus along
those lines.  I just want to highlight this aspect.


Should we consider fixing behaviour if the bug manifests in a user 
application and not in glibc itself?  i.e. a crash because glibc either 
returned the unnormal or misclassified the unnormal number?


At the minimu ISTM that we should at least make the classification 
consistent with gcc.


Siddhesh

Possible code to remove DECL_NONSHAREABLE?

[Matthew Malcomson via Gcc]

Hi there,

I was just looking through the history of how some code came about, and 
get the impression that DECL_NONSHAREABLE was meant to be removed.


It seems like it was added to solve PR49103, with the idea that it could 
be removed once a more robust solution was added.


Original comment and email mentioning the idea of this not being the 
final solution:

https://gcc.gnu.org/bugzilla/show_bug.cgi?id=49103#c12
https://gcc.gnu.org/legacy-ml/gcc-patches/2011-06/msg00480.html

Email mentioning the idea to remove it later
https://gcc.gnu.org/legacy-ml/gcc-patches/2011-06/msg01025.html


I seems that the more invasive solution was eventually added.
https://gcc.gnu.org/legacy-ml/gcc-patches/2011-11/msg00253.html
Commit 47598145be, From-SVN: r181172


Does that mean that the original DECL_NONSHAREABLE hack can be removed?
(It seems like the extra bit in the tree structure can't since it's now 
used for something else, but I suspect we can still remove the code 
using it for this DECL_NONSHAREABLE purpose).


I've ran a quick test on an AArch64 native configuration I had handy and 
that fortran test still passed, but don't have the time to look into it 
fully (especially given how I'm not familiar with this area).


Cheers,
Matthew

Re: unnormal Intel 80-bit long doubles and isnanl

[Florian Weimer via Gcc]

* Siddhesh Poyarekar:

> On 11/27/20 5:01 PM, Florian Weimer wrote:
>> I think the last part (the “bug”) is new.  I welcome a consensus along
>> those lines.  I just want to highlight this aspect.
>
> Should we consider fixing behaviour if the bug manifests in a user
> application and not in glibc itself?  i.e. a crash because glibc
> either returned the unnormal or misclassified the unnormal number?

I think in general, that's a bit like fixing buffer overflows in
applications.  It's just not possible with the current compilation
model.  So I find it difficult to come up with a general rule.

The nature of these non-normal numbers is that the CPU does not produce
them.  I think we should make sure that glibc doesn't, either, with
obvious exceptions such as memcpy.  But beyond that, I don't know.

> At the minimu ISTM that we should at least make the classification
> consistent with gcc.

Yes, I agree.

Thanks,
Florian
-- 
Red Hat GmbH, https://de.redhat.com/ , Registered seat: Grasbrunn,
Commercial register: Amtsgericht Muenchen, HRB 153243,
Managing Directors: Charles Cachera, Brian Klemm, Laurie Krebs, Michael O'Neill

Re: unnormal Intel 80-bit long doubles and isnanl

[Joseph Myers]

On Fri, 27 Nov 2020, Florian Weimer via Gcc wrote:

> The nature of these non-normal numbers is that the CPU does not produce
> them.  I think we should make sure that glibc doesn't, either, with
> obvious exceptions such as memcpy.  But beyond that, I don't know.

Exceptions probably also include the non-computational functions that just 
manipulate the sign bit (fabsl and copysignl).  And likewise the 
 functions conjl, creall, cimagl.  (For all of these, built-in 
functions would normally be expanded inline rather than using the library 
version anyway.)

> > At the minimu ISTM that we should at least make the classification
> > consistent with gcc.
> 
> Yes, I agree.

If you want to be consistent with how the processor interprets these as 
invalid operands, that would also mean issignaling treats all these 
invalid operands as signaling NaNs.

We haven't so far tried to make  functions do anything in 
particular with signaling NaNs (TS 18661-1 and C2x leave it 
implementation-defined how they are handled in such functions).  Arguably 
the functions other than those listed above should return (qNaN, qNaN) for 
any complex argument with at least one part sNaN (and thus for any complex 
argument with at least one part an invalid operand) rather than treating 
operands with one part sNaN and one Inf as an infinity.  (cabs should 
already do that, via its use of hypot which does handle sNaN.)

-- 
Joseph S. Myers
jos...@codesourcery.com

Motivation for diagnosing conversions that discard 'restrict' qualifier (as warning/error)

[Bruno De Fraine via Gcc]

Hello,

Gcc will diagnose C or C++ code that converts a pointer to a restrict qualified
type to a pointer to a type without restrict:

  int * __restrict p;
  int **pp = &p; // [C] warning: initialization discards 'restrict' qualifier
 // [C++] error: invalid conversion from 'int* __restrict__*' 
to 'int**'

I wanted to raise the question why a diagnostic when discarding restrict is
necessary, both by the language specification, and to protect the programmer
from inadvertent mistakes.
A similar diagnostic when discarding const or volatile qualifiers has a clear
purpose: such a conversion must be diagnosed, because the conversion can allow
following valid code to inadvertently perform an invalid access on a const or
volatile qualified object. For example:

  const int c = 123;
  int *pc = &c; // invalid: loss of 'const'
  *pc = 456;// valid

A similar example, also using the const qualifier, is given in the C99 standard
(section 6.5.16.1) and C++ standard (section [conv.qual]). Note that it is
undefined behavior to access a const or volatile object through an lvalue
without respectively the const or volatile qualifier.

The restrict qualifier works entirely different than const and volatile
qualifiers, however. Its implications are specified in C99, section 6.7.3.1,
and are defined in terms of aliasing between lvalues that have their address
"based on" a pointer object declared with restrict qualifier (versus those that
are not "based on"), within the scope of that declaration. For the definition
of "based on", it is irrelevant whether the restrict qualifier is retained in
the pointer expression or not, for example:

  int * __restrict p;
  int *q = p + j;
  int **qp = &q;
  *p, p[i], *q, **qp  // all lvalues based on 'p'

Consequently, there seems to be no value in diagnosing a loss of restrict
qualifier, or preventing that a restrict qualified pointer is inadvertently
accessed through an lvalue without this qualifier?

But is the compiler required by the language specification to diagnose this?
For C, it surely seems required: in the C99 specification, restrict is a type
qualifier, and section 6.5.16.1 lists as assignment constraint for pointers:
"both operands are pointers to qualified or unqualified versions of compatible
types, and the type pointed to by the left has all the qualifiers of the type
pointed to by the right". In the original example from this e-mail, the "has
all the qualifiers" part of the constraint is not satisfied, so compilers must
diagnose the violation.

The assignment constraint from the C specification is arguably too strict
though, because the following is also a violation:

  int* p;
  volatile int* const* pp = &p;
While this "has all the qualifiers", the pointers do not point to compatible
types ('int*' versus 'volatile int*'). The danger is that a pointer to a
volatile could be stored in 'p' by writing it through '*pp', but the const
qualifier prevents any stores through '*pp', so this is in fact safe. The C++
specification did address this overly strict constraint, and does allow the
above assignment when the const qualifier is present, as detailed for
qualification conversions in section [conv.qual].

Presumably, because the C constraint is known to be overly strict, gcc is only
diagnosing violations of the strict constraint in C with a warning, while it is
diagnosing violations of the relaxed constraint in C++ with an error?

While an error seems appropriate for the loss of a const or volatile qualifier
in C++, gcc now also gives an error for the loss of a restrict qualifier in
C++, where I've tried to make the point in this e-mail that the loss of
restrict seems harmless. Unfortunately, the C++ language specification does
not consider the restrict qualifier (although __restrict is a common C++
language extension), so there is no direct answer whether this should be
considered a violation that should be diagnosed.

Thanks for any feedback that motivates the gcc implementation on this point.
I'll note that it sometimes raises questions, for example [1].

Best regards,
Bruno De Fraine

[1] https://stackoverflow.com/questions/48926403/

Re: [RFC] Increase libstdc++ line length to 100(?) columns

[Ed Smith-Rowland via Gcc]

On 11/26/20 6:50 PM, Jonathan Wakely via Gcc wrote:

I've touched on the subject a few times, e.g.
https://gcc.gnu.org/pipermail/gcc/2019-December/230993.html
and https://gcc.gnu.org/pipermail/gcc/2019-December/231013.html

Libstdc++ code is indented by 2 columns for the enclosing namespace,
usually another two for being in a template, and is full of __
prefixes for reserved names. On top of that, modern C++ declarations
are *noisy* (template head, requires-clause, noexcept-specifier, often
'constexpr' or 'inline' and 'explicit', and maybe some attributes.

All that gets hard to fit in 80 columns without compromising
readability with line breaks in unnatural places.

Does anybody object to raising the line length for libstdc++ code
(not the rest of GCC) to 100 columns?

Please read my replies in the thread linked above before telling me
that the code should be split up into smaller functions to avoid deep
nesting. The function I pointed to cannot easily be split up without
making the code slower to compile and potentially slower to run:
https://gcc.gnu.org/git/?p=gcc.git;a=blob;f=libstdc%2B%2B-v3/include/bits/stl_algobase.h;h=a2fd306e6d0cca579b510148ba1a7089e2b2f3a2;hb=HEAD#l1499 




+1

gcc-9-20201127 is now available

[GCC Administrator via Gcc]

Snapshot gcc-9-20201127 is now available on
  https://gcc.gnu.org/pub/gcc/snapshots/9-20201127/
and on various mirrors, see http://gcc.gnu.org/mirrors.html for details.

This snapshot has been generated from the GCC 9 git branch
with the following options: git://gcc.gnu.org/git/gcc.git branch releases/gcc-9 
revision 11e0a9e68ed2cc99d5d9f45c8c07be4a28edd048

You'll find:

 gcc-9-20201127.tar.xzComplete GCC

  SHA256=8e2894ec40c2efcf8eefd4c7be965bc36df0c7207a10b139502ea7ad1eb676c5
  SHA1=f20b6fa2441dca47cc014e77dd32f66c6085b5c9

Diffs from 9-20201120 are available in the diffs/ subdirectory.

When a particular snapshot is ready for public consumption the LATEST-9
link is updated and a message is sent to the gcc list.  Please do not use
a snapshot before it has been announced that way.