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

[library.headers],[cstdint.syn.2], [cstdint.syn.2] is not in line with intention. Also fixes #3521 #3528

Closed
wants to merge 3 commits into from
Closed

[library.headers],[cstdint.syn.2], [cstdint.syn.2] is not in line with intention. Also fixes #3521 #3528

wants to merge 3 commits into from

Conversation

dawidpilarski
Copy link

@dawidpilarski dawidpilarski commented Dec 6, 2019

this is a fix for issue #3521

  • removed cstdint.syn bullet telling, that cheader provide the definitions same as header.h
  • added bullet to the [library.headers] saying, that if there is corresponding typedef in the header.g for the cheader the typedefs must alias the same type.

@jensmaurer
Copy link
Member

See discussion in #3521. We might want a general statement instead of a point fix.

@jensmaurer jensmaurer added the decision-required A decision of the editorial group (or the Project Editor) is required. label Dec 10, 2019
@dawidpilarski
Copy link
Author

@jensmaurer

I believe what I did is general statement.

I added general note saying, that if there are corresponding typedefs in the header.h they need to be the same. This is regarding all cname vs name.h headers. The normative text is already in place for that:

http://eel.is/c++draft/headers#5

Except as noted in [library] through [thread] and [depr], the contents of each header cname is the same as that of the corresponding header name.h as specified in the C standard library. In the C++ standard library, however, the declarations (except for names which are defined as macros in C) are within namespace scope of the namespace std. It is unspecified whether these names (including any overloads added in [language.support] through [thread] and [depr]) are first declared within the global namespace scope and are then injected into namespace std by explicit using-declarations.

plus I removed the potentially misleading bullet from cstdint.

source/lib-intro.tex Outdated Show resolved Hide resolved
@jensmaurer
Copy link
Member

Ok, then please change the commit message accordingly. Currently, it reads as-if you'd only change [cstdint.syn].
Instructions: https://github.com/cplusplus/draft/wiki/Commit-message-format

@dawidpilarski dawidpilarski changed the title [issue-3521] [cstdint.syn.2] is not in line with intention [library.headers],[cstdint.syn.2], [cstdint.syn.2] is not in line with intention. fixes #3521 Dec 10, 2019
@dawidpilarski dawidpilarski changed the title [library.headers],[cstdint.syn.2], [cstdint.syn.2] is not in line with intention. fixes #3521 [library.headers],[cstdint.syn.2], [cstdint.syn.2] is not in line with intention. Also fixes #3521 Dec 10, 2019
@jensmaurer jensmaurer removed the decision-required A decision of the editorial group (or the Project Editor) is required. label Dec 10, 2019
@jensmaurer
Copy link
Member

jensmaurer commented Dec 10, 2019

For actual types, we have [extern.types].

I'm not seeing in the text that you quoted that types designated by typedefs must be the same. Consider:

typedef int my_int;
typedef __my_int uint32_t;

namespace std {
  typedef long __my_int;
  typedef __my_int uint32_t;
}

Arguably, the presence or absence of implementation-reserved symbols should not make a difference for "same contents", and the two typedefs for uint32_t are token-for-token identical, yet we want such an implementation to be non-conforming. Where do we currently say that, normatively?

@dawidpilarski
Copy link
Author

As to my understanding

the contents of each header cname is the same as that of the corresponding header name.h as specified in the C standard library

doesn't mean, that only specified typedefs definitions needs to be the same. Whole header files content needs to be the same.

So I think, that

typedef int my_int;
typedef __my_int uint32_t;

namespace std {
  typedef long __my_int;
  typedef __my_int uint32_t;
}

will break the

the contents of each header cname is the same as that of the corresponding header name.h as specified in the C standard library.

because, the content will be different with how __my_int is defined.

Standard allows differences only in where the names are defined (in namespace std).

Is my understanding wrong?

@jensmaurer
Copy link
Member

So, my devil implementation has:

// __impl.h
typedef int __my_int;
namespace std { typedef long __my_int; }

cname header:

#include "__impl.h"

namespace std {
  typedef __my_int uint32_t;
}

name.h header:

#include "__impl.h"
typedef __my_int uint32_t;

It seems plausible to read "contents is the same" as being satisfied here. After all, we have [basic.def.odr] if we want to say "same tokens and same meaning", and we're obviously not doing this here.

@dawidpilarski
Copy link
Author

:) You are right. Now I see your point. The note should be normative text in fact. I will change that!

@jensmaurer
Copy link
Member

... and adding normative text is usually not editorial, but (here) LWG matter. Let's ask around whether we can squint the right way in this case.

@jensmaurer jensmaurer added the decision-required A decision of the editorial group (or the Project Editor) is required. label Dec 10, 2019
@dawidpilarski
Copy link
Author

According to the discussion in #3521 it's editorial :
#3521 (comment)

should I ask more people about that? Maybe on the reflectors?

@jwakely
Copy link
Member

jwakely commented Dec 11, 2019

@dawidpilarski

Whole header files content needs to be the same.

I don't know what that means. The whole problem with the current wording is that "the same" is imprecise, and effectively meaningless.

And if I've interpreted you correctly, you're wrong. This should be a conforming implementation:

// <stdint.h>
typedef int __int32_t;
typedef __int32_t int32_t;

// <cstdint>
namespace std {
  using int32_t = signed int;
}

I don't think anybody would reasonably argue that the "whole header file content" is "the same". That's why I want to change the wording to be precise, and say that the typedefs specified in the standard denote the same types. That doesn't prevent implementations adding other declarations to the <name.h> header and/or the <cname> header (using reserved names as appropriate) and doesn't require identical tokens.

@jensmaurer

I'm not seeing in the text that you quoted that types designated by typedefs must be the same.

That's certainly the intention, but if it's not clear then let's open an LWG issue.

@dawidpilarski
Copy link
Author

@jwakely can you confirm then, that the introduced change fixes that issue and it's editorial?
Should whole headers#5 disappear, because of it's impreciseness?

@jwakely
Copy link
Member

jwakely commented Dec 11, 2019

I don't get to decide what is and isn't editorial.

Removing the whole paragraph also removes the wording about the macros. The new paragraph you're adding doesn't say anything about macros.

@dawidpilarski
Copy link
Author

@jwakely @jensmaurer

can I please ask for further guidance? Shall I:

  • create LWG issue,
  • ask on reflectors, whether this kind of change is editorial or not,
  • write a paper,
  • something else?

@jwakely
Copy link
Member

jwakely commented Dec 11, 2019

I suggest an LWG issue. https://cplusplus.github.io/LWG/lwg-active.html#submit_issue

@jensmaurer
Copy link
Member

The editorial group will get together in Prague. We can decide there.

@jwakely
Copy link
Member

jwakely commented Jan 31, 2020

It's just come to my attention that http://wg21.link/extern.types already requires ::int32_t and std::int32_t to be the same type.

@jensmaurer
Copy link
Member

Editorial meeting: C standard library headers are a different thing than C++ library headers, even if they have notionally the same name. The specification is incorporating the definition of the C std library headers into the definition of the C++ <cblah> header.

@jensmaurer jensmaurer closed this Feb 10, 2020
@jensmaurer jensmaurer removed the decision-required A decision of the editorial group (or the Project Editor) is required. label Feb 10, 2020
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.

None yet

4 participants