Skip to content

Commit

Permalink
Docs and explanations.
Browse files Browse the repository at this point in the history
  • Loading branch information
CyrusNajmabadi committed Sep 23, 2020
1 parent 2840836 commit 14a7234
Showing 1 changed file with 10 additions and 3 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,12 @@ private static bool CastHasRuntimeImpact(
ExpressionSyntax castNode, ExpressionSyntax castedExpressionNode,
SemanticModel semanticModel, CancellationToken cancellationToken)
{
// Look for patterns we know will never cause any runtime changes.
// Look for simple patterns we know will never cause any runtime changes.
if (CastDefinitelyHasNoRuntimeImpact(castNode, castedExpressionNode, semanticModel, cancellationToken))
return false;

return !CastHasNoRuntimeImpact(speculationAnalyzer, castNode, castedExpressionNode, semanticModel, cancellationToken);
// Call into our legacy codepath that tries to make the same determination.
return !CastHasNoRuntimeImpact_Legacy(speculationAnalyzer, castNode, castedExpressionNode, semanticModel, cancellationToken);
}

private static bool CastDefinitelyHasNoRuntimeImpact(
Expand All @@ -89,6 +90,9 @@ private static bool CastDefinitelyHasNoRuntimeImpact(
SemanticModel semanticModel,
CancellationToken cancellationToken)
{
// NOTE: Keep this method simple. Each type of runtime impact check should just be a new check added
// independently from the rest. We want to make it very clear exactly which cases each check is covering.

// castNode is: `(Type)expr` or `expr as Type`.
// castedExpressionnode is: `expr`

Expand All @@ -109,11 +113,14 @@ private static bool CastDefinitelyHasNoRuntimeImpact(
return false;
}

private static bool CastHasNoRuntimeImpact(
private static bool CastHasNoRuntimeImpact_Legacy(
SpeculationAnalyzer speculationAnalyzer,
ExpressionSyntax castNode, ExpressionSyntax castedExpressionNode,
SemanticModel semanticModel, CancellationToken cancellationToken)
{
// Note: Legacy codepaths for determining if a cast is removable. As much as possible we should attempt to
// extract simple and clearly defined checks from here and move to CastDefinitelyHasNoRuntimeImpact.

// Then look for patterns for cases where we never want to remove casts.
if (CastMustBePreserved(castNode, castedExpressionNode, semanticModel, cancellationToken))
return false;
Expand Down

0 comments on commit 14a7234

Please sign in to comment.