<div dir="ltr"><div>Comments inline<br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Wed, Jul 13, 2022 at 9:40 PM Ville Voutilainen <<a href="mailto:ville.voutilainen@gmail.com">ville.voutilainen@gmail.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, 14 Jul 2022 at 00:50, Alan Ruttenberg <<a href="mailto:alanruttenberg@gmail.com" target="_blank">alanruttenberg@gmail.com</a>> wrote:<br>
><br>
> This is what I came up with:<br>
><br>
> <a href="https://github.com/alanruttenberg/abcl/commit/a9c5541d372012d24c0daa704a22fc637398e086" rel="noreferrer" target="_blank">https://github.com/alanruttenberg/abcl/commit/a9c5541d372012d24c0daa704a22fc637398e086</a><br>
><br>
> Depending on the value of switch switch sys::*allow-defstruct-redefinition*. In order to allow the structure to be redefined, we delete the structure class, if there is already one.<br>
><br>
> The undefined behavior is now<br>
>  1. use of an existing struct from before the redefinition.<br>
>  2. creation of functions with the same name as a structure element that has been removed.<br>
>  3. running existing compiled code that uses an accessor for a slot that has changed relative position in the structure.<br>
><br>
><br>
> #2 can be fixed by removing the source transformation for the accessor.<br>
> (sys::%set-function-info accessor  nil). It's not hard - involves iterating through the accessors just before the defstruct is redefined.<br>
> I don't think I'm going to bother fixing this at the moment.<br>
><br>
> #3 can be avoided by (declare (notinline accessor))  in the function being defined. Arguably this is what should be done if (declare (optimize (debug 3))).<br>
> I could also have sys::not-inline-p return true if debug is 3. I may try to do this, since it will be easy to forget to recompile.<br>
> We could at least provide warnings for such functions if we recorded that the source transform was applied, during compilation<br>
><br>
> BTW, if you have an existing (regular) class and create a defstruct with the same name, it blows away the previous class.<br>
> That probably deserves a warning.<br>
><br>
> Comments welcome.<br>
<br>
Greetings from the (for the two decades of it) other side of the<br>
fence, where compilations and one-definition-rules<br>
are rather more static than here. :)<br>
<br>
Sure, this looks plausible, and it probably works in many cases. But<br>
if you COMPILE something with one definition<br>
of a defstruct, then defstruct again, what happens if you try to call<br>
the thing you compiled before?<br></blockquote><div><br></div><div>Seems to work, as long as you don't allow the source transform. Currently I'm experimenting with a new option to defstruct. So</div><div><br></div><div>I compile a file with</div><div><span style="font-family:monospace">(defstruct (test2 (:optimize nil)) a b)<br>(defun foo (a) (test2-a a))</span><br></div><div>Then load it. Then</div><div><span style="font-family:monospace">(foo (make-test2 :a 10))<br>-> 10<br>Then eval</span></div><div><span style="font-family:monospace">(defstruct (test2 (:optimize nil)) b a) ;; swap order of slots<br>(foo (make-test2 :a 10))</span></div><div><span style="font-family:monospace">--> 10 ;; still the correct answer</span><br></div><div><br></div><div>The :optimize nil option prevents the source transforms from being created. The source transforms are what put in positional accessors the generated code like: lispObject.getSlotValue_0 (in the disassembly). Without the source transformation, it compiles to a call to test-2-a. If it didn't then when defstruct was redefined the fixed position accessor could get the wrong slot. But, when you redefine the defstruct test2-a is also redefined. The new function will get the wrong answer if you give it a struct that was created *before* defstruct is redefined. But that's what I've noted in my case 1 above as undefined behavior.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
I don't claim to claim it "can't work". But I have a vague<br>
understanding why there might be a reason for "this might not work".<br>
:P<br>
<br>
As an unsubstantiated rumination, it might be *more* difficult to make<br>
this work in a language that can do dynamic compilation<br>
at any point in a program than it is in a language that is more static<br>
as far as struct definitions and their compilations are concerned.<br>
My architecture-brain can't tell how you could possibly know where all<br>
the 'references' to the old defstruct could possibly be,<br>
considering that compilations with the old one and uses of those can<br>
be so dynamic, and where the uses that would expect<br>
the newer redefinitions might be, and how you'd track that.<br></blockquote><div><br></div><div>It would be possible if there was an annotation on a function that noted the use of a structure accessor. Looking for that annotation across all functions will identify all uses of the accessors, which are *potential* problems. But you would have to sort new from old. However, if the warnings are given just before the new defstruct is created then only functions that were defined before would get warned about. It would be your job to recompile those. But we would only have to do that if the positional assessors were inlined. The (:optimize nil) forces that not to happen. <br></div><div><br></div><div>There's still an issue if you first defined a defstruct without using (:optimize nil), and then redefined it with (:optimize nil). In that case you may lose if you delete or reorder the slots. Not a problem if your redefinition is just adding slots to the end. Anyways,  Don't Do That. Otherwise you will get bugs unless I implement something like what's described in the previous paragraph.<br></div><div><br></div><div>In my use case it's fine because I'm using the option in the first place.<br></div><div><br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
Again, I'm not saying this can't work. I just find it daunting to even<br>
ponder what sort of funny situations where your program<br>
manages to confuse itself about which struct is which you can end up<br>
with. Maybe that's a theoretical problem, but it hurts my brain. :D<br></blockquote><div><br></div><div>There's definitely a screw case, as I explained above. It's a compromise. You get the ability to redefine defstruct, but if you add :optimize nil in the redefinition without it being in the original, then you might have stale compiled code with fixed position accessors. I'm thinking that's a lot better than the current situation, where you can't redefine defstruct at all.  I'm also only planning on doing this while developing the code.</div><div><br></div><div>I'm leaning towards using the defstruct :optimize option vs the (declare (optimize (debug 3))) , since the latter hurts performance of everything. The defstruct option is more targeted.<br></div><div><br></div><div>Alan<br></div></div></div>