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: Move ast_map into it's own crate #24757

Closed
wants to merge 1 commit into from

Conversation

erickt
Copy link
Contributor

@erickt erickt commented Apr 24, 2015

This lets us remove the arena dependency from libsyntax, which is unstable, and supports making libsyntax compile on beta/1.0 (#24518).

r? @alexcrichton

@erickt erickt changed the title syntax: Move ast_map into it's own module syntax: Move ast_map into it's own crate Apr 24, 2015
@erickt erickt force-pushed the split-libsyntax branch 2 times, most recently from 7ea8f33 to fec0421 Compare April 24, 2015 03:46
@alexcrichton
Copy link
Member

r? @nikomatsakis, @pnkfelix, or @nrc, I don't mess around with compiler internals enough on this I think.

@nikomatsakis
Copy link
Contributor

I have no objection, but I wonder if this should be a rustc crate? Seems pretty specific to the compiler.

@erickt
Copy link
Contributor Author

erickt commented Apr 26, 2015

@nikomatsakis: I had thought it could be useful for future syntax extensions, but I suppose if we go down that road we can always rename rustc_ast_map to syntax_ast_map. I'll change the name.

@richo
Copy link
Contributor

richo commented Apr 26, 2015

I haven't touched this codebase in a long time (and even then it was super
experimental, I was just curious to see how feasible this approach was),
but are changes like this going to affect visibility for something like:
https://github.com/richo/commavm/blob/master/src/main.rs#L1-L5

On Sun, Apr 26, 2015 at 12:55 PM, Erick Tryzelaar notifications@github.com
wrote:

@nikomatsakis https://github.com/nikomatsakis: I had thought it could
be useful for future syntax extensions, but I suppose if we go down that
road we can always rename rustc_ast_map to syntax_ast_map. I'll change
the name.


Reply to this email directly or view it on GitHub
#24757 (comment).

@erickt
Copy link
Contributor Author

erickt commented Apr 26, 2015

@alexcrichton / @nikomatsakis: Updated the PR to rename this new package to rustc_ast_map.

@richo: As far as I can tell you're not using ast_map, so this shouldn't impact you. I suggest you check out syntex though, which hopefully will allow for syntax extensions in 1.0 :)

@nikomatsakis
Copy link
Contributor

@bors r+ ecef439

@bors
Copy link
Contributor

bors commented Apr 27, 2015

☔ The latest upstream changes (presumably #23606) made this pull request unmergeable. Please resolve the merge conflicts.

@bors
Copy link
Contributor

bors commented Apr 28, 2015

🔒 Merge conflict

This lets us remove the arena dependency from libsyntax, which
is unstable.
@nikomatsakis
Copy link
Contributor

@bors r+

@bors
Copy link
Contributor

bors commented May 5, 2015

📌 Commit 19f1a15 has been approved by nikomatsakis

@bors
Copy link
Contributor

bors commented May 5, 2015

⌛ Testing commit 19f1a15 with merge 5770f07...

@bors
Copy link
Contributor

bors commented May 5, 2015

💔 Test failed - auto-mac-64-nopt-t

@bors
Copy link
Contributor

bors commented May 12, 2015

☔ The latest upstream changes (presumably #25323) made this pull request unmergeable. Please resolve the merge conflicts.

@Gankra
Copy link
Contributor

Gankra commented May 31, 2015

@erickt ping (needs rebase)

bors added a commit that referenced this pull request Jun 10, 2015
Gets libsyntax one step closer to running on stable (see #24518).
Closes #24757, erickt's previous attempt at this.
bors added a commit that referenced this pull request Jun 10, 2015
Gets libsyntax one step closer to running on stable (see #24518).
Closes #24757, erickt's previous attempt at this.
@bors bors closed this in #26141 Jun 10, 2015
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

Successfully merging this pull request may close these issues.

6 participants