public inbox for gcc-bugs@sourceware.org
help / color / mirror / Atom feed
* [Bug libstdc++/97600] New: [ranges] result of static assertion depends on unrelated statement
@ 2020-10-27 18:45 ppalka at gcc dot gnu.org
2020-10-27 19:22 ` [Bug libstdc++/97600] " ppalka at gcc dot gnu.org
` (6 more replies)
0 siblings, 7 replies; 8+ messages in thread
From: ppalka at gcc dot gnu.org @ 2020-10-27 18:45 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97600
Bug ID: 97600
Summary: [ranges] result of static assertion depends on
unrelated statement
Product: gcc
Version: 11.0
Status: UNCONFIRMED
Severity: normal
Priority: P3
Component: libstdc++
Assignee: unassigned at gcc dot gnu.org
Reporter: ppalka at gcc dot gnu.org
Target Milestone: ---
$ cat testcase.C
$#include <sstream>
#include <ranges>
int main() {
using namespace std::ranges;
basic_istream_view<int, char, std::char_traits<char>> v;
v.begin();
static_assert(range<decltype(v)>);
}
$ g++ -std=c++20 testcase.C
testcase.C:8:17: error: static assertion failed
8 | static_assert(range<decltype(v)>);
| ^~~~~~~~~~~~~~~~~~
Unexpectedly, this static assert fails. Even more unexpectedly, if we remove
the 'v.begin()' line before it, then the static assert succeeds.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/97600] [ranges] result of static assertion depends on unrelated statement
2020-10-27 18:45 [Bug libstdc++/97600] New: [ranges] result of static assertion depends on unrelated statement ppalka at gcc dot gnu.org
@ 2020-10-27 19:22 ` ppalka at gcc dot gnu.org
2020-10-27 19:39 ` ppalka at gcc dot gnu.org
` (5 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: ppalka at gcc dot gnu.org @ 2020-10-27 19:22 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97600
Patrick Palka <ppalka at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Known to fail| |10.2.0, 11.0
--- Comment #1 from Patrick Palka <ppalka at gcc dot gnu.org> ---
This happens because of the following chain of events:
1. Calling basic_istream_view::begin() entails instantiating its definition to
perform return type deduction.
2. We perform overload resolution of the direct-initialization _Iterator{*this}
inside its definition.
3. As part of overload resolution, we consider the copy ctor _Iterator(const
_Iterator&).
4. We try to find an implicit conversion sequence from basic_istream_view to
const _Iterator&.
5. We consider a conversion sequence that goes through the conversion function
view_interface::operator bool().
6. We check the conversion function's constraint 'requires {
ranges::empty(_M_derived()); }'.
7. ranges::empty() checks that view_interface::empty() is callable and
therefore has satisfied constraints
8. view_interface::empty() requires range<basic_istream_view>, which requires
that basic_istream_view::begin() is callable, which requires that we deduce its
return type. But we're already in the the middle of deducing it, so we hit a
SFINAE error, which causes satisfaction of range<basic_istream_view> to fail.
9. We cache this negative satisfaction result for (the atomic constraint of)
range<basic_istream_view>.
10. We rule out the copy ctor during overload resolution and eventually resolve
the direct-initialization in the expected way.
When we later evaluate the static assert, we just return the cached
satisfaction result.
I have an experimental patch here
https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557117.html that makes
us rule out the conversion function in step 5 sooner without first checking its
constraints, which would be one way to fix this.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/97600] [ranges] result of static assertion depends on unrelated statement
2020-10-27 18:45 [Bug libstdc++/97600] New: [ranges] result of static assertion depends on unrelated statement ppalka at gcc dot gnu.org
2020-10-27 19:22 ` [Bug libstdc++/97600] " ppalka at gcc dot gnu.org
@ 2020-10-27 19:39 ` ppalka at gcc dot gnu.org
2020-10-28 16:59 ` [Bug libstdc++/97600] [ranges] satisfaction value of range<basic_istream_view> affected by prior use of basic_istream_view::begin() ppalka at gcc dot gnu.org
` (4 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: ppalka at gcc dot gnu.org @ 2020-10-27 19:39 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97600
--- Comment #2 from Patrick Palka <ppalka at gcc dot gnu.org> ---
I think all views whose begin()/end() has a placeholder return type and
performs direct initialization of a _Iterator/_Sentinel from *this are affected
by some variant of this issue.
A library-side workaround would be to templatize view_interface::operator
bool() like so:
diff --git a/libstdc++-v3/include/bits/ranges_util.h
b/libstdc++-v3/include/bits/ranges_util.h
index cc50e2ad4e4..8563a5dfcb0 100644
--- a/libstdc++-v3/include/bits/ranges_util.h
+++ b/libstdc++-v3/include/bits/ranges_util.h
@@ -86,13 +86,15 @@ namespace ranges
empty() const requires forward_range<const _Derived>
{ return ranges::begin(_M_derived()) == ranges::end(_M_derived()); }
- constexpr explicit
- operator bool() requires requires { ranges::empty(_M_derived()); }
- { return !ranges::empty(_M_derived()); }
-
- constexpr explicit
- operator bool() const requires requires { ranges::empty(_M_derived()); }
- { return !ranges::empty(_M_derived()); }
+ template<same_as<bool> _Tp>
+ constexpr explicit
+ operator _Tp() requires requires { ranges::empty(_M_derived()); }
+ { return !ranges::empty(_M_derived()); }
+
+ template<same_as<bool> _Tp>
+ constexpr explicit
+ operator _Tp() const requires requires { ranges::empty(_M_derived()); }
+ { return !ranges::empty(_M_derived()); }
constexpr auto
data() requires contiguous_iterator<iterator_t<_Derived>>
So that when we considering this conversion function in step 5, we deduce
_Tp=_Iterator and short circuit satisfaction of the conversion function's
associated constraints in step 6.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/97600] [ranges] satisfaction value of range<basic_istream_view> affected by prior use of basic_istream_view::begin()
2020-10-27 18:45 [Bug libstdc++/97600] New: [ranges] result of static assertion depends on unrelated statement ppalka at gcc dot gnu.org
2020-10-27 19:22 ` [Bug libstdc++/97600] " ppalka at gcc dot gnu.org
2020-10-27 19:39 ` ppalka at gcc dot gnu.org
@ 2020-10-28 16:59 ` ppalka at gcc dot gnu.org
2020-10-31 0:34 ` cvs-commit at gcc dot gnu.org
` (3 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: ppalka at gcc dot gnu.org @ 2020-10-28 16:59 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97600
Patrick Palka <ppalka at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Assignee|unassigned at gcc dot gnu.org |ppalka at gcc dot gnu.org
Last reconfirmed| |2020-10-28
Status|UNCONFIRMED |ASSIGNED
Ever confirmed|0 |1
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/97600] [ranges] satisfaction value of range<basic_istream_view> affected by prior use of basic_istream_view::begin()
2020-10-27 18:45 [Bug libstdc++/97600] New: [ranges] result of static assertion depends on unrelated statement ppalka at gcc dot gnu.org
` (2 preceding siblings ...)
2020-10-28 16:59 ` [Bug libstdc++/97600] [ranges] satisfaction value of range<basic_istream_view> affected by prior use of basic_istream_view::begin() ppalka at gcc dot gnu.org
@ 2020-10-31 0:34 ` cvs-commit at gcc dot gnu.org
2020-12-11 20:41 ` cvs-commit at gcc dot gnu.org
` (2 subsequent siblings)
6 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-10-31 0:34 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97600
--- Comment #3 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Patrick Palka <ppalka@gcc.gnu.org>:
https://gcc.gnu.org/g:afb8da7faa9dfe5a0d94ed45a373d74c076784ab
commit r11-4584-gafb8da7faa9dfe5a0d94ed45a373d74c076784ab
Author: Patrick Palka <ppalka@redhat.com>
Date: Fri Oct 30 20:33:19 2020 -0400
libstdc++: Don't initialize from *this inside some views [PR97600]
This works around a subtle issue where instantiating the begin()/end()
member of some views (as part of return type deduction) inadvertently
requires computing the satisfaction value of range<foo_view>.
This is problematic because the constraint range<foo_view> requires the
begin()/end() member to be callable. But it's not callable until we've
deduced its return type, so evaluation of range<foo_view> yields false
at this point. And if after both members are instantiated (and their
return types deduced) we evaluate range<foo_view> again, this time it
will yield true since the begin()/end() members are now both callable.
This makes the program ill-formed according to [temp.constr.atomic]/3:
If, at different points in the program, the satisfaction result is
different for identical atomic constraints and template arguments, the
program is ill-formed, no diagnostic required.
The views affected by this issue are those whose begin()/end() member
has a placeholder return type and that member initializes an _Iterator
or _Sentinel object from a reference to *this. The second condition is
relevant because it means explicit conversion functions are considered
during overload resolution (as per [over.match.copy], I think), and
therefore it causes g++ to check the constraints of the conversion
function view_interface<foo_view>::operator bool(). And this conversion
function's constraints indirectly require range<foo_view>.
This issue is observable on trunk only with basic_istream_view (as in
the testcase in the PR). But a pending patch that makes g++ memoize
constraint satisfaction values indefinitely (it currently invalidates
the satisfaction cache on various events) causes many existing tests for
the other affected views to fail, because range<foo_view> then remains
false for the whole compilation.
This patch works around this issue by adjusting the constructors of the
_Iterator and _Sentinel types of the affected views to take their
foo_view argument by pointer instead of by reference, so that g++ no
longer considers explicit conversion functions when resolving the
direct-initialization inside these views' begin()/end() members.
libstdc++-v3/ChangeLog:
PR libstdc++/97600
* include/std/ranges (basic_istream_view::begin): Initialize
_Iterator from 'this' instead of '*this'.
(basic_istream_view::_Iterator::_Iterator): Adjust constructor
accordingly.
(filter_view::_Iterator::_Iterator): Take a filter_view*
argument instead of a filter_view& argument.
(filter_view::_Sentinel::_Sentinel): Likewise.
(filter_view::begin): Initialize _Iterator from 'this' instead
of '*this'.
(filter_view::end): Likewise.
(transform_view::_Iterator::_Iterator): Take a _Parent* instead
of a _Parent&.
(filter_view::_Iterator::operator+): Adjust accordingly.
(filter_view::_Iterator::operator-): Likewise.
(filter_view::begin): Initialize _Iterator from 'this' instead
of '*this'.
(filter_view::end): Likewise.
(join_view::_Iterator): Take a _Parent* instead of a _Parent&.
(join_view::_Sentinel): Likewise.
(join_view::begin): Initialize _Iterator from 'this' instead of
'*this'.
(join_view::end): Initialize _Sentinel from 'this' instead of
'*this'.
(split_view::_OuterIter): Take a _Parent& instead of a _Parent*.
(split_view::begin): Initialize _OuterIter from 'this' instead
of '*this'.
(split_view::end): Likewise.
* testsuite/std/ranges/97600.cc: New test.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/97600] [ranges] satisfaction value of range<basic_istream_view> affected by prior use of basic_istream_view::begin()
2020-10-27 18:45 [Bug libstdc++/97600] New: [ranges] result of static assertion depends on unrelated statement ppalka at gcc dot gnu.org
` (3 preceding siblings ...)
2020-10-31 0:34 ` cvs-commit at gcc dot gnu.org
@ 2020-12-11 20:41 ` cvs-commit at gcc dot gnu.org
2021-04-21 3:27 ` cvs-commit at gcc dot gnu.org
2021-04-21 14:18 ` ppalka at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2020-12-11 20:41 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97600
--- Comment #4 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The master branch has been updated by Jason Merrill <jason@gcc.gnu.org>:
https://gcc.gnu.org/g:9324f7a25c7161a813bfae6cc2d180784b165740
commit r11-5954-g9324f7a25c7161a813bfae6cc2d180784b165740
Author: Jason Merrill <jason@redhat.com>
Date: Fri Dec 11 14:37:09 2020 -0500
c++: Avoid considering some conversion ops [PR97600]
Patrick's earlier patch to check convertibility before constraints for
conversion ops wasn't suitable because checking convertibility can also
lead
to unwanted instantiations, but it occurs to me that there's a smaller
check
we can do to avoid doing normal consideration of the conversion ops in this
case: since we're in the middle of a user-defined conversion, we can
exclude
from consideration any conversion ops that return a type that would need an
additional user-defined conversion to reach the desired type: namely, a
type
that differs in class-ness from the desired type.
[temp.inst]/9 allows optimizations like this: "If the function selected by
overload resolution can be determined without instantiating a class
template
definition, it is unspecified whether that instantiation actually takes
place."
gcc/cp/ChangeLog:
PR libstdc++/97600
* call.c (build_user_type_conversion_1): Avoid considering
conversion functions that return a clearly unsuitable type.
gcc/testsuite/ChangeLog:
* g++.dg/cpp2a/concepts-conv3.C: New test.
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/97600] [ranges] satisfaction value of range<basic_istream_view> affected by prior use of basic_istream_view::begin()
2020-10-27 18:45 [Bug libstdc++/97600] New: [ranges] result of static assertion depends on unrelated statement ppalka at gcc dot gnu.org
` (4 preceding siblings ...)
2020-12-11 20:41 ` cvs-commit at gcc dot gnu.org
@ 2021-04-21 3:27 ` cvs-commit at gcc dot gnu.org
2021-04-21 14:18 ` ppalka at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: cvs-commit at gcc dot gnu.org @ 2021-04-21 3:27 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97600
--- Comment #5 from CVS Commits <cvs-commit at gcc dot gnu.org> ---
The releases/gcc-10 branch has been updated by Patrick Palka
<ppalka@gcc.gnu.org>:
https://gcc.gnu.org/g:3d6bba85e1dd6cb7e213a7e6c060b9c8a0a346e2
commit r10-9737-g3d6bba85e1dd6cb7e213a7e6c060b9c8a0a346e2
Author: Patrick Palka <ppalka@redhat.com>
Date: Fri Oct 30 20:33:19 2020 -0400
libstdc++: Don't initialize from *this inside some views [PR97600]
This works around a subtle issue where instantiating the begin()/end()
member of some views (as part of return type deduction) inadvertently
requires computing the satisfaction value of range<foo_view>.
This is problematic because the constraint range<foo_view> requires the
begin()/end() member to be callable. But it's not callable until we've
deduced its return type, so evaluation of range<foo_view> yields false
at this point. And if after both members are instantiated (and their
return types deduced) we evaluate range<foo_view> again, this time it
will yield true since the begin()/end() members are now both callable.
This makes the program ill-formed according to [temp.constr.atomic]/3:
If, at different points in the program, the satisfaction result is
different for identical atomic constraints and template arguments, the
program is ill-formed, no diagnostic required.
The views affected by this issue are those whose begin()/end() member
has a placeholder return type and that member initializes an _Iterator
or _Sentinel object from a reference to *this. The second condition is
relevant because it means explicit conversion functions are considered
during overload resolution (as per [over.match.copy], I think), and
therefore it causes g++ to check the constraints of the conversion
function view_interface<foo_view>::operator bool(). And this conversion
function's constraints indirectly require range<foo_view>.
This issue is observable on trunk only with basic_istream_view (as in
the testcase in the PR). But a pending patch that makes g++ memoize
constraint satisfaction values indefinitely (it currently invalidates
the satisfaction cache on various events) causes many existing tests for
the other affected views to fail, because range<foo_view> then remains
false for the whole compilation.
This patch works around this issue by adjusting the constructors of the
_Iterator and _Sentinel types of the affected views to take their
foo_view argument by pointer instead of by reference, so that g++ no
longer considers explicit conversion functions when resolving the
direct-initialization inside these views' begin()/end() members.
libstdc++-v3/ChangeLog:
PR libstdc++/97600
* include/std/ranges (basic_istream_view::begin): Initialize
_Iterator from 'this' instead of '*this'.
(basic_istream_view::_Iterator::_Iterator): Adjust constructor
accordingly.
(filter_view::_Iterator::_Iterator): Take a filter_view*
argument instead of a filter_view& argument.
(filter_view::_Sentinel::_Sentinel): Likewise.
(filter_view::begin): Initialize _Iterator from 'this' instead
of '*this'.
(filter_view::end): Likewise.
(transform_view::_Iterator::_Iterator): Take a _Parent* instead
of a _Parent&.
(filter_view::_Iterator::operator+): Adjust accordingly.
(filter_view::_Iterator::operator-): Likewise.
(filter_view::begin): Initialize _Iterator from 'this' instead
of '*this'.
(filter_view::end): Likewise.
(join_view::_Iterator): Take a _Parent* instead of a _Parent&.
(join_view::_Sentinel): Likewise.
(join_view::begin): Initialize _Iterator from 'this' instead of
'*this'.
(join_view::end): Initialize _Sentinel from 'this' instead of
'*this'.
(split_view::_OuterIter): Take a _Parent& instead of a _Parent*.
(split_view::begin): Initialize _OuterIter from 'this' instead
of '*this'.
(split_view::end): Likewise.
* testsuite/std/ranges/97600.cc: New test.
(cherry picked from commit afb8da7faa9dfe5a0d94ed45a373d74c076784ab)
^ permalink raw reply [flat|nested] 8+ messages in thread
* [Bug libstdc++/97600] [ranges] satisfaction value of range<basic_istream_view> affected by prior use of basic_istream_view::begin()
2020-10-27 18:45 [Bug libstdc++/97600] New: [ranges] result of static assertion depends on unrelated statement ppalka at gcc dot gnu.org
` (5 preceding siblings ...)
2021-04-21 3:27 ` cvs-commit at gcc dot gnu.org
@ 2021-04-21 14:18 ` ppalka at gcc dot gnu.org
6 siblings, 0 replies; 8+ messages in thread
From: ppalka at gcc dot gnu.org @ 2021-04-21 14:18 UTC (permalink / raw)
To: gcc-bugs
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=97600
Patrick Palka <ppalka at gcc dot gnu.org> changed:
What |Removed |Added
----------------------------------------------------------------------------
Status|ASSIGNED |RESOLVED
Target Milestone|--- |10.4
Resolution|--- |FIXED
--- Comment #6 from Patrick Palka <ppalka at gcc dot gnu.org> ---
The library side workaround is in place for GCC 10.4 and 11, and GCC 11
additionally has a frontend optimization for ruling out unviable conversion ops
sooner which alone is sufficient to fix the PR. I suppose we could revert the
library-side workaround for GCC 11/12 then, but it seems harmless to keep
around.
^ permalink raw reply [flat|nested] 8+ messages in thread
end of thread, other threads:[~2021-04-21 14:18 UTC | newest]
Thread overview: 8+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-10-27 18:45 [Bug libstdc++/97600] New: [ranges] result of static assertion depends on unrelated statement ppalka at gcc dot gnu.org
2020-10-27 19:22 ` [Bug libstdc++/97600] " ppalka at gcc dot gnu.org
2020-10-27 19:39 ` ppalka at gcc dot gnu.org
2020-10-28 16:59 ` [Bug libstdc++/97600] [ranges] satisfaction value of range<basic_istream_view> affected by prior use of basic_istream_view::begin() ppalka at gcc dot gnu.org
2020-10-31 0:34 ` cvs-commit at gcc dot gnu.org
2020-12-11 20:41 ` cvs-commit at gcc dot gnu.org
2021-04-21 3:27 ` cvs-commit at gcc dot gnu.org
2021-04-21 14:18 ` ppalka at gcc dot gnu.org
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).