PDBFPO: Use references instead of pointers, where possible

[labath]

Summary

The code was passing pointers around, expecting they would be not null.
In c++ it is possible to convey this notion explicitly by using a
reference instead.

Not all uses of pointers could be converted to references (e.g. one
can't store references in a container), but this will at least make it
locally obvious that code is dealing with nonnull pointers.

[amccarth]

LGTM.

I noticed this also deleted two overloads of Visit from FPOProgramASTVisitorDWARFCodegen, but that appears to be harmless (the base class overloads were also no-ops).

[aleksandr.urakov]

LGTM too!

[labath]

In D60271#1455106, @amccarth wrote:

I noticed this also deleted two overloads of Visit from FPOProgramASTVisitorDWARFCodegen, but that appears to be harmless (the base class overloads were also no-ops).

Yep, that was a drive-by cleanup :P.

lldb/trunk/source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp

enum Kind {
     UnaryOp,
   };
 
 protected:
   FPOProgramNode(Kind kind) : m_token_kind(kind) {}
 
 public:
   virtual ~FPOProgramNode() = default;
-  virtual void Accept(FPOProgramASTVisitor *visitor) = 0;
+  virtual void Accept(FPOProgramASTVisitor &visitor) = 0;
 
   Kind GetKind() const { return m_token_kind; }
 
 private:
   Kind m_token_kind;
 };
 
 class FPOProgramNodeSymbol: public FPOProgramNode {
 public:
   FPOProgramNodeSymbol(llvm::StringRef name)
       : FPOProgramNode(Symbol), m_name(name) {}
 
-  void Accept(FPOProgramASTVisitor *visitor) override;
+  void Accept(FPOProgramASTVisitor &visitor) override;
 
   llvm::StringRef GetName() const { return m_name; }
 
 private:
   llvm::StringRef m_name;
 };
 
 class FPOProgramNodeRegisterRef : public FPOProgramNode {
 public:
   FPOProgramNodeRegisterRef(uint32_t lldb_reg_num)
       : FPOProgramNode(Register), m_lldb_reg_num(lldb_reg_num) {}
 
-  void Accept(FPOProgramASTVisitor *visitor) override;
+  void Accept(FPOProgramASTVisitor &visitor) override;
 
   uint32_t GetLLDBRegNum() const { return m_lldb_reg_num; }
 
 private:
   uint32_t m_lldb_reg_num;
 };
 
 class FPOProgramNodeIntegerLiteral : public FPOProgramNode {
 public:
   FPOProgramNodeIntegerLiteral(uint32_t value)
       : FPOProgramNode(IntegerLiteral), m_value(value) {}
 
-  void Accept(FPOProgramASTVisitor *visitor) override;
+  void Accept(FPOProgramASTVisitor &visitor) override;
 
   uint32_t GetValue() const { return m_value; }
 
 private:
   uint32_t m_value;
 };
 
 class FPOProgramNodeBinaryOp : public FPOProgramNode {
 public:
   enum OpType {
     Plus,
     Minus,
     Align,
   };
 
-  FPOProgramNodeBinaryOp(OpType op_type, FPOProgramNode *left,
-                         FPOProgramNode *right)
-      : FPOProgramNode(BinaryOp), m_op_type(op_type), m_left(left),
-        m_right(right) {}
+  FPOProgramNodeBinaryOp(OpType op_type, FPOProgramNode &left,
+                         FPOProgramNode &right)
+      : FPOProgramNode(BinaryOp), m_op_type(op_type), m_left(&left),
+        m_right(&right) {}
 
-  void Accept(FPOProgramASTVisitor *visitor) override;
+  void Accept(FPOProgramASTVisitor &visitor) override;
 
   OpType GetOpType() const { return m_op_type; }
 
   const FPOProgramNode *Left() const { return m_left; }
   FPOProgramNode *&Left() { return m_left; }
 
   const FPOProgramNode *Right() const { return m_right; }
   FPOProgramNode *&Right() { return m_right; }
 
 private:
   OpType m_op_type;
   FPOProgramNode *m_left;
   FPOProgramNode *m_right;
 };
 
 class FPOProgramNodeUnaryOp : public FPOProgramNode {
 public:
   enum OpType {
     Deref,
   };
 
-  FPOProgramNodeUnaryOp(OpType op_type, FPOProgramNode *operand)
-      : FPOProgramNode(UnaryOp), m_op_type(op_type), m_operand(operand) {}
+  FPOProgramNodeUnaryOp(OpType op_type, FPOProgramNode &operand)
+      : FPOProgramNode(UnaryOp), m_op_type(op_type), m_operand(&operand) {}
 
-  void Accept(FPOProgramASTVisitor *visitor) override;
+  void Accept(FPOProgramASTVisitor &visitor) override;
 
   OpType GetOpType() const { return m_op_type; }
 
   const FPOProgramNode *Operand() const { return m_operand; }
   FPOProgramNode *&Operand() { return m_operand; }
 
 private:
   OpType m_op_type;
   FPOProgramNode *m_operand;
 };
 
 class FPOProgramASTVisitor {
 public:
   virtual ~FPOProgramASTVisitor() = default;
 
-  virtual void Visit(FPOProgramNodeSymbol *node) {}
-  virtual void Visit(FPOProgramNodeRegisterRef *node) {}
-  virtual void Visit(FPOProgramNodeIntegerLiteral *node) {}
-  virtual void Visit(FPOProgramNodeBinaryOp *node) {}
-  virtual void Visit(FPOProgramNodeUnaryOp *node) {}
+  virtual void Visit(FPOProgramNodeSymbol &node) {}
+  virtual void Visit(FPOProgramNodeRegisterRef &node) {}
+  virtual void Visit(FPOProgramNodeIntegerLiteral &node) {}
+  virtual void Visit(FPOProgramNodeBinaryOp &node) {}
+  virtual void Visit(FPOProgramNodeUnaryOp &node) {}
 };
 
-void FPOProgramNodeSymbol::Accept(FPOProgramASTVisitor *visitor) {
-  visitor->Visit(this);
+void FPOProgramNodeSymbol::Accept(FPOProgramASTVisitor &visitor) {
+  visitor.Visit(*this);
 }
 
-void FPOProgramNodeRegisterRef::Accept(FPOProgramASTVisitor *visitor) {
-  visitor->Visit(this);
+void FPOProgramNodeRegisterRef::Accept(FPOProgramASTVisitor &visitor) {
+  visitor.Visit(*this);
 }
 
-void FPOProgramNodeIntegerLiteral::Accept(FPOProgramASTVisitor *visitor) {
-  visitor->Visit(this);
+void FPOProgramNodeIntegerLiteral::Accept(FPOProgramASTVisitor &visitor) {
+  visitor.Visit(*this);
 }
 
-void FPOProgramNodeBinaryOp::Accept(FPOProgramASTVisitor *visitor) {
-  visitor->Visit(this);
+void FPOProgramNodeBinaryOp::Accept(FPOProgramASTVisitor &visitor) {
+  visitor.Visit(*this);
 }
 
-void FPOProgramNodeUnaryOp::Accept(FPOProgramASTVisitor *visitor) {
-  visitor->Visit(this);
+void FPOProgramNodeUnaryOp::Accept(FPOProgramASTVisitor &visitor) {
+  visitor.Visit(*this);
 }
 
 class FPOProgramASTVisitorMergeDependent : public FPOProgramASTVisitor {
 public:
   FPOProgramASTVisitorMergeDependent(
       const llvm::DenseMap<llvm::StringRef, FPOProgramNode *>
           &dependent_programs)
       : m_dependent_programs(dependent_programs) {}
 
   void Merge(FPOProgramNode *&node_ref);
 
 private:
-  void Visit(FPOProgramNodeRegisterRef *node) override {}
-  void Visit(FPOProgramNodeIntegerLiteral *node) override {}
-  void Visit(FPOProgramNodeBinaryOp *node) override;
-  void Visit(FPOProgramNodeUnaryOp *node) override;
+  void Visit(FPOProgramNodeBinaryOp &node) override;
+  void Visit(FPOProgramNodeUnaryOp &node) override;
 
   void TryReplace(FPOProgramNode *&node_ref) const;
 
 private:
   const llvm::DenseMap<llvm::StringRef, FPOProgramNode *> &m_dependent_programs;
 };
 
 void FPOProgramASTVisitorMergeDependent::Merge(FPOProgramNode *&node_ref) {
   TryReplace(node_ref);
-  node_ref->Accept(this);
+  node_ref->Accept(*this);
 }
 
-void FPOProgramASTVisitorMergeDependent::Visit(FPOProgramNodeBinaryOp *node) {
-  Merge(node->Left());
-  Merge(node->Right());
+void FPOProgramASTVisitorMergeDependent::Visit(FPOProgramNodeBinaryOp &node) {
+  Merge(node.Left());
+  Merge(node.Right());
 }
-void FPOProgramASTVisitorMergeDependent::Visit(FPOProgramNodeUnaryOp *node) {
-  Merge(node->Operand());
+void FPOProgramASTVisitorMergeDependent::Visit(FPOProgramNodeUnaryOp &node) {
+  Merge(node.Operand());
 }
 
 void FPOProgramASTVisitorMergeDependent::TryReplace(
     FPOProgramNode *&node_ref) const {
 
   while (node_ref->GetKind() == FPOProgramNode::Symbol) {
     auto *node_symbol_ref = static_cast<FPOProgramNodeSymbol *>(node_ref);
 
FPOProgramASTVisitorResolveRegisterRefs(
           &dependent_programs,
       llvm::Triple::ArchType arch_type, NodeAllocator &alloc)
       : m_dependent_programs(dependent_programs), m_arch_type(arch_type),
         m_alloc(alloc) {}
 
   bool Resolve(FPOProgramNode *&program);
 
 private:
-  void Visit(FPOProgramNodeBinaryOp *node) override;
-  void Visit(FPOProgramNodeUnaryOp *node) override;
+  void Visit(FPOProgramNodeBinaryOp &node) override;
+  void Visit(FPOProgramNodeUnaryOp &node) override;
 
   bool TryReplace(FPOProgramNode *&node_ref);
 
   const llvm::DenseMap<llvm::StringRef, FPOProgramNode *> &m_dependent_programs;
   llvm::Triple::ArchType m_arch_type;
   NodeAllocator &m_alloc;
   bool m_no_error_flag = true;
 };
 
 bool FPOProgramASTVisitorResolveRegisterRefs::Resolve(FPOProgramNode *&program) {
   if (!TryReplace(program))
     return false;
-  program->Accept(this);
+  program->Accept(*this);
   return m_no_error_flag;
 }
 
 static uint32_t ResolveLLDBRegisterNum(llvm::StringRef reg_name, llvm::Triple::ArchType arch_type) {
   // lookup register name to get lldb register number
   llvm::ArrayRef<llvm::EnumEntry<uint16_t>> register_names =
       llvm::codeview::getRegisterNames();
   auto it = llvm::find_if(
bool FPOProgramASTVisitorResolveRegisterRefs::TryReplace(
   if (reg_num == LLDB_INVALID_REGNUM)
     return false;
 
   node_ref = m_alloc.makeNode<FPOProgramNodeRegisterRef>(reg_num);
   return true;
 }
 
 void FPOProgramASTVisitorResolveRegisterRefs::Visit(
-    FPOProgramNodeBinaryOp *node) {
-  m_no_error_flag = Resolve(node->Left()) && Resolve(node->Right());
+    FPOProgramNodeBinaryOp &node) {
+  m_no_error_flag = Resolve(node.Left()) && Resolve(node.Right());
 }
 
 void FPOProgramASTVisitorResolveRegisterRefs::Visit(
-    FPOProgramNodeUnaryOp *node) {
-  m_no_error_flag = Resolve(node->Operand());
+    FPOProgramNodeUnaryOp &node) {
+  m_no_error_flag = Resolve(node.Operand());
 }
 
 class FPOProgramASTVisitorDWARFCodegen : public FPOProgramASTVisitor {
 public:
   FPOProgramASTVisitorDWARFCodegen(Stream &stream) : m_out_stream(stream) {}
 
-  void Emit(FPOProgramNode *program);
+  void Emit(FPOProgramNode &program);
 
 private:
-  void Visit(FPOProgramNodeRegisterRef *node) override;
-  void Visit(FPOProgramNodeIntegerLiteral *node) override;
-  void Visit(FPOProgramNodeBinaryOp *node) override;
-  void Visit(FPOProgramNodeUnaryOp *node) override;
+  void Visit(FPOProgramNodeRegisterRef &node) override;
+  void Visit(FPOProgramNodeIntegerLiteral &node) override;
+  void Visit(FPOProgramNodeBinaryOp &node) override;
+  void Visit(FPOProgramNodeUnaryOp &node) override;
 
 private:
   Stream &m_out_stream;
 };
 
-void FPOProgramASTVisitorDWARFCodegen::Emit(FPOProgramNode *program) {
-  program->Accept(this);
+void FPOProgramASTVisitorDWARFCodegen::Emit(FPOProgramNode &program) {
+  program.Accept(*this);
 }
 
-void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeRegisterRef *node) {
+void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeRegisterRef &node) {
 
-  uint32_t reg_num = node->GetLLDBRegNum();
+  uint32_t reg_num = node.GetLLDBRegNum();
   lldbassert(reg_num != LLDB_INVALID_REGNUM);
 
   if (reg_num > 31) {
     m_out_stream.PutHex8(DW_OP_bregx);
     m_out_stream.PutULEB128(reg_num);
   } else
     m_out_stream.PutHex8(DW_OP_breg0 + reg_num);
 
   m_out_stream.PutSLEB128(0);
 }
 
 void FPOProgramASTVisitorDWARFCodegen::Visit(
-    FPOProgramNodeIntegerLiteral *node) {
-  uint32_t value = node->GetValue();
+    FPOProgramNodeIntegerLiteral &node) {
+  uint32_t value = node.GetValue();
   m_out_stream.PutHex8(DW_OP_constu);
   m_out_stream.PutULEB128(value);
 }
 
-void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeBinaryOp *node) {
+void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeBinaryOp &node) {
 
-  Emit(node->Left());
-  Emit(node->Right());
+  Emit(*node.Left());
+  Emit(*node.Right());
 
-  switch (node->GetOpType()) {
+  switch (node.GetOpType()) {
   case FPOProgramNodeBinaryOp::Plus:
     m_out_stream.PutHex8(DW_OP_plus);
     // NOTE: can be optimized by using DW_OP_plus_uconst opcpode
     //       if right child node is constant value
     break;
   case FPOProgramNodeBinaryOp::Minus:
     m_out_stream.PutHex8(DW_OP_minus);
     break;
   case FPOProgramNodeBinaryOp::Align:
     // emit align operator a @ b as
     // a & ~(b - 1)
     // NOTE: implicitly assuming that b is power of 2
     m_out_stream.PutHex8(DW_OP_lit1);
     m_out_stream.PutHex8(DW_OP_minus);
     m_out_stream.PutHex8(DW_OP_not);
 
     m_out_stream.PutHex8(DW_OP_and);
     break;
   }
 }
 
-void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeUnaryOp *node) {
-  Emit(node->Operand());
+void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeUnaryOp &node) {
+  Emit(*node.Operand());
 
-  switch (node->GetOpType()) {
+  switch (node.GetOpType()) {
   case FPOProgramNodeUnaryOp::Deref:
     m_out_stream.PutHex8(DW_OP_deref);
     break;
   }
 }
 
 } // namespace
 
for (size_t i = 1; i < tokens.size(); ++i) {
     if (ops_binary_it != ops_binary.end()) {
       // token is binary operator
       if (eval_stack.size() < 2) {
         return false;
       }
       FPOProgramNode *right = eval_stack.pop_back_val();
       FPOProgramNode *left = eval_stack.pop_back_val();
       FPOProgramNode *node = alloc.makeNode<FPOProgramNodeBinaryOp>(
-          ops_binary_it->second, left, right);
+          ops_binary_it->second, *left, *right);
       eval_stack.push_back(node);
       continue;
     }
 
     auto ops_unary_it = ops_unary.find(cur);
     if (ops_unary_it != ops_unary.end()) {
       // token is unary operator
       if (eval_stack.empty()) {
         return false;
       }
       FPOProgramNode *operand = eval_stack.pop_back_val();
       FPOProgramNode *node =
-          alloc.makeNode<FPOProgramNodeUnaryOp>(ops_unary_it->second, operand);
+          alloc.makeNode<FPOProgramNodeUnaryOp>(ops_unary_it->second, *operand);
       eval_stack.push_back(node);
       continue;
     }
 
     if (cur.startswith("$")) {
       eval_stack.push_back(alloc.makeNode<FPOProgramNodeSymbol>(cur));
       continue;
     }
bool lldb_private::npdb::TranslateFPOProgramToDWARFExpression(
   NodeAllocator node_alloc;
   FPOProgramNode *target_program =
       ParseFPOProgram(program, register_name, arch_type, node_alloc);
   if (target_program == nullptr) {
     return false;
   }
 
   FPOProgramASTVisitorDWARFCodegen codegen(stream);
-  codegen.Emit(target_program);
+  codegen.Emit(*target_program);
   return true;
 }