fixes struct contract violation message#2999
fixes struct contract violation message#2999tommymchugh wants to merge 1 commit intoracket:masterfrom tommymchugh:struct-contract-violation
Conversation
| (begin | ||
| (define-values () (begin auto-check ... (values))) | ||
| (define (guard super-field ... non-auto-name ... struct-name) | ||
| (define (guard-contract super-field ... non-auto-name ...) |
There was a problem hiding this comment.
Is there now any difference between this guard-contract and values?
The only differences I'm aware of are arity and name. The arity shouldn't matter here. The name shouldn't matter either, as the error messages come from the contract form which supplies the name with (quote maker).
So you should be able to get by without defining guard-contract, with
(define (guard super-field ... non-auto-name ... struct-name)
((contract (-> super-contract ... non-auto-contracts ... any)
values
....)
super-field ... non-auto-name ...))
One more minor question, I noticed you replaced (current-contract-region) with (quote maker). I don't think there's good reason to change it. Since it's a syntax parameter not a run-time parameter, it won't be affected by being inside the function body.
There was a problem hiding this comment.
Yes, made these changes, but this will probably change anyway with the new ->
|
It isn't a good idea to put the call to I think the right approach here is to make (yet another) variation of |
|
(also, of course, we need test cases!) |
|
@rfindler Roger that, will work on that and update this PR. |
So is it better to put the Is this okay, or is there a downside that I'm not seeing, which would make a new variation of |
|
@AlexKnauth Oh, of course. That's better! Thanks. |
|
That sounds great, just pushed those changes to this PR. |
|
I'm still wondering about the blame information. The old However, the new So to keep the positive blame party the same as it was before, the To do this I would put the definitions of both |
|
@AlexKnauth That did the trick! Thanks so much for the advice. That was what I had noticed when I had initially changed the blame-id since it was outside the struct, but now there is no regression when the |
|
There are no new test cases and the new behavior certainly can be captured with tests. |
|
Sounds good, will add those!
On Wed, Jan 8, 2020 at 7:33 AM Robby Findler ***@***.***> wrote:
There are no new test cases and the new behavior certainly can be captured
with tests.
—
You are receiving this because you authored the thread.
Reply to this email directly, view it on GitHub
<#2999?email_source=notifications&email_token=AAMRJXNW2VPMCZ2FSIGKSMLQ4XI25A5CNFSM4KC46BT2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEIMNHJI#issuecomment-572052389>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AAMRJXPU6GOARQS4TJZVWILQ4XI25ANCNFSM4KC46BTQ>
.
--
*Thomas McHugh*
*Northwestern University*
Undergraduate Student
Email: mchugh@u.northwestern.edu
Phone: 847-922-4801
|
|
(Also we'll want a test case that captures the issue that @AlexKnauth noticed, too -- assuming there wasn't already one?) |
|
ping! Can we look at this again so it is not forgotten? There's an approved review by @AlexKnauth but no merge. |
|
Looks to me like it isn't ready to merge based on the discussion, @pmatos . |
|
No rush! Thanks for your contributions. |
Resolves #2998
Example:
Old Output:
New Output: