-
Notifications
You must be signed in to change notification settings - Fork 219
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
[RFC] change the late resource syntax #202
Comments
AFAIK there's a deeper issue here: This doesn't parse at all, so it would be impossible to implement because macros, including proc. macro attributes, are only expanded after the code is parsed. Regarding the RFC itself: I do agree that it's a good idea to change the late resource syntax. The magic Note that |
I agree with @jonas-schievink : I don't like the actual syntax, but I don't like this proposition more. Maybe #[rtfm::app(device = ..)]
const APP: () = {
static mut LATE: SomeStruct = rtfm::LATE_RESOURCE;
#[init]
fn init(_: init::Context) -> init::LateResources {
init::LateResources { LATE: SomeStruct::new() }
}
}; Not a big fan neither. |
You are correct. I was thinking of bang macros; those do accept any custom syntax.
True, but |
Would it be less terrible to use For completeness sake here are all the variations of today's syntax that we could use (regardless of how (un)reasonable they are) const APP: () = {
// expressions that are unlikely to be used in applications
static mut A: Type = (); // today's syntax
static mut B: Type = ..; // `core::ops::FullRange`
static mut C: Type = {}; // also evaluates to the unit type `()`
static mut D: Type = []; // `[T; 0]`
static mut E: Type = '_'; // `char`
static mut F: Type = ""; // `&'static str`
// super weird but valid syntax
static mut G: Type = break;
static mut H: Type = continue;
static mut I: Type = return;
// invalid syntax
// static mut J: Type = _;
// static mut K: Type = ?;
}; Alternatively, we could use a variation of #143 that uses two attributes: one for late resources ( const APP: () = {
static mut EARLY1: u32 = 0;
static EARLY2: u32 = 0;
#[strawman]
struct StaticMut {
// === static mut late1: u32 = ();
late1: u32,
late2: u32,
}
#[proposal]
struct Static {
// === static late3: u32 = ();
late3: u32,
late4: u32,
}
#[init]
fn init(_: init::Context) -> init::LateResources {
init::LateResources {
late1: 0,
late2: 1,
late3: 2,
late4: 3,
}
}
}; |
Argh, how annoying :(
I think I'm more of a fan of what you said in https://github.com/japaric/cortex-m-rtfm/issues/143#issuecomment-470616181, where late resources would be put into their own struct. This also neatly solves the somewhat weird requirement that Combining that with the crate-level |
You can also use attributes to differenciate #[late]
struct LateResources {
#[mut]
late1: u32,
#[mut]
late2: u32,
late3: u32,
late4: u32,
} |
Having late resources in a struct and non-late resources as statics will cause strange naming conventions: late resources will be snake case while non-late resources would be screaming case. |
And an argument to specify the initial value? Oh, and we can't use // all resources
#[resources] // <- not really needed, I think
struct Resources {
#[r#mut(init = SomeType::const_new())]
early_resource: SomeType,
#[r#mut]
late_resource: OtherType,
#[not(r#mut)] // <- or something else that makes more sense
late_static: AndAnotherType,
#[not(r#mut(init = YetAnotherType::const_new()))]
early_static: YetAnotherType,
}
#[init]
fn init(c: init::Context) -> init::LateResources {
init::LateResources {
late_resource: OtherType::new(),
late_static: AndAnotherType::new(),
}
} That parses.
Even with the above syntax task locals would still have ALL_CAPS names. #[task(resources = [resource])]
fn a(c: a::Context) {
static mut LOCAL: u32 = 0;
*c.resources.resource += 1;
*LOCAL += 1;
} I think I can live with the resource syntax not reflecting that resources are all static variables. The current syntax does have a small refactor advantage: one can easily turn a task local into a resource or a plain |
I like this proposition. As an explanation of the not I'd separate mutable and init, it would do // all resources
struct Resources {
#[mutable, init = SomeType::const_new()]
early_resource: SomeType,
#[mutable]
late_resource: OtherType,
// nothing: static late resource
late_static: AndAnotherType,
// static inited, but is it really useful compared to plain static?
#[init = YetAnotherType::const_new()]
early_static: YetAnotherType,
}
#[init]
fn init(c: init::Context) -> init::LateResources {
init::LateResources {
late_resource: OtherType::new(),
late_static: AndAnotherType::new(),
}
} |
@TeXitoi I like the syntax you have proposed but #[mutable, init = SomeType::const_new()]
#[init = YetAnotherType::const_new()] It seems that if you want arguments ( #[path(key = value)]
(the advantage is access control: you can enforce (declaratively) that the static is only accessible from certain tasks -- you won't get the same if you stick a normal |
I am not much for the first syntax proposed, as what has What @TeXitoi is quite good though! Problematic though if it's not liked. Is there a way to get it close to that which |
If we have more breaking-changes budget to spare we could mix this with just the shared access syntax of RFC #129. The main idea would be that there would no longer be a difference between This means that we wouldn't need a In a nutshell we would go from this: #[rtfm::app]
const APP: () = {
static mut A: Late = ();
static mut B: Early = Early::new();
static C: Late = ();
static D: Early = Early::new();
#[task(resources = [A, B, C, D])]
fn foo(cx: foo::Context) {
cx.resources.a.lock(|_| {});
let b: &mut Early = cx.resources.b;
let c: &Late = cx.resources.c;
let d: &Early = cx.resources.d;
}
} to this #[rtfm::app]
const APP: () = {
struct Resources {
a: Late,
#[init(Early::new())]
b: Early,
c: Late,
#[init(Early::new())]
d: Early,
}
#[task(resources = [a, b, &c, &d])]
fn foo(cx: foo::Context) {
cx.resources.a.lock(|_| {});
let b: &mut Early = cx.resources.b;
let c: &Late = cx.resources.c;
let d: &Early = cx.resources.d;
}
}; We could accept both And to not add new locking features we would reject mixing #[rtfm::app]
const APP: () = {
struct Resources {
x: X,
}
#[task(resources = [x])]
fn foo(cx: foo::Context) { .. }
#[task(resources = [&x])] //~ error: [insert message that makes sense here]
fn bar(cx: bar::Context) { .. }
} One more thing worth pondering about is how this #![rtfm::app]
#[resource]
static mut X: u64 = 0;
mod a {
#[resource]
static mut Y: u64 = 0;
}
mod b {
#[task(resources = [X, a::Y])]
fn foo(cx: foo::Context) {
// ..
}
} Plus the path to static variable syntax looks natural. It's not clear to me how the Finally, we could leave the resources syntax unchanged for v0.5.0 and revisit it for the v0.6.0 release. |
SGTM, right now I have a hard time to get a feel for how these would be to use. I'll ponder it a bit and come back with more feedback. |
I like the proposition in https://github.com/japaric/cortex-m-rtfm/issues/202#issuecomment-506963445 For the problem of // all resources
struct Resources {
#[mutable]
#[init(SomeType::const_new())]
early_resource: SomeType,
#[mutable]
late_resource: OtherType,
// nothing: static late resource
late_static: AndAnotherType,
// static inited, but is it really useful compared to plain static?
#[init(YetAnotherType::const_new())]
early_static: YetAnotherType,
}
#[init]
fn init(c: init::Context) -> init::LateResources {
init::LateResources {
late_resource: OtherType::new(),
late_static: AndAnotherType::new(),
}
} |
I agree on this approach, using the resource access list does indeed look like a good solution! |
This |
202: Enable building semihosting for Armv8m r=korken89 a=aurabindo Semihosting protocol has not changed for Arvmv8m and hence it need not be disabled for this architecture. Co-authored-by: Aurabindo Jayamohanan <mail@aurabindo.in>
Current behavior
The current syntax for late resources is very similar to the syntax for normal
resources: one simply uses the unit value (
()
) as the "initial value" for thelate resource.
Rationale
Some people have expressed their dissatisfaction with the current syntax because
it wouldn't type check in normal Rust code; they would expect the initial
value to be omitted as semantically there's no initial value. However, dropping
the initial value would render the RTFM syntax un-
rustfmt
-able.Proposal
This RFC proposes using the
extern static
syntax to specify late resources.This lets us omit the initial value in the syntax while keeping the syntax
rustfmt
-able.Detailed design
Drawbacks
There would be an ABI attached to the late resources. This gives the false
impression that late resources use the "C" ABI:
Which is not true because the ABI is a property of the type not of the
declaration.
Changing the declaration to
extern "Rust"
doesn't really help because theresource could actually be using a C representation:
Alternatives
RFC #143 proposes using an
struct
syntax to specify resources. That proposalhas two issues which this proposal doesn't have:
The syntax doesn't let the user distinguish resources (today's
static mut
variables) from
static
variables -- the distinction is important becausestatic
variables are treated differently in theSend
/Sync
analysisIt gets rid of non-late resources so one can no longer specify resources, or
static
variables, whose initial values are known at compile time -- this couldhave a small negative impact on the initialization time and program size of all
RTFM applications.
Another alternative would be to special case
core::mem::MaybeUninit
to meanlate resources so one would write:
However, this gets tricky quickly because the macro has no way to know whether
the identifier
MaybeUninit
refers tocore::mem::MaybeUninit
or some userdefined type.
cc @TeXitoi
The text was updated successfully, but these errors were encountered: