Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ergonomics: the error message when trying to instrument const fn is hard to read #2414

Closed
Michcioperz opened this issue Dec 14, 2022 · 2 comments · Fixed by #2418
Closed
Labels
crate/attributes Related to the `tracing-attributes` crate good first issue Good for newcomers kind/bug Something isn't working

Comments

@Michcioperz
Copy link

Bug Report

I tried using #[instrument] on a const fn in a rush in a larger codebase which didn't use tracing before, and got a slightly convoluted compile error message. Once I stopped being angry at my code, it clicked to me why logging in const context would be kind of difficult, but the message doesn't make it super easy.

I think it would be nice if the attribute macro outright rejected instrumenting const functions?

I looked for similar issues, but I suppose I'm first.

Version

michcioperz@Hiyori> cargo tree | grep tracing
└── tracing v0.1.37
    ├── tracing-attributes v0.1.23 (proc-macro)
    └── tracing-core v0.1.30

Platform

michcioperz@Hiyori> uname -a
Darwin Hiyori.local 22.1.0 Darwin Kernel Version 22.1.0: Sun Oct  9 20:15:52 PDT 2022; root:xnu-8792.41.9~2/RELEASE_ARM64_T8112 arm64
michcioperz@Hiyori> rustc --version
rustc 1.65.0 (897e37553 2022-11-02)

Crates

tracing-attributes, I think

Description

I got a minimal reproduction like this, cargo new --lib a, cargo add tracing,

// src/lib.rs
#[tracing::instrument]
const fn a() {
}

Then if you try to compile it:

michcioperz@Hiyori> cargo build
   Compiling a v0.1.0 (/private/var/folders/_0/cbkgpmq97b1bcm8xkcssk_hh0000gn/T/tmp.jEjglWQ0/a)
error[E0277]: can't compare `Level` with `_` in const contexts
 --> src/lib.rs:1:1
  |
1 | #[tracing::instrument]
  | ^^^^^^^^^^^^^^^^^^^^^^ no implementation for `Level < _` and `Level > _`
  |
  = help: the trait `~const PartialOrd<_>` is not implemented for `Level`
note: the trait `PartialOrd<_>` is implemented for `Level`, but that implementation is not `const`
 --> src/lib.rs:1:1
  |
1 | #[tracing::instrument]
  | ^^^^^^^^^^^^^^^^^^^^^^
  = note: this error originates in the macro `tracing::level_enabled` which comes from the expansion of the attribute macro `tracing::instrument` (in Nightly builds, run with -Z macro-backtrace for more info)

error[E0277]: can't compare `Level` with `_` in const contexts
 --> src/lib.rs:1:1
  |
1 | #[tracing::instrument]
  | ^^^^^^^^^^^^^^^^^^^^^^ no implementation for `Level < _` and `Level > _`
  |
  = help: the trait `~const PartialOrd<_>` is not implemented for `Level`
note: the trait `PartialOrd<_>` is implemented for `Level`, but that implementation is not `const`
 --> src/lib.rs:1:1
  |
1 | #[tracing::instrument]
  | ^^^^^^^^^^^^^^^^^^^^^^
  = note: this error originates in the macro `$crate::level_enabled` which comes from the expansion of the attribute macro `tracing::instrument` (in Nightly builds, run with -Z macro-backtrace for more info)

For more information about this error, try `rustc --explain E0277`.
error: could not compile `a` due to 4 previous errors

Thanks for the crate and your time

@hawkw
Copy link
Member

hawkw commented Dec 16, 2022

Hmm, yeah, it looks like those are the errors that are emitted by rustc when the code generated by the #[instrument] macro is expanded into a const fn. We could probably improve this by adding code to #[instrument] that checks if the #[instrument]ed function is const, and expands to our own compile_error! that has a more helpful message, instead.

This should hopefully be pretty straightforward to do, so I'm going to tag this as a "good first issue" if anyone's interested in working on it!

@hawkw hawkw added kind/bug Something isn't working good first issue Good for newcomers crate/attributes Related to the `tracing-attributes` crate labels Dec 16, 2022
@andrewpollack
Copy link
Contributor

andrewpollack commented Dec 17, 2022

I'll take a look at this now! Is there a way to assign myself to this issue?

hawkw pushed a commit that referenced this issue Dec 31, 2022
## Motivation

The `#[instrument]` macro cannot be used on `const fn`s, because the
generated code will perform runtime tracing behavior. However, when
adding the attribute to a `const fn`, the compiler errors generated
currently are somewhat unclear (see #2414). It would be better if we
generated a less verbose error that simply states that `#[instrument]`
is not supported on `const fn`s.

## Solution

This branch changes the `#[instrument]` macro to detect when the
annotated function is a `const fn`, and emit a simpler, more descritpive
error message. The new error simply states that the `#[instrument]`
attribute cannot be used on `const fn`s, and should be much less
confusing to the user.

Fixes #2414
hawkw pushed a commit that referenced this issue Apr 21, 2023
## Motivation

The `#[instrument]` macro cannot be used on `const fn`s, because the
generated code will perform runtime tracing behavior. However, when
adding the attribute to a `const fn`, the compiler errors generated
currently are somewhat unclear (see #2414). It would be better if we
generated a less verbose error that simply states that `#[instrument]`
is not supported on `const fn`s.

## Solution

This branch changes the `#[instrument]` macro to detect when the
annotated function is a `const fn`, and emit a simpler, more descritpive
error message. The new error simply states that the `#[instrument]`
attribute cannot be used on `const fn`s, and should be much less
confusing to the user.

Fixes #2414
hawkw pushed a commit that referenced this issue Apr 21, 2023
## Motivation

The `#[instrument]` macro cannot be used on `const fn`s, because the
generated code will perform runtime tracing behavior. However, when
adding the attribute to a `const fn`, the compiler errors generated
currently are somewhat unclear (see #2414). It would be better if we
generated a less verbose error that simply states that `#[instrument]`
is not supported on `const fn`s.

## Solution

This branch changes the `#[instrument]` macro to detect when the
annotated function is a `const fn`, and emit a simpler, more descritpive
error message. The new error simply states that the `#[instrument]`
attribute cannot be used on `const fn`s, and should be much less
confusing to the user.

Fixes #2414
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
crate/attributes Related to the `tracing-attributes` crate good first issue Good for newcomers kind/bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants