Go: Make DataFlowType a singleton #11697
Conversation
| | InsecureHostKeyCallbackExample.go:32:3:34:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:31:14:34:4 | type conversion : signature type | | ||
| | InsecureHostKeyCallbackExample.go:45:3:47:3 | function literal : signature type | InsecureHostKeyCallbackExample.go:52:20:52:48 | type conversion | | ||
| | InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : HostKeyCallback | InsecureHostKeyCallbackExample.go:62:20:62:27 | callback | | ||
| | InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : signature type | InsecureHostKeyCallbackExample.go:62:20:62:27 | callback | |
There was a problem hiding this comment.
Note: the above two lines become one line because they are the same when the type is removed.
| | InsecureHostKeyCallbackExample.go:52:20:52:48 | type conversion | semmle.label | type conversion | | ||
| | InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : HostKeyCallback | semmle.label | definition of callback : HostKeyCallback | | ||
| | InsecureHostKeyCallbackExample.go:58:39:58:46 | definition of callback : signature type | semmle.label | definition of callback : signature type | |
There was a problem hiding this comment.
Note: the above two lines become one line because they are the same when the type is removed.
| class DataFlowType = Type; | ||
| private newtype TDataFlowType = | ||
| TTodoDataFlowType() or | ||
| TTodoDataFlowType2() // Add a dummy value to prevent bad functionality-induced joins arising from a type of size 1. |
There was a problem hiding this comment.
I note only some other languages do this -- might as well avoid the hack if not necessary
There was a problem hiding this comment.
Those languages did need it though. I assume it wouldn't be hard to remove our hack when the other hacks are removed, if it isn't needed any more. Are you suggesting that I omit the second branch, run a performance check to see if we can get by without it, and if so then omit it and hope that we don't hit the performance problem in future?
There was a problem hiding this comment.
Basically yes, though I wouldn't object to the flow merge as-is (after the requested double-check below), then experimentally remove the hack from all languages.
Suggest also querying on slack whether the comment about singleton types still applies or has perhaps been fixed since.
There was a problem hiding this comment.
Otherwise looks good -- please script up checking the diffs are just disappearing types (suggested mechanism: hack AccessPath.toString to omit the : typeDescription suffix; run tests in --learn mode; then run again with your branch checked out. Expected result: a small number of nodes merging as you have already observed; no other changes.
|
I have done the check that you requested, and the results were as you expected. |
owen-mc
changed the title
All test result changes are due to no longer printing the type of each data flow node. I have added comments in two places where this causes one less line, because two lines become identical.