Feature flags to make lsp and readline support optional

[ahupp]

This diff moves support for readline (used by the breakpoint console) and the LSP server behind a cargo feature. Readline is enabled in the lib by default to preserve debugging when embedding, while lsp is only enabled when building the starlark binary.

The motivation is to support a wasm32-unknown-unknown build of starlark.

[stepancheg]

It would be better just unconditionally not add the dependencies on these crates. Something like [dependencies.cfg(not(wasm))].

Or move LSP/readline implementations into a separate crate.

The issue with feature flags, is it that it is not enough to just add them, also CI need to be set up, and all permutations properly tested.

Also, to make starlark-rust work on wasm, much more is needed than removal of these two dependencies.

[ahupp]

I can make readline unconditional based on platform.

Lots of the symbols are pub(crate) so couldn't be exposed to a separate LSP server crate without substantial changes. There might be a simpler solution I missed though. I'm also not aware of a way to make a [[bin]] conditional on platform.

Moving readline out would leave a single feature ("lsp", though maybe it should be called "cli"), that's disabled by default and enabled when building the CLI. That would also be good for embedders who don't need to build clap/lsp-server etc anyways.

WDYT?

[stepancheg]

I'm also not aware of a way to make a [[bin]] conditional on platform.

Bin could still exist, but work without readline on wasm.

Anyway, the larger issues with wasm are:

• making it work. Just removing dependencies won't be enough for wasm. Maybe we should postpone it until we have at least proof of concept working wasm. Like interpreter running hello world on wasm.
• CI. If we have an implementation for wasm, we need to make sure it won't regress. Is it possible to somehow run cargo test -p starlark --some-flag-to-make-it-run-on-wasm?

[ahupp]

bin could still exist

You mean the repl without lsp?

Anyway, the larger issues with wasm are:

I don't have it working, but am hopefully far enough down the path to have a sense of what the issues are. I have a solution for rust-ctor (though not it's landable). Not sure about 32-bit tagged pointers, will try either using a u64 instead of usize on all platforms and just wasting a few bytes for 32-bit clients, or requiring a custom allocator (pretty common on wasm) that can promise an extra free high bit.

Maybe we should postpone it until we have at least proof of concept working wasm.

Makes sense.

Is it possible to somehow run cargo test -p starlark --some-flag-to-make-it-run-on-wasm

I've not tried it but believe this is possible.

[stepancheg]

Not sure about 32-bit tagged pointers

Currently we only need third bit to mark the pointer is string, which is for optimization, because string are the most common types.

We can disable string optimization when working in 32-bit setup.