rustc_builtin_macros: rename bench parameter to avoid collisions with user-defined function names #148279
Conversation
… user-defined function names
rustbot
added
S-waiting-on-review
This comment has been minimized.
This comment has been minimized.
|
|
||
| // A simple ident for a lambda, using the user's function name within it to avoid collisions. | ||
| let param_name = format!("__bench_{}", fn_.ident.name); | ||
| let bencher_param = Ident::new(Symbol::intern(¶m_name), attr_sp); |
There was a problem hiding this comment.
why did you change it from from_str_and_span to new + intern?
There was a problem hiding this comment.
It’s mostly a style preference, @oli-obk. Since format! gives us a String (unlike the old str), we still need either a conversion or an interning step - from_str_and_span would need a .as_str() call, while new needs a Symbol::intern call. So from_str_and_span doesn’t really simplify anything, and I think the explicit intern call makes it flow a bit clearer and cleaner than a conversion, and also separates the logical steps of building the string and then interning it. That said, if you’d rather keep from_str_and_span and just add .as_str() for consistency, I’m good with that too, I really don't mind. Thank you for your time in reviewing! 🙂
There was a problem hiding this comment.
That could work to as well I think, I considered that but preferred the explicit intern style. I'm fine with whatever everyone else wants to go for. :)
There was a problem hiding this comment.
Since the helper function exists it's probably a good idea to keep using it...
| let param_name = format!("__bench_{}", fn_.ident.name); | ||
| let bencher_param = Ident::new(Symbol::intern(¶m_name), attr_sp); | ||
| let bencher_param = Ident::from_str_and_span(¶m_name, attr_sp); |
There was a problem hiding this comment.
I'd probably also inline param_name...
|
r? hkBst |
|
Failed to set assignee to
|
|
oh heh @bors delegate=hkBst |
7 participants
Resolves #148275 by preventing name collisions in the
#[bench]macro.Previously, a user-defined function named "b" could not be benchmarked because
the macro-generated lambda identity collided with the same name. We now generate
the lambda ident as
__bench_<function_name>, ensuring it is always distinctfrom the user’s function.
Because the prefix is applied recursively (e.g. benchmarking
__bench_bproduces a lambda ident
__bench___bench_b), there is no possible functionname that can equal its corresponding lambda ident. This guarantees that
the user can safely bench a function of any valid name without risk of
identifier collision.