Skip to content

Conversation

@LLLXXXCCC
Copy link

No description provided.

results.Add(methodSymbol, new HashSet<ISymbol>());
Debug.Assert(callGraph.Keys.Contains(methodSymbol), methodSymbol.Name + "was not present in the dictionary.");

foreach (var child in callGraph[methodSymbol].Keys)
Copy link

@dotpaul dotpaul Jul 19, 2019

Choose a reason for hiding this comment

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

Instead of just a Debug.Assert(), we could also gracefully handle missing keys in retail builds.

if (!callGraph.TryGetValue(methodSymbol, out var calledMethods))
{
    Debug.Fail(methodSymbol.Name + " was not found in callGraph");
    return;
}

foreach (var child in calledMethods.Keys)

Choose a reason for hiding this comment

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

+1

results[methodSymbol].UnionWith(results[child]);
}
FindCalledDangerousMethod(child, visited, results);
Debug.Assert(results.Keys.Contains(child), child.Name + "was not present in the dictionary.");
Copy link

Choose a reason for hiding this comment

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

Debug.Assert(results.Keys.Contains(child), child.Name + "was not present in the dictionary."); [](start = 32, length = 94)

Same thing here.

return;
}

results[methodSymbol].UnionWith(result);
Copy link

Choose a reason for hiding this comment

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

I think here, instead of returning, you could just ignore that particular child if it's not found:

if (results.TryGetValue(child, out var result))
{
    results[methodSymbol].UnionWith(result);
}
else
{
    Debug.Fail(child.Name + " was not found in callGraph");
}

Copy link

@dotpaul dotpaul left a comment

Choose a reason for hiding this comment

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

🚢

@dotpaul dotpaul merged commit eac34eb into dotnet:master Jul 22, 2019
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.

4 participants