Closed vi closed 4 years ago
(answering to https://github.com/jonathandturner/rhai/issues/100#issuecomment-656695339)
On the other hand, it conforms to "standard".
Not only it conforms to a standard, it also allows using Rhai multiple times in dependecy tree without cooperation between those dependencies.
I think I'll let the community ponder it more before deciding to change it for 1.0...
Community involvement is encouraged, even if just to inform about pending API breakage long before it actually happens.
You can announce that, for example, rhai v0.22.0
would have the new scheme (and write a warning in the book and crate doccomment near that features table), so users can upgrade gradually.
I think Cargo feature revamp should definitely happen before the "v1.0" release, but there is no reason to rush it.
In current form it looks like Rhai is indended to be used only by apps, not as a transitive, private dependency inside libraries.
It's something that I agree should be fixed. Since it will be a hard breaking change, users will have to fix their end when they upgrade. It doesn't matter whether you inform of the change early in advance, the change can still only be made the moment a new version with the change is available. It's not something that users can slowly migrate to like a deprecated function. Therefore, I'd rather see it fixed as early as possible, so that new users won't have to make the changes down the road. Just make it clear in the release notes and changelog.
Additionally, I think keywords from missing features should still be reserved in scripts (unless explicitly configured otherwise from the Rust side), to avoid typical Rhai scripts breaking when a feature suddenly gets turned on from afar.
There is a problem with this.
When a feature is turned off, the current design is that it is as if the feature doesn't even exist.
Therefore, if no_function
is on, then fn
is a valid variable name.
You can further disable keywords., If you disable if
, then if
is a valid variable name.
The lack of a feature maybe dependent by script writers. Therefore, features are actually not additive. That is, a script that runs fine on a system without some language feature may not run in a system with that language feature, simply because the script writer assumes the lack of such language feature.
The idea is that there should only be one scripting language in an application. Therefore, the feature set of the Rhai language should be fixed.
However, you bring up a good scenario in which, for example, the application has its own embedded scripting language, but then also pull in crates such as casbin-rs
which also has its own scripts.
In such a scenario, even if features are additive, you'll have trouble. That's because casbin
assumes a restricted language; if you turn those features on, then casbin
may not be able to handle it.
In other words, it might actually be better to turn off language features using configuration instead of features... But then, of course, we can never make a "minimal" build because you end up pulling everything in, even though it is not needed.
Another alternative is to keep the features set, but then also add a configuration API to turn off certain elements even when the features itself are there.
Proposal for pre-1.0:
Make features additive. The default should have almost everything. For example, baseline will have no modules, and "modules" feature adds it. Default will have "modules".
Make API: Engine::disable_feature
that selectively disables a feature even if it is there. If it isn't there, it does nothing.
Any app who wants to restrict the language needs to omit the feature from the crate, and also call Engine::disable_feature
on each Engine
just in case something upstream includes that feature.
Does this make sense?
However, you bring up a good scenario in which, for example, the application has its own embedded scripting language, but then also pull in crates such as
casbin-rs
which also has its own scripts.In such a scenario, even if features are additive, you'll have trouble. That's because
casbin
assumes a restricted language; if you turn those features on, thencasbin
may not be able to handle it.
I believe the best solution to this would be for Cargo to allow private instances of dependencies that are not shared between crates (there is an RFC for this), but we probably don't want to wait until it is implemented.
Make API:
Engine::disable_feature
that selectively disables a feature even if it is there. If it isn't there, it does nothing.Any app who wants to restrict the language needs to omit the feature from the crate, and also call
Engine::disable_feature
on eachEngine
just in case something upstream includes that feature.
How about using marker type parameters so that language features can be disabled during compilation? But obviously this will take more effort to implement, will increase compile times, and users will probably have to make a type alias to refer to the Engine.
Engine::disable_feature
is negative thinking again, not protected against future new features. Primary and recommended way should be enable_feature
instead.
I.e. Engine::new()
automatically enables all features present in Cargo (i.e. all features by default) and is recommended for applications. And EngineBuilder::no_features().add_feature(Feature::Functions).add_feature(Feature::Modules).....build()
enables only selected features and is recommended for libraries.
Therefore, if
no_function
is on, thenfn
is a valid variable name.
fn
being valid variable name splits Rhai scripts ecosystem into incompatible parts, also interferes with naturally upgrading Rhai environment from initially simple expression evaluator to more involved scripting environment as time passes and project scope gets bigger. It is like using class
in C code, thinking "We'll never need to interface C++, so why not?".
So a function like EngineBuilder::reserve_all_keywords
function can be nice to make scripts more restricted and prevent future upgrade problems. It can just Engine::disable_symbol
all the unused keywords. It should be recommended in most environments, except of maybe if memory is so constrained that even holding a list of banned names is wasteful, or when Rhai is used just as an expression evaluator.
If Rhai has some pending, not-yet-developed features that also require keywords, they should also be disabled ahead of time.
EngineBuilder::no_features().add_feature(Feature::Functions).add_feature(Feature::Modules).....build()
This looks feasible...
I ended up adding a -> &mut Self
to most API methods.
A builder style does not allow subsequent modification of the Engine
. It is actually more convenient to just be able to chain API calls.
I have been giving this some more thought and done a bunch of reading.
Such as this: https://stephencoakley.com/2019/04/24/how-rust-solved-dependency-hell
Seems like Rust is quite smart. For crate invocations that are clearly different, it pulls in both versions under different mangled names.
Now, the question is what it'll do when confronted with the same version of the crate, but with different features enabled.
Does it:
From https://stackoverflow.com/questions/56921098/cargo-build-package-with-conflicting-features-from-the-same-git-repository it looks like (1).
Seems that to get (2) you need to clone the repo into another location. Then Cargo
will treat it as a separate dependency.
In Rhai, features are really not very composable. They really turn on/off certain language features. Therefore, having a feature turned on when an app expects it to be off is not a mere wastage of more code. It may be incorrect - the app expects the lack of certain language features. The non-existence of a feature is as much as differentiator as a feature.
So it is really not as simple as making all feature additive, because the lack of a feature is a feature by itself.
the app expects the lack of certain language features lack of a feature is a feature by itself.
Isn't it a design flaw? Imagine an ecosystem of user-contributed Rhai scripts that can be downloaded and executed. Without additional restrictions (such as that Rhai scripts should not break if extra features get enabled) it would be tricky to enhance script features (or even support both old-style and new-style scripts).
This is why I also propose to make it easy to reserve keywords for all possible features (maybe even some future unimplemented ones)
Additionally, I think keywords from missing features should still be reserved in scripts ...
What things in Rhai, apart from keywods being available or not available for use as identifiers, do require absense of particular features?
This is why I also propose to make it easy to reserve keywords for all possible features (maybe even some future unimplemented ones)
Yes, reserved keywords are already merged, thx to your idea. The keywords are already reserved.
What things in Rhai, apart from keywods being available or not available for use as identifiers, do require absense of particular features?
Probably not. I guess there is nothing that cannot be done by simply disabling certain keywords and be done with it. There is even disable_symbol
to do exactly that.
However, that also means that somebody writing a library that uses Rhai in a restricted environment must go thru the hassle of disabling all the features that he/she doesn't need, just in case the upstream app also uses Rhai and starts enabling some things.
And also there are features that are mutually exclusive and not additional. For example: sync
. This is a feature that is additive, however, its behavior is not additive.
With sync
: Engine
must be Send + Sync
and supports multi-threading. Without sync
, Engine
can handle any data type but not multi-threaded.
Say a library writer uses Rhai. He/she obviously doesn't want to restrict the library to sync
only, because apps using the lib may not be multi-threaded.
However, if an app using the lib needs a scripting feature which needs to be thread-safe, if it pulls in sync
, it affects the version used by the lib itself.
I believe right now the only solution is to have marker traits and then have type-def's that refer to common Engine
invocations...
Closing this for the time being. It is not a simple matter to make Rhai features additive.
Should Rhai be explicitly documented against usage as a private (internal) dependency in libraries then?
Something like "Don't use Rhai in libraries. Rhai as a language is sensitive to the chosen Cargo feature set and is expected to appear only once in a project's dependency tree. Internal Rhai dependency may introduce unwanted Rhai feature switches or be confused by activating unwanted switches from a neighbouring Rhai dependency inclusion".
Unfortunately many libraries use Rhai especially as an expression evaluator... so it is probably not easy to completely ban it.
Do you plan leaving the "Rhai v1.0" with cargo features usage still unfixed?
What does mean "for the time being" and how does it interact with the "wontfix" label?
Or maybe adjusting feature completeness vs compactness will be moved away from Cargo features to e.g. plugins? (can it handle sync
and !sync
engines within the same project?)
can it handle
sync
and!sync
engines within the same project
Unfortunately no. Not without finding a way to keep two separate copies. That's why Rhai features are not additive. They are more like toggle switches.
Another example will be no_float
. It is simple: If the target does not have floating point support, it should exclude floating-point math types in its code. However, say a library does not specify no_float
and may already register floating-point functions. In other words, the library actually requires floating-point support; therefore technically speaking it should not be used in any project that requires no_float
. In other words, the feature, similar to no-std
, is not additive; adding or removing features break compatibility altogether instead of adjusting behavior.
The current "features" feature in Cargo attempts to model additive behavior. It is technically not intended to be used to model pre-requisits. But the only way to model pre-requisits right now is to break up the project into multiple foundation crates, one implementing a single feature, then use runtime code to pull them all in. That's why you get the wide range of sub-crates in many Rust projects.
TL;DR So in conclusion, it is not a simple solution to make features additive. The "right" solution is to break up Rhai into multiple sub-crates.
It is technically not intended to be used to model pre-requisits.
I don't understand this phrase.
Do you consider Tokio's usage of feature flags an intended use case of Cargo features?
Libraries may want to exclude feature to minimize code bloat even if otherwise it is no problem to leave feature enabled.
Yes, that's the main problem and why features are this way. Embedded targets (even WASM) will want minimal builds to exclude unnecessary code. If hidden behind API switches, all those code will remain. The only way is to exclude those code with compile-time config switches.
That's why many of Rhai's language features are tagged under feature flags instead of a "configuration" API.
The only way is to exclude those code with compile-time config switches.
Or usage of compile-time evaluation (const functions) and optimisation (including whole-program fat LTO). String formatting and panicking code is often excluded this way when doing embedded.
I don't understand this phrase.
Probably not very scientific, but I'm at a loss to find the right phrase.
Basically, unlike tokio
as you pointed out, Rhai features cannot be used this way.
In tokio
, for example, if you turn on tcp
, then it pulls in all the tcp
types. Fine. If another library doesn't want it, it doesn't harm. It'll just sit there occupying code space and nothing else.
The problem with "it doesn't harm" doesn't work with Rhai. Image that, for some wierd reason, we add a "while-loop" feature to Rhai, enabling/disabling the while
statement.
A library pulling in Rhai with while-loop
set will conflict with other code that doesn't require it. In other words, it does harm. The other code that doesn't set while-loop
will expect the while
statement to throw a syntax error; it will not expect a while
statement to magically work.
Or usage of compile-time evaluation (const functions) and optimisation (including whole-program fat LTO). String formatting and panicking code is often excluded this way when doing embedded.
Not easily. String formatting and panicking code is excluded via feature gates.
A library pulling in Rhai with while-loop set will conflict with other code that doesn't require it. In other words, it does harm. The other code that doesn't set while-loop will expect the while statement to throw a syntax error.
It may be solved by double check: a library which wants to fine tune which parts of Rhai it wants does two things:
This way two Rhai engines: the one with while
identifier and the other one with while
keyword can coexist.
will expect the while statement to throw a syntax error
I think relying on things being syntax errors is not future-proof anyway. It is like relying on optimisation removing bad code.
I think relying on things being syntax errors is not future-proof anyway.
That was just an example. There are many language features that fall into such category. Having it and not having it is not a simple matter of: If I have it but don't use it, it's fine. Many such cases are more like: Oh heck, I don't need it and suddenly it shows up and it is screwing me up.
It may be solved by double check: a library which wants to fine tune which parts of Rhai it wants does two things:
1. Includes appropriate Cargo feature 2. Enables the keywords using in-code configuration (runtime or compiletime)
This way two Rhai engines: the one with
while
identifier and the other one withwhile
keyword can coexist
You are absolutely right. This is the correct way to add/remove features, which is in-code configuration.
But this way it is very hard to remove the code if you don't need it.
There is one way that works, which is to have both additive features plus runtime configuration switches. But then the user must remember to do both.
But this way it is very hard to remove the code if you don't need it.
What about things like
pub trait RhaiFeatures {
const ENABLE_FLOATS : bool = true;
const ENABLE_OPTIMIZE: bool = true;
...
}
impl<F:RhaiFeatures> Something<F> {
pub fn do_something() {
if F::ENABLE_FLOATS {
heavyweight_function();
}
}
}
Does it provide enough assurance that heavyweight_function
is really not inclued?
Maybe a fork/branch of Rhai with experimental feature selection revamp should be created?
Maybe a fork/branch of Rhai with experimental feature selection revamp should be created?
I wouldn't mind this at all! I tried but it wasn't easy. If somebody would like to take a shot at it, it'll be good!
Does it provide enough assurance that
heavyweight_function
is really not inclued?
It does, but then the Engine
is going to have a generic parameter tagged on. And there will need to be different traits with different combinations of true
and false
based on the features set. Essentially this is equivalent to in-code configuration.
I tried but it wasn't easy.
Maybe it can be done gradually, starting from some one feature?
Is there some statistics of how Rhai is used (especially in libraries), which features are typically enabled and where absense of features are really required?
You told that Rhai is sometimes used as math expression evaulator in some libraries (as far as I understood). Which of reverse dependencies are worth revieing? Are there known projects that are not on crates.io?
Is there some statistics of how Rhai is used (especially in libraries), which features are typically enabled and where absense of features are really required?
I'm not sure if there is somewhere where these telementaries are kept... crates.io
doesn't seem to capture it...
You told that Rhai is sometimes used as math expression evaulator in some libraries (as far as I understood). Which of reverse dependencies are worth revieing? Are there known projects that are not on crates.io?
Not sure about non-crates.io projects. There is an outstanding issue about projects using Rhai but it is not updated.
If you scan thru the issues list, you'll find a lot of questions about using Rhai, probably in projects no on crates.io.
Adding more Cargo features should bring in more code and features, not less. Also adding a Cargo feature should almost never break existing scripts.
With a major semver bump, I suggest to make the following changes:
Reverse it to
std
.I don't know what it is, probably leave it as is.
Reverse to
checked
.Leave as is.
Reverse to
optimize
. Mayberhai::OptimizationLevel::None
should be available even without it.Reverse to
float
This one is tricky. There should probably be
integer
feature that enables all integer types and alsoi32
andi64
features that selectively enable those types.integer
should imply (but not be limited to)i32
+i64
.A compile error should be emitted if none of three features (
i32
,i64
,integers
) are activated, unless Rhai can work without integers at all.It should be OK to activate both
i32
andi64
features withoutinteger
feature.Reverse to
index
.Reverse to
object
Reverse to
function
Reverse to
module
.Leave as is.
Additionally, I think keywords from missing features should still be reserved in scripts (unless explicitly configured otherwise from the Rust side), to avoid typical Rhai scripts breaking when a feature suddenly gets turned on from afar.