IRGeneratorForStatements: Remove outdated check against usr$ prefixing of builtins

[clonker]

Builtins are mapped to BuiltinHandles instead of YulNames.

[cameel]

Now that we don't handle builtins in this function, the comment is a bit out of context and does not even refer to the code that is below it, because that only handles identifiers. We should put it somewhere else or at least note that it refers to builtins, which are not processed here.

It's still relevant though, just now it's about mapping builtin handles rather than names. And m_dialect no longer exists.

// Strictly, the dialect used by inline assembly (m_dialect) could be different
// from the Yul dialect we are compiling to. So we are assuming here that the builtin
// functions are identical. This should not be a problem for now since everything
// is EVM anyway.

The fact that we're not checking this is actually quite dangerous now that we transitioned to handles. Before, it was enough that all builtins from the source dialect existed in the target dialect as well and had matching names, which was relatively easy to ensure in EVMDialect::createBuiltins(). With sequential numeric IDs though the names do not matter and we instead rely on defining them in the same order, which is less visible and can easily be affected by making builtins conditional.

For a moment I was worried that we already broke it with EOF, but looks like it so happened that the builtins unavailable in inline assembly were last, so they weren't shifting other IDs when unevailable. And then #15783 made them all unavailable. This could easily break in the future though - we're planning to enable eofcreate() and it's the middle one there. If we don't change the order while doing that, we'll end up with eofcreate() in inline assembly being translated to returncontract() in full Yul.

We should at least add some big fat warnings in createBuiltins() to be very careful with ordering. But it would be much better if we could have sanity checks.

I guess this goes back to my concern that handles are just plain numbers, not associated with the dialect.

[cameel]

Actually, you did secure it a bit with the createIfObjectAccess(), which still reserves the ID even if the builtin does not exist. So enabling eofcreate() should have been safe... but isn't :) The EOF code ignores your helper and uses an ordinary condition. We need to fix that.

So we should still add some warnings in createBuiltins() and mention that we do rely on this property in practice. It's just too easy to violate it by not paying attention.

[clonker]

Yeah you are absolutely right. I will update the comment here and have opened PR #15961 that should improve the situation. Also contains the fix for EOF.