Re: [PATCH] c++: overeager type completion in convert_to_void [PR111419]
On Mon, 18 Sep 2023, Patrick Palka wrote:
> On Sun, 17 Sep 2023, Jason Merrill wrote:
>
> > On 9/16/23 17:41, Patrick Palka wrote:
> > > On Sat, 16 Sep 2023, Jason Merrill wrote:
> > >
> > > > On 9/15/23 12:03, Patrick Palka wrote:
> > > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK
> > > > > for
> > > > > trunk?
> > > > >
> > > > > -- >8 --
> > > > >
> > > > > Here convert_to_void always completes the type of an INDIRECT_REF or
> > > > > VAR_DECL expression, but according to [expr.context] an
> > > > > lvalue-to-rvalue
> > > > > conversion is applied to a discarded-value expression only if "the
> > > > > expression is a glvalue of volatile-qualified type". This patch
> > > > > restricts
> > > > > convert_to_void's type completion accordingly.
> > > > >
> > > > > PR c++/111419
> > > > >
> > > > > gcc/cp/ChangeLog:
> > > > >
> > > > > * cvt.cc (convert_to_void) : Only call
> > > > > complete_type if the type is volatile and the INDIRECT_REF
> > > > > isn't an implicit one.
> > > >
> > > > Hmm, what does implicit have to do with it? The expression forms listed
> > > > in
> > > > https://eel.is/c++draft/expr.context#2 include "id-expression"...
> > >
> > > When there's an implicit INDIRECT_REF, I reckoned the type of the
> > > id-expression is really a reference type, which can't be cv-qualified?
> >
> > A name can have reference type, but its use as an expression doesn't:
> > https://eel.is/c++draft/expr.type#1.sentence-1
>
> Ah, makes sense..
>
> >
> > > > > diff --git a/gcc/testsuite/g++.dg/expr/discarded1a.C
> > > > > b/gcc/testsuite/g++.dg/expr/discarded1a.C
> > > > > new file mode 100644
> > > > > index 000..5516ff46fe9
> > > > > --- /dev/null
> > > > > +++ b/gcc/testsuite/g++.dg/expr/discarded1a.C
> > > > > @@ -0,0 +1,16 @@
> > > > > +// PR c++/111419
> > > > > +
> > > > > +struct Incomplete;
> > > > > +
> > > > > +template struct Holder { T t; }; // { dg-error
> > > > > "incomplete" }
> > > > > +
> > > > > +extern volatile Holder a;
> > > > > +extern volatile Holder& b;
> > > > > +extern volatile Holder* c;
> > > > > +
> > > > > +int main() {
> > > > > + a; // { dg-message "required from here" }
> > > > > + b; // { dg-warning "implicit dereference will not access object" }
> > > > > + // { dg-bogus "required from here" "" { target *-*-* } .-1 }
> > > >
> > > > ...so it seems to me this line should get the lvalue-rvalue conversion
> > > > (and
> > > > not the warning about no access).
>
> Sounds good, like so? I also added a test to verify such loads don't
> get discarded in the generated code.
On second thought, it seems better to split this into two patches, one
that correctly restricts type completion, and the next that makes us
correctly handle warning/code generation of volatile references.
Patch series incoming...
>
> -- >8 --
>
> Subject: [PATCH] c++: overeager type completion in convert_to_void [PR111419]
>
> Here convert_to_void always completes the type of an indirection or
> id-expression, but according to [expr.context] an lvalue-to-rvalue
> conversion is applied to a discarded-value expression only if "the
> expression is a glvalue of volatile-qualified type". This patch
> restricts convert_to_void's type completion accordingly.
>
> Jason pointed out that implicit loads of volatile references also need
> to undergo lvalue-to-rvalue conversion, but we currently emit a warning
> in this case and discard the load. This patch also changes this behavior
> so that we preserve the load and don't issue a warning.
>
> PR c++/111419
>
> gcc/cp/ChangeLog:
>
> * cvt.cc (convert_to_void) : Only call
> complete_type if the type is volatile. Remove warning for
> implicit indirection of a volatile reference. Simplify as if
> is_reference=false. Check REFERENCE_REF_P in the -Wunused-value
> branch.
> : Only call complete_type if the type is volatile.
>
Re: [PATCH] c++: overeager type completion in convert_to_void [PR111419]
On Sun, 17 Sep 2023, Jason Merrill wrote:
> On 9/16/23 17:41, Patrick Palka wrote:
> > On Sat, 16 Sep 2023, Jason Merrill wrote:
> >
> > > On 9/15/23 12:03, Patrick Palka wrote:
> > > > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > > > trunk?
> > > >
> > > > -- >8 --
> > > >
> > > > Here convert_to_void always completes the type of an INDIRECT_REF or
> > > > VAR_DECL expression, but according to [expr.context] an lvalue-to-rvalue
> > > > conversion is applied to a discarded-value expression only if "the
> > > > expression is a glvalue of volatile-qualified type". This patch
> > > > restricts
> > > > convert_to_void's type completion accordingly.
> > > >
> > > > PR c++/111419
> > > >
> > > > gcc/cp/ChangeLog:
> > > >
> > > > * cvt.cc (convert_to_void) : Only call
> > > > complete_type if the type is volatile and the INDIRECT_REF
> > > > isn't an implicit one.
> > >
> > > Hmm, what does implicit have to do with it? The expression forms listed
> > > in
> > > https://eel.is/c++draft/expr.context#2 include "id-expression"...
> >
> > When there's an implicit INDIRECT_REF, I reckoned the type of the
> > id-expression is really a reference type, which can't be cv-qualified?
>
> A name can have reference type, but its use as an expression doesn't:
> https://eel.is/c++draft/expr.type#1.sentence-1
Ah, makes sense..
>
> > > > diff --git a/gcc/testsuite/g++.dg/expr/discarded1a.C
> > > > b/gcc/testsuite/g++.dg/expr/discarded1a.C
> > > > new file mode 100644
> > > > index 000..5516ff46fe9
> > > > --- /dev/null
> > > > +++ b/gcc/testsuite/g++.dg/expr/discarded1a.C
> > > > @@ -0,0 +1,16 @@
> > > > +// PR c++/111419
> > > > +
> > > > +struct Incomplete;
> > > > +
> > > > +template struct Holder { T t; }; // { dg-error
> > > > "incomplete" }
> > > > +
> > > > +extern volatile Holder a;
> > > > +extern volatile Holder& b;
> > > > +extern volatile Holder* c;
> > > > +
> > > > +int main() {
> > > > + a; // { dg-message "required from here" }
> > > > + b; // { dg-warning "implicit dereference will not access object" }
> > > > + // { dg-bogus "required from here" "" { target *-*-* } .-1 }
> > >
> > > ...so it seems to me this line should get the lvalue-rvalue conversion
> > > (and
> > > not the warning about no access).
Sounds good, like so? I also added a test to verify such loads don't
get discarded in the generated code.
-- >8 --
Subject: [PATCH] c++: overeager type completion in convert_to_void [PR111419]
Here convert_to_void always completes the type of an indirection or
id-expression, but according to [expr.context] an lvalue-to-rvalue
conversion is applied to a discarded-value expression only if "the
expression is a glvalue of volatile-qualified type". This patch
restricts convert_to_void's type completion accordingly.
Jason pointed out that implicit loads of volatile references also need
to undergo lvalue-to-rvalue conversion, but we currently emit a warning
in this case and discard the load. This patch also changes this behavior
so that we preserve the load and don't issue a warning.
PR c++/111419
gcc/cp/ChangeLog:
* cvt.cc (convert_to_void) : Only call
complete_type if the type is volatile. Remove warning for
implicit indirection of a volatile reference. Simplify as if
is_reference=false. Check REFERENCE_REF_P in the -Wunused-value
branch.
: Only call complete_type if the type is volatile.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/concepts-requires36.C: New test.
* g++.dg/expr/discarded1.C: New test.
* g++.dg/expr/discarded1a.C: New test.
* g++.dg/expr/volatile2.C: New test.
* g++.old-deja/g++.bugs/900428_01.C: No longer expect
warnings for implicit loads of volatile references.
---
gcc/cp/cvt.cc | 60 +++
.../g++.dg/cpp2a/concepts-requires36.C| 16 +
gcc/testsuite/g++.dg/expr/discarded1.C| 15 +
gcc/testsuite/g++.dg/expr/discarded1a.C | 16 +
gcc/testsuite/g++.dg/expr/volatile2.C | 11
.../g++.old-deja/g++.bugs/900428_01.C | 26
6 files changed, 79 insertions(+), 65 deletions(-)
create mode 100644
Re: [PATCH] c++: overeager type completion in convert_to_void [PR111419]
On 9/16/23 17:41, Patrick Palka wrote:
On Sat, 16 Sep 2023, Jason Merrill wrote:
On 9/15/23 12:03, Patrick Palka wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
-- >8 --
Here convert_to_void always completes the type of an INDIRECT_REF or
VAR_DECL expression, but according to [expr.context] an lvalue-to-rvalue
conversion is applied to a discarded-value expression only if "the
expression is a glvalue of volatile-qualified type". This patch restricts
convert_to_void's type completion accordingly.
PR c++/111419
gcc/cp/ChangeLog:
* cvt.cc (convert_to_void) : Only call
complete_type if the type is volatile and the INDIRECT_REF
isn't an implicit one.
Hmm, what does implicit have to do with it? The expression forms listed in
https://eel.is/c++draft/expr.context#2 include "id-expression"...
When there's an implicit INDIRECT_REF, I reckoned the type of the
id-expression is really a reference type, which can't be cv-qualified?
A name can have reference type, but its use as an expression doesn't:
https://eel.is/c++draft/expr.type#1.sentence-1
diff --git a/gcc/testsuite/g++.dg/expr/discarded1a.C
b/gcc/testsuite/g++.dg/expr/discarded1a.C
new file mode 100644
index 000..5516ff46fe9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/discarded1a.C
@@ -0,0 +1,16 @@
+// PR c++/111419
+
+struct Incomplete;
+
+template struct Holder { T t; }; // { dg-error "incomplete" }
+
+extern volatile Holder a;
+extern volatile Holder& b;
+extern volatile Holder* c;
+
+int main() {
+ a; // { dg-message "required from here" }
+ b; // { dg-warning "implicit dereference will not access object" }
+ // { dg-bogus "required from here" "" { target *-*-* } .-1 }
...so it seems to me this line should get the lvalue-rvalue conversion (and
not the warning about no access).
+ *c; // { dg-message "required from here" }
+}
Re: [PATCH] c++: overeager type completion in convert_to_void [PR111419]
On Sat, 16 Sep 2023, Jason Merrill wrote:
> On 9/15/23 12:03, Patrick Palka wrote:
> > Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
> > trunk?
> >
> > -- >8 --
> >
> > Here convert_to_void always completes the type of an INDIRECT_REF or
> > VAR_DECL expression, but according to [expr.context] an lvalue-to-rvalue
> > conversion is applied to a discarded-value expression only if "the
> > expression is a glvalue of volatile-qualified type". This patch restricts
> > convert_to_void's type completion accordingly.
> >
> > PR c++/111419
> >
> > gcc/cp/ChangeLog:
> >
> > * cvt.cc (convert_to_void) : Only call
> > complete_type if the type is volatile and the INDIRECT_REF
> > isn't an implicit one.
>
> Hmm, what does implicit have to do with it? The expression forms listed in
> https://eel.is/c++draft/expr.context#2 include "id-expression"...
When there's an implicit INDIRECT_REF, I reckoned the type of the
id-expression is really a reference type, which can't be cv-qualified?
>
> > diff --git a/gcc/testsuite/g++.dg/expr/discarded1a.C
> > b/gcc/testsuite/g++.dg/expr/discarded1a.C
> > new file mode 100644
> > index 000..5516ff46fe9
> > --- /dev/null
> > +++ b/gcc/testsuite/g++.dg/expr/discarded1a.C
> > @@ -0,0 +1,16 @@
> > +// PR c++/111419
> > +
> > +struct Incomplete;
> > +
> > +template struct Holder { T t; }; // { dg-error "incomplete" }
> > +
> > +extern volatile Holder a;
> > +extern volatile Holder& b;
> > +extern volatile Holder* c;
> > +
> > +int main() {
> > + a; // { dg-message "required from here" }
> > + b; // { dg-warning "implicit dereference will not access object" }
> > + // { dg-bogus "required from here" "" { target *-*-* } .-1 }
>
> ...so it seems to me this line should get the lvalue-rvalue conversion (and
> not the warning about no access).
>
> > + *c; // { dg-message "required from here" }
> > +}
>
>
Re: [PATCH] c++: overeager type completion in convert_to_void [PR111419]
On 9/15/23 12:03, Patrick Palka wrote:
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
-- >8 --
Here convert_to_void always completes the type of an INDIRECT_REF or
VAR_DECL expression, but according to [expr.context] an lvalue-to-rvalue
conversion is applied to a discarded-value expression only if "the
expression is a glvalue of volatile-qualified type". This patch restricts
convert_to_void's type completion accordingly.
PR c++/111419
gcc/cp/ChangeLog:
* cvt.cc (convert_to_void) : Only call
complete_type if the type is volatile and the INDIRECT_REF
isn't an implicit one.
Hmm, what does implicit have to do with it? The expression forms listed
in https://eel.is/c++draft/expr.context#2 include "id-expression"...
diff --git a/gcc/testsuite/g++.dg/expr/discarded1a.C
b/gcc/testsuite/g++.dg/expr/discarded1a.C
new file mode 100644
index 000..5516ff46fe9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/discarded1a.C
@@ -0,0 +1,16 @@
+// PR c++/111419
+
+struct Incomplete;
+
+template struct Holder { T t; }; // { dg-error "incomplete" }
+
+extern volatile Holder a;
+extern volatile Holder& b;
+extern volatile Holder* c;
+
+int main() {
+ a; // { dg-message "required from here" }
+ b; // { dg-warning "implicit dereference will not access object" }
+ // { dg-bogus "required from here" "" { target *-*-* } .-1 }
...so it seems to me this line should get the lvalue-rvalue conversion
(and not the warning about no access).
+ *c; // { dg-message "required from here" }
+}
[PATCH] c++: overeager type completion in convert_to_void [PR111419]
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
-- >8 --
Here convert_to_void always completes the type of an INDIRECT_REF or
VAR_DECL expression, but according to [expr.context] an lvalue-to-rvalue
conversion is applied to a discarded-value expression only if "the
expression is a glvalue of volatile-qualified type". This patch restricts
convert_to_void's type completion accordingly.
PR c++/111419
gcc/cp/ChangeLog:
* cvt.cc (convert_to_void) : Only call
complete_type if the type is volatile and the INDIRECT_REF
isn't an implicit one.
: Only call complete_type if the type is
volatile.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/concepts-requires36.C: New test.
* g++.dg/expr/discarded1.C: New test.
* g++.dg/expr/discarded1a.C: New test.
---
gcc/cp/cvt.cc| 11 +++
gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C | 16
gcc/testsuite/g++.dg/expr/discarded1.C | 15 +++
gcc/testsuite/g++.dg/expr/discarded1a.C | 16
4 files changed, 54 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
create mode 100644 gcc/testsuite/g++.dg/expr/discarded1.C
create mode 100644 gcc/testsuite/g++.dg/expr/discarded1a.C
diff --git a/gcc/cp/cvt.cc b/gcc/cp/cvt.cc
index c6b52f07050..5e2c01476e3 100644
--- a/gcc/cp/cvt.cc
+++ b/gcc/cp/cvt.cc
@@ -1252,10 +1252,12 @@ convert_to_void (tree expr, impl_conv_void implicit,
tsubst_flags_t complain)
tree type = TREE_TYPE (expr);
int is_reference = TYPE_REF_P (TREE_TYPE (TREE_OPERAND (expr, 0)));
int is_volatile = TYPE_VOLATILE (type);
- int is_complete = COMPLETE_TYPE_P (complete_type (type));
+ if (is_volatile && !is_reference)
+ complete_type (type);
+ int is_complete = COMPLETE_TYPE_P (type);
/* Can't load the value if we don't know the type. */
- if (is_volatile && !is_complete)
+ if (is_volatile && !is_reference && !is_complete)
{
if (complain & tf_warning)
switch (implicit)
@@ -1412,9 +1414,10 @@ convert_to_void (tree expr, impl_conv_void implicit,
tsubst_flags_t complain)
{
/* External variables might be incomplete. */
tree type = TREE_TYPE (expr);
- int is_complete = COMPLETE_TYPE_P (complete_type (type));
- if (TYPE_VOLATILE (type) && !is_complete && (complain & tf_warning))
+ if (TYPE_VOLATILE (type)
+ && !COMPLETE_TYPE_P (complete_type (type))
+ && (complain & tf_warning))
switch (implicit)
{
case ICV_CAST:
diff --git a/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
b/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
new file mode 100644
index 000..8d3a4fcd2aa
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp2a/concepts-requires36.C
@@ -0,0 +1,16 @@
+// PR c++/111419
+// { dg-do compile { target c++20 } }
+
+template
+concept invocable = requires(F& f) { f(); };
+
+template
+concept deref_invocable = requires(F& f) { *f(); };
+
+struct Incomplete;
+
+template
+struct Holder { T t; };
+
+static_assert(invocable& ()>);
+static_assert(deref_invocable* ()>);
diff --git a/gcc/testsuite/g++.dg/expr/discarded1.C
b/gcc/testsuite/g++.dg/expr/discarded1.C
new file mode 100644
index 000..c0c22e9e95b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/discarded1.C
@@ -0,0 +1,15 @@
+// PR c++/111419
+
+struct Incomplete;
+
+template struct Holder { T t; }; // { dg-bogus "incomplete" }
+
+extern Holder a;
+extern Holder& b;
+extern Holder* c;
+
+int main() {
+ a;
+ b;
+ *c;
+}
diff --git a/gcc/testsuite/g++.dg/expr/discarded1a.C
b/gcc/testsuite/g++.dg/expr/discarded1a.C
new file mode 100644
index 000..5516ff46fe9
--- /dev/null
+++ b/gcc/testsuite/g++.dg/expr/discarded1a.C
@@ -0,0 +1,16 @@
+// PR c++/111419
+
+struct Incomplete;
+
+template struct Holder { T t; }; // { dg-error "incomplete" }
+
+extern volatile Holder a;
+extern volatile Holder& b;
+extern volatile Holder* c;
+
+int main() {
+ a; // { dg-message "required from here" }
+ b; // { dg-warning "implicit dereference will not access object" }
+ // { dg-bogus "required from here" "" { target *-*-* } .-1 }
+ *c; // { dg-message "required from here" }
+}
--
2.42.0.158.g94e83dcf5b
