On 10/18/2018 03:12 PM, David Malcolm wrote:

Here's an updated version of the patch, addressing your above comments,
and those from Martin and Richard (I hope).

Thanks, this one looks more readable. Some more specific comments included inline below.

I have a couple of texinfo questions:

(a) the guidelines frequently have contrasting pairs
of examples showing how to do something vs how not to do it.  Is there
a way of marking these up in texinfo beyond just @smallexample?
(and manually putting in "BAD" and "OK", as I've done)

No, there's no markup for this. I think the brief comments in the code example and longer discussion in the surrounding text is fine.

(b) what's the best way of showing example output from gcc?  In
particular I wasn't able to properly express the single quotes emitted by
GCC's %qs, %<, and %> directives: everything I've tried so far has issues
in at least one of the pdf vs the html output.  I've settled for using
single quotes, which is easy to emit via LANG=C and looks OK in html,
but less good in pdf.

I don't understand this question. Isn't the best way to show single quotes in the output, single quotes? :-S


+@cindex diagnostics, true positive
+@cindex false positive
+@cindex true positive
+
+Warnings should have a good @dfn{signal-to-noise ratio}: we should have few
+@dfn{false positives} (falsely issuing a warning when no warning is
+warranted) and few @dfn{false negatives} (failing to issue a warning when
+one @emph{is} justified).
+
+Note that a ``false positive'' can mean, in practice, a warning that the

No quote markup needed there.

+@noindent
+This will emit either one diagnostic with two locations:

s/will emit/emits/

+Avoid using the @code{input_location} global, and the diagnostic functions
+that implicitly use it - use @code{error_at} and @code{warning_at} rather

Long dashes in Texinfo are marked up as '---' (three hyphens) with no surrounding whitespace.

+@noindent
+@anchor{input_location_example}
+For example, in the example of imprecise wording
+above, the diagnostic was generated using @code{warning}:

How about rephrasing that as

For example, generating the diagnostic using @code{warning} results in the imprecise wording in the example above:

which puts it both in the present tense and active voice.


+would lead to:
+
+@smallexample
+// OK: use location of attribute, with a secondary location
+demo.c:1:24: warning: attribute 'noinline' on variable 'foo' was ignored 
[-Wattributes]

The above line seems long enough that it might overflow into the right margin. You probably want to use -fmessage-length=70 or something like that for these examples.

+@subsection Coding Conventions
+
+See the @uref{https://gcc.gnu.org/codingconventions.html#Diagnostics,
+diagnostics section} of the GCC coding conventions.
+
+In the C++ frontend, when comparing two types in a message, use @code{%H}

s/frontend/front end/

I think you should be using @samp markup, rather than @code, on all instances of these %-format directives throughout the running text.

+and @code{%I} rather tha @code{%T}, as this allows the diagnostics

s/tha/than/

+subsystem to highlight differences between template-based types.
+For example, rather than using @code{%qT}:
+
+@smallexample
+  // BAD: a pair of %qT used in C++ frontend for type comparison

s/frontend/front end/ again

+  error_at (loc, "could not convert %qE from %qT to %qT", expr,
+            TREE_TYPE (expr), type);
+@end smallexample
+
+@noindent
+which could lead to:
+
+@smallexample
+error: could not convert 'map<int, double>()' from 'map<int,double>' to 
'map<int,int>'

That line looks too long too.

+@end smallexample
+
+@noindent
+using @code{%H} and @code{%I} (via @code{%qH} and @code{%qI}):
+
+@smallexample
+  // OK: compare types in C++ frontend via %qH and %qI

s/frontend/front end/ again

+  error_at (loc, "could not convert %qE from %qH to %qI", expr,
+            TREE_TYPE (expr), type);
+@end smallexample
+
+@noindent
+allows the above output to be simplified to:
+
+@smallexample
+error: could not convert 'map<int, double>()' from 'map<[...],double>' to 
'map<[...],int>'

Another too-long line.

+@end smallexample
+
+@noindent
+where the @code{double} and @code{int} are colorized to highlight them.
+
+@c %H and %I were added in r248698.
+
+Use @code{auto_diagnostic_group} when issuing multiple related
+diagnostics (seen in various examples on this page).  This informs the
+diagnostic subsystem that all diagnostics issued within the lifetime
+of the @code{auto_diagnostic_group} are related.  (Currently it doesn't
+do anything with this information, but we may implement that in the
+future).

I'm not real keen on documenting not-yet-implemented features. I've deleted a bunch of "we may implement that in the future" things from the GCC user documentation that were written 20+ years ago and never implemented. :-P

+@noindent
+which can lead to:
+
+@smallexample
+spellcheck-typenames.C:73:1: error: 'singed' does not name a type; did you 
mean 'signed'?

Another too-long line.

+73 | singed char ch;
+   | ^~~~~~
+   | signed
+@end smallexample
+
+Non-trivial edits can be built up by adding multiple fix-it hints to one
+@code{rich_location}.  It's best to express the edits in terms of the
+locations of individual tokens.  Various handy functions for adding
+fix-it hints for idiomatic C and C++ can be seen in
+@file{gcc-rich-location.h}.
+
+@subsubsection Fix-it hints should be applyable

"Applyable" isn't a word.  How about just saying "Fix-it hints should work"?

+@noindent
+The above implementation will erroneously strip out the macro and the

s/will erroneously strip/erroneously strips/

If this were user-facing documentation I'd be a bit more nit-picky, but it's readable and well-organized as is. Plus the rest of the GCC internals manual has much worse problems with markup (etc) throughout and still manages to be useful, so I'm not interested in letting the perfect be the enemy of the good here. :-) Again, thanks for writing this up!

-Sandra

Reply via email to