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

Improve handling of Constant scalars #1807

Open
EmilyBourne opened this issue Mar 22, 2024 · 2 comments · May be fixed by #1811
Open

Improve handling of Constant scalars #1807

EmilyBourne opened this issue Mar 22, 2024 · 2 comments · May be fixed by #1811

Comments

@EmilyBourne
Copy link
Member

Relevant Discussion

#1805 (comment)

Describe the feature

Declare constant scalars with the fortran parameter keyword and use the fact that their value is known to treat indexing with a constant in the same way as indexing with a literal.

Test Code

Provide code which does not currently work but which should do when this issue is fixed:

def f(a : 'int[:]'):
     pad: 'const int' = 3
     b = a[pad:-pad]

Proposed Solution

A constant variable initialised with a Literal should be saved in a pyccel.ast.variable.Constant . Constants should be declared in the generated code as parameters. They should be handled in the same way as literals when handling indexing (i.e. avoiding unnecessary positivity checks.

_Originally suggested by @said-hadjout in #1805 (comment)

@said-hadjout
Copy link
Contributor

said-hadjout commented Mar 23, 2024

We should handle more complex expressions that can be evaluated at compile time e.g.

def f(a : 'int[:]|int[:,:]'):
     pad: 'const int' = len(a.shape)

otherwise, they will raise an error in Fortran

@EmilyBourne
Copy link
Member Author

As described the issue only optimises for Literals. I agree that handling your example would be great but this is more complex so I would rather leave it for a second issue to avoid having huge PRs

@EmilyBourne EmilyBourne linked a pull request Apr 2, 2024 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants