Skip to content

Small fix from Kevin's remark in #9516 #9627

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 26 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
9eb147a
Merge from Re-enable-tests-for-operators
abelbraaksma Jun 20, 2020
f2fe117
Add tests that cover the changes for fixing FS0670
abelbraaksma Jun 23, 2020
a291564
Un-inline `string` to allow used in generic overrides and types
abelbraaksma Jun 23, 2020
cbe9123
Fix 3x C# default impl tests by removing dep. on FSharp.Core.dll
abelbraaksma Jun 24, 2020
fcb21dd
Implement new 'string' ideas from RFC-1089
abelbraaksma Jun 28, 2020
38fe68c
Merge with master
abelbraaksma Jun 28, 2020
94ca2bb
Some housekeeping, adding IFormattable to the list
abelbraaksma Jun 29, 2020
25c5f81
Further optimizations for unsigned ints and (u)nativeint
abelbraaksma Jun 30, 2020
e2c7932
Merge branch 'master' into Fix-string-FS0670-error
abelbraaksma Jun 30, 2020
2532eb6
Adding back 'inline' but not SRTP
abelbraaksma Jul 2, 2020
960a60e
Ignore NCrunch temp files and local user files
abelbraaksma Jul 3, 2020
a64a6c5
Fix string of enum-of-char
abelbraaksma Jul 3, 2020
9fdbb91
Ignore NCrunch temp files and local user files
abelbraaksma Jul 3, 2020
f6256e3
Showing only failing line in ExprTests, cleanup temp files
abelbraaksma Jul 4, 2020
3eb988d
Show sensible message when UnresolvedPathReferenceNoRange is raised b…
abelbraaksma Jul 4, 2020
07822df
Attempt to re-enable two base tests, may need to be re-disabled for Mono
abelbraaksma Jul 4, 2020
3f0d533
Cleanup tests that use global state, ensure deletion of temp files
abelbraaksma Jul 4, 2020
f1c0904
Fix difference Mono, fix spacing difference CI vs VS IDE
abelbraaksma Jul 4, 2020
513345e
Normalize null vs None
abelbraaksma Jul 4, 2020
bc2c707
Fix next issue difference between CI and local
abelbraaksma Jul 4, 2020
8caa000
Make sure both smoke tests use the same filterhack function
abelbraaksma Jul 4, 2020
6684c2b
Next fix, overlooked
abelbraaksma Jul 4, 2020
5411dc6
Merge branch 'Fix-issue-with-tests-9617' into Fix-string-FS0670-error
abelbraaksma Jul 5, 2020
489ab64
Fix tests in ExprTests.fs
abelbraaksma Jul 5, 2020
0a762ce
Distinguish between FSharp.Core 4.5, 4.6 and 4.7 in tests in ExprTest…
abelbraaksma Jul 5, 2020
a467ec0
Fix @KevinRansom's change request from #9516
abelbraaksma Jul 5, 2020
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -133,3 +133,7 @@ msbuild.binlog
/tests/fsharp/regression/5531/compilation.output.test.txt
/tests/fsharp/core/fsfromfsviacs/compilation.langversion.old.output.txt
/tests/fsharp/core/fsfromfsviacs/compilation.errors.output.txt
*ncrunch*.user
_NCrunch_*
.*crunch*.local.xml
nCrunchTemp_*
7 changes: 6 additions & 1 deletion src/fsharp/ErrorLogger.fs
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,12 @@ exception PossibleUnverifiableCode of range

exception UnresolvedReferenceNoRange of (*assemblyName*) string
exception UnresolvedReferenceError of (*assemblyName*) string * range
exception UnresolvedPathReferenceNoRange of (*assemblyName*) string * (*path*) string
exception UnresolvedPathReferenceNoRange of (*assemblyName*) string * (*path*) string with
override this.Message =
match this :> exn with
| UnresolvedPathReferenceNoRange(assemblyName, path) -> sprintf "Assembly: %s, full path: %s" assemblyName path
| _ -> "impossible"

exception UnresolvedPathReference of (*assemblyName*) string * (*path*) string * range


Expand Down
68 changes: 50 additions & 18 deletions src/fsharp/FSharp.Core/prim-types.fs
Original file line number Diff line number Diff line change
Expand Up @@ -781,9 +781,9 @@ namespace Microsoft.FSharp.Core

let inline anyToString nullStr x =
match box x with
| :? IFormattable as f -> f.ToString(null, CultureInfo.InvariantCulture)
| null -> nullStr
| :? System.IFormattable as f -> f.ToString(null,System.Globalization.CultureInfo.InvariantCulture)
| obj -> obj.ToString()
| _ -> x.ToString()

let anyToStringShowingNull x = anyToString "null" x

Expand Down Expand Up @@ -3746,6 +3746,8 @@ namespace Microsoft.FSharp.Core
open System.Diagnostics
open System.Collections.Generic
open System.Globalization
open System.Text
open System.Numerics
open Microsoft.FSharp.Core
open Microsoft.FSharp.Core.LanguagePrimitives
open Microsoft.FSharp.Core.LanguagePrimitives.IntrinsicOperators
Expand Down Expand Up @@ -4457,23 +4459,53 @@ namespace Microsoft.FSharp.Core
when ^T : ^T = (^T : (static member op_Explicit: ^T -> nativeint) (value))

[<CompiledName("ToString")>]
let inline string (value: ^T) =
let inline string (value: 'T) =
anyToString "" value
// since we have static optimization conditionals for ints below, we need to special-case Enums.
// This way we'll print their symbolic value, as opposed to their integral one (Eg., "A", rather than "1")
when ^T struct = anyToString "" value
when ^T : float = (# "" value : float #).ToString("g",CultureInfo.InvariantCulture)
when ^T : float32 = (# "" value : float32 #).ToString("g",CultureInfo.InvariantCulture)
when ^T : int64 = (# "" value : int64 #).ToString("g",CultureInfo.InvariantCulture)
when ^T : int32 = (# "" value : int32 #).ToString("g",CultureInfo.InvariantCulture)
when ^T : int16 = (# "" value : int16 #).ToString("g",CultureInfo.InvariantCulture)
when ^T : nativeint = (# "" value : nativeint #).ToString()
when ^T : sbyte = (# "" value : sbyte #).ToString("g",CultureInfo.InvariantCulture)
when ^T : uint64 = (# "" value : uint64 #).ToString("g",CultureInfo.InvariantCulture)
when ^T : uint32 = (# "" value : uint32 #).ToString("g",CultureInfo.InvariantCulture)
when ^T : int16 = (# "" value : int16 #).ToString("g",CultureInfo.InvariantCulture)
when ^T : unativeint = (# "" value : unativeint #).ToString()
when ^T : byte = (# "" value : byte #).ToString("g",CultureInfo.InvariantCulture)
when 'T : string = (# "" value : string #) // force no-op

// Using 'let x = (# ... #) in x.ToString()' leads to better IL, without it, an extra stloc and ldloca.s (get address-of)
// gets emitted, which are unnecessary. With it, the extra address-of-variable is not created
when 'T : float = let x = (# "" value : float #) in x.ToString(null, CultureInfo.InvariantCulture)
when 'T : float32 = let x = (# "" value : float32 #) in x.ToString(null, CultureInfo.InvariantCulture)
when 'T : decimal = let x = (# "" value : decimal #) in x.ToString(null, CultureInfo.InvariantCulture)
when 'T : BigInteger = let x = (# "" value : BigInteger #) in x.ToString(null, CultureInfo.InvariantCulture)

// no IFormattable
when 'T : char = let x = (# "" value : 'T #) in x.ToString() // use 'T, because char can be an enum
when 'T : bool = let x = (# "" value : bool #) in x.ToString()
when 'T : nativeint = let x = (# "" value : nativeint #) in x.ToString()
when 'T : unativeint = let x = (# "" value : unativeint #) in x.ToString()

// Integral types can be enum:
// It is not possible to distinguish statically between Enum and (any type of) int. For signed types we have
// to use IFormattable::ToString, as the minus sign can be overridden. Using boxing we'll print their symbolic
// value if it's an enum, e.g.: 'ConsoleKey.Backspace' gives "Backspace", rather than "8")
when 'T : sbyte = (box value :?> IFormattable).ToString(null, CultureInfo.InvariantCulture)
when 'T : int16 = (box value :?> IFormattable).ToString(null, CultureInfo.InvariantCulture)
when 'T : int32 = (box value :?> IFormattable).ToString(null, CultureInfo.InvariantCulture)
when 'T : int64 = (box value :?> IFormattable).ToString(null, CultureInfo.InvariantCulture)

// unsigned integral types have equal behavior with 'T::ToString() vs IFormattable::ToString
// this allows us to issue the 'constrained' opcode with 'callvirt'
when 'T : byte = let x = (# "" value : 'T #) in x.ToString()
when 'T : uint16 = let x = (# "" value : 'T #) in x.ToString()
when 'T : uint32 = let x = (# "" value : 'T #) in x.ToString()
when 'T : uint64 = let x = (# "" value : 'T #) in x.ToString()


// other common mscorlib System struct types
when 'T : DateTime = let x = (# "" value : DateTime #) in x.ToString(null, CultureInfo.InvariantCulture)
when 'T : DateTimeOffset = let x = (# "" value : DateTimeOffset #) in x.ToString(null, CultureInfo.InvariantCulture)
when 'T : TimeSpan = let x = (# "" value : TimeSpan #) in x.ToString(null, CultureInfo.InvariantCulture)
when 'T : Guid = let x = (# "" value : Guid #) in x.ToString(null, CultureInfo.InvariantCulture)
when 'T struct =
match box value with
| :? IFormattable as f -> f.ToString(null, CultureInfo.InvariantCulture)
| _ -> value.ToString()

// other commmon mscorlib reference types
when 'T : StringBuilder = let x = (# "" value : StringBuilder #) in x.ToString()
when 'T : IFormattable = let x = (# "" value : IFormattable #) in x.ToString(null, CultureInfo.InvariantCulture)

[<NoDynamicInvocation(isLegacy=true)>]
[<CompiledName("ToChar")>]
Expand Down
6 changes: 3 additions & 3 deletions src/fsharp/FSharp.Core/prim-types.fsi
Original file line number Diff line number Diff line change
Expand Up @@ -2824,12 +2824,12 @@ namespace Microsoft.FSharp.Core

/// <summary>Converts the argument to a string using <c>ToString</c>.</summary>
///
/// <remarks>For standard integer and floating point values the <c>ToString</c> conversion
/// uses <c>CultureInfo.InvariantCulture</c>. </remarks>
/// <remarks>For standard integer and floating point values the and any type that implements <c>IFormattable</c>
/// <c>ToString</c> conversion uses <c>CultureInfo.InvariantCulture</c>. </remarks>
/// <param name="value">The input value.</param>
/// <returns>The converted string.</returns>
[<CompiledName("ToString")>]
val inline string : value:^T -> string
val inline string : value:'T -> string

/// <summary>Converts the argument to System.Decimal using a direct conversion for all
/// primitive numeric types. For strings, the input is converted using <c>UInt64.Parse()</c>
Expand Down
4 changes: 0 additions & 4 deletions tests/FSharp.Core.UnitTests/FSharp.Core.UnitTests.fsproj
Original file line number Diff line number Diff line change
Expand Up @@ -107,8 +107,4 @@
<PackageReference Include="xunit.runner.visualstudio" Version="2.4.1" PrivateAssets="All" />
</ItemGroup>

<ItemGroup>
<Content Remove="FSharp.Core\OperatorsModule2.fs" />
</ItemGroup>

</Project>
60 changes: 60 additions & 0 deletions tests/FSharp.Core.UnitTests/FSharp.Core/OperatorsModule2.fs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,19 @@ namespace FSharp.Core.UnitTests.Operators
open System
open FSharp.Core.UnitTests.LibraryTestFx
open NUnit.Framework
open System.Globalization
open System.Threading

/// If this type compiles without error it is correct
/// Wrong if you see: FS0670 This code is not sufficiently generic. The type variable ^T could not be generalized because it would escape its scope.
type TestFs0670Error<'T> =
| TestFs0670Error of 'T
override this.ToString() =
match this with
| TestFs0670Error x ->
// This used to raise FS0670 because the type is generic, and 'string' was inline
// See: https://github.com/dotnet/fsharp/issues/7958
Operators.string x

[<TestFixture>]
type OperatorsModule2() =
Expand Down Expand Up @@ -718,6 +731,53 @@ type OperatorsModule2() =
// reference type
let result = Operators.string "ABC"
Assert.AreEqual("ABC", result)

// reference type without a `ToString()` overload
let result = Operators.string (obj())
Assert.AreEqual("System.Object", result)

let result = Operators.string 1un
Assert.AreEqual("1", result)

let result = Operators.string (obj())
Assert.AreEqual("System.Object", result)

let result = Operators.string 123.456M
Assert.AreEqual("123.456", result)

// Following tests ensure that InvariantCulture is used if type implements IFormattable

// safe current culture, then switch culture
let currentCI = Thread.CurrentThread.CurrentCulture
Thread.CurrentThread.CurrentCulture <- CultureInfo.GetCultureInfo("de-DE")

// make sure the culture switch happened, and verify
let wrongResult = 123.456M.ToString()
Assert.AreEqual("123,456", wrongResult)

// test that culture has no influence on decimals with `string`
let correctResult = Operators.string 123.456M
Assert.AreEqual("123.456", correctResult)

// make sure that the German culture is indeed selected for DateTime
let dttm = DateTime(2020, 6, 23)
let wrongResult = dttm.ToString()
Assert.AreEqual("23.06.2020 00:00:00", wrongResult)

// test that culture has no influence on DateTime types when used with `string`
let correctResult = Operators.string dttm
Assert.AreEqual("06/23/2020 00:00:00", correctResult)

// reset the culture
Thread.CurrentThread.CurrentCulture <- currentCI

[<Test>]
member _.``string: don't raise FS0670 anymore``() =
// The type used here, when compiled, should not raise this error:
// "FS0670 This code is not sufficiently generic. The type variable ^T could not be generalized because it would escape its scope."
// See: https://github.com/dotnet/fsharp/issues/7958
let result = TestFs0670Error 32uy |> Operators.string
Assert.AreEqual("32", result)

[<Test>]
member _.tan() =
Expand Down
11 changes: 6 additions & 5 deletions tests/fsharp/Compiler/Language/DefaultInterfaceMemberTests.fs
Original file line number Diff line number Diff line change
Expand Up @@ -813,15 +813,15 @@ type Test () =
member __.M(_x: int) = Console.Write("InTest")

member __.M<'Item> (x: int, y: 'Item) =
Console.Write(string x)
Console.Write(x.ToString())
Console.Write(y.ToString ())

member __.M<'TTT> (x: 'TTT) =
Console.Write(x.ToString ())

member __.M (x: int, text: string) =
Console.Write("ABC")
Console.Write(string x)
Console.Write(x.ToString())
Console.Write(text)

member __.M<'U> (_x: 'U, _y: int) = ()
Expand Down Expand Up @@ -1166,7 +1166,8 @@ type Test () =
let main _ =
let x = Test () :> I1
let y = Test () :> I2
Console.Write(string (x + y))
let result = x + y
Console.Write(result.ToString())
0
"""

Expand Down Expand Up @@ -4229,15 +4230,15 @@ type Test () =
member __.M(_x: int) = Console.Write("InTest")

member __.M<'Item> (x: int, y: 'Item) =
Console.Write(string x)
Console.Write(x.ToString())
Console.Write(y.ToString ())

member __.M<'TTT> (x: 'TTT) =
Console.Write(x.ToString ())

member __.M (x: int, text: string) =
Console.Write("ABC")
Console.Write(string x)
Console.Write(x.ToString())
Console.Write(text)

type Test2 () =
Expand Down
Loading