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

Fix split init of const variables and update lvalue errors #15800

Merged
merged 2 commits into from
Jun 10, 2020

Conversation

mppf
Copy link
Member

@mppf mppf commented Jun 10, 2020

Resolves #15335
Related to #5068

Besides allowing split-init of const variables, this PR adjusts l-value
checking errors from resolution to give an error about constness instead
of one mentioning the term "lvalue" when that makes sense. This helps
with, but does not completely resolve, issue #5068.

Note that the future
https://github.com/chapel-lang/chapel/tree/master/test/variables/constants
has requested different wording for the lvalue error that talks about
const variables instead. This PR moves in that direction but it does not
completely remove the potential for getting an lvalue error. In
particular, something like the following example

proc returnsIntByValue() { return 1; }  
returnsIntByValue() = 1;

still gives an error about lvalues.

The compiler change amounted to adjusting isLegalLvalueActualArg to
return whether the error is from the argument being a call-expression
temporary or is from the argument being const. Then, used that to avoid
giving constness errors for out intent arguments at this time, because
later parts of resolution might change them to split-init. Constness
errors for out intent will still be detected in late const checking.

Reviewed by @vasslitvinov - thanks!

  • full local futures testing

@mppf mppf changed the title Fix split init of const variables Fix split init of const variables and update lvalue errors Jun 10, 2020
@mppf mppf merged commit 4fd4a11 into chapel-lang:master Jun 10, 2020
@mppf mppf deleted the fix-15335 branch June 10, 2020 19:58
e-kayrakli added a commit that referenced this pull request Jun 22, 2020
Add performance annotations

- String regressions

  Caused by: #15758
  Likely fix: #15870

- Trivial leak

  Caused by: #15713
  Fixed by: #15871

- AoA Forall Intent Improvement

  Caused by: #15767

- Sampler compile time regression

  Likely caused by: #15800

[Reviewed by @ronawho]
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.

error with split init of const variable through out intent
2 participants