Skip to content

Make SemanticModel's Analyze*Flow methods public. Remove corresponding extension methods. #187

New issue
New issue
Closed
Closed
Make SemanticModel's Analyze*Flow methods public. Remove corresponding extension methods.#187
Labels
Area-CompilersConcept-APIThis issue involves adding, removing, clarification, or modification of an API.Feature RequestResolution-By DesignThe behavior reported in the issue matches the current designVerified
@JoshVarty

Description

@JoshVarty
JoshVarty
opened on Jan 31, 2015

Both AnalyzeControlFlow and AnalyzeDataFlow are marked internal within SemanticModel.

They're exposed via extension methods in ModelExtensions.AnalyzeControlFlow and ModelExtension.AnalyzeDataFlow

I don't think accessing these methods through extension methods provides any value. I propose the extension methods be removed (for the above methods, and their other overload) and we simply mark the corresponding methods within SemanticModel as public.

If the team agrees, I can go ahead and submit a PR for this.

Activity

JoshVarty
changed the title [-]Make SemanticModel 's Analyze*Flow methods public. Remove extension methods.[/-] [+]Make SemanticModel's Analyze*Flow methods public. Remove extension methods.[/+] on Jan 31, 2015
JoshVarty
changed the title [-]Make SemanticModel's Analyze*Flow methods public. Remove extension methods.[/-] [+]Make SemanticModel's Analyze*Flow methods public. Remove corresponding extension methods.[/+] on Jan 31, 2015
jmarolf
added
Concept-APIThis issue involves adding, removing, clarification, or modification of an API.
Area-Compilers
Enhancement
help wantedThe issue is "up for grabs" - add a comment if you are interested in working on it
on Jan 31, 2015
jmarolf

jmarolf commented on Jan 31, 2015

@jmarolf
jmarolf
on Jan 31, 2015
Contributor

This looks like a mistake from when we unified the interface ISemanticModel to the abstract class SemanticModel. Go ahead and submit a PR.

gafter

gafter commented on Jan 31, 2015

@gafter
gafter
on Jan 31, 2015
Member

If I recall, the language-specific extension methods are type safe (e.g. Taking language-specific StatementSyntax)

JoshVarty

JoshVarty commented on Jan 31, 2015

@JoshVarty
JoshVarty
on Jan 31, 2015
ContributorAuthor

There are language-specific extension methods and they should be left alone.

But I don't think that the language-agnostic extension methods serve a purpose that wouldn't be achieved by making the following methods public instead of internal.

SemanticModel.AnalyzeControlFlow(SyntaxNode)
SemanticModel.AnalyzeControlFlow(SyntaxNode, SyntaxNode)
SemanticModel.AnalyzeDataFlow(SyntaxNode)
SemanticModel.AnalyzeDataFlow(SyntaxNode, SyntaxNode)
JoshVarty
mentioned this on Jan 31, 2015
DustinCampbell

DustinCampbell commented on Feb 4, 2015

@DustinCampbell
DustinCampbell
on Feb 4, 2015
Member

That seems fair to me.

gafter
self-assigned this
on Feb 6, 2015
JoshVarty

JoshVarty commented on Feb 6, 2015

@JoshVarty
JoshVarty
on Feb 6, 2015
ContributorAuthor

As discussed in #190 this is by design.

JoshVarty
closed this as completedon Feb 6, 2015
gafter
added
Resolution-By DesignThe behavior reported in the issue matches the current design
4 - In ReviewA fix for the issue is submitted for review.
and removed
help wantedThe issue is "up for grabs" - add a comment if you are interested in working on it
4 - In ReviewA fix for the issue is submitted for review.
on Feb 6, 2015
gafter
added
Verified
on Feb 18, 2015
jaredpar
unassigned
gafter
on Nov 23, 2015
jaredpar
added
Feature Request
and removed
Enhancement
on Nov 23, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    Area-CompilersConcept-APIThis issue involves adding, removing, clarification, or modification of an API.Feature RequestResolution-By DesignThe behavior reported in the issue matches the current designVerified

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

      Development

      No branches or pull requests

        Participants

        @DustinCampbell@jaredpar@JoshVarty@gafter@jmarolf

        Issue actions
          Make SemanticModel's Analyze*Flow methods public. Remove corresponding extension methods. · Issue #187 · dotnet/roslyn