Skip to content

Conversation

dsyme
Copy link
Contributor

@dsyme dsyme commented May 22, 2019

Continuation of #6325

Brings #2290 up-to-date with master

Implementation of RFC FS-1003

Testing verified:

  • handle cases where nameof already resolves to something user-defined, to avoid being a breaking change
  • works in quotations
  • works with method parameters
  • works with local variables
  • works with local (nested) functions
  • works with local curried functions
  • works with local tupled functions
  • can get name from inside a local function (needs to be let rec)
  • can get member names
  • can get static member names
  • can get static property names
  • can get names that quoted in ``
  • can be used in pattern matching
  • can be used with generic functions/types
  • when used like a function, let f = nameof ;; f x
  • when used with pipe operator, x |> nameof
  • can get names of operators like +, |>, typeof, nameof, ...
  • works in attributes
  • works with namespaces, nameof System.Diagnostics
  • works with types, nameof System.String
  • works with modules, nameof List

Feature limitation:

  • the feature will not work with type arguments, let f<'t> (x : 't) = nameof 't

Vasily Kirichenko and others added 30 commits January 18, 2017 16:24
Conflicts:
	src/fsharp/FSComp.txt
	src/fsharp/FSharp.Core.Unittests/SurfaceArea.Silverlight.2.0.fs
	src/fsharp/FSharp.Core.Unittests/SurfaceArea.net20.fs
	src/fsharp/PostInferenceChecks.fs
	src/fsharp/TastOps.fs
	src/fsharp/TcGlobals.fs
This reverts commit 529cc6f.
This reverts commit 223d313.

Conflicts:
	tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/NameOf/E_NameOfAdditionExpr.fs
	tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/NameOf/E_NameOfAppliedFunction.fs
	tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/NameOf/E_NameOfAsAFunction.fs
	tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/NameOf/E_NameOfDictLookup.fs
	tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/NameOf/E_NameOfIntConst.fs
	tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/NameOf/E_NameOfIntegerAppliedFunction.fs
	tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/NameOf/E_NameOfParameterAppliedFunction.fs
	tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/NameOf/E_NameOfPartiallyAppliedFunction.fs
	tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/NameOf/E_NameOfStringConst.fs
	tests/fsharpqa/Source/Conformance/Expressions/DataExpressions/NameOf/E_NameOfWithPipe.fs
it raises a proper error if applicated to non (Long)Ident arg
remove `typenameof and its tests
@KevinRansom
Copy link
Contributor

@cartermp on it

Copy link
Contributor

@cartermp cartermp left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After chatting with @TIHan we should remove all baseline-style tests and move these to FSharp.Compiler.UnitTests. We can handle this on our end.

@KevinRansom KevinRansom closed this Jul 2, 2019
@KevinRansom KevinRansom reopened this Jul 2, 2019
@dsyme
Copy link
Contributor Author

dsyme commented Jul 3, 2019

@KevinRansom What's the status of getting this updated with the language version flag checks please? I thought I saw you doing some work on this but I can't find it anywhere.

If you are doing something, please make it a PR to the feature/nameof branch?

@KevinRansom
Copy link
Contributor

I am happy to merge this and then add the languageversion into release/fsharp47. That would be a better way of dealing with these language features.

@cartermp cartermp dismissed their stale review July 3, 2019 21:58

Outdated

@KevinRansom
Copy link
Contributor

Almost squashed it .... merging :-)

@KevinRansom KevinRansom merged commit 92e4a52 into release/fsharp47 Jul 3, 2019
@KevinRansom KevinRansom deleted the feature/nameof branch July 10, 2019 21:54
@cartermp cartermp modified the milestones: .NET Core 3.0, 16.3 Aug 5, 2019
@cmeeren
Copy link
Contributor

cmeeren commented Aug 8, 2019

Sorry if this is the wrong place to ask, but is it intended to be possible (like in C#) to get the name of an instance member without an instance, e.g. nameof SomeType.InstanceProperty? I installed FSharp.Core 4.7 to test this, but currently compilation fails with Property 'InstanceProperty' is not static. I'm using VS 16.2.1 if that's relevant; do I perhaps need to wait for 16.3?

@cartermp
Copy link
Contributor

cartermp commented Aug 8, 2019

@cmeeren That's for @dsyme to answer. But since it's not supported, the answer is "no" for now.

@cartermp
Copy link
Contributor

cartermp commented Aug 8, 2019

It's also worth noting that what is allowable in C# for nameof is adjusted compared to normal statements. SomeType.InstanceProperty is not allowable outside of a nameof() expression. A similar kind of relaxation could be built for F#, but that doesn't exist today.

@@ -4142,6 +4142,9 @@ namespace Microsoft.FSharp.Core
[<CompiledName("TypeOf")>]
let inline typeof<'T> = BasicInlinedOperations.typeof<'T>

[<CompiledName("NameOf")>]
let inline nameof (_: 'T) : string = raise (Exception "may not call directly, should always be optimized away")
Copy link
Contributor

@TIHan TIHan Aug 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we could make this have NoDynamicInvocation

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it's not needed because the compiler knows of the special rules for this library identifier


[<Test>]
member this.``lookup name of |> operator`` () =
let a = nameof(|>)
Copy link
Contributor

@TIHan TIHan Aug 9, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we really want to allow nameof to get the actual names of the operators?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes I'm fine with this

@cartermp cartermp modified the milestones: 16.3, Unknown / not bug Aug 14, 2019
@dsyme
Copy link
Contributor Author

dsyme commented Aug 16, 2019

@cmeeren That's for @dsyme to answer. But since it's not supported, the answer is "no" for now.

Yes the answer is "no" for now, though it would be good to find a way to support this as we take the feature out of preview

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants