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

Add E0044 and E0071 error explanations #26431

Merged
merged 5 commits into from
Jun 24, 2015
Merged

Conversation

GuillaumeGomez
Copy link
Member

Part of #24407.
cc @michaelsproul

@rust-highfive
Copy link
Collaborator

r? @nrc

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

```
"##,

E0045: r##"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not an explanation of non-extern "C" variadic functions.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand. Isn't it about just extern function ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

E0045, // variadic function must have C calling convention

E0045 is about non-foreign functions with variadic params

@bors
Copy link
Contributor

bors commented Jun 19, 2015

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

@GuillaumeGomez
Copy link
Member Author

r? @Manishearth

@rust-highfive rust-highfive assigned Manishearth and unassigned nrc Jun 20, 2015
extern { fn some_func<T>(); }
```

Just remove the type parameter to make this code works:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"To make this code work"

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You should also have more advice here. "Removing the type parameter" won't make this code work, because now you need concrete types here.

Something along the lines of manually putting in the versions that you need (manual monomorphization) is what we want here

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand what you mean here. You want to say something like:

extern { fn some_func<i32>(); }

?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That doesn't do anything.

extern { fn some_func_i32(); }

is what I mean

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's an example, no ? What do you want me to add ?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, the current example would likely break any code using generics. You should change the advice to not recommend "just removing the type parameter".

Something like "to fix this, replace the type parameter with the specializations that you need"

Examples would be:

extern {
  fn some_func_i32(x: i32); 
  fn some_func_f64(x: f64);
}

over fn some_func<T>(x: T)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh okay ! Pfiouuuu... Took me a while to get it. I change it !

@GuillaumeGomez
Copy link
Member Author

@Manishearth: I updated it. Sorry for taking this long to get it...

@GuillaumeGomez GuillaumeGomez changed the title Add E0044 and E0045 error explanations Add E0044 and E0071 error explanations Jun 22, 2015
@GuillaumeGomez
Copy link
Member Author

About the E0071, the error can occurs in others places:

But it doesn't throw the error code. Is that normal ?

@Manishearth
Copy link
Member

No, that should probably be fixed to use the same error code uniformly.

@GuillaumeGomez
Copy link
Member Author

Okay, I'll create a PR then.

@GuillaumeGomez GuillaumeGomez changed the title Add E0044 and E0071 error explanations Add E0044, E0045 and E0071 error explanations Jun 22, 2015
@GuillaumeGomez GuillaumeGomez force-pushed the patch-2 branch 2 times, most recently from 30f7459 to d148036 Compare June 22, 2015 20:13
@GuillaumeGomez GuillaumeGomez changed the title Add E0044, E0045 and E0071 error explanations Add E0044 and E0071 error explanations Jun 22, 2015
@bors
Copy link
Contributor

bors commented Jun 23, 2015

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

@GuillaumeGomez
Copy link
Member Author

@Manishearth: Are we good here ?

Example of erroneous code:

```
enum Foo { f };
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

capitalize f throughout this example

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'll create a "complete" name instead of just "f".

@GuillaumeGomez
Copy link
Member Author

Updated !

@Manishearth
Copy link
Member

@bors r+

@bors
Copy link
Contributor

bors commented Jun 23, 2015

📌 Commit 4c2587c has been approved by Manishearth

@Manishearth
Copy link
Member

@bors rollup

@bors
Copy link
Contributor

bors commented Jun 24, 2015

⌛ Testing commit 4c2587c with merge 067b254...

@bors
Copy link
Contributor

bors commented Jun 24, 2015

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

extern { fn some_func_i32(x: i32); }
extern { fn some_func_i64(x: i64); }
```

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You're missing an end of string here, currently E0045 and its message are included in E0044's message!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh crap ! Bad rebase...

@GuillaumeGomez
Copy link
Member Author

Fixed.

@Manishearth
Copy link
Member

@bors r+ rollup

@bors
Copy link
Contributor

bors commented Jun 24, 2015

📌 Commit 3c98781 has been approved by Manishearth

@bors
Copy link
Contributor

bors commented Jun 24, 2015

⌛ Testing commit 3c98781 with merge c411f13...

bors added a commit that referenced this pull request Jun 24, 2015
@bors bors merged commit 3c98781 into rust-lang:master Jun 24, 2015
@GuillaumeGomez GuillaumeGomez deleted the patch-2 branch June 24, 2015 10:30
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.

7 participants