Skip to content
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

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

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?

source/utilities.tex Outdated Show resolved Hide resolved
source/utilities.tex Outdated Show resolved Hide resolved
source/utilities.tex Outdated Show resolved Hide resolved
@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.

@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
Copy link
Contributor Author

Will these two be enough to gauge how it feels?

@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>

@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

@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 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.

None yet

6 participants