chapel-py: check for invalid instance of bridged classes to avoid int…

…ernal errors (#26309)

Closes #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:

<details>
<summary>Expand console output</summary>

```console
>>> import chapel
>>> c = chapel.Context()
>>> id = chapel.Identifier(c)
>>> id.unique_id()
Traceback (most recent call last):
  File "<python-input-3>", line 1, in <module>
    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 "<python-input-4>", line 1, in <module>
    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 "<python-input-5>", line 1, in <module>
    tmp = BoolParam()
          ^^^^^^^^^
NameError: name 'BoolParam' is not defined
>>> tmp = chapel.BoolParam()
Traceback (most recent call last):
  File "<python-input-6>", line 1, in <module>
    tmp = chapel.BoolParam()
TypeError: function takes exactly 1 argument (0 given)
>>> tmp = chapel.BoolParam(c)
>>> print(tmp)
Traceback (most recent call last):
  File "<python-input-8>", line 1, in <module>
    print(tmp)
    ~~~~~^^^^^
RuntimeError: invalid instance of class 'Param'; please do not manually construct instances of this class.
```
</details>

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
- [x] `make test-cls` works

tools/chapel-py/src/core-types-gen.cpp

@@ -102,7 +102,8 @@ struct InvokeHelper<void(Args...)> {
  */
 #define METHOD(NODE, NAME, DOCSTR, TYPEFN, BODY)\
   PyObject* NODE##Object_##NAME(PyObject *self, PyObject *argsTup) {\
-    auto&& node = ((NODE##Object*) self)->unwrap(); \
+    auto* node = ((NODE##Object*) self)->unwrap(); \
+    if (!node) return nullptr; \
     auto contextObject = ((NODE##Object*) self)->context(); \
     auto context = &contextObject->value_; \
     auto args = PythonFnHelper<TYPEFN>::unwrapArgs(contextObject, argsTup); \
@@ -123,6 +124,7 @@ struct InvokeHelper<void(Args...)> {
 #define ACTUAL_ITERATOR(NAME)\
   PyObject* NAME##Object_actuals(PyObject *self, PyObject *Py_UNUSED(ignored)) { \
     auto node = ((NAME##Object*) self)->unwrap(); \
+    if (!node) return nullptr; \
     \
     auto argList = Py_BuildValue("(O)", (PyObject*) self); \
     auto astCallIterObjectPy = PyObject_CallObject((PyObject *) &AstCallIterType, argList); \

tools/chapel-py/src/core-types-gen.h

@@ -37,7 +37,13 @@
     static constexpr const char* Name = #NAME; \
     static constexpr const char* DocStr = "An object that represents a Chapel " #NAME; \
   \
-    const auto unwrap() const { return parent.value_->to##NAME(); } \
+    const auto unwrap() const { \
+      if (!parent.value_) { \
+        raiseExceptionForIncorrectlyConstructedType(#NAME); \
+        return static_cast<decltype(parent.value_->to##NAME())>(nullptr); \
+      } \
+      return parent.value_->to##NAME(); \
+    } \
     ContextObject* context() const { return (ContextObject*) parent.contextObject; } \
   }; \
   \

tools/chapel-py/src/core-types.cpp

@@ -146,6 +146,10 @@ std::string generatePyiFile() {
 }
 
 PyObject* AstNodeObject::iter(AstNodeObject *self) {
+  if (!self->value_) {
+    raiseExceptionForIncorrectlyConstructedType("AstNode");
+    return nullptr;
+  }
   return wrapIterPair((ContextObject*) self->contextObject, self->value_->children());
 }
 
@@ -155,13 +159,21 @@ void ChapelTypeObject_dealloc(ChapelTypeObject* self) {
 }
 
 PyObject* ChapelTypeObject::str(ChapelTypeObject* self) {
+  if (!self->value_) {
+    raiseExceptionForIncorrectlyConstructedType("Type");
+    return nullptr;
+  }
   std::stringstream ss;
   self->value_->stringify(ss, CHPL_SYNTAX);
   auto typeString = ss.str();
   return Py_BuildValue("s", typeString.c_str());
 }
 
 PyObject* ParamObject::str(ParamObject* self) {
+  if (!self->value_) {
+    raiseExceptionForIncorrectlyConstructedType("Param");
+    return nullptr;
+  }
   std::stringstream ss;
   self->value_->stringify(ss, CHPL_SYNTAX);
   auto typeString = ss.str();

tools/chapel-py/src/core-types.h

@@ -117,7 +117,7 @@ struct LocationObject : public PythonClass<LocationObject, chpl::Location> {
 
   static PyTypeObject configurePythonType() {
     // Configure the necessary methods to make inserting into sets working:
-    PyTypeObject configuring = PythonClassWithObject<LocationObject, chpl::Location>::configurePythonType();
+    PyTypeObject configuring = PythonClassWithContext<LocationObject, chpl::Location>::configurePythonType();
     configuring.tp_str = (reprfunc) str;
     configuring.tp_as_number = &location_as_number;
     configuring.tp_as_number->nb_subtract = (binaryfunc) subtract;
@@ -126,23 +126,23 @@ struct LocationObject : public PythonClass<LocationObject, chpl::Location> {
   }
 };
 
-struct ScopeObject : public PythonClassWithObject<ScopeObject, const chpl::resolution::Scope*> {
+struct ScopeObject : public PythonClassWithContext<ScopeObject, const chpl::resolution::Scope*> {
   static constexpr const char* QualifiedName = "chapel.Scope";
   static constexpr const char* Name = "Scope";
   static constexpr const char* DocStr = "A scope in the Chapel program, such as a block.";
 };
 
 using VisibleSymbol = std::tuple<chpl::UniqueString, std::vector<const chpl::uast::AstNode*>>;
 
-struct AstNodeObject : public PythonClassWithObject<AstNodeObject, const chpl::uast::AstNode*> {
+struct AstNodeObject : public PythonClassWithContext<AstNodeObject, const chpl::uast::AstNode*> {
   static constexpr const char* QualifiedName = "chapel.AstNode";
   static constexpr const char* Name = "AstNode";
   static constexpr const char* DocStr = "The base type of Chapel AST nodes";
 
   static PyObject* iter(AstNodeObject *self);
 
   static PyTypeObject configurePythonType() {
-    PyTypeObject configuring = PythonClassWithObject<AstNodeObject, const chpl::uast::AstNode*>::configurePythonType();
+    PyTypeObject configuring = PythonClassWithContext<AstNodeObject, const chpl::uast::AstNode*>::configurePythonType();
     configuring.tp_iter = (getiterfunc) AstNodeObject::iter;
     configuring.tp_flags = Py_TPFLAGS_BASETYPE;
     return configuring;
@@ -151,37 +151,37 @@ struct AstNodeObject : public PythonClassWithObject<AstNodeObject, const chpl::u
 
 using QualifiedTypeTuple = std::tuple<const char*, Nilable<const chpl::types::Type*>, Nilable<const chpl::types::Param*>>;
 
-struct ChapelTypeObject  : public PythonClassWithObject<ChapelTypeObject, const chpl::types::Type*> {
+struct ChapelTypeObject  : public PythonClassWithContext<ChapelTypeObject, const chpl::types::Type*> {
   static constexpr const char* QualifiedName = "chapel.ChapelType";
   static constexpr const char* Name = "ChapelType";
   static constexpr const char* DocStr = "The base type of Chapel types";
 
   static PyObject* str(ChapelTypeObject* self);
 
   static PyTypeObject configurePythonType() {
-    PyTypeObject configuring = PythonClassWithObject<ChapelTypeObject, const chpl::types::Type*>::configurePythonType();
+    PyTypeObject configuring = PythonClassWithContext<ChapelTypeObject, const chpl::types::Type*>::configurePythonType();
     configuring.tp_str = (reprfunc) ChapelTypeObject::str;
     configuring.tp_flags = Py_TPFLAGS_BASETYPE;
     return configuring;
   }
 };
 
-struct ParamObject : public PythonClassWithObject<ParamObject, const chpl::types::Param*> {
+struct ParamObject : public PythonClassWithContext<ParamObject, const chpl::types::Param*> {
   static constexpr const char* QualifiedName = "chapel.Param";
   static constexpr const char* Name = "Param";
   static constexpr const char* DocStr = "The base type of Chapel parameters (compile-time known values)";
 
   static PyObject* str(ParamObject* self);
 
   static PyTypeObject configurePythonType() {
-    PyTypeObject configuring = PythonClassWithObject<ParamObject, const chpl::types::Param*>::configurePythonType();
+    PyTypeObject configuring = PythonClassWithContext<ParamObject, const chpl::types::Param*>::configurePythonType();
     configuring.tp_str = (reprfunc) ParamObject::str;
     configuring.tp_flags = Py_TPFLAGS_BASETYPE;
     return configuring;
   }
 };
 
-struct ResolvedExpressionObject : public PythonClassWithObject<ResolvedExpressionObject, const chpl::resolution::ResolvedExpression*> {
+struct ResolvedExpressionObject : public PythonClassWithContext<ResolvedExpressionObject, const chpl::resolution::ResolvedExpression*> {
   static constexpr const char* QualifiedName = "chapel.ResolvedExpression";
   static constexpr const char* Name = "ResolvedExpression";
   static constexpr const char* DocStr = "Container for type information about a particular AST node.";
@@ -194,7 +194,7 @@ struct MostSpecificCandidateAndPoiScope {
   const chpl::resolution::PoiScope* poiScope;
 };
 
-struct MostSpecificCandidateObject : public PythonClassWithObject<MostSpecificCandidateObject, MostSpecificCandidateAndPoiScope> {
+struct MostSpecificCandidateObject : public PythonClassWithContext<MostSpecificCandidateObject, MostSpecificCandidateAndPoiScope> {
   static constexpr const char* QualifiedName = "chapel.MostSpecificCandidate";
   static constexpr const char* Name = "MostSpecificCandidate";
   static constexpr const char* DocStr = "A candidate function returned from call resolution that represents the most specific overload matching the call.";
@@ -207,7 +207,7 @@ struct TypedSignatureAndPoiScope {
   const chpl::resolution::PoiScope* poiScope;
 };
 
-struct TypedSignatureObject : public PythonClassWithObject<TypedSignatureObject, TypedSignatureAndPoiScope> {
+struct TypedSignatureObject : public PythonClassWithContext<TypedSignatureObject, TypedSignatureAndPoiScope> {
   static constexpr const char* QualifiedName = "chapel.TypedSignature";
   static constexpr const char* Name = "TypedSignature";
   static constexpr const char* DocStr = "The signature of a particular function. Could include types gathered when instantiating the function";
@@ -230,7 +230,7 @@ struct TypedSignatureObject : public PythonClassWithObject<TypedSignatureObject,
 
   static PyTypeObject configurePythonType() {
     // Configure the necessary methods to make inserting into sets working:
-    PyTypeObject configuring = PythonClassWithObject<TypedSignatureObject, TypedSignatureAndPoiScope>::configurePythonType();
+    PyTypeObject configuring = PythonClassWithContext<TypedSignatureObject, TypedSignatureAndPoiScope>::configurePythonType();
     configuring.tp_hash = (hashfunc) hash;
     configuring.tp_richcompare = (richcmpfunc) richcompare;
     return configuring;

tools/chapel-py/src/error-tracker.h

@@ -27,15 +27,15 @@
 
 struct ContextObject;
 
-struct ErrorObject : public PythonClassWithObject<ErrorObject, chpl::owned<chpl::ErrorBase>> {
+struct ErrorObject : public PythonClassWithContext<ErrorObject, chpl::owned<chpl::ErrorBase>> {
   static constexpr const char* QualifiedName = "chapel.Error";
   static constexpr const char* Name = "Error";
   static constexpr const char* DocStr = "An error that occurred as part of processing a file with the Chapel compiler frontend";
 };
 
 using LocationAndNote = std::tuple<chpl::Location, std::string>;
 
-struct ErrorManagerObject : public PythonClassWithObject<ErrorManagerObject, std::tuple<>> {
+struct ErrorManagerObject : public PythonClassWithContext<ErrorManagerObject, std::tuple<>> {
   static constexpr const char* QualifiedName = "chapel.ErrorManager";
   static constexpr const char* Name = "ErrorManager";
   static constexpr const char* DocStr = "A wrapper container to help track the errors from a Context.";

tools/chapel-py/src/method-tables/core-methods.h

@@ -26,7 +26,7 @@
 
 CLASS_BEGIN(Context)
   PLAIN_GETTER(Context, introspect_parsed_files, "Inspect the list of files that have been parsed by the Context",
-               std::vector<chpl::UniqueString>, return parsing::introspectParsedFiles(&node))
+               std::vector<chpl::UniqueString>, return parsing::introspectParsedFiles(node))
   PLAIN_GETTER(Context, track_errors, "Return a context manager that tracks errors emitted by this Context",
                ErrorManagerObject*, std::ignore = node; return ErrorManagerObject::create(contextObject, std::make_tuple()))
   PLAIN_GETTER(Context, _get_pyi_file, "Generate a stub file for the Chapel AST nodes",
@@ -38,7 +38,7 @@ CLASS_BEGIN(Context)
          auto fileNameUS = std::get<0>(args);
          auto parentPathUS = chpl::UniqueString();
          auto& builderResult =
-           parsing::parseFileToBuilderResultAndCheck(&node, fileNameUS, parentPathUS);
+           parsing::parseFileToBuilderResultAndCheck(node, fileNameUS, parentPathUS);
          std::vector<const chpl::uast::AstNode*> topLevelNodes;
          for (auto i = 0; i < builderResult.numTopLevelExpressions(); i++) {
            topLevelNodes.push_back(builderResult.topLevelExpression(i));
@@ -49,36 +49,36 @@ CLASS_BEGIN(Context)
 
          auto& paths = std::get<0>(args);
          auto& filenames = std::get<1>(args);
-         parsing::setupModuleSearchPaths(&node, false, false, paths, filenames))
+         parsing::setupModuleSearchPaths(node, false, false, paths, filenames))
   METHOD(Context, is_bundled_path, "Check if the given file path is within the bundled (built-in) Chapel files",
          bool(chpl::UniqueString),
 
          auto pathUS = std::get<0>(args);
-         return parsing::filePathIsInInternalModule(&node, pathUS) ||
-                parsing::filePathIsInStandardModule(&node, pathUS) ||
-                parsing::filePathIsInBundledModule(&node, pathUS))
+         return parsing::filePathIsInInternalModule(node, pathUS) ||
+                parsing::filePathIsInStandardModule(node, pathUS) ||
+                parsing::filePathIsInBundledModule(node, pathUS))
   METHOD(Context, advance_to_next_revision, "Advance the context to the next revision",
          void(bool),
 
          auto prepareToGc = std::get<0>(args);
-         node.advanceToNextRevision(prepareToGc))
+         node->advanceToNextRevision(prepareToGc))
   METHOD(Context, get_file_text, "Get the text of the file at the given path",
-         std::string(chpl::UniqueString), return parsing::fileText(&node, std::get<0>(args)).text())
+         std::string(chpl::UniqueString), return parsing::fileText(node, std::get<0>(args)).text())
 CLASS_END(Context)
 
 CLASS_BEGIN(Location)
   PLAIN_GETTER(Location, start, "Get the line-column pair where this Location starts",
-               LineColumnPair, return std::make_tuple(node.firstLine(), node.firstColumn()))
+               LineColumnPair, return std::make_tuple(node->firstLine(), node->firstColumn()))
   PLAIN_GETTER(Location, end, "Get the line-column pair where this Location ends",
-               LineColumnPair, return std::make_tuple(node.lastLine(), node.lastColumn()))
+               LineColumnPair, return std::make_tuple(node->lastLine(), node->lastColumn()))
   PLAIN_GETTER(Location, path, "Get the file path of this Location",
-               chpl::UniqueString, return node.path())
+               chpl::UniqueString, return node->path())
   METHOD(Location, clamp_left, "Get a new Location removes the left part of the current Location based on another Location",
          chpl::Location(chpl::Location),
          auto left = node;
          auto right = std::get<0>(args);
 
-         return Location(left.path(), std::max(left.start(), right.start()), left.end());
+         return Location(left->path(), std::max(left->start(), right.start()), left->end());
   )
 CLASS_END(Location)
 
@@ -123,28 +123,28 @@ CLASS_BEGIN(Error)
                std::vector<chpl::ErrorCodeSnippet>,
 
                CompatibilityWriter writer(context);
-               node->write(writer);
+               (*node)->write(writer);
                return writer.codeSnippets())
   PLAIN_GETTER(Error, location, "Get the location at which this error occurred",
-               chpl::Location, return node->location(context))
+               chpl::Location, return (*node)->location(context))
   PLAIN_GETTER(Error, message, "Retrieve the contents of this error message",
-               std::string, return node->message())
+               std::string, return (*node)->message())
   PLAIN_GETTER(Error, notes, "Get the locations and text of additional notes printed by this error",
                std::vector<LocationAndNote>,
 
                std::vector<LocationAndNote> toReturn;
                CompatibilityWriter writer(context);
-               node->write(writer);
+               (*node)->write(writer);
                for (auto& note : writer.notes()) {
                  toReturn.push_back(std::make_tuple(std::get<0>(note).computeLocation(context),
                                                     std::get<1>(note)));
                }
                return toReturn)
   PLAIN_GETTER(Error, kind, "Retrieve the kind ('error', 'warning') of this type of error",
-               const char*, return chpl::ErrorBase::getKindName(node->kind()))
+               const char*, return chpl::ErrorBase::getKindName((*node)->kind()))
   PLAIN_GETTER(Error, type, "Retrieve the unique name of this type of error",
                std::optional<const char*>,
-               const char* name = chpl::ErrorBase::getTypeName(node->type());
+               const char* name = chpl::ErrorBase::getTypeName((*node)->type());
                return name ? std::optional(name) : std::nullopt;
                )
 CLASS_END(Error)
@@ -191,7 +191,7 @@ CLASS_END(ResolvedExpression)
 
 CLASS_BEGIN(MostSpecificCandidate)
   PLAIN_GETTER(MostSpecificCandidate, function, "Get the signature of the function called by this candidate.",
-               TypedSignatureObject*, return TypedSignatureObject::create(contextObject, {node.candidate->fn(), node.poiScope }))
+               TypedSignatureObject*, return TypedSignatureObject::create(contextObject, {node->candidate->fn(), node->poiScope }))
 
   // Note: calling node.resolve().formal_actual_mapping() -- thus using the
   // below method -- should be equivalent to calling node.formal_actual_mapping().
@@ -204,7 +204,7 @@ CLASS_BEGIN(MostSpecificCandidate)
                std::vector<int> res;
 
                int i = 0;
-               while (auto formalActual = node.candidate->formalActualMap().byActualIdx(i++)) {
+               while (auto formalActual = node->candidate->formalActualMap().byActualIdx(i++)) {
                  res.push_back(formalActual->formalIdx());
                }
                return res)
@@ -215,18 +215,18 @@ CLASS_BEGIN(TypedSignature)
          std::optional<QualifiedTypeTuple>(int),
 
          auto arg = std::get<int>(args);
-         if (arg < 0 && arg >= node.signature->numFormals()) {
+         if (arg < 0 && arg >= node->signature->numFormals()) {
            return {};
          }
 
-         auto& qt = node.signature->formalType(arg);
+         auto& qt = node->signature->formalType(arg);
          if (qt.isUnknown()) {
            return {};
          }
 
          return std::make_tuple(intentToString(qt.kind()), qt.type(), qt.param()))
   PLAIN_GETTER(TypedSignature, is_instantiation, "Check if this function is an instantiation of a generic function",
-               bool, return node.signature->instantiatedFrom() != nullptr)
+               bool, return node->signature->instantiatedFrom() != nullptr)
   PLAIN_GETTER(TypedSignature, ast, "Get the AST from which this function signature is computed",
-               Nilable<const chpl::uast::AstNode*>, return chpl::parsing::idToAst(context, node.signature->id()))
+               Nilable<const chpl::uast::AstNode*>, return chpl::parsing::idToAst(context, node->signature->id()))
 CLASS_END(TypedSignature)