Rewrite AST before elaborate

[HungMingWu]

Hi Mike,
I have one question.
Can I operator like this?

class TestRewriter : public SyntaxRewriter<TestRewriter> {
public:
    Compilation compilation;
    BumpAllocator alloc;
    SemanticModel model;

    TestRewriter(const std::shared_ptr<SyntaxTree>& tree) : model(compilation) {
        compilation.addSyntaxTree(tree);
    }

    ~TestRewriter() { compilation.steal(std::move(alloc)); }

    void handle(const HierarchyInstantiationSyntax& decl) {
        auto &new_decl = factory.hierarchyInstantiation(decl.attributes, decl.type, decl.parameters, decl.instances, decl.semi);
        auto type = decl.type.withRawText(alloc, "abcde");
        new_decl.type = type;
        replace(decl, new_decl);
    }
};
auto tree = SyntaxTree::fromBuffer(buffer, sourceManager, options);
tree = TestRewriter(tree).transform(tree);
compilation.addSyntaxTree(tree);

In my brief test, The metadata have been changed.
So I need to serialize to string and construct it again?

[MikePopoloski]

My intention was that you should be able to do things exactly like that, but you make a good point -- I hadn't considered the effect it would have on the metadata. I need to think about how best to handle that. Obviously we could make the metadata modifiable but needing to manually update it seems annoying, it would be nice if it just figured it out for you.

[HungMingWu]

Hi Mike, I have another question.
[I need the following operation like this.

    void handle(const TypedefDeclarationSyntax& decl) {
        if (decl.type->kind != SyntaxKind::EnumType)
            return;

        // Create a new localparam hardcoded with the number of entries in the enum.
        auto type = model.getDeclaredSymbol(decl.type->as<EnumTypeSyntax>());
        REQUIRE(type);

        size_t count = type->as<EnumType>().members().size();
        auto& newNode = parse(
            fmt::format("\n    localparam int {}__count = {};", decl.name.valueText(), count));
        insertAfter(decl, newNode);

        auto& newNode1 = parse(
            fmt::format("\n    localparam int {}__count = {};", decl.name.valueText(), count));
        insertAfter(decl, newNode1);
    }

It doesn't work, the following code snippet doesn't work either.

        auto& newNode = parse(
            fmt::format("\n    localparam int {}__count = {};", decl.name.valueText(), count));
        insertAfter(decl, newNode);

        auto& newNode1 = parse(
            fmt::format("\n    localparam int {}__count = {};", decl.name.valueText(), count));
        insertAfter(newNode, newNode1);

Since the newNode is not oldNode.
Wtach SyntaxVisitor.cpp

struct CloneVisitor {
    SyntaxNode* visit(const T& node) {
        // Ignore
        auto it = changes.find(child);
        if (it == changes.end()) {
            // ignore
         }  
        else {
            switch (it->second.kind) {
                // Ignore
            }
        }
    }
};

It only accept one modification.
Is it a good idea to modify the data structure to

using ChangeMap = flat_hash_map<const SyntaxNode*, std::vector<detail::SyntaxChange>>;

[MikePopoloski]

Sure, that change makes sense. You can probably unify that list with the ListChangeMap which already does the same thing.