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

sem: fix error propagation for tuple construction #1403

Merged

Conversation

zerbina
Copy link
Collaborator

@zerbina zerbina commented Aug 7, 2024

Summary

  • fix errors in anonymous tuple constructions not being propagated
  • fix the error for colon expressions in anonymous tuple constructions
    being "invalid expression" instead of "named expression not allowed
    here"

Details

  • properly propagate errors from element expressions
  • don't analyze nkExprColonExpr as a normal expression
  • don't modify input AST
  • skip tuple type constructor analysis if any of the element
    expressions is erroneous

Summary
=======

* fix errors in anonymous tuple constructions not being propagated
* fix the error for colon expressions in anonymous tuple constructions
  being "invalid expression" instead of "named expression not allowed
  here"

Details
=======

* properly propagate errors from element expressions
* don't analyze `nkExprColonExpr` as a normal expression
* don't modify input AST
* skip tuple type constructor analysis if any of the element
  expressions is erroneous
@zerbina zerbina added bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler labels Aug 7, 2024
tupExp[i] = c.config.newError(tupExp[i],
PAstDiag(kind: adSemNamedExprNotAllowed))
elif isTupleType != (tupExp[i].typ.kind == tyTypeDesc):
if isTupleType != (tupExp[i].typ.kind == tyTypeDesc):
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do we want to allow this in a compile time context?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

At some point, yes, but only very little of the compiler is aware of tyTypeDesc's additional meaning of "fist-class type value", so I don't think it's realistic right now.

@saem
Copy link
Collaborator

saem commented Aug 7, 2024

/merge

Copy link

github-actions bot commented Aug 7, 2024

Merge requested by: @saem

Contents after the first section break of the PR description has been removed and preserved below:


Notes for Reviewers

  • I couldn't come up with some easy way to just verify that error propagation works, so there's no test
  • constructions of named tuples (semTupleFieldConstr) are affected by the same bug

@chore-runner chore-runner bot added this pull request to the merge queue Aug 7, 2024
Merged via the queue into nim-works:devel with commit 22d24d7 Aug 8, 2024
31 checks passed
@zerbina zerbina deleted the sem-fix-tuple-constr-error-propagation branch August 13, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working compiler/sem Related to semantic-analysis system of the compiler
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants