Re: [Info-gnuprologjava] gnuprologjava patch: fixes for is_list

[Michiel Hendriks]

Hi all,

I don't recall which behavior I had in mind when I originally wrote that
predicate, but I'm tempted to go with the current SWI-Prolog
behavior. I'm sure the devs and users of SWI-Prolog had plenty
discussion on that subject.

Also, the patch resembles more my original implementation, as originally
it didn't try to unify the argument with the empty list atom.

Small note to the patch, use TermConstants.emptyListAtom.equals(lst)
rather than the instance check.


kind regards,
Michiel


On Mon, May 07, 2012 at 09:20:39PM +0100, Daniel Thomas wrote:
> Hmm,
> 
> ISO/IEC DTR 13211–3:2006 defines is_list in its reference implementation
> of DCGs as:
> is_list([]) :-
> !.
> is_list([_| Tail]) :-
> is_list(Tail).
> 
> but SWI-Prolog use:
> is_list(X) :- 
> var(X), !,
>         fail.
> is_list([]).
> is_list([_|T]) :-
>         is_list(T).
> and http://www.swi-prolog.org/pldoc/man?predicate=is_list%2F1 claims
> this is the de-facto standard.
> 
> It seems probable that we should switch to the SWI-Prolog behaviour as
> your patch proposes, the subtle failure is particularly nasty. However I
> would like to double check that with Michiel Hendriks (hopefully CCed)
> who wrote Predicate_is_list.
> 
> Daniel
> 
> On Mon, 2012-05-07 at 20:30 +0100, Paul Singleton wrote: 
> > On 07/05/2012 17:21, Daniel Thomas wrote:
> > > Again please can I have a test case to show what this fixes and prevent
> > > the same problem happening again. test/inriasuite/extra/list/is_list
> > > contains the current tests.
> > 
> > These inria tests are broken (is_list/1 should never instantiate further):
> > 
> > [is_list(X),[[X <-- []]]].
> > [is_list([a,b|X]),[[X <-- []]]].
> > [is_list([1,b|X]),[[X <-- []]]].
> > [is_list([3,2|X]),[[X <-- []]]].
> > [is_list([3.2,0.1|X]),[[X <-- []]]].
> > [is_list([k,82.3|X]),[[X <-- []]]].
> > [is_list([a,b,d,y,e,foo(a),2,4,6,dt,9.2|X]),[[X <-- []]]].
> > 
> > all should yield failure, i.e.
> > 
> > [is_list(X),failure].
> > [is_list([a,b|X]),failure].
> > [is_list([1,b|X]),failure].
> > [is_list([3,2|X]),failure].
> > [is_list([3.2,0.1|X]),failure].
> > [is_list([k,82.3|X]),failure].
> > [is_list([a,b,d,y,e,foo(a),2,4,6,dt,9.2|X]),failure].
> > 
> > Fix them and they act as test cases to show up the first bug below.
> > 
> > NB inria maybe thinks is_list/1 should succeed for var-terminated or 
> > proper lists, as it also has is_proper_list/1, but ISO is_list/1 
> > demands a proper list, and the tests above are broken anyway.
> > 
> > The second bug is more subtle:
> > 
> > [(Q=foo,is_list([1|Q])),failure]
> > 
> > but unpatched is_list destructively rebinds Q to [] and succeeds!
> > 
> > > On Fri, 2012-03-23 at 12:08 +0000, Paul Singleton wrote:
> > >> fix bugs in vm.buildins.list.Predicate_is_list (unifies var with empty
> > >> list, fails to dereference tail of list) and remove redundant arity
> > >> check after checking ./2 tag
> > >>
> > >> ----
> > >>
> > >> diff --git a/src/gnu/prolog/vm/buildins/list/Predicate_is_list.java
> > >> b/src/gnu/prolog/vm/buildins/list/Predicate_is_list.java
> > >> index 5ea3ea1..0ab4dbf 100644
> > >> --- a/src/gnu/prolog/vm/buildins/list/Predicate_is_list.java
> > >> +++ b/src/gnu/prolog/vm/buildins/list/Predicate_is_list.java
> > >> @@ -19,6 +19,7 @@
> > >>     */
> > >>    package gnu.prolog.vm.buildins.list;
> > >>
> > >> +import gnu.prolog.term.AtomTerm;
> > >>    import gnu.prolog.term.CompoundTerm;
> > >>    import gnu.prolog.term.Term;
> > >>    import gnu.prolog.vm.ExecuteOnlyCode;
> > >> @@ -41,7 +42,7 @@
> > >>                  Term lst = args[0];
> > >>                  while (lst != null)
> > >>                  {
> > >> -                        if 
> > >> (interpreter.unify(TermConstants.emptyListAtom, lst) ==
> > >> RC.SUCCESS_LAST)
> > >> +                        if (lst instanceof AtomTerm&&  ((AtomTerm) lst) 
> > >> ==
> > >> TermConstants.emptyListAtom)
> > >>                          {
> > >>                                  return RC.SUCCESS_LAST;
> > >>                          }
> > >> @@ -50,11 +51,7 @@
> > >>                                  return RC.FAIL;
> > >>                          }
> > >>                          CompoundTerm ct = (CompoundTerm) lst;
> > >> -                        if (ct.args.length != 2)
> > >> -                        {
> > >> -                                return RC.FAIL;
> > >> -                        }
> > >> -                        lst = ct.args[1];
> > >> +                        lst = ct.args[1].dereference();
> > >>                  }
> > >>                  return RC.FAIL;
> > >>          }
> > >
> > 
> > 
> 
> 



-- 
Michiel "elmuerte" Hendriks             address@hidden
                                              http://elmuerte.com