Remove ExplicitSelf from AST

[petrochenkov]

The AST part of #33505.
#33505 isn't landed yet, so this PR is based on top of it.

r? @nrc

plugin-[breaking-change] cc #31645 @Manishearth

[petrochenkov]

The logic is the same as in #33505: self argument is kept in the argument list as a usual Arg and treated as such for most purposes.
Sometimes (rarely enough), for example for pretty-printing, it's convenient to represent self arguments in more "selfy" way. In this case the first Arg can be converted to ast::ExplicitSelf, see Arg::to_self/FnDecl::get_self. ast::ExplicitSelfs however aren't kept in the AST.
Based on the refactorings I did, this scheme feels the most adequate, @nrc metioned a better scheme in #33505 (comment) but I'm not sure what can be done better.

Regarding self_shortcut: bool, the only purpose of it now is to report errors for self: _ and self: &_. Given that _ usually means an inferred type in type context and the types of self and &self are indeed inferred, maybe it would be reasonable to just allow these forms.

PS: derive for methods is a huge mess and I mostly left it as is.

[nrc]

I'd like to explore the design space a bit here. My motivation is that the AST should be as close as possible to the source text. It is meant to be useful as a representation of the source, rather than a more abstract representation of the program (which is the job of the HIR). As such, I think it is less important to make the AST as small as possible at the expense of losing information about the original program. The fact that we need the bool hack in MethodSig in order to accomodate self as a regular argument suggests to me that the homogeneous argument approach is not the right one here.

What is the motivation for this? AIUI the major problem is that explicit self is represented both by an argument and by an ExplicitSelf. Are there other goals here too?

The obvious fix seems to be remove it from the argument list (clearly it ought to be in the argument list in the HIR, but I don't see a need in the AST). Alternatively, we could make an Argument enum with an explicit self variant and a 'regular' argument variant and make the inputs of FnDecl an array of that enum. The downside of this is that we don't limit self to methods or to the first argument position.

[petrochenkov]

What is the motivation for this?

To remove as much code as possible :)
On a serious note, all current passes running on AST (and later on HIR), except for pretty printing, want self argument to be treated uniformly with the rest of the argument list. Otherwise, they have to add some special code for processing ExplicitSelf separately or joining self and non-self arguments in one sequence. Even pretty printing doesn't want to split the arguments into two lists, only to treat self arguments differently. So, keeping self and the rest of argument list separately would be pretty inconvenient.
(For reference, here's the original old PR by @eddyb that made self a part of argument list: #11595)

My motivation is that the AST should be as close as possible to the source text.

Indeed! However, as long as one of the two representations (separate self, which may be intuitively close to the source, vs uniform self) can be easily and losslessly converted to another, it's largely a question of defaults. And I argue, that the uniform default is more convenient.

(The current duplication is the worst from both worlds, for example, name resolution for identifiers is done on the self from argument list, and name resolution for lifetimes is done on self from explicit_self. Also, hir::ExplicitSelf used Names and I wondered "How on earth self can be hygienic without SyntaxContext?!" until I discovered, that self was also kept in the argument lists and was resolved from there.)

I've pushed one more commit, adding a new AST-only type variant TyKind::SelfInfer, it makes the conversion between Arg and ExplicitSelf lossless even in presence of ill-formed code like self: _ and renders the hacky flag self_shortcut unnecessary. This is probably a more clear solution.

[nrc]

Thanks for the update, I like the idea of using SelfInfer as a type, and agree it should be AST only. However, it needs a better name, since we don't do any inference here. Maybe SelfArg or ImplicitSelf or something?

[eddyb]

👍 on ImplicitSelf.

[petrochenkov]

SelfInfer is replaced with ImplicitSelf.

ping @Manishearth, this is supposed to be a part of the breaking batch too

[Manishearth]

Is this reviewed? if not, could someone review it?

[nrc]

@Manishearth: r+