P0898R3 Standard Library Concepts #2176

CaseyCarter · 2018-06-13T21:15:58Z

Pushing from a branch in a remote repo rather than motions-2018-06-lwg-28 as directed in the new application procedure since I don't have commit access and cannot make new branches.

Reviewers: Please be brutal. Tiny errors I've made herein will be magnified by one to two orders of magnitude when the One Ranges Proposal is merged - I need the practice.

I'd appreciate if someone could assign this to the "post-2018-06" milestone as well.

jensmaurer · 2018-06-19T20:20:12Z

Hi @CaseyCarter , thanks for the pull request.

First of all, we really want to get rid of \textit in the standard text, because it does not convey any semantics. If we ever want to change the formatting of, say, C++98 concepts, \textit is not enough.
So, please add a new macro \oldconcept or something like that to macros.tex, defined to \textit, and use \oldconcept{blah} throughout the text proper. (If you have a better name for the macro, I'm all ears.)
(One of the first use-cases that comes to mind for \oldconcept is to actually start indexing the uses of defined terms, not just the definitions. Not now, though.)

CaseyCarter · 2018-06-19T20:30:52Z

@jensmaurer Would it make sense to similarly define a \concept or \libconcept macro for styling (and potential indexing) of new-style concepts?

jensmaurer

Before I lose too much, first round of comments.

jensmaurer · 2018-06-19T20:26:58Z

source/algorithms.tex

@@ -1126,39 +1126,39 @@
 or
 \tcode{Input\-Iterator2},
 the template argument shall satisfy the
-requirements of an input iterator\iref{input.iterators}.
+\textit{Cpp98InputIterator} requirements\iref{input.iterators}.


Could you point to the location in the incoming paper where authorization is given to make this change? The paper says "prepend the prefix 'Cpp98' to uses of the named requirement sets below", but "input iterator" is not a CamelCase fixed-width requirement, it's a prose plain-text requirement, and does not appear in the list in subclause 1.3 of the paper.

Subclause 1.3 is woefully incomplete: I missed the Iterator requirements tables and some more obscure sets of named requirements. The design intent - as discussed in LEWG and I believe LWG as well - was to rename all of the old named requirements tables to make way for current and future concepts.

It's also important that all references to the requirements use the proper name. The colloquialism "input iterators", for example, can now be interpreted to mean either types that meet the Cpp98InputIterator requirements or types that model the InputIterator concept. We need to very clearly define for users exactly to which set of requirements each such usage is intended to refer.

I understand the desires, but the paper as moved doesn't say so, and there isn't even a trace of a hint in the paper that this might be the intent, given that only CamelCased requirements are quoted in 1.3. (I'd be happy to be shown otherwise.)

@zygoloid, are we overstepping our editorial leeway here by changing such phrases, in particular since the result could be incorrectly interpreted as referring to the table only, not the entire set of requirements? (I think we had an editorial issue a few months ago where exactly that point was a concern.)

I think that was me, and the issue was an edit that changed a cross-reference to the NullablePointer requirements section to point to the table instead.

jensmaurer · 2018-06-19T20:29:05Z

source/algorithms.tex

-the template argument shall satisfy the requirements
-of a forward iterator\iref{forward.iterators}.
+the template argument shall satisfy the
+\textit{Cpp98ForwardIterator} requirements\iref{forward.iterators}.


The issue here is that a reference to "forward iterator" (plain text) refers to the entire section [forward.iterators], including (for example) the multi-pass guarantee. In contrast, Cpp98ForwardIterator appears to refer to the table only (which just gives a bit of syntax). That feels like a regression to me.

"forward iterators" will be ambiguous once Ranges are merged. I understand your point about the distinction between the table and the additional requirements outside the table, though. Do you agree it would be clearer if I retitle the existing sections e.g. "Cpp98ForwardIterator requirements"?

Yes, I think so. I fear there are Cpp98 concepts definitions that don't have a section of their own, though, so this strategy might fail here and there. Before you subscribe for more work, @zygoloid should really chime in how he wants to proceed here.
In any case, make sure all those changes are a (one) separate commit so that they can be messed with easily.

jensmaurer · 2018-06-19T20:31:17Z

source/algorithms.tex

@@ -4514,10 +4514,10 @@
 The ranges \range{first}{middle} and \range{middle}{last} shall be
 sorted with respect to \tcode{operator<} or \tcode{comp}.
 \tcode{BidirectionalIterator} shall satisfy the
-\tcode{ValueSwappable} requirements\iref{swappable.requirements}. The type
+\textit{Cpp98ValueSwappable} requirements\iref{swappable.requirements}. The type


Going forward, we really need to find standardized wording to say "satisfies the syntax constraints" vs. "syntax + semantics", and apply such phrasing uniformly.

I'm working on that. My current plan is to uniformly use "T meets the CppXXX requirements" for "old" concepts (which is the prevalent library usage, this occurrence is aberrant), "T satisfies Concept" to mean purely syntactic conformance (which is the proper core language meaning of :"satisfy"), and "T models Concept" to mean that T must both satisfy Concept and must further meet the semantic requirements or the program is ill-formed NDR.

I'm working on rewording the combined Ranges paper to use that style, and plan to submit future editorial changes to the rest of the library specification to make everything uniform.

See also some earlier discussion in #1263. I understand some decisions are obsolete now.

jensmaurer · 2018-06-19T20:33:47Z

source/algorithms.tex

-the template argument shall satisfy the requirements
-of an output iterator\iref{output.iterators}.
+the template argument shall satisfy the
+\textit{Cpp98OutputIterator} requirements\iref{output.iterators}.


I'm wondering whether we can find a slightly less awkward spelling than "Cpp98Blah" for these old-style concepts. Maybe even something innovative such as OutputIterator₉₈ would work.

I'm certainly not married to "Cpp98Blah". It's better than LEWG's suggestion "STL1Blah", but that wasn't a high bar to meet. Now that the styling is macro-ized, we can change it with a one-liner.

Cpp98MoveConstructible certainly feels anachronistic.

Cpp98MoveConstructible certainly feels anachronistic.

We specifically pointed out that example in both LEWG discussion of the Ranges proposal and LWG discussion of this proposal, and both subgroups found the confusion level acceptable.

jensmaurer · 2018-06-19T20:41:46Z

source/lib-intro.tex

+\definition{expression-equivalent}{defns.expression-equivalent}
+\indexdefn{expression-equivalent}%
+relationship that exists between two expressions \tcode{E1} and \tcode{E2} such that
+\begin{itemize}


ISO wants us to phrase definitions of such terms in a way so that you can macro-expand the definition into the point-of-use and the sentence stays intact (modulo semantic parentheses implied by the macro). I think this wouldn't work here.

I'll fix this.

jensmaurer · 2018-06-19T20:49:03Z

source/lib-intro.tex

+A declaration may explicitly impose requirements through its associated
+constraints\iref{temp.constr.decl}. When the associated constraints refer to a
+concept\iref{temp.concept}, additional semantic requirements are imposed on the
+use of the declaration.


"additional" is great, but which ones in particular? Maybe the semantic requirements explained in the subclause that defines the concept?

How about "When the associated constraints refer to a concept\iref{temp.concept}, the semantic constraints specified for that concept are additionally imposed on the use of the declaration."?

Yes, seems to be an improvement. (Make sure to keep those rephrasings separate from the original commit with the as-moved paper.)

jensmaurer · 2018-06-19T20:50:33Z

source/algorithms.tex

-the template argument shall satisfy the requirements
-of a forward iterator\iref{forward.iterators}.
+the template argument shall satisfy the
+\oldconcept{ForwardIterator} requirements\iref{forward.iterators}.


The [forward.iterators] section is not so named, and the table of that name doesn't show semantics.

Repeating my reply to your original comment (that Github will likely bury after the \oldconcept push):

"forward iterators" will be ambiguous once Ranges are merged. I understand your point about the distinction between the table and the additional requirements outside the table, though. Do you agree it would be clearer if I retitle the existing sections e.g. "Cpp98ForwardIterator requirements"?

I'm ok with that, although that doesn't really remove the ambiguity if the table continues to be labeled the same as the section. @zygoloid?

Yes, I'd thought about the ambiguity of both subclauses and tables titled "Foo requirements," although that's pre-existing, for example, [allocator.requirements] is titled "Allocator requirements" and contains Table 31 "Allocator requirements". We should retitle those tables to something generic like "Allocator valid expressions with semantics" - [structure.requirements]/4 says they are "a
table that specifies an initial set of the valid expressions and their semantics." Which is also a misnomer.

I'm waiting for Richard's nod to enact these changes. I have more work to do on the merged Ranges proposal than time before the mailing deadline to do it in, so I don't want to walk down any blind alleys.

jensmaurer · 2018-06-19T20:52:23Z

source/algorithms.tex

@@ -1126,39 +1126,39 @@
 or
 \tcode{Input\-Iterator2},
 the template argument shall satisfy the
-requirements of an input iterator\iref{input.iterators}.
+\oldconcept{InputIterator} requirements\iref{input.iterators}.


Where in the incoming paper is authorization given for this change? InputIterator is not in the list of CamelCase concepts in section 1.3 of the paper.

Repeating my reply to your original comment (that Github will likely bury after the \oldconcept push):

Subclause 1.3 is woefully incomplete: I missed the Iterator requirements tables and some more obscure sets of named requirements. The design intent - as discussed in LEWG and I believe LWG as well - was to rename all of the old named requirements tables to make way for current and future concepts.

It's also important that all references to the requirements use the proper name. The colloquialism "input iterators", for example, can now be interpreted to mean either types that meet the Cpp98InputIterator requirements or types that model the InputIterator concept. We need to very clearly define for users exactly to which set of requirements each such usage is intended to refer.

@zygoloid, what's your guidance here?

jensmaurer · 2018-06-19T20:55:31Z

source/lib-intro.tex

+overloads matching the signatures of overloads defined in namespace \tcode{std}.
+When the deleted overloads are viable, user-defined overloads must be more
+specialized\iref{temp.func.order} or more constrained\iref{temp.constr.order} to
+be used by a customization point object.


"more constrained" is a special case of "more specialized".

[temp.constr.order]/3 defines "more constrained" as an orthogonal notion to "more specialized." The two notions interact in overload resolution where "more constrained" is used to discriminate between overloads neither of which is more specialized, but "more constrained" is not otherwise a special case.

Agreed, sorry for the noise.

jensmaurer · 2018-06-19T20:58:10Z

source/lib-intro.tex

@@ -2652,7 +2733,7 @@
 paragraph.
 \item
 if an incomplete type\iref{basic.types} is used as a template
-argument when instantiating a template component, unless specifically
+argument when instantiating a template component or evaluating a concept, unless specifically


(Not editorial) I find it odd that an incomplete type would cause undefined behavior; it's a compile-time failure and should be ill-formed (possibly no diagnostic required).

LWG has been addressing a similar problem with the type traits to require implementations to diagnose precondition violations. There's currently no technology to do so with concepts, but this at least leaves the avenue open for future changes.

I'm complaining about the fact that this situation is in the "undefined behavior" category, not in a (separate) category "ill-formed, no diagnostic required". The latter would leave avenues open, too, but would also permit implementations to diagnose and refuse e.g. incomplete types today. And, maybe there is even a conflict here if some preconditions says "shall be a complete type" (i.e. compile-time diagnostic), yet this section says "undefined behavior" (i.e. only bad if you actually evaluate it, which seems a misguided idea for a compile-time violation).

I agree. I've made a note to file an LWG issue to adjust this wording to IFNDR.

This is LWG 3142.

jensmaurer · 2018-06-19T21:03:47Z

@CaseyCarter "Would it make sense to similarly define a \concept or \libconcept macro for styling (and potential indexing) of new-style concepts?"
Yes, please.

jensmaurer · 2018-06-19T21:17:00Z

source/utilities.tex

+ \hspace*{2ex}\tcode{basic_common_reference;}
+ &
+ A program may specialize this trait if at least one
+ template parameter in the specialization depends on a user-defined


LWG 2139 (to be applied in #2193) now uses "program-defined type" here.

I'll update uses of "user-defined type" accordingly.

jensmaurer · 2018-06-19T21:19:26Z

source/utilities.tex

+ &
+ A program may specialize this trait if at least one
+ template parameter in the specialization depends on a user-defined
+ type. If there is a member \tcode{type}, it shall be a \grammarterm{typedef-name}.


(not editorial) This seems an overspecification; certainly a user could define the member "type" as a nested class (which is not a typedef-name). The difference is, I believe, unobservable.

Technically, it's observable in some cases with an elaborated-type-specifier.

I'd be happy to change this to "If there is a member \tcode{type}, it shall denote a type." if we're willing to call that change editorial-ish.

@timsong-cpp is right, such a change would be observable so can't be done editorially. @CaseyCarter, could you please check whether there is already an LWG issue for that, and file one if not?

Yes I will. I have a set of notes on all the comments here so I can track which changes I've accomplished. There are six entries now for issues to file post-merge.

jensmaurer · 2018-06-19T21:22:00Z

source/utilities.tex

+\pnum
+Let:
+\begin{itemize}
+\item \tcode{CREF(A)} be \tcode{add_lvalue_reference_t<const remove_reference_t<A>{}>},


We usually do \placeholder here (cf. INVOKE), because CREF(A) is not valid C++ source code; it's a funny abstract-level meta-type-function.

jensmaurer · 2018-06-19T21:25:26Z

source/utilities.tex

+    \tcode{const volatile short}.
+  \end{example}
+\item \tcode{COND_RES(X, Y)} be
+  \tcode{decltype(false ? declval<X(\&)()>()() : declval<Y(\&)()>()())}.


Why are we creating two invented function types here, just to call them and look at their result values? What's the net difference vs. omitting the indirection via the function references? What's the net difference vs. common_type_t<X,Y>?

This is a truly disgusting hack to avoid declval's add_rvalue_reference_t on the return type. decltype(declval<X>()) is add_rvalue_reference_t<X>, decltype(declval<X(&)()>()() is exactly X. We discussed at length in LWG that this is abominable, and that no one had an idea for an improvement.

... and common_type_t<X,Y> doesn't work because?

Because X and Y may be cv-qualified or reference types, and common_type decays its arguments: that's the entire motivation for adding common_reference.

For cppreference, I just invented a declval substitute that returns T instead and used that, FWIW.

(decltype(declval<X(&)()>()()) is also not exactly X; it's subject to the usual prvalue cv-qualification adjustment rules and the rvalue-reference-to-function-is-lvalue rule.)

I'd hate to define yet another exposition-only helper here that's only going to be used in this one place; COMMON_REF is overrun with them. Do we agree that template <class T> T __val() noexcept; // \expos ... decltype(false ? __val<X>() : __val<Y>()) is an improvement?

I'd hate to define yet another exposition-only helper here

I thought this discussion was familiar: ericniebler/stl2#338 (comment).

Maybe we shouldn't change anything here and go with the formulation in the paper. These nuances are important, but the code snippets for the meta-functions are way too obtuse to communicate that effectively. Can we maybe add a few notes or examples explaining what the intent is (in particular involving subtleties of prvalues vs. xvalues)? (As a matter of principle, I'm wondering whether some prose relying on the core language mechanics of the ?: operator or "common type" would be shorter and easier to understand. Not now.)

jensmaurer · 2018-06-19T21:35:01Z

source/utilities.tex

-     \begin{note}
-     This will not apply if there is a specialization \tcode{common_type<D1, D2>}.
-     \end{note}
+    denotes a valid type, let \tcode{C} denote that type.


Good, you already changed "its type" to "that type" here.

jensmaurer · 2018-06-19T21:45:15Z

source/utilities.tex

+
+\pnum
+Notwithstanding the provisions of\iref{meta.type.synop}, and
+pursuant to\iref{namespace.std}, a program may specialize


Both \iref's should be \ref's (with a space in front); the \iref automatically adds parens that we don't want here (compare to the parallel phrasing for common_type two paragraphs above).

I somewhat blindingly replaced refs with irefs - I'll audit them all.

jensmaurer · 2018-06-19T21:47:09Z

source/utilities.tex

+Moreover, \tcode{basic_common_reference<T, U, TQual, UQual>::type} shall denote
+the same type, if any, as does \tcode{basic_common_reference<U, T, UQual, TQual>::type}.
+A program shall not specialize \tcode{basic_common_reference} on the third or
+fourth parameters, \tcode{TQual} or \tcode{UQual}. No diagnostic is required for


What does that mean? Are you talking about partial specialization here? Then you should say so at the start of this paragraph. (A specialization is a non-template; a partial specialization is a template: two fundamentally different things.)

Yes, the intent here is that users partially specialize basic_common_reference. I admit that I can't see a reason now for "A program shall not specialize \tcode{basic_common_reference} on the third or fourth parameters, \tcode{TQual} or \tcode{UQual}." I believe the intent is to make it harder for users to do something stupid and violate the other requirements and/or point them more directly down the intended path.

@ericniebler, can you clarify the intent of this particular requirement?

Yes, the intent here is to disallow specifying the third or fourth template parameters in either a partial or explicit specialization. Those parameters are specified by the common_reference trait. The exact unary templates used as template arguments to the (possibly user-specialized) basic_common_reference instantiation are unspecified, and implementors are free to change them between releases. Any code that specializes on those parameters (either partial or explicit) is de facto wrong.

Does a program with such a specialization need to be IFNDR? IMO it's sufficient that common_reference specifies how and when it works, and users shouldn't be surprised when their broken specializations are never selected.

I suppose not. Striketh the sentence if it offendeth thee.

An alternative which I hadn't considered until just now is to redesign the trait to make it impossible to misuse:

template <class A, class B> struct basic_common_reference { template <template <class> class AQual, template <class> class BQual> using type = ...; };

EDIT: That is obviously not editorial. :-)

I don't know that I buy "impossible to misuse", but I'll make a note to possibly file an issue to make this design change after we investigate it more thoroughly.

...and strike the requirement for now, since we think it adds no useful normative effect.

My original concern here was the use of the word "specialization" for a case where only partial specializations should ever appear in user programs (because the arguments for AQual and BQual are controlled from the outside, so specializing on them doesn't make sense). I'd appreciate an LWG issue for at least that part.

I did change the first sentence of what is now "Note D" to read:

Notwithstanding the provisions of \ref{meta.type.synop}, and pursuant to \ref{namespace.std}, a program may partially specialize \tcode{basic_common_reference<T, U, TQual, UQual>} for types \tcode{T} and \tcode{U} such that \tcode{is_same_v<T, decay_t{>}} and \tcode{is_same_v<U, decay_t{>}} are each \tcode{true}.

which I consider to be an editorial clarification. If you feel that change is insufficient or not editorial, I'll revert it and file an issue.

jensmaurer · 2018-06-19T21:49:20Z

source/numerics.tex

@@ -1786,6 +1786,30 @@
 \indextext{requirements!uniform random bit generator|)}%
 \indextext{uniform random bit generator!requirements|)}



Not done: "[Editor’s note: Relocate "Header /tcode synopsis" [rand.synopsis] before 29.6.1 "Requirements"
[rand.req]]"

I must have ignored this directive upon finding no header named /tcode<random> in the library ;)

jensmaurer · 2018-06-19T21:53:52Z

source/numerics.tex

@@ -1786,6 +1786,30 @@
 \indextext{requirements!uniform random bit generator|)}%
 \indextext{uniform random bit generator!requirements|)}

+\pnum
+The \tcode{UniformRandomBitGenerator} concept is a slight relaxation of the uniform random bit generator requirements, in that it does not require a nested \grammarterm{typedef-name} \tcode{result_type}.


Please add line breaks after at most 79 chars, preferably "semantic line breaks", i.e. do not break in the middle of a phrase or term.

jensmaurer · 2018-06-19T21:55:29Z

source/numerics.tex

@@ -1786,6 +1786,30 @@
 \indextext{requirements!uniform random bit generator|)}%
 \indextext{uniform random bit generator!requirements|)}

+\pnum
+The \tcode{UniformRandomBitGenerator} concept is a slight relaxation of the uniform random bit generator requirements, in that it does not require a nested \grammarterm{typedef-name} \tcode{result_type}.
+


The presentation seems suboptimal and redundant here. It seems more compact to introduce \concept{UniformRandomBitGenerator} first (which is the more relaxed concept) and then define "uniform random bit generator" in terms of that, adding the result_type requirement.

You mean something like urbg.pdf? That seems to go a bit past "editorial", but I'm game if you folks are.

It also relaxes the rules by only requiring the result type to model UnsignedIntegral instead of being an unsigned integer type. The former permits bool and character types that are unsigned.

@CaseyCarter: Yes, that looks like an improvement. Do consider @timsong-cpp's comment, though, "char" (if it happens to be unsigned on a particular platform) seems to be an UnsignedIntegral type (but is not an unsigned integral type per the core language). Similar for "bool". Keep this rewording in a separate commit so that it can be reviewed extra-carefully. (In my view, doing this rewording editorially is fine as long as the normative outcome is exactly the same as before. The larger the rewording, the more danger to introduce inadvertent differences, though, so @zygoloid may want to eventually drop the patch when merging.)

I'll reword to:

A class \tcode{G} meets the \term{uniform random bit generator} requirements if \tcode{G} models \libconcept{UniformRandomBitGenerator}, \tcode{invoke_result_t<G\&>} is an unsigned integer type\iref{basic.fundamental}, and \tcode{G} provides a nested \grammarterm{typedef-name} \tcode{result_type} that denotes the same type as \tcode{invoke_result_t<G\&>}.

to fix the normative difference between "unsigned integer type" and UnsignedIntegral and add this in a separate commit.

jensmaurer · 2018-06-19T21:57:07Z

I've reviewed all changes except the entirely-new top-level clause "concepts".

jensmaurer · 2018-06-19T22:36:16Z

source/concepts.tex

+\rSec2[concepts.lib.general.equality]{Equality Preservation}
+
+\pnum
+An expression is \term{equality preserving} if, given equal inputs, the expression results in


This needs to become a defined term with an index entry. Use \defnx{equality preserving}{expression!equality preserving} .

jensmaurer · 2018-06-19T22:40:52Z

source/concepts.tex

+\end{note}
+
+\pnum
+Expressions declared in a \grammarterm{requires-expression} in this document are


I have a hard time associating "declared" with expressions. We declare names and functions and variables and whatnot, but not expressions. Also, a requires-expression may also appear in non-declaration contexts (e.g. as an expression-statement).
Also, in which situation could a requires-expression not be equality preserving? It seems to be a totally compile-time concepts-checking expression that never modifies any object at all.

So, this is intended to refer to the expression inside a simple-requirement and inside a compound-requirement.
Maybe the phrasing is missing a crucial "were they evaluated" or similar, because requires-expressions are unevaluated operands and thus never actually evaluated (and thus, talking about equality preserving doesn't really make sense). (This ventures beyond editorial.)

Follow-on question: We're only talking about "expression" here, but what about the implicit conversion to the specified type in a compound-requirement? That is syntactically not part of the expression, yet could affect "equality preserving".

We always want explicit convertibility as well when we require implicit convertibility, which means that we'd use ConvertibleTo instead. (But perhaps ConvertibleTo is missing a requirement that the conversion is equality-preserving?)

Hm... ConvertibleTo has a simple-requirement, which contains a static_cast<> expression, which we're instructed needs to be equality preserving. Ah, but... see below.

I wonder if in the library preamble we can handwave that when the library uses an implicit conversion constrain we also require explicit conversion (with the same semantics) and equality preservation. And then stump for more help from the core language regarding conversion constraints.

I'd prefer to be explicit about conversions. I don't think writing { E } -> ConvertibleTo<T>; is a significant burden over { E } -> T; - not enough so to have the same syntax implicitly result in different checked constraints, anyway. I'll file an issue to deal with the interaction of equality-preservation and implicit conversion constraints. It doesn't really matter to Ranges, but I suspect someone will eventually want to specify a library component with such a constraint and it needs to have a well-defined meaning rather than us simply telling them to not do that bad thing.

Clarification: today we have to write E; requires ConvertibleTo<decltype((E)), T>; to express that constraint properly in terms of the exact type and value category of E. Once P1084 is adopted, it can be simplified to { E } -> ConvertibleTo<T>;.

@CaseyCarter: I don't buy the "expression pattern" theory for the case of new-style concepts. (I agree regarding old-style concepts.) With concepts in the core language, we no longer have to handwave which expressions we mean: Concepts show actual code that can be compiled. There are no "patterns" going on beyond what the core language prescribes. To wit: the text is referring to the grammar non-terminal requires-expression.
My narrow (editorial) issue here is that "expressions declared in..." sounds broken to me; expressions are never "declared". Further (new concern), it's unclear whether "in" is also intended to cover any subexpression of the top-level expressions in the requirements.
I'd appreciate a clear syntax decomposition of requires-expression to define which expressions are in view here (maybe call them "required expressions" or so). Maybe that's not editorial to start with. @zygoloid?

Would replacing "Expressions declared in a requires-expression in this document..." with "Expressions that appear in a simple-requirement or compound-requirement of a requires-expression in the standard library..." be an improvement? That seems to address much of your concerns.

jensmaurer · 2018-06-19T22:43:33Z

source/concepts.tex

+
+\pnum
+An expression that may alter the value of one or more of its inputs in a manner
+observable to equality preserving expressions is said to modify those inputs.


"equality preserving" is used as an adjective here and needs a hyphen. @zygoloid should decide whether we want to always hyphenate, even if "preserving" could be interpreted as a verb, with the view that "equality-preserving" is a new, specially made-up adjective that we want to use as a fixed term (and we never mentally disassemble it into "equality" and "preserving").

I believe all uses of "equality preserving" (with the obvious exception of the definition) are adjective phrases and should be hyphenated.

Then we should also hyphenate the definition (and maybe consider rephrasing it to "An equality-preserving expression is an expression that ...")

jensmaurer · 2018-06-19T22:55:59Z

source/concepts.tex

+as ill-formed a program which requires \tcode{C<T>}.
+\end{example}
+
+\rSec1[concepts.lib.synopsis]{Header \tcode{<concepts>} synopsis}


Please use [concepts.syn] as the label, i.e. header name plus ".syn".

jensmaurer · 2018-06-19T22:58:20Z

source/concepts.tex

+%!TEX root = std.tex
+\rSec0[concepts.lib]{Concepts library}
+
+\rSec1[concepts.lib.general]{General}


Please drop ".lib" from all labels. We no longer mark library labels specially.

jensmaurer · 2018-06-19T23:11:41Z

source/concepts.tex

+whose return type is \tcode{U}. \tcode{CommonReference<T, U>} is satisfied
+only if:
+\begin{itemize}
+\item \tcode{C(t())} equals \tcode{C(t())} if and only if \tcode{t()} is an


The "if and only if" feels wrong here; it seems to say that C(t()) must have a value different from C(t()) if t() is not equality preserving. That seems hard to do, since t() could modify some unknown global variable.

Multiple evaluations of a non-equality-preserving t() could also sometimes have the same value, in which case C(t()) might be equal.

I think fixing this will require normative changes and not simply editorial clarification - I'll have to file an issue. @ericniebler, thoughts?

Agreed with an LWG issue.

jensmaurer · 2018-06-19T23:14:30Z

source/concepts.tex

+
+\end{itemdescr}
+
+\rSec2[concepts.lib.corelang.integral]{Concept \tcode{Integral}}


In general, we're trying to avoid single-paragraph sections. This area is getting dangerously close to the cliff, with four subheadings on one page. Is there any plan at all to possibly have multiple (simple) concepts in one subclause? Maybe not. Net result: no change requested for now.

I'd be mildly concerned about confusion as to which concepts correspond with which prose requirements - despite that prose requirements are always introduced with "Let T be ... Concept<T> is satisfied only if"- but I don't think that's insurmountable.

I expect these sections to shrink further once we remove the unnecessary "need be no subsumption" wording.

jensmaurer · 2018-06-19T23:15:05Z

source/concepts.tex

+\tcode{is_integral_v<T>}.
+\end{itemdescr}
+
+\rSec2[concepts.lib.corelang.signedintegral]{Concept \tcode{SignedIntegral}}


Label: concept.signed.int

jensmaurer · 2018-06-19T23:15:21Z

source/concepts.tex

+\end{note}
+\end{itemdescr}
+
+\rSec2[concepts.lib.corelang.unsignedintegral]{Concept \tcode{UnsignedIntegral}}


Label: concept.unsigned.int

jensmaurer · 2018-06-19T23:20:32Z

source/concepts.tex

+
+\rSec1[concepts.lib.compare]{Comparison concepts}
+
+\rSec2[concepts.lib.compare.general]{General}


While ISO says "no hanging paragraphs", I'm not enamoured with single-sentence sections; I think having the essentially non-normative intro in the parent section would work, too. @zygoloid?

jensmaurer · 2018-06-19T23:27:56Z

source/concepts.tex

+User code can ensure that the evaluation of \tcode{swap} calls
+is performed in an appropriate context under the various conditions as follows:
+\begin{codeblock}
+#include <utility>


Is there a #include <concepts> missing?

Also #include <cassert>, or change the assertions in main to the shiny [[assert: ... ]];

timsong-cpp · 2018-06-20T02:02:27Z

source/concepts.tex

+\begin{codeblock}
+#include <utility>
+
+template <class T, SwappableWith<T> U>


and a std::.

How so? We usually don't prefix with std:: in the library sections of the standard, but maybe I'm overlooking something.

We also don't usually include headers in library examples. Since this particular example is hypothetical user code, not library code, with a main() and all, I think it makes more sense to write it like a user would.

Elaborating @timsong-cpp's statement: I dropped this example in R2 of the proposal, and Daniel Krügler specifically asked me to replace it. This particular example is crucial to readers understanding how the ADL 2-step works in practice. It needs full qualification to make it clear how the namespaces and qualifications interact.

W-E-Brown · 2018-06-20T04:37:58Z

On Jun 19, 2018, at 9:12 PM, timsong-cpp ***@***.***> wrote: @timsong-cpp commented on this pull request. In source/numerics.tex: > @@ -1786,6 +1786,30 @@ \indextext{requirements!uniform random bit generator|)}% \indextext{uniform random bit generator!requirements|)} +\pnum +The \tcode{UniformRandomBitGenerator} concept is a slight relaxation of the uniform random bit generator requirements, in that it does not require a nested \grammarterm{typedef-name} \tcode{result_type}. + It also relaxes the rules by only requiring the result type to model UnsignedIntegral instead of be an unsigned integer type. The former permits bool and character types that are unsigned.

I believe not. Please see [rand.req.genl]/1f.

timsong-cpp · 2018-06-20T04:55:00Z

We don't use UIntType anywhere in [rand.req.urng]. The "uniform random bit generator" requirements merely requires result_type to be "an unsigned integer type". The UniformRandomBitGenerator concept relaxes that and only requires the result type model UnsignedIntegral.

The engine templates in the standard do use UIntType, so instantiating them with, say, char32_t or unsigned char, is undefined. But it appears to be possible to write a type yourself with a result_type of unsigned char and have it meet the uniform random bit generator requirements.

jensmaurer · 2018-06-20T07:18:38Z

source/concepts.tex

+expression is required to be well-defined.
+
+\pnum
+Expressions required by this specification to be equality preserving are further


this specification -> this document

jensmaurer · 2018-06-20T07:23:27Z

source/concepts.tex

+
+\pnum
+There need be no subsumption relationship between \tcode{Assignable<LHS, RHS>}
+and \tcode{is_\-lval\-ue_\-ref\-er\-ence_v<LHS>}.


Could you please limit the hyphenation hint to exactly the one you need to avoid the overfull \hbox, if any?

jensmaurer · 2018-06-20T07:26:46Z

source/concepts.tex

+\pnum
+Expressions required by this specification to be equality preserving are further
+required to be stable: two evaluations of such an expression with the same input
+objects must have equal outputs absent any explicit intervening modification of


Don't use "must"; it's only used for consequences of laws of nature and similar such things in the ISO world.
In this case "are required to" seems appropriate. Please do me a favor and "grep" for "must" in the rest of the text and eradicate it.

I sincerely hope that "the rest of the text" means the text added in this PR, and not the full text of the Standard. There are a great many "must"s that we must eradicate to please ISO.

@CaseyCarter: Yes, the text of the PR. Our other "musts" should be in notes only.

jensmaurer · 2018-06-20T09:11:02Z

source/concepts.tex

+\begin{itemdecl}
+template <class From, class To>
+concept ConvertibleTo = is_convertible_v<From, To> &&
+  requires(From (&f)()) { static_cast<To>(f()); };


So, we need to ascertain that the expression static_cast<To>(f()) is equality preserving, right?
Could you help me along what the "inputs" are, here? I presume just "f", which is a reference to some unknown function.
And the "outputs" here are, presumably, just the result, because no operand is modified here.
Now the definition tells me the thing is equality preserving if I get repeatably equal "To" prvalue objects given equal "f" (reference) inputs. That seems to be wrong, because it does not consider the "From" values at all. Surely, different "To" values are expected for differing "From" values?

The "operands"/"inputs"/"outputs" wording in concepts.general.equality is still somewhat soft and in need of clarification: it's incredibly hard to specify without exploding the text into full mathematical rigor that would be unacceptable for the IS. My mildly hand-wavy response is that I would consider both f and f() to be operands of this expression pattern.

The prose does try to clarify this:

and let `\tcode{f}` be a function with no arguments and return type \tcode{From} such that \tcode{f()} is equality-preserving. \tcode{ConvertibleTo<From, To>} is satisfied only if:

but there's no correspondence established between f in the concept definition and this f. I suppose we could clarify in the concept definition itself:

template <class From, class To> concept ConvertibleTo = is_convertible_v<From, To> && requires(From (&f)()) { // f is an arbitrary function such that f() is an equality-preserving expression static_cast<To>(f()); };

Would you consider that an improvement?

"f is an arbitrary function" -> "f refers to an arbitrary function" (f is still a reference, not a function). Otherwise looks like an improvement.

jensmaurer · 2018-06-20T09:20:38Z

source/numerics.tex

@@ -1786,6 +1786,30 @@
 \indextext{requirements!uniform random bit generator|)}%
 \indextext{uniform random bit generator!requirements|)}

+\pnum
+The \tcode{UniformRandomBitGenerator} concept is a slight relaxation of the uniform random bit generator requirements, in that it does not require a nested \grammarterm{typedef-name} \tcode{result_type}.
+


@CaseyCarter: Yes, that looks like an improvement. Do consider @timsong-cpp's comment, though, "char" (if it happens to be unsigned on a particular platform) seems to be an UnsignedIntegral type (but is not an unsigned integral type per the core language). Similar for "bool". Keep this rewording in a separate commit so that it can be reviewed extra-carefully. (In my view, doing this rewording editorially is fine as long as the normative outcome is exactly the same as before. The larger the rewording, the more danger to introduce inadvertent differences, though, so @zygoloid may want to eventually drop the patch when merging.)

jensmaurer · 2018-06-20T09:27:18Z

Please rebase and add the feature-test macro (is there any?), now that the basic infrastructure for feature-test macros has been merged.

CaseyCarter · 2018-06-20T19:39:14Z

Pushing what I have so far. I still need to ~~fix the definition of "expression-equivalent" and~~ cleanup the weird use of "if and only if" in Common and CommonReference, and we haven't reached a decision about renaming the "old concept" subclauses to "Cpp98Foo requirements".

EDIT: We can now remove the needs-rabase label.

jensmaurer · 2018-06-21T05:54:02Z

Sorry, @CaseyCarter, github still tells me there are conflicts that need a rebase.

To clarify the rules on "Fixup" vs. regular commits: "Fixup" is stuff that would have been in another commit if the typist of the commit wouldn't have made a dumb mistake. That includes fixing typos (commas, misspellings) and incorrect transcriptions from the incoming paper. For example, the move of the "random" synopsis is a "fixup", as is the \oldconcept stuff and the \defnx and \ref / \iref stuff and \placeholder. (Note that the point-of-reference for "fixup" is the rendered text, not the situation of the LaTeX source.)

The change of the section labels should come right after that in a separate commit.

Adding "Note C" and "Note D" (or striking a superfluous sentence), in contrast, is not a fixup, but a separate editorial rephrasing that should follow our regular commit guidelines. For example, start the commit comment with [section.label].

As agreed with @zygoloid, the feature-test macro is not a fixup because the actual wording diff was not in the incoming paper (for lack of a reference in the working draft).

Yes, you'll need to git push --force-with-lease the rephrasing of the commit logs that I ask for here. Since you'll be doing that, you might as well fold the real fixups into the commits they refer to --- but some of your "fixup"s are actually editorial rephrasings.

CaseyCarter · 2018-06-21T06:18:35Z

Sorry, @CaseyCarter, github still tells me there are conflicts that need a rebase.

I shouldn't have bothered to ask: I'm touching enough that virtually every commit causes a conflict.

To clarify the rules on "Fixup" vs. regular commits: ...

Thank you for the clarification. I'll clean up the commit history.

CaseyCarter · 2018-06-21T19:34:28Z

The commit history has been so rewritten - after two broken attempts. Apologies for the commit spam.

Add macro \libconcept to tag references to library concepts.

…is document"

* Remove ".lib" * Replace "concepts." with "concept." in the stable names of the subclauses that define specific concepts, and remove the "collection" tags (".corelang", ".object", ".compare", ".callable") so e.g. [concepts.lib.corelang.same] becomes [concept.same] * Replace ".corelang" with ".lang" * Rename [concepts.lib.synopsis] to [concepts.syn] * Rename [concepts.lib.corelang.signedintegral] to [concepts.signed.int] * Rename [concepts.lib.corelang.unsignedintegral] to [concepts.unsigned.int]

…r P0898R3

To be replaceable in context, and add a clarifying example.

… added by P0898R3 with "program-defined" ...to be consistent with the intent of LWG2139

…ents, not parameters

…sistent ...by tagging the specification of common_reference with "Note C" and basic_common_reference "Note D". Also remove the allowance to specialize basic_common_reference "if at least one template parameter in the specialization depends on a program-defined type"; it's redundant with "...pursuant to [namespace.std]". (This is consistent with `common_type`'s wording.)

…normative effect ...except to make some programs ill-formed NDR. Also clarify that users may *partially* specialize basic_common_reference.

... by removing the redundance between the new concept and the "old" requirements. Also fixup the reference in [rand.req.eng] to properly refer [rand.req.urng] instead of the requirements table. The table has been removed, and the reference should have been to the subclause in the first place to clearly incorporate the requirements outside of the table.

... by replacing "a type \tcode{U} such that \tcode{is_same_v<U, remove_cvref_t<U>>} is \tcode{true}" with "a non-reference cv-unqualified type \tcode{U}"

Also fix occurrences introduced by P898R3 in [rand.synopsis], [rand.req.urng], [func.identity], [meta.type.synop], and [meta.trans.other]

Also fix one occurrence in [functional.syn] introduced by P898R3.

Also update "Cpp98"s in the text to "Cpp17" to agree.

... the core language rules already imply it.

…d [concept.unsigned.int]

…vention for requirement names

zygoloid · 2018-06-25T22:57:23Z

Merging to motions branch. Will open a new pull request to merge into master.

CaseyCarter mentioned this pull request Jun 15, 2018

[concepts.lib.compare] has many instances of "const lvalue of type Meow" ericniebler/stl2#548

Closed

CaseyCarter force-pushed the p898_merge branch from 0b66eb1 to e550d3c Compare June 18, 2018 13:55

jensmaurer added this to the post-2018-06 milestone Jun 19, 2018

jensmaurer self-requested a review June 19, 2018 20:35

jensmaurer changed the title ~~P898R3 Standard Library Concepts~~ P0898R3 Standard Library Concepts Jun 19, 2018

jensmaurer requested changes Jun 19, 2018

View reviewed changes

jensmaurer reviewed Jun 19, 2018

View reviewed changes

timsong-cpp reviewed Jun 20, 2018

View reviewed changes

jensmaurer reviewed Jun 20, 2018

View reviewed changes

jensmaurer requested changes Jun 20, 2018

View reviewed changes

jensmaurer added the needs rebase The pull request needs a git rebase to resolve merge conflicts. label Jun 20, 2018

CaseyCarter force-pushed the p898_merge branch from 38be945 to 257f065 Compare June 20, 2018 19:36

CaseyCarter force-pushed the p898_merge branch 3 times, most recently from 533db2b to e8dbe96 Compare June 21, 2018 19:33

CaseyCarter added 23 commits June 25, 2018 08:07

[P0898R3] Standard Library Concepts: full wording

2d4e82c

Add macro \libconcept to tag references to library concepts.

[concepts.lib.general.equality] Replace "this specification" with "th…

af4a3f2

…is document"

[concepts.lib.general.equality] Hyphenate equality-preserving

e800697

[concepts.lang] Retitle as "Language-related concepts"

e9c46dd

[support.limits.general] Add feature test macro __cpp_lib_concepts fo…

31682fb

…r P0898R3

[concepts] Rephrase ocurrences of "must"

9ad9c13

[definitions] redefine expression-equivalent per ISO directives

d8db67f

To be replaceable in context, and add a clarifying example.

[customization.point.object][meta.trans.other] Replace "user-defined"…

d3c70aa

… added by P0898R3 with "program-defined" ...to be consistent with the intent of LWG2139

[concepts.swappable] Correct example

245a8e3

[concepts.general][library.general] concepts constrain template argum…

d5e62d6

…ents, not parameters

[concept.convertibleto] Add clarifying comment

0301e37

[structure.requirements] Rephrase para 8 for clarity

e0aa860

[meta.trans.other] Remove basic_common_reference requirement with no …

089a362

…normative effect ...except to make some programs ill-formed NDR. Also clarify that users may *partially* specialize basic_common_reference.

[meta.trans.other] Simplify common_type

a9981a4

... by replacing "a type \tcode{U} such that \tcode{is_same_v<U, remove_cvref_t<U>>} is \tcode{true}" with "a non-reference cv-unqualified type \tcode{U}"

[concepts] Replace "template <" with "template<"

ebe222d

Also fix occurrences introduced by P898R3 in [rand.synopsis], [rand.req.urng], [func.identity], [meta.type.synop], and [meta.trans.other]

[concepts.syn] Remove trailing ":" from reference comments in synopsis

53b285d

Also fix one occurrence in [functional.syn] introduced by P898R3.

macros.tex: change the \oldconcept prefix to "Cpp17"

acb06e3

Also update "Cpp98"s in the text to "Cpp17" to agree.

[concepts] There need be no "there need be no subsumption relationship"

7677798

... the core language rules already imply it.

[concepts.integral] Merge [concept.integral], [concept.signed.int] an…

6523119

…d [concept.unsigned.int]

[structure.requirements] new para that explains the typographical con…

11877fc

…vention for requirement names

CaseyCarter force-pushed the p898_merge branch from c663726 to 11877fc Compare June 25, 2018 15:13

zygoloid changed the base branch from master to motions-2018-06-lwg-28 June 25, 2018 22:56

zygoloid merged commit c6f48e2 into cplusplus:motions-2018-06-lwg-28 Jun 25, 2018

JohelEGP mentioned this pull request Dec 11, 2018

[concepts] Use "model" instead of "satisfy" for the semantic requirements #2591

Closed

jensmaurer mentioned this pull request Dec 14, 2018

[lib] Referring to standard library requirements. #1263

Open

CaseyCarter deleted the p898_merge branch December 16, 2019 22:15

		@@ -1786,6 +1786,30 @@
		\indextext{requirements!uniform random bit generator\|)}%
		\indextext{uniform random bit generator!requirements\|)}


		\end{itemdescr}

		\rSec2[concepts.lib.corelang.integral]{Concept \tcode{Integral}}


		\rSec1[concepts.lib.compare]{Comparison concepts}

		\rSec2[concepts.lib.compare.general]{General}

P0898R3 Standard Library Concepts #2176

P0898R3 Standard Library Concepts #2176

Conversation

CaseyCarter commented Jun 13, 2018 • edited Loading

jensmaurer commented Jun 19, 2018 • edited Loading

CaseyCarter commented Jun 19, 2018

jensmaurer left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jensmaurer commented Jun 19, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

CaseyCarter Jun 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ericniebler Jun 20, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

timsong-cpp Jun 20, 2018 • edited Loading

Choose a reason for hiding this comment

CaseyCarter commented Jun 13, 2018 •

edited

Loading

jensmaurer commented Jun 19, 2018 •

edited

Loading

CaseyCarter Jun 20, 2018 •

edited

Loading

ericniebler Jun 20, 2018 •

edited

Loading

timsong-cpp Jun 20, 2018 •

edited

Loading