Skip to content

Macros generating macro applications and phases #1908

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

Closed
munificent opened this issue Oct 15, 2021 · 10 comments
Closed

Macros generating macro applications and phases #1908

munificent opened this issue Oct 15, 2021 · 10 comments
Assignees
Labels
static-metaprogramming Issues related to static metaprogramming

Comments

@munificent
Copy link
Member

munificent commented Oct 15, 2021

The macro proposal allows macros to produce code which contains macro applications. When that happens, it says:

If a macro application is added which implements an earlier phase, that phase
is not ran. This should result in a warning if the macro does not also
implement some phase that will be ran.

If a macro application is added which runs in the same phase as the current
one, then it is immediately expanded after execution of the current macro,
following the normal ordering rules.

That seems both confusing and brittle to me. I can't imagine a macro author ever wanting an earlier phase of their macro to be silently skipped just because it happens to have a later phase and some other macro happened to generate the application.

My first inclination is to say we disallow generating macro applications entirely. I don't know that we have a strong use case for it. If we do have a compelling use case, I think we should consider looping the entire expansion process. Something like:

  1. Apply all phase 1 macros.
  2. Apply all phase 2 macros (that were not added in this cycle).
  3. Apply all phase 3 macros (that were not added in this cycle).
  4. If any of those produced new macro applications, then goto 1 and run all three phases of those new macro applications.
@munificent munificent added the static-metaprogramming Issues related to static metaprogramming label Oct 15, 2021
@safasofuoglu
Copy link

In pyo3, the rust-python bridge, it's desirable for a macro application to expand to be able to accomplish multiple things:

use pyo3::prelude::*;

/// Formats the sum of two numbers as string.
#[pyfunction]
fn sum_as_string(a: usize, b: usize) -> PyResult<String> {
    Ok((a + b).to_string())
}

/// A Python module implemented in Rust. The name of this function must match
/// the `lib.name` setting in the `Cargo.toml`, else Python will not be able to
/// import the module.
#[pymodule]
fn string_sum(_py: Python, m: &PyModule) -> PyResult<()> {
    m.add_function(wrap_pyfunction!(sum_as_string, m)?)?;

    Ok(())
}

With the current proposal, I guess the above can be achieved by composing macro code, without resorting to generating macro applications.

Another example is from rkyv, where the intent is to produce macros for generated code. Not sure if this qualifies as compelling.

#[derive(Archive, Serialize, Deserialize)]
#[derive(Debug, PartialEq, PartialOrd)]
// This will add #[derive(Debug, PartialEq, PartialOrd)] to the generated type
#[archive_attr(derive(Debug, PartialEq, PartialOrd)]
pub struct MyStruct {
    x: i32,
    y: i32,
    width: i32,
    height: i32,
}

@lrhn
Copy link
Member

lrhn commented Oct 16, 2021

If we make macros the recommended way to do data-classes, then it's very, very likely that some higher-level macro abstraction will want to introduce helper data-classes, or built-values, or mocks.

What might be reasonable is to only allow phase-1 macros to introduce macro annotations (they'll have to rely on re-exporting the macro annotations that they rely on, if you can't add more macro imports), and then run multiple phase-1 macro expansions.

After that, phase two macros can't introduce new macro annotations. That might be too restrictive if we have expression macros, since expressions aren't generated until phase ... 3?

@jakemac53
Copy link
Contributor

That seems both confusing and brittle to me. I can't imagine a macro author ever wanting an earlier phase of their macro to be silently skipped just because it happens to have a later phase and some other macro happened to generate the application.

Allowing for this primarily allows macro authors to write a single macro class where they might otherwise be forced to implement multiple.

Consider the functional widget case, in phase 1 that macro will want to create a new type and apply itself to that type. In phase 2 it will add some abstract methods to that type and again apply itself to those methods (this would fail without the carveout). In phase 3 it will actually fill in the bodies of those methods. I think this will be a common scenario for any macro that contributes types in phase 1 (they will want to contribute in all 3 phases and apply themselves in each phase to new declarations).

A macro author could split each of those phases up into separate macros to work around this, but I think its convenient to not have to.

My first inclination is to say we disallow generating macro applications entirely. I don't know that we have a strong use case for it. If we do have a compelling use case, I think we should consider looping the entire expansion process. Something like:

  1. Apply all phase 1 macros.
  2. Apply all phase 2 macros (that were not added in this cycle).
  3. Apply all phase 3 macros (that were not added in this cycle).
  4. If any of those produced new macro applications, then goto 1 and run all three phases of those new macro applications.

This would be problematic because it would break our ordering and correctness guarantees. Once we finish a phase we cannot come back to that phase, because that means last time we gave incorrect information.

@munificent
Copy link
Member Author

Consider the functional widget case, in phase 1 that macro will want to create a new type and apply itself to that type. In phase 2 it will add some abstract methods to that type and again apply itself to those methods (this would fail without the carveout).

I believe that should be supported directly. It should be trivially easy for a macro to declare and implement a method without having to resort to manually re-applying itself to the code it itself declared. This is the bread and butter use case for macros. Should it even be possible for a macro to declare a method that it doesn't implement?

@jakemac53
Copy link
Contributor

Should it even be possible for a macro to declare a method that it doesn't implement?

It might be applying a different macro to it that does know how to implement it etc, so yes I think so.

I will admit I don't quite understand the pushback on manually applying yourself to declarations you create if you want to run on them later - to me that is trivially easy to do and also intuitive 🤷‍♂️ . I don't think its worth creating some separate api to design around it.

@munificent
Copy link
Member Author

to me that is trivially easy to do and also intuitive 🤷‍♂️ .

I mean... I am literally working on this feature, have designed languages before, and still didn't realize that that was the expected way to write a macro that adds a method to a class. We can and should gather more usability data on this, but I'd be very surprised if "generate code that declares an external method with a macro application of the same macro class" was considered an obvious way for a macro to add a method.

@jakemac53
Copy link
Contributor

jakemac53 commented Nov 15, 2021

EDIT: This "v2" design was reverted so this comment is irrelevant now

Note that with the newer "V2" api design (what is currently in the proposal dir), I think there is now a conflict between the existing spec and the API.

Specifically, I think the API as designed now may not support allowing you to add explicit macro annotations at all, but certainly not ones that expect to run in earlier phases. We no longer have a separate method that gets invoked per phase - just a single visit* method. Invoking that after any phase has already completed will potentially result in errors, if that macro tries to perform an earlier phase operation.

Ultimately this may or may not be problematic... it is hard to say. Consider a macro that wants to add a new declaration and apply some other macro to that declaration. We could say that they must just directly invoke code from that macro. That requires some explicit opt-in from each macro author though. They have to expose some additional, imperative api that performs the work of their macro.

@jakemac53
Copy link
Contributor

Circling back here after >1.5 years (wow, it's really been that long?).

Note that we do now support macros applied to classes getting a "builder" for any members of the class, which covers some but not all of these use cases, although it is not a very direct way of doing it either.

I am also working on library macros, which will tentatively be able to get builders in the definition phase for any declaration in the library (this includes nested ones, transitively through a TypeDefinitionBuilder).

Neither of these cover all use cases though. I still think we need a solution here, and still believe that the best way to do that is by directly adding a macro application in code. This could likely be a unified solution for adding general metadata annotations. I think you would likely want the ability to add macro applications and metadata to existing declarations as well though. Potentially in any phase.

I would be happy to make it stricter and not allow adding macro applications that would apply in the current or any previous phase. While possibly convenient it is not necessary and could also be misleading.

@jakemac53
Copy link
Contributor

I have a PR out to make it an error now to generate macro applications that apply to previous phases (#3214)

@jakemac53 jakemac53 moved this from Todo to In Progress in Static Metaprogramming - design/prototype Jul 12, 2023
@jakemac53 jakemac53 self-assigned this Jul 12, 2023
jakemac53 added a commit that referenced this issue Jul 13, 2023
@jakemac53 jakemac53 moved this from In Progress to Backlog in Static Metaprogramming - design/prototype Jul 13, 2023
@jakemac53
Copy link
Contributor

Moving this to the backlog - we can listen to feedback and add additional API if needed here in a later phase. I would like to start with the abilities we already have:

  • library level macros can augment anything in the library.
  • class level macros can augment anything in the class.
  • when generating new declarations, you can include macro annotations, but they can only apply to the current phase or later phases.

lrhn pushed a commit that referenced this issue Jul 21, 2023
@davidmorgan davidmorgan closed this as not planned Won't fix, can't repro, duplicate, stale Feb 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
static-metaprogramming Issues related to static metaprogramming
Projects
Development

No branches or pull requests

5 participants