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

Dyno: fix decls with tuple type expressions and initialization expressions #25951

Merged
merged 2 commits into from
Sep 30, 2024

Conversation

DanilaFe
Copy link
Contributor

@DanilaFe DanilaFe commented Sep 16, 2024

Closes https://github.com/Cray/chapel-private/issues/6385; prior to this PR, the following program:

var x: 2*int = (7,3);

Produced an error:

─── error in onetuple.chpl:1 [IncompatibleTypeAndInit] ───
  Type mismatch between declared type of 'x' and initialization expression.
  In the following declaration:
      |
    1 | var x: 2*int = (7,3);
      |        ⎺⎺⎺⎺⎺   ⎺⎺⎺⎺⎺
      |
  the type specifier has type '(int(64), int(64))', while the initial value has type '(int(64), int(64))'.

The (chapel syntax) output is quite confusing in the error message, but the difference between the specified type and the actual type are the intents of the components. On the left hand site we have a 'type' tuple, but on the right we have a value tuple.

There is already logic on main to adjust the type of the declaration at the end to mark the tuple components 'var' or 'ref', etc. However, the type-value mismatch happens while the type of the declaration is being inferred, before the final adjustment. The solution seems to be to adjust the type of the tuple's components using the same rules as "usual" (described, I believe, in the specification here). So, this PR extracts the "adjustment" process, runs it once on the type expression to ensure it's compatible with the value, then again on the final type.

Reviewed by @benharsh -- thanks!

Testing

  • dyno tests

Signed-off-by: Danila Fedorin <daniel.fedorin@hpe.com>
Signed-off-by: Danila Fedorin <daniel.fedorin@hpe.com>
@DanilaFe DanilaFe merged commit c77aedc into chapel-lang:main Sep 30, 2024
7 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants