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

NLL Diagnostic Review 3: Diagnostic mentions lack of initialization instead of incorrect mutability #55652

Closed
davidtwco opened this issue Nov 3, 2018 · 1 comment · Fixed by #55657
Assignees
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-diagnostics Working towards the "diagnostic parity" goal P-medium Medium priority

Comments

@davidtwco
Copy link
Member

@pnkfelix's review comments:

ui/borrowck/reassignment_immutable_fields_overlapping.nll.stderr AST-borrowck complains about the local being immutable. NLL complains about lack of initialization. This is probably expected, but we should double-check.

@davidtwco davidtwco self-assigned this Nov 3, 2018
@davidtwco davidtwco added A-NLL Area: Non-lexical lifetimes (NLL) NLL-diagnostics Working towards the "diagnostic parity" goal labels Nov 3, 2018
@pnkfelix
Copy link
Member

pnkfelix commented Nov 6, 2018

triage: P-medium. (We should look into this, but even if its wrong, I'm not going to push to a backport its hypothetical bug fix.)

@pnkfelix pnkfelix added the P-medium Medium priority label Nov 6, 2018
bors added a commit that referenced this issue Nov 11, 2018
NLL Diagnostic Review 3: Unions not reinitialized after assignment into field

Fixes #55651, #55652.

This PR makes two changes:

First, it updates the dataflow builder to add an init for the place
containing a union if there is an assignment into the field of
that union.

Second, it stops a "use of uninitialized" error occuring when there is an
assignment into the field of an uninitialized union that was previously
initialized. Making this assignment would re-initialize the union, as
tested in `src/test/ui/borrowck/borrowck-union-move-assign.nll.stderr`.
The check for previous initialization ensures that we do not start
supporting partial initialization yet (cc #21232, #54499, #54986).

This PR also fixes #55652 which was marked as requiring investigation
as the changes in this PR add an error that was previously missing
(and mentioned in the review comments) and confirms that the error
that was present is correct and a result of earlier partial
initialization changes in NLL.

r? @pnkfelix (due to earlier work with partial initialization)
cc @nikomatsakis
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-NLL Area: Non-lexical lifetimes (NLL) NLL-diagnostics Working towards the "diagnostic parity" goal P-medium Medium priority
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants