Skip to content

Fix data-layout field in x86_64-unknown-linux-gnu.json test file #45108

New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Merged
merged 2 commits into from
Oct 10, 2017

Conversation

phil-opp
Copy link
Contributor

@phil-opp phil-opp commented Oct 8, 2017

The current data-layout causes the following error:

rustc: /checkout/src/llvm/lib/CodeGen/MachineFunction.cpp:151: void llvm::MachineFunction::init(): Assertion `Target.isCompatibleDataLayout(getDataLayout()) && "Can't create a MachineFunction using a Module with a " "Target-incompatible DataLayout attached\n"' failed.

The new value was generated according to this comment by @japaric.

The value was generated according to [this comment by @japaric](rust-lang#31367 (comment)).
@phil-opp phil-opp changed the title Fix data-layout field Fix data-layout field in x86_64-unknown-linux-gnu.json test file Oct 8, 2017
@carols10cents
Copy link
Member

r? @japaric

Thanks for the PR! We’ll periodically check in on it to make sure that @japaric or someone else from the team reviews it soon.

@carols10cents carols10cents added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties. label Oct 9, 2017
@japaric
Copy link
Member

japaric commented Oct 9, 2017

The change LGTM but the data-layout changed quite long time ago; why hasn't this break a test?

It seems to me that this file is not being used at all. When rustc sees --target $T it will use the built-in definition of $T if there is one; otherwise it will look for a specification file named $T.json. So I think that the test that is meant to use this file is actually using the built-in target definition. I think it would be better to rename the file to not match the name of a built-in target and then adjust the test accordingly.

Could you look into it @phil-opp? (TBH, I don't even know which test is supposed to be using this file.)

It seems like the file wasn't actually used, since there is a built-in target with the same name. See rust-lang#45108 (comment) for more details.
@phil-opp
Copy link
Contributor Author

phil-opp commented Oct 10, 2017

the data-layout changed quite long time ago; why hasn't this break a test?

Yeah, that wondered me too. I think that the file should be used by the target-specs test (see the Makefile).

I added a commit that renames the file to my-x86_64-unknown-linux-gnu-platform.json (to match the other test targets) and adjusts the target in the Makefile.

(I couldn't run the test locally, since I only have my laptop with a small SSD at the moment.)

@japaric
Copy link
Member

japaric commented Oct 10, 2017

Thanks, @phil-opp.

Travis seems to be happy so let's land this.

@bors r+ rollup

@bors
Copy link
Collaborator

bors commented Oct 10, 2017

📌 Commit 06b9168 has been approved by japaric

@japaric japaric 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 Oct 10, 2017
kennytm added a commit to kennytm/rust that referenced this pull request Oct 10, 2017
Fix data-layout field in x86_64-unknown-linux-gnu.json test file

The current data-layout causes the following error:

> rustc: /checkout/src/llvm/lib/CodeGen/MachineFunction.cpp:151: void llvm::MachineFunction::init(): Assertion `Target.isCompatibleDataLayout(getDataLayout()) && "Can't create a MachineFunction using a Module with a " "Target-incompatible DataLayout attached\n"' failed.

The new value was generated according to [this comment by @japaric](rust-lang#31367 (comment)).
bors added a commit that referenced this pull request Oct 10, 2017
Rollup of 9 pull requests

- Successful merges: #44775, #45089, #45095, #45099, #45101, #45108, #45116, #45135, #45146
- Failed merges:
@bors bors merged commit 06b9168 into rust-lang:master Oct 10, 2017
@phil-opp phil-opp deleted the patch-2 branch October 13, 2017 08:58
# for free to join this conversation on GitHub. Already have an account? # 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.

4 participants