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

P1754R1 Rename concepts to standard_case for C++20 #3149

Merged
merged 84 commits into from Aug 7, 2019

Conversation

zygoloid
Copy link
Member

@zygoloid zygoloid commented Aug 7, 2019

Dawn Perchik and others added 27 commits August 6, 2019 14:47
zygoloid and others added 21 commits August 6, 2019 17:49
Rename non-normative names that collide with new concept names.
Dawn Perchik and others added 2 commits August 6, 2019 17:51
…tions for consistency.

* concept.convertibleto => concept.convertible
* concept.derivedfrom => concept.derived
* concept.stricttotallyordered => concept.totallyordered
@zygoloid zygoloid merged commit 554a4e1 into master Aug 7, 2019
@@ -4889,7 +4899,7 @@
\begin{itemdescr}
\pnum
\effects
Initializes \tcode{base_} with \tcode{views::all(std::forward<R>(r))} and
Initializes \tcode{base_} with \tcode{views::all(std::forward<R>(r))}, and
Copy link
Contributor

Choose a reason for hiding this comment

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

This added comma (and the one above) go against the convention.

Copy link
Member Author

Choose a reason for hiding this comment

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

I added these for consistency with the constructor further above. I have no particular preference here other than consistency; please feel free to provide a PR to remove them.

@@ -143,7 +143,7 @@
// \ref{indirectcallable}, indirect callable requirements
// \ref{indirectcallable.indirectinvocable}, indirect callables
template<class F, class I>
concept IndirectUnaryInvocable = @\seebelow@;
concept indirectly_unary_invocable = @\seebelow@;
Copy link
Contributor

Choose a reason for hiding this comment

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

Does R1 have some indirectly_-prefixed concepts? For R0, all are indirect_-prefixed.

Copy link
Member Author

@zygoloid zygoloid Aug 7, 2019

Choose a reason for hiding this comment

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

Yes. In R1, only indirect_unary_predicate, indirect_relation, and indirect_strict_weak_order are indirect_; the rest are indirectly_ (including two that were previously Indirect not Indirectly).

@@ -2566,10 +2566,10 @@
\rSec3[range.ref.view]{Class template \tcode{ref_view}}

\pnum
\tcode{ref_view} is a \tcode{View} of the elements of some other \tcode{Range}.
\tcode{ref_view} is a \tcode{View} of the elements of some other \tcode{range}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should view and range use \libconcept here?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes.

Copy link
Contributor

Choose a reason for hiding this comment

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

Library folk have a longstanding tradition of colloquially referring to:

  • models of concept foo,
  • instances of types that model concept foo,
  • specializations of class template foo, and
  • instances of specializations of class template foo

as "foos". (Aside: one of the best days of working on Ranges was when we finally created Iterator as a distinct concept so we could finally say what the Standard means everywhere when it talks about "iterators". One of the worst was when LEWG renamed that concept to input_or_output_iterator, once again breaking that connection.)

The introductions for the XX_view types take this colloquialism to the extreme: "ref_view is a view of the elements of some other range" very concisely states that specializations of ref_view model the view concept, and that the elements denoted by an instance of a specialization of ref_view are shared with some other instance of an object type that also models the range concept. The only real novelty I introduced in these bits of wording was to use code font to visually link the colloquial uses more strongly to the concepts to indicate that yes: when we say "view" here we actually mean the view concept.

So yes, I think all of these should be \libconcepted.

that has constant time copy, move, and assignment operators; that is, the cost of
these operations is not proportional to the number of elements in the
\tcode{View}.
\tcode{view}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be "in the \libconcept{view} concept"? If not, what view are we talking about?

Copy link
Contributor

Choose a reason for hiding this comment

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

IMO this should be the number of elements in the \libconcept{view}. A concept doesn't have elements, but an instance of some object type that models range (or a refinement thereof like view) does. Adding the word "concept" here wouldn't be consistent with the usage of the colloquialism and would cause confusion.

@@ -1089,7 +1089,7 @@

\pnum
The class template \tcode{view_interface} is a helper for defining
\tcode{View}-like types that offer a container-like interface. It is
\tcode{view}-like types that offer a container-like interface. It is
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be \libconcept{view}?

Copy link
Contributor

Choose a reason for hiding this comment

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

Ick, I hate this wording. What do "view-like" and "container-like" mean?

IMO this sentence should be The class template \tcode{view_interface} is a helper for defining \libconcept{view}s. or more formally ... defining class templates whose specializations model \libconcept{view}.

@@ -2566,7 +2566,7 @@
\rSec3[range.ref.view]{Class template \tcode{ref_view}}

\pnum
\tcode{ref_view} is a \tcode{View} of the elements of some other \tcode{range}.
\tcode{ref_view} is a \tcode{view} of the elements of some other \tcode{range}.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be \libconcept{view} and \libconcept{range}?


\pnum
\begin{example}
Examples of \tcode{View}s are:
Examples of \tcode{view}s are:
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be "\libconcept{view}s"? And should it be "\libconcept{view} types"?

See also the use of "views", not \tcoded, a few lines down from here in the file. The terms "view" and "views" are used like this throughout the library. Are they referring to the "\tcode{view}s / \libconcept{view}s" above? If so, they should be fixed to use the same formatting, and if not, a definition of a "view" should be supplied. This is a common problem throughout the library for "predicate"s, "sentinel"s, etc.

Copy link
Contributor

Choose a reason for hiding this comment

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

This is a common problem throughout the library for "predicate"s, "sentinel"s, etc.

Yes, it is. And I failed to achieve consistency in [ranges], let alone the rest of the library. Maybe we should copy my comment above into an editorial issue and make a determination about whether or not it should be policy so we can enforce it library-wide rather than having it be my personal war on the library wording?

@@ -2545,7 +2545,7 @@
\rSec2[range.all]{All view}

\pnum
\tcode{views::all} returns a \libconcept{View} that includes all elements of
\tcode{views::all} returns a \libconcept{view} that includes all elements of
its \libconcept{range} argument.
Copy link
Contributor

Choose a reason for hiding this comment

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

Here view is \libconcept'd, so either this is wrong or the uses elsewhere should be \libconcept'd as well. Also, should uses like this be changed to "\libconcept{view} type"s?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants