Add suggestion for mul_add

[EthanTheMaster]

Issue #4001: Whenever a*b+c is found where a,b, and c are floats, a lint is suggested saying to use a.mul_add(b, c). Using mul_add may give a performance boost depending on the target architecture and also has higher numerical accuracy as there is no round off when doing a*b.

changelog: New lint: manual_mul_add

[flip1995]

Thanks for contributing and welcome to Clippy! 🎉

LGTM overall, only some small changes and this should be good to go

[EthanTheMaster]

When trying to build, I got an error for a non-existent CVarArgs in rustc::hir::TyKind so I just commented it out. Did CVarArgs get removed in a new update of rustc?

[tesuji]

Yes, you need to rebase this PR on top of master.

[flip1995]

Suggested change

	#![warn(clippy::manual_mul_add)]
	// run-rustfix
	#![warn(clippy::manual_mul_add)]

I'm not sure if this will work, because some of the suggestions have to be applied one after another. If that's the case, just split up the test in a run-rustfix file and a non-run-rustfix file.

[EthanTheMaster]

When testing whether a direct substitution of the suggestion would work, I noticed that I would sometimes get compilation errors because the type of a float (f32 or f64) could not be determined. See this demo.. Would using rustfix and MachineApplicable be fine in this scenario as substituting in the solution might give a cryptic error?

[flip1995]

Would using rustfix and MachineApplicable be fine in this scenario as substituting in the solution might give a cryptic error?

No that would be bad. You can just make it MaybeIncorrect (but split up the test cases in fixable and non-fixable). Another possibility is to only provide a suggestion if it is auto applicable (by checking for this case), but that is more work.

[EthanTheMaster]

What would splitting the test cases look like? Is that something I change in the uitest file with a macro or comment?

[flip1995]

No, it's simply to make a test file for auto fixable tests and one test file for non-autofixable tests. 😉

[EthanTheMaster]

So I need to make a ‘.fixed’ file containing ‘//run-rustfix’ at the top and code where the suggestions can be substituted without error?

[flip1995]

No, the *.fixed file is a auto generated file. What you need to do is

1. Create a file tests/ui/manual_mul_add_fixable.rs and add // run-rustfix to the top (In this file are the auto fixable tests)
2. Create a file tests/ui/manual_mul_add.rs without // run-rustfix (in this file are the tests that are not auto fixable)
3. Run sh tests/ui/update-all-references.sh (This should create 3 files: 2 *.stderr files and one *.fixed file)

[EthanTheMaster]

Thanks for the clarification. I‘ll give that a try later today. I remember, however, trying to run the update script which created the .stderr file but not the .fixed file. Is there some command I run before the update script like running rustfix or cargo test?

[flip1995]

The *.fixed file is only created if you add // run-rustfix to the top of the test (*.rs) file. So once you added that to the fixable test file, this should be created (after running cargo +master uitest+sh tests/ui/update-all-references.sh)

[phansch]

@bors r=flip1995

[bors]

📌 Commit 048a5f9 has been approved by flip1995

[bors]

⌛ Testing commit 048a5f9 with merge 13b423c...

[bors]

💔 Test failed - status-appveyor

[phansch]

@EthanTheMaster This will need a rebase as the lint count recently changed

[tesuji]

And squashing the commits I think.

[tesuji]

You need to run ./util/dev update_lints to update the lint count.

[phansch]

@bors retry

[phansch]

@bors r=flip1995

[bors]

📌 Commit 327c91f has been approved by flip1995

[bors]

⌛ Testing commit 327c91f with merge 1dc7f5c...

[bors]

☀️ Test successful - checks-travis, status-appveyor
Approved by: flip1995
Pushing 1dc7f5c to master...