Python: add test for Argument[0, self, self:] for instance methods
#16155
Conversation
github-actions
bot
added
Python
and removed
no-change-note-required
| SINK_F(y) | ||
|
|
||
| ms = c.explicit_self | ||
| SINK(ms(SOURCE)) # $ MISSING: flow="SOURCE, l:0 -> ms(SOURCE)" |
There was a problem hiding this comment.
Should this be marked as MISSING or as a SINK_F? I.e. do we want/expect Argument[self:] to work here? (Or anywhere..)
| m = c.instance_method | ||
| x, y = (SOURCE, NONSOURCE) | ||
| SINK(x) # $ flow="SOURCE, l:-1 -> x" | ||
| SINK_F(y) |
There was a problem hiding this comment.
you didn't use m here, which seems odd.
| ms = c.explicit_self | ||
| SINK(ms(SOURCE)) # $ MISSING: flow="SOURCE, l:0 -> ms(SOURCE)" |
There was a problem hiding this comment.
I don't understand this test. You've added a flow summary for using keyword argument named self, but are passing in a positional argument...
That is, it's totally legal to do this:
class Foo:
def method(s, *, self):
# a method where you MUST pass in keyword argument `self`
....
I also don't quite understand why you've made a use of a bound-method, instead of just calling the method on the same line? that is, why not just rewrite the test to this?
| ms = c.explicit_self | |
| SINK(ms(SOURCE)) # $ MISSING: flow="SOURCE, l:0 -> ms(SOURCE)" | |
| SINK(c.explicit_self(SOURCE)) # $ MISSING: flow="SOURCE, l:0 -> ms(SOURCE)" |
There was a problem hiding this comment.
That was to expose the self argument. The test is testing whether Argument[self:] resolves to that (and the test above does the same for Argument[self]).
But I did not manage to trigger that calling convention; I fixed that now..
There was a problem hiding this comment.
I think you need to use c.explicit_self(self=SOURCE)... and then it's a test that even on an instance method you can use keyword arguments called self.
I don't think ☝️ is what you intended, but I think what I'm outlining above is the correct behavior MaD should exhibit
There was a problem hiding this comment.
That makes sense. And the tests agree too. I have added some comments to explain what is going on.
The way to expose the `self` arguemnt is to call an instance method on the class, not on the instance...
also add comments
yoff
force-pushed
the
python/MaD-method-arg-tests
branch
from
029804c to
3716b8c
Compare
|
Force pushed since conflicts were resolved cleanly by |
No description provided.