Skip to content

Conversation

@Jand42
Copy link
Contributor

@Jand42 Jand42 commented Oct 7, 2015

New classes are added to expose SlotSig and SlotParam typed in Symbols API. Previously there was no way to tell which abstract slot an override or an interface implementation member were using.

Also there is a small fix included to have all assembly-level attributes visible on FSharpAssemblySignature.Attributes, not just the ones declared in the last code file.

@7sharp9
Copy link
Member

7sharp9 commented Oct 7, 2015

The terminology in the symbols API is strange, "abstract slot signatures" sounds far more technical than it needs to.

@Jand42
Copy link
Contributor Author

Jand42 commented Oct 7, 2015

@7sharp9 Can you suggest some better naming? Thanks!

rojepp pushed a commit to rojepp/FSharp.Compiler.Service that referenced this pull request Oct 8, 2015
…error

fixes fsharp#362
closes fsharp#430

commit 22bf8ebf25de08905342d5984052b683479e5a65
Author: Don Syme <donsyme@fastmail.fm>
Date:   Sat May 9 12:08:27 2015 +0100

    fix 362

commit bc28158073b8605dc03a0816a1679a729595fda0
Author: Don Syme <donsyme@fastmail.fm>
Date:   Sat May 9 12:06:27 2015 +0100

    fix 362 (2)

commit 6bfdeac2c8718032be43c75c6a09ea66866bbbc5
Author: Don Syme <donsyme@fastmail.fm>
Date:   Sat May 9 11:58:48 2015 +0100

    fix 362
Copy link
Contributor

Choose a reason for hiding this comment

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

The "Value" might fail here (MemberInfo is an option). (When the value is not a member then "ImplementedSlotSigs" in the symbols API should I think return an empty list or at least give a good exception?).

@dsyme
Copy link
Contributor

dsyme commented Oct 10, 2015

This looks good. I'm surprised at the AppVeyor build failure w.r.t. SourceLink - ideally that part of the build shouldn't be being run on appveyor.

@dsyme
Copy link
Contributor

dsyme commented Oct 10, 2015

@Jand42 Could you please add tests for the abstract slot signatures? Thanks

@Jand42
Copy link
Contributor Author

Jand42 commented Oct 13, 2015

@dsyme Added some tests and fixed possibly failing .Value. Thanks for the review.

@Jand42
Copy link
Contributor Author

Jand42 commented Oct 13, 2015

Sorry for the extra commits.

dsyme added a commit that referenced this pull request Oct 13, 2015
Exposing implemented abstract slots
@dsyme dsyme merged commit 4f2ed4f into fsharp:master Oct 13, 2015
@dsyme
Copy link
Contributor

dsyme commented Oct 13, 2015

Thanks for the great contribution

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.

3 participants