On Tue, Jun 12, 2012 at 5:11 PM, Dmitri Gribenko <[email protected]> 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 <[email protected]> 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.
doxydocs-2v2.patch
Description: Binary data
_______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
