Drop Node::SetId()
Node::SetId() has a TODO to drop it. Ideally, this would be private to the parser or not even exist.
Node IDs are usually OK to treat as immutable, but they aren't while parsing. The difficulty is that nodes you haven't parsed yet might want the ID that you would otherwise use for the node you're currently parsing. You can either patch up nodes you've already parsed when you encounter a node that wants a taken ID or defer setting IDs when unspecified and go set them when you've parsed everything.
Another approach is to first scan the input for id=<ID> and build a set of taken IDs before parsing anything. Then when IDs are unspecified you have all the information to get the next available ID.
I did some experimentation with making ID immutable and having FunctionBase store nodes in a btree_set<unique_ptr<Node>, NodeIdLessThan>. The idea was to make node iteration faster than the current std::list<>, but the benefit I saw was small (~2% speedup?).