> On May 4, 2016, at 12:48 PM, Eli Barzilay <[email protected]> wrote: > > On Wed, May 4, 2016 at 1:14 PM, John Clements <[email protected]> > wrote: >> >>> On May 4, 2016, at 9:59 AM, Sam Tobin-Hochstadt <[email protected]> >>> wrote: >>> >>> That seems fine, but the general approach the handin-server is taking >>> seems wrong to me. If it wants an exception with a different message, >>> it should just create that, rather than assuming that all exception >>> structures are reasonable to modify. >> >> Well, I can certainly do both. >> >> In general, perhaps the right solution for the handin server would be >> to deliver an exn:fail:handin-server with an additional field >> containing the original exception. That way, no information is lost. >> >> The danger, of course, is that this might break code that depends on >> the exception satisfying some predicate (the filesystem and network >> exceptions are the ones that worry me). Perhaps Eli can comment on >> this? > > TBH, I have no memory of this -- and looking at the code (I'm assuming > it's the `reraise` bit in `wrap-evaluator`??) I'm not sure that I wrote > it. > > [...Doing some archaeological digging...] > > OK, I think that the following is everything that I can say about it, > and let you judge what would be the best way to re-solve it. I think > that the main thing that changed that you're talking about is Matthew's > comment that "All the built-in exn structs are fully transparent, > though". Also, there's the motivation for doing this: "It's important > to keep the same exception, because ...". (And both of these might be a > justification to make it transparent, or to fix it in a different way > that makes the "because" thing work, and maybe make a note of the lack > of transparency somewhere; I have no significant opinion about it.)
So, I’d say this is basically an ergonomics issue. If we change this code to raise a new exception, then it might potentially confuse a handin-server-checker-writer, who expects (e.g.) to see a ‘exn:fail:contract:variable?’ but actually gets back a ‘exn:fail:handin-server?’. IIUC, clear documentation could resolve this. Of course, this would be a breaking change for people who currently use the handin-server with such tests, but I’m generally in favor of breaking backward compatibility to make the world a better place. Is this change making the world a better place? John > > So -- assuming that function is the right place that you're talking > about, I see that this is code that I committed in: > >> commit fd858f081c564a3c94a682aee5896bc535fd9956 >> Author: Eli Barzilay <[email protected]> >> Date: 2007-01-24 07:52:51 +0000 >> >> removed the tweaker hack for a solution that creates a new exception >> >> svn: r5446 > > and it removes a simple hack that uses a `current-error-message-tweaker` > ("tweaker" is surely mine...) and adds instead the code that assembles a > new exception. > > I then did some more digging in my mail, and found this email exchange > between me and Matthew about this: > > [Eli] >> For some corner of the handin server I wanted to capture exceptions, >> then reraise a modified version of the exception (basically turn any >> exn to one that has "<same message> while evaluating <some expr>"). >> It's important to keep the same exception, because some tests rely >> on it (like catching an `exn:fail:contract:variable?' when testing >> for a bound identifier). >> >> Looks like `copy-struct' is not enough, because it wants a struct-id. >> Is there some easy way to do that? (I know that it's possible, >> because I did similar stuff in reflecting mzscheme structs as swindle >> classes, but I'm looking for a simple solution.) > > [Matthew] >> If you have a sufficiently powerful inspector, then `struct-info' >> and `struct-type-info' let you do what you want, and that's the only >> possibility that I see. > > [Eli] >> Does the code below look reasonable? -- I'm using struct-info just >> to make sure that struct->vector does return all the field values. >> (I think that this code will break with auto fields, but it should >> be fine with the exn hierarchy.) >> >> (define ((make-chatty-eval eval) expr) >> (define (reraise exn) >> (raise >> (let-values ([(struct-type skipped?) (struct-info exn)]) >> (if (and struct-type (not skipped?)) >> (let ([vals (vector->list (struct->vector exn))]) >> (apply (struct-type-make-constructor struct-type) >> (string->immutable-string >> (format "while evaluating ~s:\n ~a" expr (cadr vals))) >> (cddr vals))) >> e)))) >> (with-handlers ([exn? reraise]) (eval expr))) > > [Matthew] >> Looks fine to me. >> >> I don't think the use of `struct-type' ensures that `struct->vector' >> returns all the fields. The immediate struct could be transparentand >> the next one opaque. All the built-in exn structs are fully >> transparent, though. > > [Eli] >> OK -- so I thought that a proper solution would be to check the >> chain up the all the way (and just mention that in a comment in case >> someone uses this code), but then I realized that there is a much >> simpler way: simply define a local unique value, hand that as the >> second argument to struct->vector, and make sure that the unique >> value is not a memq of the result. > > -- > ((x=>x(x))(x=>x(x))) Eli Barzilay: > http://barzilay.org/ Maze is Life! -- You received this message because you are subscribed to the Google Groups "Racket Users" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. For more options, visit https://groups.google.com/d/optout.
