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

Raise an error if gcov profiling and incremental compilation are both enabled #51645

Merged
merged 1 commit into from
Jul 2, 2018

Conversation

marco-c
Copy link
Contributor

@marco-c marco-c commented Jun 19, 2018

Fixes #50203.

@rust-highfive
Copy link
Collaborator

r? @eddyb

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Jun 19, 2018
@oli-obk
Copy link
Contributor

oli-obk commented Jun 20, 2018

r? @michaelwoerister

@marco-c
Copy link
Contributor Author

marco-c commented Jun 20, 2018

CC @kennytm

@eddyb
Copy link
Member

eddyb commented Jun 22, 2018

I'm not sure this is the correct approach. Shouldn't LLVM re-generate all the necessary files?

@marco-c
Copy link
Contributor Author

marco-c commented Jun 22, 2018

I'm not sure this is the correct approach. Shouldn't LLVM re-generate all the necessary files?

I'm not exactly sure how incremental compilation works, so I don't know. This is just aknowledging the current situation, where you can't use them both, I think it's better than failing without an explanation. The other option would be to allow it but print a warning, like you're doing for some other features (count_llvm_insns, time_llvm_passes). In those cases the feature is half broken though, here it's completely broken.

@TimNN
Copy link
Contributor

TimNN commented Jun 26, 2018

IIRC, @michaelwoerister is unavailable for reviews until early July, could someone else from @rust-lang/compiler review this?

@oli-obk
Copy link
Contributor

oli-obk commented Jun 27, 2018

While I do agree with @eddyb I also think until we have an idea how to fix it properly we should do this.

Maybe we just open an issue to fix it and merge this PR?

@michaelwoerister
Copy link
Member

Thanks for the PR, @marco-c!
It's definitely better to have an explicit early error instead of the ones reported in #50203.

@bors r+

@marco-c, would you mind opening an issue about this problem so we don't lose track of it?

@bors
Copy link
Contributor

bors commented Jul 2, 2018

📌 Commit 28670b6 has been approved by michaelwoerister

@bors bors added S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion. and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. labels Jul 2, 2018
@bors
Copy link
Contributor

bors commented Jul 2, 2018

⌛ Testing commit 28670b6 with merge e75e782...

bors added a commit that referenced this pull request Jul 2, 2018
…michaelwoerister

Raise an error if gcov profiling and incremental compilation are both enabled

Fixes #50203.
@bors
Copy link
Contributor

bors commented Jul 2, 2018

☀️ Test successful - status-appveyor, status-travis
Approved by: michaelwoerister
Pushing e75e782 to master...

@bors bors merged commit 28670b6 into rust-lang:master Jul 2, 2018
@marco-c
Copy link
Contributor Author

marco-c commented Jul 2, 2018

@marco-c, would you mind opening an issue about this problem so we don't lose track of it?

Sure! I've opened #51983.

@michaelwoerister
Copy link
Member

Thanks a lot!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: Waiting on bors to run and complete tests. Bors will change the label on completion.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants