Re: Why can't defstructs be redefined?

Alan Ruttenberg <[email protected]> Wed, 13 Jul 2022 23:46:36 -0400
Newsgroups gmane.editors.j.devel
Message-ID <CAFKQJ8kZdzHTbk+Df7Nv-q2HOmn06Sh19zu9+1TsKAuKwYA6ww@mail.gmail.com>
--00000000000034e6ac05e3bbc121
Content-Type: text/plain; charset="UTF-8"

Here's the patch for (:optimize nil) option.

https://github.com/alanruttenberg/abcl/commit/81d3a202544f2071aa6acbbeeb893fdeb7d19175

Thanks for the comments, keep them coming.

On Wed, Jul 13, 2022 at 10:33 PM Alan Ruttenberg <[email protected]>
wrote:

> Comments inline
>
> On Wed, Jul 13, 2022 at 9:40 PM Ville Voutilainen <
> [email protected]> wrote:
>
>> On Thu, 14 Jul 2022 at 00:50, Alan Ruttenberg <[email protected]>
>> wrote:
>> >
>> > This is what I came up with:
>> >
>> >
>> https://github.com/alanruttenberg/abcl/commit/a9c5541d372012d24c0daa704a22fc637398e086
>> >
>> > 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.
>> >
>> > The undefined behavior is now
>> >  1. use of an existing struct from before the redefinition.
>> >  2. creation of functions with the same name as a structure element
>> that has been removed.
>> >  3. running existing compiled code that uses an accessor for a slot
>> that has changed relative position in the structure.
>> >
>> >
>> > #2 can be fixed by removing the source transformation for the accessor.
>> > (sys::%set-function-info accessor  nil). It's not hard - involves
>> iterating through the accessors just before the defstruct is redefined.
>> > I don't think I'm going to bother fixing this at the moment.
>> >
>> > #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))).
>> > 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.
>> > We could at least provide warnings for such functions if we recorded
>> that the source transform was applied, during compilation
>> >
>> > BTW, if you have an existing (regular) class and create a defstruct
>> with the same name, it blows away the previous class.
>> > That probably deserves a warning.
>> >
>> > Comments welcome.
>>
>> Greetings from the (for the two decades of it) other side of the
>> fence, where compilations and one-definition-rules
>> are rather more static than here. :)
>>
>> Sure, this looks plausible, and it probably works in many cases. But
>> if you COMPILE something with one definition
>> of a defstruct, then defstruct again, what happens if you try to call
>> the thing you compiled before?
>>
>
> Seems to work, as long as you don't allow the source transform. Currently
> I'm experimenting with a new option to defstruct. So
>
> I compile a file with
> (defstruct (test2 (:optimize nil)) a b)
> (defun foo (a) (test2-a a))
> Then load it. Then
> (foo (make-test2 :a 10))
> -> 10
> Then eval
> (defstruct (test2 (:optimize nil)) b a) ;; swap order of slots
> (foo (make-test2 :a 10))
> --> 10 ;; still the correct answer
>
> 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.
>
>
>> I don't claim to claim it "can't work". But I have a vague
>> understanding why there might be a reason for "this might not work".
>> :P
>>
>> As an unsubstantiated rumination, it might be *more* difficult to make
>> this work in a language that can do dynamic compilation
>> at any point in a program than it is in a language that is more static
>> as far as struct definitions and their compilations are concerned.
>> My architecture-brain can't tell how you could possibly know where all
>> the 'references' to the old defstruct could possibly be,
>> considering that compilations with the old one and uses of those can
>> be so dynamic, and where the uses that would expect
>> the newer redefinitions might be, and how you'd track that.
>>
>
> 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.
>
> 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.
>
> In my use case it's fine because I'm using the option in the first place.
>
> Again, I'm not saying this can't work. I just find it daunting to even
>> ponder what sort of funny situations where your program
>> manages to confuse itself about which struct is which you can end up
>> with. Maybe that's a theoretical problem, but it hurts my brain. :D
>>
>
> 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.
>
> 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.
>
> Alan
>

--00000000000034e6ac05e3bbc121
Content-Type: text/html; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable

<div dir=3D"ltr"><div dir=3D"ltr"><div>Here&#39;s the patch for (:optimize =
nil) option.</div><div><br></div><div><a href=3D"https://github.com/alanrut=
tenberg/abcl/commit/81d3a202544f2071aa6acbbeeb893fdeb7d19175" target=3D"_bl=
ank">https://github.com/alanruttenberg/abcl/commit/81d3a202544f2071aa6acbbe=
eb893fdeb7d19175</a></div><div><br></div><div>Thanks for the comments, keep=
 them coming.<br></div></div><br><div class=3D"gmail_quote"><div dir=3D"ltr=
" class=3D"gmail_attr">On Wed, Jul 13, 2022 at 10:33 PM Alan Ruttenberg &lt=
;<a href=3D"mailto:[email protected]" target=3D"_blank">alanruttenbe=
[email protected]</a>&gt; wrote:<br></div><blockquote class=3D"gmail_quote" styl=
e=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);paddin=
g-left:1ex"><div dir=3D"ltr"><div>Comments inline<br></div><br><div class=
=3D"gmail_quote"><div dir=3D"ltr" class=3D"gmail_attr">On Wed, Jul 13, 2022=
 at 9:40 PM Ville Voutilainen &lt;<a href=3D"mailto:ville.voutilainen@gmail=
.com" target=3D"_blank">[email protected]</a>&gt; wrote:<br></div=
><blockquote class=3D"gmail_quote" style=3D"margin:0px 0px 0px 0.8ex;border=
-left:1px solid rgb(204,204,204);padding-left:1ex">On Thu, 14 Jul 2022 at 0=
0:50, Alan Ruttenberg &lt;<a href=3D"mailto:[email protected]" targe=
t=3D"_blank">[email protected]</a>&gt; wrote:<br>
&gt;<br>
&gt; This is what I came up with:<br>
&gt;<br>
&gt; <a href=3D"https://github.com/alanruttenberg/abcl/commit/a9c5541d37201=
2d24c0daa704a22fc637398e086" rel=3D"noreferrer" target=3D"_blank">https://g=
ithub.com/alanruttenberg/abcl/commit/a9c5541d372012d24c0daa704a22fc637398e0=
86</a><br>
&gt;<br>
&gt; Depending on the value of switch switch sys::*allow-defstruct-redefini=
tion*. In order to allow the structure to be redefined, we delete the struc=
ture class, if there is already one.<br>
&gt;<br>
&gt; The undefined behavior is now<br>
&gt;=C2=A0 1. use of an existing struct from before the redefinition.<br>
&gt;=C2=A0 2. creation of functions with the same name as a structure eleme=
nt that has been removed.<br>
&gt;=C2=A0 3. running existing compiled code that uses an accessor for a sl=
ot that has changed relative position in the structure.<br>
&gt;<br>
&gt;<br>
&gt; #2 can be fixed by removing the source transformation for the accessor=
.<br>
&gt; (sys::%set-function-info accessor=C2=A0 nil). It&#39;s not hard - invo=
lves iterating through the accessors just before the defstruct is redefined=
.<br>
&gt; I don&#39;t think I&#39;m going to bother fixing this at the moment.<b=
r>
&gt;<br>
&gt; #3 can be avoided by (declare (notinline accessor))=C2=A0 in the funct=
ion being defined. Arguably this is what should be done if (declare (optimi=
ze (debug 3))).<br>
&gt; I could also have sys::not-inline-p return true if debug is 3. I may t=
ry to do this, since it will be easy to forget to recompile.<br>
&gt; We could at least provide warnings for such functions if we recorded t=
hat the source transform was applied, during compilation<br>
&gt;<br>
&gt; BTW, if you have an existing (regular) class and create a defstruct wi=
th the same name, it blows away the previous class.<br>
&gt; That probably deserves a warning.<br>
&gt;<br>
&gt; 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&#39;t allow the source transform. Currently I&#39=
;m experimenting with a new option to defstruct. So</div><div><br></div><di=
v>I compile a file with</div><div><span style=3D"font-family:monospace">(de=
fstruct (test2 (:optimize nil)) a b)<br>(defun foo (a) (test2-a a))</span><=
br></div><div>Then load it. Then</div><div><span style=3D"font-family:monos=
pace">(foo (make-test2 :a 10))<br>-&gt; 10<br>Then eval</span></div><div><s=
pan style=3D"font-family:monospace">(defstruct (test2 (:optimize nil)) b a)=
 ;; swap order of slots<br>(foo (make-test2 :a 10))</span></div><div><span =
style=3D"font-family:monospace">--&gt; 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 posit=
ional accessors the generated code like: lispObject.getSlotValue_0 (in the =
disassembly). Without the source transformation, it compiles to a call to t=
est-2-a. If it didn&#39;t then when defstruct was redefined the fixed posit=
ion 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 y=
ou give it a struct that was created *before* defstruct is redefined. But t=
hat&#39;s what I&#39;ve noted in my case 1 above as undefined behavior.<br>=
</div><div><br></div><blockquote class=3D"gmail_quote" style=3D"margin:0px =
0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">
<br>
I don&#39;t claim to claim it &quot;can&#39;t work&quot;. But I have a vagu=
e<br>
understanding why there might be a reason for &quot;this might not work&quo=
t;.<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&#39;t tell how you could possibly know where all<=
br>
the &#39;references&#39; 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&#39;d track that.<br></blockq=
uote><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 an=
notation across all functions will identify all uses of the accessors, whic=
h are *potential* problems. But you would have to sort new from old. Howeve=
r, 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 posi=
tional assessors were inlined. The (:optimize nil) forces that not to happe=
n. <br></div><div><br></div><div>There&#39;s still an issue if you first de=
fined 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 sl=
ots. Not a problem if your redefinition is just adding slots to the end. An=
yways,=C2=A0 Don&#39;t Do That. Otherwise you will get bugs unless I implem=
ent something like what&#39;s described in the previous paragraph.<br></div=
><div><br></div><div>In my use case it&#39;s fine because I&#39;m using the=
 option in the first place.<br></div><div><br></div><blockquote class=3D"gm=
ail_quote" style=3D"margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,=
204,204);padding-left:1ex">
Again, I&#39;m not saying this can&#39;t work. I just find it daunting to e=
ven<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&#39;s a theoretical problem, but it hurts my brain. :D<br>=
</blockquote><div><br></div><div>There&#39;s definitely a screw case, as I =
explained above. It&#39;s a compromise. You get the ability to redefine def=
struct, but if you add :optimize nil in the redefinition without it being i=
n the original, then you might have stale compiled code with fixed position=
 accessors. I&#39;m thinking that&#39;s a lot better than the current situa=
tion, where you can&#39;t redefine defstruct at all.=C2=A0 I&#39;m also onl=
y planning on doing this while developing the code.</div><div><br></div><di=
v>I&#39;m leaning towards using the defstruct :optimize option vs the (decl=
are (optimize (debug 3))) , since the latter hurts performance of everythin=
g. The defstruct option is more targeted.<br></div><div><br></div><div>Alan=
<br></div></div></div>
</blockquote></div></div>

--00000000000034e6ac05e3bbc121--