RFC FS-1012 - Support for caller info argument attributes (CallerLineNumber, CallerFileName, CallerMemberName) #1114

Merged
merged 59 commits into from Jun 25, 2016

Conversation

Projects
None yet
8 participants
@latkin
Contributor

latkin commented Apr 25, 2016

Paperwork:

Adds support for emerging .NET standard "caller info" attributes. Put this together with @AviAvni as part of the FSSF mentorship program.

Right now this provides complete support (modulo feedback/bugs) for CallerLineNumber and CallerFilePath.

CallerMemberName is not yet complete, as this information does not seem to be available in the right place, and we didn't see a simple way to add it in. We are looking for suggestions/help from others with finishing that part. What part of the compilation would have this info, and how can we get access to it in a sensible place for this feature?

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 25, 2016

Contributor

Regarding red build: that's because latest nightly dotnet cli is breaking our build. I already reported that in my latest PR.

Contributor

forki commented Apr 25, 2016

Regarding red build: that's because latest nightly dotnet cli is breaking our build. I already reported that in my latest PR.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Apr 25, 2016

Contributor

Merging #1116 should fix the dotnet cli error.

Contributor

forki commented Apr 25, 2016

Merging #1116 should fix the dotnet cli error.

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Apr 26, 2016

Contributor

@dotnet-bot test this please

Contributor

KevinRansom commented Apr 26, 2016

@dotnet-bot test this please

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Apr 26, 2016

Contributor

@dotnet-bot test this please

Contributor

KevinRansom commented Apr 26, 2016

@dotnet-bot test this please

src/fsharp/FSComp.txt
@@ -256,7 +256,7 @@ chkUnionCaseDefaultAugmentation,"default augmentation of the union case"
438,chkDuplicateMethod,"Duplicate method. The method '%s' has the same name and signature as another method in this type."
438,chkDuplicateMethodWithSuffix,"Duplicate method. The method '%s' has the same name and signature as another method in this type once tuples, functions, units of measure and/or provided types are erased."
439,chkDuplicateMethodCurried,"The method '%s' has curried arguments but has the same name as another method in this type. Methods with curried arguments cannot be overloaded. Consider using a method taking tupled arguments."
-440,chkCurriedMethodsCantHaveOutParams,"Methods with curried arguments cannot declare 'out', 'ParamArray', 'optional', 'ReflectedDefinition' or 'byref' arguments"
+440,chkCurriedMethodsCantHaveOutParams,"Methods with curried arguments cannot declare 'out', 'ParamArray', 'optional', 'ReflectedDefinition', 'byref', 'CallerLineNumber', or 'CallerFilePath' arguments"

This comment has been minimized.

@dsyme

dsyme Apr 26, 2016

Contributor

What about CallerMemberName?

@dsyme

dsyme Apr 26, 2016

Contributor

What about CallerMemberName?

This comment has been minimized.

@latkin

latkin Apr 26, 2016

Contributor

Not implemented yet, so not added quite everywhere. Any thoughts on how to flow in the caller member name information? It didn't seem obvious to me but I could have missed something.

@latkin

latkin Apr 26, 2016

Contributor

Not implemented yet, so not added quite everywhere. Any thoughts on how to flow in the caller member name information? It didn't seem obvious to me but I could have missed something.

This comment has been minimized.

@dsyme

dsyme Apr 26, 2016

Contributor

Ah yes, hadn't realized that. Tricky since in some situations there isn't even a valid name, e.g. a script that just calls one of these members. I suppose we'd add it to TcEnv and set it as we descend into each function/method definition, and give a compile-time warning or error if there is no valid caller method name.

@dsyme

dsyme Apr 26, 2016

Contributor

Ah yes, hadn't realized that. Tricky since in some situations there isn't even a valid name, e.g. a script that just calls one of these members. I suppose we'd add it to TcEnv and set it as we descend into each function/method definition, and give a compile-time warning or error if there is no valid caller method name.

This comment has been minimized.

@dsyme

dsyme Apr 26, 2016

Contributor

Could you add those issues to the RFC please? Thanks

@dsyme

dsyme Apr 26, 2016

Contributor

Could you add those issues to the RFC please? Thanks

This comment has been minimized.

@latkin

latkin Apr 26, 2016

Contributor

Sure, will add. Similar situation on, say, assembly-level attribute constructor - no enclosing member.

@latkin

latkin Apr 26, 2016

Contributor

Sure, will add. Similar situation on, say, assembly-level attribute constructor - no enclosing member.

src/fsharp/TypeChecker.fs
@@ -8713,7 +8713,7 @@ and TcMethodApplication
let denv = env.DisplayEnv
- let isSimpleFormalArg (isParamArrayArg, isOutArg, optArgInfo: OptionalArgInfo, _reflArgInfo: ReflectedArgInfo) =
+ let isSimpleFormalArg (isParamArrayArg, isOutArg, optArgInfo: OptionalArgInfo, _callerInfoInfo: CallerInfoInfo, _reflArgInfo: ReflectedArgInfo) =
not isParamArrayArg && not isOutArg && not optArgInfo.IsOptional

This comment has been minimized.

@dsyme

dsyme Apr 26, 2016

Contributor

I suspect you may want to check that callerInfo is NoCallerInfo here. I believe this relates to the question of what happens on first-class uses of methods with these attributes, see the various uses of isSimpleFormalArg below

@dsyme

dsyme Apr 26, 2016

Contributor

I suspect you may want to check that callerInfo is NoCallerInfo here. I believe this relates to the question of what happens on first-class uses of methods with these attributes, see the various uses of isSimpleFormalArg below

This comment has been minimized.

@latkin

latkin Apr 26, 2016

Contributor

Will test that, thanks

@latkin

latkin Apr 26, 2016

Contributor

Will test that, thanks

src/fsharp/infos.fs
+ | CallerMemberName
+ | CallerFilePath
+
+ override x.ToString() =

This comment has been minimized.

@dsyme

dsyme Apr 26, 2016

Contributor

Probably just use override x.ToString() = sprintf "%+A" x, simpler?

@dsyme

dsyme Apr 26, 2016

Contributor

Probably just use override x.ToString() = sprintf "%+A" x, simpler?

This comment has been minimized.

@latkin

latkin Apr 26, 2016

Contributor

In normal F# code %A nice-prints union types, but it seems that in the compiler this is not the case? I forgot about %+A, will try that out.

@latkin

latkin Apr 26, 2016

Contributor

In normal F# code %A nice-prints union types, but it seems that in the compiler this is not the case? I forgot about %+A, will try that out.

This comment has been minimized.

@dsyme

dsyme Apr 26, 2016

Contributor

Yes, %+A is needed when types are internal

@dsyme

dsyme Apr 26, 2016

Contributor

Yes, %+A is needed when types are internal

+
+let scriptName = if Array.contains "--exec" (System.Environment.GetCommandLineArgs()) then "ViaInteractive.fsx" else "stdin"
+let checkPath = sprintf "Conformance\\SpecialAttributesAndTypes\\Imported\\CallerInfo\\%s" scriptName
+;;

This comment has been minimized.

@dsyme

dsyme Apr 26, 2016

Contributor

No need for ;; I think. Or use let _ = ... or some other formulation if needed?

@dsyme

dsyme Apr 26, 2016

Contributor

No need for ;; I think. Or use let _ = ... or some other formulation if needed?

This comment has been minimized.

@latkin

latkin Apr 26, 2016

Contributor

Will check

@latkin

latkin Apr 26, 2016

Contributor

Will check

+open CSharpLib
+[<MyCallerInfo>]
+type MyTy() =
+ static member GetCallerLineNumber([<CallerLineNumber>] ?line : int) =

This comment has been minimized.

@dsyme

dsyme Apr 26, 2016

Contributor

Add tests for first-class uses of GetCallerLineNumber and friends (and record the resolution of any design issues that arise in the RFC)

This includes "piped" uses () |> GetCallerLineNumber etc.

@dsyme

dsyme Apr 26, 2016

Contributor

Add tests for first-class uses of GetCallerLineNumber and friends (and record the resolution of any design issues that arise in the RFC)

This includes "piped" uses () |> GetCallerLineNumber etc.

This comment has been minimized.

@latkin

latkin Apr 26, 2016

Contributor

Sure

@latkin

latkin Apr 26, 2016

Contributor

Sure

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Apr 26, 2016

Contributor

The code basically looks very sound to me - great work! Added some comments above about some cases related to the first-class use of attributed methods.

Contributor

dsyme commented Apr 26, 2016

The code basically looks very sound to me - great work! Added some comments above about some cases related to the first-class use of attributed methods.

src/fsharp/infos.fs
+ // if multiple caller info attributes are specified, pick the "wrong" one here
+ // so that we get an error later
+ if p.Type.TypeRef.FullName = "System.Int32" then CallerFilePath
+ else CallerLineNumber

This comment has been minimized.

@enricosada

enricosada Apr 26, 2016

Contributor

if style

if then
   //code
else
   //code
@enricosada

enricosada Apr 26, 2016

Contributor

if style

if then
   //code
else
   //code

This comment has been minimized.

@dsyme

dsyme Apr 26, 2016

Contributor

@enricosada The existing code is idiomatic given it's context in the compiler (and the compiler rule "code like the code around you")

@dsyme

dsyme Apr 26, 2016

Contributor

@enricosada The existing code is idiomatic given it's context in the compiler (and the compiler rule "code like the code around you")

This comment has been minimized.

@enricosada

enricosada Apr 26, 2016

Contributor

yes sry, before i wrote the comment i checked the same file for if/else style and usually was like my comment, not always, it's mixed.
But now i see the code near is like that style, so np

@enricosada

enricosada Apr 26, 2016

Contributor

yes sry, before i wrote the comment i checked the same file for if/else style and usually was like my comment, not always, it's mixed.
But now i see the code near is like that style, so np

+ let o1 = MyTy("42")
+
+ match o.Path with
+ | Some(path) when matchesPath "Conformance\\SpecialAttributesAndTypes\\Imported\\CallerInfo\\CallerFilePath.fs" path -> ()

This comment has been minimized.

@enricosada

enricosada Apr 26, 2016

Contributor

the \ is ok for windows, but on other os?
it's possibile to compare full path or normalize to uri?
or use __SOURCE_FILE__

@enricosada

enricosada Apr 26, 2016

Contributor

the \ is ok for windows, but on other os?
it's possibile to compare full path or normalize to uri?
or use __SOURCE_FILE__

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 9, 2016

Contributor

@AviAvni Could you send a PR to the RFC to list the resolution of all unresolved issues? And make any other updates to the RFC that you think are needed to, to track the status of the work. Once that's in place we'll do what will (hopefully!) be a final review.

https://github.com/fsharp/FSharpLangDesign/blob/master/RFCs/FS-1012-caller-info-attributes.md

Thanks!

Contributor

dsyme commented Jun 9, 2016

@AviAvni Could you send a PR to the RFC to list the resolution of all unresolved issues? And make any other updates to the RFC that you think are needed to, to track the status of the work. Once that's in place we'll do what will (hopefully!) be a final review.

https://github.com/fsharp/FSharpLangDesign/blob/master/RFCs/FS-1012-caller-info-attributes.md

Thanks!

@AviAvni

This comment has been minimized.

Show comment
Hide comment
@AviAvni

AviAvni Jun 9, 2016

Contributor

@dsyme thanks great I sent the PR to the RFC

Contributor

AviAvni commented Jun 9, 2016

@dsyme thanks great I sent the PR to the RFC

+ let functionVal = MyTy.GetCallerMemberName
+ let typeLetValue = MyTy.GetCallerMemberName()
+ let typeLetFunc (i:int) = i, MyTy.GetCallerMemberName()
+ let typeLetFuncNested () =

This comment has been minimized.

@dsyme

dsyme Jun 10, 2016

Contributor

Could you please also specify and test what happens when the call to GetCallerMemberName is made from

  • an anonymous lambda experession, e.g. (fun () -> ...).
  • an object expression member implementation implementing an interface
  • an object expression member implementation implementing an abstract member in a base class
  • a delegate implementation e.g. new System.Func<int,int>(fun a -> ...)
  • within an async expression async { ... } (whose desugaring has some implied lambda expressions)

Thanks

@dsyme

dsyme Jun 10, 2016

Contributor

Could you please also specify and test what happens when the call to GetCallerMemberName is made from

  • an anonymous lambda experession, e.g. (fun () -> ...).
  • an object expression member implementation implementing an interface
  • an object expression member implementation implementing an abstract member in a base class
  • a delegate implementation e.g. new System.Func<int,int>(fun a -> ...)
  • within an async expression async { ... } (whose desugaring has some implied lambda expressions)

Thanks

This comment has been minimized.

@AviAvni

AviAvni Jun 10, 2016

Contributor

@dsyme I added the tests you asked I hope I understand you correctly
I think maybe the anonymous lambda expression result is wrong what do you think is the expected value?

@AviAvni

AviAvni Jun 10, 2016

Contributor

@dsyme I added the tests you asked I hope I understand you correctly
I think maybe the anonymous lambda expression result is wrong what do you think is the expected value?

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 10, 2016

Contributor

@AviAvni Thanks for updating the PR. We'll do a (hopefully final) review now.

So far I added one comment here: https://github.com/Microsoft/visualfsharp/pull/1114/files#r66590569. (Perhaps there was testing for these cases but I didn't see it as yet)

Contributor

dsyme commented Jun 10, 2016

@AviAvni Thanks for updating the PR. We'll do a (hopefully final) review now.

So far I added one comment here: https://github.com/Microsoft/visualfsharp/pull/1114/files#r66590569. (Perhaps there was testing for these cases but I didn't see it as yet)

+ new(a : int) =
+ { A = a }
+ then
+ MyTy.Check(MyTy.GetCallerMemberName(), Some(".ctor"), "struct ctor")

This comment has been minimized.

@dsyme

dsyme Jun 10, 2016

Contributor

Does C# also use .ctor as the name?

@dsyme

dsyme Jun 10, 2016

Contributor

Does C# also use .ctor as the name?

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 10, 2016

Contributor

Basically this is looking really good. I added another question about .ctor as the name.

Contributor

dsyme commented Jun 10, 2016

Basically this is looking really good. I added another question about .ctor as the name.

@eiriktsarpalis

This comment has been minimized.

Show comment
Hide comment
@eiriktsarpalis

eiriktsarpalis Jun 10, 2016

Contributor

Awesome feature! Can't wait to start working on an Async PoC.

Contributor

eiriktsarpalis commented Jun 10, 2016

Awesome feature! Can't wait to start working on an Async PoC.

@AviAvni

This comment has been minimized.

Show comment
Hide comment
@AviAvni

AviAvni Jun 10, 2016

Contributor

@eiriktsarpalis thanks it is great feature and it was fun work on it
I learned a lot about the f# compiler

Contributor

AviAvni commented Jun 10, 2016

@eiriktsarpalis thanks it is great feature and it was fun work on it
I learned a lot about the f# compiler

@@ -45,6 +47,8 @@ type MyTy() =
|> List.head
MyTy.Check(result, Some("CheckMembers"), "lambda")
MyTy.Check(functionVal (), Some("functionVal"), "functionVal")
+ MyTy.Check(anonymousLambda, Some("anonymousLambda"), "anonymousLambda")
+ MyTy.Check(asyncVal, Some("asyncVal"), "asyncVal")

This comment has been minimized.

@dsyme

dsyme Jun 10, 2016

Contributor

Certainly asyncVal result seems wrong. If the thing being defined by let x = ... is not a function then it doesn't seem sensible to be using the name x.

I suspect you will see similar dubious results for let x = if 1 > 0 then MyTy.GetCallerMemberName() else ""?

What do you get for

let f () = 
    let x = MyTy.GetCallerMemberName() 
    x

or

let f () = 
    let x = 
        let g () = MyTy.GetCallerMemberName() 
        g()
    x

???

For anonymousLamba there is a quirk in the F# language spec where let x = fun args -> ... is considered identical to let x args = ... for module-defined function values. So it may be you need to use an expression-bound definition.

@dsyme

dsyme Jun 10, 2016

Contributor

Certainly asyncVal result seems wrong. If the thing being defined by let x = ... is not a function then it doesn't seem sensible to be using the name x.

I suspect you will see similar dubious results for let x = if 1 > 0 then MyTy.GetCallerMemberName() else ""?

What do you get for

let f () = 
    let x = MyTy.GetCallerMemberName() 
    x

or

let f () = 
    let x = 
        let g () = MyTy.GetCallerMemberName() 
        g()
    x

???

For anonymousLamba there is a quirk in the F# language spec where let x = fun args -> ... is considered identical to let x args = ... for module-defined function values. So it may be you need to use an expression-bound definition.

This comment has been minimized.

@AviAvni

AviAvni Jun 10, 2016

Contributor

thanks @dsyme I'll debug this on sunday night

@AviAvni

AviAvni Jun 10, 2016

Contributor

thanks @dsyme I'll debug this on sunday night

This comment has been minimized.

@dsyme

dsyme Jun 10, 2016

Contributor

I think the basic thing is that you need to set the new value in TcEnv back to None when the named "let" binding you are moving into isn't a member or function.

@dsyme

dsyme Jun 10, 2016

Contributor

I think the basic thing is that you need to set the new value in TcEnv back to None when the named "let" binding you are moving into isn't a member or function.

This comment has been minimized.

@latkin

latkin Jun 10, 2016

Contributor

It needs to be laid out in the RFC, but the behavior is that "top-level" bindings (methods, module-level and class-level functions, module-level and class-level values) will be captured. Sub-level bindings ("local variable" let bindings, nested functions) are not captured.

So for your example (we do have tests for this kind of thing), let's say it's part of a module:

module Test
let f () = 
    let x = 
        let g () = MyTy.GetCallerMemberName() 
        g()
    x

I would expect f to be the result.

I think one can defend all sorts of possible positions, but this was the most natural result when we coded it, and it seems to make sense to me. The name "member" hints at something primarily associated with the type or module. Is a local variable or a nested function a "member" of the type? I'd say no.

@latkin

latkin Jun 10, 2016

Contributor

It needs to be laid out in the RFC, but the behavior is that "top-level" bindings (methods, module-level and class-level functions, module-level and class-level values) will be captured. Sub-level bindings ("local variable" let bindings, nested functions) are not captured.

So for your example (we do have tests for this kind of thing), let's say it's part of a module:

module Test
let f () = 
    let x = 
        let g () = MyTy.GetCallerMemberName() 
        g()
    x

I would expect f to be the result.

I think one can defend all sorts of possible positions, but this was the most natural result when we coded it, and it seems to make sense to me. The name "member" hints at something primarily associated with the type or module. Is a local variable or a nested function a "member" of the type? I'd say no.

This comment has been minimized.

@dsyme

dsyme Jun 10, 2016

Contributor

I see. There's logic to that for sure.

What about

  • "do" or other top-level side effect within a module
  • "static do" within a class type

I glanced again just now and didn't spot tests for those

@dsyme

dsyme Jun 10, 2016

Contributor

I see. There's logic to that for sure.

What about

  • "do" or other top-level side effect within a module
  • "static do" within a class type

I glanced again just now and didn't spot tests for those

This comment has been minimized.

@dsyme

dsyme Jun 10, 2016

Contributor

Please also update the RFC with this info, thanks

@dsyme

dsyme Jun 10, 2016

Contributor

Please also update the RFC with this info, thanks

This comment has been minimized.

@latkin

latkin Jun 10, 2016

Contributor

do in a primary ctor is covered (returns ".ctor") https://github.com/Microsoft/visualfsharp/pull/1114/files#diff-7e9f3ca9172a26b1556a899077a98bfcR22

Not with explicit do keyword, but top-level effect in module is covered here: https://github.com/Microsoft/visualfsharp/pull/1114/files#diff-7e9f3ca9172a26b1556a899077a98bfcR111 (returns ".cctor" which is defensible, though not ideal. Returning the module name would be better IMO)

Explicit static do should be added. All of this definitely needs to be added to RFC.

Static

@latkin

latkin Jun 10, 2016

Contributor

do in a primary ctor is covered (returns ".ctor") https://github.com/Microsoft/visualfsharp/pull/1114/files#diff-7e9f3ca9172a26b1556a899077a98bfcR22

Not with explicit do keyword, but top-level effect in module is covered here: https://github.com/Microsoft/visualfsharp/pull/1114/files#diff-7e9f3ca9172a26b1556a899077a98bfcR111 (returns ".cctor" which is defensible, though not ideal. Returning the module name would be better IMO)

Explicit static do should be added. All of this definitely needs to be added to RFC.

Static

This comment has been minimized.

@AviAvni

AviAvni Jun 13, 2016

Contributor

@dsyme I changed the tests in the previous one the asyncVal was member of MyTy and because of that the value was "asyncVa" now the asyncVal is value of main and the expected value is "main"

@AviAvni

AviAvni Jun 13, 2016

Contributor

@dsyme I changed the tests in the previous one the asyncVal was member of MyTy and because of that the value was "asyncVa" now the asyncVal is value of main and the expected value is "main"

@dsyme dsyme changed the title from WIP - add support for caller info attributes to F# RFC FS-1012 - Support for caller info argument attributes (CallerLineNumber, CallerFileName, CallerMemberName) Jun 10, 2016

@dsyme dsyme changed the title from F# RFC FS-1012 - Support for caller info argument attributes (CallerLineNumber, CallerFileName, CallerMemberName) to RFC FS-1012 - Support for caller info argument attributes (CallerLineNumber, CallerFileName, CallerMemberName) Jun 11, 2016

@AviAvni

This comment has been minimized.

Show comment
Hide comment
@AviAvni

AviAvni Jun 13, 2016

Contributor

@dsyme @latkin I added a static do test but currently it's faild because the expected value is .cctor but the actual value is .ctor I tried to debug this but didn't found how to decide if the ClassLetBinding is static any idea?

Contributor

AviAvni commented Jun 13, 2016

@dsyme @latkin I added a static do test but currently it's faild because the expected value is .cctor but the actual value is .ctor I tried to debug this but didn't found how to decide if the ClassLetBinding is static any idea?

@AviAvni

This comment has been minimized.

Show comment
Hide comment
@AviAvni

AviAvni Jun 16, 2016

Contributor

@dsyme any Idea about the static do?

Contributor

AviAvni commented Jun 16, 2016

@dsyme any Idea about the static do?

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 16, 2016

Contributor

@AviAvni I believe memberFlags.IsInstance should indicate instance v. static in this case.

Contributor

dsyme commented Jun 16, 2016

@AviAvni I believe memberFlags.IsInstance should indicate instance v. static in this case.

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 16, 2016

Contributor

@AviAvni You'll need to update this branch after the integration of #1132 . Let me know if you have trouble doing that.

Contributor

dsyme commented Jun 16, 2016

@AviAvni You'll need to update this branch after the integration of #1132 . Let me know if you have trouble doing that.

@AviAvni

This comment has been minimized.

Show comment
Hide comment
@AviAvni

AviAvni Jun 16, 2016

Contributor

@dsyme I tried that but memberFlags is null in case of static do or do

Contributor

AviAvni commented Jun 16, 2016

@dsyme I tried that but memberFlags is null in case of static do or do

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 16, 2016

Contributor

I think you may need to add a flag to ClassLetBinding and set it to true/false (or isStatic) at each callsite. I took a quick look and it seems plausible to do that

Contributor

dsyme commented Jun 16, 2016

I think you may need to add a flag to ClassLetBinding and set it to true/false (or isStatic) at each callsite. I took a quick look and it seems plausible to do that

@AviAvni

This comment has been minimized.

Show comment
Hide comment
@AviAvni

AviAvni Jun 16, 2016

Contributor

@dsyme OK I added the flag to ClassLetBinding now I'm building and testing then I'll do the merge thanks

Contributor

AviAvni commented Jun 16, 2016

@dsyme OK I added the flag to ClassLetBinding now I'm building and testing then I'll do the merge thanks

AviAvni added some commits Jun 16, 2016

@AviAvni

This comment has been minimized.

Show comment
Hide comment
@AviAvni

AviAvni Jun 16, 2016

Contributor

@dsyme the build pass now and the static do test also pass

Contributor

AviAvni commented Jun 16, 2016

@dsyme the build pass now and the static do test also pass

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 16, 2016

Contributor

Great. @KevinRansom I think this is now ready to merge!

Contributor

dsyme commented Jun 16, 2016

Great. @KevinRansom I think this is now ready to merge!

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 20, 2016

Contributor

We have a test failure here in "TestFunction10.il" code generation test. It seems an odd failure to suddenly start happening.

Contributor

dsyme commented Jun 20, 2016

We have a test failure here in "TestFunction10.il" code generation test. It seems an odd failure to suddenly start happening.

@dsyme dsyme closed this Jun 20, 2016

@dsyme dsyme reopened this Jun 20, 2016

@dsyme

This comment has been minimized.

Show comment
Hide comment
@dsyme

dsyme Jun 22, 2016

Contributor

This is ready to merge

Contributor

dsyme commented Jun 22, 2016

This is ready to merge

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Jun 24, 2016

Contributor

@AviAvni @latkin Thank you for taking care of this.

Kevin

Contributor

KevinRansom commented Jun 24, 2016

@AviAvni @latkin Thank you for taking care of this.

Kevin

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Jun 24, 2016

Contributor

@AviAvni @latkin
Guys, I see this error, running tests, am I missing something?

+++ Conformance\SpecialAttributesAndTypes\Imported\CallerInfo (W_CallerMemberName.fs) +++
Microsoft (R) F# Compiler version (private)
Copyright (c) Microsoft Corporation. All Rights Reserved.
(7,41-7,57): warning FS3205: The CallerMemberNameAttribute applied to parameter 'name' will have no effect. It is overridden by the CallerFilePathAttribute.
(1,81-1,81): warning FS0988: Main module of program is empty: nothing will happen when it is run

*** The following necessary lines were never matched:
***     \(7,41-7,57\):.+warning FS3204:.+The CallerMemberNameAttribute applied to parameter 'name' will have no effect. It is overridden by the CallerFilePathAttribute.


*** The following necessary lines were incorrectly matched:

Unexpected Compiler Output
FAIL
Contributor

KevinRansom commented Jun 24, 2016

@AviAvni @latkin
Guys, I see this error, running tests, am I missing something?

+++ Conformance\SpecialAttributesAndTypes\Imported\CallerInfo (W_CallerMemberName.fs) +++
Microsoft (R) F# Compiler version (private)
Copyright (c) Microsoft Corporation. All Rights Reserved.
(7,41-7,57): warning FS3205: The CallerMemberNameAttribute applied to parameter 'name' will have no effect. It is overridden by the CallerFilePathAttribute.
(1,81-1,81): warning FS0988: Main module of program is empty: nothing will happen when it is run

*** The following necessary lines were never matched:
***     \(7,41-7,57\):.+warning FS3204:.+The CallerMemberNameAttribute applied to parameter 'name' will have no effect. It is overridden by the CallerFilePathAttribute.


*** The following necessary lines were incorrectly matched:

Unexpected Compiler Output
FAIL
@latkin

This comment has been minimized.

Show comment
Hide comment
@latkin

latkin Jun 24, 2016

Contributor

@KevinRansom it's a mismatch of warning number. Are you fully synced to the latest commit? The PR adds warning 3204, but your output above shows warning 3205 (that number was used in older commit).

Contributor

latkin commented Jun 24, 2016

@KevinRansom it's a mismatch of warning number. Are you fully synced to the latest commit? The PR adds warning 3204, but your output above shows warning 3205 (that number was used in older commit).

@KevinRansom

This comment has been minimized.

Show comment
Hide comment
@KevinRansom

KevinRansom Jun 25, 2016

Contributor

Ahh merge thingy, 3204 has now been used ... thanks I will take care of it.

Contributor

KevinRansom commented Jun 25, 2016

Ahh merge thingy, 3204 has now been used ... thanks I will take care of it.

@forki

This comment has been minimized.

Show comment
Hide comment
@forki

forki Jun 25, 2016

Contributor

Yeah these new compiler error numbers are always a logical merge conflict.
On Jun 25, 2016 10:15 AM, "Kevin Ransom (msft)" notifications@github.com
wrote:

Ahh merge thingy, 3204 has now been used ... thanks I will take care of it.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1114 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AADgNJPAA3XHKlRQeJarjIO0ZLTmAhPyks5qPOOhgaJpZM4IPENL
.

Contributor

forki commented Jun 25, 2016

Yeah these new compiler error numbers are always a logical merge conflict.
On Jun 25, 2016 10:15 AM, "Kevin Ransom (msft)" notifications@github.com
wrote:

Ahh merge thingy, 3204 has now been used ... thanks I will take care of it.


You are receiving this because you commented.
Reply to this email directly, view it on GitHub
#1114 (comment),
or mute the thread
https://github.com/notifications/unsubscribe/AADgNJPAA3XHKlRQeJarjIO0ZLTmAhPyks5qPOOhgaJpZM4IPENL
.

@KevinRansom KevinRansom merged commit 0756750 into Microsoft:master Jun 25, 2016

4 checks passed

Windows_NT Debug Build Build finished.
Details
Windows_NT Release_ci_part1 Build Build finished.
Details
Windows_NT Release_ci_part2 Build Build finished.
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment