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

Give spans to individual path segments in AST #40369

Merged
merged 2 commits into from
Mar 12, 2017

Conversation

petrochenkov
Copy link
Contributor

@petrochenkov petrochenkov commented Mar 8, 2017

And use these spans in path resolution diagnostics.

The spans are spans of identifiers in segments, not whole segments. I'm not sure what spans are more useful in general, but identifier spans are a better fit for resolve errors.

HIR still doesn't have spans.

Fixes #38927 (comment) #38890 (comment)

r? @nrc @eddyb

@eddyb
Copy link
Member

eddyb commented Mar 8, 2017

cc @jseyfried

@eddyb
Copy link
Member

eddyb commented Mar 8, 2017

@bors r+

@bors
Copy link
Contributor

bors commented Mar 8, 2017

📌 Commit 7cfe20c has been approved by eddyb

@petrochenkov
Copy link
Contributor Author

Memory consumption:

Before:
   Compiling rustc v0.0.0 (file:///C:/msys64/home/we/rust/src/librustc)
time: 0.214; rss: 60MB  parsing
time: 0.000; rss: 61MB  recursion limit
time: 0.000; rss: 61MB  crate injection
time: 0.000; rss: 61MB  plugin loading
time: 0.000; rss: 61MB  plugin registration
time: 0.894; rss: 179MB expansion
time: 0.000; rss: 179MB maybe building test harness
time: 0.023; rss: 179MB maybe creating a macro crate
time: 0.000; rss: 179MB checking for inline asm in case the target doesn't support it
time: 0.055; rss: 179MB early lint checks
time: 0.024; rss: 179MB AST validation
time: 0.398; rss: 204MB name resolution
time: 0.045; rss: 204MB complete gated feature checking
time: 0.306; rss: 293MB lowering ast -> hir
time: 0.040; rss: 285MB indexing hir
time: 0.017; rss: 285MB attribute checking
time: 0.008; rss: 251MB language item collection
...

After:
   Compiling rustc v0.0.0 (file:///C:/msys64/home/we/rust/src/librustc)
time: 0.220; rss: 64MB  parsing
time: 0.000; rss: 64MB  recursion limit
time: 0.000; rss: 64MB  crate injection
time: 0.000; rss: 64MB  plugin loading
time: 0.000; rss: 64MB  plugin registration
time: 0.844; rss: 186MB expansion
time: 0.000; rss: 186MB maybe building test harness
time: 0.024; rss: 186MB maybe creating a macro crate
time: 0.000; rss: 186MB checking for inline asm in case the target doesn't support it
time: 0.057; rss: 186MB early lint checks
time: 0.027; rss: 186MB AST validation
time: 0.384; rss: 211MB name resolution
time: 0.048; rss: 211MB complete gated feature checking
time: 0.308; rss: 300MB lowering ast -> hir
time: 0.040; rss: 292MB indexing hir
time: 0.018; rss: 292MB attribute checking
time: 0.008; rss: 256MB language item collection
...

@bors
Copy link
Contributor

bors commented Mar 9, 2017

🔒 Merge conflict

@eddyb
Copy link
Member

eddyb commented Mar 9, 2017

@nrc Can you use this to get rid of the span hacks in save-analysis? Or is it not enough?

@alexcrichton
Copy link
Member

@bors: retry

arielb1 pushed a commit to arielb1/rust that referenced this pull request Mar 9, 2017
Give spans to individual path segments in AST

And use these spans in path resolution diagnostics.

The spans are spans of identifiers in segments, not whole segments. I'm not sure what spans are more useful in general, but identifier spans are a better fit for resolve errors.

HIR still doesn't have spans.

Fixes rust-lang#38927 (comment) rust-lang#38890 (comment)

r? @nrc @eddyb
alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 10, 2017
Give spans to individual path segments in AST

And use these spans in path resolution diagnostics.

The spans are spans of identifiers in segments, not whole segments. I'm not sure what spans are more useful in general, but identifier spans are a better fit for resolve errors.

HIR still doesn't have spans.

Fixes rust-lang#38927 (comment) rust-lang#38890 (comment)

r? @nrc @eddyb
alexcrichton pushed a commit to arielb1/rust that referenced this pull request Mar 10, 2017
Give spans to individual path segments in AST

And use these spans in path resolution diagnostics.

The spans are spans of identifiers in segments, not whole segments. I'm not sure what spans are more useful in general, but identifier spans are a better fit for resolve errors.

HIR still doesn't have spans.

Fixes rust-lang#38927 (comment) rust-lang#38890 (comment)

r? @nrc @eddyb
@alexcrichton
Copy link
Member

@bors: r=eddyb

@bors
Copy link
Contributor

bors commented Mar 10, 2017

📌 Commit ffdcf74 has been approved by eddyb

alexcrichton added a commit to alexcrichton/rust that referenced this pull request Mar 10, 2017
Give spans to individual path segments in AST

And use these spans in path resolution diagnostics.

The spans are spans of identifiers in segments, not whole segments. I'm not sure what spans are more useful in general, but identifier spans are a better fit for resolve errors.

HIR still doesn't have spans.

Fixes rust-lang#38927 (comment) rust-lang#38890 (comment)

r? @nrc @eddyb
@bors
Copy link
Contributor

bors commented Mar 12, 2017

⌛ Testing commit ffdcf74 with merge ef7bbdd...

frewsxcv added a commit to frewsxcv/rust that referenced this pull request Mar 12, 2017
Give spans to individual path segments in AST

And use these spans in path resolution diagnostics.

The spans are spans of identifiers in segments, not whole segments. I'm not sure what spans are more useful in general, but identifier spans are a better fit for resolve errors.

HIR still doesn't have spans.

Fixes rust-lang#38927 (comment) rust-lang#38890 (comment)

r? @nrc @eddyb
@frewsxcv
Copy link
Member

@bors retry

bors added a commit that referenced this pull request Mar 12, 2017
Rollup of 5 pull requests

- Successful merges: #40369, #40390, #40426, #40449, #40453
- Failed merges:
@bors bors merged commit ffdcf74 into rust-lang:master Mar 12, 2017
@nrc
Copy link
Member

nrc commented Mar 13, 2017

@eddyb some of them, but not all.

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