guicho271828 / trivia

Pattern Matcher Compatible with Optima
Other
332 stars 22 forks source link

Bug with property #89

Closed bon closed 6 years ago

bon commented 6 years ago

I expect this to return 88 but it returns NIL.

(match '(:y 88)
  ((property :x x) x)
  ((property :y y) y))
guicho271828 commented 6 years ago

I will check this when I got home.

guicho271828 commented 6 years ago

Ok, now I figured out the problem. The cause of this unexpected behavior is conceptual rather than petty mistakes.

What is the right semantics for property pattern? Should the :x property of (:y 88) be NIL and matched, or should it get errors and fail (i.e. next clause)?

Property list, both plist or symbol-plist respectively accessed by getf and get, assumes the default result of getting an undefined/unregistered property is NIL. I would like to follow this convention.

Therefore, in order to achieve the desired behavior, you should instead write

(match '(:y 88)
  ((property :x (and x (not nil))) x)
  ((property :y y) y))
; => 88
bon commented 6 years ago

Thanks for the quick and clear reply.

It might be a matter of taste but I don't thinks that's the most elegant solution here. Could I suggest an alternative solution?

Note that the Hyperspec says of both get and getf that there is no way for them to distinguish an absent property from one whose value is default.

Also bear in mind that the semantics of match is to try each subpattern in turn until it finds one that matches.

So in this case the first subpattern matches and returns NIL

(match '(:y 88 :x nil)
  ((property :x x) x)
  ((property :y y) y))
;; => NIL

While in this case the first subpattern fails to match so the second subpattern matches

(match '(:y 88)
  ((property :x) x)
  ((property :y y) y))
;; => 88

This has the benefit of not needing the ugly (IMO) (and x (not nil))) expression.

guicho271828 commented 6 years ago

I agree that the workaround is ugly. I felt the same. Well, let me review the full docstring of property pattern... (you can see it from the slime C-c)

Lambda-List: (KEY SUBPATTERN &OPTIONAL (DEFAULT NIL) (FOUNDP NIL FOUNDP-SUPPLIEDP))
  It matches when the object X is a list, and then further matches the contents
  returned by (getf KEY X DEFAULT) against SUBPATTERN.
  FOUNDP is bound to T in order to indicate the reason that NIL is matched.
  It is implementation-dependent whether it matches against a list of odd number of elements or it signals an error.
  Also, the result may be affected by the safety setting of the optimization option.

I almost forgot about the last optional argument. From the docstring the purpose is clear. So you can also implement it as:

(match '(:y 88 :x nil)
  ((property :x x nil t) x)  ; constant t matches the constant t.
  ((property :y y) y))
;; -> nil

(match '(:y 88)
  ((property :x x nil t) x)    ; t does not match nil, thus fail
  ((property :y y) y))
;; -> 88

Another alternative way is this: Since I hesitate to deviate from the concepts in CLHS, I would like to keep the original semantics for property. However, I am eager to add a new version of property, maybe property* or property! (! indicating a stricter version, well, this is not conventional... usually ! is for destructive operations in scheme), that fails when the property was nil.

PuercoPop commented 6 years ago

FWIW, The property* seems a good middle-ground.

guicho271828 commented 6 years ago

I eventually chose property!, since in this library I use the convention that * is for "soft-match variants" of the original. property is, in a sense, soft by default. The chance that the use of ! confuses the users would be low since a destructive operation during pattern matching does not make sense.

bon commented 6 years ago

Good catch on the foundp option to property, @guicho271828.

property! is a consistent and elegant solution.

guicho271828 commented 6 years ago

Now this gets merged, it means I added a case for ! convention and I can rename and add the idea in #84 as ematch!.