Bug 14633 – DDoc: false warnings for missing parameters on template declaration

Status
RESOLVED
Resolution
FIXED
Severity
enhancement
Priority
P1
Component
dmd
Product
D
Version
D2
Platform
All
OS
All
Creation time
2015-05-30T15:02:00Z
Last change time
2016-10-01T11:47:10Z
Keywords
ddoc, pull
Assigned to
lio+bugzilla
Creator
lio+bugzilla

Comments

Comment #0 by lio+bugzilla — 2015-05-30T15:02:26Z
There are two cases where the ddoc compiler issues a warning for missing parameters: /** Blah Params: T = some type test = something */ template case1(T) { void case1(R)(R test) { } } ///ditto alias case2 = case1!int; In both cases `test` refers to the function parameter for the template function `case1`. The ddoc compiler current looks for function parameters and template parameters, but does not check the function parameters of a nested/eponymous function template declaration.
Comment #1 by lio+bugzilla — 2015-05-30T15:12:56Z
Comment #2 by k.hara.pg — 2015-05-31T12:55:20Z
I think the enhancement implementation should support this a little more exhaustive test case. /** Blah Params: T = The warning for this documentation should be suppressed R = The warning for this documentation should be suppressed r = The warning for this documentation should be suppressed XT = This documentation should be warned XR = This documentation should be warned xr = This documentation should be warned */ template case1(T) { void case1(R)(R r) { } } /// ditto alias case2 = case1!int; /// ditto alias case3 = unrelated!int; template unrelated(XT) { void unrelated(XR)(XR xr) {} }
Comment #3 by lio+bugzilla — 2015-05-31T13:37:56Z
I don't agree with all the new test cases. Note that "ditto" is as if you'd copy-paste the entire comment on the new declaration. So 'unrelated' is not unrelated. It's just as related as the other parts. Which is why with the current PR there's not warning emitted for `xr` and `XT` (same as `r` and `T`.) I also don't agree that we should allow `R` (nor `XR`) to be documented on the parent. The only reason to have this pattern [explicit template with nested eponymous template] is to allow one explicit template parameter (`T`), while at the same time having overloads where the other template parameter(s) (`R`) are deduced from the regular function parameter (`r`.) Instantiating the template by specifying (and therefor documenting) both template parameters seem useless. (I don't even know what that instantiation should look like.)
Comment #4 by k.hara.pg — 2015-05-31T13:58:09Z
(In reply to Lionello Lunesu from comment #3) > I don't agree with all the new test cases. > > Note that "ditto" is as if you'd copy-paste the entire comment on the new > declaration. So 'unrelated' is not unrelated. It's just as related as the > other parts. Which is why with the current PR there's not warning emitted > for `xr` and `XT` (same as `r` and `T`.) I didn't check the documentation result of my case. Le'ts see it. template case1(T) alias case2 = case1!int.case1(R)(R r); alias case3 = unrelated!int.unrelated(XR)(XR xr); Blah Params: T The warning for this documentation should be suppressed R The warning for this documentation should be suppressed R r The warning for this documentation should be suppressed XT This documentation should be warned XR This documentation should be warned XR xr This documentation should be warned Hmm, XT is not documented in the signatures, but others are appeared. > I also don't agree that we should allow `R` (nor `XR`) to be documented on > the parent. The only reason to have this pattern [explicit template with > nested eponymous template] is to allow one explicit template parameter > (`T`), while at the same time having overloads where the other template > parameter(s) (`R`) are deduced from the regular function parameter (`r`.) > Instantiating the template by specifying (and therefor documenting) both > template parameters seem useless. (I don't even know what that instantiation > should look like.) R and XR can be specified explicitly via the aliases case2 and case3. case2!string("a"); // R == string case3!(int[])([1,2]); // And more, currently documenting T in the following code is not warned. /** Blah Params: T = x t = y */ void foo(T)(T t) {}
Comment #5 by ag0aep6g — 2015-05-31T14:05:03Z
(In reply to Lionello Lunesu from comment #3) > Note that "ditto" is as if you'd copy-paste the entire comment on the new > declaration. So 'unrelated' is not unrelated. It's just as related as the > other parts. Which is why with the current PR there's not warning emitted > for `xr` and `XT` (same as `r` and `T`.) `unrelated` doesn't have a ditto comment. XT can't be set through case3, so why should it be documented there? > I also don't agree that we should allow `R` (nor `XR`) to be documented on > the parent. The only reason to have this pattern [explicit template with > nested eponymous template] is to allow one explicit template parameter > (`T`), while at the same time having overloads where the other template > parameter(s) (`R`) are deduced from the regular function parameter (`r`.) > Instantiating the template by specifying (and therefor documenting) both > template parameters seem useless. What harm would documenting R/XR on the parent do? > (I don't even know what that instantiation > should look like.) ---- alias c = case1!string; c!byte(42); ----
Comment #6 by lio+bugzilla — 2015-05-31T14:18:28Z
(In reply to Kenji Hara from comment #4) > Hmm, XT is not documented in the signatures, but others are appeared. I see. You think it should warn for `XT` because it's not actually being used. That's fair, but it's not related to the issue I'm fixing here. > R and XR can be specified explicitly via the aliases case2 and case3. > > case2!string("a"); // R == string > case3!(int[])([1,2]); // > > And more, currently documenting T in the following code is not warned. > > /** > Blah > Params: > T = x > t = y > */ > void foo(T)(T t) {} I understand. I can easily add this.
Comment #7 by k.hara.pg — 2015-05-31T14:40:20Z
(In reply to Lionello Lunesu from comment #6) > I see. You think it should warn for `XT` because it's not actually being > used. That's fair, but it's not related to the issue I'm fixing here. Finally I think for the example in comment#2: - T, R, r, XR, and xr can be documented without warning - and XT should be warned. Warning for XT is related to the issue you're fixing. you're trying to handle the case 2, and in there you should not change the current warning for XT. >> void foo(T)(T t) {} > > I understand. I can easily add this. Do you mean that T should be warned? I cannot agree with that.
Comment #8 by lio+bugzilla — 2015-05-31T15:04:41Z
(In reply to Kenji Hara from comment #7) > Do you mean that T should be warned? I cannot agree with that. No, I mean allow documenting both `R` and `T`.
Comment #9 by k.hara.pg — 2015-05-31T15:12:34Z
(In reply to Lionello Lunesu from comment #8) > No, I mean allow documenting both `R` and `T`. OK.
Comment #10 by github-bugzilla — 2016-07-10T08:05:27Z
Commit pushed to master at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/122bcfaa5b70165e676522f94d984c6a4f2f2187 Issue 14633: Fixed false DDoc warnings (check parent template decl)
Comment #11 by ag0aep6g — 2016-08-12T21:01:47Z
(In reply to Lionello Lunesu from comment #1) > https://github.com/D-Programming-Language/dmd/pull/4689 With the pull request merged, can this be closed as FIXED?
Comment #12 by github-bugzilla — 2016-10-01T11:47:10Z
Commit pushed to stable at https://github.com/dlang/dmd https://github.com/dlang/dmd/commit/122bcfaa5b70165e676522f94d984c6a4f2f2187 Issue 14633: Fixed false DDoc warnings (check parent template decl)