search

conjure-cp / conjure

Conjure: The Automated Constraint Modelling Tool
Other
94 stars 20 forks source link

spurious comma ignored instead of flagged as error #602

Closed ott2 closed 9 months ago

ott2 commented 9 months ago

The spec

find f : int(0..10)
such that f = sum i,: int(1..3) . i

generates the answer 6. I was expecting an error or a warning. It's almost as if the parser treats this like the spec below, for which the answer really should be 6:

find f : int(0..10)
such that f = sum i : int(1..3) . i

This is with Repository version 0955541f3 (2023-08-24 22:02:18 +0100), now building current head to check if this issue is still present.

ozgurakgun commented 9 months ago

I think @Maurice-Byrne's parser changed all comma separated lists to have a final optional comma these days.

Maurice-Byrne commented 9 months ago

That's right, I can't remember the exact reasoning for it. I think it improves error tolerance at the syntax level but I don't think there'd be any issues making it semantically invalid/warning.

ozgurakgun commented 9 months ago

ha! I wasn't expecting a reply and definitely not one this quickly! not sure about this, I quite like being able to do things like:

[ a,
  b, 
  c, 
]

especially if a,b,c are longer and/or programmatically generated.

I guess it looks bizarre when it's just a single item? Or when it's a variable declaration like this? We could fix/improve/refine but we would have to decide what we want to happen. @ott2, what would you want to happen?

ozgurakgun commented 9 months ago

Closing, feel free to reopen @ott2

ott2 commented 9 months ago

Any resolution is fine, but this needs to be documented.

pwn1 commented 9 months ago

sum i,: int(1..3) . i has no sensible interpretation other than its actual meaning. Same for lists with a trailing comma.