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

Refactor string attributes and add constant string implementations #2524

Merged
merged 9 commits into from
Feb 18, 2024

Conversation

advikkabra
Copy link
Collaborator

  • istitle and isalpha were implemented only for variables and not constant strings, so I added implementations and tests.
  • Moved their tests to test_str_attributesalong with all the other string methods.
  • Removed some unnecessarycode in the python implementation of istitle
  • Refactored the istitle and isalpha logic in python_ast_to_asr.cpp to follow the other is methods.

@certik
Copy link
Contributor

certik commented Feb 11, 2024

Not sure what is going on, but somehow the CI shows an error:

semantic error: Variable 'x' not declared
   --> /home/runner/work/lpython/lpython/src/bin/../runtime/lpython_builtin.py:348:16
    |
348 |     return c64(x) + c64(0)*1j
    |                ^ 

@certik certik marked this pull request as draft February 11, 2024 22:53
@advikkabra
Copy link
Collaborator Author

I am not sure what is going on either, as there are no changes in lpython_builtin.py which should cause an error there. I will take a look again.

@advikkabra
Copy link
Collaborator Author

advikkabra commented Feb 12, 2024

I tried building and running the tests locally, but I could not replicate the issue. I am using Ubuntu 22.04, so I am unsure why the CI is failing.

@interface
@overload
def complex(x: f64) -> c64:
    return c64(x) + c64(0)*1j

This is the function where there is an error. x is declared, so I do not know why there is a bug.

@advikkabra
Copy link
Collaborator Author

@certik Please take a look at this when you can, I am not sure how to fix this issue.

@advikkabra advikkabra marked this pull request as ready for review February 17, 2024 18:32
Copy link
Contributor

@certik certik left a comment

Choose a reason for hiding this comment

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

I think this is fine. I don't know why it failed. We'll watch the CI for this error.

@certik certik merged commit 21ba898 into lcompilers:main Feb 18, 2024
13 checks passed
@advikkabra advikkabra deleted the strattributes branch July 21, 2024 13:22
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