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

Syntax change in macro call produces erroneous code. #669

Open
baggepinnen opened this issue Dec 22, 2022 · 5 comments
Open

Syntax change in macro call produces erroneous code. #669

baggepinnen opened this issue Dec 22, 2022 · 5 comments
Labels
bug Something isn't working

Comments

@baggepinnen
Copy link

JuliaFormatter does not understand the @variables macro from ModelingToolkit, when formatting, it tries to change the syntax appearing here

sts = @variables x(t)=x_start [description = "State of SecondOrder filter $name"] xd(t)=xd_start [
        description = "Derivative state of SecondOrder filter $name",
    ]

from short to long function definition:

function xd(t)
        xd_start
    end [
        description = "Derivative state of SecondOrder filter $name",
    ]

This produces incorrect code. Perhaps JuliaFormatter should be more careful with reformatting macro calls since they may expect a certain syntax, and the alternative syntax after formatting may be invalid.

@domluna
Copy link
Owner

domluna commented Dec 23, 2022

I guess disabling this transform in a macro block is what we should do.

@domluna
Copy link
Owner

domluna commented Dec 23, 2022

can you post the format arguments so I can easily reproduce it

@baggepinnen
Copy link
Author

The formatter arguments are specified by this toml file
https://github.com/SciML/ModelingToolkitStandardLibrary.jl/blob/main/.JuliaFormatter.toml

@domluna domluna added the bug Something isn't working label Jan 7, 2023
@domluna
Copy link
Owner

domluna commented Jan 31, 2023

ok so I have a solution for this but it would apply to all macro block calls so the following would also not turn into a long form function.

    str = raw"""
       @noinline require_complete(m::Matching) =
           m.inv_match === nothing && throw(ArgumentError("Backwards matching not defined. `complete` the matching first."))
    """
    formatted_str = raw"""
    @noinline function require_complete(m::Matching)
        m.inv_match === nothing &&
            throw(ArgumentError("Backwards matching not defined. `complete` the matching first."))
    end
    """

A way to workaround this is to have a list of macros which are known to be "fine" with this sort of thing. So @noinline would be one of those.

thoughts @baggepinnen

@domluna
Copy link
Owner

domluna commented Jan 31, 2023

18e78ae

would also have to add a case for the opposite. Not sure what other transform is problematic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants