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

newtype_index macro DEBUG_FORMAT = custom not working #45763

Closed
spastorino opened this issue Nov 4, 2017 · 11 comments
Closed

newtype_index macro DEBUG_FORMAT = custom not working #45763

spastorino opened this issue Nov 4, 2017 · 11 comments

Comments

@spastorino
Copy link
Member

spastorino commented Nov 4, 2017

I'm trying to change DefIndex to be defined using newtype_index and calling newtype_index macro this way ...

newtype_index!(DefIndex
    {
        DEBUG_FORMAT = custom

        const DEF_INDEX_HI_START = 1 << 31,

        const CRATE_DEF_INDEX = 0,
    });

Does not work.

/cc @Nashenas88 @nikomatsakis

@durka
Copy link
Contributor

durka commented Nov 4, 2017

Are you just missing a comma after the DEBUG_FORMAT line? It seems to be expected:

// Replace existing default for debug_format
(@derives [$($derives:ident,)*]
@pub [$($pub:tt)*]
@type [$type:ident]
@max [$max:expr]
@debug_format [$_debug_format:expr]
DEBUG_FORMAT = $debug_format:expr,
$($tokens:tt)*) => (

@spastorino
Copy link
Member Author

Yeah I wasn't paying a lot of attention to the macro definition, anyway ... with comma ...

error: format argument must be a string literal.
  --> src/librustc/hir/def_id.rs:91:24
   |
91 |         DEBUG_FORMAT = custom,
   |                        ^^^^^^

With comma and making custom a string ....

error: argument never used
  --> src/librustc/hir/def_id.rs:89:1
   |
89 | / newtype_index!(DefIndex
90 | |     {
91 | |         DEBUG_FORMAT = "custom",
92 | |
...  |
95 | |         const CRATE_DEF_INDEX = 0,
96 | |     });
   | |_______^

@spastorino
Copy link
Member Author

So ... or either I don't understand how to use the macro or there's a bug somewhere.

@durka
Copy link
Contributor

durka commented Nov 4, 2017

Yes, I think there is a bug. There is a base case for custom debug format but no rule for parsing that from the macro input like there is for ENCODABLE.

// base case for handle_debug where format is custom. No Debug implementation is emitted.
(@handle_debug
@derives [$($_derives:ident,)*]
@type [$type:ident]
@debug_format [custom]) => ();

@spastorino
Copy link
Member Author

spastorino commented Nov 4, 2017

@durka, actually after checking, the case you mentioned is not the right match arm.

We should be going through

// base case for handle_debug where format is custom. No Debug implementation is emitted.
(@handle_debug
@derives [$($_derives:ident,)*]
@type [$type:ident]
@debug_format [custom]) => ();

@durka
Copy link
Contributor

durka commented Nov 4, 2017

Yeah that is the one I linked. But that's the end of the macro, there needs to be another parsing rule that eats DEBUG_FORMAT = custom,.

@spastorino
Copy link
Member Author

@durka exactly!

@durka
Copy link
Contributor

durka commented Nov 4, 2017

OK, I figured out what the issue is. Parsing DEBUG_FORMAT = custom is actually fine since custom is a valid expr. However once you've parsed something into an expr you can't "look inside" it again -- matching it against the ident custom on line 97 doesn't work.

What needs to be done I believe is changing $debug_format:expr to $debug_format:tt everywhere in the macro. This should still work as a string literal is a single token, and it will preserve custom as a token that can be matched again.

@spastorino
Copy link
Member Author

After changing what you've said I'm getting ...

error: no rules expected the token `derives`
  --> src/librustc/hir/def_id.rs:89:1
   |
89 | / newtype_index!(DefIndex
90 | |     {
91 | |         DEBUG_FORMAT custom
92 | |
...  |
98 | |         const CRATE_DEF_INDEX = 0,
99 | |     });
   | |_______^
   |
   = note: this error originates in a macro outside of the current crate

error: aborting due to previous error

error: Could not compile `rustc`.

@durka
Copy link
Contributor

durka commented Nov 4, 2017

Now you have DEBUG_FORMAT custom instead of DEBUG_FORMAT = custom,? Or the error output is just mangling your source? I can take a look at your branch directly if you want.

@spastorino
Copy link
Member Author

Hey @durka, thanks, I think it's working now. I'm testing it and will open a PR if everything is OK. Thanks for the tips, at some point I'd need to read at least something about macros ;).

@spastorino spastorino changed the title newtype_index macro DEBUG_FORMAT custom not working newtype_index macro DEBUG_FORMAT = custom not working Nov 4, 2017
@spastorino spastorino changed the title newtype_index macro DEBUG_FORMAT = custom not working newtype_index macro DEBUG_FORMAT = custom not working Nov 4, 2017
bors added a commit that referenced this issue Nov 5, 2017
Make last structs indexes definitions use newtype_index macro

This PR makes the last two index structs not using newtype_index macro to use it and also fixes this #45763 issue.
@bors bors closed this as completed in f9bc8e7 Nov 6, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants