Another thanks for doing this.
Martin Sebor <mse...@gmail.com> writes:
> On 10/12/2018 09:43 AM, David Malcolm wrote:
>> +Avoid using the @code{input_location} global, and the diagnostic functions
>> +that implicitly use it - use @code{error_at} and @code{warning_at} rather
>> +than @code{error} and @code{warning}.
>> +
>> +@c TODO labelling of ranges
>> +
>> +@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}
>> +and @code{%I} rather tha @code{%T}, as this allows the diagnostics
>> +subsystem to highlight differences between template-based types.
>> +
>> +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).
>
> Same here. I have never used this and even though I saw the %H
> and %I patches go in I probably wouldn't have thought of using
> these codes. Having a little tutorial (or an example showing
> the effect of these directives) might help.
Yeah, agree a bad %T vs. good %H/%I example would help here.
>> +
>> +@subsection Spelling and Terminology
>> +
>> +See the @uref{https://gcc.gnu.org/codingconventions.html#Spelling
>> +Spelling, terminology and markup} section of the GCC coding conventions.
>> +
>> +@subsection Tense of messages
>> +Syntax errors occur in the present tense e.g.
>> +
>> +@smallexample
>> +error_at (loc, "cannot convert %qH to %qI");
>> +@end smallexample
>> +
>> +and thus
>> +
>> +@smallexample
>> +// CORRECT: usage of present tense:
>> +error: cannot convert 'int' to 'void *'
>> +@end smallexample
>
> This sounds fine. It would be nice to change the few instances
> of the past tense. I think past tense is appropriate in runtime
> messages like "could not open file" so it might be worth mentioning.
Yeah. Is the conversion example above really a case of fixing the tense,
or of fixing the error to be in terms of the user's code? The implicit
subject in "could not convert ..." is clearly "I"/"the compiler",
making the past tense appropriate. If the compiler failed to find
something, it wouldn't make sense to do a straight tense swap from
"failed to find..." to "fail to find...".
In "cannot convert ..." I think the subject becomes "you"/"the code" and
the message becomes a prohibition. That's a good thing because like you
say the error should be in terms of the user's code (although I guess,
going back to your quote at the start of the page, that also means it
has a slightly more lecturing tone -- "hey, you can't do that!")
I guess using the present tense also means that we shouldn't treat the
code as a narrative, so maybe the examples should include diagnostics
that correctly talk in terms of the user's code but use the wrong tense.
E.g. if we don't want to use the past tense:
error ("redefinition of %q#D", value);
inform (DECL_SOURCE_LOCATION (value),
"%q#D previously defined here", value);
should presumably be something like:
error ("redefinition of %q#D", value);
inform (DECL_SOURCE_LOCATION (value),
"previous definition of %q#D is here", value);
(Although TBH the original or:
"previous definition of %q#D was here"
sound more natural to me.)
>> +rather than
>> +
>> +@smallexample
>> +// BAD: usage of past tense:
>> +error: could not convert 'int' to 'void *'
>> +@end smallexample
>> +
>> +Predictions about run-time behavior should be described in the future tense
>> e.g.
>> +
>> +@smallexample
>> +warning_at (loc, OPT_some_warning,
>> + "this code will read past the end of the array");
>> +@end smallexample
>
> I don't think this will always achieve a good result. Consider
> warnings like -Wuninitialized:
>
> 'x' is used uninitialized in this function
>
> A slightly different example is -Walloca, -Warray-bounds, and
> -Wvla: it makes more sense (to me) to say "value is too large"
> or "index is out of bounds" than "it will be too large" etc.
>
> It would be fine for warnings like -Wformat-truncation but all
> it will do there is add a word without making things any clearer:
> "output truncated" vs "output will be truncated."
FWIW, this would also be the opposite of the GCC convention for
documentation, where predictions about compiler behaviour in particular
situations should be given in the present tense rather than the future
tense.
Thanks,
Richard
