Skip to content
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

MIR inlining should work with any ty::FnDef callee operand, not just constants. #54193

Closed
eddyb opened this issue Sep 13, 2018 · 3 comments · Fixed by #54416
Closed

MIR inlining should work with any ty::FnDef callee operand, not just constants. #54193

eddyb opened this issue Sep 13, 2018 · 3 comments · Fixed by #54416
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.

Comments

@eddyb
Copy link
Member

eddyb commented Sep 13, 2018

This code:

if let TerminatorKind::Call {
func: Operand::Constant(ref f), .. } = terminator.kind {

should not pattern-match on func, but rather call its ty method to get its type.

What this means is that let f = foo; f(); should inline as well as foo() (but today it doesn't).

cc @nikomatsakis

@eddyb eddyb added the E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue. label Sep 13, 2018
@nagisa
Copy link
Member

nagisa commented Sep 13, 2018

Actually it should handle both cases, I think. Handling the func constant is useful when the type has been erased but constant propagation managed to propagate the actual function anyway.

@eddyb
Copy link
Member Author

eddyb commented Sep 13, 2018

@nagisa It's type-based today, but it only handles the Operand::Constant case.
While handling function pointer types based on the value would be interesting, it's not done today.

@pvdrz
Copy link
Contributor

pvdrz commented Sep 20, 2018

Working on this one :)

bors added a commit that referenced this issue Sep 24, 2018
Extend MIR inlining to all operand variants

This fixes #54193
r? @eddyb
bors added a commit that referenced this issue Sep 24, 2018
Extend MIR inlining to all operand variants

This fixes #54193
r? @eddyb
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
E-easy Call for participation: Easy difficulty. Experience needed to fix: Not much. Good first issue.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants