[Feature Request][compiler-v2] refactor/unify visibility checking

[brmataptos]

🚀 Feature Request

PR https://github.com/aptos-labs/aptos-core/pull/11079 will add visibility checking for calls to inline functions as part of a function_checker module (function check_access_and_use). Meanwhile, a struct VisibilityChecker implements FunctionTargetProcessor to do visibility checking at the bytecode level for other functions. The resulting code seems a bit redundant, but reducing this redundancy will require some thought or possibly too much mechanism.

Some comments from @wrwg on that PR:

Generally looks good, but regards the visibility checks, I'm concerned about landing without the unification with the visibility checker work. There are actually two ways we can unify:

Move visibility checker logic over here. Have a special expression (and later opcode) to mark inlined functions, then let the existing visibility checker handle it For the later, there is basically a Call(_, Operation::InlinedBlock(qual_inst_id), [exp]) and then we add also a bytecode. There may be other advantages to marking the inlined code beside this one, so I wonder whether this might be the better approach.

Yeah, definitely wait -- if it would be done in the current visibility checker, then it would be also likely Vineeth to do it. Notice however, that the design philosophy is (written in the design doc also since long) to do as much as we can on bytecode level.

The solution is simple: wrap the inlined expansion into a special Operation::InlinedFunction(fun_id). This is semantically an identity function. Then on stackless bytecode level, generate begin and end markers for inlined code section. This enables the current visibility checker to check correct visibility of inline functions. Happily roll up a small PR setting this up once we agree.

Comment from Brian

Scoped marker expressions like this seem like a brittle hack. As you optimize code it will become useless. Probably sufficient to have a single libary usable from both places (pre-inlining and iin-bytecode) to reduce redundancy, though with the lack of abstraction common in Rust this may not be useful: the convenience of using match expressions leads to code which isn't portable between different representations, even if they abstractly represent the same thing.