This is the mail archive of the kawa@sourceware.org mailing list for the Kawa project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: GSOC | Extending Common Lisp support


On 23 May 2012 23:36, Jamison Hope <jrh@theptrgroup.com> wrote:
> 1. I presume that this change in PropertyLocation.java
>>
>> Â Â Â Â- Â Â Â if (symbol instanceof Symbol)
>> Â Â Â Â+ Â Â Â if (symbol instanceof Symbol && lloc != null)
>
> Â is intended to avoid a possible NullPointerException? But after that
> Â if block there's an unguarded "lloc.set(plist);" anyway, so you'll
> Â still get the NPE, just at a different line number.

I vaguely remember doing this, must have come across the bug a while
ago when I was torturing the compiler. Sorry about that. There's a
load of stuff in my svn diff that can only be described as
experimental tinkering that I didn't intend to send in my rushed
patch. I tried to delete most of this, but clearly that one leaked
through :-)

> 2. In moving the bulk of checkDefaultBinding() from SchemeCompilation.java
> Â to Translator.java, the XmlNamespace/unitNamespace checks have been
> Â effectively reordered to be after the "ends with '?'" check. Does that
> Â have any functional repercussions? (I suspect not, but haven't tested
> either.)

I was conscious of this at the time. That was one of the reasons I
asked whether relying on the test suite was OK for refactorings. I had
planned to test these things when I got time to do so, but as you
mentioned, it's quite a rabbit hole, and whilst I was "in the zone", I
just tried to refactor as much as possible without horrifically
breaking everything and then go back over it with the tooth comb.

> 3. Also in the checkDefaultBinding() relocation, Translator.java now has:
>>
>> Â Â Â Âint len = name.length();
>> Â Â Â Âchar ch0 = name.charAt(0);
>
> Â where the SchemeCompilation version used to have an "if (length == 0)"
> Â check in between. Could that be a problem? Is name ever the empty string?

I probably intended to keep that check. I don't think you can trigger
that error from the language (I'm probably wrong about that), but it
certainly should be there as a consistency check I think.

> 4. Scheme.getNamedType() directly calls LispLanguage.getLispTypeMap().get(),
> Â but it looks like it should be calling LispLanguage.getNamedLispType()
> Â instead, as that's where the name.startsWith("clisp:") stuff is now.
> Â Also, evaluating |clisp:boolean| in Scheme doesn't work anymore, even if
> Â LispLanguage.getLispTypeMap().get is replaced with
> Â LispLanguage.getNamedLispType. It looks like this is because
> Â LispLanguage.getNamedLispType("boolean") returns null.

I noticed that one when compiling the distribution. I figured I'd done
the refactoring wrong at the time, or that I need to add some extra
machinery. This is because the common lisp boolean type is now visible
only to CommonLisp (it's initialised in CommonLisp#getTypeFor) and the
type name "boolean" is only checked properly when we're in that
language. I'm still not sure what's the best way to fix this problem,
but I didn't have much time to spend on it.

> Re |clisp:boolean|, I got it to work again by adding
>>
>> Â Â Â Âtypes.put ("boolean", Type.booleanType);
>
>
> to the getLispTypeMap() initialization, so that then
>>
>> Â Â Â ÂClass clas =
>> getNamedLispType(name.substring(colon+1)).getReflectClass();
>
>
> succeeds and passes Boolean.TYPE to CommonLisp's getTypeFor(Class), which
> then
> returns CommonLisp's booleanType.

So I've repeated what you already know above then :-). I hadn't
thought of this solution. It doesn't seem too hackish to me, isn't
this one of the uses of reflection when we're in such a situation?
(I'm not very experienced with reflection, so please elaborate if I'm
missing the point).

> That's because CommonLisp doesn't override Lisp2's getNamedType(), which
> just
> defers to LispLanguage.getNamedLispType(). The only way to get the correct
> Type
> is to (somehow) first get the corresponding Class and pass it to
> getTypeFor(Class).
> In particular, note that CommonLisp.getTypeFor(String) will not do the right
> thing.
> I think this is related to the "CL not is not not" bug I came across a few
> weeks ago.
>
> This is turning out to be quite the rabbit hole.

Unfortunately exams interrupted me when I'd loaded my interpretation
of all this stuff ties together into my head. So I'll have to
reinitialise after exams to finish it.
>
> It looks like you're at least far enough along that class literals work,
> and even if you still have to call it |clisp:boolean| I think you should
> be able to finish translating PrimOps.scm now (if you incorporate my above
> hack).

PrimOps (which I renamed primitives.lisp, oops just realised I didn't
add this to my working copy) is finished. Other than a few return
types I may have omitted :-D

> Clearly there's a sizable mess here with getTypeFor(String) vs
> getTypeFor(Class)
> vs getNamedType(String) vs string2Type(String), and that's tangential to
> your
> actual proposed summer project.
>
> Per, what if you and I (yes, I am aware that likely simplifies to "I") take
> over
> that part of it so that Charlie can focus on DECLARE, sequences, and LOOP?
> (Will 1.12 be coming soon so we can check some of this stuff in?)

I'm not complaining that someone wants to take this off me, but I hope
I'm not falling behind too soon. My exams finish on Saturday, so I
will be able to focus my full attention on the problem. I suppose with
this kind of thing, were I to continue focusing on it, I should stay
in closer contact with one of the mentors so that it's easier to
follow what I've been doing and so that I can be told when a
particular approach isn't a good design decision.

Thanks for your time,
Charlie.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]