Visitor pattern apply on shared_ptr or raw pointer?

Question

I am working on a small compiler. Now i have a hierarchy system to express the Abstract Syntax Tree(AST).

class Ast{
public:
    // ...
}

class Expr : public Ast{
public:
    // ...
}

using ExprNode = shared_ptr<Expr>;

class BinaryOp : public Expr{
public:
    ExprNode lhs;
    ExprNode rhs;
}

All the class in the Ast hierarchy system use shared_ptr to manage their member(if needed), for example, the BinaryOp holds two member to express its operand.

I'd like to apply visitor pattern on the tree to produce IR code for the AST. My problem is, should the visitor accept raw pointer as parameter, or accept shared_ptr as parameter, which may import shared_from_this(consider the pros and cons)? If shared_ptr is need, should i use pass it by value or by reference?

---

Raw pointer

class AstVisitor{
public:
   virtual Ast* visit(Ast* ast);
   virtual Ast* visitBinaryOp(BinaryOp* binary){
      visit(binary->lhs.get());
      visit(binary->rhs.get());
      // ...
   }
}

class Ast{
public:
    virtual Ast* accept(AstVisitor& visitor);
}

class BinaryOp:{
public:
    virtual Ast* accept(AstVisitor& visitor) override{
        return visitor.visitBinaryOp(this);
    }
}

shared_ptr

using AstNode = shared_ptr<Ast>;
using BinaryOpNode = shared_ptr<BinaryOp>;
class AstVisitor{
public:
   virtual AstNode visit(AstNode ast);
   virtual AstNode visitBinaryOp(BinaryOpNode binary){
      visit(binary->lhs);
      visit(binary->rhs);
      // ...
   }
}

class Ast : public enable_shared_from_this<Ast>{
public:
    virtual AstNode accept(AstVisitor& visitor);
}

class BinaryOp:{
public:
    virtual AstNode accept(AstVisitor& visitor) override{
        return visitor.visitBinaryOp(static_pointer_cast<BinaryOp>(shared_from_this()));
    }
}

Answer 1

I think that C++ Core Guidelines edited by B. Stroustrup and H. Sutter would be helpful for this problem. Please note this guideline:

R.30: Take smart pointers as parameters only to explicitly express lifetime semantics

Reason Accepting a smart pointer to a widget is wrong if the function just needs the widget itself. It should be able to accept any widget object, not just ones whose lifetimes are managed by a particular kind of smart pointer. A function that does not manipulate lifetime should take raw pointers or references instead.

For instance, in your BinaryOp class, managing child nodes with shared_ptr is a good practice. Each node has it's parent as an owner and these shared_ptrs keep the subtree alive with RAII semantics. (std::unique_ptr would be more preferable because each node has a single parent which is a single owner.)

OTOH, since your visit and visitBinaryOp seem to just need Ast and BinaryOp themselves respectively, accepting a raw pointer (or a reference if possible) would be preferable in this case.

Answer 2

A function accepting a shared_ptr tells to the reader that it makes a copy of it and keeps it after the function returns.

Functions should not take shared_ptr unless they make a copy of it.