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

CWG Poll 1: P2103R0 (Core Language Changes for NB Comments at the February, 2020 (Prague) meeting #3782

Merged
merged 6 commits into from Feb 25, 2020

Conversation

zygoloid
Copy link
Member

@zygoloid zygoloid commented Feb 21, 2020

Fixes NB US028, US033, US041, CA104, CA107, US115, and US117 (C++20 CD).

Fixes #3685.
Fixes cplusplus/papers#829.
Also fixes cplusplus/nbballot#27.
Also fixes cplusplus/nbballot#32.
Also fixes cplusplus/nbballot#40.
Also fixes cplusplus/nbballot#103.
Also fixes cplusplus/nbballot#106.
Also fixes cplusplus/nbballot#114.
Also fixes cplusplus/nbballot#116.

Did not remove all the indicated words in [module.global]p4. We don't
define what it means to be decl-reachable from a declaration-seq, only
from a declaration, so the removed words here are still necessary.
NB US 117 (C++20 CD): Comparing types and type-constraints
Added obviously-missing (if any) to the mention of a trailing
requires-clause in the definition of signature for a friend function
template.
@jensmaurer jensmaurer added this to the post-2020-02 milestone Feb 21, 2020
@jensmaurer jensmaurer changed the title P2103R0 (Core Language Changes for NB Comments at the February, 2020 (Prague) meeting CWG Poll 1: P2103R0 (Core Language Changes for NB Comments at the February, 2020 (Prague) meeting Feb 21, 2020
@burblebee burblebee self-assigned this Feb 22, 2020
Copy link
Contributor

@burblebee burblebee left a comment

Choose a reason for hiding this comment

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

See issue with CA107 & US117 - can the wording be fixed? Otherwise, looks good.

\end{example}
This similarity includes the situation where a program is ill-formed, no diagnostic required,
when the meaning of the program depends on whether two constructs are equivalent,
and they are functionally equivalent but not equivalent.
Copy link
Contributor

Choose a reason for hiding this comment

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

What is "they"? The "two constructs"? How can they be "equivalent" and "not
equivalent" at the same time?

Copy link
Contributor

Choose a reason for hiding this comment

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

That portion is a quote from [temp.over.link]/7. I am having a hard time finding another interpretation of "they" that would necessitate deviating from the formulation in [temp.over.link]. As for the latter question, they can (at the same time) be "functionally equivalent" and "not equivalent".

Copy link
Contributor

Choose a reason for hiding this comment

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

This doesn't help me - I read "they" as the "two constraints", and this makes no sense. Yes, entities can (at the same time) be "functionally equivalent" and "not equivalent". but here we're saying that they can be "equivalent" and "not
equivalent" at the same time: "two constructs are equivalent,
and they are functionally equivalent but not equivalent."

Copy link
Member

@jwakely jwakely Feb 25, 2020

Choose a reason for hiding this comment

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

No, that's not what it says. It is talking about cases where the program depends on whether they are equivalent, but they are not equivalent. That's not "they are equivalent and not equivalent", it's "they need to be equivalent, but they are not".

You are selectively quoting, and ignoring the "when the meaning of the program depends on whether ..." part.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok, thanks to @jwakely, I finally get what this wording is trying to say, in which case the "and" should be changed to "but" at the very least, or preferably reworded so that it's not possible to parse it as I did.

Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
and they are functionally equivalent but not equivalent.
+but they are merely functionally equivalent and not equivalent.

Copy link
Contributor

Choose a reason for hiding this comment

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

@hubert-reinterpretcast Much better, thank you! Sadly, Richard has already merged this motion, so we'll have to create a separate PR for this and your suggestion below (for "one or more constraints"). Would you mind doing that since it's your wording?

Copy link
Contributor

Choose a reason for hiding this comment

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

On reading this again, I am having trouble with the change I suggested above. I put in a more extensive change in #3807.


\item the (unique) object (if any) that contains the original object
as a subobject\iref{intro.object} is transparently replaceable by
the (unique) object (if any) that contains the new object.
Copy link
Contributor

Choose a reason for hiding this comment

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

The last should probably say "contains the new object as a subobject".

Copy link
Member Author

Choose a reason for hiding this comment

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

Please consider #3796.

@@ -271,6 +280,17 @@
and
trailing \grammarterm{requires-clause}\iref{dcl.decl} (if any)

\indexdefn{signature}%
\definition{signature}{defns.signature.templ.friend}
\defncontext{friend function template with constraint involving enclosing template parameters}
Copy link
Contributor

Choose a reason for hiding this comment

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

I believe this should say "one or more constraints".

Copy link
Contributor

Choose a reason for hiding this comment

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

Opened #3806.

@zygoloid zygoloid merged commit d046aa6 into master Feb 25, 2020
hubert-reinterpretcast pushed a commit to hubert-reinterpretcast/draft that referenced this pull request Feb 27, 2020
Adjusts a note identified by cplusplus#3782 as being difficult to
parse.
hubert-reinterpretcast pushed a commit to hubert-reinterpretcast/draft that referenced this pull request Feb 27, 2020
Adjusts a note identified by cplusplus#3782 as being difficult to
parse.
hubert-reinterpretcast pushed a commit to hubert-reinterpretcast/draft that referenced this pull request Feb 27, 2020
Adjusts a note identified in cplusplus#3782 as being difficult to
parse.
with a constraint that depends on a template parameter from an enclosing template
shall be a definition.
Such a constrained friend function or function template declaration
does not declare the same function or function template as a declaration in any other scope.
Copy link

Choose a reason for hiding this comment

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

How is "in any other scope" to be interpreted here? [class.friend]/7 mentions lexical scope (of the class) for hidden friend definitions, but namespace scope, as per [class.friend]/6, for the function itself.

As per [over.dcl]/1 two function declarations refer to the same function only if they have the same trailing requires-clauses (if any). This should arguably also apply for non-template hidden friend definitions which differs only in their trailing requires-clauses. The non-normative text of [over.dcl]/1 contains an example for this particular case but for non-template member functions as compared to hidden friends.

This seems to have a few compilers confused, at least, as GCC, Clang and MSVC all (using current HEAD) implement different behaviours for overloading hidden friends only by different trailing requires-clauses:

struct Base {};

template<int N>
struct S : public Base {
    friend int f(Base&) requires (N == 1) { return 1; }
    friend int f(Base&) requires (N == 2) { return 3; }
    // Separate functions (which overload resolution will
    // have a hard time to disambiguate)?
};

int main() {
    S<1> s1{};
    S<2> s2{};    // GCC error: redefinition 
    (void)f(s1);  // MSVC error: ambiguous call (correct?)
    (void)f(s2);
    // Clang error: definition with same mangled name
    //              '_Z3fooR4Base' as another definition
}

Is MSVC correct here?

Should I be filing compiler bug reports for this, or should the normative text possibly be updated to be more explicit regarding this special case?

Copy link
Contributor

Choose a reason for hiding this comment

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

I think MSVC is correct here. With respect to the Clang behaviour, the description of the signature indicates that the mangling should be unique. With respect to the GCC behaviour, the "does not declare" wording is meant to say that the friends declared by each specialization is unique.

Copy link

@dfrib dfrib Nov 10, 2020

Choose a reason for hiding this comment

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

Hubert, thanks for the answer and the clarification; it's clear that Clang is wrong by violation of [defns.signature.friend], and that GCC is wrong as per the intended meaning of [temp.friend]/9.

However, I'm wondering what is the actual correct behaviour is here; is MSVC really correct, or is "correct" even well-specified?

I'm having a hard time finding the normative reference for constraints checking when finding viable candidates in overload resolution for the case of overloaded, constrained hidden friends, where the signatures solely differ by means of the their (mutually exclusive) trailing requires-clauses. Particularly, whether and how constraints checking will be performed during overload resolution on a free (friend) non-template function.

Starting with lookup, [basic.lookup.argdep]/4.3 states that any (namespace-scope) friend function

[...] declared in classes with reachable definitions in the set of associated entities are visible [...]

can be found via ADL on associated entities. Thus, as per [basic.lookup.argdep]/2.2, for the calls f(s1) and the f(s2) in the example above the associated entity is the class template itself, even if the arguments are objects of particular specializations of the class template. This would arguably mean that ADL, for both calls f(s1) and f(s2), can find both overloads as candidate functions? So far, this is the interpretation MSVC has done, as it proceeds (and subsequently fails) with overload resolution.

For finding viable functions during overload resolution, [over.match.viable]/3 applies:

Second, for a function to be viable, if it has associated constraints ([temp.constr.decl]), those constraints shall be satisfied ([temp.constr.constr]).

but the case of hidden friends with associated constraints is special in this regard. In say the call f(s1), how would constraint satisfaction be checked? At this point, given that ADL has actually found both candidates (as is the interpretation of MSVC), the constraint is in no apparent way connected to the particular argument (which is an object of the specialization which uniquely defined one of the overloads).

Intuitively, the only plausible variations are that

  • (A) constraints satisfaction cannot evaluated, and both candidate functions should be considered non-viable, or that
  • (B) ADL should never find a constrained hidden friend as a candidate function for lookup on a specialization for which the friend's constraint is not satisfied.

(A) would result in the program above being ill-formed (no viable function as constraints cannot be checked for satisfaction), but this would generalize also to non-overloaded hidden friends and thus possibly not be applicable.

This leads us to (B), which would make the program above well-formed by only finding candidate functions whose constraints are actually satisfied, but afaict constraint satisfaction is not part of finding candidates, but only of the later step of rejecting candidates when create the set of viable candidates.

None of (A) or (B) are implemented by MSVC, which seemingly accepts both overloads as viable and fails due to ambiguity when attempting to find the best viable function (no constraints checking?).

Copy link
Contributor

Choose a reason for hiding this comment

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

Starting with lookup, [basic.lookup.argdep]/4.3 states that any (namespace-scope) friend function

[...] declared in classes with reachable definitions in the set of associated entities are visible [...]

can be found via ADL on associated entities. Thus, as per [basic.lookup.argdep]/2.2, for the calls f(s1) and the f(s2) in the example above the associated entity is the class template itself, even if the arguments are objects of particular specializations of the class template. This would arguably mean that ADL, for both calls f(s1) and f(s2), can find both overloads as candidate functions? So far, this is the interpretation MSVC has done, as it proceeds (and subsequently fails) with overload resolution.

Yes, it seems it should have succeeded because only one of the candidate functions is viable in each lookup result.

In say the call f(s1), how would constraint satisfaction be checked?

The constraint satisfaction can be checked in the context of the template instantiation.

@jensmaurer jensmaurer deleted the motions-2020-02-cwg-1 branch February 12, 2021 20:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment