offload_kernel macro expansion#156642
Conversation
This comment has been minimized.
This comment has been minimized.
|
Nice! I'll look closer after RustWeek, but the tests look fine. We should update the rustc-dev guide to look prettier |
This comment has been minimized.
This comment has been minimized.
|
Still need to check the rustc code, but the test lgtm, we don't need anything inside of main here. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
| ) -> Option<(ast::Visibility, ast::FnSig, Ident, ast::Generics, Option<Box<ast::Block>>)> { | ||
| match item { | ||
| Annotatable::Item(iitem) => match &iitem.kind { | ||
| ast::ItemKind::Fn(box ast::Fn { sig, ident, generics, body, .. }) => { |
There was a problem hiding this comment.
| Annotatable::Item(item) | ||
| }; | ||
|
|
||
| if is_device(ecx) { vec![device_item] } else { vec![host_item] } |
There was a problem hiding this comment.
This is cursed :D But I kind of like it. Let me just confirm if the surrounding infra is expecting a (builtin) macro to change based on a (tracked) flag.
@JonathanBrouwer, as an attribute person, this is fine, or?
There was a problem hiding this comment.
I think this is perfectly fine, since no incremental happens on builtin macros, I don't see what could go wrong.
But as an attribute person I've actually not done that much with the builtin_macros attributes yet so don't count me as an expert on this
This comment has been minimized.
This comment has been minimized.
|
somewhat related since we are updating
Is this still needed? The path mentioned differs on my system and "might" implies it might not be needed which would make automation via shell scripting easier. |
This comment has been minimized.
This comment has been minimized.
|
The rustc-dev-guide subtree was changed. If this PR only touches the dev guide consider submitting a PR directly to rust-lang/rustc-dev-guide otherwise thank you for updating the dev guide with your changes. |
I don't think so, but we could correct that directly in the dev guide. Thanks! This will make the upstreaming of tests nicer. With the latest iteration, we should be able to support generics. Also, having it expand differently for host and device is nice. In the future, we should also think about having a solution to make the function callable on both host and device. @bors r+ rollup |
…uwer Rollup of 8 pull requests Successful merges: - #157006 (net tests: let the OS pick the port numbers) - #157126 (Avoid redundant note when a #[derive] is already suggested) - #151690 (std: Refactor `env::var` function) - #155826 (std::process: uefi: avoid panicking in Stdio From impls.) - #156109 (Migrate libraries from ptr::slice_from_raw_parts to .cast_slice) - #156642 (`offload_kernel` macro expansion) - #156823 (Add regression test) - #157162 (elaborate_drop: Avoid as_mut.unwrap dance, empty vectors are cheap.) Failed merges: - #157173 (rustc-dev-guide subtree update)
View all comments
r? @ZuseZ4