Re: [Kawa-commonlisp-dev] [PATCH] Type refactoring.

[Jamison Hope]

Hi Charles,

On Sep 21, 2012, at 2:49 PM, Charles Turner <address@hidden> wrote:

> Hi all,
> 
> On 20 August 2012 21:10, Charles Turner <address@hidden> wrote:
>> I decided to change my approach a little, and since the patch is
>> getting quite large, I thought it best to pause and get a check up.
> 
> So I took no response to mean it was OK :-)

Yeah, sorry about that. The day job was keeping me pretty busy for
a while with low-level C and robots, which was making it hard to
find time for the mental context switch to Kawa.

But in the meantime I also got a new laptop for work and so I'm
back to Java hacking for a bit as I attempt to get all the usual
software packages installed again. Kawa compiled without issue
under Oracle's JDK 7u7, but some other packages have some hard-coded
paths to the old Apple Java directories or other catching up to do
for Mountain Lion.

> -- I've gone ahead and
> done the rest of the type refactoring, find attached. I couldn't see a
> decent compromise between small overlapping patches and this
> monolithic patch. To much interdependence.

Looks pretty good to me for the most part. A few comments:

1. The only change to kawa/lang/Translator.java is to remove a
   javadoc comment on checkDefaultBinding() which states
   "The default implementation does nothing." But the implementation
   of that method is still <code>{ return null; }</code>, so the
   comment still seems to apply. I recommend taking Translator.java
   out of the diff.

2. It looks like the only changes to Language#getLangTypeFor(Type)
   are indentation? Let's keep that as a separate change, too.
   At some point when Per's got some free time (haha) maybe we can push
   through a few untabify/indent patches to make Kawa source whitespace
   more consistent.

3. I'm not a huge fan of how CommonLisp's booleanType is instantiated
   in both getTypeFor(Class) and getNamedType(String). It seems like
   one of those methods should just defer to the other. Perhaps this
   would work (haven't actually tried it):

@Override
public Type getTypeFor (Class clas)
{
  if (clas.isPrimitive())
  {
    return getNamedType(clas.getName());
  }
  return Type.make(clas);
}

-Jamie

--
Jamison Hope
The PTR Group
www.theptrgroup.com