Skip to content

Comments

[refactor] Simplify parameter mismatch error code#8263

Merged
dlang-bot merged 2 commits intodlang:masterfrom
ntrel:match-fix
May 20, 2018
Merged

[refactor] Simplify parameter mismatch error code#8263
dlang-bot merged 2 commits intodlang:masterfrom
ntrel:match-fix

Conversation

@ntrel
Copy link
Contributor

@ntrel ntrel commented May 19, 2018

  • Use one OutBuffer instead of 3, reducing memory allocations.
  • Avoid repeating common string messages.
  • Add a missing test for typesafe variadics.

@dlang-bot
Copy link
Contributor

Thanks for your pull request and interest in making D better, @ntrel! We are looking forward to reviewing it, and you should be hearing from a maintainer soon.
Please verify that your PR follows this checklist:

  • My PR is fully covered with tests (you can see the annotated coverage diff directly on GitHub with CodeCov's browser extension
  • My PR is as minimal as possible (smaller, focused PRs are easier to review than big ones)
  • I have provided a detailed rationale explaining my changes
  • New or modified functions have Ddoc comments (with Params: and Returns:)

Please see CONTRIBUTING.md for more information.


If you have addressed all reviews or aren't sure how to proceed, don't hesitate to ping us with a simple comment.

Bugzilla references

Your PR doesn't reference any Bugzilla issue.

If your PR contains non-trivial changes, please reference a Bugzilla issue or create a manual changelog.

Testing this PR locally

If you don't have a local development environment setup, you can use Digger to test this PR:

dub fetch digger
dub run digger -- build "master + dmd#8263"

const rv = !arg.isLvalue() && par.storageClass & (STC.ref_ | STC.out_);
const char* fmt = rv ?
"cannot pass rvalue argument `%s` of type `%s` to parameter `%s`" :
"cannot pass argument `%s` of type `%s` to parameter `%s`";
Copy link
Contributor

Choose a reason for hiding this comment

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

Seems a bit unnecessary to duplicate the whole format string when the only difference is rvalue. Perhaps rvalue can be passed as a parameter to printf as well?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yep, last time I tried something in this direction, it was complained that this increases the binary size.
It's just "cannot pass%s" and then rv ? " rvalue" : "", no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix is here: ntrel@6219e5e

Copy link
Contributor

Choose a reason for hiding this comment

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

Cool. Would you mind submitting it as PR too? Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants