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

Add #[use_export] attribute #12

Open
sam0x17 opened this issue Jun 14, 2023 · 5 comments
Open

Add #[use_export] attribute #12

sam0x17 opened this issue Jun 14, 2023 · 5 comments
Assignees
Labels
enhancement New feature or request

Comments

@sam0x17
Copy link
Owner

sam0x17 commented Jun 14, 2023

So right now export_tokens already can take an ident to be used as a disambiguation name (needed for Items that don't have an inherent ident, or (previously) items that would otherwise collide with another item name-wise at the crate level). Now that we don't have to deal with crate-wide collisions, I think it would be reasonable to re-purpose this so that the ident you specify becomes the real name of the item.

The problem with doing this, though, is that then from the perspective of import_tokens_attr and import_tokens_proc we have no way of knowing which kind it is, one with a fake name or one with a real ident, and currently in Rust there is no way to be like "does this symbol exist? if not do this".

Now all of that said, let's pretend for a moment that we don't have to worry about names colliding with the item we are attaching #[export_tokens] to. If that were the case, then we could have #[export_tokens] emit something like this:

#[export_tokens]
struct MyStruct {
    field: usize,
}

=>

mod MyStruct {
    macro_rules! __export_tokens_tt_my_struct_0 {
        // ...
    }
}

Then we would be able to do an import like use my::crate::MyItem and MyItem::__export_tokens_tt_my_item_0 would work without issue.

The problem of course with this approach is the name collides with the original item, so it would only work with the no_emit version of #[export_tokens].

So at the end of the day I agree that the best we can do right now until rust adds the ability to ask if a symbol is defined at the current compilation step is to write some sort of #[use_export] macro

The problem with writing such a macro, though, is we will have to know the COUNTER for that particular export_tokens ident...

So it could be that we require specifying an ident like #[export_tokens(MyIdent)] for #[export_tokens] attributes that we want to be able to use with #[use_export], since for those we won't have the COUNTER problem because they will have an explicit name.

Yeah I think that is the solution. The unfortunate thing is I don't think there is a way to get an intelligent compile-error here pushing people towards adding an ident to their #[export_tokens] other than the built in error which is just going to say "type my::crate::MyItem is a type not an expression" or something like that.

Originally posted by @sam0x17 in paritytech/substrate#14356 (comment)

@sam0x17 sam0x17 self-assigned this Jun 14, 2023
@sam0x17 sam0x17 added the enhancement New feature or request label Jun 14, 2023
@gui1117
Copy link
Contributor

gui1117 commented Jun 14, 2023

we could also take into attribute the optional ident given to export_tokens:

#[use_export_tokens(MyIdent)]
use foo::MyStruct;

@sam0x17
Copy link
Owner Author

sam0x17 commented Jun 14, 2023

We could, but my point is we couldn't support it anyway for cases where they don't specify an ident because we won't know the COUNTER value, so we can really only work with situations where they specified a hard-coded ident.

now for derive_impl that is fine because it just so happens impl can't have an inherent ident anyway so they always have to specify one

@gui1117
Copy link
Contributor

gui1117 commented Jun 14, 2023

but AFAICT we don't need the counter, we can re-export from the locally defined re-export without counter.

for

#[export_tokens]
pub struct MyStruct;

it generates in scope the items


pub use __export_tokens_tt_my_struct_0 as __export_tokens_tt_my_struct;

then in the other file

#[use_export_tokens]
use foo::MyStruct;

will expand to

use foo::MyStruct;
use foo::__export_tokens_tt_my_struct;

and for the one if ident it will do like this:

#[export_tokens(Foo)]
pub struct MyStruct;

it generates in scope the items


pub use __export_tokens_tt_foo_0 as __export_tokens_tt_foo;

then in the other file

#[use_export_tokens(Foo)]
use foo::MyStruct;

will expand to

use foo::MyStruct;
use foo::__export_tokens_tt_foo;

(but in general it feels like we can deprecate the attribute of export_tokens)

@sam0x17
Copy link
Owner Author

sam0x17 commented Jun 14, 2023

yes this seems reasonable to me

(but in general it feels like we can deprecate the attribute of export_tokens)

do you mean the attribute #[export_tokens] itself, or the argument #[export_tokens(MyIdent)]. The latter is needed because there are some items that can't otherwise have #[export_tokens] attached to them because they have no concept of a naming ident, like an impl block

@gui1117
Copy link
Contributor

gui1117 commented Jun 15, 2023

I meant the latter. But I see your point now.

I understand what you meant. In case they do #[export_tokens(Foo)] and there is no item Foo, then they cannot do

#[use_export_tokens(Foo)]
use somecrate::Foo;

Indeed, I have no idea what strategy is best.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants