Why can't defstructs be redefined?
alanruttenberg at gmail.com
Thu Jul 14 03:46:36 UTC 2022
Here's the patch for (:optimize nil) option.
Thanks for the comments, keep them coming.
On Wed, Jul 13, 2022 at 10:33 PM Alan Ruttenberg <alanruttenberg at gmail.com>
> Comments inline
> On Wed, Jul 13, 2022 at 9:40 PM Ville Voutilainen <
> ville.voutilainen at gmail.com> wrote:
>> On Thu, 14 Jul 2022 at 00:50, Alan Ruttenberg <alanruttenberg at gmail.com>
>> > This is what I came up with:
>> > 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".
>> 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.
-------------- next part --------------
An HTML attachment was scrubbed...
More information about the armedbear-devel