gemini-code-assist[bot] commented on code in PR #19722:
URL: https://github.com/apache/tvm/pull/19722#discussion_r3391862725
##########
src/relax/analysis/well_formed.cc:
##########
@@ -134,9 +140,48 @@ class WellFormedChecker : public relax::ExprVisitor,
kMatchVarDef
};
- void Malformed(Diagnostic diag) {
- well_formed_ = false;
- LOG(WARNING) << "This IR is not well formed: " << diag->message;
+ /*!
+ * \brief A streaming message builder that captures the offending node.
+ *
+ * Constructed via `WellFormedError(node)` at each check site; the node is
+ * recorded so `Malformed` can seed a VisitErrorContext on the thrown error
+ * (the caller resolves it to an access path). An ObjectRef node is captured;
+ * other arguments (e.g. a `Span`) are accepted and ignored.
+ */
+ class WellFormedError {
+ public:
+ explicit WellFormedError(const ffi::ObjectRef& node) : node_(node) {}
+ template <typename T>
+ explicit WellFormedError(const T&) {} // NOLINT(*) - non-ObjectRef arg
ignored
+
+ template <typename T>
+ WellFormedError& operator<<(const T& val) { // NOLINT(*)
+ stream_ << val;
+ return *this;
+ }
+
+ std::string message() const { return stream_.str(); }
+ const ffi::Optional<ffi::ObjectRef>& node() const { return node_; }
+
+ private:
+ ffi::Optional<ffi::ObjectRef> node_;
+ std::stringstream stream_;
+ };
+
+ // Throw on the first malformedness. Seed a VisitErrorContext from the
+ // offending node so the caller can resolve an access path.
+ [[noreturn]] void Malformed(const WellFormedError& err) {
+ std::optional<ffi::ObjectRef> extra_context;
+ if (err.node().has_value()) {
+ extra_context = ffi::details::MakeVisitErrorContext(err.node().value());
+ }
+ const TVMFFIByteArray* backtrace = TVMFFIBacktrace(__FILE__, __LINE__, "",
0);
+ throw ffi::Error("ValueError", err.message(), std::string(backtrace->data,
backtrace->size),
+ std::nullopt, std::move(extra_context));
+ }
Review Comment:
The `Malformed` function hardcodes the error type to `ValueError`. While
this is a reasonable default, many of the checks within `WellFormedChecker`
could benefit from more specific error types, such as `TypeError` for struct
info mismatches.
Consider extending the `WellFormedError` helper class to accept an error
kind, which can then be passed to the `ffi::Error` constructor. This would
align with the practice in other parts of the codebase where
`TVM_FFI_VISIT_THROW` is used with specific error types, enhancing the
precision of diagnostics.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
