Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Macros Derive PlopAhead and PlopBehind #2390

Closed
wants to merge 6 commits into from
Closed

Macros Derive PlopAhead and PlopBehind #2390

wants to merge 6 commits into from

Conversation

Nokel81
Copy link
Contributor

@Nokel81 Nokel81 commented Apr 5, 2018

This is an rfc to adds some traits that can be derived for macros.

Rendered

@Centril Centril added the T-lang Relevant to the language team, which will review and decide on the RFC. label Apr 6, 2018
@Ixrec
Copy link
Contributor

Ixrec commented Apr 6, 2018

First, a relatively minor point: #[derive(...)] is almost always about making a type implement a trait, so it's really confusing to see it here for something totally different. Why not just #[plopahead] and #[plopbehind] attributes?

More generally, this feels like a very non-trivial, counterintuitive extension to basic control flow syntax for a fairly small benefit. If this is worth doing, I think we need much more justification. For example, it seems like supporting else {} in a while! macro wouldn't be nearly difficult or complicated enough to warrant a core language change like this (I suspect you could even write a small crate for else {} support in macros); a good first step would be explaining why this is so much more painful than I think it is.

@Centril
Copy link
Contributor

Centril commented Apr 6, 2018

If designed well, something roughly along these lines could potentially improve embedded domain specific languages (EDSLs).

@ExpHP
Copy link

ExpHP commented Apr 6, 2018

There is more to the issue than simply what was discussed in #2335.


What you seem to be picturing is:

  • The compiler expands the macros
    • (but with some weird kind of protection on the output to prevent certain kinds of usage, which you hope to disable with these "traits")
  • The compiler parses everything into an AST.

The reality is closer to the following:

  1. The compiler parses everything into an AST.
  2. Then it finds macro calls, expands them, and parses their output.

Which is to say that when the compiler sees this:

// defined earlier:
// macro_rules! Int { () => { i32 }; }
// macro_rules! id { ($($t:tt)*) => { $($t)* }; }

let foo: Int![] = 2 * id![3 + 2];

It first parses into something vaguely like the following (step 1):

Stmt::Let {
    pat: Pat::Ident("foo"),

    ty: Some(Type::Mac {
        ident: "Int",
        tt: vec![],
    }),

    // 2 * id![3 + 2]
    expr: Expr::BinOp {
        op: BinOp::Mul,
        lhs: Expr::Literal(Lit::Int(2)),
        rhs: Expr::Mac {
            ident: "id",
            tt: vec![
                TokenTree::Literal(Lit::Int(3)),
                TokenTree::Plus,
                TokenTree::Literal(Lit::Int(2)),
            ],
        },
    }
}

Then it expands the macros. It traverses the AST, encounters the Type::Mac, expands it and parses the output as a Type, and puts it back where the Type::Mac was. Then it encounters the Expr::Mac, expands it, parses the output as an Expr, and puts it back where the Expr::Mac was.

Stmt::Let {
    pat: Pat::Ident("foo"),

    ty: Some(Type::I32),

    // equivalent to '2 * (3 + 2)',
    // even though there's really no parentheses!
    expr: Expr::BinOp {
        op: BinOp::Mul,
        lhs: Expr::Literal(Lit::Int(2)),
        rhs: Expr::BinOp {
            op: BinOp::Add,
            lhs: Expr::Literal(Lit::Int(3)),
            rhs: Expr::Literal(Lit::Int(2)),
        },
    }
}

This design is why we don't need to worry about things like wrapping a macro's output in parentheses (it just naturally produces correctly-nested expressions).

It is also the reason why an expression macro can only be parsed as a complete expression, and why an item macro can only be parsed as zero or more complete items. Because in order for step 1 to even be possible, rustc must be able to figure out how to parse the stuff that appears after a macro call without knowing what the macro expands into.

When rustc chokes on your examples, that's because it can't finish step 1. Suffice it to say, there's no Expr::MacFollowedByElseBlock variant.


The kind of feature that you're looking for (and which is not currently provided by rustc in any sort of fashion) is often referred to as early expansion. I.e., macro expansion before construction of an AST.
I am having trouble at the moment locating past discussions on this topic, but if you look around hard enough on past RFCs, rust/issues, and irlo I am sure you will find plenty.

@ExpHP
Copy link

ExpHP commented Apr 6, 2018

Sorry, I missed this:

This would only be allowed to connect to if, else if, or else statments as they are currently the only blocks that this sort of connection would make sense.

With this limitation in place, your specific examples (lifted from the prose, with my interpretation in code)

#[derive(PlopBefore, PlopAfter)]
macro if_macro {
    () => {
        if true { }
    };
}

//" ... then a following else would be able to become the else for that produced if."
if_macro!() else { }

// "...allowed to attach to a daginling else to form an else if."
if cond { } else if_macro!() { }

do seem to could be possible to incorporate into the grammar unambiguously, without requiring early expansion.


That said, my overall reaction is still generally negative:

  • I do not find the use cases compelling.
  • Implementing "traits" on macros comes completely out of left field.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Apr 6, 2018

So the reason that I chose derive trait is because I thought it was the most rust way of annotating the macro definition. From what you said it seems that I was mistaken in this and a simple attribute would be better. I will change the RFC with them then.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Apr 6, 2018

For the reason why incorporating a design for optional handling within the macro itself instead of what I am proposing would most like follow the line of reasoning behind ergonomics. An ergonomic language is not only one that is easy to understand why core language features work the way they do but also one that allows or even encourages its users on how to use and macro extensions to the language. This is especially true for languages that allow some form of meta programming. I believe that such a feature as this would make explaining the use of macros that use it easier and it would be a more unified form of how to handle such things within the language instead of each macro trying to solve the same problem.

@clarfonthey
Copy link
Contributor

clarfonthey commented Apr 10, 2018

First, I noticed that this is using an old version of the RFC template; prior art and the guide/reference-level explanation split are missing. This could definitely benefit from those.

Also, I'm confused why you can't just do:

while_else! {
    while cond {
        // ...
    } else {
        // ...
    }
}

To me, making macros glom onto everything around them is very confusing to understand and can lead to probably a lot of very hard-to-understand bugs.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Apr 10, 2018

Oh I didn't know that there was a new format. I should pull that into my fork then.

However, I don't think that that is possible because unless I am mistaken macros cannot request words that are restricted keywords. So it would be something like Else instead which has the same problem

@clarfonthey
Copy link
Contributor

Macros can; if they couldn't, lots of existing crates couldn't exist.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Apr 10, 2018

Sorry then, but I think that this is still a better solution because of interoperability and less nesting. I would argue that the mental strain is also less if this was to be implemented and used

@tmccombs
Copy link

tmccombs commented Apr 12, 2018

Also, I'm confused why you can't just do:

while_else! {
    while cond {
        // ...
    } else {
        // ...
    }
}

I think the motivation is that that is kind of ugly. However, I don't think this RFC is the right solution.

I would rather have the curly brace delimited body separate from the arguments to the macro, similar to how a ruby function can take a block. So the above macro could be written like:

mywhile!(cond) {{
  // body
} else {
  // ...
}}

Which I think is more readable than the above while_else, but doesn't come at the cost of having segments of invalid syntax.

or for another example:

regex_match!(expr) {
  "foo([a-z]+)" as _(arg) => ...
  "[0-9]+" as arg => ...
   ...
}

Alternative:

If a macro call is immediately followed by a "!" it continues parsing the next syntactical element as part of the macro:

macro_rules! while_else {
   (cond:expr)!{body:block}!else!{else_body:block} => ...
}

while_else!(cond)!{
    ...
}!else!{
    ...
}

@Nokel81
Copy link
Contributor Author

Nokel81 commented Apr 12, 2018

Though I like the look of the last alternative the best it seems weird to have so many ! in such a case and I would argue would be even harder to use, namely because you have to remember to put all of them in.

I could agree that the current names for this RFC is as bit odd and doesn't really explain the goal or what I am proposing. Maybe ExtendFromIf and ExtendWithIf.

I don't agree that this would have segments of invalid syntax because this is a form of meta programming. The syntax would only be considered invalid if the macro expanded to something that did not end or start with an if statement.

@nikomatsakis
Copy link
Contributor

@rfcbot fcp postpone

I would like to see movement in this direction, but I feel like we are not ready to accept any RFC on this topic yet. Another use we should make sure to consider is declaring "type-like things" -- e.g., class! Foo { ... }

@rfcbot rfcbot added the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label Apr 12, 2018
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 12, 2018

Team member @nikomatsakis has proposed to postpone this. The next step is review by the rest of the tagged teams:

No concerns currently listed.

Once a majority of reviewers approve (and none object), this will enter its final comment period. If you spot a major issue that hasn't been raised at any point in this process, please speak up!

See this document for info about what commands tagged team members can give me.

@joshtriplett
Copy link
Member

I think we need to figure out more syntactically rich and integrated macros as part of the full macros 2.0 story.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Apr 12, 2018

I am fine with this outcome because I can understand the sentiment. If we want to discuss the full macros 2.0 story where would a good place to do that be?

@Centril
Copy link
Contributor

Centril commented Apr 12, 2018

If we want to discuss the full macros 2.0 story where would a good place to do that be?

The internals forum sounds like a good place to flesh out designs on this as a pre-pre-pre-RFC ;)

@H2CO3
Copy link

H2CO3 commented Apr 13, 2018

This seems like a very confusing thing to do. Even if it's opt-in. Macros can already be a little magical, but this would seem like total madness to read for someone other than the author of such a macro. And because it's magical/surprising, it has the potential of introducing bugs, which is very undesirable.

@Nokel81
Copy link
Contributor Author

Nokel81 commented Apr 13, 2018

@H2CO3 I don't think that it would be that magical since unless you know about this why would you write a dangling else {} block after a macro which could be the cause of some bugs sure. From a reader's perspective, I think that the sight of an else {} block after a macro would imply that some condition in the macro is false and we can "know" about it in this block.

@H2CO3
Copy link

H2CO3 commented Apr 13, 2018

@Nokel81 I would never ever write a dangling else anywhere because it looks wrong and confusing no matter where it is. A worse case is when I don't mean to intentionally write one, but I make an editing mistake (e.g. double paste after a cut or accidental paste via a mistyped key combination), and the code ends up having a dangling else clause right after such a magic macro. And the code now compiles, and the compiler doesn't tell me I've made a syntactic error, and the program will run and do the wrong thing.

And even if I suspected that macro tricks are going on, I would not want to base my understanding of the code upon assumptions, suspicions, and gut feelings. A related point is that there's even a Rust API guideline that "macro invocation syntax should be evocative of generated code" which this proposal also breaks in a subtle way.

All in all, starting to allow syntax that has always been a sign of accidental, erroneous code in the past is a very bad idea exactly because it will hide errors. Rust is not a language where programmers want brittleness and fragility arising out of surprising syntactic interactions.

@timbess
Copy link

timbess commented Apr 16, 2018

Python allows else blocks to be used in super weird places like after a for loop or after an except block and it's one of my least favorite things in the language. I don't see the use cases in the RFC being very helpful either.

let handler = dbOpen!{
    ip = address,
    username = user,
    password = pass
} else {
    // handle the case where the connection fails to open
}

That doesn't seem like a very Rusty way to do that. What seems better to me is:

let handler = match dbOpen(address, user, pass) {
    Ok(h) => h,
    Err(e) => {
        //handle error here
    }
};

As for the other case:

while! (expr {
    // do something
}) else {
    // if expr was never true
}

Versus @tmccombs alternatives

mywhile!(cond) {{
  // body
} else {
  // ...
}}

// or

while_else!(cond)!{
    ...
}!else!{
    ...
}

I wouldn't consider either of those alternatives less readable than the example in the RFC. In fact it's far better because now I know that the else block is part of the macro. There is no question of whether or not the else block in relying on something from inside the macro.

And that brings about the larger point that I really really don't like the idea of macros having the ability to affect the surrounding code. One of the huge benefits of Rust macros is that they are hygienic. This would break that and honestly the benefits of this feature don't outweigh the loss of hygienic macros in my opinion.

@rfcbot rfcbot added the final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. label Apr 19, 2018
@rfcbot rfcbot removed the proposed-final-comment-period Currently awaiting signoff of all team members in order to enter the final comment period. label Apr 19, 2018
@rfcbot
Copy link
Collaborator

rfcbot commented Apr 19, 2018

🔔 This is now entering its final comment period, as per the review above. 🔔

@rfcbot
Copy link
Collaborator

rfcbot commented Apr 29, 2018

The final comment period is now complete.

@Centril Centril added the postponed RFCs that have been postponed and may be revisited at a later time. label Apr 29, 2018
@Centril
Copy link
Contributor

Centril commented Apr 29, 2018

Thank you @Nokel81 for the RFC!

Per review #2390 (comment), this RFC is hereby postponed.

@Centril Centril closed this Apr 29, 2018
@Centril
Copy link
Contributor

Centril commented Oct 9, 2018

cc rust-lang/rust#39412

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
final-comment-period Will be merged/postponed/closed in ~10 calendar days unless new substational objections are raised. postponed RFCs that have been postponed and may be revisited at a later time. T-lang Relevant to the language team, which will review and decide on the RFC.
Projects
None yet
Development

Successfully merging this pull request may close these issues.