Fold/visit tweaks#156130
Conversation
rustbot
added
S-waiting-on-review
|
|
| fn has_type_flags(&self, flags: TypeFlags) -> bool { | ||
| let res = | ||
| self.visit_with(&mut HasTypeFlagsVisitor { flags }) == ControlFlow::Break(FoundFlags); | ||
| res |
There was a problem hiding this comment.
imo comparing with ControlFlow::Break(FoundFlags) is clearer, because it asserts that Break contains FoundFlags
There was a problem hiding this comment.
Even though FoundFlags is a unit type?
There was a problem hiding this comment.
Note that has_vars_bound_at_or_above uses .is_break(). I was fixing the inconsistency and .is_break() seemed nicer.
There was a problem hiding this comment.
Even though FoundFlags is a unit type?
Yes; With == ControlFlow::Break(FoundFlags) it's clearer why Break means that the flags have been found, without looking at surrounding code / type returned by visit_with.
But ultimately this is such a small nitpick...
I always choose a reviewer rather than letting bors randomly assign. Usually I choose somebody who knows the relevant part of the code well. Or if it's an easier PR that doesn't require specific expertise I just pick someone who I haven't picked for a while. |
- Remove comment about blanket implementation of `FallibleTypeFolder` -- it does not exist. (It probably used to.) - Fix some incorrect name references. - Fix minor grammatical errors. - Fix stale comment on `visit_region` -- it was a no-op once, but no longer. LLM disclosure: Claude Code identified these problems with comments when I asked it to review `fold.rs` and `visit.rs`. I verified the correctness of the problems and made the changes by hand.
nnethercote
force-pushed
the
fold-visit-tweaks
branch
from
fd068fd to
dd4ce84
Compare
|
I addressed two of the three comments, and left the |
| fn has_type_flags(&self, flags: TypeFlags) -> bool { | ||
| let res = | ||
| self.visit_with(&mut HasTypeFlagsVisitor { flags }) == ControlFlow::Break(FoundFlags); | ||
| res |
There was a problem hiding this comment.
Even though FoundFlags is a unit type?
Yes; With == ControlFlow::Break(FoundFlags) it's clearer why Break means that the flags have been found, without looking at surrounding code / type returned by visit_with.
But ultimately this is such a small nitpick...
- Introduce `HAS_REGIONS`/`has_regions`, which is true if any regions are present. - Add `fold_clauses` to `Shifter` and `RegionFolder` for consistency. - Simplify `has_type_flags`. - Remove unnecessary local variables in `HasTypeFlagsVisitor` methods. - Use `|=` operator in one place. - Avoid `is_break` in one place for consistency with nearby code. LLM disclosure: Claude Code suggested these changes when I asked it to review `fold.rs` and `visit.rs`. I verified the correctness of the suggestions and made the changes by hand.
nnethercote
force-pushed
the
fold-visit-tweaks
branch
from
dd4ce84 to
803c7ba
Compare
|
I addressed the latest two comments. |
|
@bors r+ rollup |
rust-bors
Bot
added
S-waiting-on-bors
…affleLapkin Fold/visit tweaks Details in individual commits. r? @WaffleLapkin
3 participants
Details in individual commits.
r? @WaffleLapkin