Skip to content

Java/C++/C#: Add DataFlowErasedType class#2396

Merged
semmle-qlci merged 4 commits intogithub:masterfrom
hvitved:dataflow/erased-type-class
Nov 25, 2019
Merged

Java/C++/C#: Add DataFlowErasedType class#2396
semmle-qlci merged 4 commits intogithub:masterfrom
hvitved:dataflow/erased-type-class

Conversation

@hvitved
Copy link
Contributor

@hvitved hvitved commented Nov 20, 2019

This PR adds a new parameter, DataFlowErasedType, to the implicit signature of the shared data flow implementation. The motivation is to be able to distinguish erased types from normal types: currently this is not done anywhere, but it will be in the C# implementation of type pruning.

@hvitved hvitved requested review from aschackmull and jbj November 20, 2019 13:20
@hvitved hvitved requested review from a team as code owners November 20, 2019 13:20
@hvitved hvitved removed the request for review from a team November 20, 2019 13:21
@aschackmull
Copy link
Contributor

AFAICS DataFlowType is then no longer used? Then I think it'll be cleaner to just reuse DataFlowType to mean what DataFlowErasedType currently means. If you then want to use distinct names in the C# implementation you should have both DataFlowType and DotNet::Type available to you.

@aschackmull
Copy link
Contributor

Perhaps we could even get rid of getErasedRepr in the implicit signature and replace it with DataFlowType getDataFlowType(Node n) { getErasedRepr(n.getType()) }

@hvitved
Copy link
Contributor Author

hvitved commented Nov 25, 2019

AFAICS DataFlowType is then no longer used? Then I think it'll be cleaner to just reuse DataFlowType to mean what DataFlowErasedType currently means. If you then want to use distinct names in the C# implementation you should have both DataFlowType and DotNet::Type available to you.

You are right; I have changed DataFlowErasedType back to DataFlowType.

@semmle-qlci semmle-qlci merged commit d58a6b0 into github:master Nov 25, 2019
@hvitved hvitved deleted the dataflow/erased-type-class branch November 25, 2019 15:22
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.

4 participants