Skip to content

[lib] Change 'expression inside noexcept' to 'exception specification' #4105

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Feb 1, 2021

Conversation

JohelEGP
Copy link
Contributor

@JohelEGP JohelEGP commented Aug 1, 2020

https://wg21.link/[stringbuf.assign]#5 doesn't use a code block. Is that OK?

@jwakely
Copy link
Member

jwakely commented Aug 1, 2020

Personally, I don't like this language at all, neither "in" nor "inside". The expression is not "inside" noexcept. We should use language consistent with the core wording or the grammar.

[except.spec] says "that constant expression is the exception specification of the function type in which the noexcept-specifier appears" (emphasis mine).

So we could just say "the exception specification is equivalent to..."

Or we could define a new element which defines the see below expression, and do away with "the expression inside noexcept is equivalent to" entirely.

Others might not agree though, so don't rewrite everything based on this comment alone.

@jensmaurer
Copy link
Member

There are several different changes here, and I'd like to see them separated a bit more.

  • There is the question of phrasing for "the expression inside", which applies (in principle) to both noexcept and explicit. This needs a wider audience to decide.
  • There are typo-level fixes (logop, colons) that could be applied right away.

@JohelEGP , please post a separate pull request that only contains separate commits for the "logop" fixes and the colon fixes.

@JohelEGP JohelEGP force-pushed the expr-in-x-equivalent-to branch from b2681d7 to aa71d31 Compare August 2, 2020 17:53
@zygoloid zygoloid added this to the C++23 milestone Sep 18, 2020
@zygoloid zygoloid added the decision-required A decision of the editorial group (or the Project Editor) is required. label Oct 18, 2020
@zygoloid
Copy link
Member

We should pick our preferred wording for this construct and apply it uniformly everywhere.

@jensmaurer
Copy link
Member

Editorial meeting: Say "the /constant-expression/ of the /noexcept-specifier/ is equivalent to" and see how it feels.

@jensmaurer jensmaurer removed the decision-required A decision of the editorial group (or the Project Editor) is required. label Dec 4, 2020
@JohelEGP JohelEGP force-pushed the expr-in-x-equivalent-to branch from aa71d31 to 3458a6f Compare December 29, 2020 05:34
@JohelEGP
Copy link
Contributor Author

Will these two be enough to gauge how it feels?

@jensmaurer jensmaurer requested a review from jwakely December 29, 2020 08:04
@jensmaurer jensmaurer added the decision-required A decision of the editorial group (or the Project Editor) is required. label Dec 29, 2020
@JohelEGP
Copy link
Contributor Author

JohelEGP commented Dec 29, 2020

For requires see below, we have:

https://timsong-cpp.github.io/cppwp/reverse.iterators

    constexpr pointer   operator->() const requires see below;
constexpr pointer operator->() const
  requires (is_pointer_v<Iterator> ||
            requires (const Iterator i) { i.operator->(); });

https://timsong-cpp.github.io/cppwp/iterators.common

    decltype(auto) operator->() const
      requires see below;
decltype(auto) operator->() const
  requires see below;

The expression in the requires-clause is equivalent to:

indirectly_­readable<const I> &&
(requires(const I& i) { i.operator->(); } ||
 is_reference_v<iter_reference_t<I>> ||
 constructible_­from<iter_value_t<I>, iter_reference_t<I>>)

No Remarks:.


https://timsong-cpp.github.io/cppwp/#ranges

  template<movable Val, class CharT, class Traits = char_traits<CharT>>
    requires see below
  class basic_istream_view;

  template<movable Val, class CharT, class Traits>
    requires default_initializable<Val> &&
             stream-extractable<Val, CharT, Traits>
  class basic_istream_view : public view_interface<basic_istream_view<Val, CharT, Traits>> {

  template<input_range V, size_t N>
    requires see below;
  class elements_view;

  template<input_range V, size_t N>
    requires view<V> && has-tuple-element<range_value_t<V>, N> &&
             has-tuple-element<remove_reference_t<range_reference_t<V>>, N>
  class elements_view : public view_interface<elements_view<V, N>> {

These use exposition-only concepts defined before the class template definition.


https://timsong-cpp.github.io/cppwp/range.iota

    constexpr auto size() const requires see below;
constexpr auto size() const requires see below;

Remarks: The expression in the requires-clause is equivalent to

(same_as<W, Bound> && advanceable<W>) || (integral<W> && integral<Bound>) ||
  sized_sentinel_for<Bound, W>

https://timsong-cpp.github.io/cppwp/range.ref.view

    template<not-same-as<ref_view> T>
      requires see below
    constexpr ref_view(T&& t);
template<not-same-as<ref_view> T>
  requires see below
constexpr ref_view(T&& t);

Remarks: Let FUN denote the exposition-only functions

void FUN(R&);
void FUN(R&&) = delete;

The expression in the requires-clause is equivalent to

convertible_to<T, R&> && requires { FUN(declval<T>()); }

https://timsong-cpp.github.io/cppwp/#time

    template<class Rep1, class Period1, class Rep2, class Period2>
      requires see below
      constexpr auto operator<=>(const duration<Rep1, Period1>& lhs,
                                 const duration<Rep2, Period2>& rhs);
template<class Rep1, class Period1, class Rep2, class Period2>
    requires three_­way_­comparable<typename CT::rep>
  constexpr auto operator<=>(const duration<Rep1, Period1>& lhs,
                             const duration<Rep2, Period2>& rhs);

CT is defined in the subclause of the itemdecl.


Consider how the iota_view case would look like with the resolution of the editorial meeting, if applied for consistency:

constexpr auto size() const requires see below;
-Remarks: The expression in the requires-clause is equivalent to
+Remarks: The _constraint-logical-or-expression_ of the _requires-clause_
+is equivalent to:
(same_as<W, Bound> && advanceable<W>) || (integral<W> && integral<Bound>) ||
  sized_sentinel_for<Bound, W>

@jensmaurer jensmaurer closed this Jan 14, 2021
@jensmaurer jensmaurer reopened this Jan 14, 2021
@JohelEGP JohelEGP force-pushed the expr-in-x-equivalent-to branch from 3458a6f to cb543c7 Compare January 14, 2021 19:43
@JohelEGP
Copy link
Contributor Author

1610653348
1610653503

@jensmaurer
Copy link
Member

Editorial meeting: We've reconsidered. Please try "The exception specification is equivalent to:"

@jensmaurer jensmaurer added changes requested Changes to the wording or approach have been requested and not yet applied. and removed decision-required A decision of the editorial group (or the Project Editor) is required. labels Jan 29, 2021
@JohelEGP
Copy link
Contributor Author

Still gauging how it feels, or should I fix it generally now?
1611942239

@JohelEGP JohelEGP force-pushed the expr-in-x-equivalent-to branch from cb543c7 to b075f81 Compare January 29, 2021 17:44
@jensmaurer jensmaurer added decision-required A decision of the editorial group (or the Project Editor) is required. and removed changes requested Changes to the wording or approach have been requested and not yet applied. labels Jan 29, 2021
@jensmaurer
Copy link
Member

Still gauging how it feels; we don't want to generate a lot of work while we're still unsure.

Copy link
Member

@jwakely jwakely left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This works for me.

@tkoeppe
Copy link
Contributor

tkoeppe commented Jan 29, 2021

I'd also find this a nice improvement. Let's leave this for another week or two for feedback (@CaseyCarter, @brycelelbach?), but let's tentatively consider this ready.

@jensmaurer jensmaurer removed the decision-required A decision of the editorial group (or the Project Editor) is required. label Jan 29, 2021
@brycelelbach
Copy link
Contributor

Personally I'm happy if @jwakely is happy. I think "The exception specification is equivalent to" is a more understandable/approachable for new C++ proposal authors. I do wonder, though, do we have enough of these to merit their own paragraph type? E.g. "Exception Specification: Equivalent to:"? This would nicely mirror the pattern of "Effects: Equivalent to:".

@tkoeppe
Copy link
Contributor

tkoeppe commented Feb 1, 2021

Thanks! We had talked about a new specification element, too, and were generally sympathetic to that idea. I suggested as a first step to go with this proposed change for now, and if it turns out to come up often enough to warrant a new element, we can consider that as a second step.

@JohelEGP
Copy link
Contributor Author

JohelEGP commented Feb 1, 2021

There's 22 of these.

@JohelEGP JohelEGP force-pushed the expr-in-x-equivalent-to branch from b075f81 to 428fcf5 Compare February 1, 2021 02:06
@JohelEGP JohelEGP force-pushed the expr-in-x-equivalent-to branch from 428fcf5 to d7bba7c Compare February 1, 2021 02:11
@JohelEGP JohelEGP changed the title [lib] Fix "The expression in...is equivalent to" [lib] Change 'expression inside noexcept' to 'exception specification' Feb 1, 2021
@jwakely
Copy link
Member

jwakely commented Feb 1, 2021

E.g. "Exception Specification: Equivalent to:"? This would nicely mirror the pattern of "Effects: Equivalent to:".

I don't think mirroring "Effects: Equivalent to ..." is necessarily desirable. For the Effects: paragraph that has very specific meaning with magic powers (it inherits all the constraints and other properties from the equivalent construct). We don't need that for the exception specification, we're just describing a boolean constant.

Copy link
Member

@jensmaurer jensmaurer left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The actual proposed changes look fine to me, and in line with what was decided in the editorial meeting.

@tkoeppe
Copy link
Contributor

tkoeppe commented Feb 1, 2021

OK, let's go with the current version for now, as agreed. I think @jwakely makes a good point, so this might be good enough and we just leave it like this, esp. if only 22 paragraphs would be affected by this anyway.

@tkoeppe tkoeppe merged commit 1aee0ca into cplusplus:master Feb 1, 2021
@JohelEGP JohelEGP deleted the expr-in-x-equivalent-to branch February 1, 2021 16:49
@JohelEGP JohelEGP mentioned this pull request Feb 11, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants