Re: [PATCH v5] c++: implement [[gnu::non_owning]] [PR110358]
On Mon, Mar 04, 2024 at 11:00:18AM +, Jonathan Wakely wrote:
> On 01/03/24 15:38 -0500, Jason Merrill wrote:
> > On 3/1/24 14:24, Marek Polacek wrote:
> > > +@smallexample
> > > +template
> > > +[[gnu::no_dangling(std::is_reference_v)]] int foo (T& t) @{
> >
> > I think this function should return a reference.
>
> The condition in the attribute can only ever be true if you call this
> function with an explicit template argument list: foo(i). Is
> that intentional?
Not intentional. I just wanted to make it clear that the user
can use something like std::is_reference as the attribute argument,
but I didn't think about it very long.
> And if T is non-const it can't be called with a temporary and so
> dangling seems less of a problem for this function anyway, right?
Right.
> Would it make more sense as something like this?
>
> template
> [[gnu::no_dangling(std::is_lvalue_reference_v)]]
> decltype(auto) foo(T&& t) {
> ...
> }
>
> Or is this getting too complex/subtle for a simple example?
I like your example; it's only slightly more complex than the
original one and most likely more realistic. I'm pushing the
following patch. Thanks!
[pushed] doc: update [[gnu::no_dangling]]
...to offer a more realistic example.
gcc/ChangeLog:
* doc/extend.texi: Update [[gnu::no_dangling]].
---
gcc/doc/extend.texi | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/gcc/doc/extend.texi b/gcc/doc/extend.texi
index f679c81acf2..df0982fdfda 100644
--- a/gcc/doc/extend.texi
+++ b/gcc/doc/extend.texi
@@ -29370,7 +29370,8 @@ Or:
@smallexample
template
-[[gnu::no_dangling(std::is_reference_v)]] int& foo (T& t) @{
+[[gnu::no_dangling(std::is_lvalue_reference_v)]]
+decltype(auto) foo(T&& t) @{
@dots{}
@};
@end smallexample
base-commit: 77eb86be8841989651b3150a020dd1a95910cc00
--
2.44.0
Re: [PATCH v5] c++: implement [[gnu::non_owning]] [PR110358]
On 01/03/24 15:38 -0500, Jason Merrill wrote:
On 3/1/24 14:24, Marek Polacek wrote:
+@smallexample
+template
+[[gnu::no_dangling(std::is_reference_v)]] int foo (T& t) @{
I think this function should return a reference.
The condition in the attribute can only ever be true if you call this
function with an explicit template argument list: foo(i). Is
that intentional?
And if T is non-const it can't be called with a temporary and so
dangling seems less of a problem for this function anyway, right?
Would it make more sense as something like this?
template
[[gnu::no_dangling(std::is_lvalue_reference_v)]]
decltype(auto) foo(T&& t) {
...
}
Or is this getting too complex/subtle for a simple example?
Re: [PATCH v5] c++: implement [[gnu::non_owning]] [PR110358]
On 3/1/24 16:23, Patrick Palka wrote: Sorry for not asking this sooner, but does it matter whether we attach the attribute to the function type rather than the function declaration? I noticed e.g. nodiscard gets attached to the decl. And we document it as a function attribute despite attaching it to the function type. I think it doesn't matter much, some attributes are represented on the type and some on the decl. Might be a bit better on the decl but I wasn't worrying about it. Jason
Re: [PATCH v5] c++: implement [[gnu::non_owning]] [PR110358]
On Fri, 1 Mar 2024, Jason Merrill wrote:
> On 3/1/24 14:24, Marek Polacek wrote:
> > On Fri, Mar 01, 2024 at 01:19:40PM -0500, Jason Merrill wrote:
> > > On 3/1/24 12:39, Marek Polacek wrote:
> > > >@option{-Wdangling-reference} also warns about code like
> > > >@smallexample
> > > > @@ -3932,6 +3935,10 @@ struct Span @{
> > > >as @code{std::span}-like; that is, the class is a non-union class
> > > >that has a pointer data member and a trivial destructor.
> > > > +The warning can be disabled by using the @code{gnu::no_dangling}
> > > > attribute
> > > > +on a function (@pxref{Common Function Attributes}), or a class type
> > > > +(@pxref{C++ Attributes}).
> > >
> > > It seems surprising that one is in a generic attributes section and the
> > > other in the C++-specific section. Maybe both uses could be covered in
> > > the
> > > C++ attributes section?
> >
> > Arg yes, definitely. Done here.
> >
> > > >This warning is enabled by @option{-Wall}.
> > > >@opindex Wdelete-non-virtual-dtor
> > > > diff --git a/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
> > > > b/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
> > > > new file mode 100644
> > > > index 000..02eabbc5003
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
> > > > @@ -0,0 +1,38 @@
> > > > +// { dg-do compile { target c++11 } }
> > > > +// { dg-options "-Wdangling-reference" }
> > > > +
> > > > +int g = 42;
> > > > +
> > > > +struct [[gnu::no_dangling]] A {
> > > > + int *i;
> > > > + int () { return *i; }
> > > > +};
> > > > +
> > > > +struct A2 {
> > > > + int *i;
> > > > + [[gnu::no_dangling]] int () { return *i; }
> > > > + [[gnu::no_dangling]] static int (const int &) { return * }
> > > > +};
> > > > +
> > > > +union [[gnu::no_dangling]] U { };
> > > > +
> > > > +A a() { return A{}; }
> > > > +A2 a2() { return A2{}; }
> > > > +
> > > > +class X { };
> > > > +const X x1;
> > > > +const X x2;
> > > > +
> > > > +[[gnu::no_dangling]] const X& get(const int& i)
> > > > +{
> > > > + return i == 0 ? x1 : x2;
> > > > +}
> > > > +
> > > > +void
> > > > +test ()
> > > > +{
> > > > + [[maybe_unused]] const X& x = get (10); // { dg-bogus
> > > > "dangling" }
> > > > + [[maybe_unused]] const int = a().foo(); // { dg-bogus
> > > > "dangling" }
> > > > + [[maybe_unused]] const int = a2().foo(); // { dg-bogus
> > > > "dangling" }
> > > > + [[maybe_unused]] const int = a2().bar(10);// { dg-bogus
> > > > "dangling" }
> > > > +}
> > >
> > > Do you want to add destructors to A/A2 like you did in other tests?
> >
> > Added. I think this test predates the recent heuristic.
> >
> > Ok for trunk?
> >
> > -- >8 --
> > Since -Wdangling-reference has false positives that can't be
> > prevented, we should offer an easy way to suppress the warning.
> > Currently, that is only possible by using a #pragma, either around the
> > enclosing class or around the call site. But #pragma GCC diagnostic tend
> > to be onerous. A better solution would be to have an attribute.
> >
> > To that end, this patch adds a new attribute, [[gnu::no_dangling]].
> > This attribute takes an optional bool argument to support cases like:
> >
> >template
> >struct [[gnu::no_dangling(std::is_reference_v)]] S {
> > // ...
> >};
> >
> > PR c++/110358
> > PR c++/109642
> >
> > gcc/cp/ChangeLog:
> >
> > * call.cc (no_dangling_p): New.
> > (reference_like_class_p): Use it.
> > (do_warn_dangling_reference): Use it. Don't warn when the function
> > or its enclosing class has attribute gnu::no_dangling.
> > * tree.cc (cxx_gnu_attributes): Add gnu::no_dangling.
> > (handle_no_dangling_attribute): New.
> >
> > gcc/ChangeLog:
> >
> > * doc/extend.texi: Document gnu::no_dangling.
> > * doc/invoke.texi: Mention that gnu::no_dangling disables
> > -Wdangling-reference.
> >
> > gcc/testsuite/ChangeLog:
> >
> > * g++.dg/ext/attr-no-dangling1.C: New test.
> > * g++.dg/ext/attr-no-dangling2.C: New test.
> > * g++.dg/ext/attr-no-dangling3.C: New test.
> > * g++.dg/ext/attr-no-dangling4.C: New test.
> > * g++.dg/ext/attr-no-dangling5.C: New test.
> > * g++.dg/ext/attr-no-dangling6.C: New test.
> > * g++.dg/ext/attr-no-dangling7.C: New test.
> > * g++.dg/ext/attr-no-dangling8.C: New test.
> > * g++.dg/ext/attr-no-dangling9.C: New test.
> > ---
> > gcc/cp/call.cc | 38 ++--
> > gcc/cp/tree.cc | 26
> > gcc/doc/extend.texi | 47 ++
> > gcc/doc/invoke.texi | 6 ++
> > gcc/testsuite/g++.dg/ext/attr-no-dangling1.C | 40
> > gcc/testsuite/g++.dg/ext/attr-no-dangling2.C | 29 +
> > gcc/testsuite/g++.dg/ext/attr-no-dangling3.C | 24
> > gcc/testsuite/g++.dg/ext/attr-no-dangling4.C | 14 +
> > gcc/testsuite/g++.dg/ext/attr-no-dangling5.C |
Re: [PATCH v5] c++: implement [[gnu::non_owning]] [PR110358]
On 3/1/24 14:24, Marek Polacek wrote:
On Fri, Mar 01, 2024 at 01:19:40PM -0500, Jason Merrill wrote:
On 3/1/24 12:39, Marek Polacek wrote:
@option{-Wdangling-reference} also warns about code like
@smallexample
@@ -3932,6 +3935,10 @@ struct Span @{
as @code{std::span}-like; that is, the class is a non-union class
that has a pointer data member and a trivial destructor.
+The warning can be disabled by using the @code{gnu::no_dangling} attribute
+on a function (@pxref{Common Function Attributes}), or a class type
+(@pxref{C++ Attributes}).
It seems surprising that one is in a generic attributes section and the
other in the C++-specific section. Maybe both uses could be covered in the
C++ attributes section?
Arg yes, definitely. Done here.
This warning is enabled by @option{-Wall}.
@opindex Wdelete-non-virtual-dtor
diff --git a/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
b/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
new file mode 100644
index 000..02eabbc5003
--- /dev/null
+++ b/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
@@ -0,0 +1,38 @@
+// { dg-do compile { target c++11 } }
+// { dg-options "-Wdangling-reference" }
+
+int g = 42;
+
+struct [[gnu::no_dangling]] A {
+ int *i;
+ int () { return *i; }
+};
+
+struct A2 {
+ int *i;
+ [[gnu::no_dangling]] int () { return *i; }
+ [[gnu::no_dangling]] static int (const int &) { return * }
+};
+
+union [[gnu::no_dangling]] U { };
+
+A a() { return A{}; }
+A2 a2() { return A2{}; }
+
+class X { };
+const X x1;
+const X x2;
+
+[[gnu::no_dangling]] const X& get(const int& i)
+{
+ return i == 0 ? x1 : x2;
+}
+
+void
+test ()
+{
+ [[maybe_unused]] const X& x = get (10); // { dg-bogus "dangling" }
+ [[maybe_unused]] const int = a().foo(); // { dg-bogus "dangling" }
+ [[maybe_unused]] const int = a2().foo(); // { dg-bogus "dangling" }
+ [[maybe_unused]] const int = a2().bar(10);// { dg-bogus "dangling" }
+}
Do you want to add destructors to A/A2 like you did in other tests?
Added. I think this test predates the recent heuristic.
Ok for trunk?
-- >8 --
Since -Wdangling-reference has false positives that can't be
prevented, we should offer an easy way to suppress the warning.
Currently, that is only possible by using a #pragma, either around the
enclosing class or around the call site. But #pragma GCC diagnostic tend
to be onerous. A better solution would be to have an attribute.
To that end, this patch adds a new attribute, [[gnu::no_dangling]].
This attribute takes an optional bool argument to support cases like:
template
struct [[gnu::no_dangling(std::is_reference_v)]] S {
// ...
};
PR c++/110358
PR c++/109642
gcc/cp/ChangeLog:
* call.cc (no_dangling_p): New.
(reference_like_class_p): Use it.
(do_warn_dangling_reference): Use it. Don't warn when the function
or its enclosing class has attribute gnu::no_dangling.
* tree.cc (cxx_gnu_attributes): Add gnu::no_dangling.
(handle_no_dangling_attribute): New.
gcc/ChangeLog:
* doc/extend.texi: Document gnu::no_dangling.
* doc/invoke.texi: Mention that gnu::no_dangling disables
-Wdangling-reference.
gcc/testsuite/ChangeLog:
* g++.dg/ext/attr-no-dangling1.C: New test.
* g++.dg/ext/attr-no-dangling2.C: New test.
* g++.dg/ext/attr-no-dangling3.C: New test.
* g++.dg/ext/attr-no-dangling4.C: New test.
* g++.dg/ext/attr-no-dangling5.C: New test.
* g++.dg/ext/attr-no-dangling6.C: New test.
* g++.dg/ext/attr-no-dangling7.C: New test.
* g++.dg/ext/attr-no-dangling8.C: New test.
* g++.dg/ext/attr-no-dangling9.C: New test.
---
gcc/cp/call.cc | 38 ++--
gcc/cp/tree.cc | 26
gcc/doc/extend.texi | 47 ++
gcc/doc/invoke.texi | 6 ++
gcc/testsuite/g++.dg/ext/attr-no-dangling1.C | 40
gcc/testsuite/g++.dg/ext/attr-no-dangling2.C | 29 +
gcc/testsuite/g++.dg/ext/attr-no-dangling3.C | 24
gcc/testsuite/g++.dg/ext/attr-no-dangling4.C | 14 +
gcc/testsuite/g++.dg/ext/attr-no-dangling5.C | 31 ++
gcc/testsuite/g++.dg/ext/attr-no-dangling6.C | 65
gcc/testsuite/g++.dg/ext/attr-no-dangling7.C | 31 ++
gcc/testsuite/g++.dg/ext/attr-no-dangling8.C | 30 +
gcc/testsuite/g++.dg/ext/attr-no-dangling9.C | 25
13 files changed, 400 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling2.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling3.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling4.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling5.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling6.C
create
[PATCH v5] c++: implement [[gnu::non_owning]] [PR110358]
On Fri, Mar 01, 2024 at 01:19:40PM -0500, Jason Merrill wrote:
> On 3/1/24 12:39, Marek Polacek wrote:
> > @option{-Wdangling-reference} also warns about code like
> > @smallexample
> > @@ -3932,6 +3935,10 @@ struct Span @{
> > as @code{std::span}-like; that is, the class is a non-union class
> > that has a pointer data member and a trivial destructor.
> > +The warning can be disabled by using the @code{gnu::no_dangling} attribute
> > +on a function (@pxref{Common Function Attributes}), or a class type
> > +(@pxref{C++ Attributes}).
>
> It seems surprising that one is in a generic attributes section and the
> other in the C++-specific section. Maybe both uses could be covered in the
> C++ attributes section?
Arg yes, definitely. Done here.
> > This warning is enabled by @option{-Wall}.
> > @opindex Wdelete-non-virtual-dtor
> > diff --git a/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
> > b/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
> > new file mode 100644
> > index 000..02eabbc5003
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
> > @@ -0,0 +1,38 @@
> > +// { dg-do compile { target c++11 } }
> > +// { dg-options "-Wdangling-reference" }
> > +
> > +int g = 42;
> > +
> > +struct [[gnu::no_dangling]] A {
> > + int *i;
> > + int () { return *i; }
> > +};
> > +
> > +struct A2 {
> > + int *i;
> > + [[gnu::no_dangling]] int () { return *i; }
> > + [[gnu::no_dangling]] static int (const int &) { return * }
> > +};
> > +
> > +union [[gnu::no_dangling]] U { };
> > +
> > +A a() { return A{}; }
> > +A2 a2() { return A2{}; }
> > +
> > +class X { };
> > +const X x1;
> > +const X x2;
> > +
> > +[[gnu::no_dangling]] const X& get(const int& i)
> > +{
> > + return i == 0 ? x1 : x2;
> > +}
> > +
> > +void
> > +test ()
> > +{
> > + [[maybe_unused]] const X& x = get (10); // { dg-bogus "dangling" }
> > + [[maybe_unused]] const int = a().foo(); // { dg-bogus
> > "dangling" }
> > + [[maybe_unused]] const int = a2().foo(); // { dg-bogus
> > "dangling" }
> > + [[maybe_unused]] const int = a2().bar(10);// { dg-bogus
> > "dangling" }
> > +}
>
> Do you want to add destructors to A/A2 like you did in other tests?
Added. I think this test predates the recent heuristic.
Ok for trunk?
-- >8 --
Since -Wdangling-reference has false positives that can't be
prevented, we should offer an easy way to suppress the warning.
Currently, that is only possible by using a #pragma, either around the
enclosing class or around the call site. But #pragma GCC diagnostic tend
to be onerous. A better solution would be to have an attribute.
To that end, this patch adds a new attribute, [[gnu::no_dangling]].
This attribute takes an optional bool argument to support cases like:
template
struct [[gnu::no_dangling(std::is_reference_v)]] S {
// ...
};
PR c++/110358
PR c++/109642
gcc/cp/ChangeLog:
* call.cc (no_dangling_p): New.
(reference_like_class_p): Use it.
(do_warn_dangling_reference): Use it. Don't warn when the function
or its enclosing class has attribute gnu::no_dangling.
* tree.cc (cxx_gnu_attributes): Add gnu::no_dangling.
(handle_no_dangling_attribute): New.
gcc/ChangeLog:
* doc/extend.texi: Document gnu::no_dangling.
* doc/invoke.texi: Mention that gnu::no_dangling disables
-Wdangling-reference.
gcc/testsuite/ChangeLog:
* g++.dg/ext/attr-no-dangling1.C: New test.
* g++.dg/ext/attr-no-dangling2.C: New test.
* g++.dg/ext/attr-no-dangling3.C: New test.
* g++.dg/ext/attr-no-dangling4.C: New test.
* g++.dg/ext/attr-no-dangling5.C: New test.
* g++.dg/ext/attr-no-dangling6.C: New test.
* g++.dg/ext/attr-no-dangling7.C: New test.
* g++.dg/ext/attr-no-dangling8.C: New test.
* g++.dg/ext/attr-no-dangling9.C: New test.
---
gcc/cp/call.cc | 38 ++--
gcc/cp/tree.cc | 26
gcc/doc/extend.texi | 47 ++
gcc/doc/invoke.texi | 6 ++
gcc/testsuite/g++.dg/ext/attr-no-dangling1.C | 40
gcc/testsuite/g++.dg/ext/attr-no-dangling2.C | 29 +
gcc/testsuite/g++.dg/ext/attr-no-dangling3.C | 24
gcc/testsuite/g++.dg/ext/attr-no-dangling4.C | 14 +
gcc/testsuite/g++.dg/ext/attr-no-dangling5.C | 31 ++
gcc/testsuite/g++.dg/ext/attr-no-dangling6.C | 65
gcc/testsuite/g++.dg/ext/attr-no-dangling7.C | 31 ++
gcc/testsuite/g++.dg/ext/attr-no-dangling8.C | 30 +
gcc/testsuite/g++.dg/ext/attr-no-dangling9.C | 25
13 files changed, 400 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling1.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling2.C
create mode 100644 gcc/testsuite/g++.dg/ext/attr-no-dangling3.C
create mode 100644
