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

fix: Inconsistent behaviour with lifetime elision on tracked fn #596

Merged
merged 1 commit into from
Oct 17, 2024

Conversation

ShoyuVanilla
Copy link
Contributor

@ShoyuVanilla ShoyuVanilla commented Oct 15, 2024

Fixes #519

I think that there would be roughly two options for fixing this;

  1. Returning error while proc macro expansion
  2. Inserting a type alias like type __T<'db> = DbInputArgType; inside the expanded function's body so that the expanded code fails to compile

In this PR, I chose the first one because the second is little bit non-intuitive and the expanded code is slightly more bloated.
But the second one also has its own merit; It can utilize rustc's more detailed error message and is more consistent with the case that the other input parameter has an elided lifetime.
So, I'm okay to switch to the second one if I have to 😄

Copy link

netlify bot commented Oct 15, 2024

Deploy Preview for salsa-rs canceled.

Name Link
🔨 Latest commit 57f38aa
🔍 Latest deploy log https://app.netlify.com/sites/salsa-rs/deploys/670fe3a07841da0008accd0e

Copy link

codspeed-hq bot commented Oct 15, 2024

CodSpeed Performance Report

Merging #596 will not alter performance

Comparing ShoyuVanilla:issue-519 (57f38aa) with master (c6c51a0)

Summary

✅ 8 untouched benchmarks

Copy link
Member

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it.

Copy link
Member

@nikomatsakis nikomatsakis left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: one test

tests/compile-fail/tracked_fn_incompatibles.rs Outdated Show resolved Hide resolved
@ShoyuVanilla ShoyuVanilla force-pushed the issue-519 branch 2 times, most recently from 6e024f8 to bbc253f Compare October 16, 2024 15:59
@nikomatsakis nikomatsakis added this pull request to the merge queue Oct 17, 2024
Merged via the queue into salsa-rs:master with commit 710691d Oct 17, 2024
10 checks passed
@ShoyuVanilla ShoyuVanilla deleted the issue-519 branch October 17, 2024 14:57
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.

inconsistent behavior with respect to lifetime elision
2 participants