Support constant expressions as instrument field names

[nico-incubiq]

Motivation

The #[instrument] proc macro only allows setting field names as identifiers, which is not on par with other macros such as span! or event! that allow constant expressions. Using constants for example can ensure consistency across a project for the field names used.

This addresses #2969, which was already mentioned in this comment here on #2617 (note that "classic macros" mentioned there already work, there are tests attesting of it).
I also updated the doc block to mention field values can be any arbitrary expressions, which was implemented by #672 but never updated.

Solution

I updated the instrument proc macro to allow constant expressions (const, const fn, literal str, ...) as field names, using the same syntax as #2617, enclosed in curly braces.

const CONST_FIELD_NAME: &str = "foo.bar";

#[instrument(fields({CONST_FIELD_NAME} = "quux"))]
fn fn_const_field_name() {}

Notes

• The identifier (Punctuated) field names have a convenient feature allowing to supersede non-skipped function arguments (a field named s would be used instead of a function argument s in the span). As far as I'm aware this is not possible to implement for constant expressions, as the compiler doesn't allow to evaluate constants at build time (MIRI might be a solution but that seems complicated, and https://rustc-dev-guide.rust-lang.org/const-eval seems experimental). It's a minor inconsistency; in this scenario, the code would panic immediately at startup, and the workaround is as simple as skipping the function argument.

[hds]

Let's add a test for the panic here, to ensure that this check doesn't get accidentally removed at some point in the future. It may be necessary to move the test to a "ui" test to validate the compiler error message.

[nico-incubiq]

I no longer think adding a panic or deduplication logic for constant expressions is feasible at compile time. Every solution I tested incurs a runtime cost, as constant expressions need to be evaluated first, and then every value needs to be compared against each other.
Given that the impact of not doing it is "just" duplicated fields reported, and is limited to people newly opting into braced constant expressions, I don't expect this to cause much problem.

I updated the docs to explain this.

[NickLarsenNZ]

@nico-incubiq , are you still planning to work on this?

[nico-incubiq]

@nico-incubiq , are you still planning to work on this?

I'd like to pick this back up yes.
Although, I'm not sure the suggested approach of creating a new constructor to avoid introducing a panic achieves the original goal as it wouldn't be possible to use constants in existing attributes. I would like attributes to be on par with macros as introduced by #2617.
I've been looking for a way to evaluate the constant at compile time to avoid panicking, but I'm not sure how.

Also I've used Block for the FieldName because it is already parsed enclosed in braces, but that's much too broad, ideally I'd rather parse { + Expr + }. Done.

[nico-incubiq]

This is ready for review again when someone has time 🙏🏼