[llvm-branch-commits] [llvm] [BOLT] Gadget scanner: clarify MCPlusBuilder callbacks interface (PR #136147)

[Kristof Beyls via llvm-branch-commits]

================
@@ -787,19 +794,20 @@ shouldReportSigningOracle(const BinaryContext &BC, const MCInstReference &Inst,
                           const SrcState &S) {
   static const GadgetKind SigningOracleKind("signing oracle found");
 
-  MCPhysReg SignedReg = BC.MIB->getSignedReg(Inst);
-  if (SignedReg == BC.MIB->getNoRegister())
+  std::optional<MCPhysReg> SignedReg = BC.MIB->getSignedReg(Inst);
+  if (!SignedReg)
     return std::nullopt;
 
+  assert(*SignedReg != BC.MIB->getNoRegister());
----------------
kbeyls wrote:

This assert looks a bit strange. Since we're using `std::optional<MCPhysReg>` everywhere now, is there any reason why `*SignedReg` could be `getNoRegister()` here more than in any other place where we call an API returning `std::optional<MCPhysReg>`?
If not, maybe it's better to remove this assert?

https://github.com/llvm/llvm-project/pull/136147