Skip to content

[docs]: warning for long, culong being OS-dependent #25012

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

Merged
merged 4 commits into from
Jun 27, 2025

Conversation

ZoomRmc
Copy link
Contributor

@ZoomRmc ZoomRmc commented Jun 22, 2025

Docs are routinely compiled on a different OS so often don't reflect reality of CT-conditionals.

I bet there's a few of other places like this in the stdlib.

Docs are routinely compiled on a different OS so often
don't reflect reality of CT-conditionals.
@arnetheduck
Copy link
Contributor

they're not int or int32 etc on any platform really - rather you have to use them consistently when you use them - ie a better doc would say that they are of an undefined integer type on the nim side (since one might imagine changing/adapting them in the future for some new c library)

@ZoomRmc
Copy link
Contributor Author

ZoomRmc commented Jun 23, 2025

a better doc would say that they are of an undefined integer type on the nim side

That isn't very helpful, isn't it?

I understand that C types are allowed to be implementation-defined within constraints, but that's like a warning to a warning. This PR is to fix a blatant lie when your docs were compiled on different type of OS. With it your code might possibly still be incorrect, but at least it will compile if you follow the docs 🤦.

Anyway, Nim sources are rather unnuanced on the topic - they assume LLP64 for Windows and LP64 for everything else.

BTW, what C standard does Nim aim for? Fixed-sizes ints have been there since C99, I believe.

since one might imagine changing/adapting them in the future for some new c library

Not sure I get you, why would you adapt basic types for some C library?

@arnetheduck
Copy link
Contributor

arnetheduck commented Jun 23, 2025

That isn't very helpful, isn't it?

it's very helpful: it tells the reader that if they want a specific behavior, they have to define it, ostensibly by using other types in their public API.

The issue is that int, long etc is that they don't have a fixed size in C, so you can't expect them to have a fixed size or type on the nim side - they can be anything really (as long as they respect the minimum sizes - ie int can be 8 or 16 bytes for example) and representing them with any one specific type on the nim side is wrong - in fact, it's a bit of a bummer that these types are not defined as type clong {.emit: "clong".} = object; payload: int32/int64/etc because they are not semantically equivalent to any Nim integer and at least this way, it would be clear.

The use case for any cxxx type is to faithfully represent a C ABI .. for example void f(int* x) and void f(long* x) are two distinct function signatures even if sizeof(int) == sizeof(long) and you need to use the right one for nim to have a chance at correctly interacting with an api like this.

I'm not sure what problem you're facing but I can't think of any where relying on that a particular nim type is used for any of these C types.

Nim sources are rather unnuanced

Nim sources are rather unnuanced (some would call them buggy), for a lot of things.

Fixed-sizes ints

On the C side, you're maybe confusing int with int32_t and friends?

Not sure I get you,

If nim were to become more nuanced, it would potentially have to be more creative in its int usage on the nim side - ie when supporting x32 or 128-bit hardware.

@arnetheduck
Copy link
Contributor

OS-dependent: int on non-Windows.

to be clear: the problem with this statement is that it asserts that on non-windows, you get an int - this is not a promise to make since it's eventually going to be incorrect depending on what those platforms end up being. Nim in particular makes the promise that int and pointer have the same size - this is not a guarantee that C makes and herein lies the semantic gap between the two languages.

@Araq
Copy link
Member

Araq commented Jun 23, 2025

Well if you want to warn, make the warnings reflect reality. The reality is these types are ABI defined. (Also, we don't generate "C code", we generate "GNU C", "MSV C", etc. avoiding most of C's problems. (You cannot even memcmp two structs in C due to their unspecified alignment gaps...))

@ZoomRmc
Copy link
Contributor Author

ZoomRmc commented Jun 23, 2025

On the C side, you're maybe confusing int with int32_t and friends?

I'm not confusing, I'm specifically asking about int32_t "and friends" and whether Nim could possibly define its types with those.

so you can't expect them to have a fixed size or type on the nim side

Well, I can (the latter), as Nim int is defined as not having a fixed size but is equal to a pointer.

I'm not sure what problem you're facing but I can't think of any where relying on that a particular nim type is used for any of these C types.

I bumped into some code that converted between long and int in a non-portable way so the code didn't compile but the docs were misleadingly saying there shouldn't be an issue. I know better so was annoyed the docs are inconsistent with Nim implementation. The points @arnetheduck argues for are all right but they're just on another level that's rooted in how Nim defines its base types and not just what the docs say, so deserves its own discussion/RFC and lie outside of this PR's scope.

@ZoomRmc
Copy link
Contributor Author

ZoomRmc commented Jun 24, 2025

I though about reducing the branching to just conditionally defining C[u]longImpl and using that, doing away with the special case for nimdoc, but this will lead to those types leaking into error messages, instead of more helpful int/int32 types.

@arnetheduck
Copy link
Contributor

arnetheduck commented Jun 24, 2025

instead of more helpful

One could argue that the implementation-defined typedef is more helpful since the user of these api is not supposed to rely on these being of any specific (nim) type (or size!) and should always explicitly convert them back and forth if they happen to interact with nim types - the typedef will guide them towards the documentation instead of thinking that these are normal Nim integers.

Put another way, whenever you cross the boundary between nim and C, you need to include explicit conversion code that also checks for things like the size of the type.

@ZoomRmc
Copy link
Contributor Author

ZoomRmc commented Jun 24, 2025

Extending this line of arguments, one could say there's not much point of really providing any Nim type implementation in the type's signature (distinct object - unless Nim can request the size from the C compiler and choose the suitable Nim equivalent at CT).

Then we can safely use those types casting to whatever, as God commanded.

@arnetheduck
Copy link
Contributor

not much point of really providing any Nim type

well, that's what implementation-defined means in this context - we want to hide it as far as possible so that developers don't rely on it being anything in particular because its key property is being an importc: "long".

re ct, type clong{.importc: "clong", size: 4, alignment: 4, byval, ....} (without =) would be a perfectly adequate way to express this thing.. slightly onerous with that approach is that you'd have to redefine + and so on .. then you would also have to decide if clong should be covered by range / overflow checking - in C it is undefined again, what happens on overflow, so one could make the argument that Nim should not perform such checks either etc. it all depends on how far beyond the current 80/20 approach you want to take things.

@ZoomRmc
Copy link
Contributor Author

ZoomRmc commented Jun 24, 2025

re ct, type clong{.importc: "clong", size: 4, alignment: 4, byval, ....} (without =) would be a perfectly adequate way to express this thing.. slightly onerous with that approach is that you'd have to redefine + and so on .. then you would also have to decide if clong should be covered by range / overflow checking - in C it is undefined again, what happens on overflow, so one could make the argument that Nim should not perform such checks either etc. it all depends on how far beyond the current 80/20 approach you want to take things.

Ok, I wrote a long and slightly too personal of a response which boils down to "thanks, sorry, out of scope"1. I fully understand the seriousness of the implications of that whole mountain of leaky abstractions built on top of C, but considering personal circumstances, I'm not the one to attempt to fix it even slightly, for the moment.

I'd appreciate direct objections or GH-powered suggestions to this (initially drive-by by intention) PR.

Sorry for not being receptive enough.

Footnotes

  1. scope being semantic behaviour of the compiler not corresponding with the state of docs as rendered and published

@arnetheduck
Copy link
Contributor

reducing the branching to just conditionally defining

my comment was in support of using this style, not sure gh-suggest is the right avenue for it. it's not a blocker for the PR, merely an idea to reduce maintenance for a future in which this gets fixed propery - I certainly agree that the proper fix is out of scope, this is a messy corner.

@Araq Araq merged commit 6bdb069 into nim-lang:devel Jun 27, 2025
18 checks passed
Copy link
Contributor

Thanks for your hard work on this PR!
The lines below are statistics of the Nim compiler built from 6bdb069

Hint: mm: orc; opt: speed; options: -d:release
182824 lines; 8.672s; 659.531MiB peakmem

narimiran pushed a commit that referenced this pull request Jun 27, 2025
Docs are routinely compiled on a different OS so often don't reflect
reality of CT-conditionals.

I bet there's a few of other places like this in the stdlib.

(cherry picked from commit 6bdb069)
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.

3 participants