Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

SwIPC generator: the awakening #248

Merged
merged 29 commits into from
Apr 19, 2019
Merged

Conversation

roblabla
Copy link
Member

@roblabla roblabla commented Apr 6, 2019

Implement an IPC client code generator based on SwIPC definitions. The goal is to eventually remove all manually defined IPC client code from libuser in favor of being all automatically generated.

This first draft defines vi.id using SwIPC, and swaps the libuser manual client code with this version. The generator is implemented as a proc macro. The parser is implemented using a pest grammar and a quick-n-dirty AST layered on top.

Implement an IPC client code generator based on SwIPC definitions.
The goal is to eventually remove all manually defined IPC client code
from libuser in favor of being all automatically generated.

This first draft defines vi.id using SwIPC, and swaps the libuser manual
client code with this version. The generator is implemented as a proc
macro. The parser is implemented using a pest grammar and a quick-n-dirty
AST layered on top.
@roblabla
Copy link
Member Author

roblabla commented Apr 6, 2019

(BTW this is totally untested, so please don't merge unless you make sure it actually runs - not that I expect this to get merged without objections anyways :P. I'll test it tomorrow)

@Orycterope
Copy link
Member

forcing travis build after repo transfer : closing and reopening PR

@Orycterope Orycterope closed this Apr 7, 2019
@Orycterope Orycterope reopened this Apr 7, 2019
@Orycterope
Copy link
Member

@roblabla being an higher-kinded programmer, the legend has it that he hasn't written a single line of code since the beginning of the year, and he is stuck at the meta level.

Copy link
Member

@Orycterope Orycterope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great, the parser is really simple, I like it.

roblabla added 17 commits April 9, 2019 12:56
- Added the ability to decorate service names.
- Added the ability to parse unknown decorators.
- Added a @managedport service decorator, allowing to differenciate
between services hosted by sm: and those managed by the kernel.
We now inject `const _: &[u8] = include_bytes!(file.id);` in the
generated output. This tells the rust compiler that we depend on the
file.id file, without actually including it in the final binary. As
such, the crate will be recompiled if file.id changes.
We now turn KHandle<_, type> into an instance of the real underlying
type instead of just a generic Handle. For instance, client_session is
turned into a ClientSession.

Not all types are supported yet.
Fixes crashes on IPC calls that send/receive handles.
How else are we supposed to access it ^^'.
We now have two different code path for the raw_new, one handling the
Managed Port, and the other handling normal service ports.
Requires an update to cargo-make 0.17.1!

- Keep a single list with the clippy lints in an environment variable,
shared between all clippy rules.
- Remove clippy-ci. Use cargo make clippy -- -D warnings instead.
- Run clippy on swipc-gen/swipc-parser. Requires a separate rule to
handle running clippy on crates that require std.
When running cargo clippy twice, there will be no output as cargo check,
for whatever reason, decides not to rerun. To fix this, we add a rule
dependency on cargo make clippy that touches all the crates.
Unfortunately, this likely breaks on windows...
This will simplify testing later.
Mod attributes still require rustc to resolve the module, so swipc mods
should be defined as an empty in-line module instead of an external file
module, e.g.:

```rust
 #[gen_ipc()]
 pub mod sm {}
```

We also fix the namespace path of ViInterface which wrongly used libuser
instead of kfs_libuser.
KFS got renamed to Sunrise, rename everything.

Small fix to vi.id: get_resolution should be called
get_screen_resolution.
AHCI and SM client code are now generated based on SwIPC definitions.
Copy link
Member

@Orycterope Orycterope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great

Calling any function that had objects would cause a panic when
attempting to push it to the move_handles arrayvec, due to them not
being counted as handles.

Objects are now counted as handles.
The introduction of SwIPC-gen in the workspace broke documentation
building due to it depending on stdlib, which is not available when
building with --target=i386-unknown-none.

Documentation shall now be built without passing a target. Instead, any
cfg blocking out based on target architecture or OS should additionally
allow rustdoc, e.g:

```rust
```

This will allow rustdoc to punch through the cfg. It brings a few issues
however: we cannot define lang items, as they would be duplicated.
Those should not allow rustdoc through. Furthermore, we might have
problem in the future with multiple architectures. We'll see when it
comes.
Make some light touches on the documentation of the generator. Renames
the clippy-kfs target to clippy-sunrise.
The SwIPC-Gen binary now has a better interface to play around with the
code generated. Furthermore, to make using it simple, a Makefile rule,
swipc-gen, was added, that will build and run the tool with the required
arguments.
@roblabla roblabla requested review from marysaka and Orycterope April 16, 2019 00:09
Copy link
Member

@marysaka marysaka left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

We currently use common variable names in the function implementation,
such as buf, msg, etc... This can cause conflicts with argument names,
causing hard to debug errors.

Ideally, we'd create separate spans for everything, for proper hygiene.
But doing this would require refactoring the code to not use strings
everywhere. As a temporary band-aid, we should suffix __ to all internal
variable names.
Copy link
Member

@Orycterope Orycterope left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Great

@todo
Copy link

todo bot commented Apr 19, 2019

More thorough keyword sanitizer in swipc generator.

The SwIPC generator does little to no sanitizing of argument names that may be keywords in rust. We should have a list of keywords and a central function to fix them up.


// TODO: More thorough keyword sanitizer in swipc generator.
// BODY: The SwIPC generator does little to no sanitizing of argument
// BODY: names that may be keywords in rust. We should have a list of
// BODY: keywords and a central function to fix them up.
// Rename type to ty since type is a keyword in rust.
Some("type") => s += "ty",
Some(name) => s += name,
None => s += &format!("unknown_{}", idx)
}
s += ": ";
s += &get_type(idx >= args.len(), ty)?;


This comment was generated by todo based on a TODO comment in 52d9e02 in #248. cc @roblabla.

@todo
Copy link

todo bot commented Apr 19, 2019

Handle return C buffers.

// TODO: Handle return C buffers.
let ipc_count = 0;
let handle_move_count = cmd.ret.iter().filter(|(argty, _)| match argty {
Alias::Handle(false, _) | Alias::Object(_) => true,
_ => false
}).count();
let handle_copy_count = cmd.ret.iter().filter(|(argty, _)| match argty {
Alias::Handle(true, _) => true,
_ => false
}).count();


This comment was generated by todo based on a TODO comment in 52d9e02 in #248. cc @roblabla.

@todo
Copy link

todo bot commented Apr 19, 2019

Support nested type

// TODO: Support nested type
for line in doc.lines() {
writeln!(s, " /// {}", line).unwrap();
}
let tyname = match ty {
Type::Alias(alias) => get_type(false, alias)?,
_ => unimplemented!()
};
writeln!(s, " {}: {},", name, tyname).unwrap();
}
writeln!(s, "}}").unwrap();


This comment was generated by todo based on a TODO comment in 52d9e02 in #248. cc @roblabla.

@todo
Copy link

todo bot commented Apr 19, 2019

Deduce from template

writeln!(s, "#[repr(u32)]").unwrap(); // TODO: Deduce from template
writeln!(s, "pub enum {} {{", struct_name).unwrap();
for (doc, name, num) in &enu.fields {
for line in doc.lines() {
writeln!(s, " /// {}", line).unwrap();
}
writeln!(s, " {} = {},", name, num).unwrap();
}
writeln!(s, "}}").unwrap();
},
Type::Alias(alias) => {


This comment was generated by todo based on a TODO comment in 52d9e02 in #248. cc @roblabla.

@todo
Copy link

todo bot commented Apr 19, 2019

Prevent alias of buffer/pid/handles

// TODO: Prevent alias of buffer/pid/handles
writeln!(s, "pub type {} = {};", struct_name, get_type(false, &alias)?).unwrap();
},
}
Ok(s)
}
/// A module hierarchy.
#[derive(Debug)]
struct Mod {
/// Generated code for the types at the current level of the hierarchy.


This comment was generated by todo based on a TODO comment in 52d9e02 in #248. cc @roblabla.

@todo
Copy link

todo bot commented Apr 19, 2019

Bring the SwIPC parser in-line with new upstream format.

Unknown can now carry a size (which behaves like bytes). Unsized unknown should be treated as an unsupported struct.
Struct should now carry their size (and can optionally be validated).
Buffer size is now optional, and defaults to variable/client-sized.


// TODO: Bring the SwIPC parser in-line with new upstream format.
// BODY: Unknown can now carry a size (which behaves like bytes). Unsized unknown
// BODY: should be treated as an unsupported struct.
// BODY:
// BODY: Struct should now carry their size (and can optionally be validated).
// BODY:
// BODY: Buffer size is now optional, and defaults to variable/client-sized.
#[macro_use]
extern crate pest_derive;


This comment was generated by todo based on a TODO comment in 52d9e02 in #248. cc @roblabla.

@todo
Copy link

todo bot commented Apr 19, 2019

SwIPC-parser: Merge multiple type/interface definition

When the parser encounters multiple definitions of a type or interface, it should try to merge them into a single one.
This will make it easier to implement the logic of merging auto.id and switchbrew.id: we can just treat them as one big file, merging every entry, keeping the "best" information of each, while ensuring they are compatible.


// TODO: SwIPC-parser: Merge multiple type/interface definition
// BODY: When the parser encounters multiple definitions of a type
// BODY: or interface, it should try to merge them into a single
// BODY: one.
// BODY:
// BODY: This will make it easier to implement the logic of
// BODY: merging auto.id and switchbrew.id: we can just treat
// BODY: them as one big file, merging every entry, keeping the
// BODY: "best" information of each, while ensuring they are
// BODY: compatible.
Def::Type(tydef) => { ctx.types.insert(tydef.name.clone(), tydef); },


This comment was generated by todo based on a TODO comment in 52d9e02 in #248. cc @roblabla.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants