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

Please use uint8_t instead of unsigned char etc #1283

Open
safinaskar opened this Issue Aug 8, 2018 · 9 comments

Comments

Projects
None yet
6 participants
@safinaskar

safinaskar commented Aug 8, 2018

Subsection "TypedArray Objects" has a table. The table has a column "Equivalent C Type", which currently reads: "signed char", "unsigned char", "short" etc. Unfortunately, C standard doesn't guarantee exact size for all this types (even for chars). So, please use types int8_t, uint8_t, int16_t etc instead, or, better, simply remove this column

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Aug 8, 2018

Member

@safinaskar That sounds like a great idea. Would you like to make a PR for this change?

Member

littledan commented Aug 8, 2018

@safinaskar That sounds like a great idea. Would you like to make a PR for this change?

@safinaskar

This comment has been minimized.

Show comment
Hide comment
@safinaskar

safinaskar Aug 8, 2018

@littledan, no. I don't know what you will decide: to fix column or to remove it

safinaskar commented Aug 8, 2018

@littledan, no. I don't know what you will decide: to fix column or to remove it

@safinaskar

This comment has been minimized.

Show comment
Hide comment
@safinaskar

safinaskar Aug 8, 2018

@littledan, anyway I am not interested in making trivial PRs

safinaskar commented Aug 8, 2018

@littledan, anyway I am not interested in making trivial PRs

@kaicataldo

This comment has been minimized.

Show comment
Hide comment
@kaicataldo

kaicataldo Aug 8, 2018

What's the required change here? I'd love to make a trivial PR!

kaicataldo commented Aug 8, 2018

What's the required change here? I'd love to make a trivial PR!

@jridgewell

This comment has been minimized.

Show comment
Hide comment
@jridgewell

jridgewell Aug 8, 2018

Member

@kaicataldo: Change the column values to use int8_t, etc.

Member

jridgewell commented Aug 8, 2018

@kaicataldo: Change the column values to use int8_t, etc.

@devsnek

This comment has been minimized.

Show comment
Hide comment
@devsnek

devsnek Aug 8, 2018

Contributor

@kaicataldo in the table here: https://tc39.github.io/ecma262/#sec-typedarray-objects

there's a column called "Equivalent C Type." it currently contains types like signed char and double. However these types actually do not have any specified width, unlike the TypedArray types they are in the table with.

The fix would be to either change stuff like signed char to int8_t and unsigned char to uint8_t etc, or to just remove the column entirely.

Personally I would just remove the column entirely, the other columns are more than enough information.

Contributor

devsnek commented Aug 8, 2018

@kaicataldo in the table here: https://tc39.github.io/ecma262/#sec-typedarray-objects

there's a column called "Equivalent C Type." it currently contains types like signed char and double. However these types actually do not have any specified width, unlike the TypedArray types they are in the table with.

The fix would be to either change stuff like signed char to int8_t and unsigned char to uint8_t etc, or to just remove the column entirely.

Personally I would just remove the column entirely, the other columns are more than enough information.

@littledan

This comment has been minimized.

Show comment
Hide comment
@littledan

littledan Aug 9, 2018

Member

I am on the fence here. When I was implementing TypedArray methods, I felt like that column was kind of silly and not meaningful. However, looking back, it seems like some good exposition for new readers. If we keep it, I think it would be nice to make it clear that it is non-normative (i.e. a note).

Member

littledan commented Aug 9, 2018

I am on the fence here. When I was implementing TypedArray methods, I felt like that column was kind of silly and not meaningful. However, looking back, it seems like some good exposition for new readers. If we keep it, I think it would be nice to make it clear that it is non-normative (i.e. a note).

@safinaskar

This comment has been minimized.

Show comment
Hide comment
@safinaskar

safinaskar Aug 10, 2018

Also, C standard doesn't guarantee that float, double etc confirm to IEEE. So, we have to leave corresponding cells empty. I still think that the best solution is to remove this column. Even if you choose to keep it, please state that this column is informational

safinaskar commented Aug 10, 2018

Also, C standard doesn't guarantee that float, double etc confirm to IEEE. So, we have to leave corresponding cells empty. I still think that the best solution is to remove this column. Even if you choose to keep it, please state that this column is informational

@kaicataldo

This comment has been minimized.

Show comment
Hide comment
@kaicataldo

kaicataldo Aug 28, 2018

Sorry - had a family emergency and lost track of this. Sounds like there isn't consensus on the best way forward yet, so I'll hold off on a PR for now.

kaicataldo commented Aug 28, 2018

Sorry - had a family emergency and lost track of this. Sounds like there isn't consensus on the best way forward yet, so I'll hold off on a PR for now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment