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

[Per Bothner]

On 09/22/2012 06:15 AM, Charles Turner wrote:
> On 22 September 2012 01:24, Per Bothner <address@hidden> wrote:
> > My biggest concern is the various uses of Scheme.getInstance().xxx()
> > It seem like these should be plain xxx() or (in the case of the
> > subclasses of Compilation) getLanguage.xxx().  Any reason why not?
>
> I don't think so. I've updated XQuery and ELisp to remove their
> dependence on Scheme, only these files now call
> Scheme.getInstance().xxx():
>
> gnu/xquery/lang/XQResolveNames.java:350:  type =
> Scheme.getInstance().string2Type(sym.getName());
>
> I'd like for this not to be the case, but I get testsuite failures if
> I instead call the method from Language.getCurrent() (or
> XQuery.getInstance(), which should be equivalent in this case).

One reason I think it would be better to avoid static methods is
that there may multiple instances of a given language, corresponding
to different variants/dialects.  Thu it is better to get the specific
current language.  Mostly Language.getCurrent() should work, but for
example in XQResolveNames we could use parser.interpreter.  Not only
is this faster, but it also is guaranteed to the get the same Language
instance as currently being used.

> There
> seems to be a dependence on Scheme's boolean type, I don't know enough
> about XQuery to decide if that's necessary, or whether we can give
> XQuery its own boolean type (or if this is even the problem!).
> XDataType seems to just use Type.booleanType.

If you have an updated patch with the other changes I'll take a look
at you XQResolveNames doesn't work.

> gnu/kawa/servlet/ReplSession.java:112:
> this(kawa.standard.Scheme.getInstance());
>
> Seems benign.

Agree.  That just picks a default language.

> > It might also be help to add some comments - for example what is
> > the difference between getNamedType and string2Type.
>
> At first I thought I could manually inline getNamedType into
> string2Type, and be rid of getNamedType, but that turned out not to be
> true, and I'm struggling to see why.
>
> In SchemeCompilation,:291, if the line is this:
>
> Type type = getLanguage().getNamedType(cname);
>
> All is OK, if it's this:
>
> Type type = getLanguage().string2Type(cname);
>
> I get this when doing a fresh make:
> (compiling srfi69.scm)
> srfi69.scm:29:25: 'hash-table?' exported but never defined
> srfi69.scm:31:40: 'hash-table-set!' exported but never defined
> srfi69.scm:32:2: 'hash-table-delete!' exported but never defined
> srfi69.scm:32:21: 'hash-table-exists?' exported but never defined
> srfi69.scm:34:2: 'hash-table-size' exported but never defined
>
> Just going off the names (hash-table? etc), I can't see how
> string2Type differs in its execution path to what getNamedType would
> do...

Certainly we don't need to merge string2Type and getNamedType
at this time - though I would like to document better the difference.

Anyway, when you have an updated patch I'll check it out.
--
        --Per Bothner
address@hidden   http://per.bothner.com/