Branch: refs/heads/thaines/expression_base Home: https://github.com/dyninst/dyninst Commit: d9a20aed6e620517652998d7c6e5ead55fcfa4c4 https://github.com/dyninst/dyninst/commit/d9a20aed6e620517652998d7c6e5ead55fcfa4c4 Author: Tim Haines <thaines.as...@gmail.com> Date: 2025-04-17 (Thu, 17 Apr 2025)
Changed paths: M dataflowAPI/src/Absloc.C M dataflowAPI/src/AbslocInterface.C M dyninstAPI/src/StackMod/StackAccess.C M instructionAPI/doc/2-Abstractions.tex M instructionAPI/doc/3-API.tex M instructionAPI/doc/API/BinaryFunction.tex M instructionAPI/doc/API/Dereference.tex M instructionAPI/doc/API/Expression.tex M instructionAPI/doc/API/Immediate.tex R instructionAPI/doc/API/InstructionAST.tex M instructionAPI/doc/API/MultiRegisterAST.tex M instructionAPI/doc/API/RegisterAST.tex M instructionAPI/doc/API/TernaryAST.tex M instructionAPI/h/BinaryFunction.h M instructionAPI/h/Dereference.h M instructionAPI/h/Expression.h M instructionAPI/h/Immediate.h M instructionAPI/h/InstructionAST.h M instructionAPI/h/MultiRegister.h M instructionAPI/h/Register.h M instructionAPI/h/Ternary.h M instructionAPI/src/Expression.C M instructionAPI/src/Immediate.C M instructionAPI/src/MultiRegister.C M instructionAPI/src/Operand.C M instructionAPI/src/Register.C M instructionAPI/src/Ternary.C M instructionAPI/src/full_inheritance_graph.dot M patchAPI/src/PatchBlock.C Log Message: ----------- Make Expression the base of the InstructionAPI AST hierarchy The InstructionAST abstraction doesn't provide any functionality beyond its immediate child, Expression. This is particularly evident by 1) all uses of InstructionAST in Dyninst are immediately converted to an Expression 2) the lackluster description provided in the docs: The InstructionAST class is the base class for all nodes in the ASTs used by the Operand class. It defines the necessary interfaces for traversing and searching an abstract syntax tree representing an operand. But the real problem is with 'getChildren'. Because both InstructionAST::getChildren(std::vector<InstructionAST::Ptr> const&)=0 and Expression::getChildren(std::vector<Expression::Ptr> const&)=0 exist, then every AST type has to implement both which is redundant and provides no logical separation between an InstructionAST and an Expression. In C++, these are referred to as overloaded virtual functions and are generally a sign of an error since the one from the base class will be hidden (see -Woverloaded-virtual). The author "overcame" this by making them pure virtual. That doesn't remove the logical incorrectness, though. InstructionAST should have implemented 'getChildren' (or even better, 'getChildNodes') and Expression should have implemented 'getSubexpressions'. Then all of this would have been separated in a useful way. This merger is an ABI break only. No user code changes will be needed. To unsubscribe from these emails, change your notification settings at https://github.com/dyninst/dyninst/settings/notifications _______________________________________________ Dyninst-api mailing list Dyninst-api@cs.wisc.edu https://lists.cs.wisc.edu/mailman/listinfo/dyninst-api