Skip to content

Commit

Permalink
Fix PyCLIF-C-API codegen bug for @extend @classmethod (b/334753945).
Browse files Browse the repository at this point in the history
Support for `@extend` `@classmethod` was added with cl/331177628. The bug fixed by this CL was introduced with cl/333112933 in Sep 2020.

Manual testing using this command:

```
blaze test --config=asan third_party/clif/testing/python:extend_classmethods_test
```

* With the new test code but WITHOUT the change in clif/python/gen.py: http://sponge2/c9071f9f-d4df-4e42-88a8-02619b814418 (failed)

```
SUMMARY: UndefinedBehaviorSanitizer: signed-integer-overflow third_party/clif/testing/python/extend_classmethods_clif_aux.h:22:29
```

* With this CL as mailed: http://sponge2/4e335657-ff83-4300-ae4d-0b68003346e5 (passed)

* With `--//devtools/clif/python/codegen_mode=pybind11`: http://sponge2/f765d23c-caed-4402-921c-eae64b122068 (passed)

PiperOrigin-RevId: 625564244
  • Loading branch information
rwgk committed Apr 17, 2024
1 parent f2903d6 commit 4a15608
Show file tree
Hide file tree
Showing 4 changed files with 28 additions and 5 deletions.
4 changes: 2 additions & 2 deletions clif/python/gen.py
Original file line number Diff line number Diff line change
Expand Up @@ -878,8 +878,8 @@ def YieldCheckNullptr(ii):
else:
num_params = n
# extended methods need to include `self` as the first parameter, but
# extended constructors do not.
if func_ast.is_extend_method:
# extended classmethods (and constructors) do not.
if func_ast.is_extend_method and not func_ast.classmethod:
num_params += 1
call_with_params = call + astutils.TupleStr(params[:num_params])
yield I+I+'%s; break;' % call_with_params
Expand Down
6 changes: 6 additions & 0 deletions clif/testing/python/extend_classmethods.clif
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,12 @@ from "clif/testing/python/extend_classmethods_clif_aux.h":
@classmethod
def set_static_value(cls, v: int)

@extend
@classmethod
def function_with_defaults(cls, i: int = default,
j: int = default,
k: int = default) -> int

class TestNestedClassmethod:
class Inner:
@extend
Expand Down
5 changes: 5 additions & 0 deletions clif/testing/python/extend_classmethods_clif_aux.h
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,11 @@ inline void Abc__extend__set_static_value(int v) {
Abc::static_value = v;
}

inline int Abc__extend__function_with_defaults(int i = 3, int j = 5,
int k = 7) {
return (Abc::static_value * i + j) * k;
}

inline int TestNestedClassmethod_Inner__extend__get_static_value() {
return 472;
}
Expand Down
18 changes: 15 additions & 3 deletions clif/testing/python/extend_classmethods_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -19,15 +19,27 @@

class ExtendClassmethodsTest(absltest.TestCase):

def setUp(self):
super().setUp()
extend_classmethods.Abc.set_static_value(5432)

def test_create_from_value(self):
expected_value = 543
abc = extend_classmethods.Abc.from_value(expected_value)
self.assertEqual(abc.get_value(), expected_value)

def test_set_class_variable(self):
expected_value = 5432
extend_classmethods.Abc.set_static_value(expected_value)
self.assertEqual(extend_classmethods.Abc.get_static_value(), expected_value)
self.assertEqual(extend_classmethods.Abc.get_static_value(), 5432)

def test_function_with_defaults(self):
fwd = extend_classmethods.Abc.function_with_defaults
self.assertEqual(fwd(), (5432 * 3 + 5) * 7)
self.assertEqual(fwd(2), (5432 * 2 + 5) * 7)
self.assertEqual(fwd(2, 6), (5432 * 2 + 6) * 7)
self.assertEqual(fwd(2, 6, 8), (5432 * 2 + 6) * 8)
self.assertEqual(fwd(i=11), (5432 * 11 + 5) * 7)
self.assertEqual(fwd(j=13), (5432 * 3 + 13) * 7)
self.assertEqual(fwd(k=15), (5432 * 3 + 5) * 15)

def test_nested_classmethod(self):
v = extend_classmethods.TestNestedClassmethod.Inner.get_static_value()
Expand Down

0 comments on commit 4a15608

Please sign in to comment.