[asdf-devel] patch system-source-file

[Robert Goldman]

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