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

RFC 2229: Capture Precise Places #31

Closed
wants to merge 9 commits into from
Closed

Conversation

arora-aman
Copy link
Member

@arora-aman arora-aman commented Nov 23, 2020

Following test cases fail and have been disabled.

    [ui] ui/closures/2229_closure_analysis/run_pass/capture-enums.rs
    [ui] ui/closures/2229_closure_analysis/run_pass/destructure_patterns.rs
    [ui] ui/closures/2229_closure_analysis/run_pass/wild_patterns.rs

Test log: http://csclub.uwaterloo.ca/~a52arora/rust-builds/log_53cd79f4-12fa-4392-a0c9-ff6bfa301347

  • I think in the case of patterns, the entire place on the right is being consumed, even though it might not be read at the end.
    Work that remains:
  • Liveness analysis using min_capture map
  • Debug information here should probably be more specific.
  • rustc_mir/interpret/validity.rs
  • regionck
  • Update error logging in borrow code rust-lang/project-rfc-2229#8
  • remove closure_captures and upvar_capture_map

This change is Reviewable

arora-aman and others added 2 commits November 22, 2020 21:13
- When `capture_disjoint_fields` is not set, we would init
  the first pass capture information map with upvars that are mentioned
  within the closure. This would ensure that variables are captured
  in entirety and also handle the case of `let _ = x` when the feature
  gate is disabled.
- However, the place generated before ExprUseVisitor contains
  uninferred types, that can result in same place being recorded
  twice in min capture map.

  Eg:
  ```rust
  let x = 10i32;
  || x += 10;
  ```

  If we initialize before ExprUseVisitor we get two Places in the
  min captured map `Place(x, base_ty: Infer)` and `Place(x, base_ty: i32)`

Co-authored-by: Jennifer Wills <wills.jenniferg@gmail.com>
Co-authored-by: Logan Mosier <logmosier@gmail.com>
Co-authored-by: Jennifer Wills <wills.jenniferg@gmail.com>
Co-authored-by: Logan Mosier <logmosier@gmail.com>
@arora-aman arora-aman requested review from sexxi-bot and a team November 23, 2020 04:10
@sexxi-goose sexxi-goose deleted a comment from sexxi-bot Nov 23, 2020
@sexxi-goose sexxi-goose deleted a comment from sexxi-bot Nov 23, 2020
@arora-aman arora-aman removed the request for review from sexxi-bot November 23, 2020 04:19
@arora-aman arora-aman force-pushed the mir_min_cap_writeback branch 4 times, most recently from 8f759c9 to 2621497 Compare November 26, 2020 05:14
- This allows us to handle upvars when all projections to
  form a capture have been applied to the builder

Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
@arora-aman arora-aman force-pushed the mir_min_cap_writeback branch from 2621497 to d07ac32 Compare November 26, 2020 05:56
arora-aman and others added 6 commits November 26, 2020 00:57
- final_upvar_tys now reads types from places instead of using `node_ty`

Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
- Use closure_min_capture maps to capture precise paths
- PlaceBuilder now searches for ancestors in min_capture list
- Add API to `Ty` to allow access to the n-th element in a
  tuple in O(1) time.

Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
- Closures now use closure_min_captures to figure out captured paths
- Build upvar_mutbls using closure_min_captures
- Change logic in limit_capture_mutability to differ on nested closure
  based on PlaceBase.

Co-authored-by: Roxane Fruytier <roxane.fruytier@hotmail.com>
- Use closure_min_captures to generate the Upvar structure that
  stores information for diagnostics and information about
  mutability of captures.
Wildcards in patterns currently don't work when `capture_disjoint_fields`
is enabled, disabling some run-pass tests until the issue is resolved.
@arora-aman arora-aman force-pushed the mir_min_cap_writeback branch from d07ac32 to ee466ae Compare November 26, 2020 05:58
@arora-aman arora-aman closed this Nov 26, 2020
roxelo pushed a commit that referenced this pull request Jul 4, 2021
New lint: `disallowed_script_idents`

This PR implements a new lint to restrict locales that can be used in the code,
as proposed in rust-lang#7376.

Current concerns / unresolved questions:

- ~~Mixed usage of `script` (as a Unicode term) and `locale` (as something that is easier to understand for the broad audience). I'm not sure whether these terms are fully interchangeable and whether in the current form it is more confusing than helpful.~~ `script` is now used everywhere.
- ~~Having to mostly copy-paste `AllowedScript`. Probably it's not a big problem, as the list of scripts is standardized and is unlikely to change, and even if we'd stick to the `unicode_script::Script`, we'll still have to implement custom deserialization, and I don't think that it will be shorter in terms of the amount of LoC.~~ `unicode::Script` is used together with a filtering deserialize function.
- Should we stick to the list of "recommended scripts" from [UAX #31](http://www.unicode.org/reports/tr31/#Table_Recommended_Scripts) in the configuration?

*Please write a short comment explaining your change (or "none" for internal only changes)*

changelog: ``[`disallowed_script_idents`]``

r? `@Manishearth`
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.

1 participant