Page MenuHomePhabricator

Mark llvm::ConstantExpr::getAsInstruction as const
ClosedPublic

Authored by AlexDenisov on Nov 12 2019, 2:20 AM.

Details

Summary

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

Diff Detail

Event Timeline

AlexDenisov created this revision.Nov 12 2019, 2:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptNov 12 2019, 2:20 AM
jmolloy accepted this revision.Nov 12 2019, 5:11 AM

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

This revision is now accepted and ready to land.Nov 12 2019, 5:11 AM

@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.

This revision was automatically updated to reflect the committed changes.

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

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.