chapel-lang / chapel

a Productive Parallel Programming Language
https://chapel-lang.org
Other
1.8k stars 421 forks source link

chapel-py: check for invalid instance of bridged classes to avoid internal errors #26309

Closed DanilaFe closed 3 days ago

DanilaFe commented 3 days ago

Closes https://github.com/chapel-lang/chapel/issues/25937.

This PR improves the situation in which AST or other bridged objects are incorrectly constructed (specifically, when they are constructed explicitly by the user). In this case, their internal representations can contain null values, which causes exception. This PR introduces checking for such cases, raising Python exceptions.

Now, none of the following operations produce internal errors, and raise instead:

Expand console output ```console >>> import chapel >>> c = chapel.Context() >>> id = chapel.Identifier(c) >>> id.unique_id() Traceback (most recent call last): File "", line 1, in id.unique_id() ~~~~~~~~~~~~^^ RuntimeError: invalid instance of class 'AstNode'; please do not manually construct instances of this class. >>> for node in id: ... print(node) ... Traceback (most recent call last): File "", line 1, in for node in id: ^^ RuntimeError: invalid instance of class 'AstNode'; please do not manually construct instances of this class. >>> tmp = BoolParam() Traceback (most recent call last): File "", line 1, in tmp = BoolParam() ^^^^^^^^^ NameError: name 'BoolParam' is not defined >>> tmp = chapel.BoolParam() Traceback (most recent call last): File "", line 1, in tmp = chapel.BoolParam() TypeError: function takes exactly 1 argument (0 given) >>> tmp = chapel.BoolParam(c) >>> print(tmp) Traceback (most recent call last): File "", line 1, in print(tmp) ~~~~~^^^^^ RuntimeError: invalid instance of class 'Param'; please do not manually construct instances of this class. ```

The main change in this PR is that, to signal an incorrectly-constructed type, we need an "empty value". This has to happen for all objects in method-tables.h, because all those methods are generated using the same body. Some objects defined in method-tables.h contain non-pointer values and would return them by reference; however, references cannot be used to signal an empty value, and cannot be stored in std::optional. Thus, this PR switches the implementation to always return pointers from .unwrap(), using nullptr to signal failure. This requires an SFINAE-based re-implementation of unwrap for PythonClass and subtypes. The generated types do not use PythonClass, and always use pointers, so their implementation is only adjusted to raise.

Reviewed by @jabraham17 -- thanks!

Testing