[cfe-commits] PATCH: Fix another bunch of Doxygen-formatting gremlins

[James Dennett]

On Tue, Jun 12, 2012 at 5:11 PM, Dmitri Gribenko <gribozavr at gmail.com> wrote:
> Hi James,
>
> Thank you for this work!  I have two comments.

Much appreciated.  Replies below.

> On Tue, Jun 12, 2012 at 4:57 PM, James Dennett <jdennett at google.com> wrote:
>> This reduces the number of warnings generated by Doxygen by about 100
>> (roughly 10%).
>
> Index: include/clang/Frontend/HeaderSearchOptions.h
> ===================================================================
> --- include/clang/Frontend/HeaderSearchOptions.h        (revision 158335)
> +++ include/clang/Frontend/HeaderSearchOptions.h        (working copy)
> @@ -17,12 +17,12 @@ namespace clang {
>
>  namespace frontend {
>   /// IncludeDirGroup - Identifiers the group a include entry belongs to, which
> -  /// represents its relative positive in the search list.  A #include of a ""
> +  /// represents its relative positive in the search list.  A \#include of a ""
>   /// path starts at the -iquote group, then searches the Angled group, then
>   /// searches the system group, etc.
>   enum IncludeDirGroup {
> -    Quoted = 0,     ///< '#include ""' paths, added by'gcc -iquote'.
> -    Angled,         ///< Paths for '#include <>' added by '-I'.
> +    Quoted = 0,     ///< '\#include ""' paths, added by'gcc -iquote'.
> +    Angled,         ///< Paths for '\#include <>' added by '-I'.
>     IndexHeaderMap, ///< Like Angled, but marks header maps used when
>                        ///  building frameworks.
>     System,         ///< Like Angled, but marks system directories.
>
> Missing space in "by'gcc".

Yes, that's worth fixing as a drive-by while I'm here.  Patch updated
for that 1-char delta (and attached).

> Index: include/clang/Basic/SourceManagerInternals.h
> ===================================================================
> --- include/clang/Basic/SourceManagerInternals.h        (revision 158335)
> +++ include/clang/Basic/SourceManagerInternals.h        (working copy)
> @@ -30,11 +30,11 @@ struct LineEntry {
>   /// FileOffset - The offset in this file that the line entry occurs at.
>   unsigned FileOffset;
>
> -  /// LineNo - The presumed line number of this line entry: #line 4.
> +  /// LineNo - The presumed line number of this line entry: \#line 4.
>   unsigned LineNo;
>
> Doxygen understands which comment is attached to which declaration.
> Programmer sees that too.  So there is no need to duplicate the
> identifier within comments.  I know that this duplication is all over
> the clang codebase, but since you are touching those lines anyway...

I agree that it would be good to avoid this duplication (which makes
the Doxygen output less friendly also).  However: unless there's an
objection, I'd like to do that in separate patches, so that (1) I can
do it consistently (for example, in 9 places in this one file), and
(2) so that I can see if there's consensus that we'd prefer to avoid
these redundant identifiers, and get a feel for what level of churn in
comments is acceptable.  I feel that the worst of all possible worlds
is to update just the few lines that are being otherwise touched -- if
global consistency isn't possible, I'd like to at least aim for (a)
being consistent within a file, and (b) having agreement on the
preferred style for new documentation.

Thanks again.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: doxydocs-2v2.patch
Type: application/octet-stream
Size: 34910 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20120612/9aa722a8/attachment.obj>