Ptr#1469
Conversation
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 42ce9fb159
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: ce37ed5675
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if root_local.is_some_and(|local| { | ||
| self.body | ||
| .locals | ||
| .get(local.index()) | ||
| .is_some_and(|local| local.ty.as_borrow(self.db).is_some()) | ||
| }) { | ||
| current |
There was a problem hiding this comment.
Preserve non-root deref projections in place classification
This Projection::Deref branch skips dereferencing whenever the root local is a borrow, but it does so for every deref in the projection path instead of only the root-level borrow deref. For places like Deref -> Field -> Deref -> ... (e.g. accessing through a pointer field of a borrowed root), the second deref is incorrectly ignored, so later projection is computed against the wrong runtime class and can mis-lower or panic on invalid class projections. The skip needs to be limited to the initial borrow-root deref, not all derefs.
Useful? React with 👍 / 👎.
|
On hold till after 26.2 release. |
micahscopes
left a comment
There was a problem hiding this comment.
agent:
One borrowck question without a single line to hang it on: callee analysis
seeds pointer params as pairwise-disjoint pointees, but I couldn't find
anywhere that rejectsf(p, p)— no check at the call site, no diagnostic in
the callee. Is the no-alias contract meant to be checked, modeled as may-alias,
or documented as a soundness precondition?
| if dst_ty.as_borrow(self.db).is_some() || !is_pointer_bearing_type(self.db, dst_ty) { | ||
| return Ok(()); | ||
| } |
There was a problem hiding this comment.
agent:
If I'm reading this right, a call whose result isn't pointer-bearing exits here,
and since summaries only describe the return value, callee writes through mut
args never invalidate caller facts —fn set(h: mut Holder, p: *u256) { h.ptr = p }
leaves the caller holding a stale strong fact, and the later mut/mut conflict on
*h.ptrgoes undetected (strong_lookup's exact hit suppresses the Unknown
fallback). Is havoc on arg-reachable slots planned? Similar staleness shows up
with >32-elem arrays: a whole-array overwrite at[Any]doesn't clear an earlier
arr[5]fact.
| impl<T> MemArray<T> { | ||
| #[arithmetic(unchecked)] | ||
| pub fn new_uninit(_ len: u256) -> Self { | ||
| Self::from_raw_parts(ptr: cast<u8, T>(alloc_raw(len * size_of<T>())), len: len) |
There was a problem hiding this comment.
agent:
len * size_of<T>()wraps here (fn is#[arithmetic(unchecked)]), so len ≥ 2^251
gives a tiny allocation while the stored len keeps the Index bounds check happy —
OOB writes through the safe API.byte_len()is checked; probably wants a checked
multiply here too.
|
agent:
|
|
agent:
|
|
agent:
|
Diff line count is mostly due to the treesitter grammar update.