Fix 457 - add ones complement operator with backwards compat

[dsyme]

This is a tentative fix for #457

Some testing still needs to be added:

• test use of ~~~x for the very rare case of a user-defined type that supports the op_LogicalNot operator when compiling against FSharp.Core 4.3.1.0 (when moving to FSharp.Core 4.4.0.0 and F# 4.0, this code will need to be changed to !!!x)

The F# Language Spec will need to be updated to say !!! is for op_LogicalNot and ~~~ is for op_OnesComplement.

In theory this is a breaking change in some situations - in particular "let (~~~) x = x" now compiles as a function called "op_OnesComplement" instead of "op_LogicalNot". However FSharp.Core remains fully backwards binary compatible because both functions are now present. I think this is ok given the situation.

[latkin]

Yeah seems like op_LogicalNot is pretty rare so I doubt we will break a lot of people. No usages in mscorlib. Full-GitHub search returns only testcode for language-type platforms that I can see (plus our incorrect usage 😄)

Will this play nicely with quotations when used cross-version? e.g. https://code.google.com/p/unquote/issues/detail?id=49&can=1&q=op_LogicalNot

[dsyme]

It should all work across version. So <@ ~~~1 @> compiled with F# 3.1 and evaluated with F# 4.0 (or with an F# 3.x component with a binding redirect to FSharp.Core 4.4.0.0) should work correctly. That testing would be good to add (basically cross-version execution for tests/fsharp/core/queriesLeafExpressionConvert/test.fsx I believe)

[latkin]

Wow, NotQ, LogicalNotQ, and BitwiseNotQ all map to Expression.Not... That guy will pick if he represents logical or bitwise operation based on operand type and availability of the corresponding op_* guys: source. The docs only mention the bitwise operation.

Sheesh, queries are confusing...

[dsyme]

Thanks for checking!

[latkin]

Blah, this indeed has some nasty cross-version problems.

Scenario 1

printfn "%A" <| (~~~ 2)

• Build as exe w/ 4.0 compiler, targeting 3.1
  • Fails at runtime System.NotSupportedException: Specified method is not supported.

Scenario 2

module Test
let q = <@ ~~~ 1 @>

• Builds ok w/ 3.1 compiler targeting 3.1
• Builds ok w/ 4.0 compiler targeting 4.0
• Fails w/ 4.0 compiler targeting 3.1
  • error FS0458: Quotations cannot contain expressions that make member constraint calls, or uses of operators that implicitly resolve to a member constraint call

Scenario 3

module Test

let (!!!) (b : bool) = 
    printfn "in custom bool !!! op"
    b

• Build w/ 3.1, consume w/ 4.0
  • !!! now maps to built-in op_LogicalNot, not to the custom guy
  • To access custom operator, need to use op_BangBangBang
  • Test.( !!! ) ;; fails - The value, constructor, namespace or type '!!!' is not defined
• Build w/ 4.0 targeting 3.1, consume w/ 3.1
  • Custom operator is incorrectly interpreted as ~~~

Scenario 4

module Test

let (~~~) (b : bool) = 
    printfn "in custom bool ~~~ op"
    b

• Build w/ 3.1, consume w/ 4.0
  • Custom operator is incorrectly interpreted as !!!
• Build w/ 4.0 targeting 3.1, consume w/ 3.1
  • Custom operator can only be accessed via op_OnesComplement
  • ~~~ still maps to built-in op_LogicalNot

[dsyme]

Urgh. Good work. Tricky problems.

I'll take a look tomorrow. It's possible most of the "4.0 targeting 3.1" problems go away if we vary the name mapping used based on a 4.3.x.0 v. 4.4.0.0 library target.

[PatrickMcDonald]

Is the naming of this correct? I always thought bitwise and logical were mutually exclusive.

[dsyme]

This is VNext at this stage (hopefully first OOB, if we can find a way to make it fully backwards compat, otherwise next major language revision I suppose)

[latkin]

This has been idle for a couple months and not yet clear how to fix this without a surface area change and still maintain cross-version compat. Thus closing out for now.

[dsyme]

@latkin I'm not sure "idle for a couple of months" is justification when the repo has effectively been closed for business for bug fixing for at least a couple of months :)

Lets just prompt the owners to reopen the PRs against the appropriate branch?

[latkin]

@dsyme I'm only closing idle PRs which have unresolved feedback and/or known issues, not those which are thought to be fully ready but "waiting in the queue."

[dsyme]

My fear for this particular one is that if it's closed - apart from bug #457 - then I'll just keep punting it. "Out of sight out of mind, I tried once and it didn't stick so I gave up" etc.

We need to have a plan in place to fix this in the language design. I suppose tracking it on fslang.uservoice.com would make sense since it's effectively baked in as part of the 4.0 design at the moment.

[dsyme]

Given the failure of the above approach I suppose a way forward would be

1. add new complement and logicalNot operators that work as intended. See also this workaround
2. add a compiler warning when ~~~ statically resolves to a non-primitive op_LogicalNot operator, or doesn't resolve statically at all (in let inline ... code). The warning would direct the user to use complement or logicalNot explicitly instead.
3. In some far off future revision of F# change ~~~ to resolve to op_OnesCoplement and consider adding the symbolic !!! operator for op_LogicalNot.

Does that sound reasonable? If the approach is sound then steps 1-2 require updating FSharp.Core of course.

[dsyme]

I've added an entry on fslang.uservoice.com to track this: http://fslang.uservoice.com/forums/245727-f-language/suggestions/9168976-add-complement-and-logicalnot-operators-to-res, documenting the above plan

[latkin]

That sounds reasonable to me