Mark llvm::ConstantExpr::getAsInstruction as const

[AlexDenisov]

Summary

getAsInstruction is the only non-const member method.
It is impossible to enforce const-correctness because of it.

[jmolloy]

LGTM, but in which cases are we enforcing const-correctness? const-correctness is not a property LLVM enforces almost anywhere because of its virality.

[AlexDenisov]

@jmolloy in our case we use LLVM as a library. We use getAsInstruction down the road and it forces us to remove all const qualifiers from a custom InstVisitor even though we only read the values and do not modify anything.

[jmolloy]

Hi Alex,

Thanks. That's fair, and because this is the only member that wasn't const-correct I approved this change. But bear in mind in general that LLVM is not intended to be const-correct by design. More invasive patches may get more pushback.

Cheers,

James

[AlexDenisov]

Thanks for the heads up @jmolloy. Unfortunately, I learned the hard way that const qualified objects in LLVM are not necessarily constant and should not be considered as such.

llvm/include/llvm/IR/Constants.h

public:
 
   /// Returns an Instruction which implements the same operation as this
   /// ConstantExpr. The instruction is not linked to any basic block.
   ///
   /// A better approach to this could be to have a constructor for Instruction
   /// which would take a ConstantExpr parameter, but that would have spread
   /// implementation details of ConstantExpr outside of Constants.cpp, which
   /// would make it harder to remove ConstantExprs altogether.
-  Instruction *getAsInstruction();
+  Instruction *getAsInstruction() const;
 
   /// Methods for support type inquiry through isa, cast, and dyn_cast:
   static bool classof(const Value *V) {
     return V->getValueID() == ConstantExprVal;
   }
 
 private:
   // Shadow Value::setValueSubclassData with a private forwarding method so that

llvm/lib/IR/Constants.cpp

Value *ConstantExpr::handleOperandChangeImpl(Value *From, Value *ToV) {
   if (Constant *C = getWithOperands(NewOps, getType(), true))
     return C;
 
   // Update to the new value.
   return getContext().pImpl->ExprConstants.replaceOperandsInPlace(
       NewOps, this, From, To, NumUpdated, OperandNo);
 }
 
-Instruction *ConstantExpr::getAsInstruction() {
+Instruction *ConstantExpr::getAsInstruction() const {
   SmallVector<Value *, 4> ValueOperands(op_begin(), op_end());
   ArrayRef<Value*> Ops(ValueOperands);
 
   switch (getOpcode()) {
   case Instruction::Trunc:
   case Instruction::ZExt:
   case Instruction::SExt:
   case Instruction::FPTrunc: