Skip to content

Conversation

@Wasabi375
Copy link

Currently when parsing a "safe fn" syn will represent this as a ForeignItem::Verbatim.
This makes it hard to use them in proc-macros.

Right now parse_signature will return Ok(Some(sig)) for signatures without the "safe" keyword. If the "safe" keyword is found it returns Ok(None) instead.
If None is returned ForeignItem::parse will assume that the function signature contains the "safe" keyword and returns a Verbatim.

I see 2 possible solutions to improve this:

  1. add Option<Safe> to Signature
  2. change parse_signature to return Ok(sig, Some(safe)) and store the "safe" keyword in the ForeignItemFn
  3. Introduce a ForeignSignature that extends Signature with the optional safe keyword

There are some Pros/Cons to all 3 approaches

  1. The "safe" keyword is only valid within an extern block so I am not sure if this is a good abstraction. However this would be the simplest to implement.
  2. Still simple to implement, but it stores the "safe" keyword as a field on ForeignItemFn which means that there needs to be special handling in ForeignItemFn::to_tokens to ensure that the "safe" keyword is inserted at the correct position.
  3. This is the cleanest approach, as it properly represents valid rust code. However it is probably the solution with the most code. In any case I decided to implement this. Option 1 and 2 feel like hacks to me.

Open questions:

  1. Are there any tests I should add for this. If so can you point me to an existing test that I could use to orient myself. I'm not sure I fully understand the test framework here.
  2. I'm not sure how you handle versioning. This change changes the fields in ForeignItemFn and I guess that is a breaking change. However I can't imagine you release a breaking version change every time rust adds a new feature with new syntax. No idea how you want to handle something like this.

@nik-rev
Copy link

nik-rev commented Jan 26, 2026

Rust actually accepts safe fn outside of extern blocks:

#[cfg(false)]
safe fn foo() {}

This is syntactically correct. So I think it'd be better to make that unsafety into an enum like this:

pub enum Unsafety {
    Safe(Token![safe]),
    Unsafe(Token![unsafe])
}

Then, change the unsafety field in Signature from Option<Token![unsafe]> to Option<Unsafety>

#1756 (comment)

@Wasabi375
Copy link
Author

Thanks for the feedback and also for linking the issue. I remember looking for it but not finding one. Maybe there is something strange with search on github if the search is quoted? Not sure.

Rust actually accepts safe fn outside of extern blocks

I did not know that and the rust reference specifically states that this is not actually allowed. I'll continue this discussion in the issue you linked. That way any information is not lost, if this PR is not accepted.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants