CLEANUP: Exception code review #2

[dtonhofer]

This is another review of the exception generating code.

Regarding the previous "simple exception text lookup via meaningless keywords" (e.g. "ea01"), nobody was really happy with the keyword solution.

In this proposal, the keywords is replaced by a (possibly compound) term, which, while still abstract, can carry parameters and also indicate what the exception is actually about, for example: "param_not_assignable(Params)". This term plus a "location term" (which is quite similar to a predicate indicator) are used to look up exception information, which includes the actual formal term that shall be thrown. In this way, the formal term doesn't even appear in the actual code.

Thus one goes from code like this:

throw_type_error(Pred,instantiable,X,ea03)

to more compact code and interpretable and compiler-verifiable code like this:

throwme(jpl_new,x_not_instantiable(X))

which has all parametrizations relegated to a dedicated clause:

exc_desc(jpl_new,x_not_instantiable(X),
         jpl_new/3,
         type_error(instantiable,X),
         '1st arg must be a classname, descriptor or object type').

Simples!

How to test whether all the calls are good? Here is how:

I have added a subdirectory exception_tests (don't really know where to put it) with a Perl script which extracts throwme/2 calls from jpl.pl (should have been done in Prolog, but I'm not ready). The resulting printout can then be copy-pasted to the end of exception_testcode.pl which contains pluinit code which attempts to call each of those throwme/2 calls and complains if no "lookup clause" was found, multiple "lookup clauses" were found or the exception doesn't look like an ISO-standard exception at all.

[dtonhofer]

No comments yet?

[JanWielemaker]

I like to leave this to @ssardina and @anionic. I think it is a little cleaner. I do not like Perl files (and surely not called .pl Perl files) in the system as it harms simple Prolog code scans over the code. Well, it was something used temporary so it can be left out (I think). I'm typically not a fan of large source changes that makes things only a bit cleaner. They make it harder to track which change broke what (using git blame)

[dtonhofer]

Yes, the Perl code just exists to extract the throwme/2 calls to put them into plunit-adorned code to see whether all the calls have correct syntax (the only way I can think of to exercise all of these calls, even if only in an artifical setting). The source change may look big, but the good thing is it's very uniform - always the same stuff (a long exercise in keeping attention up, really; I was thinking about coding a rewriter, but that would probably have taken longer). Expected to be problemless.

[anionic]

I am satisfied, and happy, that this leaves jpl.pl cleaner, and keen to encourage @dtonhofer's involvement in JPL maintenance and development ;-) I would not discard the source-scanning plunit stuff, but can it not be reimplemented entirely in (SWI-)Prolog as an on-the-fly plunit test? perhaps later? using clause/2 et al. rather than scanning source text? I am a fan of any source changes which make things a bit cleaner, and despise Git for discouraging tidying, canonisation of style and coding idioms, improved naming,.. I am comfortable that this change preserves the correctness of jpl.pl, even if git can't see that ;-) I vote to commit, and look forward (from the sidelines) to further non-breaking refinements (looking at you jpl.c).

[dtonhofer]

Thanks @anionic - that's some serious endorsement. Yes, the Perl code can certainly be moved to Prolog, it would be interesting to have this "extract code from jpl.pl, crreate a plunit file, run plunit file" run from inside a plunit file. We might have to write some infrastructure predicates. That's going to take a bit.

[dtonhofer]

Hi guys. Is this going to be included or are you waiting for me to make amendements. Throw me a bone here. (I will be looking into changing the DCG in any case, need to get that off my disk otherwise it will be there forever .. expecting complaints for THAT one)

[JanWielemaker]

Been mostly on holidays ... As we seem to agree the new exception code is better than the old I have included the changes to jpl.pl. At least for now I've excluded the exception_tests directory. As far as I can see it the perl extractor was a one-time-use only script, so there is no use keeping it. I'm less sure about the exception_testcode.pl, but as is it won't fly and I have the impression it has little use after this conversion as well as it seems to test the fairly simply exception term manipulation in your exception layer. That code mostly proofs itself :) If you disagree on this, please merge the relevant parts into test_jpl.pl. Setting up test files for JPL is a portability nightmare, so I'd rather extend the already working test_jpl.pl than starting something new. In general, a test file shall conform to the rules below, but for JPL it is even harder ...

• Be called test_XYZ.pl and reside in the top directory of the package
• Define a module test_XYZ that exports test_XYZ/0 which shall succeed iff the test passes (it typically calls run_tests/1 specifying the units defined in the file, but can do anything else).

The conflict is due to the partial and squashed merge.

[dtonhofer]

Thank you Jan. Yes, you are right, exception_testcode.pl has no intrinsic value and should be generated by a script, preferable Prolog rather than Perl. But it's actually something that is of interest only when developing (in a sense, it ensures 100% code coverage of the throwme/2 calls, which is good as these are not likely to be exercised all that often, even in test code)

A side question: the test files are modularized - is this supposed to be applied widely or is that an approach of the SWI Prolog distribution? Up to now, I never put test code explicitly into module files as the plunit block is transformed into its own module. Additionally, the file ending is .pl, shouldn't it be .plt? (That brings its own problems as github thinks .plt is gnuplot code...)

P.S:

The latest version of the exception-throwing boilerplate code with comments is here, I got rid of a clause...

throwme_nonmodular.pl