-
Notifications
You must be signed in to change notification settings - Fork 1.6k
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
C#: Correctly parse operator names in MaD #14678
Conversation
0ddfa1d
to
4a4b924
Compare
4a4b924
to
3e3ea51
Compare
/** | ||
* Holds if a source model exists for the given parameters. | ||
*/ | ||
extensible predicate sourceModel( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could we keep this in the ExternalFlowExtensions
file instead - to make sure this is streamlined with other languages (for java - the model related predicates in ExternalFlow
are not just aliases)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That means those predicates are exposed; do we really want/need that?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe then move the entire ExternalFlowExtensions file as well (also for Java)?
pragma[nomagic] | ||
private predicate callableSpecInfo(Callable c, string namespace, string type, string name) { | ||
c.getDeclaringType().hasQualifiedName(namespace, type) and | ||
c.getName() = name | ||
hasName(c, name) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🙀 So was it the case, that even though the models existed they were not applied correctly to the operator?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
We were incorrectly using the operator name (e.g.
+
) instead of the operator function name (e.g.op_Addition
).