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

Use-before-def problem in queried argument values (when array types are involved?) #23533

Closed
damianmoz opened this issue Sep 26, 2023 · 5 comments · Fixed by #23807
Closed

Use-before-def problem in queried argument values (when array types are involved?) #23533

damianmoz opened this issue Sep 26, 2023 · 5 comments · Fixed by #23807

Comments

@damianmoz
Copy link

Please update the heading/title I have given this if it is imprecise or just plain wrong.

The following complains

proc primefactorsfill(f : [?D] uint(?w), r : [D] uint(w))
{
    ...
}

It says that w is used before being defined.

@bradcray bradcray changed the title Problem in parameter with multiple formals for same type Use-before-def problem in queried argument values (when array types are involved?) Sep 26, 2023
@damianmoz
Copy link
Author

This works:

proc fftplanfactors(n : uint(?w), ufr : [?uD] uint(w))
{
   // use uint(w) within the routine
...
}

This fails:

proc fftplanfactors(ufr : [?uD] uint(?w), n : uint(w))
{
   // use uint(w) within the routine
...
}

Hope this helps.

@dlongnecke-cray
Copy link
Contributor

dlongnecke-cray commented Nov 8, 2023

It looks like fixupArrayFormal in normalize is just straight up deleting the type query?! What the heck...

It's a strange order of operations error... fixupArrayFormal is supposed to replace any mentions of the array element type with arr.eltType, but that deletes the type query...

We currently do this strange "stamping out" in the production compiler where any time we encounter int(?w) we stamp out overloads where w is replaced by each int width. However, this doesn't seem to happen properly when the int(?w) is an array formal's element type e.g., [?d] int(?w). Is this just because this was a pattern we couldn't write in previous releases?

The question is whether or not we stamp out overloads for this array type case as well or if we just leave it generic and see what happens...

@dlongnecke-cray
Copy link
Contributor

dlongnecke-cray commented Nov 8, 2023

It's kind of silly, but I'm just going to extend the current cloning we do today to apply to [?d] int(?w) as well. In the future when the new frontend is the resolver we won't have to do any of this nonsense.

dlongnecke-cray added a commit that referenced this issue Nov 16, 2023
Resolves #23533.

Add support for primitive type queries that are embedded in an array
type expression, e.g., `x: [...] int(?w)`.

During `normalize`, any time the production compiler sees a primitive
type query e.g., `int(?w)`, it will opt to stamp out overloads where `w`
is replaced by each valid bit width (`8`, `16`, `32`, `64`). This
process is repeated recursively for each primitive type query.

This PR just extends the above described process to apply to array types
that are primitive type queries.

This strategy is not ideal, but it is done in the production compiler
because the implementation is convenient. I think ideally we would just
rely on normal generic instantiation to create only the overloads
required by the program. We can wait for this functionality to be
supplied by the new frontend.

TESTING

- [x] `linux64`, `standard`

Reviewed by @riftEmber. Thanks!
@bradcray
Copy link
Member

Thanks for filing this @damianmoz, and for improving the state of things, @dlongnecke-cray! Damian, if you get a chance to try out the fix in your code, please let us know if you're seeing the expected behavior now.

@damianmoz
Copy link
Author

Next week. Thanks

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.

3 participants