Re: [PATCH] libstdc++: Don't initialize from *this inside some views [PR97600]

[Jonathan Wakely <jwakely@redhat.com>]

On 30/10/20 11:11 -0400, Patrick Palka via Libstdc++ wrote:
>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 at any point 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 satisaction 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.

Nice solution.

>Tested on x86_64-pc-linux-gnu, and also verified that this fixes the
>testsuite failures when combined with the mentioned frontend patch
>(https://gcc.gnu.org/pipermail/gcc-patches/2020-October/557237.html).
>Does this look OK for trunk?

Yes, thanks.