[asdf-devel] patch system-source-file

Tue Jul 14 16:56:29 UTC 2009

james anderson wrote:
> hello;
> 
> i am not sure, exactly to which message to reply - i suspect i'm  
> missing parts of the thread.
> in any case,
> 
> 1. when dealing with a large system, latent unbound slots are  
> insidious. an initialization protocol should either provide a default  
> value which  is within the class contract, or it should signal an  
> exception - most likely an error. that is, in the minimal form,  
> either the slot should be defined as
> 
>   (slot :initarg :slot :initform (error "SLOT required."))
> 
> or initialize instance should be specialized to effect
> 
>   (defmethod initialize-instance (instance &key (slot (error "SLOT  
> required."))) (call-next-method))
> 
> in which latter case there can be logic for the use case where a  
> presence constraint applies to some combination(s) of a set of slots.
> 
> absent such guards, the slot-unbound error will eventually occur at  
> some point much later in the computation, far from the initialization  
> time and site, in a manner which complicates the problem and obscures  
> its resolution.

This is a good description of the general problem, but it overstates the
problem here.  Nobody should be setting these slots after they are set
(or not) in the defsystem form, so a slot-unbound error will not be
difficult to resolve.  However, it /might/ create a cumbersome nuisance
for code that wishes to process asdf components.

Unfortunately, these slots are not required and are often left unset.

I think our debate boils down to figuring out what it means to "provide
a default value which is within the class contract" in a way that won't
make too much of a nuisance for asdf component object consumers.

> 
> 2. the slot type should be extended to (or NULL string) only if there  
> is some form of slot-group interaction - as noted above, or if the  
> value NIL is really acceptable to allow operators to produce correct  
> results. the same for "".
> 
> 3. the relation between extensions and &allow-other-keys does not  
> come through in the messages i've been able to find for the thread.  
> is it described succinctly somewhere?

We need to provide &allow-other-keys because people could, at any time,
add new subclasses of system.  OTOH, this isn't such a big problem
because at the end of the day the keyword arguments get passed into
make-instance, so will cause an error.  So this was a red herring (which
I introduced and for which I apologize).

> 
> 4. if one would like to see the code at the version which constitutes  
> the current approach to this issue, which revision should one pull?

I don't have a good answer for this, sorry.

I think that James clearly states the issues.  I propose we just try to
come to consensus on something like the following:

1.  We specify which of the system string initargs are actually optional.

then either

2a.  We specify these as being of type string and specify that they take
"" as a default.  Supplying NIL would be a type error.  For the benefit
of non-type-checking lisps, we could add :after methods on
initialize-instance to reject non-string values.

2b.  We specify that the type is (or null string) because we want to be
able to distinguish unsupplied from explicitly empty.

These both seem reasonable alternatives, as long as we state one of them
clearly.

If someone will express a clear preference that isn't shouted down, I'd
be happy to provide help either documenting or coding up support.

best,
r

> 
> thanks,
> 
> On 2009-07-14, at 17:21 , Robert Goldman wrote:
> 
>> Greg Pfeil wrote:
>>> On 12 Jul 2009, at 11:17, Robert Goldman wrote:
>>>
>>>> Greg Pfeil wrote:
>>>>> On 9 Jul 2009, at 13:08, Robert Goldman wrote:
>>>>>
>>>>>> Greg Pfeil wrote:
>>>>>>> Here are a couple changes to ASDF that I made in the process of
>>>>>>> creating
>>>>>>> an ASDF browser for CCL's IDE:
>>>>>>>
>>>>>>>  system-source-file now works for systems without their own .asd
>>>>>>>  optional parts of systems (version, maintainer, etc.) don't  
>>>>>>> leave
>>>>>>> their slots unbound
>>>>>> The first of these looks good, but I'm less fond of the second  
>>>>>> change.
>>>>>> I've been bitten repeatedly by bugs caused by initforms that  
>>>>>> caused
>>>>>> slots not explicitly set to have some value that hides a mistaken
>>>>>> failure to fill the slot.
>>>>> Yeah, I should have sent these as separate patches. The first  
>>>>> one is
>>>>> more important, IMO, and the second is definitely debatable.
>>>> How about sending that first patch while we work through the second
>>>> issue.  Even if we add the initforms, seems like it's still worth
>>>> discussing the tradeoff b/w "" and NIL as defaults.  I'd be  
>>>> inclined to
>>>> prefer the latter, even at the expense of :type (or null string), so
>>>> that an unsupplied value is readily distinguishable from an empty  
>>>> value.
>>> Here's the system-source-file patch.
>>>
>>>
>>>
>>>> The argument for slot-unbound is that it makes an error when you  
>>>> think
>>>> you have set a slot, but you haven't.  For example, let's say I  
>>>> misspell
>>>> an initarg so that the value quietly vanishes.  Then I'd rather the
>>>> system hurl an error instead of quietly going off and doing  
>>>> something I
>>>> don't expect.
>>>>>> I'd rather have us handle slot-unbound on those optional parts  
>>>>>> of the
>>>>>> system instead of stuffing a bunch of NILs in there.
>>>>> When you say "us", do you mean implementing the accessors  
>>>>> explicitly in
>>>>> ASDF to handle the condition, or having the caller handle/avoid the
>>>>> condition?
>>>> The latter.
>>>>>> If one is expecting strings here one must still handle checking  
>>>>>> for
>>>>>> NIL,
>>>>>> so having to check for slot-boundp doesn't seem that much more  
>>>>>> onerous.
>>>>> Checking slot-boundp has a number of problems:
>>>>>
>>>>>  you have to export the slot names
>>>> Is this really an issue?  Would someone outside the ASDF package  
>>>> really
>>>> be looking into these things?  I suppose possibly so...
>>>>
>>>>>  it breaks the accessor abstraction because the slot names  
>>>>> don't match
>>>>> the accessor names
>>>>>    (when (slot-boundp foo 'asdf::description) (system- 
>>>>> description foo))
>>>> (handler-case ....
>>>>  (slot-unbound () ...)
>>>>
>>>> avoids this problem, if you know that you want to quash unbound  
>>>> slots.
>>> You know, I considered this, but quickly dismissed it as too verbose.
>>> However, while
>>>
>>>     (handler-case (system-description foo) (slot-unbound () ""))
>>>
>>> is worse than the
>>>
>>>     (or (system-description foo) "")
>>>
>>> that I wanted, it's still better than the
>>>
>>>     (when (slot-boundp foo 'asdf::description) (system-description  
>>> foo))
>>>
>>> I was afraid of. I'm happy to use that pattern.
>>>
>>> I'd still prefer optional values to not signal slot-unbound, but I
>>> understand the tradeoff with catching errors in initforms, etc.
>> I take your point.  My earlier suggestion was to offer, e.g.,
>>
>> (component-version component &optional error-p)
>>
>> However, it occurs to me that this would foil uses of WITH- 
>> ACCESSORS, so
>> is quite likely undesirable.  :-(
>>
>> I am coming around to a modified version of your PoV:  have default
>> values, let them be NIL, and add :type (or null string) to the  
>> relevant
>> slots.  Then NIL is a quasi-exception, and readily distinguishable  
>> from
>> "", which would be reserved for an explicitly-supplied empty value as
>> opposed to an implicitly-supplied default value.
>>
>> Ideally, we could catch bad initializations at initialization time,  
>> but
>> that objective is foiled by the quite important objective of
>> extensibility, which seems to force &allow-other-keys upon us.
>>
>> [BTW, Greg, I get a signature verification failure on that email.  Not
>> sure why.]
>>
>>
>>
>> Best,
>> r
>>
>>
>>
>> _______________________________________________
>> asdf-devel mailing list
>> asdf-devel at common-lisp.net
>> http://common-lisp.net/cgi-bin/mailman/listinfo/asdf-devel
> 
> 
> _______________________________________________
> asdf-devel mailing list
> asdf-devel at common-lisp.net
> http://common-lisp.net/cgi-bin/mailman/listinfo/asdf-devel