labath updated this revision to Diff 194705.
labath added a comment.
After trying to use this in new code, I realized that the CRTP is not really
needed for what I am trying to achieve here. The same can be achieved through
more standard virtual functions.
This updates the patch to use virtual functions, which may be 0.01% slower, but
that certainly does not matter here. I also rename the function arguments from
"node" to more descriptive names.
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D60410/new/
https://reviews.llvm.org/D60410
Files:
source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
Index: source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
===================================================================
--- source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
+++ source/Plugins/SymbolFile/NativePDB/PdbFPOProgramToDWARFExpression.cpp
@@ -25,12 +25,11 @@
namespace {
-class FPOProgramNode;
-class FPOProgramASTVisitor;
-
class NodeAllocator {
public:
template <typename T, typename... Args> T *makeNode(Args &&... args) {
+ static_assert(std::is_trivially_destructible<T>::value,
+ "This object will not be destroyed!");
void *new_node_mem = m_alloc.Allocate(sizeof(T), alignof(T));
return new (new_node_mem) T(std::forward<Args>(args)...);
}
@@ -53,9 +52,6 @@
FPOProgramNode(Kind kind) : m_token_kind(kind) {}
public:
- virtual ~FPOProgramNode() = default;
- virtual void Accept(FPOProgramASTVisitor &visitor) = 0;
-
Kind GetKind() const { return m_token_kind; }
private:
@@ -67,8 +63,6 @@
FPOProgramNodeSymbol(llvm::StringRef name)
: FPOProgramNode(Symbol), m_name(name) {}
- void Accept(FPOProgramASTVisitor &visitor) override;
-
llvm::StringRef GetName() const { return m_name; }
static bool classof(const FPOProgramNode *node) {
@@ -84,8 +78,6 @@
FPOProgramNodeRegisterRef(uint32_t lldb_reg_num)
: FPOProgramNode(Register), m_lldb_reg_num(lldb_reg_num) {}
- void Accept(FPOProgramASTVisitor &visitor) override;
-
uint32_t GetLLDBRegNum() const { return m_lldb_reg_num; }
static bool classof(const FPOProgramNode *node) {
@@ -101,8 +93,6 @@
FPOProgramNodeIntegerLiteral(uint32_t value)
: FPOProgramNode(IntegerLiteral), m_value(value) {}
- void Accept(FPOProgramASTVisitor &visitor) override;
-
uint32_t GetValue() const { return m_value; }
static bool classof(const FPOProgramNode *node) {
@@ -126,8 +116,6 @@
: FPOProgramNode(BinaryOp), m_op_type(op_type), m_left(&left),
m_right(&right) {}
- void Accept(FPOProgramASTVisitor &visitor) override;
-
OpType GetOpType() const { return m_op_type; }
const FPOProgramNode *Left() const { return m_left; }
@@ -155,8 +143,6 @@
FPOProgramNodeUnaryOp(OpType op_type, FPOProgramNode &operand)
: FPOProgramNode(UnaryOp), m_op_type(op_type), m_operand(&operand) {}
- void Accept(FPOProgramASTVisitor &visitor) override;
-
OpType GetOpType() const { return m_op_type; }
const FPOProgramNode *Operand() const { return m_operand; }
@@ -171,84 +157,108 @@
FPOProgramNode *m_operand;
};
+template <typename ResultT = void>
class FPOProgramASTVisitor {
-public:
- virtual ~FPOProgramASTVisitor() = default;
+protected:
+ virtual ResultT Visit(FPOProgramNodeBinaryOp &binary,
+ FPOProgramNode *&ref) = 0;
+ virtual ResultT Visit(FPOProgramNodeUnaryOp &unary, FPOProgramNode *&ref) = 0;
+ virtual ResultT Visit(FPOProgramNodeRegisterRef ®, FPOProgramNode *&) = 0;
+ virtual ResultT Visit(FPOProgramNodeIntegerLiteral &integer,
+ FPOProgramNode *&) = 0;
+ virtual ResultT Visit(FPOProgramNodeSymbol &symbol, FPOProgramNode *&ref) = 0;
+
+ ResultT Dispatch(FPOProgramNode *&node) {
+ switch (node->GetKind()) {
+ case FPOProgramNode::Register:
+ return Visit(llvm::cast<FPOProgramNodeRegisterRef>(*node), node);
+ case FPOProgramNode::Symbol:
+ return Visit(llvm::cast<FPOProgramNodeSymbol>(*node), node);
+
+ case FPOProgramNode::IntegerLiteral:
+ return Visit(llvm::cast<FPOProgramNodeIntegerLiteral>(*node), node);
+ case FPOProgramNode::UnaryOp:
+ return Visit(llvm::cast<FPOProgramNodeUnaryOp>(*node), node);
+ case FPOProgramNode::BinaryOp:
+ return Visit(llvm::cast<FPOProgramNodeBinaryOp>(*node), node);
+ }
+ llvm_unreachable("Fully covered switch!");
+ }
- 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 FPOProgramNodeRegisterRef::Accept(FPOProgramASTVisitor &visitor) {
- visitor.Visit(*this);
-}
+class FPOProgramASTVisitorMergeDependent : public FPOProgramASTVisitor<> {
+public:
+ void Visit(FPOProgramNodeBinaryOp &binary, FPOProgramNode *&) override {
+ Dispatch(binary.Left());
+ Dispatch(binary.Right());
+ }
-void FPOProgramNodeIntegerLiteral::Accept(FPOProgramASTVisitor &visitor) {
- visitor.Visit(*this);
-}
+ void Visit(FPOProgramNodeUnaryOp &unary, FPOProgramNode *&) override {
+ Dispatch(unary.Operand());
+ }
-void FPOProgramNodeBinaryOp::Accept(FPOProgramASTVisitor &visitor) {
- visitor.Visit(*this);
-}
+ void Visit(FPOProgramNodeRegisterRef &, FPOProgramNode *&) override {}
+ void Visit(FPOProgramNodeIntegerLiteral &, FPOProgramNode *&) override {}
+ void Visit(FPOProgramNodeSymbol &symbol, FPOProgramNode *&ref) override;
-void FPOProgramNodeUnaryOp::Accept(FPOProgramASTVisitor &visitor) {
- visitor.Visit(*this);
-}
+ static void Merge(const llvm::DenseMap<llvm::StringRef, FPOProgramNode *>
+ &dependent_programs,
+ FPOProgramNode *&ast) {
+ FPOProgramASTVisitorMergeDependent(dependent_programs).Dispatch(ast);
+ }
-class FPOProgramASTVisitorMergeDependent : public FPOProgramASTVisitor {
-public:
+private:
FPOProgramASTVisitorMergeDependent(
const llvm::DenseMap<llvm::StringRef, FPOProgramNode *>
&dependent_programs)
: m_dependent_programs(dependent_programs) {}
- void Merge(FPOProgramNode *&node_ref);
-
-private:
- 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);
-}
+void FPOProgramASTVisitorMergeDependent::Visit(FPOProgramNodeSymbol &symbol,
+ FPOProgramNode *&ref) {
-void FPOProgramASTVisitorMergeDependent::Visit(FPOProgramNodeBinaryOp &node) {
- Merge(node.Left());
- Merge(node.Right());
-}
-void FPOProgramASTVisitorMergeDependent::Visit(FPOProgramNodeUnaryOp &node) {
- Merge(node.Operand());
+ auto it = m_dependent_programs.find(symbol.GetName());
+ if (it == m_dependent_programs.end())
+ return;
+
+ ref = it->second;
+ Dispatch(ref);
}
-void FPOProgramASTVisitorMergeDependent::TryReplace(
- FPOProgramNode *&node_ref) const {
+class FPOProgramASTVisitorResolveRegisterRefs
+ : public FPOProgramASTVisitor<bool> {
+public:
+ static bool Resolve(const llvm::DenseMap<llvm::StringRef, FPOProgramNode *>
+ &dependent_programs,
+ llvm::Triple::ArchType arch_type, NodeAllocator &alloc,
+ FPOProgramNode *&ast) {
+ return FPOProgramASTVisitorResolveRegisterRefs(dependent_programs,
+ arch_type, alloc)
+ .Dispatch(ast);
+ }
- while (auto *symbol = llvm::dyn_cast<FPOProgramNodeSymbol>(node_ref)) {
- auto it = m_dependent_programs.find(symbol->GetName());
- if (it == m_dependent_programs.end()) {
- break;
- }
+ bool Visit(FPOProgramNodeBinaryOp &binary, FPOProgramNode *&) override {
+ return Dispatch(binary.Left()) && Dispatch(binary.Right());
+ }
- node_ref = it->second;
+ bool Visit(FPOProgramNodeUnaryOp &unary, FPOProgramNode *&) override {
+ return Dispatch(unary.Operand());
}
-}
-class FPOProgramASTVisitorResolveRegisterRefs : public FPOProgramASTVisitor {
-public:
+ bool Visit(FPOProgramNodeRegisterRef &, FPOProgramNode *&) override {
+ return true;
+ }
+
+ bool Visit(FPOProgramNodeIntegerLiteral &, FPOProgramNode *&) override {
+ return true;
+ }
+
+ bool Visit(FPOProgramNodeSymbol &symbol, FPOProgramNode *&ref) override;
+
+private:
FPOProgramASTVisitorResolveRegisterRefs(
const llvm::DenseMap<llvm::StringRef, FPOProgramNode *>
&dependent_programs,
@@ -256,27 +266,11 @@
: 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;
-
- 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);
- 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 =
@@ -294,62 +288,49 @@
return npdb::GetLLDBRegisterNumber(arch_type, reg_id);
}
-bool FPOProgramASTVisitorResolveRegisterRefs::TryReplace(
- FPOProgramNode *&node_ref) {
- auto *symbol = llvm::dyn_cast<FPOProgramNodeSymbol>(node_ref);
- if (!symbol)
- return true;
-
+bool FPOProgramASTVisitorResolveRegisterRefs::Visit(
+ FPOProgramNodeSymbol &symbol, FPOProgramNode *&ref) {
// Look up register reference as lvalue in preceding assignments.
- auto it = m_dependent_programs.find(symbol->GetName());
+ auto it = m_dependent_programs.find(symbol.GetName());
if (it != m_dependent_programs.end()) {
// Dependent programs are handled elsewhere.
return true;
}
uint32_t reg_num =
- ResolveLLDBRegisterNum(symbol->GetName().drop_front(1), m_arch_type);
+ ResolveLLDBRegisterNum(symbol.GetName().drop_front(1), m_arch_type);
if (reg_num == LLDB_INVALID_REGNUM)
return false;
- node_ref = m_alloc.makeNode<FPOProgramNodeRegisterRef>(reg_num);
+ ref = m_alloc.makeNode<FPOProgramNodeRegisterRef>(reg_num);
return true;
}
-void FPOProgramASTVisitorResolveRegisterRefs::Visit(
- FPOProgramNodeBinaryOp &node) {
- m_no_error_flag = Resolve(node.Left()) && Resolve(node.Right());
-}
-
-void FPOProgramASTVisitorResolveRegisterRefs::Visit(
- FPOProgramNodeUnaryOp &node) {
- m_no_error_flag = Resolve(node.Operand());
-}
-
-class FPOProgramASTVisitorDWARFCodegen : public FPOProgramASTVisitor {
+class FPOProgramASTVisitorDWARFCodegen : public FPOProgramASTVisitor<> {
public:
- FPOProgramASTVisitorDWARFCodegen(Stream &stream) : m_out_stream(stream) {}
+ static void Emit(Stream &stream, FPOProgramNode *&ast) {
+ FPOProgramASTVisitorDWARFCodegen(stream).Dispatch(ast);
+ }
- void Emit(FPOProgramNode &program);
+ void Visit(FPOProgramNodeRegisterRef ®, FPOProgramNode *&);
+ void Visit(FPOProgramNodeBinaryOp &binary, FPOProgramNode *&);
+ void Visit(FPOProgramNodeUnaryOp &unary, FPOProgramNode *&);
+ void Visit(FPOProgramNodeSymbol &symbol, FPOProgramNode *&) {
+ llvm_unreachable("Symbols should have been resolved by now!");
+ }
+ void Visit(FPOProgramNodeIntegerLiteral &integer, FPOProgramNode *&);
private:
- void Visit(FPOProgramNodeRegisterRef &node) override;
- void Visit(FPOProgramNodeIntegerLiteral &node) override;
- void Visit(FPOProgramNodeBinaryOp &node) override;
- void Visit(FPOProgramNodeUnaryOp &node) override;
+ FPOProgramASTVisitorDWARFCodegen(Stream &stream) : m_out_stream(stream) {}
-private:
Stream &m_out_stream;
};
-void FPOProgramASTVisitorDWARFCodegen::Emit(FPOProgramNode &program) {
- program.Accept(*this);
-}
-
-void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeRegisterRef &node) {
+void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeRegisterRef ®,
+ FPOProgramNode *&) {
- uint32_t reg_num = node.GetLLDBRegNum();
+ uint32_t reg_num = reg.GetLLDBRegNum();
lldbassert(reg_num != LLDB_INVALID_REGNUM);
if (reg_num > 31) {
@@ -362,18 +343,18 @@
}
void FPOProgramASTVisitorDWARFCodegen::Visit(
- FPOProgramNodeIntegerLiteral &node) {
- uint32_t value = node.GetValue();
+ FPOProgramNodeIntegerLiteral &integer, FPOProgramNode *&) {
+ uint32_t value = integer.GetValue();
m_out_stream.PutHex8(DW_OP_constu);
m_out_stream.PutULEB128(value);
}
-void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeBinaryOp &node) {
+void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeBinaryOp &binary,
+ FPOProgramNode *&) {
+ Dispatch(binary.Left());
+ Dispatch(binary.Right());
- Emit(*node.Left());
- Emit(*node.Right());
-
- switch (node.GetOpType()) {
+ switch (binary.GetOpType()) {
case FPOProgramNodeBinaryOp::Plus:
m_out_stream.PutHex8(DW_OP_plus);
// NOTE: can be optimized by using DW_OP_plus_uconst opcpode
@@ -395,10 +376,11 @@
}
}
-void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeUnaryOp &node) {
- Emit(*node.Operand());
+void FPOProgramASTVisitorDWARFCodegen::Visit(FPOProgramNodeUnaryOp &unary,
+ FPOProgramNode *&) {
+ Dispatch(unary.Operand());
- switch (node.GetOpType()) {
+ switch (unary.GetOpType()) {
case FPOProgramNodeUnaryOp::Deref:
m_out_stream.PutHex8(DW_OP_deref);
break;
@@ -523,19 +505,16 @@
lldbassert(rvalue_ast);
// check & resolve assignment program
- FPOProgramASTVisitorResolveRegisterRefs resolver(dependent_programs,
- arch_type, alloc);
- if (!resolver.Resolve(rvalue_ast)) {
+ if (!FPOProgramASTVisitorResolveRegisterRefs::Resolve(
+ dependent_programs, arch_type, alloc, rvalue_ast))
return nullptr;
- }
if (lvalue_name == register_name) {
// found target assignment program - no need to parse further
// emplace valid dependent subtrees to make target assignment independent
// from predecessors
- FPOProgramASTVisitorMergeDependent merger(dependent_programs);
- merger.Merge(rvalue_ast);
+ FPOProgramASTVisitorMergeDependent::Merge(dependent_programs, rvalue_ast);
return rvalue_ast;
}
@@ -557,7 +536,6 @@
return false;
}
- FPOProgramASTVisitorDWARFCodegen codegen(stream);
- codegen.Emit(*target_program);
+ FPOProgramASTVisitorDWARFCodegen::Emit(stream, target_program);
return true;
}
_______________________________________________
lldb-commits mailing list
[email protected]
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
