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

[temp.over.link] Reword to clarify that declarations correspond #5999

Merged
merged 4 commits into from
Aug 5, 2024

Conversation

tkoeppe
Copy link
Contributor

@tkoeppe tkoeppe commented Nov 22, 2022

Fixes ##5997.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 22, 2022

@burblebee, @opensdh, @jensmaurer: For your consideration, please!

@W-E-Brown
Copy link
Contributor

W-E-Brown commented Nov 22, 2022 via email

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 22, 2022

@W-E-Brown: Could you please tell me how you understand the original wording? I also find this a bit weird, but I don't think this change is at fault here -- the problem for me already resided in the original third bullet, "would declare the same entity considering them to correspond". I thought I'm just preserving that weirdness. If you tell me what the original meant, I can try and look for a clearer rewording.

@W-E-Brown
Copy link
Contributor

W-E-Brown commented Nov 23, 2022 via email

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 24, 2022

The wording came in fresh out of Core with CWG2603, and I wouldn't normally second-guess Core's deliberations and care. @jensmaurer, @opensdh Could you perhaps shed some light on how "considering them to correspond" is to be interpreted?

@jensmaurer
Copy link
Member

jensmaurer commented Nov 24, 2022

The idea of that bullet is to do a "specification function call" to "same entity" with the override "the declarations do correspond" (although, in fact, they don't).

"Same entity" says in 6.6 p8:

Two declarations of entities declare the same entity if, considering declarations of unnamed types to introduce their names for linkage purposes, if any (9.2.4, 9.7.1), they correspond (6.4.1), have the same target scope that is not a function or template parameter scope, and either ...

We want to have this list of conditions checked, except that "they correspond" should be assumed to be true for that specification function call.

"considering them to correspond" seems to be on-point for that.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 24, 2022

OK, I get that, but I still think "if two function templates [...] would declare the same entity considering them to correspond" is a pretty awkward way to say that. I think the fact that the "them" refers to something quite distant, and also to the fact that you're expected to spot that "correspond" is a detail of "same entity" make this quite a strain.

In other words, this wording makes sense if you already have your explanation in mind, but the explanation itself seems hard to discover just based off this wording.

Would it hurt to just spell this reasoning out?

"""

  • would declare the same entity, where in the determination in \ref{basic.link} of whether two declarations do declare the same entity, the declarations are assumed to correspond, and

"""

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 24, 2022

Maybe we can decorate the whole sentence a bit more:

""
Furthermore, if two declarations $A$ and $B$ of function templates that do not correspond

  • declare the same name,
  • have corresponding signatures,
  • would declare the same entity, where in the determination in \ref{basic.link} of whether two declarations do declare the same entity, $A$ and $B$ are assumed to correspond, and
  • accept and are satisfied by the same set of template argument lists,

the program is ill-formed [...]
"""

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Nov 24, 2022

We could also move the punchline (that the declarations do not correspond) to the end:

"""
Furthermore, if two declarations $A$ and $B$ of function templates [List] but do not correspond, the program is ill-formed [...]
"""

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jul 31, 2024

image

@jensmaurer, @opensdh What do you think of this suggestion?

@jensmaurer
Copy link
Member

I like moving the punchline. I'm not too happy with that third bullet; it feels too wordy.

maybe

"would declare the same entity, when considering A and B to correspond in the determination per 6.6,"

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jul 31, 2024

Done:

image

@jensmaurer
Copy link
Member

@opensdh, what do you think?

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Jul 31, 2024

Even more radical, and I'm not actually proposing that:

===
The program is ill-formed, no diagnostic required, if

  • the validity or meaning of the program [...] and they are functionally equivalent but not equivalent, or
  • two declarations $A$ and $B$ do not correspond, but they
    • declare the same name,
    • have corresponding signatures,
    • would declare the same entity [...], and
    • accept and are satisfied by the same set of template argument lists.

===

@opensdh
Copy link
Contributor

opensdh commented Aug 5, 2024

I like 58b98d1 (though perhaps with some slight rewordings like "declare the same name" → "introduce the same name" and "to correspond in the determination per 6.6," → "in that determination (6.6),"). The whole point here is to recompute the same-entity status with a deliberately weaker filter just as with "functionally equivalent but not equivalent".

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Aug 5, 2024

Thanks, @opensdh, I applied that!

@tkoeppe tkoeppe requested a review from jensmaurer August 5, 2024 10:34
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.

Works for me.

@tkoeppe
Copy link
Contributor Author

tkoeppe commented Aug 5, 2024

Thanks, everyone!

@tkoeppe tkoeppe merged commit 15a43d5 into cplusplus:main Aug 5, 2024
2 checks passed
@tkoeppe tkoeppe deleted the corrfunc branch August 5, 2024 11:22
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.

4 participants