2

Closed

double backtick escaped identifiers for active patterns compiler bug

description

Is this a compiler bug? Compilation fails with the message "The pattern discriminator 'Fizz Buzz' is not defined"
Removing the single space in the identifier in question allows compilation to complete.
let (|Fizz|_|) i =
  if i % 3 = 0 then Some i
  else None
 
let (|Buzz|_|) i =
  if i % 5 = 0 then Some i
  else None
 
let (|``Fizz Buzz``|_|) i =
  match i with
  | Fizz _ & Buzz _ -> Some "Fizz Buzz"
  | Fizz i -> Some "Fizz"
  | Buzz i -> Some "Buzz"
  | _ -> None
 
for i in [1..100] do
  match i with
  |``Fizz Buzz`` fb -> System.Console.WriteLine fb
  | _               -> System.Console.WriteLine "nothing"
Closed Jun 19, 2014 at 11:19 PM by latkin
Fixed with 9e55115c42e65f1, related change 19368748b246.

Spaces are now allowed in backticked active pattern case identifiers, so the repro code above now works. Various other "special" chars are still disallowed, though. Also fixed the parser to prevent weirdness like my example above.

comments

latkin wrote Jun 6, 2014 at 8:39 PM

Issue is in validation of active pattern name in IsActivePatternName, PrettyNaming.fs ~392. It currently disallows pattern cases with space in them, due to the space character being present in opCharDict.

Need to adjust so that space is allowed, e.g. something like this (which seems to resolve the issue based on initial testing):
    let IsActivePatternName (nm:string) =
        (nm.IndexOf '|' = 0) &&
        nm.Length >= 3 &&
        (nm.LastIndexOf '|' = nm.Length - 1) &&
        (
           let core = nm.Substring(1, nm.Length - 2) 
           // no operator characters except '|' and ' '
           core |> String.forall (fun c -> c = '|' || c = ' ' || not (opCharDict.ContainsKey c)) &&
           // at least one non-operator or space character
           core |> String.exists (fun c -> c = ' ' || not (opCharDict.ContainsKey c))
        )
Though if I interpret the spec correctly (high probability that I'm not :)), this is still more restrictive than it's supposed to be:
Section 3.4
Any sequence of characters that is enclosed in double-backtick marks, excluding newlines, tabs, and double-backtick pairs themselves, is treated as an identifier. Note that when an identifier is used for the name of a types, union type case, module, or namespace, the following characters are not allowed even inside double-backtick marks:
'.', '+', '$', '&', '[', ']', '/', '\', '*', '\"', '``'

opCharDict will prevent usage of a handful more characters than the above exclusion list. Maybe proper fix is to come up with authoritative collection of disallowed characters for active patterns, rather than special-casing them one-by-one.

latkin wrote Jun 6, 2014 at 8:44 PM

Also worth noting that current handling of backtick cases is broken at a deeper level. For example, this nonsense actually compiles and runs:
let (|``Foo|Bar``|) x = 
    match x with
    | 0 -> Foo
    | _ -> Bar

match 42 with
| Foo -> "foo"
| Bar -> "bar"