Skip to content
This repository was archived by the owner on Jan 22, 2025. It is now read-only.

Fix hygiene issues in declare_program! and declare_loader! (bp #10905) #11073

Merged
merged 10 commits into from
Jul 15, 2020
Merged

Fix hygiene issues in declare_program! and declare_loader! (bp #10905) #11073

merged 10 commits into from
Jul 15, 2020

Conversation

mergify[bot]
Copy link
Contributor

@mergify mergify bot commented Jul 15, 2020

This is an automated backport of pull request #10905 done by Mergify

Cherry-pick of 05445c7 has failed:

On branch mergify/bp/v1.2/pr-10905
Your branch is up to date with 'origin/v1.2'.

You are currently cherry-picking commit 05445c718.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:

	modified:   programs/bpf_loader/Cargo.toml
	modified:   programs/bpf_loader/src/lib.rs
	modified:   sdk/macro/Cargo.toml
	modified:   sdk/macro/src/lib.rs
	modified:   sdk/src/entrypoint_native.rs
	modified:   sdk/src/lib.rs

Unmerged paths:
  (use "git add <file>..." to mark resolution)

	both modified:   Cargo.lock
	both modified:   sdk/Cargo.toml

Cherry-pick of 42f8848 has failed:

On branch mergify/bp/v1.2/pr-10905
Your branch is ahead of 'origin/v1.2' by 2 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit 42f88484f.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add <file>..." to mark resolution)

	both modified:   programs/bpf/Cargo.lock
	both modified:   programs/librapay/Cargo.lock
	both modified:   programs/move_loader/Cargo.lock

no changes added to commit (use "git add" and/or "git commit -a")

Cherry-pick of 95490ff has failed:

On branch mergify/bp/v1.2/pr-10905
Your branch is ahead of 'origin/v1.2' by 5 commits.
  (use "git push" to publish your local commits)

You are currently cherry-picking commit 95490ff56.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Unmerged paths:
  (use "git add/rm <file>..." as appropriate to mark resolution)

	deleted by us:   sdk/build.rs
	both modified:   sdk/src/lib.rs

no changes added to commit (use "git add" and/or "git commit -a")

To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

Mergify commands and options

More conditions and actions can be found in the documentation.

You can also trigger Mergify actions by commenting on this pull request:

  • @Mergifyio refresh will re-evaluate the rules
  • @Mergifyio rebase will rebase this PR on its base branch
  • @Mergifyio update will merge the base branch into this PR
  • @Mergifyio backport <destination> will backport this PR on <destination> branch

Additionally, on Mergify dashboard you can:

  • look at your merge queues
  • generate the Mergify configuration with the simulator.

Finally, you can contact us on https://mergify.io/

Aaron1011 and others added 6 commits July 15, 2020 01:04
@Aaron1011
Fix hygiene issues in declare_program! and declare_loader!
2a5a92f 
The `declare_program!` and `declare_loader!` macros both expand to
new macro definitions (based on the `$name` argument). These 'inner'
macros make use of the special `$crate` metavariable to access items in
the crate where the 'inner' macros is defined.

However, this only works due to a bug in rustc. When a macro is
expanded, all `$crate` tokens in its output are 'marked' as being
resolved in the defining crate of that macro. An inner macro (including
the body of its arms) is 'just' another set of tokens that appears in
the body of the outer macro, so any `$crate` identifiers used there are
resolved relative to the 'outer' macro.

For example, consider the following code:

```rust
macro_rules! outer {
    () => {
        macro_rules! inner {
            () => {
                $crate::Foo
            }
        }
    }
}
```

The path `$crate::Foo` will be resolved relative to the crate that defines `outer`,
**not** the crate which defines `inner`.

However, rustc currently loses this extra resolution information
(referred to as 'hygiene' information) when a crate is serialized.
In the above example, this means that the macro `inner` (which gets
defined in whatever crate invokes `outer!`) will behave differently
depending on which crate it is invoked from:

When `inner` is invoked from the same crate in which it is defined,
the hygiene information will still be available,
which will cause `$crate::Foo` to be resolved in the crate which defines 'outer'.

When `inner` is invoked from a different crate, it will be loaded from
the metadata of the crate which defines 'inner'. Since the hygiene
information is currently lost, rust will 'forget' that `$crate::Foo` is
supposed to be resolved in the context of 'outer'. Instead, it will be
resolved relative to the crate which defines 'inner', which can cause
incorrect code to compile.

This bug will soon be fixed in rust (see rust-lang/rust#72121),
which will break `declare_program!` and `declare_loader!`. Fortunately,
it's possible to obtain the desired behavior (`$crate` resolving in the
context of the 'inner' macro) by use of a procedural macro.

This commit adds a `respan!` proc-macro to the `sdk/macro` crate.
Using the newly-stabilized (on Nightly) `Span::resolved_at` method,
the `$crate` identifier can be made to be resolved in the context of the
proper crate.

Since `Span::resolved_at` is only stable on the latest nightly,
referencing it on an earlier version of Rust will cause a compilation error.
This requires the `rustversion` crate to be used, which allows conditionally
compiling code epending on the Rust compiler version in use. Since this method is already
stabilized in the latest nightly, there will never be a situation where
the hygiene bug is fixed (e.g. rust-lang/rust#72121)
is merged but we are unable to call `Span::resolved_at`.

(cherry picked from commit 05445c7)

# Conflicts:
#	Cargo.lock
#	sdk/Cargo.toml
@Aaron1011
Replace FIXME with an issue link
4038844 
(cherry picked from commit b0cb2b0)
@jackcmay
Update lock files
795e639 
(cherry picked from commit 42f8848)

# Conflicts:
#	programs/bpf/Cargo.lock
#	programs/librapay/Cargo.lock
#	programs/move_loader/Cargo.lock
@Aaron1011
Split comment over multiple lines
9bacfb7 
Due to rust-lang/rustfmt#4325, leaving this as
one line causes rustfmt to add extra indentation to the surrounding
code.

(cherry picked from commit fed69e9)
@Aaron1011
Fix clippy lints
b64a82f 
(cherry picked from commit e7387f6)
@Aaron1011
Apply #![feature(proc_macro_hygiene)] when needed
c692481 
This allows the rust-bpf-builder toolchain to build the sdk

(cherry picked from commit 95490ff)

# Conflicts:
#	sdk/build.rs
#	sdk/src/lib.rs
@mergify mergify bot added conflicts automerge Merge this Pull Request automatically once CI passes labels Jul 15, 2020
@mergify mergify bot removed the automerge Merge this Pull Request automatically once CI passes label Jul 15, 2020
@mergify
Copy link
Contributor Author

mergify bot commented Jul 15, 2020

automerge label removed due to a CI failure

@mvines mvines force-pushed the mergify/bp/v1.2/pr-10905 branch from 8e90993 to 60a33f1 Compare July 15, 2020 01:16
@mergify mergify bot added the automerge Merge this Pull Request automatically once CI passes label Jul 15, 2020
@codecov
Copy link

codecov bot commented Jul 15, 2020

Codecov Report

Merging #11073 into v1.2 will decrease coverage by 0.0%.
The diff coverage is 20.8%.

@@           Coverage Diff            @@
##            v1.2   #11073     +/-   ##
========================================
- Coverage   82.1%    82.1%   -0.1%     
========================================
  Files        308      309      +1     
  Lines      72563    72587     +24     
========================================
- Hits       59631    59610     -21     
- Misses     12932    12977     +45

@mergify mergify bot merged commit d1c0f4b into v1.2 Jul 15, 2020
@mergify mergify bot deleted the mergify/bp/v1.2/pr-10905 branch July 15, 2020 02:40
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers
No reviews
Assignees
No one assigned
Labels
automerge Merge this Pull Request automatically once CI passes conflicts
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@Aaron1011 @jackcmay @mvines