http://d.puremagic.com/issues/show_bug.cgi?id=10236



--- Comment #10 from bearophile_hugs@eml.cc 2013-08-25 09:54:14 PDT ---
(In reply to comment #9)

> this patch has found numerous DDoc mistakes in phobos and druntime!

A different outcome would have shocked me :-)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10236



--- Comment #11 from bearophile_hugs@eml.cc 2013-08-25 10:57:20 PDT ---
(In reply to comment #9)

> FWIW, this patch has found numerous DDoc mistakes in phobos and druntime! I'd say that's proof enough.

Where's a compact list of all the Phobos documentation errors found by the new warnings?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10236



--- Comment #12 from Lionello Lunesu <lio+bugzilla@lunesu.com> 2013-08-25 18:19:43 PDT ---
(In reply to comment #11)
> (In reply to comment #9)
> 
> > FWIW, this patch has found numerous DDoc mistakes in phobos and druntime! I'd say that's proof enough.
> 
> Where's a compact list of all the Phobos documentation errors found by the new warnings?

http://d.puremagic.com/issues/show_bug.cgi?id=10893

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10236



--- Comment #13 from Lionello Lunesu <lio+bugzilla@lunesu.com> 2013-08-25 18:23:42 PDT ---
...and I filed this one for druntime: http://d.puremagic.com/issues/show_bug.cgi?id=10894

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10236



--- Comment #14 from Lionello Lunesu <lio+bugzilla@lunesu.com> 2013-08-25 18:35:13 PDT ---
While solving the warnings in druntime, there's only one instance where I thought the current patch might need to be improved:

  /**
     * Resets this fiber so that it may be re-used.  This routine may only be
     * called for fibers that have terminated, as doing otherwise could result
     * in scope-dependent functionality that is not executed.  Stack-based
     * classes, for example, may not be cleaned up properly if a fiber is reset
     * before it has terminated.
     *
     * Params:
     *  fn = The fiber function.
     *  dg = The fiber function.
     *
     * In:
     *  This fiber must be in state TERM.
     */
    final void reset();

    /// ditto
    final void reset( void function() fn );

    /// ditto
    final void reset( void delegate() dg );

This shows three warnings for the first reset(), no warnings for the "ditto". A quick fix would be to copy the whole ddoc for the two overloads, but perhaps there's something smarter that can be done here?

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10236



--- Comment #15 from bearophile_hugs@eml.cc 2013-08-25 19:12:47 PDT ---
(In reply to comment #14)

> This shows three warnings for the first reset(), no warnings for the "ditto". A quick fix would be to copy the whole ddoc for the two overloads, but perhaps there's something smarter that can be done here?

Here "Params" is used in a non-standard way, it shows the merged arguments of all the overloads of the "reset" function. This merging should be accepted by the warning code only if dittos are used. So a possible solution is to create a set of all the arguments of the overloads tagged with "ditto", and then verify such set is the same as the set of arguments listed in the "Params" section.

(But usually warnings aren't 100.00% reliable. Usually there are very uncommon cases where a warning gives false positives and false negatives. This ddoc warning seems to be nearly perfect, but I don't expect it to be really perfect. I think here a error rate of 0.1% or 0.01% is acceptable. Keeping warning implementation simple is sometimes better than trying to further reduce down that tiny percentage of errors.)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10236



--- Comment #16 from Lionello Lunesu <lio+bugzilla@lunesu.com> 2013-08-27 04:38:47 PDT ---
(In reply to comment #15)
> (But usually warnings aren't 100.00% reliable. Usually there are very uncommon cases where a warning gives false positives and false negatives. This ddoc warning seems to be nearly perfect, but I don't expect it to be really perfect. I think here a error rate of 0.1% or 0.01% is acceptable. Keeping warning implementation simple is sometimes better than trying to further reduce down that tiny percentage of errors.)

I agree that this case is not a priority and the patch would be acceptable with it unsolved.

Another issue though is that I'm not checking template parameters. This is actually a bigger issue, as it happens far more often in Phobos and druntime. In fact, there are many occurences of both: many templates that do and many that don't document the template parameters.

Now I wonder whether it's a good idea to use "Params:" for both compile time and runtime parameters. In the very least the generated documentation should mention whether it's one or the other.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10236



--- Comment #17 from Lionello Lunesu <lio+bugzilla@lunesu.com> 2013-09-01 20:51:22 PDT ---
Pull request was updated to allow for template parameters:
 * parameters in the DDoc Params section must either be function parameters of
template parameters
 * all function parameters must appear in the Params section, but this is not
checked for template parameters.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10236



--- Comment #18 from github-bugzilla@puremagic.com 2013-09-15 00:52:10 PDT ---
Commits pushed to master at https://github.com/D-Programming-Language/dmd

https://github.com/D-Programming-Language/dmd/commit/9dbb905f84dfe5a240033179172ff603f2a4b741 Fix issue 10236 Ddoc: Warning on wrong parameter names/count

https://github.com/D-Programming-Language/dmd/commit/51afa9a24d94211e2e93dfd11bcb66c4d76ed96a Fix issue 10236 test case

https://github.com/D-Programming-Language/dmd/commit/359dc6697b92ca3c7b1fc4fddc30c2b981cf56eb Fix issue 10236: allow template parameters in Params section + test

https://github.com/D-Programming-Language/dmd/commit/151ad0104e457ed3476bd4b2b9447bb7ce710cc5 Merge pull request #2121 from lionello/bug10236

Fix issue 10236 Ddoc: Warning on wrong parameter names/count

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10236


Walter Bright <bugzilla@digitalmars.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
             Status|NEW                         |RESOLVED
         Resolution|                            |FIXED
           Severity|normal                      |enhancement


-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------