Stream: t-compiler

Topic: #60406


Caio (May 01 2019 at 15:05, on Zulip):

Can someone guide me on issue #60406?

nikomatsakis (May 01 2019 at 15:13, on Zulip):

Tracking issue for RFC 2565, "Attributes in formal function parameter position" #60406

nikomatsakis (May 01 2019 at 15:13, on Zulip):

For context :)

nikomatsakis (May 01 2019 at 15:13, on Zulip):

I feel like @pnkfelix added support for this? Oh, no, it was type parameters that they added support for

nikomatsakis (May 01 2019 at 15:13, on Zulip):

Still, that PR might serve as a good "guidepost"

Caio (May 02 2019 at 15:55, on Zulip):

Unfortunately, I was unable to find on GitHub this specific PR from @pnkfelix. Still looking though

centril (May 02 2019 at 16:07, on Zulip):

@Caio https://github.com/rust-lang/rust/pull/34764

centril (May 02 2019 at 16:07, on Zulip):

also cc @Vadim Petrochenkov

Caio (May 03 2019 at 01:21, on Zulip):

@centril Thanks!

Caio (May 08 2019 at 17:17, on Zulip):

libsyntax/parse/parser.rs -> The parsing of a function parameter passes through parse_pat_with_range_pat and then lit_token, which tells if a token is illegal or not. In my case, expected argument name, found '#' for something like foo(#[bar] u8)

Caio (May 08 2019 at 17:23, on Zulip):

Should another literal by added in token::Lit to handle this new feature?

Caio (May 08 2019 at 18:13, on Zulip):

Actually, NVM. I was in the wrong trail

Caio (May 08 2019 at 20:47, on Zulip):

Are the tokens normalized in parser.rs?

Caio (May 08 2019 at 20:49, on Zulip):

Like, the user provided foo( #[bar] field : u8) and the additional whitespaces are already stripped in parser.rs

Caio (May 08 2019 at 20:57, on Zulip):

Also, now that ast::Arg has Attributes, how should I expose it to the user through the proc_macro crate?

Caio (May 12 2019 at 19:40, on Zulip):

I don't want to be a boring person but these questions came out of despair

Caio (May 12 2019 at 19:40, on Zulip):

I know you guys are too busy and I'm sorry for bothering

Caio (May 12 2019 at 19:44, on Zulip):

The only thing left to complete this feature is macro resolution, which I couldn't fully understand after hours of struggle

Caio (May 12 2019 at 19:45, on Zulip):

Any tips about it would be very welcoming

Caio (May 12 2019 at 19:45, on Zulip):

PR is in https://github.com/rust-lang/rust/pull/60669

centril (May 12 2019 at 19:51, on Zulip):

cc @Vadim Petrochenkov ^--- since you are reviewing the PR

Vadim Petrochenkov (May 12 2019 at 19:56, on Zulip):

Sorry for the delay, I'll get to this PR during the week.

Vadim Petrochenkov (May 12 2019 at 19:57, on Zulip):

Macros are not supposed to be resolved in that position (not until some infrastructural pre-requisite work is done at least).

Vadim Petrochenkov (May 12 2019 at 19:59, on Zulip):

Look what happens with attributes on e.g. generic parameters fn f<#[attr] T>, they are not resolved, they are kind of whitelisted through a hack in feature_gate.rs.

Vadim Petrochenkov (May 12 2019 at 19:59, on Zulip):

(Which doesn't require any extra work for fn param attributes specifically.)

Vadim Petrochenkov (May 12 2019 at 20:01, on Zulip):

cfg support will need to be implemented though, so I'd recommend to look at that (libsyntax/config.rs, by analogy with attributes in other positions).

Vadim Petrochenkov (May 12 2019 at 20:04, on Zulip):

not until some infrastructural pre-requisite work is done at least

That's why I always had a feeling that it's not the best time to implement support for attributes in more locations right now.

Vadim Petrochenkov (May 12 2019 at 20:05, on Zulip):

I hoped for that the RFC staying unimplemented for some time, but well...

Vadim Petrochenkov (May 12 2019 at 20:06, on Zulip):

(The pre-requisite work is in my queue, it's a part of the macro modularization work, I've just consistently happened to be busy in February-April.)

Caio (May 12 2019 at 23:02, on Zulip):

Thank you @Vadim Petrochenkov , this shed some light on the matter

Caio (May 15 2019 at 22:16, on Zulip):

The closest I could get in the lexer is the following:

'(' => {
    self.bump();
    if self.ch_is('#') && self.nextch_is('[') && ident_start(self.nextnextch()) {
        loop {
            let raw_start = self.pos;
            self.bump();
            while ident_continue(self.ch) { self.bump(); }
            let span = self.mk_sp(raw_start, self.pos);
            self.sess.param_attr_spans.borrow_mut().push(span);
            self.bump();
            if !self.ch_is('#')
                || !self.nextch_is('[')
                || !ident_start(self.nextnextch())
            {
                break;
            }
        }
        Ok(token::CloseDelim(token::Bracket))
    }
    else {
        Ok(token::OpenDelim(token::Paren))
    }
}
Caio (May 15 2019 at 22:18, on Zulip):

Unfortunately, this won't work because of tuple structs: struct Foo(#[attr] i32)

Vadim Petrochenkov (May 16 2019 at 09:56, on Zulip):

@Caio , are you trying to collect spans for feature gating? (https://github.com/rust-lang/rust/pull/60669#discussion_r283902442)

In that case you need the parser, not lexer, the spans can be collected on every call to fn parse_arg_attributes.

Caio (May 16 2019 at 15:19, on Zulip):

@Vadim Petrochenkov Sorry, it was a misunderstanding of mine (lexer instead of parser).

Caio (May 16 2019 at 15:19, on Zulip):

Thanks!

Caio (May 17 2019 at 11:30, on Zulip):

What would be the best thing to add into the Annotable enum?
According to the RFC, people expect pat: type, so I think it should be Arg((P<ast::Pat>, P<ast::Ty>))

Caio (May 17 2019 at 11:33, on Zulip):

Or, instead, should a new member be added into InvocationKind?

pub enum InvocationKind {
    Bang { ... },
    Attr { ... },
    ArgAttr { ... },
    Derive { ... },
}
Vadim Petrochenkov (May 17 2019 at 12:17, on Zulip):

@Caio
Why do you need to touch Annotatable?

Vadim Petrochenkov (May 17 2019 at 12:17, on Zulip):

Or InvocationKind.

Vadim Petrochenkov (May 17 2019 at 12:17, on Zulip):

That's the macro support which is out of scope now.

Vadim Petrochenkov (May 17 2019 at 12:21, on Zulip):

Use any other non-macro attribute position as an example (e.g. attributes on match arms), they are implemented without ever working with Annotatable, or AstFragment, or all that macro machinery.

Caio (May 17 2019 at 12:22, on Zulip):

Actually, I don't know why exactly :P
Just wanted to expand attributes inside arguments

Caio (May 17 2019 at 12:22, on Zulip):

(e.g. attributes on match arms)

Thanks! I will look into the PR

Caio (May 19 2019 at 01:53, on Zulip):

@Vadim Petrochenkov The user needs a way to declare a custom macro for parameters
Since proc_macro_attribute isn't allowed, I guess a new attribute like proc_macro_arg_attribute would be required for proc-macro crates
That's right?

Vadim Petrochenkov (May 19 2019 at 09:18, on Zulip):

The user needs a way to declare a custom macro for parameters

The RFC says this is not supported.
If it's supported in the future, then it will be done directly with proc_macro_attribute, without introducing a new proc_macro_* attribute.

Caio (May 19 2019 at 10:22, on Zulip):

@Vadim Petrochenkov Thanks!

Caio (May 19 2019 at 19:09, on Zulip):

@Vadim Petrochenkov This Pr (https://github.com/rust-lang/rust/pull/12812) that enables attrs on match is from 2014 (pre 1.0) and didn't help much

Caio (May 19 2019 at 19:11, on Zulip):

I also tried to see get some clues from fold.rs to no avail

Caio (May 19 2019 at 19:14, on Zulip):

I tried and I am tying really hard to expand macros but still I have no idea where to start

Caio (May 19 2019 at 19:15, on Zulip):

Can you indicate some file or anything that can help me?

Vadim Petrochenkov (May 20 2019 at 09:15, on Zulip):

This Pr (https://github.com/rust-lang/rust/pull/12812) that enables attrs on match is from 2014 (pre 1.0) and didn't help much

Even if it's old it still contains all the relevant pieces of code actually (I rather meant looking at the current state of match arm attributes than that PR from 2014).
Your PR already implements everything except for the libsyntax/config.rs part.
As I previously said, expanding macros is out of scope for this PR, no need to think about it right now.
#[cfg] expansion is a separate mechanism from macros, if that clarifies the matter.

Caio (May 20 2019 at 12:38, on Zulip):

@Vadim Petrochenkov Finally got it. I wasn't calling the expand.rs bits, only the config.rs stuff

Caio (May 23 2019 at 10:11, on Zulip):

@centril @Vadim Petrochenkov I managed to finish the last (hopefully) issues

Vadim Petrochenkov (May 23 2019 at 10:16, on Zulip):

I started reviewing yesterday, but run out of time, unfortunately.
I'll try to do it today.

Caio (May 23 2019 at 10:56, on Zulip):

Thanks! Take your time

Caio (May 24 2019 at 18:18, on Zulip):

@Vadim Petrochenkov I think unused_variables needs implementation, maybe in liveness.rs?

Caio (May 24 2019 at 18:19, on Zulip):

If so, could you describe some brief steps to implement it?

Vadim Petrochenkov (May 24 2019 at 18:25, on Zulip):

Guess 1: All UI tests disable unused warnings by default to avoid noise.
Make sure that they are enabled for the test you are working on.

Vadim Petrochenkov (May 24 2019 at 18:28, on Zulip):

Guess 2: Function parameters do not have "lint scopes", those scopes are used to demark regions of code on which allow/warn/etc attributes work.
In this case you'll need to do something very similar to what this recent commit does - https://github.com/rust-lang/rust/pull/60174/commits/e784595c280da04c98e76a5ce9d603b58f6a88e2

Vadim Petrochenkov (May 24 2019 at 18:31, on Zulip):

I.e. the expected changes are in librustc/lint/mod.rs.
unused_variables logic by itself should work correctly, the issue is with enabling/disabling diagnostic output from it (which is what allow/warn and friends do).

yodal (May 24 2019 at 20:18, on Zulip):

If you wanted to test guess 1 you could run the compiler directly rather than through the test framework

Caio (May 25 2019 at 00:17, on Zulip):

@Vadim Petrochenkov Sorry for the delay, I had to leave my PC

Caio (May 25 2019 at 00:19, on Zulip):

I think it is probably going to be Guess 2. Nevertheless, all these information are very important. Thank you

Caio (May 25 2019 at 00:21, on Zulip):

@yodal Thank you for the suggestion. By the way, you made a terrific job on const generics

yodal (May 25 2019 at 00:23, on Zulip):

Thank you, though I insist @varkor gets most of the credit as he did most of the hard work

Caio (Jun 04 2019 at 01:31, on Zulip):

I am stuck with the same problem for 2 whole days. Very sad :/

Caio (Jun 04 2019 at 01:34, on Zulip):

fn foo(#[cfg_attr(included_flag, cfg(not_included_flag)] a: i32) successfully removes the parameter but fn foo(#[cfg_attr(not_included_flag, cfg(included_flag)] a: i32) does not

Caio (Jun 04 2019 at 01:35, on Zulip):

I tried so many different files, compiled and tested things over and over to no avail

Vadim Petrochenkov (Jun 04 2019 at 12:27, on Zulip):

Isn't that expected behavior?

Vadim Petrochenkov (Jun 04 2019 at 12:29, on Zulip):

If we are expanding step-by-step:
- fn foo(#[cfg_attr(included_flag, cfg(not_included_flag)] a: i32) -> fn foo(#[cfg(not_included_flag)] a: i32) -> fn foo()
- fn foo(#[cfg_attr(not_included_flag, cfg(included_flag)] a: i32) -> fn foo(a: i32)

Vadim Petrochenkov (Jun 04 2019 at 12:31, on Zulip):

fn foo(#[cfg_attr(included_flag, anything)] a: i32) -> fn foo(#[anything] a: i32)
fn foo(#[cfg_attr(not_included_flag, anything)] a: i32) -> fn foo(a: i32)

Caio (Jun 05 2019 at 16:53, on Zulip):

HAHAHAHAHAHAHAHAHAHAHAHA

Caio (Jun 05 2019 at 16:53, on Zulip):

This is even sadder

Caio (Jun 05 2019 at 16:53, on Zulip):

:cry:

Caio (Jun 05 2019 at 16:55, on Zulip):

Thanks @Vadim Petrochenkov

Vadim Petrochenkov (Jun 05 2019 at 17:00, on Zulip):

:slight_smile:

Caio (Jun 14 2019 at 19:16, on Zulip):

I did everything @oli said in #61238 but things didn't work out as expected. I guess liveness.rs needs some kind of modification because this line is what triggers the linting messages.

oli (Jun 14 2019 at 19:18, on Zulip):

@Caio can you open a PR with your code so I can have a look?

oli (Jun 14 2019 at 19:18, on Zulip):

oh, now I understand

oli (Jun 14 2019 at 19:19, on Zulip):

yea, liveness would need a change then

oli (Jun 14 2019 at 19:19, on Zulip):

hmm

oli (Jun 14 2019 at 19:20, on Zulip):

weird, it's using the correct HirIds in https://github.com/rust-lang/rust/blob/fc550d4295a654f9e7c621d957d81fbf1426c70b/src/librustc/middle/liveness.rs#L1580

Caio (Jun 14 2019 at 19:22, on Zulip):

@oli I tried to figure out myself but couldn't find the root cause. Any clues are very welcome

Caio (Jun 14 2019 at 19:24, on Zulip):

Sure, I will open a PR in next few hours.

Caio (Jun 17 2019 at 19:27, on Zulip):

ast::visit and hir::intravisit have not been greatly modified because the arguments are already visited in visit_fn_decl and visit_body, respectively

Caio (Jun 17 2019 at 19:28, on Zulip):

For all closures, bare function, function pointers, blocks and impl blocks

Caio (Jun 17 2019 at 19:29, on Zulip):

Am I in the right track?

Caio (Jun 17 2019 at 19:30, on Zulip):

@oli @Vadim Petrochenkov #61856

oli (Jun 18 2019 at 10:30, on Zulip):

@Caio I believe you also need to teach https://github.com/rust-lang/rust/blob/fff08cb04389497d254fb40948674cbbee402908/src/librustc/lint/mod.rs#L809 about the new attributes

oli (Jun 18 2019 at 10:31, on Zulip):

the unused_variable lint is not a regular lint

oli (Jun 18 2019 at 10:31, on Zulip):

also your early-lint-pass changes do not do the attribute dance, only the late-lint-pass changes do

Caio (Jun 19 2019 at 12:27, on Zulip):

I will take Closure for example

Caio (Jun 19 2019 at 12:29, on Zulip):

LintLevelMapBuilder implements visit_expr from Visitor that calls hir_visit::walk_expr with with_lint_attrs

Caio (Jun 19 2019 at 12:31, on Zulip):

Inside walk_expr, the ExprKind::Closure part calls visit_fn that calls walk_fn

Caio (Jun 19 2019 at 12:33, on Zulip):

walk_fn ultimately calls walk_body which visits all arguments and its attributes

Caio (Jun 19 2019 at 12:36, on Zulip):

visti_arg method of LintLevelMapBuilder is also properly "overridden" and should be probably being called for each visit_arg invocation of intravisit::Visitor

Caio (Jun 19 2019 at 12:39, on Zulip):

@oli Do you mean that I should re-implement this logic inside LintLevelMapBuilder?

oli (Jun 19 2019 at 12:41, on Zulip):

@Caio no, that logic is all good, what I believe is missing is a visit_arg override that wraps the walk_arg call in a with_lint_attrs call

oli (Jun 19 2019 at 12:42, on Zulip):

the with_lint_attrs always needs to happen on the specific item like you did in https://github.com/rust-lang/rust/pull/61856/files#diff-837439bcaa26732bb48bcbb0c60716ccR815

oli (Jun 19 2019 at 12:42, on Zulip):

it's not the point that with_lint_attrs is ever called, but that it is called on each node that can have lint attributes

oli (Jun 19 2019 at 12:42, on Zulip):

and since we now can have lint attributes on Arg, we need to implement visit_arg to honor these lint attributes

Caio (Jun 19 2019 at 12:55, on Zulip):

Oddly enough, the unsued_variables warnings are still appearing even after a wint_lint_attrs wrapper for both early and late lint pass

Caio (Jun 19 2019 at 12:55, on Zulip):

The current implementation is roughly based on visit_arm, which doesn't use the with_lint_attrscall in some key parts

oli (Jun 19 2019 at 12:57, on Zulip):

unused_variables does not use early or late lint passes

oli (Jun 19 2019 at 12:57, on Zulip):

it uses the LintLevelMapBuilder

oli (Jun 19 2019 at 12:57, on Zulip):

it's a hardwired lint

oli (Jun 19 2019 at 12:58, on Zulip):

they have their own visiting logic

oli (Jun 19 2019 at 12:58, on Zulip):

and then use the LintLevelMapBuilder transitively via span_lint

Caio (Jun 19 2019 at 12:58, on Zulip):

@oli I see

Caio (Jun 19 2019 at 13:01, on Zulip):

Your feedback is very important. Thank you for helping me

Caio (Jun 19 2019 at 13:02, on Zulip):

In the meanwhile, I'll keep trying to fix the lint

Caio (Jun 20 2019 at 13:35, on Zulip):

I managed to make it work

Caio (Jun 20 2019 at 13:35, on Zulip):

Turns out, the Hir part wasn't implemented

Caio (Jun 20 2019 at 13:36, on Zulip):

I will be away for a while but a new commit might appear in the next days

Caio (Jun 20 2019 at 13:37, on Zulip):

Thank you guys

Caio (Jun 24 2019 at 15:32, on Zulip):

Unfortunately, only unused_variables works

Caio (Jun 24 2019 at 15:34, on Zulip):

A proper scope probably needs to be added like in https://github.com/rust-lang/rust/pull/60174/commits/f506aea1fac0977c7215b4240f4d99b45bf7ae97

Caio (Jun 24 2019 at 15:35, on Zulip):

I tried to do so but nothing seemed to work

Caio (Jun 24 2019 at 15:36, on Zulip):

What is hair?

Caio (Jun 24 2019 at 17:38, on Zulip):

https://github.com/rust-lang/rust/pull/61856/commits/76da8558416416c0f6e216d342fa87d8de6ee6d9

oli (Jun 24 2019 at 20:43, on Zulip):

HAIR is an intermediate datastructure for converting HIR to MIR

oli (Jun 24 2019 at 20:44, on Zulip):

I'm not sure what you mean by proper scopes

Caio (Jun 24 2019 at 22:49, on Zulip):

HAIR is an intermediate datastructure for converting HIR to MIR

Thanks!

Caio (Jun 24 2019 at 22:49, on Zulip):

I'm not sure what you mean by proper scopes

Honestly, me neither :/

Caio (Jun 24 2019 at 22:53, on Zulip):

By seeing the related commit, it looks like that something is missing to make unused_mut and other types of lints to actually work. This might be related to some kind of lint scope

Caio (Jun 24 2019 at 23:01, on Zulip):

At first, I thought that such scope was the pattern span being inserted and collected here

oli (Jun 25 2019 at 08:19, on Zulip):

I would start by looking where unused_mut is emitted. If it's being emitted by giving it a HirId just like unused_variables, then it should have worked automatically. If it is a LateContext, then it's related to the intravisit::Visitor impl used for calling the LateContext::check_* functions. If it's something else entirely (like a custom intravisit::Visitor, then that visitor probably needs an override of visit_arg just like the others.

Caio (Jun 25 2019 at 19:45, on Zulip):

@oli Thanks

Caio (Jun 25 2019 at 19:46, on Zulip):

I think I am getting the idea

Caio (Jun 25 2019 at 19:47, on Zulip):

In order to make unused_mut work, the argument local must be inserted into used_mut as stated in https://github.com/rust-lang/rust/blob/04a3dd8a872633ca1e4c217d11f741cc35cb19a5/src/librustc_mir/borrow_check/mod.rs#L321

Caio (Jun 25 2019 at 19:49, on Zulip):

To do so, the DataflowResultsConsumer implementation for MirBorrowckCtxt must be modified

Caio (Jun 25 2019 at 19:50, on Zulip):

Because used_mut is initially populated on the analyze_results call -> https://github.com/rust-lang/rust/blob/04a3dd8a872633ca1e4c217d11f741cc35cb19a5/src/librustc_mir/borrow_check/mod.rs#L276

Caio (Jun 25 2019 at 19:53, on Zulip):

Probably here or here

Caio (Jun 25 2019 at 19:54, on Zulip):

How should the DataflowResultsConsumer be modified to collect the arguments locals into used_mut?

Caio (Jun 25 2019 at 19:59, on Zulip):

Sorry, I might be asking for too much but any clue will be very welcome as this whole MIR stuff is pretty new to me

oli (Jun 26 2019 at 07:38, on Zulip):

hmm... i would not abort this early, I think instead we should look into why https://github.com/rust-lang/rust/blob/04a3dd8a872633ca1e4c217d11f741cc35cb19a5/src/librustc_mir/borrow_check/mod.rs#L347 does not hold the appropriate information about the arguments

oli (Jun 26 2019 at 07:38, on Zulip):

it calls struct_span_lint_hir, which I believe internally checks whether the hir node has the lint attributes

oli (Jun 26 2019 at 07:38, on Zulip):

you may have to follow a few function calls within struct_span_lint_hir to find that spot

Caio (Jun 26 2019 at 11:16, on Zulip):

@oli Thank you for the tips. I will continue investing the issue

oli (Jun 26 2019 at 11:35, on Zulip):

cc @Esteban Küber do you have time for the next two weeks to take over assisting here? I won't be available starting saturday

Esteban Küber (Jun 26 2019 at 17:38, on Zulip):

I'll be available next week but not the following. Feel free to ping me with questions, I might be able to help given the context I'm seeing.

Caio (Jul 17 2019 at 12:04, on Zulip):

Thank you guys for all the help. I wouldn't make it all by myself

Caio (Jul 17 2019 at 12:04, on Zulip):

Soon (I guess), the linting PR will the merged and the feature completed

Caio (Jul 17 2019 at 12:06, on Zulip):

Can we talk about support for proc_macro_attribute? Does it need another RFC? Is it possible to send a PR implementing it in the next weeks?

Caio (Jul 17 2019 at 12:07, on Zulip):

@Vadim PetrochenkovOnce you told me that you were going to refactor some parsing code. Did you finish it?

Vadim Petrochenkov (Jul 17 2019 at 15:19, on Zulip):

@Caio
Not parsing, but expansion.
I've been simplifying / cleaning up the expansion infra over the last couple of months with the bottleneck partially being on reviewing.
Here's a list of the relevant PRs:

Vadim Petrochenkov (Jul 17 2019 at 15:23, on Zulip):

https://github.com/rust-lang/rust/pull/60750
https://github.com/rust-lang/rust/pull/61024
https://github.com/rust-lang/rust/pull/61547
https://github.com/rust-lang/rust/pull/61606
https://github.com/rust-lang/rust/pull/61898
https://github.com/rust-lang/rust/pull/62042
https://github.com/rust-lang/rust/pull/62243
https://github.com/rust-lang/rust/pull/62258
https://github.com/rust-lang/rust/pull/62476
https://github.com/rust-lang/rust/pull/62684
https://github.com/rust-lang/rust/pull/62735
https://github.com/rust-lang/rust/pull/62086
https://github.com/rust-lang/rust/issues/61733

Vadim Petrochenkov (Jul 17 2019 at 15:27, on Zulip):

The next steps are
- merging Annotatable and perhaps Nonterminal into AstFragment
- Introducing more AST fragment kinds for currently non macro expanded positions and rewriting InvocationCollector and macro expander interfaces using those unified AST fragments, simultaneously addressing #61733

Vadim Petrochenkov (Jul 17 2019 at 15:28, on Zulip):

After those infrastructural changes making the function parameter position macro-expanded will be trivial.

Vadim Petrochenkov (Jul 17 2019 at 15:30, on Zulip):

At that point the question will be whether it's enough motivation to make that position macro-expanded, from language point of view.
I don't know.

Vadim Petrochenkov (Jul 17 2019 at 15:38, on Zulip):

(This may seem disappointing if you are aiming to put your hands on implementing a specific feature, but my goal in this process is rather longer term maintainability with features being implemented kind of naturally/automatically as side effects.)

Caio (Jul 17 2019 at 16:01, on Zulip):

Wow! There is a lot of stuff going on. Thanks for writing it out

Caio (Jul 17 2019 at 16:02, on Zulip):

If there is something that I can help out to make user attributes possible in this whole process, please let me know

Vadim Petrochenkov (Jul 17 2019 at 16:09, on Zulip):

I'll check my todo list for some more or less isolated tasks in this area.

Vadim Petrochenkov (Jul 21 2019 at 21:31, on Zulip):

@Caio
Here's how macro attributes are supported for a new attribute position (namely, generic parameter position)
https://github.com/petrochenkov/rust/tree/resall

Vadim Petrochenkov (Jul 21 2019 at 21:32, on Zulip):

Currently unsupported positions are ast::{Variant_, GenericParam, Arm, Arg, FieldPat, Field, StructField}.
All of those are "list" contexts like generic parameters, so implementations should be identical to the linked one, basically.

Vadim Petrochenkov (Jul 21 2019 at 21:34, on Zulip):

If you have time in the next 1-2 weeks and is able to implement any of those (perhaps all?), I'll wait and switch to some other work.

Vadim Petrochenkov (Jul 21 2019 at 21:37, on Zulip):

(param.ident.as_str() == "placeholder" is a temporary solution and should probably be replaced with boolean flags is_placeholder in AST structures, which is as simple, but not as dirty.)

Vadim Petrochenkov (Jul 21 2019 at 21:42, on Zulip):

Arm, FieldPat and Field also lack NodeIds which are necessary for the attribute resolution to work, so they'll have to be added.

Caio (Jul 21 2019 at 23:34, on Zulip):

Thanks @Vadim Petrochenkov

Caio (Jul 21 2019 at 23:35, on Zulip):

Looks like the intention is to generalize the use of macro attributes, which is awesome

Caio (Jul 21 2019 at 23:38, on Zulip):

I will probably be able to complete all necessary work in a week. It doesn't seem too difficult, I am a bit familiar with this area and your commit will help a lot

Vadim Petrochenkov (Jul 21 2019 at 23:47, on Zulip):

Looks like the intention is to generalize the use of macro attributes, which is awesome

Yes, generalize to macro attributes, but still emit a non-fatal error if it's actually a macro to avoid changing the observable language.
Name resolution support is tightly coupled to the macro support, so if you support the former, the latter needs to be supported as well.

Caio (Jul 28 2019 at 18:07, on Zulip):

This explain the reason of all the panics across the commit like Annotatable::Foo( .. ) => panic!( .. )

Caio (Jul 28 2019 at 18:11, on Zulip):

I couldn't finish all tasks because I had to redirect my time to the lint on args PR but everything will probably be ready in the next days

Caio (Aug 05 2019 at 00:12, on Zulip):

https://github.com/c410-f3r/rust/tree/attrs

Caio (Aug 05 2019 at 00:12, on Zulip):

Still not ready not I am getting closer

Vadim Petrochenkov (Aug 05 2019 at 08:23, on Zulip):

Nice, thanks for the update.

Caio (Aug 05 2019 at 10:32, on Zulip):

One question, the span fn of Annotatable expects a span of its elements. So, we probably need to use Variant instead of Variant_ for the whole modification

Vadim Petrochenkov (Aug 05 2019 at 13:06, on Zulip):

It would be better to merge Variant and Variant_.

Vadim Petrochenkov (Aug 05 2019 at 13:08, on Zulip):

In general, if you see Spanned<T> where T is not an enum that's a good candidate for refactoring.

Caio (Aug 05 2019 at 13:49, on Zulip):

Great! It will make things easier

Caio (Aug 06 2019 at 23:23, on Zulip):

in expand.rs, the ast_fragments! macro expects a single elements in a fn visit_* declaration (visitor.$visit_ast_elt(ast_elt);)

Caio (Aug 06 2019 at 23:25, on Zulip):

but visit_variant has a &mut self, v: &'ast Variant, g: &'ast Generics, item_id: NodeId signature that conflicts with ast_fragments!

Caio (Aug 06 2019 at 23:28, on Zulip):

The additional parameters are mainly used by the visit_variant_data function

Caio (Aug 06 2019 at 23:30, on Zulip):

I do not know what is the best way to solve this problem. Maybe create another visitor function?

Vadim Petrochenkov (Aug 07 2019 at 00:24, on Zulip):

If it doesn't fit into the macro it can be written explicitly, like AstFragment::OptExpr.

Vadim Petrochenkov (Aug 07 2019 at 00:28, on Zulip):

There's also a good chance that the extra arguments are never used by AST visitors in practice and can be removed.

Vadim Petrochenkov (Aug 21 2019 at 00:00, on Zulip):

@Caio
Any news?

Vadim Petrochenkov (Aug 21 2019 at 00:01, on Zulip):

FYI, if the status changes you can use @rustbot to change github labels, e.g. to S-waiting-on-review, even without having write rights in the repo.
https://github.com/rust-lang/triagebot/wiki/Labeling

Caio (Aug 21 2019 at 10:19, on Zulip):

@Vadim Petrochenkov I am sorry. I had some personal troubles and I will try to finish the PR today

Vadim Petrochenkov (Aug 21 2019 at 14:43, on Zulip):

No problem.

Caio (Aug 21 2019 at 17:21, on Zulip):

I don't know exactly why some tests are currently panicking

Caio (Aug 21 2019 at 17:21, on Zulip):

I may need another day to investigate it

Caio (Aug 21 2019 at 17:22, on Zulip):

This work started as a way to speed up macro stuff but, unfortunately, it is now a bottleneck :/

Vadim Petrochenkov (Aug 21 2019 at 18:43, on Zulip):

I actually did a bunch of macro-related stuff in parallel (and I still have some to do), so it's ok.

Caio (Sep 08 2019 at 13:46, on Zulip):

Any thoughts about #64282?
I can investigate the problem but I not very familiar with pprint

centril (Sep 08 2019 at 15:03, on Zulip):

I'll try to investigate it

Caio (Sep 08 2019 at 17:00, on Zulip):

@centril Should this issue and #64031 be part of the stabilization report?

centril (Sep 08 2019 at 21:00, on Zulip):

@Caio we should probably fix the issue first before that :P

Caio (Sep 08 2019 at 21:25, on Zulip):

Sure. Just wanted to confirm the possible need for modification

Last update: Nov 16 2019 at 02:10UTC