-
Notifications
You must be signed in to change notification settings - Fork 825
[WIP] F# RFC-1003 - Implementing nameof operator #908
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
Conversation
@forki put |
@@ -195,7 +195,7 @@ module LeafExpressionConverter = | |||
|
|||
let isFunctionType typ = equivHeadTypes typ (typeof<(int -> int)>) | |||
let getFunctionType typ = | |||
if not (isFunctionType typ) then invalidArg "typ" "cannot convert recursion except for function types" | |||
if not (isFunctionType typ) then invalidArg (nameof typ) "cannot convert recursion except for function types" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i dont think we should use nameof
in this pr.
Like that you cannot compile FSharp.Core
with LKG compiler (i think that's used somewhere in proto)
Also that make this pr bigger than needed
The use of nameof
can be an additional pr after this one is merged
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah. I'm removing the use in FSharp.Core - that wasn't really a good idea
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done
btw, jenkins fails /cc @otawfik-ms @TyOverby
|
afeff33
to
d8f83bf
Compare
[<Test>] | ||
member this.``lookup name of + operator`` () = | ||
let b = nameof(+) | ||
Assert.AreEqual("op_Addition",b) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's expected behaviour for operators? i mean nameof(+)
shouldnt be +
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
op_Addition is compiled name IIRC
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but it's what we want for nameof(+)
? just asking, it make sense as op_Addition
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do you have a C# compiler at hand? What's that thingy saying?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, let me check
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
and tbh it's probably never used in practice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
side question: why is "dotnet repl" starting the C# repl? Because CLR = C# language runtime?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@forki 👍 for op_Addition
, can be nice to know compiled name, and i dont think it's really usefull do nameof(+)
@forki about dotnet/cli it's ok, dotnet repl
run dotnet-repl-csi
because default lang is c#
.
No need to fight anymore for defaults, i tried with dotnet new
, a nice discussion and now i think there is a nice command line api (cmd api is language agnostic, default is c# sometimes, like dotnet new
or dotnet repl
)
The real big boss dotnet build
doesnt need a lang argument.
About default c#, i think we can live with that if the language support is ok.
So dotnet repl f#
runs dotnet-repl-fsi
. BTW because fsi is in path (it's near dotnet.exe), you can do just fsi
i'll implement dotnet repl lang
soon (lang argument already works, we need a dotnet-repl-fsi
to call fsi
) . For example my dotnet new
f# support got merged two days ago, but dotnet/cli is a nice repo, discussion => actions and team help a lot
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@forki side answer update, now there is dotnet/cli#1118 about use a DOTNET_DEFAULT_LANGUAGE
env var for dotnet new
and dotnet repl
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See. Sometimes we have to ask the right questions.
On Jan 29, 2016 1:41 PM, "Enrico Sada" [email protected] wrote:
In src/fsharp/FSharp.Core.Unittests/NameOfTests.fs
#908 (comment):
- member this.
static class function name
() =let b = nameof(Tuple.Create)
Assert.AreEqual("Create",b)
+[]
+type OperatorNameTests() =
+- []
- member this.
lookup name of typeof operator
() =let b = nameof(typeof<int>)
Assert.AreEqual("TypeOf",b)
- []
- member this.
lookup name of + operator
() =let b = nameof(+)
Assert.AreEqual("op_Addition",b)
@forki https://github.com/forki side answer update, now there is
dotnet/cli#1118 https://github.com/dotnet/cli/issues/1118 about use a
DOTNET_DEFAULT_LANGUAGE env var for dotnet new and dotnet repl.—
Reply to this email directly or view it on GitHub
https://github.com/Microsoft/visualfsharp/pull/908/files#r51256391.
add gitignore of fsharp.core in tests ( see #894 (comment) ) |
Nah, that should be done in separate PR and applied immediately. Annoying to the max. /cc @otawfik-ms |
let myFunction parameter1 = nameof parameter1 | ||
|
||
Assert.AreEqual("parameter1",myFunction "x") | ||
Assert.AreEqual("parameter1",myFunction "x") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this second assert of any value?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We want to be really really sure here!
Guess it's merge /rebase issue from long time ago
25d9b98
to
a40c767
Compare
match tyargs with | ||
| (typeName:TType):: _ -> | ||
let name = | ||
match typeName with |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please call stripTyEqns typeName
. Consider, for example, typenameof<'T> where 'T gets later instantiated to int, e.g.
let f(x:'T) = typenameof<'T>; (x = 3)
This will raise a warning that 'T
can't be generalized and gets set to int
, but apart from the warning the code is allowed.
Also, please consider what happens in the presence of type aliases type x = int
. I assume the intended spec is that the name of the canonical representation is returned. Using stripTyEqns
will achieve this.
It would be awesome to have this revived and shipped in next language update. I imagine there is a way to do similar thing with code quotations but a proper operator (even if it doesn't cover all the cases) is a much better option. |
@smoothdeveloper You can like this: match <@@ (Unchecked.Defaultof<MyType> : MyType.MyMethodWithIntParam<'a>(0) @@> with
| Patterns.Call(_,mi,_) -> ... Works well. |
@KevinRansom Let's keep open any WIP PRs that correspond to an approved F# RFC. Inactivity is OK for RFCs - they can take months or years to get through the system. |
I rebased this on master |
9ef9719
to
c2b74ad
Compare
Closing |
Why are you closing this? |
@isaacabraham I think it's reasonable to close PRs that haven't been touched for a very long time. The PR can definitely be resubmitted as part of F# 4.2 work, it's not lost. It's probably on my plate to review the RFC and implementation once again. But the RFC tracks the issue for now. |
I might like to be able to write inline functions that take the parameter that nameof receives and pass it directly along to the operator invocation. It seems like this should be possible. Is this possible with the current implementation? If not, can we consider that as a feature? |
Actually, as it turns out, my above comment in its initial form was not very accurate (I have since altered it). I looked at my code base, and as it turns out, I only have speculative uses for making the nameof parameter forwardable from inline functions. It wouldn't do anything for my current code base after all. Doesn't mean the capability isn't worth consideration tho, I suppose. Apologies for the misdirection. |
RFC: https://github.com/fsharp/FSharpLangDesign/blob/master/RFCs/FS-1003-nameof-operator.md
This is a rebased version of #13, which was moved from https://visualfsharp.codeplex.com/SourceControl/network/forks/forki/fsharp/contribution/7698 (see discussion there)
https://fslang.uservoice.com/forums/245727-f-language/suggestions/5900625-add-nameof-operator-to-follow-c-vb-update suggests we implement a "nameof" operator which is something similar to the typeof operator.
This feature was "approved in principle" by @dsyme.
Related information:
C#/VB spec for nameof: https://roslyn.codeplex.com/discussions/570551
Mads showing the nameof operator: http://channel9.msdn.com/Events/Visual-Studio/Connect-event-2014/116 (at 5:35)
Current status:
+
,|>
,typeof
,nameof
, ...