tqchen commented on code in PR #19916:
URL: https://github.com/apache/tvm/pull/19916#discussion_r3498894229
##########
src/script/printer/doc_printer/base_doc_printer.cc:
##########
@@ -267,6 +267,7 @@ void DocPrinter::Append(const Doc& doc, const
PrinterConfig& cfg) {
for (const AccessPath& p : cfg->path_to_underline) {
path_to_underline_.push_back(p);
current_max_path_depth_.push_back(0);
+ current_visible_paths_.push_back(std::nullopt);
current_underline_candidates_.push_back(std::vector<ByteSpan>());
}
Review Comment:
Thanks for checking this. `ffi::Array` has a non-null default contract: its
default constructor assigns `ArrayObj::Empty()`, and
`PrinterConfigNode::path_to_underline` is default-constructed with that type.
The constructor only replaces it when a concrete array is supplied. This loop
also predates this PR; the change only adds initialization of
`current_visible_paths_`. A `.defined()` guard would therefore be permanently
true for valid `PrinterConfig` instances, so I am leaving the existing
iteration unchanged.
##########
src/script/printer/script_printer.cc:
##########
@@ -23,8 +23,36 @@
#include <tvm/script/printer/printer.h>
#include <algorithm>
+#include <optional>
+
+#include "visible_path.h"
namespace tvm {
+namespace {
+
+using AccessPath = ffi::reflection::AccessPath;
+
+ffi::Array<ffi::Optional<AccessPath>> EmptyVisiblePaths(const PrinterConfig&
cfg) {
+ ffi::Array<ffi::Optional<AccessPath>> result;
+ for (size_t i = 0; i < cfg->path_to_underline.size(); ++i) {
+ result.push_back(std::nullopt);
+ }
+ return result;
+}
Review Comment:
This is safe for the same container-contract reason: `ffi::Array()`
initializes itself to `ArrayObj::Empty()`, so an omitted `path_to_underline` is
a defined empty array. `ScriptWithVisiblePathsForPython` also materializes a
valid `PrinterConfig` with `cfg.value_or(PrinterConfig())` before calling this
helper. There is no valid null-array state for this field, and adding a
`.defined()` branch would be redundant.
##########
tests/python/tvmscript/test_tvmscript_printer_structural_equal.py:
##########
@@ -29,11 +29,58 @@ def _error_message(exception):
return str(exception)
+def _access_step_text(step):
+ kind = AccessKind(step.kind)
+ if kind == AccessKind.ATTR:
+ return f".{step.key}"
+ if kind == AccessKind.ARRAY_ITEM:
+ return f"[{step.key}]"
+ if kind == AccessKind.MAP_ITEM:
+ return f"[{step.key!r}]"
+ if kind == AccessKind.ATTR_MISSING:
+ return f"[<missing:{step.key!r}>]"
+ if kind == AccessKind.ARRAY_ITEM_MISSING:
+ return f"[<missing:{step.key}>]"
+ if kind == AccessKind.MAP_ITEM_MISSING:
+ return f"[<missing:{step.key!r}>]"
+ raise ValueError(f"Unknown AccessKind: {kind}")
+
+
+def _hidden_path_suffix(requested_path, visible_path):
+ return "".join(
+ _access_step_text(step) for step in
requested_path.to_steps()[visible_path.depth :]
+ )
+
+
+def _location_block(obj, objpath):
+ script, visible_paths = obj.script(
+ path_to_underline=[objpath],
+ syntax_sugar=False,
+ render_invisible_path_info=True,
+ )
+ visible_path = visible_paths[0] if visible_paths else None
+ lines = [f"Access path: {objpath}"]
+ if visible_path is not None and visible_path != objpath and
visible_path.is_prefix_of(objpath):
+ lines.extend(
+ [
+ f"Highlighted object: {visible_path}",
+ f"Hidden field: {_hidden_path_suffix(objpath, visible_path)}",
+ "Note: The hidden field is not rendered in TVMScript, so the
underline "
+ "points to the nearest visible object in the access path.",
+ ]
+ )
+ context = "\n".join(lines)
+ return f"{context}\n\n{script}"
Review Comment:
I am keeping this expected-value helper independent on purpose. These
assertions test the exact diagnostic text produced through
`_script_with_invisible_path_info`; calling that production helper again to
construct the expected text would make the test self-referential and allow
formatter regressions to pass on both sides. The small duplicated formatter is
the test oracle here, including the visible-prefix/hidden-suffix behavior.
--
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]
