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: Add comptime to trait impls and structs #5511

Merged
merged 5 commits into from
Jul 16, 2024
Merged

Conversation

jfecher
Copy link
Contributor

@jfecher jfecher commented Jul 15, 2024

Description

Problem*

Resolves

Summary*

This is somewhat of a quick fix to the issue in the trait constraint PR and in @asterite's comptime prefix operator PR.

The issue was that the comptime functions/blocks were using other functions which weren't elaborated yet. So they saw an empty function body. Just adding comptime to a function in an impl wasn't sufficient since the compiler didn't look in every function within an impl to know whether to add the impl to the list of comptime items. Moreover, it wouldn't be possible to only add this comptime to a subset of functions within an impl so I decided to allow comptime as a keyword on an entire impl as well - and this is what decides whether or not to add it to the comptime items.

I'm not really satisfied with this solution since it overloads comptime to mean both "run this at compile-time (always)" and "able to be run at compile-time" but I don't have a better solution right now. To some degree the separation is necessary since we need to separate runtime functions that can be modified by comptime functions from comptime functions that cannot be modified (because we've already run them).

Additional Context

Requires #5517 to work

Documentation*

Check one:

  • No documentation needed.
  • Documentation included in this PR.
  • [For Experimental Features] Documentation to be submitted in a separate PR.

PR Checklist*

  • I have tested the changes locally.
  • I have formatted the changes with Prettier and/or cargo fmt on default settings.

@jfecher jfecher requested a review from a team July 15, 2024 16:30
@jfecher jfecher mentioned this pull request Jul 15, 2024
5 tasks
Copy link
Contributor

@michaeljklein michaeljklein left a comment

Choose a reason for hiding this comment

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

LGTM

@jfecher jfecher merged commit 3361f46 into jf/trait-constraint Jul 16, 2024
40 of 42 checks passed
@jfecher jfecher deleted the jf/lazy branch July 16, 2024 20:57
github-merge-queue bot pushed a commit that referenced this pull request Jul 17, 2024
# Description

## Problem\*

Resolves an issue mentioned in this PR:
#5511 And in @asterite's comptime
prefix operators PR.

## Summary\*

Previously, if a function was every (erroneously) allowed to be called
in a comptime context before it had been elaborated, we'd see just an
empty block as its body. I've changed this so we don't default function
bodies to an empty block anymore. Using an option we can now properly
error in this case as well.

## Additional Context



## Documentation\*

Check one:
- [x] No documentation needed.
- [ ] Documentation included in this PR.
- [ ] **[For Experimental Features]** Documentation to be submitted in a
separate PR.

# PR Checklist\*

- [x] I have tested the changes locally.
- [x] I have formatted the changes with [Prettier](https://prettier.io/)
and/or `cargo fmt` on default settings.

---------

Co-authored-by: Tom French <15848336+TomAFrench@users.noreply.github.com>
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.

2 participants