Skip to content

CWG2781 [basic.def.odr] "... for D itself, the behavior is as if there is a single entity with a single definition" renders the complere ODR teethless #402

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

Open
RealLitb opened this issue Aug 18, 2023 · 10 comments

Comments

@RealLitb
Copy link

The ODR applies to multiple definitions of the same entity. For example this is rendered IFNDR by https://eel.is/c++draft/basic.def.odr#14.5 :

In each such definition, corresponding names, looked up according to [basic.lookup], shall refer to the same entity, ...

int x;
inline int f() { return x; } // refers to different entities!

But there is a rule in p15:

For each such entity and for D itself, the behavior is as if there is a single entity with a single definition, including in the application of these requirements to other entities.

Therefore, the above code example is not anymore IFNDR because the behave is "as if" there is only a single definition of f? I hear that this is not the case, because the requirements still apply. So if we change f to this:

inline int f() { return []{return 0;}(); } 

If we check the requirements in p14 and then for the remainder of the spec behave as if there was only a single definition of D (which is what prior specs said), then the requirement in p14 that says this is violated and the function f becomes IFNDR:

In each such definition, except within the default arguments and default template arguments of D, corresponding lambda-expressions shall have the same closure type

@t3nsor
Copy link

t3nsor commented Aug 18, 2023

Possible duplicate of #271

@RealLitb
Copy link
Author

Unfortunately #271 is closed and the wording is still highly ambiguous. I see no way to read p14 and p15 to make the first example IFNDR and the second example wellformed.

@t3nsor
Copy link

t3nsor commented Aug 18, 2023

If enough people want #271 fixed, maybe it will get reopened :)

@jensmaurer
Copy link
Member

jensmaurer commented Aug 18, 2023

A clarification question:

For your initial example, I'm missing a bit of context:

int x;
inline int f() { return x; } // refers to different entities!

Is this supposed to appear in several translation units of a program?
x (alone) has external linkage and is a definition here, so this seems to be IFNDR right there (due to multiple definitions for x). Or did you mean to put static in front of int x?

Also, you seem to be referring a rule in p15 that is now in p16:

These requirements also apply to corresponding entities defined within each definition of D (including the closure types of lambda-expressions, but excluding entities defined within default arguments or default template arguments of either D or an entity not defined within D). For each such entity and for D itself, the behavior is...

I feel "For each such entity" clearly refers to "corresponding entities defined within each definition of D".

Are we just missing something like "For each such entity and for D itself where these requirements are satisfied, the behavior is..." ?

@t3nsor
Copy link

t3nsor commented Aug 18, 2023

I think that was the thrust of @RealLitb 's concern with the wording, but then we run into the issue from #271. It would imply that you check all the conditions in p14 first before you can assume any of the guarantees provided by p15, which is not the intent.

@RealLitb
Copy link
Author

A clarification question:

For your initial example, I'm missing a bit of context:

int x;
inline int f() { return x; } // refers to different entities!

Is this supposed to appear in several translation units of a program? x (alone) has external linkage and is a definition here, so this seems to be IFNDR right there (due to multiple definitions for x). Or did you mean to put static in front of int x?

Yes, sorry. I meant to put "static" in front of "int x". The code is meant to appear in multiple TUs.

Also, you seem to be referring a rule in p15 that is now in p16:

These requirements also apply to corresponding entities defined within each definition of D (including the closure types of lambda-expressions, but excluding entities defined within default arguments or default template arguments of either D or an entity not defined within D). For each such entity and for D itself, the behavior is...

I feel "For each such entity" clearly refers to "corresponding entities defined within each definition of D".

Are we just missing something like "For each such entity and for D itself where these requirements are satisfied, the behavior is..." ?

I think this issue report is a dupe of #271 (which I definitely should have read more closely before opening mine). Even if you add the text "where these requirements are satisfied", then it means that for cases like in #271 or in cases like in my lambda example (where there is a local declaration of a struct - that albeit is made to have the token sequence of the lambda expression for ODR purposes, still declares a new local type - that would presumably, because it has no linkage, then declare a different entity in each of the TUs that the inline definition appears in).

There is a note in [dcl.inline]p6 (https://eel.is/c++draft/dcl.inline#6) that says

[Note 3: An inline function or variable with external or module linkage can be defined in multiple translation units ([basic.def.odr]), but is one entity with one address.
A type or static variable defined in the body of such a function is therefore a single entity.
— end note]

However, the normative text that guarantees this appears to rely entirely on the "for D itself, the behavior is as if there is a single entity with a single definition" part of the ODR. So it's important that we first apply that rule, to then satisfy the requirement in p14 that names within the inline definition that happen to refer to entities defined therein refer to the same entity. Inserting "where these requirements are satisfied" reverses the order, however, and therefore makes the lambda example and the example in #271 fail with IFNDR.

@RealLitb
Copy link
Author

I think I see now what you are saying, which I was missing at first. I think it works if we only apply p15 to D if the requirements are satisfied, but apply them to "each such entity" regardless of whether they are satisfied (in the check of D, but not in the (recursive) check for each of the entities. So in the recursive checks, all definitions are checked again.

So I think I would recommend:

If the requirements are satisfied, the behavior for D is as if there is a single entity with a single definition, including in the application of these requirements to other entities. This behavior also applies to each corresponding entity in the above requirements on definitions of D.

Maybe with a clarifying note in the comments.

@jensmaurer
Copy link
Member

First of all, let's please use the latest working draft https://isocpp.org/files/papers/N4958.pdf for any specific discussions. I believe it contains minor editorial improvements that may or may not be helpful for this discussion. At least, we should use its paragraph number for further discussion.

Furthmore, let's consider #271 dead (even though this issue might be a duplicate), and make sure we reach a common understanding here.

Please bear with me while I walk through the examples that have been presented here, and please tell me where I take the wrong turn according to your reading of the wording. All references are to [basic.def.odr].

static int x;
inline int f() { return x; } // refers to different entities!

Here, we want to find out whether the definition of f is IFNDR, which is our "D".

p14.2 tells use we need to check "the following requirements". p14.5 says we need to check the lookup results for x. Those refer to distinct entities in different translation units, so the requirements are not met. The rule "For each such entity and for D itself, the behavior is as if there is a single entity" doesn't change anything, because x is not defined within D. However, it does make f have a single definition for purposes of the ODR for any other checking we need to do elsewhere.

Now, on to the case in #271, slightly simplified:

inline void f2(){
   int a = 0;
   int b = a;
}

We again arrive at p14.5 for D = f2. We have to check whether a refers to the same entity in both definitions. At first sight, it doesn't, but let's read on.

p16 says "These requirements also apply to corresponding entities defined within each definition of D". So, the requirements apply (recursively) to a, for which they are satisfied. In the invocation of the rules for D = a, we arrive at " ... for D itself, the behavior is as if there is a single entity with a single definition, including in the application of these requirements to other entities."

So, we consider "a" to have a single definition, and we use that knowledge when resuming the checking for D = f2, where we also reach the "single definition" conclusion.

I do agree that a reference to the recursive requirements checking on a does not clearly imply that we do the "as-if single definition" thing at every recursion step.

@jensmaurer jensmaurer changed the title [basic.def.odr] "... for D itself, the behavior is as if there is a single entity with a single definition" renders the complere ODR teethless CWG2781 [basic.def.odr] "... for D itself, the behavior is as if there is a single entity with a single definition" renders the complere ODR teethless Aug 19, 2023
@jensmaurer
Copy link
Member

@xmh0511
Copy link

xmh0511 commented Aug 22, 2023

Why do we not prefer to rearrange a similar assumption to the proposed wording

This behavior also apply applies to corresponding entities defined within each definition of D (including the closure types of lambda-expressions, but excluding entities defined within default arguments or default template arguments of either D or an entity not defined within D)

before the requirement list that is used to check D? That is before we check some D, we first have an assumption that all entities that are defined or be considered defined within the definition of D are assumed as if they have the same single definition such that we can check whether D violate or obey ODR-rule, this is natural like we invoke a program for checking.

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

No branches or pull requests

4 participants