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 for VVM with non-fixed version pragma #315

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ritzdorf
Copy link
Contributor

What I did

There was the following problem with non-fixed pragmas (vyper_version = 0.4.0):

import boa
boa.load("examples/simple.vy")

Leads to error:

vyper.exceptions.VersionException: Version specification "~=0.3.9" is not compatible with compiler version "0.4.0"

simple.vy:

# pragma version ^0.3.9
@external
def foo() -> uint256:
    x: uint256 = 1
    return x + 7

That happens because _detect_version returns None in this case, so VVM is not used, but compilation also fails.

So I changed _detect_version so that it:

  1. Parses the full pragma (including >= or ^)
  2. Decides on the right version depending on the pragma and the optionally provided vyper_version
  3. Returns the right version

How I did it

  • If a fixed version is specified: it is always returned
  • If ^version is specified:
    • If the vyper_version is specified and allowed, the vyper_version is returned
    • Else, the minimum version is returned
  • If >=version is specified:
    • If the vyper_version is specified and allowed, the vyper_version is returned
    • Else, the minimum version is returned

How to verify it

See the new tests I added

Description for the changelog

Parse version pragma more precisely to use vvm if necessary

Cute Animal Picture

Copy link
Member

@charles-cooper charles-cooper left a comment

Choose a reason for hiding this comment

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

@ritzdorf we just merged vyperlang/vvm@c9a294b in vvm, will that help address the issue here?

@DanielSchiavini
Copy link
Collaborator

@charles-cooper I guess we will need a release. But first I found this issue: vyperlang/vvm#24

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.

3 participants