Warning for unused function args

[TurkeyMan]

This has annoyed me for years, but recently, multiple people on my team have been angry about it.
They're not D people, and they have had bugs where this would have saved them more time than it took me to work out how to implement it!

The natural resolution to the warning is:

1. Just don't name unreferenced args; ie, arguments to a callback/handler function that's unused by the implementation
2. cast(void)arg; naturally silences the warning.

This is introduced behind -preview=warnunusedparams, which can give people a year or 2 to implement; but we should announce the intention to make it standard at some time in the future, and recommend people enable it and migrate their local code during the lead-up.

[dlang-bot]

Thanks for your pull request and interest in making D better, @TurkeyMan! 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 coverage diff by visiting the details link of the codecov check)
• 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 run digger -- build "master + dmd#22695"

[thewilsonator]

Warning are generally frowned upon, if you want to get this in, in a reasonable timeframe, it will need to be behind a flag. See also the large number of failures this causes in druntime, not to mention user code e.g. buildkite.

Also this does not play well with static if

void __doPostblit(T)(T[] arr)
{
    // infer static postblit type, run postblit if any
    static if (__traits(hasPostblit, T))
    {
        static if (__traits(isStaticArray, T) && is(T : E[], E))
            __doPostblit(cast(E[]) arr);
        else
        {
            import core.internal.traits : Unqual;
            foreach (ref elem; (() @trusted => cast(Unqual!T[]) arr)())
                elem.__xpostblit();
        }
    }
}

this could be rewritten to

void __doPostblit(T)(T[] arr) if (__traits(hasPostblit, T))
{
    // infer static postblit type, run postblit if any
    static if (__traits(hasPostblit, T))
    {
        static if (__traits(isStaticArray, T) && is(T : E[], E))
            __doPostblit(cast(E[]) arr);
        else
        {
            import core.internal.traits : Unqual;
            foreach (ref elem; (() @trusted => cast(Unqual!T[]) arr)())
                elem.__xpostblit();
        }
    }
+    else 
+         cast(void) arr;
}

to pass with this, but this cannot be merged without a passing CI, and cannot break user code. Are your team using d-scanner? I'm sure this is already a lint rule available there.

[thewilsonator]

And any breakage it would cause would have to be rolled into an appropriate edition.

[TurkeyMan]

Warning are generally frowned upon,

Yeah I know... but sometimes people are wrong ;)

if you want to get this in, in a reasonable timeframe, it will need to be behind a flag.

What kind of flag controls particular warnings like this? I was thinking if it should be -w(/i)=unused_args or something, I'd be happy with that, but it creates a precedent. Maybe that's the way to introduce it and give time for corrections to be distributed though?

I'll rework it in those terms then?

See also the large number of failures this causes in druntime, not to mention user code e.g. buildkite.

Yup, it's a worry! That said, I've scanned through many of the cases, and some of them look like might actually be legit bugs being exposed... can't be sure without investigating them all, but I actually think it clearly demonstrates how much of a problem this is.
I've had numerous bugs where this would have saved time and money. Every codebase I've ever worked in has this option explicitly enabled, for good reason. It's distinctly felt when it's missing. If you never had it, you don't miss it, but when you've used it forever it's a sorely missing tool.

Also this does not play well with static if

void __doPostblit(T)(T[] arr)

Interaction with conditional compilation is well understood by other languages with warn-unused-args; the standard resolution in that case is something to the effect of cast(void)arg; (or in C, you can just do arg; as a no-op statement) to silence in cases where it's not sure if used or not. I actually thought I'd need to make that work in this patch and I imagined that would be 90% of the patch(!), but it turns out that's already implemented and works perfectly to resolve that issue in the 'universally accepted' way.

I'm amazed how tiny this patch is.

[TurkeyMan]

I put it behind a -preview, then we can give it some years for migration, and recommend people enable it to reach compliance in the meantime... probably the best way?

[TurkeyMan]

Are your team using d-scanner? I'm sure this is already a lint rule available there.

No, d-scanner is not integrated into VS. Depending on linting tools like that stratifies the ecosystem, it's the wrong place. VS uses the dmd-as-lib tooling.

[TurkeyMan]

Can you see why those couple failed? I'm not close with the CI ecosystem, so I'm not sure what to expect, but it's not jumping out at me what's wrong with those.

[thewilsonator]

>>> TARGET FAILED: compilable/previewhelp.d

   =fixImmutableConv disallow `void[]` data from holding immutable data (https://dlang.org/changelog/2.101.0.html#dmd.fix-immutable-conv, https://issues.dlang.org/show_bug.cgi?id=17148)
   =systemVariables  disable access to variables marked '@system' from @safe code (https://dlang.org/spec/attribute.html#system-variables)
   =fastdfa          Fast dataflow analysis engine, experimental
+  =warnunusedparams enable warnings for unused function parameters

need to update compilable/previewhelp.d with the above diff

[TurkeyMan]

I'm adding the spec and changelog now... thanks for flagging those other touch-points.
Spec: dlang/dlang.org#4413

[TurkeyMan]

I don't understand that circle.ci failure; and I'm not on a platform where I can follow the instruction in the log output... :/

[thewilsonator]

The Azure ones are spurious, the CircleCI one ons is not. The circleCI has a diff that you need to apply for frontend.h

[TurkeyMan]

The Azure ones are spurious, the CircleCI one ons is not. The circleCI has a diff that you need to apply for frontend.h

Yeah I looked at some past diffs and spotted what I missed (that giant long line!)

[tgehr]

I put it behind a -preview, then we can give it some years for migration, and recommend people enable it to reach compliance in the meantime... probably the best way?

I won't comply and I will fight against making this the default. After many years we finally removed dead code warnings. This is almost as annoying, particularly when conditional compilation is involved.

[0xEAB]

FYI D-Scanner supports checks for both…

• Unused variables.
• Unused parameters (check is skipped if function is marked "override").

[0xEAB]

While I'm all for adding a linter to the compiler itself, I believe this should be done thoroughly and properly — not piecemeal.

I see no value in adding “random” warnings and would prefer an actual linter.

[gorsing]

Go and Rust ship these checks in the compiler for a reason — it works better for the whole ecosystem, not just for those who set up linters.

[gorsing]

Speaking of best practices — in Go and Rust, this kind of check (unused parameters/variables) is built directly into the compiler, not relegated to an external linter. This ensures every developer gets the same feedback regardless of their IDE or CI setup.

Relying solely on D-Scanner creates a fragmented ecosystem where only those who configure it benefit from these checks. Newcomers or teams without strict linting setups miss out on catching potential bugs early.

If D wants to lower the barrier to entry and improve code quality out-of-the-box, following the approach of these languages makes sense. Having this in the compiler (even behind a flag initially) is a step in the right direction.

[rikkimax]

If we are doing this, let's give it its own pass over the modules.

With a dedicated switch like -lint.

[tgehr]

If D wants to lower the barrier to entry

New programmers are usually recommended to get started with languages such as Python, which will let you do type errors at runtime. Breaking their builds will actually throw them off.

and improve code quality out-of-the-box,

It is fundamentally untrue that deleting descriptive parameter names or sprinkling a bit of cast(void)arg over your code base will improve code quality.

following the approach of these languages makes sense.

This PR does not even do that. Also, Go and Rust don't have D's approach to conditional compilation, nor to warnings, so you can't simply copy-paste anyway.

[gorsing]

"New programmers are usually recommended to get started with languages such as Python, which will let you do type errors at runtime. Breaking their builds will actually throw them off."

We're not talking about runtime type errors — we're talking about compile-time warnings for unused parameters. These are two completely different things. Python's approach is irrelevant here because Python doesn't have a compile stage at all.

Also, nobody is proposing to "break builds" by default. This PR puts it behind -preview flag, exactly so that nobody gets surprised. The whole point is to give the ecosystem time to migrate.

"It is fundamentally untrue that deleting descriptive parameter names or sprinkling a bit of cast(void)arg over your code base will improve code quality."

That's a strawman. The improvement comes from detecting unintentionally unused parameters. The two resolutions you mentioned are exactly that — resolutions for when the parameter is intentionally unused. They're not the feature, they're the escape hatch.

The actual quality improvement happens when a developer accidentally leaves a parameter unused and the compiler says: "Hey, you declared ctx but never used it — did you forget something?" That's caught real bugs for me and my team, multiple times.

"This PR does not even do that. Also, Go and Rust don't have D's approach to conditional compilation, nor to warnings, so you can't simply copy-paste anyway."

Of course it's not a copy-paste — nobody is asking to copy Go's or Rust's code. The point is about philosophy: these languages treat basic code quality checks as a compiler responsibility, not an optional external tool.

[tgehr]

Disgusting AI slop.

[gorsing]

@tgehr, I respect your expertise and deep history with D's semantics. Fair enough, let's put the phrasing aside. My only goal is to discuss how we can improve the D ecosystem's baseline quality.

The -preview flag is exactly for what you mentioned: finding where it doesn't play well with templates or static if and refining it before anything becomes a default. I'd appreciate your technical input on those edge cases so we can make the check smarter, rather than just relying on external tools.

[rikkimax]

The -preview flag is exactly for what you mentioned: finding where it doesn't play well with templates or static if and refining it before anything becomes a default.

This will never be the default.

Walter is very against warnings. Either they should be errors, or they shouldn't exist.

This is why the fast DFA engine can only produce errors, things it knows 100% are incorrect behaviour.

[tgehr]

It's not about phrasing, it was not sophisticated enough to understand the actual points, it seems it lacks proper context on D, and it made misrepresentations by hallucinating plain lies. With dub all warnings are errors by default, so inevitably people will have to deal with PRs that try to delete parameter names or add cast(void)arg. It does not matter whether now this is behind a -preview flag, I was arguing against making it the default. All -preview flags are introduced with the intention that they could be made the default behavior in the future. Walter is fundamentally against fine-grained flags to enable and disable warnings, and he has spoken out against having any warnings in the first place. Unfortunately he gave in. But D warnings are simply not comparable in practice to what exists in other languages, for a big portion of D users (but not all) these are hard errors.

[gorsing]

I understand the concerns about the impact on the ecosystem. For many teams, this is still a justifiable compromise, allowing for early detection of bugs. If the community doesn't consider this a feature, consider making it available as an opt-in tool for those who need it. Finding a middle ground would truly help developers who are currently wasting their time identifying exactly these kinds of issues.

[tgehr]

The only way refusing compilation for a linter check that is rather unspecific might ever be a valid compromise for anyone is if you are technically unable to do that check in the appropriate way, at the appropriate place, with the appropriate severity.

Furthermore, introducing unnecessary friction also drives bugs because it wastes competent people's time on trivial little structural things that are not all that impactful overall. Why you would ever want to break your own build because some perfectly valid and thoroughly tested third-party code you are importing decided e.g. to consistently name the parameters on methods in a class hierarchy or used a static if to choose which parameter to use is just beyond me. It makes no sense. Run the linter on your own code, maybe use CI.

Catch bugs using code review that actually involves a semantic understanding of what the code is doing. You can even just paste your PR into an AI model, having it criticize you is a much better use of the technology than prompting it to confirm your priors by inventing incorrect facts.

[gorsing]

The -preview flag is exactly for what you mentioned: finding where it doesn't play well with templates or static if and refining it before anything becomes a default.

This will never be the default.

Walter is very against warnings. Either they should be errors, or they shouldn't exist.

This is why the fast DFA engine can only produce errors, things it knows 100% are incorrect behaviour.

@rikkimax You hit the nail on the head maby perhaps regarding the elegant solve:

"Walter is very against warnings. Either they should be errors, or they shouldn't exist."

If the core objection here is the introduction of a warning, why not lean into Walter's philosophy completely? We could make this a hard error, but keep it strictly opt-in (perhaps via a dedicated flag like -strict=unusedparams or the -lint switch you suggested earlier, rather than -preview which implies a future default).

For teams that need this feature to catch bugs, a hard error is actually preferable to a warning anyway. This approach would completely respect D's "no warnings" dogma, avoid breaking existing template-heavy codebases by default, while still providing developers with the built-in compiler diagnostic they desperately need.

[tgehr]

AI slop without the em dashes is still AI slop.

[Herringway]

Warnings are not helpful. Make it an error.

[ntrel]

Can it be just for functions not inside a template? That would presumably have no false positives so it could be a deprecation.

Edit: And disable it for non-final class methods.

[ntrel]

Or even include templates by default, but exclude them when there's a static if or mixin.

[Kapendev]

Having a -strict=unusedparams flag will create a dialect: strictD and yoloD.
It's not worth it, especially for a simple thing that can be done with a linter.
Anyway, at least try making it a per module thing if something like this gets added. Maybe a pragma?

[0xEAB]

@gorsing

I understand the concerns about the impact on the ecosystem. For many teams, this is still a justifiable compromise, allowing for early detection of bugs.

That seems like an arbitrary bar that can be applied to about any linting diagnostic.

For teams that need this feature to catch bugs, a hard error is actually preferable to a warning anyway.

That seems like an arbitrary bar that can be applied to about any linting diagnostic.

consider making it available as an opt-in tool for those who need it.

You are not going to believe what this tool does: https://github.com/dlang-community/D-Scanner ^^