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

attributes: add more compile fail tests #306

Closed
japaric opened this issue Dec 15, 2020 · 0 comments · Fixed by #331
Closed

attributes: add more compile fail tests #306

japaric opened this issue Dec 15, 2020 · 0 comments · Fixed by #331
Assignees
Labels
status: needs PR Issue just needs a Pull Request implementing the changes type: test Test coverage or infrastructure improvements
Milestone

Comments

@japaric
Copy link
Member

japaric commented Dec 15, 2020

now that there's CI infrastructure in place for compile fail tests (added in #293) we should add tests for these scenarios:

  • timestamp and panic_handler used on a function with the wrong signature. For instance, all these examples are UB (they are rejected at compile time today):
#[timestamp]
fn foo(x: u64) -> u64 { x + 1 }
#[panic_handler]
fn foo(x: bool) -> ! { info!("{:?}", x); loop {} }
#[panic_handler]
fn foo() {}
  • global_logger used on a struct that has fields. I don't think this is much of an issue (unsure if it's even checked today) but the fields won't be accessed by defmt so accepting this would be misleading at best.

these compile fail tests are also a good place to check the helpfulness of our error messages. For example, this:

defmt::write!(42, "hello");

should say "error: expected defmt::Formatter, found i32", ideally with a span that points to the 42 expression, instead of the a helpful "method needs_tag not found on i32 value". (not sure what the current error message says)

@japaric japaric added status: needs PR Issue just needs a Pull Request implementing the changes type: test Test coverage or infrastructure improvements labels Dec 15, 2020
@japaric japaric added this to the v0.2.0 milestone Jan 5, 2021
@bors bors bot closed this as completed in be07961 Jan 11, 2021
BriocheBerlin added a commit that referenced this issue Jan 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: needs PR Issue just needs a Pull Request implementing the changes type: test Test coverage or infrastructure improvements
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants