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.

Sign up for GitHub

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

Jump to bottom

Allow prefixes in any call path. #472

Merged
merged 43 commits into from
Feb 23, 2022
Merged

Allow prefixes in any call path. #472

merged 43 commits into from
Feb 23, 2022

Conversation

emilyaherbert
Copy link
Contributor

Closes #75

@emilyaherbert emilyaherbert added the compiler General compiler. Should eventually become more specific as the issue is triaged label Dec 17, 2021
@emilyaherbert emilyaherbert self-assigned this Dec 17, 2021
@emilyaherbert emilyaherbert marked this pull request as ready for review January 13, 2022 21:06
@emilyaherbert
Copy link
Contributor Author

emilyaherbert commented Jan 14, 2022 •
edited
Loading

The failing test case is due to a separate issue (#642) that will need to be fixed before this can go in.

@emilyaherbert emilyaherbert force-pushed the emilyaherbert-75/allow-prefixes-in-any-callpath branch from 8fa1589 to 6ef76ff Compare February 1, 2022 00:06
sezna
sezna previously approved these changes Feb 2, 2022
View reviewed changes
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

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

@otrho @mohammadfawaz does this make the issues with symbols not being absolute any better? The namespace should be able to determine what's what based on the call path now...I think.

@otrho
Copy link
Contributor

otrho commented Feb 3, 2022

@otrho @mohammadfawaz does this make the issues with symbols not being absolute any better? The namespace should be able to determine what's what based on the call path now...I think.

I'll have to see it in practice -- my impression was the prefix is now available everywhere but still optional?

@sezna
Copy link
Contributor

sezna commented Feb 3, 2022

🚨 merge conflicts 🚨

@emilyaherbert
Copy link
Contributor Author

I'll have to see it in practice -- my impression was the prefix is now available everywhere but still optional?

That's correct!

@sezna
Copy link
Contributor

sezna commented Feb 11, 2022

This is my fault for not reviewing quickly but another conflict has arisen :(

otrho
otrho suggested changes Feb 16, 2022
View reviewed changes
Copy link
Contributor

@otrho otrho left a comment

Choose a reason for hiding this comment

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

This guy is getting a bit old, I was considering fixing the merge request myself and merging this PR since I know you're busy ATM, but I still have a couple of questions.

test/src/e2e_vm_tests/test_programs/match_expressions_non_exhaustive_example/Forc.toml Outdated Show resolved Hide resolved
test/src/e2e_vm_tests/test_programs/use_full_path_names/src/main.sw Outdated Show resolved Hide resolved
@emilyaherbert
Copy link
Contributor Author

Pinging @otrho @sezna @canndrew before any merge conflicts 😄

sezna
sezna previously approved these changes Feb 22, 2022
View reviewed changes
Copy link
Contributor

@sezna sezna left a comment

Choose a reason for hiding this comment

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

left tiny nit

test/src/e2e_vm_tests/test_programs/use_full_path_names/src/main.sw Outdated Show resolved Hide resolved
@emilyaherbert
Copy link
Contributor Author

Pinging @otrho @sezna again 😄

@mohammadfawaz mohammadfawaz mentioned this pull request Feb 23, 2022
Merged
@otrho otrho merged commit 24c879c into master Feb 23, 2022
@otrho otrho deleted the emilyaherbert-75/allow-prefixes-in-any-callpath branch February 23, 2022 23:51
This was referenced Mar 14, 2022
Closed
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@adlerjohn adlerjohn adlerjohn left review comments

@otrho otrho otrho approved these changes

@sezna sezna sezna approved these changes

@canndrew canndrew Awaiting requested review from canndrew

Assignees

@emilyaherbert emilyaherbert

Labels
compiler General compiler. Should eventually become more specific as the issue is triaged
Projects
Fuel Network
Archived in project
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Allow for :: prefixes in _any_ call path, not just imports.
4 participants
@emilyaherbert @otrho @sezna @adlerjohn