[Bug c++/102482] Winit-list-lifetime false positive for temporaries with std::initializer_list

2021-10-07 Thread cvs-commit at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102482

--- Comment #9 from CVS Commits  ---
The master branch has been updated by Jonathan Wakely :

https://gcc.gnu.org/g:9b239d05ffd5a17ede44abd55bc6622c6e279868

commit r12-4228-g9b239d05ffd5a17ede44abd55bc6622c6e279868
Author: Jonathan Wakely 
Date:   Wed Sep 29 21:19:49 2021 +0100

c++: Do not warn about lifetime of std::initializer_list& [PR102482]

An initializer-list constructor taking a non-const lvalue cannot be
called with a temporary, so the array's lifetime probably doesn't end
with the full expression. -Winit-list-lifetime should not warn for that
case.

PR c++/102482

gcc/cp/ChangeLog:

* init.c (maybe_warn_list_ctor): Do not warn for a reference to
a non-const std::initializer_list.

gcc/testsuite/ChangeLog:

* g++.dg/warn/Winit-list5.C: New test.

[Bug c++/102482] Winit-list-lifetime false positive for temporaries with std::initializer_list

2021-10-04 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102482

Jonathan Wakely  changed:

   What|Removed |Added

   Keywords||patch

--- Comment #8 from Jonathan Wakely  ---
Patch: https://gcc.gnu.org/pipermail/gcc-patches/2021-October/580771.html

[Bug c++/102482] Winit-list-lifetime false positive for temporaries with std::initializer_list

2021-09-29 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102482

--- Comment #7 from Jonathan Wakely  ---
(In reply to Jonathan Wakely from comment #6)
> The warning should not trigger if the constructor takes the initializer_list
> by non-const lvalue reference.

I think this change to cp/init.c:maybe_warn_list_ctor will check if the
initializer_list parameter is a non-const lvalue reference:

--- a/gcc/cp/init.c
+++ b/gcc/cp/init.c
@@ -750,7 +750,8 @@ maybe_warn_list_ctor (tree member, tree init)
 return;

   tree parms = FUNCTION_FIRST_USER_PARMTYPE (current_function_decl);
-  tree initlist = non_reference (TREE_VALUE (parms));
+  tree parm1 = TREE_VALUE (parms);
+  tree initlist = non_reference (parm1);
   tree targs = CLASSTYPE_TI_ARGS (initlist);
   tree elttype = TREE_VEC_ELT (targs, 0);

@@ -758,6 +759,10 @@ maybe_warn_list_ctor (tree member, tree init)
   (TREE_TYPE (memtype), elttype))
 return;

+  if (TYPE_REF_P (parm1) && !TYPE_REF_IS_RVALUE (parm1)
+  && !(cp_type_quals (initlist) & TYPE_QUAL_CONST))
+return;
+
   tree begin = find_list_begin (init);
   if (!begin)
 return;

[Bug c++/102482] Winit-list-lifetime false positive for temporaries with std::initializer_list

2021-09-29 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102482

--- Comment #6 from Jonathan Wakely  ---
(In reply to Jonathan Wakely from comment #5)
> True, but approximately nobody writes constructors taking a non-const lvalue
> reference to std::initializer_list, so this corner case doesn't seem very
> important. This does seem like a concrete enhancement that could be made
> easily though, even if low value in practice.

Just to make this bit easier to find in my wall of text above:

The warning should not trigger if the constructor takes the initializer_list by
non-const lvalue reference.

[Bug c++/102482] Winit-list-lifetime false positive for temporaries with std::initializer_list

2021-09-29 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102482

Jonathan Wakely  changed:

   What|Removed |Added

 CC||dmalcolm at gcc dot gnu.org
 Status|UNCONFIRMED |NEW
   Last reconfirmed||2021-09-29
 Ever confirmed|0   |1

--- Comment #5 from Jonathan Wakely  ---
(In reply to Federico Kircheis from comment #4)
> Note that following equivalent snippet
> 
> 
> #include 
> 
> struct span {
> span(std::initializer_list il) noexcept : begin(nullptr),
> size(il.size()) { begin = il.begin();}
> const int* begin;
> std::size_t size;
> };
> 
> 
> does not trigger the warning.

Ideally it would warn, but I hope the reason it doesn't its fairly obvious. The
C++ front end doesn't perform arbitrarily complex data flow analysis for the
constructor body, it just looks for the most common pattern of storing a
pointer to the temporary array in a mem-initializer. Using the array in the
constructor body /without storing it/ is less problematic, because only the
non-static data members persist beyond the end of the constructor e.g. this is
fine:

span(std::initializer_list il)
{ for (auto i : il) std::cout << i << ' '; }


As a simple heuristic to warn about the majority of cases, GCC checks for
storing the pointer in the mem-initializer-list only.

Maybe you're misunderstanding what the warning is telling you. It's not saying
that the statement `return foo(span(std::initializer_list{}));` has a bug,
because that's not the location of the warning. It's telling you that your
constructor definition is potentially dangerous. Which is totally true. How is
the compiler supposed to know that you never plan to use that constructor for
lvalues? (C++ doesn't allow a && ref-qualifier on a constructor, which would
enforce that at the language level). Should it re-compile the constructor every
time it's called, passing in whether it's constructing an rvalue or lvalue, so
that only some uses of it warn? That would slow down compilation for every
class with an initializer-list constructor. What if the constructor is not
defined inline, so is compiled completely separate from its callers?

If **you** know that nobody will never use that constructor for lvalues, then
just disable the warning around the constructor:

struct span {
#pragma GCC diagnostic push
#pragma GCC diagnostic ignored "-Winit-list-lifetime"
span(std::initializer_list il) noexcept : begin(il.begin()),
size(il.size()) {}
#pragma GCC diagnostic pop
const int* begin;
std::size_t size;
};

Or if you don't think the warning is useful, just disable it on the command
line via -Wno-init-list-lifetime. But the compiler can't know how you plan to
use that constructor, so it tells you it's got a potential bug.

> > And the fabulous manual [...]
> 
> Mhm, I should have done a little more research before opening this ticket.

Reading the documentation for the warning you're reporting a bug for is
probably a good start :-)

> > *   When a list constructor stores the "begin" pointer from the
> > "initializer_list" argument, this doesn't extend the lifetime of
> > the array, so if a class variable is constructed from a temporary
> > "initializer_list", the pointer is left dangling by the end of the
> > variable declaration statement.
> 
> The first statement is correct (does not extend lifetime), but the second is
> not always (the pointer is not always left dangling).

If the pointer's lifetime has not ended, it dangles. The manual seems correct
as written (it could be made more verbose to talk about special cases, but
approximately nobody reads the manual now, adding more words isn't going to
help).


> Also following snippet generates the warning:
> 
> 
> #include 
> 
> struct span {
> span(std::initializer_list& il) noexcept : begin(il.begin()),
> size(il.size()) {}
> const int* begin;
> std::size_t size;
> };
> 
> 
> but I currently cannot imagine a scenario where this would dangle, as one
> cannot write "span({1,2,3})"

True, but approximately nobody writes constructors taking a non-const lvalue
reference to std::initializer_list, so this corner case doesn't seem very
important. This does seem like a concrete enhancement that could be made easily
though, even if low value in practice.

The point is, this is a warning that warns about **potentially** problematic
code in the constructor. All warnings like this have false positives (otherwise
they would be errors not warnings).

If you have concrete suggestions for improving the heuristics then it might be
possible to reduce the false positive rate, but otherwise this amounts to "this
warning has false positives" and the response is "yes".

The current -Winit-list-lifetime is a simple, heuristic-based warning done in
the front end, so is 

[Bug c++/102482] Winit-list-lifetime false positive for temporaries with std::initializer_list

2021-09-28 Thread federico.kircheis at gmail dot com via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102482

--- Comment #4 from Federico Kircheis  ---
Note that following equivalent snippet


#include 

struct span {
span(std::initializer_list il) noexcept : begin(nullptr),
size(il.size()) { begin = il.begin();}
const int* begin;
std::size_t size;
};


does not trigger the warning.

> And the fabulous manual [...]

Mhm, I should have done a little more research before opening this ticket.

> *   When a list constructor stores the "begin" pointer from the
> "initializer_list" argument, this doesn't extend the lifetime of
> the array, so if a class variable is constructed from a temporary
> "initializer_list", the pointer is left dangling by the end of the
> variable declaration statement.

The first statement is correct (does not extend lifetime), but the second is
not always (the pointer is not always left dangling).


Also following snippet generates the warning:


#include 

struct span {
span(std::initializer_list& il) noexcept : begin(il.begin()),
size(il.size()) {}
const int* begin;
std::size_t size;
};


but I currently cannot imagine a scenario where this would dangle, as one
cannot write "span({1,2,3})"

[Bug c++/102482] Winit-list-lifetime false positive for temporaries with std::initializer_list

2021-09-26 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102482

--- Comment #3 from Jonathan Wakely  ---
And the fabulous manual:

> warn about uses of "std::initializer_list" that are likely to result in
> dangling pointers

This is behaving exactly as documented:

> *   When a list constructor stores the "begin" pointer from the
> "initializer_list" argument, this doesn't extend the lifetime of
> the array, so if a class variable is constructed from a temporary
> "initializer_list", the pointer is left dangling by the end of the
> variable declaration statement.

[Bug c++/102482] Winit-list-lifetime false positive for temporaries with std::initializer_list

2021-09-26 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102482

--- Comment #2 from Jonathan Wakely  ---
(In reply to Jonathan Wakely from comment #1)
> > so I assume the warning is std::initializer_list specific.
> 
> Yes. std::initializer_list is a magic language type that the compiler knows
> all about. std::vector is just opaque C++ code that the compiler knows
> nothing about.

The clue is in the name: -Winit-list-lifetime

[Bug c++/102482] Winit-list-lifetime false positive for temporaries with std::initializer_list

2021-09-26 Thread redi at gcc dot gnu.org via Gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=102482

Jonathan Wakely  changed:

   What|Removed |Added

   Keywords||diagnostic

--- Comment #1 from Jonathan Wakely  ---
(In reply to Federico Kircheis from comment #0)
> While the warning is true, the code is completely safe, but the warnings
> seems to imply that foo will access a dangling pointer.

In this case it's safe because the span obejct has the same lifetime as the
initializer_list, but in general that's not true. The warning comes from the
constructor, and is independent of the context in which it's being constructed.

> so I assume the warning is std::initializer_list specific.

Yes. std::initializer_list is a magic language type that the compiler knows all
about. std::vector is just opaque C++ code that the compiler knows nothing
about.