Skip to content
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

Support BoundUsingLocalDeclarations in IOperation and CFG #32100

Closed
chsienki opened this issue Jan 2, 2019 · 7 comments · Fixed by #39125
Closed

Support BoundUsingLocalDeclarations in IOperation and CFG #32100

chsienki opened this issue Jan 2, 2019 · 7 comments · Fixed by #39125
Assignees
Labels
Area-Compilers Bug Developer Community The issue was originally reported on https://developercommunity.visualstudio.com Feature - Flow Analysis Flow Analysis Feature - IOperation IOperation New Language Feature - enhanced using Using pattern and declaration Urgency-Soon
Milestone

Comments

@chsienki
Copy link
Contributor

chsienki commented Jan 2, 2019

BoundUsingLocalDeclarations are currently implemented as OperationKind.None, and return the List of local declarations as its children. We should implement this correctly with an Operation node.

We should also support CFG correctly with using declarations. Currently we report the using declaration as OperationKind.None in a single block, and thus don't report the actual underlying block structure generated by lowering.

[jcouv update:] When fixing this, let's also verify IDE behavior got fixed. For example #36502

@chsienki chsienki self-assigned this Jan 2, 2019
@chsienki chsienki changed the title Support BoundUsingLocalDeclarations in IOperation Support BoundUsingLocalDeclarations in IOperation and CFG Jan 10, 2019
@jcouv
Copy link
Member

jcouv commented Feb 19, 2019

Once fixed, please verify this scenario (copied from #33464):

Using-declaration should not trigger RemoveUnusedValues.

I suspect this is a compiler issue, as the AbstractRemoveUnusedParametersAndValuesDiagnosticAnalyzer relies on the control flow graph to determine how variables are used.

image

image


We should also test ExtractMethod on a using-declaration (tracked by this issue)

@yyjdelete
Copy link

yyjdelete commented Feb 25, 2019

Maybe the same as #33464.
An quick fix Inline temporary variable should not be provided for the first x1.

using var x1 = File.OpenText("test");
var x2 = x1.ReadToEnd();

Expect: Inline temporary variable should not be provided.
Actual:

var x2 = File.OpenText("test").ReadToEnd();

[jcouv update:] Yes. Also we can close this issue when we've fixed/verified the InlineVariable scenario.

@mavasani
Copy link
Contributor

#36502 - another report where IOperation/CFG based analyses in IDE is falling over due to this issue. There are now at least 4 known IDE features that give false results due to this missing support.

@jcouv jcouv added this to the 16.3 milestone Jun 17, 2019
@jinujoseph jinujoseph added the Developer Community The issue was originally reported on https://developercommunity.visualstudio.com label Jun 24, 2019
@jinujoseph
Copy link
Contributor

also reported here

@mavasani
Copy link
Contributor

Adding more context to @jinujoseph's comment: We have added new CFG/dataflow analysis based dispose analyzers in the IDE in 16.2, and they generate false positives due to this missing support:

using System.Diagnostics;

public class Class1
{
    public void M()
    {
        using Process _ = new Process  // Dispose analyzer flags this object creation as not-disposed.
        {
            StartInfo = new ProcessStartInfo()
        };
    }
}

@mavasani
Copy link
Contributor

I have created #36734 to workaround this issue in couple of IDE analyzers. When the IOperation/CFG support has been added for using declarations, kindly revert the product workarounds in that PR and ensure the added unit tests still pass.

mavasani added a commit to mavasani/roslyn-analyzers that referenced this issue Jul 26, 2019
Workaround for missing IOperation/CFG support for using declarations: dotnet/roslyn#32100
Fixes dotnet#2703
@jcouv jcouv modified the milestones: 16.3, 16.4 Aug 28, 2019
@jaredpar jaredpar added the Bug label Sep 9, 2019
@chsienki
Copy link
Contributor Author

Fixed with #39125

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area-Compilers Bug Developer Community The issue was originally reported on https://developercommunity.visualstudio.com Feature - Flow Analysis Flow Analysis Feature - IOperation IOperation New Language Feature - enhanced using Using pattern and declaration Urgency-Soon
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants