On Tue, Oct 15, 2019 at 07:34:31PM +0200, Jakub Jelinek wrote:
> On Tue, Oct 15, 2019 at 01:17:17PM -0400, Marek Polacek wrote:
> > 2019-10-15 Marek Polacek <pola...@redhat.com>
> >
> > PR c++/92106 - ICE with structured bindings and -Wreturn-local-addr.
> > * typeck.c (maybe_warn_about_returning_address_of_local): Avoid
> > recursing on null initializer.
> >
> > * g++.dg/cpp1z/decomp50.C: New test.
> >
> > diff --git gcc/cp/typeck.c gcc/cp/typeck.c
> > index 141d86f50c9..1825540016f 100644
> > --- gcc/cp/typeck.c
> > +++ gcc/cp/typeck.c
> > @@ -9354,10 +9354,8 @@ maybe_warn_about_returning_address_of_local (tree
> > retval)
> > binding. */
> > tree base = DECL_DECOMP_BASE (whats_returned);
> > if (TYPE_REF_P (TREE_TYPE (base)))
> > - {
> > - tree init = DECL_INITIAL (base);
> > + if (tree init = DECL_INITIAL (base))
> > return maybe_warn_about_returning_address_of_local (init);
> > - }
>
> Actually, seeing the dg-warning in the testcase, I think we shouldn't warn,
> for range-for it is just too hard to find out if it will be returning
> address of a local or not. &value in itself is not address of a local
> variable when the structured binding is a reference.
> Well, in the testcase as is it actually is (perhaps just in the reduced
> one and not original):
> const struct J & D.2293;
> const int name [value-expr: D.2293->name];
> const int value [value-expr: D.2293->value];
> ...
> struct reference D.2352;
>
> try
> {
> D.2352 = D<A::J*, int>::operator* (&__for_begin);
> D.2293 = &D.2352;
> but a small change to the testcase:
> template <typename _Iterator, typename> class D {
> public:
> - typename B<_Iterator>::reference operator*();
> + typename B<_Iterator>::reference &operator*();
> void operator++();
> };
> results in D.2293 = D<A::J*, int>::operator* (&__for_begin);
> and then it might very well not be address of a local variable.
>
> This isn't just about a false positive warning, when
> maybe_warn_about_returning_address_of_local returns true, then
> we actually return NULL pointer instead of the value user wanted.
>
> So, I think you want to add and do else return false;
> if init is NULL.
That's an interesting point, thanks. Here's a patch with that return
added.
Bootstrapped/regtested on x86_64-linux, ok for trunk / 9?
2019-10-15 Marek Polacek <pola...@redhat.com>
PR c++/92106 - ICE with structured bindings and -Wreturn-local-addr.
* typeck.c (maybe_warn_about_returning_address_of_local): Avoid
recursing on null initializer and return false instead.
* g++.dg/cpp1z/decomp50.C: New test.
diff --git gcc/cp/typeck.c gcc/cp/typeck.c
index 141d86f50c9..90bd5bb9cdc 100644
--- gcc/cp/typeck.c
+++ gcc/cp/typeck.c
@@ -9355,8 +9355,10 @@ maybe_warn_about_returning_address_of_local (tree retval)
tree base = DECL_DECOMP_BASE (whats_returned);
if (TYPE_REF_P (TREE_TYPE (base)))
{
- tree init = DECL_INITIAL (base);
- return maybe_warn_about_returning_address_of_local (init);
+ if (tree init = DECL_INITIAL (base))
+ return maybe_warn_about_returning_address_of_local (init);
+ else
+ return false;
}
}
bool w = false;
diff --git gcc/testsuite/g++.dg/cpp1z/decomp50.C
gcc/testsuite/g++.dg/cpp1z/decomp50.C
new file mode 100644
index 00000000000..13b40146379
--- /dev/null
+++ gcc/testsuite/g++.dg/cpp1z/decomp50.C
@@ -0,0 +1,51 @@
+// PR c++/92106 - ICE with structured bindings and -Wreturn-local-addr.
+// { dg-do compile { target c++17 } }
+
+template <typename> struct B;
+template <typename _Tp> struct B<_Tp *> { typedef _Tp reference; };
+struct C {
+ template <typename _Up> using rebind = _Up *;
+};
+template <typename _Iterator, typename> class D {
+public:
+ typename B<_Iterator>::reference operator*();
+ void operator++();
+};
+
+template <typename _Iterator, typename _Container>
+bool operator!=(D<_Iterator, _Container>, D<_Iterator, _Container>);
+template <typename _Tp> class F {
+public:
+ typedef _Tp value_type;
+};
+
+template <typename _Alloc> struct G {
+ template <typename _Tp> struct H { using type = C::rebind<_Tp>; };
+ using const_pointer = typename H<typename _Alloc::value_type>::type;
+};
+template <typename _Tp, typename _Alloc = F<_Tp>> class I {
+ typedef D<typename G<_Alloc>::const_pointer, int> const_iterator;
+
+public:
+ const_iterator begin();
+ const_iterator end();
+};
+
+struct A {
+ struct J {
+ int name;
+ int value;
+ };
+ I<J> members;
+ template <typename Key> const int *find(Key) {
+ for (const auto &[name, value] : members)
+ // See <https://gcc.gnu.org/ml/gcc-patches/2019-10/msg01107.html>
+ // for why we don't warn here.
+ return &value; // { dg-bogus "address of local variable" }
+ return nullptr;
+ }
+};
+int main() {
+ A a;
+ a.find("");
+}
