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

Remove analyzer workarounds for using declarations #39324

Closed
wants to merge 1 commit into from

Conversation

chsienki
Copy link
Contributor

Remove workarounds introduced introduced in #36734 as #32100 is now implemented.

@@ -125,13 +125,6 @@ protected override void InitializeWorker(AnalysisContext context)
ComputeDiagnostics(disposeDataAtExit, notDisposedDiagnostics, mayBeNotDisposedDiagnostics,
disposeAnalysisResult, pointsToAnalysisResult);

if (disposeAnalysisResult.ControlFlowGraph.OriginalOperation.HasAnyOperationDescendant(o => o.Kind == OperationKind.None))
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @chsienki. On second thought, I am wondering if we should leave these checks in place to future proof the analyzers from false positives in case of future language features with unimplemented IOperation support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did wonder that too. Perhaps we should Debug.Assert? Given that we shouldn't have none operations in reality, it will stop anything breaking, but save as a catch for future language features?

Copy link
Contributor

Choose a reason for hiding this comment

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

Unfortunately, we do have bunch of nodes generated with OperationKind.None due to pending IOperation support (attributes, query clauses, unsafe code, VB xml literals, etc.)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah ok. I guess we'll just leave this as-is then.

@chsienki chsienki closed this Oct 16, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants