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

Platform dependent public API #8524

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

jan-zajic
Copy link
Contributor

This PR introduce Platform annotation, which should be used to mark platform dependent API's in Std lib or any other libraries. Example usage is:

{% if flag?(:unix) || flag?(:docs) %}
@[Platform("Unix")]
def self.fork : Process
end
{% end %}

(see that or :docs flag, docs should be generated always same in my opinion, it's confusing to generate different api docs on different platforms)

which would newly emit compiler warning:
sample-compiler-warning
and create separate section in docs after cross-platform methods (methods summary):
sample-docs-summary
and methods details:
sample-docs-detail

it's only example, i don't made decision if we keep fork method and i don't mark any methods with this new annotation yet, that would be the subject for another PRs.

Resulting docs contains methods for all supported platforms but properly labeled and every consumer of this platform depended api is properly informed by compiler warning. This works for any other library so consumer can be informed in docs which methods are unsafe for use for cross-platform development and informed by compiler warning if do so.

@straight-shoota
Copy link
Member

I doubt this is the right way to go. Platform-specific code should be as minimal as possible and only be required when you actually need to do very specific things. In any case, the developer needs to be aware of the API being used and their platform-specific details, should there be any. So being specific about this in the documentation is important. And it might help to have some support for writing platform-specific notes. But I'd probably prefer something along the ideas discussed in #8353, so maybe an annotation for docs. But this might actually not be a good use case because the specifics of platform-dependend behaviour can be quite complicated and more complex than what fits in a simple schema. This can certainly not be expressed by a simple @[Platform(Unix)] directive. Most features are not binary that they work on one set of platforms and don't work on another, but there are far more details to consider, subtle differences between platforms. And ideally, most APIs should more or less work on all platforms, even if they have platform-specific implementations (and differences in behaviour).

Most certainly, we shouldn't bloat the API docs by adding another section for methods. As mentioned above, there is not a clear indication of platform-dependend methods, besides methods that are really only available on specific platforms. But that's really the exception and would separate them from methods that exist on all platforms but behave differently (which is far more common).

@asterite
Copy link
Member

I'd also like the std to work on all platforms with the same API. If a feature only works on a specific platform I'd like to see that provided by a shard. Then you can clearly see in your shard.yml whether some platform specific stuff is being used.

@asterite
Copy link
Member

Or alternatively, either raise a runtime error when a platform thing is not present, or provide sane defaults. I believe Go and Ruby do this.

@jan-zajic
Copy link
Contributor Author

Yes i only mean methods available on specific platforms.

Methods that behave differently on different platforms are completely different beasts and please don't mix them to current discussion. They should have that different behavior described in doc, or that behavior should be interally ,united there are no other ways (personally I know of a few such in stdlib but about that really another time another PR).

Now on the topic:
methods that are available only on some platforms (that means they are excluded by flags on others) and are public should:

  • be documented no matter which platform is currently used to generate docs
  • be little hidden in docs - in my proposal that new section is collapsed by default and placed behind other methods, so for me it's clearer than now
  • be dangered in docs - so nobody used them unless explicitly knows that going to use not-everywhere available api
  • be noted by compiler on supported platform - so if someone don't read doc and simply call this method on supported platform, he is informed that he can't compile on others (it's not about compiling on unsupported platform, nothing changes there, exception about missing method be raised..)

And no - API docs are not bloated, that new section only appear where and when some annotated method really exists. It's not about stdlib/not stdlib at all, if we decided to not have any such methods in stdlib this annotation simple will not be used anywhere and docs will not be affected (but we have such methods currently there, so i think it is better to mark them, until until you decided if remove them or not, but please let's discuss this elsewhere on specific cases).

In shard.yml we can add section platforms but that would be purely informational, similiar to libraries. But that make sense only if you have library that as a whole work or not on some platforms. But what if i made only some methods available on some platforms for advanced use cases?

final notes: if you make all platform specfic code in separate shards, it will lead to code duplication. The same applies to standard library. If I wanted to, for example, put fork or signals or in future some windows specific apis to separate shards and I also need them to implement internally some other cross platform method, you must implement it twice? Or make it public, so you are again at start..

@straight-shoota
Copy link
Member

  • be documented no matter which platform is currently used to generate docs

ACK

  • be little hidden in docs - in my proposal that new section is collapsed by default and placed behind other methods, so for me it's clearer than now

What's the point in hiding methods? If a shard exposes platform-specific method, it does that for a purpose. They shouldn't be pushed away.

  • be dangered in docs - so nobody used them unless explicitly knows that going to use not-everywhere available api

There is actually no danger. Or rather it depends on your intention. Platform-specific methods are in no way more dangerous than others. And in contrast to methods that exist but behave differently on different platforms, the non-existence will be noticed at compile-time. That's actually pretty safe.

  • be noted by compiler on supported platform - so if someone don't read doc and simply call this method on supported platform, he is informed that he can't compile on others (it's not about compiling on unsupported platform, nothing changes there, exception about missing method be raised..)

Why do I need that? When using such a method, it's very likely that I intended exactly this behaviour. The compiler doesn't need to tell me again.

The goal should be to eliminate the necessity for platform-specific methods as much as possible and provide portable APIs instead, both by shards and stdlib.

@asterite
Copy link
Member

final notes: if you make all platform specfic code in separate shards, it will lead to code duplication

Why? A shard can provide platform specific functionality that other shards use. Right?

@j8r
Copy link
Contributor

j8r commented Nov 28, 2019

Ruby has directly fork, Go has the unix package in the golang/sys package, and Rust has the unix module inside libstd/sys.

That's why, I think we could have a crystal-lang/unix repository, crystal-lang/sys, or inside this mono-repo, which is already the case.

The issue to have a whole new library is we may have duplication of bindings, logic, etc, and the question to add new things (e.g. bindings) in the lib or the language itself?

It is better to have everything in the same place, and the compiler consumes only what it is needed for the abstractions.

Therefore, for the time being, having a low-level System::Unix module is fine.
If in the code we see require "system/unix", this means there are platform specific stuff.

@ysbaddaden
Copy link
Contributor

Both Go golang.org/x/sys/unix and Rust sys/unix are in their respective stdlib; they're not external dependencies. Both need it to implement their platform agnostic runtime!

Go's os package is built on top of x/sys. Rust std is built on top of sys. We're doing the same in Crystal by moving platform specificities into Crystal::System namespace and only expose features that can be implemented on most platforms, but unlike Go or Rust, we aim to keep it private.

@asterite
Copy link
Member

I think Go's x/sys is an external package, not in the stdlib.

@straight-shoota
Copy link
Member

Go has some parts of x/sys integrated for the stdlib's internal implementation. That leads to duplicate code. For example: https://github.com/golang/go/blob/master/src/internal/syscall/windows/registry/key.go
https://github.com/golang/sys/blob/master/windows/registry/key.go

@yxhuvud
Copy link
Contributor

yxhuvud commented Nov 29, 2019

I'd also like the std to work on all platforms with the same API

That would be nice, but in some cases it may end up tricky getting good semantics in a way that work in all contexts. For example, currently the common asynchronicity of crystal comes from trying to force very different models of networking coexist in the same API, which work in a sort of limited way (limited in that it does not support everything that the different platforms support in an async manner).

Which for example means reading from a file is synchronous, despite windows having pretty great access to async file handling. Meanwhile, linux is getting a new interface to do async IO,(io_uring, accessed from userspace using liburing[*]). Which is also great, but very different to the windows API. BSDs (including MacOS) don't really have anything comparable that I've seen.

Is it always a good idea to limit the API of the language to the least common denominator? It seems like a recipe of not being great on any platform. There are obviously upsides to supporting similar APIs on multiple platforms too, so it is not a question with answers that is always easy.

[*] See https://kernel.dk/io_uring.pdf for an (outdated) description and https://github.com/axboe/liburing for lib + more descriptions. Nowadays there is a lot more syscalls and features like generic support for cancellation and timeouts being added to it as well.

(I suppose this is getting a bit OT from the issue at hand. Regarding that, I wouldn't mind doc markup but definitely not warnings on the correct platforms)

@straight-shoota
Copy link
Member

As always, discussing a feature based on theoretical use cases won't be enough to decide whether it's actually relevant. The use case presented in the OP becomes invalid the feature in question would be removed from stdlib (see #6421).

@RX14
Copy link
Contributor

RX14 commented Dec 5, 2019

A long time ago we decided that the compile-time API should be the same on all platforms and that methods should raise at runtime if they are not supported on a particular platform. This is the status quo in the standard library as it is now.

I'm happy to revisit that, but it should be a formal revisiting, in another issue.

Formalizing how we represent platform-specific documentation is a good thing, whether using an annotation or some other way though.

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

Successfully merging this pull request may close these issues.

None yet

8 participants