Migration from register_attr to register_tool

[oisyn]

We need this because the remove_attr feature was removed from the Rust nightly early September

[eddyb]

Nitpick: [# everywhere should be replaced with #[.
_{(the # is the attribute syntax, [ is merely a "container" - this is similar to how some languages have @foo and @bar(123) attributes but that @ is # in Rust and brackets exist to disambiguate something like #[allow(overflowing_literals)] (x + 200i8) which without the brackets would me ambiguous as to whether the parens are attached to allow - the languages that don't need that tend to be very limited in where you can apply attributes - anyway Rust can look like languages with [foo] attribute syntax but AFAIK it's closer to the @foo languages despite the aesthetics...)}

[fu5ha]

I didn't look this over exhaustively but the gist of the change is a 👍 from me

Done

[oisyn]

Waiting for #929 first

[eddyb]

These are the only files I didn't check off as "viewed":

$$('.js-reviewed-checkbox').filter(x=>!x.checked).map(x=>x.parentNode.parentNode.path.value).join('\n')

tests/ui/arch/all.rs
tests/ui/arch/any.rs
tests/ui/arch/convert_u_to_acceleration_structure_khr.rs
tests/ui/arch/demote_to_helper_invocation.rs
tests/ui/arch/emit_stream_vertex.rs
tests/ui/arch/emit_vertex.rs
tests/ui/arch/end_primitive.rs
tests/ui/arch/end_stream_primitive.rs
tests/ui/arch/execute_callable.rs
tests/ui/arch/ignore_intersection_khr.rs
tests/ui/arch/kill.rs
tests/ui/arch/report_intersection_khr.rs
tests/ui/arch/terminate_ray_khr.rs
tests/ui/arch/trace_ray_khr.rs
tests/ui/image/issue_527.rs

But looks like GH decided to merge it anyway? :/

EDIT: those ended up being fixed by:

• Changed the location of some use statements in tests #931

[eddyb]

You marked my previous comment asking about the pub here as resolved, but I don't remember what the explanation was, sorry - it's just that spirv_std::macros::... is used elsewhere, right?

[oisyn]

Oh I did this because the things in spirv-std now need spirv_std::spirv to be visible for all platforms. This way I didn't need to write use spirv_std::spirv; in every single file.

[eddyb]

We don't need to expand on this much more but to be clear this only applies to weird "leaf" things like function args, fields, etc. (which are replaced while transforming a larger thing that triggered the proc macro as usual).
Whole-definition attributes (e.g. the whole entry-point or a whole struct) will just work through normal name resolution.

Funny thing about that btw, if you only need it for a leaf, you need to do:

#[spirv()]
fn foo(
    #[spirv(ray_tracing_weirdness)] x: T,
) {}

(with the whole-fn one only serving to handle the inner ones lol - again, I don't think we have to mention this, since we don't have that many attributes to begin with, and so far such a usecase doesn't exist AFAIK, but I wanted to expand on it "for the record" so to speak)

[oisyn]

If we do end up having a case where this applies (I don't think we do currently?), I'd suggest supporting #[spirv] without the parens.

[eddyb]

I'd suggest supporting #[spirv] without the parens.

I don't think we have a say in the matter (i.e. both should work), and you likely can't tell them apart (since that's the proc macro, not the tool module one).