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: string to bool conversion #3391

Merged
merged 3 commits into from
May 8, 2023

Conversation

tserg
Copy link
Collaborator

@tserg tserg commented May 8, 2023

What I did

Conversion of string to bool should follow that of BytesT to compare non-zero bytes, but it currently compares pointer instead.

How I did it

Add StringT to condition

How to verify it

See tests

Commit message

fix: string to bool conversion

Description for the changelog

Fix string to bool conversion

Cute Animal Picture

Put a link to a cute animal picture inside the parenthesis-->

@codecov-commenter
Copy link

codecov-commenter commented May 8, 2023

Codecov Report

Merging #3391 (d2f4d08) into master (3c83947) will decrease coverage by 0.28%.
The diff coverage is 0.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##           master    #3391      +/-   ##
==========================================
- Coverage   88.92%   88.64%   -0.28%     
==========================================
  Files          86       86              
  Lines       10726    10726              
  Branches     2440     2440              
==========================================
- Hits         9538     9508      -30     
- Misses        787      814      +27     
- Partials      401      404       +3     
Impacted Files Coverage Δ
vyper/builtins/_convert.py 90.87% <0.00%> (ø)

... and 6 files with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@@ -267,7 +267,7 @@ def _literal_decimal(expr, arg_typ, out_typ):
def to_bool(expr, arg, out_typ):
_check_bytes(expr, arg, out_typ, 32) # should we restrict to Bytes[1]?

if isinstance(arg.typ, BytesT):
if isinstance(arg.typ, (BytesT, StringT)):
Copy link
Member

Choose a reason for hiding this comment

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

we probably want _BytestringT here

@charles-cooper charles-cooper added this to the v0.3.8 milestone May 8, 2023
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