Lots of further clean up.

.clang-tidy

@@ -60,6 +60,7 @@ Checks: '
   -readability-function-cognitive-complexity,
   -readability-function-size,
   -readability-simplify-boolean-expr,
+  -readability-use-anyofallof,
   '
 
 HeaderFilterRegex: '(hilti|spicy)/[a-zA-Z]+/include/([a-zA-Z]+/)*[a-zA-Z]+\.h'

hilti/toolchain/CMakeLists.txt

@@ -96,29 +96,27 @@ set(SOURCES
     src/compiler/codegen/statements.cc
     src/compiler/codegen/types.cc
     src/compiler/codegen/unpack.cc
-    src/compiler/coercion.cc
+    src/compiler/coercer.cc
+    src/compiler/constant-folder.cc
     src/compiler/context.cc
     src/compiler/cxx/elements.cc
     src/compiler/cxx/formatter.cc
     src/compiler/cxx/linker.cc
     src/compiler/cxx/unit.cc
     src/compiler/driver.cc
+    src/compiler/id-assigner.cc
     src/compiler/init.cc
     src/compiler/jit.cc
     src/compiler/optimizer.cc
     src/compiler/parser/driver.cc
     src/compiler/plugin.cc
+    src/compiler/printer.cc
+    src/compiler/renderer.cc
+    src/compiler/resolver.cc
+    src/compiler/scope-builder.cc
+    src/compiler/type-unifier.cc
     src/compiler/unit.cc
-    src/compiler/visitors/coercer.cc
-    src/compiler/visitors/constant-folder.cc
-    src/compiler/visitors/id-assigner.cc
-    src/compiler/visitors/normalizer.cc
-    src/compiler/visitors/printer.cc
-    src/compiler/visitors/renderer.cc
-    src/compiler/visitors/resolver.cc
-    src/compiler/visitors/scope-builder.cc
-    src/compiler/visitors/type-unifier.cc
-    src/compiler/visitors/validator.cc
+    src/compiler/validator.cc
     src/global.cc
     # # Already included in hilti-rt, which we pull in.
     # # src/3rdparty/utf8proc/utf8proc.c

hilti/toolchain/include/ast/ast-context.h

@@ -21,7 +21,20 @@ class Context;
 class Driver;
 struct Plugin;
 
-/** Root node for the AST inside an AST context. This will always be present exactly once. */
+/**
+ * Parses a HILTI source file into an AST.
+ *
+ * @param in stream to read from
+ * @param filename path associated with the input
+ *
+ * Returns: The parsed AST, or a corresponding error if parsing failed.
+ */
+Result<ModulePtr> parseSource(Builder* builder, std::istream& in, const std::string& filename);
+
+/**
+ * Root node for the AST inside an AST context. This will always be present
+ * exactly once.
+ */
 class ASTRoot : public Node {
 public:
     ~ASTRoot() override;
@@ -31,94 +44,162 @@ class ASTRoot : public Node {
 protected:
     ASTRoot(ASTContext* ctx) : Node(ctx, {}, Meta(Location("<root>"))) {}
 
-    std::string _render() const override;
+    std::string _render() const final;
 
     HILTI_NODE(ASTRoot);
 };
 
-// Container for AST-wide state.
+/**
+ * Environment for AST-wide state. The environment stores the AST root node and
+ * owns all nodes added to it or, recursively, any of if children. Each node
+ * can be part of just one AST context.
+ */
 class ASTContext : public std::enable_shared_from_this<ASTContext> {
 public:
-    enum ASTState { Modified, NotModified };
-
+    /**
+     * Constructor.
+     *
+     * @param context compiler context to use for logging and error reporting
+     */
     ASTContext(Context* context);
 
+    /** Returns the AST's root node. This always exists. */
     auto root() const { return _root; }
 
+    /**
+     * Parses a source file and adds it to the AST as a new module. If a module
+     * for this file is already part of the AST, returns the existing module
+     * without any further AST changes.
+     *
+     * @param path path to source file to parse
+     * @param process_extension if given, file extension indicating which
+     * plugin to use later for processing the resulting AST for the module; if
+     * not given, the same plugin will be used as for parsing (which is
+     * determined by the path's extension)
+     * @return UID of the parsed module (which is now a part of the AST), or an
+     * error if parsing failed
+     */
     Result<module::UID> parseSource(const hilti::rt::filesystem::path& path,
                                     std::optional<hilti::rt::filesystem::path> process_extension = {});
 
+    /**
+     * Imports a module from an external source file and adds it to the AST as
+     * a new module. This implements HILTI's `import` statement. If a module
+     * for the requested `import` is already part of the AST, returns the
+     * existing module without any further AST changes.
+     *
+     * @param id name of the module to import (as in: ``import <id>``)
+     * @param scope search scope for the import (as in: ``import ... from <scope>``)
+     * @param parse_extension file extension indicating which plugin to use for
+     * parsing the module's source code
+     * @param process_extension if given, file extension indicating which
+     * plugin to use later for processing the resulting AST; if not given, the
+     * same plugin will be used as for parsing
+     * @param search_dirs list of directories to search for the module's source
+     * (in addition to any globally configured search directories)
+     * @return UID of the parsed module (which is now a part of the AST), or an
+     * error if parsing failed
+     */
     Result<module::UID> importModule(const ID& id, const std::optional<ID>& scope,
                                      const hilti::rt::filesystem::path& parse_extension,
                                      const std::optional<hilti::rt::filesystem::path>& process_extension,
                                      std::vector<hilti::rt::filesystem::path> search_dirs);
 
+    /**
+     * Retrieves a module node from the AST given its UID. Returns null if no
+     * such module exists.
+     *
+     * @param uid UID of module to return
+     */
     ModulePtr getModule(const module::UID& uid) const {
         if ( auto m = _modules_by_uid.find(uid); m != _modules_by_uid.end() )
             return m->second;
         else
             return nullptr;
     }
 
-    Result<Nothing> processAST(Driver* driver); // top-level entry point for resolving/validating/optimizing AST
+    /**
+     * Processes the whole AST with all of the compiler's visitor passes. This
+     * is the top-level entry point for all resolving/validating/optimizing. If
+     * successful, the will be fully resolved and validated; and ready for code
+     * generation.
+     *
+     * @param driver current compiler driver, which AST processing may access
+     */
+    Result<Nothing> processAST(Driver* driver);
 
-    bool isFullyResolved() const { return _resolved; }
+    /**
+     * During AST processing, returns the current compiler driver. If called
+     * outside of `processAST() executing, it will return null.
+     */
+    Driver* driver() const { return _driver; }
 
     /**
      * Returns direct & indirect dependencies that a module imports. This
-     * information will be available only once the AST has been resolved
-     * successfully.
+     * information will be available only once the AST has been processed
+     * successfully through `processAST()`.
      *
-     * @param uid UID of module to return dependencies for; the module must be known, otherwise an internal error is
-     * reported
+     * @param uid UID of module to return dependencies for; the module must be
+     * known, otherwise an internal error is reported
      * @param recursive if true, return the transitive closure of all
-     * dependent units, vs just direct dependencies of the current unit
-     * @return set of dependencies, or empty vector if not yet resolved
+     * dependent units, vs just direct dependencies of the specified unit
+     * @return set of dependencies
      */
     std::vector<module::UID> dependencies(const module::UID& uid, bool recursive = false) const;
 
     /**
      * Dumps the current total AST of all modules to a debug stream.
      *
      * @param stream debug stream to write to
+     * @param prefix prefix line to start output with
      */
     void dumpAST(const hilti::logging::DebugStream& stream, const std::string& prefix);
 
-    // Only valid during AST processing.
-    Driver* driver() const {
-        assert(_driver);
-        return _driver;
-    }
-
 private:
+    // The following methods implement the corresponding phases of AST processing.
+
     Result<module::UID> _parseSource(const hilti::rt::filesystem::path& path, const std::optional<ID>& scope,
                                      std::optional<hilti::rt::filesystem::path> process_extension = {});
-    module::UID _addModuleToAST(ModulePtr module);
     Result<Nothing> _buildScopes(const Plugin& plugin);
     Result<Nothing> _resolve(const Plugin& plugin);
     Result<Nothing> _validate(const Plugin& plugin, bool pre_resolver);
     Result<Nothing> _transform(const Plugin& plugin);
     Result<Nothing> _collectErrors();
     Result<Nothing> _optimize();
+
+    // Adds a module to the AST. The module must not be part of any AST yet
+    // (including the current one).
+    module::UID _addModuleToAST(ModulePtr module);
+
+    // Performs internal consistency checks on the AST.
     void _checkAST() const;
 
+    // Dumps the AST to disk during AST processing, for debugging..
     void _saveIterationAST(const Plugin& plugin, const std::string& prefix, int round = 0);
+
+    // Dumps the AST to disk during AST processing, for debugging..
     void _saveIterationAST(const Plugin& plugin, const std::string& prefix, const std::string& tag);
 
+    // Dumps the AST to a debugging stream.
     void _dumpAST(const hilti::logging::DebugStream& stream, const Plugin& plugin, const std::string& prefix,
                   int round);
+
+    // Dumps the AST to a debugging stream.
     void _dumpAST(std::ostream& stream, const Plugin& plugin, const std::string& prefix, int round);
+
+    // Dumps all declarations nodes to the `declarations' debug stream.
     void _dumpDeclarations(const Plugin& plugin);
 
-    Context* _context;
-    NodeDerivedPtr<ASTRoot> _root;
-    bool _rebuild_scopes = true;
-    bool _resolved = false;
-    Driver* _driver = nullptr;
+    Context* _context;             // compier context.
+    NodeDerivedPtr<ASTRoot> _root; // root node of the AST.
+    bool _rebuild_scopes = true;   // true if next iteration round needs to rebuild all AST scopes
+    bool _resolved = false;        // true if `processAST()` has finished successfully
+    Driver* _driver = nullptr;     // pointer to compiler drive during `processAST()`, null outside of that
 
-    std::unordered_map<module::UID, ModulePtr> _modules_by_uid;
-    std::unordered_map<std::string, ModulePtr> _modules_by_path;
-    std::map<std::pair<ID, ID>, ModulePtr> _modules_by_id_and_scope;
+    std::unordered_map<module::UID, ModulePtr> _modules_by_uid;  // all known modules indexed by UID
+    std::unordered_map<std::string, ModulePtr> _modules_by_path; // all known modules indexed by path
+    std::map<std::pair<ID, ID>, ModulePtr>
+        _modules_by_id_and_scope; // all known modules indexed by their ID and search scope
 };
 
 } // namespace hilti

hilti/toolchain/include/ast/attribute.h

@@ -30,20 +30,31 @@ class Attribute : public Node {
     NodePtr value() const { return child(0); }
 
     /**
-     * Returns the attributes argument as type `T`. `T` must be either an
-     * `Expression`, or `std::string`. In the former case, the value must be
-     * an AST expression node. In the latter case, the argument must be a
-     * string constructor expression, and the returned value will be the
-     * string it represents.
+     * Returns the expression argument associated with the attribute.
      *
-     * @tparam T either `Expression` or `std::string`
-     * @return the argument, or an error if the argument could not be interpreted as type `T`
-     * @exception `std::out_of_range` if the attribute does not have an argument
+     * @return the argument, or an error if the attribute does not have an
+     * argument, or if it's not an expression.
      */
     Result<ExpressionPtr> valueAsExpression() const;
 
+    /**
+     * Returns the expression argument associated with the attribute as a
+     * string, assuming it represents a constant integer value.
+     *
+     * @return the argument, or an error if the attribute does not have an
+     * argument, or if it's not a constant string.
+     */
     Result<std::string> valueAsString() const;
 
+    /**
+     * Returns the expression argument associated with the attribute as a
+     * signed integer, assuming it represents a constant integer value. Both
+     * signed and unsigned integer values are accepted, with the latter cased
+     * into signed for the return value
+     *
+     * @return the argument, or an error if the attribute does not have an
+     * argument, or if it's not a constant integer.
+     */
     Result<int64_t> valueAsInteger() const;
 
     /**
@@ -57,7 +68,7 @@ class Attribute : public Node {
      */
     Result<bool> coerceValueTo(const UnqualifiedTypePtr& dst);
 
-    node::Properties properties() const override {
+    node::Properties properties() const final {
         auto p = node::Properties{{"tag", _tag}};
         return Node::properties() + p;
     }
@@ -104,11 +115,12 @@ class AttributeSet : public Node {
     /** Returns the set's attributes. */
     auto attributes() const { return children<Attribute>(0, -1); }
 
+    /** Returns true if the set is empty. */
     bool empty() const { return attributes().empty(); }
 
     /**
      * Retrieves an attribute with a given name from the set. If multiple
-     * attributes with that tag exist, it's undefined which is returned.
+     * attributes with that tag exist, it's undefined which one is returned.
      *
      * @return attribute if found
      */
@@ -135,7 +147,7 @@ class AttributeSet : public Node {
     void remove(std::string_view tag);
 
     /** Returns true if the set has at least one element. */
-    operator bool() const { return ! children().empty(); }
+    operator bool() const { return ! empty(); }
 
     static auto create(ASTContext* ctx, Attributes attrs = {}, Meta m = Meta()) {
         return NodeDerivedPtr<AttributeSet>(new AttributeSet(ctx, Nodes{std::move(attrs)}, std::move(m)));

hilti/toolchain/include/ast/builder/builder.h

@@ -3,6 +3,7 @@
 #pragma once
 
 #include <memory>
+#include <string>
 #include <utility>
 
 #include <hilti/ast/builder/node-factory.h>
@@ -12,6 +13,18 @@ namespace hilti {
 class Builder : public builder::NodeFactory {
 public:
     Builder(ASTContext* ctx) : NodeFactory(ctx) {}
+
+    // TODO: This class is WIP. We'll expand it with the other old `builder::*` functions.
+
+    ExpressionPtr namedCtor(const std::string& name, const Expressions& args, const Meta& m = Meta()) {
+        return expressionUnresolvedOperator(operator_::Kind::Call,
+                                            {expressionMember(ID(name)), expressionCtor(ctorTuple(args))}, m);
+    }
+
+    auto integer(unsigned int i, const Meta& m = Meta()) { return expressionCtor(ctorUnsignedInteger(i, 64, m), m); }
+    auto bool_(bool b, const Meta& m = Meta()) { return expressionCtor(ctorBool(b, m), m); }
+    auto string(std::string s, const Meta& m = Meta()) { return expressionCtor(ctorString(std::move(s), m), m); }
+    auto tuple(const Expressions& v, const Meta& m = Meta()) { return expressionCtor(ctorTuple(v, m), m); }
 };
 
 using BuilderPtr = std::shared_ptr<Builder>;