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)

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