[PATCH] D63847: [MC] Add MCInstrAnalysis::evaluateMemoryOperandAddress

Seiya Nuta via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon Jul 1 00:49:43 PDT 2019


seiya marked an inline comment as done.
seiya added inline comments.


================
Comment at: llvm/include/llvm/MC/MCInstrAnalysis.h:157
+  /// true on success, and the address in Target.
+  virtual bool
+  evaluateMemoryOperandAddress(const MCInst &Inst, uint64_t Addr, uint64_t Size,
----------------
bcain wrote:
> It may make more sense for this to return an llvm::Optional<uint64_t>.
It returns bool for consistency with `evaluateBranch` but I think Optional is better too. What do you think about the consistency with other method definitions? I don't have a strong opinion.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63847/new/

https://reviews.llvm.org/D63847





More information about the llvm-commits mailing list