Merge remote-tracking branch 'origin/topic/robin/gh-1803-no-hilti'

* origin/topic/robin/gh-1803-no-hilti:
  Fix namespacing of `hilti` IDs in Spicy-side diagnostic output.
  Add printer plugin hook to customize ID printing.
  Add scope guard utility class.

CHANGES

@@ -1,3 +1,27 @@
+1.12.0-dev.163 | 2024-10-24 09:59:35 +0200
+
+  * GH-1803: Fix namespacing of `hilti` IDs in Spicy-side diagnostic output. (Robin Sommer, Corelight)
+
+    We now show them with a `spicy` prefix, which makes more sense for
+    users.
+
+  * Add printer plugin hook to customize ID printing. (Robin Sommer, Corelight)
+
+    Because IDs are no longer AST nodes, we did not have a way for a
+    compiler plugin's printing code to modify how it would like them to be
+    printed. This adds a corresponding hook. It's not used anywhere yet,
+    but will be soon.
+
+    In addition, we add a notion of "user-visibility" to the printer API
+    so that printing code knows whether the resulting output is something
+    that will be shown to the user (e.g., in diagnostics), or remains
+    internal (e..g, raw code output, debugging output). Likewise not used
+    yet, but will be soon.
+
+    We also clean up the printer API a little bit.
+
+  * Add scope guard utility class. (Robin Sommer, Corelight)
+
 1.12.0-dev.159 | 2024-10-23 17:33:28 +0200
 
   * Bump pre-commit hooks (Benjamin Bannier, Corelight)

VERSION

@@ -1 +1 @@
-1.12.0-dev.159
+1.12.0-dev.163

doc/autogen/types/bytes.rst

@@ -5,7 +5,7 @@
     Returns an iterator representing the offset *i* inside the bytes
     value.
 
-.. spicy:method:: bytes::decode bytes decode False string ([ charset: spicy::Charset = hilti::Charset::UTF8 ], [ errors: spicy::DecodeErrorStrategy = hilti::DecodeErrorStrategy::REPLACE ])
+.. spicy:method:: bytes::decode bytes decode False string ([ charset: spicy::Charset = spicy::Charset::UTF8 ], [ errors: spicy::DecodeErrorStrategy = spicy::DecodeErrorStrategy::REPLACE ])
 
     Interprets the ``bytes`` as representing an binary string encoded with
     the given character set, and converts it into a UTF8 string. If data
@@ -29,7 +29,7 @@
     as printable strings. The portions will be separated by the bytes
     value to which this method is invoked as a member.
 
-.. spicy:method:: bytes::lower bytes lower False bytes ([ charset: spicy::Charset = hilti::Charset::UTF8 ], [ errors: spicy::DecodeErrorStrategy = hilti::DecodeErrorStrategy::REPLACE ])
+.. spicy:method:: bytes::lower bytes lower False bytes ([ charset: spicy::Charset = spicy::Charset::UTF8 ], [ errors: spicy::DecodeErrorStrategy = spicy::DecodeErrorStrategy::REPLACE ])
 
     Returns a lower-case version of the bytes value, assuming it is
     encoded in character set *charset*. If data is encountered that
@@ -139,7 +139,7 @@
     conversion fails, throws a `RuntimeError` exception, this can happen
     when ``bytes`` is empty or its size is larger than 8 bytes.
 
-.. spicy:method:: bytes::upper bytes upper False bytes ([ charset: spicy::Charset = hilti::Charset::UTF8 ], [ errors: spicy::DecodeErrorStrategy = hilti::DecodeErrorStrategy::REPLACE ])
+.. spicy:method:: bytes::upper bytes upper False bytes ([ charset: spicy::Charset = spicy::Charset::UTF8 ], [ errors: spicy::DecodeErrorStrategy = spicy::DecodeErrorStrategy::REPLACE ])
 
     Returns an upper-case version of the bytes value, assuming it is
     encoded in character set *charset*. If data is encountered that

doc/autogen/types/string.rst

@@ -1,6 +1,6 @@
 .. rubric:: Methods
 
-.. spicy:method:: string::encode string encode False bytes ([ charset: spicy::Charset = hilti::Charset::UTF8 ])
+.. spicy:method:: string::encode string encode False bytes ([ charset: spicy::Charset = spicy::Charset::UTF8 ])
 
     Converts the string into a binary representation encoded with the
     given character set.

hilti/toolchain/include/ast/node.h

@@ -710,17 +710,34 @@ class Node {
      *
      * @param out output stream
      * @param compact create a one-line representation
+     * @param user_visible if true, signal to the printer that the output is
+     * intended for user consumption, permitting it to do some visual polishing
      *
      */
-    void print(std::ostream& out, bool compact = false) const;
+    void print(std::ostream& out, bool compact, bool user_visible) const;
 
     /**
      * Returns a HILTI source code representation of the node and all its
-     * children. Note that this can be called from inside a debugger.
+     * children. This always renders the code as "user-visible", per the flag
+     * in the extended version of `print()`.
+     *
+     * Note that this can be called from inside a debugger.
      */
     std::string print() const;
 
-    /** Renders the node as HILTI source code (same as `print()`) */
+    /**
+     * Returns a HILTI source code representation of the node and all
+     * its children. This always renders the code as *not*
+     * "user-visible", per the flag in the extended version of Zeek.
+     *
+     * Note that this can be called from inside a debugger.
+     */
+    std::string printRaw() const;
+
+    /**
+     * Renders the node as HILTI source code, using the same semantics
+     * as `print()`.
+     */
     operator std::string() const { return print(); }
 
     /**
@@ -1207,7 +1224,7 @@ auto transform(const hilti::node::Set<X>& x, F f) {
 
 /** Renders a node as HILTI source code. */
 inline std::ostream& operator<<(std::ostream& out, const Node& n) {
-    n.print(out, true);
+    n.print(out, true, true);
     return out;
 }
 

hilti/toolchain/include/base/util.h

@@ -600,6 +600,21 @@ constexpr auto to_string(Enum value, const Value<Enum> (&values)[Size]) {
  */
 std::optional<hilti::rt::filesystem::path> cacheDirectory(const hilti::Configuration& configuration);
 
+/**
+ * Clone of `std::experimental::scope_exit`that calls an exit function on destruction.
+ */
+template<typename EF>
+struct scope_exit {
+    scope_exit(EF&& f) noexcept : _f(std::forward<EF>(f)) {}
+
+    scope_exit(const scope_exit&) = delete;
+    scope_exit(scope_exit&&) = delete;
+
+    ~scope_exit() { _f(); }
+
+    EF _f;
+};
+
 } // namespace util
 
 } // namespace hilti

hilti/toolchain/include/compiler/plugin.h

@@ -190,6 +190,17 @@ struct Plugin {
      */
     Hook<bool, Node*, printer::Stream&> ast_print = nullptr;
 
+    /**
+     * Hook called to output an ID during AST output. The hook gets to
+     * choose if it actually wants to print the ID (potentially
+     * modified), or fall back to the default printer.
+     *
+     * @param arg1 ID to print
+     * @param arg2 stream to print the ID to
+     * @return true if the hook printed the ID, false to fall back to default
+     */
+    Hook<bool, const ID&, printer::Stream&> ast_print_id = nullptr;
+
     /**
      * Hook called to replace AST nodes of one language (plugin) with nodes
      * of another coming further down in the pipeline.

hilti/toolchain/include/compiler/printer.h

@@ -13,26 +13,30 @@
 #include <hilti/ast/id.h>
 #include <hilti/base/util.h>
 
-namespace hilti::printer {
+namespace hilti {
 
-class Stream;
+struct Plugin;
 
-/**
- * Prints an AST as HILTI source code. This consults any installed plugin
- * `print_ast` hooks.
- */
-void print(std::ostream& out, Node* root, bool compact = false);
+namespace printer {
 
 /**
  * Prints an AST as HILTI source code. This consults any installed plugin
  * `print_ast` hooks.
+ *
+ * @param out output stream
+ * @param root top-level node of the AST to print (which does not need to be an `ASTRoot`)
+ * @param compact if true, create a one-line representation
+ * @param user_visible if true, signal to the printer that the output is
+ * intended for user consumption, permitting it to do some visual polishing
  */
-void print(Stream& stream, Node* root); // NOLINT
+void print(std::ostream& out, Node* root, bool compact, bool user_visible);
 
 namespace detail {
 
 /** Maintains printer state while output is in progress. */
 struct State {
+    const Plugin* current_plugin = nullptr;
+
     std::vector<ID> scopes = {{""}};
     std::string pending;
     int indent = 0;
@@ -41,6 +45,7 @@ struct State {
     bool last_in_block = false;
     bool expand_subsequent_type = false;
     bool compact = false;
+    bool user_visible = true;
 
     inline static std::unique_ptr<State> current;
     inline static uint64_t depth = 0;
@@ -102,7 +107,7 @@ class Stream {
     template<typename T, IF_DERIVED_FROM(T, Node)>
     Stream& operator<<(T* t) {
         _flush_pending();
-        ::hilti::printer::print(*this, t);
+        _print(t);
         return *this;
     }
 
@@ -150,6 +155,10 @@ class Stream {
     }
 
 private:
+    friend void printer::print(std::ostream& out, Node* root, bool compact, bool user_visible);
+
+    void _print(Node* root);
+
     void _flush_pending() {
         _stream << state().pending;
         state().pending.clear();
@@ -158,4 +167,5 @@ class Stream {
     std::ostream& _stream;
 };
 
-} // namespace hilti::printer
+} // namespace printer
+} // namespace hilti

hilti/toolchain/src/ast/node.cc

@@ -90,11 +90,19 @@ std::string Node::renderSelf(bool include_location) const {
     return s;
 }
 
-void Node::print(std::ostream& out, bool compact) const { printer::print(out, const_cast<Node*>(this), compact); }
+void Node::print(std::ostream& out, bool compact, bool user_visible) const {
+    printer::print(out, const_cast<Node*>(this), compact, user_visible);
+}
 
 std::string Node::print() const {
     std::stringstream out;
-    printer::print(out, const_cast<Node*>(this), true);
+    printer::print(out, const_cast<Node*>(this), true, true);
+    return out.str();
+}
+
+std::string Node::printRaw() const {
+    std::stringstream out;
+    printer::print(out, const_cast<Node*>(this), true, false);
     return out.str();
 }
 

hilti/toolchain/src/ast/visitor.cc

@@ -16,10 +16,10 @@ void visitor::MutatingVisitorBase::replaceNode(Node* old, Node* new_, const std:
         msg_ = util::fmt(" (%s)", msg);
 
     if ( new_ )
-        HILTI_DEBUG(_dbg, util::fmt("%s%s \"%s\" -> %s \"%s\"%s", location, old->typename_(), *old, new_->typename_(),
-                                    *new_, msg_))
+        HILTI_DEBUG(_dbg, util::fmt("%s%s \"%s\" -> %s \"%s\"%s", location, old->typename_(), old->printRaw(),
+                                    new_->typename_(), new_->printRaw(), msg_))
     else
-        HILTI_DEBUG(_dbg, util::fmt("%s%s \"%s\" -> null%s", location, old->typename_(), *old, msg_))
+        HILTI_DEBUG(_dbg, util::fmt("%s%s \"%s\" -> null%s", location, old->typename_(), old->printRaw(), msg_))
 
     assert(old->parent());
     if ( new_ && new_->parent() )
@@ -36,14 +36,14 @@ void visitor::MutatingVisitorBase::recordChange(const Node* old, Node* changed,
     if ( ! msg.empty() )
         msg_ = util::fmt(" (%s)", msg);
 
-    HILTI_DEBUG(_dbg, util::fmt("%s%s \"%s\" -> %s \"%s\"%s", location, old->typename_(), *old, changed->typename_(),
-                                *changed, msg_))
+    HILTI_DEBUG(_dbg, util::fmt("%s%s \"%s\" -> %s \"%s\"%s", location, old->typename_(), old->printRaw(),
+                                changed->typename_(), *changed, msg_))
     _modified = true;
 }
 
 void visitor::MutatingVisitorBase::recordChange(const Node* old, const std::string& msg) {
     auto location = util::fmt("[%s] ", old->location().dump(true));
-    HILTI_DEBUG(_dbg, util::fmt("%s%s \"%s\" -> %s", location, old->typename_(), *old, msg))
+    HILTI_DEBUG(_dbg, util::fmt("%s%s \"%s\" -> %s", location, old->typename_(), old->printRaw(), msg))
     _modified = true;
 }
 

hilti/toolchain/src/compiler/codegen/statements.cc

@@ -14,7 +14,7 @@ using util::fmt;
 
 using namespace hilti::detail;
 
-inline auto traceStatement(CodeGen* cg, cxx::Block* b, Statement* s, bool skip_location = false) {
+static auto traceStatement(CodeGen* cg, cxx::Block* b, Statement* s, bool skip_location = false) {
     if ( s->isA<statement::Block>() )
         return;
 
@@ -28,7 +28,7 @@ inline auto traceStatement(CodeGen* cg, cxx::Block* b, Statement* s, bool skip_l
             location = fmt("%s: ", s->meta().location().dump(true));
 
         b->addStatement(
-            fmt(R"(HILTI_RT_DEBUG("hilti-trace", "%s: %s"))", location, util::escapeUTF8(fmt("%s", *s), true)));
+            fmt(R"(HILTI_RT_DEBUG("hilti-trace", "%s: %s"))", location, util::escapeUTF8(s->printRaw(), true)));
     }
 }
 

hilti/toolchain/src/compiler/codegen/types.cc

@@ -1096,7 +1096,7 @@ const CxxTypeInfo& CodeGen::_getOrCreateTypeInfo(QualifiedType* t) {
         // Prefer the bare type name as the display value.
         display << t->type()->typeID();
     else
-        printer::print(display, t, true);
+        printer::print(display, t, true, true);
 
     if ( display.str().empty() )
         logger().internalError(fmt("codegen: type %s does not have a display rendering for type information",

hilti/toolchain/src/compiler/printer.cc

@@ -17,6 +17,11 @@ using namespace hilti;
 using util::fmt;
 
 printer::Stream& printer::Stream::operator<<(const ID& id) {
+    if ( const auto* plugin = state().current_plugin ) {
+        if ( auto hook = plugin->ast_print_id; hook && hook(id, *this) )
+            return *this; // plugin handled it
+    }
+
     if ( id.namespace_() == currentScope() )
         (*this) << std::string(id.local());
     else
@@ -1084,24 +1089,24 @@ struct Printer : visitor::PreOrder {
 
 } // anonymous namespace
 
-void printer::print(std::ostream& out, Node* root, bool compact) {
-    if ( ! detail::State::current )
+void printer::print(std::ostream& out, Node* root, bool compact, bool user_visible) {
+    if ( ! detail::State::current ) {
         detail::State::current = std::make_unique<detail::State>();
+        detail::State::current->user_visible = user_visible;
+    }
 
     ++detail::State::depth;
 
-    struct _ {
-        ~_() {
-            if ( --detail::State::depth == 0 )
-                detail::State::current.reset();
-        }
-    } __;
+    auto _ = util::scope_exit([&]() {
+        if ( --detail::State::depth == 0 )
+            detail::State::current.reset();
+    });
 
     if ( compact ) {
         std::stringstream buffer;
         auto stream = printer::Stream(buffer);
         stream.setCompact(true);
-        print(stream, root);
+        stream._print(root);
         auto data = buffer.str();
         data = util::trim(data);
         data = util::replace(data, "\n", " ");
@@ -1111,20 +1116,31 @@ void printer::print(std::ostream& out, Node* root, bool compact) {
     }
     else {
         auto stream = printer::Stream(out);
-        print(stream, root);
+        stream._print(root);
     }
 }
 
-void printer::print(printer::Stream& stream, Node* root) {
+void printer::Stream::_print(Node* root) {
     util::timing::Collector _("hilti/printer");
 
     for ( auto& p : plugin::registry().plugins() ) {
         if ( ! p.ast_print )
             continue;
 
-        if ( (*p.ast_print)(root, stream) )
+        auto* prev = std::exchange(detail::State::current->current_plugin, &p);
+        auto _ = util::scope_exit([&]() { detail::State::current->current_plugin = prev; });
+
+        if ( (*p.ast_print)(root, *this) )
+            return;
+        else {
+            // If the print hook did not succeed defer to default printer.
+            // This might still make use of the currently selected plugin.
+            Printer(*this).dispatch(root);
             return;
+        }
     }
 
-    Printer(stream).dispatch(root);
+    // Defer to the default printer with the current plugin (which might be
+    // unset).
+    Printer(*this).dispatch(root);
 }

hilti/toolchain/src/compiler/unit.cc

@@ -56,7 +56,7 @@ bool Unit::isCompiledHILTI() const {
 
 Result<Nothing> Unit::print(std::ostream& out) const {
     if ( module() )
-        printer::print(out, module());
+        printer::print(out, module(), false, false);
 
     return Nothing();
 }