-
Notifications
You must be signed in to change notification settings - Fork 4
Execute drop glue for Drop terminators #999
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
base: master
Are you sure you want to change the base?
Changes from 19 commits
c6f1b0d
e9e918d
5e64648
83eb9b5
e70f00e
e869bb7
d099d10
81ee256
da9b180
4f4c96b
432a60b
77ed24a
4f34cdc
9802073
591638b
910ae06
45f8743
c6d1d10
c7c7548
bce52aa
79bd202
60b5e5a
8c27c7e
662427f
8a59eed
7fb84c9
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -508,6 +508,17 @@ The following rule resolves this situation by using the head element. | |
| ) | ||
| => #traverseProjection(DEST, VALUE, projectionElemField(IDX, TY) PROJS, CTXTS) ... </k> // TODO mark context? | ||
| [preserves-definedness, priority(100)] | ||
|
|
||
| // Temporary bridge rule: after PointerOffset lifts a single value to Range(ListItem(...)), | ||
| // unwrap a Union head element so the existing Union + Field rules below can keep running. | ||
| rule <k> #traverseProjection( | ||
| DEST, | ||
| Range(ListItem(Union(_, _) #as VALUE) _REST:List), | ||
| projectionElemField(IDX, TY) PROJS, | ||
| CTXTS | ||
| ) | ||
| => #traverseProjection(DEST, VALUE, projectionElemField(IDX, TY) PROJS, CTXTS) ... </k> // TODO mark context? | ||
| [preserves-definedness, priority(100)] | ||
|
Comment on lines
+512
to
+521
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I am concerned to add these temporary rules. I feel getting these mix ups with the projections is indicative that we are handing the pointers wrong, and by adding rules like this we suppress the underlying problem. I know they are temporary, but a bandaid added and forgotten could cause deep confusion later...
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I agree with the concern, and I do not want these bridge rules to become permanent sediment. For this PR they are only meant as a narrow regression fix for the newly exercised drop-glue path. I opened follow-up issue #1011 to track the real cleanup: #1011 The plan there is to refactor
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Agree with both, requiring this rule indicates a problem with how pointers are handled. It is specialised to |
||
| ``` | ||
|
|
||
| #### Unions | ||
|
|
@@ -649,6 +660,25 @@ Similar to `ConstantIndex`, the slice _end_ index may count from the _end_ or t | |
| andBool START <=Int size(ELEMENTS) -Int END | ||
| [preserves-definedness] // Indexes checked to be in range for ELEMENTS | ||
|
|
||
| // Temporary bridge rule: PointerOffset is implemented below in terms of Range slicing, so | ||
| // lift a single non-Range value to Range(ListItem(...)) to reuse that shared path. | ||
| rule <k> #traverseProjection( | ||
| DEST, | ||
| VAL, | ||
| PointerOffset(OFFSET, ORIGIN_LENGTH) PROJS, | ||
| CTXTS | ||
| ) | ||
| => #traverseProjection( | ||
| DEST, | ||
| Range(ListItem(VAL)), | ||
| PointerOffset(OFFSET, ORIGIN_LENGTH) PROJS, | ||
| CTXTS | ||
| ) | ||
| ... | ||
| </k> | ||
| requires notBool isRange(VAL) | ||
| [preserves-definedness, priority(100)] | ||
|
|
||
| rule <k> #traverseProjection( | ||
| DEST, | ||
| Range(ELEMENTS), | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.