Skip to content
Snippets Groups Projects

congruent-lambda-p: be more permissive with the congruency test

Merged Daniel Kochmański requested to merge improve-congruency into develop

See the commit message for the description. This MR is a proposal, I accept the possibility, that it will be rejected as a change which breaks the conformance.

Merge request reports

Loading
Loading

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • added 1 commit

    • 15622cfb - tests: add a test for the change in congruent-lambda-p

    Compare with previous version

  • added 2 commits

    • bd3b5ded - compiler: rewrite clos::need-to-make-load-form-p
    • 0cf2e247 - defmethod: do not evaluate the method at the compilation time

    Compare with previous version

  • After reading the spec I think that the code in question is an edge case which was intended to be conformant. The offending case of the newly added test is the second one if I understand you correctly:

    (defgeneric foo (arg1 &key arg2 &allow-other-keys))
    (defmethod  foo ((arg1 integer) &key arg3))

    The generic function has a :arg2 keyword argument, but the method doesn't and neither has it &allow-other-keys nor &rest. So it violates the 4. bullet point of CLHS sec. 7.6.4

    each method must accept all of the keyword names mentioned after &key, either by accepting them explicitly, by specifying &allow-other-keys, or by specifying &rest but not &key

    But this part doesn't talk about what happens when the generic function already has &allow-other-keys, which in this case here. Then the method also accepts :arg2 keyword arguments (if you define "accept" to mean doesn't signal an error). I think that satisfying the requirement

    each method must accept all of the keyword names mentioned after &key

    is the important part, whether it happens due to an &allow-other-keys in the generic function or in the method should not matter.

    So I would prefer stating in the comments that this is an edge case, which we explicitely allow, rather than a violation of the standard.

  • I still read it as [each method must accept all of the keyword names mentioned after &key] -- and here is what we mean by accept -- [either by accepting them explicitly, by specifying &allow-other-keys …], and in this sentence it is that method "specifies" allow-other-keys. At best it is an undefined behavior (and not a standard violation), but this doesn't matter much, I will change wording in the comment.

    I've changed the comment to the following:

                   ;; Testing for a-o-k1 here may not be conformant when
                   ;; the fourth point of 7.6.4 is read literally, but it
                   ;; is more consistent with the generic function calling
                   ;; specification. Also it is compatible with popular
                   ;; implementations like SBCL and CCL. -- jd 2020-04-07
  • added 2 commits

    • 896e7c58 - congruent-lambda-p: be more permissive with the congruency test
    • b64da63e - tests: add a test for the change in congruent-lambda-p

    Compare with previous version

  • (pushed)

  • In the end, it all comes down to what is meant by the term "accepting a keyword". Anyway, the point is not that important, so I'm merging now.

  • mentioned in commit 0e601655

Please register or sign in to reply
Loading