SWI-Prolog / packages-jpl

JPL: The Prolog <-> Java interface
BSD 2-Clause "Simplified" License
54 stars 33 forks source link

Added dictionary support #61

Closed ssardina closed 4 years ago

ssardina commented 4 years ago

Extended JPL to support dictionaries

Having dictionaries is easy via the Java Map which is quite useful and commonly used in Java.

The reason was that in the newest versions of SWIPL, calls with current_prolog_flag/2 will crash because one of the options now yields a dictionary:

?- current_prolog_flag(abi_version, Y).
Y = abi{built_in:2033063160, foreign_interface:2, qlf:67, qlf_min_load:67, record:3, vmi:3726870942}.

Fixes #54.

ssardina commented 4 years ago

@JanWielemaker can you check this one? This is analogous to Rationals. I added Dictionaries as another type of data. It is easy because it maps very well to the Map in Java. Did not have to change anything on the C code, I re-used existing APIs there.

Having Dict is important because otherwise goals like current_prolog_flag(abi_version, Y). break, as Y is bound to a dictionary.

If you can check and merge this it would be great so I can continue with the PR for Maven-compatible repo. It is all working beautiful and will will make the whole repo much nicer and modern for further devel.

JanWielemaker commented 4 years ago

Thanks. That are some big steps in the right direction.

ssardina commented 4 years ago

Thanks Jan for merging and the support. :-)

I will now issue the next PR with the Maven-like repo config.

JanWielemaker commented 4 years ago

@ssardina Didn't check this is caused by this PR, but trying a release compilation I see

./org/jpl7/Dict.java:169: error: lambda expressions are not supported in -source 7
                .map(key -> key + ":" + map.get(key))
                         ^
  (use -source 8 or higher to enable lambda expressions)
./org/jpl7/Dict.java:169: error: lambda expression not expected here
                .map(key -> key + ":" + map.get(key))
                     ^
Note: org/jpl7/fli/atom_t.java uses or overrides a deprecated API.

I guess there is a simple rewrite. Moving to -source 8 is also an option, but that certainly blocks this JPL for 8.2.1. Can you create a PR?

ssardina commented 4 years ago

Ohh indeed, I used lambda expressions which are not available below 1.8.

Eventually I would like to move to -source 8, as below that is really old (and even now 1.8 is becoming "oldish"). We are even all compiling with v11 right?

I will re-write that to not use lambda and comment the lambda code; when could I get the green light to force users to use 1.8+?

Sorry, and thanks for the patience, I am not fully aware unfortunately of the "protocol"

JanWielemaker commented 4 years ago

No worries, this sort of things happens to me all the time. Lots of people use ancient stuff (for stability, don't fix what ain't broken, etc.)

when could I get the green light to force users to use 1.8+?

That probably requires some checking on Discourse whether people get upset dropping some museum piece. We can not drop such things in the stable releases though. I'd like to go a bit faster, but as long as the costs do not get prohibitive I try to keep supporting old platforms.

ssardina commented 4 years ago

Done in PR #74, please check and merge. The code is now -source 1.7.

However, I would prefer we move to 1.8 at least soon so we can use lambdas at least.

I understand we need to respect some museum pieces, so yes we can check on Discourse to see if there is anyone there who would get upset. Not sure how you do that though...