662 lines
20 KiB
Plaintext
662 lines
20 KiB
Plaintext
@c Copyright (C) 2018-2021 Free Software Foundation, Inc.
|
|
@c Free Software Foundation, Inc.
|
|
@c This is part of the GCC manual.
|
|
@c For copying conditions, see the file gcc.texi.
|
|
|
|
@node User Experience Guidelines
|
|
@chapter User Experience Guidelines
|
|
@cindex user experience guidelines
|
|
@cindex guidelines, user experience
|
|
|
|
To borrow a slogan from
|
|
@uref{https://elm-lang.org/news/compilers-as-assistants, Elm},
|
|
|
|
@quotation
|
|
@strong{Compilers should be assistants, not adversaries.} A compiler should
|
|
not just detect bugs, it should then help you understand why there is a bug.
|
|
It should not berate you in a robot voice, it should give you specific hints
|
|
that help you write better code. Ultimately, a compiler should make
|
|
programming faster and more fun!
|
|
@author Evan Czaplicki
|
|
@end quotation
|
|
|
|
This chapter provides guidelines on how to implement diagnostics and
|
|
command-line options in ways that we hope achieve the above ideal.
|
|
|
|
@menu
|
|
* Guidelines for Diagnostics:: How to implement diagnostics.
|
|
* Guidelines for Options:: Guidelines for command-line options.
|
|
@end menu
|
|
|
|
|
|
@node Guidelines for Diagnostics
|
|
@section Guidelines for Diagnostics
|
|
@cindex guidelines for diagnostics
|
|
@cindex diagnostics, guidelines for
|
|
|
|
@subsection Talk in terms of the user's code
|
|
|
|
Diagnostics should be worded in terms of the user's source code, and the
|
|
source language, rather than GCC's own implementation details.
|
|
|
|
@subsection Diagnostics are actionable
|
|
@cindex diagnostics, actionable
|
|
|
|
A good diagnostic is @dfn{actionable}: it should assist the user in
|
|
taking action.
|
|
|
|
Consider what an end user will want to do when encountering a diagnostic.
|
|
|
|
Given an error, an end user will think: ``How do I fix this?''
|
|
|
|
Given a warning, an end user will think:
|
|
|
|
@itemize @bullet
|
|
@item
|
|
``Is this a real problem?''
|
|
@item
|
|
``Do I care?''
|
|
@item
|
|
if they decide it's genuine: ``How do I fix this?''
|
|
@end itemize
|
|
|
|
A good diagnostic provides pertinent information to allow the user to
|
|
easily answer the above questions.
|
|
|
|
@subsection The user's attention is important
|
|
|
|
A perfect compiler would issue a warning on every aspect of the user's
|
|
source code that ought to be fixed, and issue no other warnings.
|
|
Naturally, this ideal is impossible to achieve.
|
|
|
|
@cindex signal-to-noise ratio (metaphorical usage for diagnostics)
|
|
@cindex diagnostics, false positive
|
|
@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
|
|
user doesn't agree with. Ideally a diagnostic should contain enough
|
|
information to allow the user to make an informed choice about whether
|
|
they should care (and how to fix it), but a balance must be drawn against
|
|
overloading the user with irrelevant data.
|
|
|
|
@subsection Sometimes the user didn't write the code
|
|
|
|
GCC is typically used in two different ways:
|
|
|
|
@itemize @bullet
|
|
@item
|
|
Semi-interactive usage: GCC is used as a development tool when the user
|
|
is writing code, as the ``compile'' part of the ``edit-compile-debug''
|
|
cycle. The user is actively hacking on the code themself (perhaps a
|
|
project they wrote, or someone else's), where they just made a change
|
|
to the code and want to see what happens, and to be warned about
|
|
mistakes.
|
|
|
|
@item
|
|
Batch rebuilds: where the user is recompiling one or more existing
|
|
packages, and GCC is a detail that's being invoked by various build
|
|
scripts. Examples include a user trying to bring up an operating system
|
|
consisting of hundreds of packages on a new CPU architecture, where the
|
|
packages were written by many different people, or simply rebuilding
|
|
packages after a dependency changed, where the user is hoping
|
|
``nothing breaks'', since they are unfamiliar with the code.
|
|
@end itemize
|
|
|
|
Keep both of these styles of usage in mind when implementing diagnostics.
|
|
|
|
@subsection Precision of Wording
|
|
|
|
Provide the user with details that allow them to identify what the
|
|
problem is. For example, the vaguely-worded message:
|
|
|
|
@smallexample
|
|
demo.c:1:1: warning: 'noinline' attribute ignored [-Wattributes]
|
|
1 | int foo __attribute__((noinline));
|
|
| ^~~
|
|
@end smallexample
|
|
|
|
@noindent
|
|
doesn't tell the user why the attribute was ignored, or what kind of
|
|
entity the compiler thought the attribute was being applied to (the
|
|
source location for the diagnostic is also poor;
|
|
@pxref{input_location_example,,discussion of @code{input_location}}).
|
|
A better message would be:
|
|
|
|
@smallexample
|
|
demo.c:1:24: warning: attribute 'noinline' on variable 'foo' was
|
|
ignored [-Wattributes]
|
|
1 | int foo __attribute__((noinline));
|
|
| ~~~ ~~~~~~~~~~~~~~~^~~~~~~~~
|
|
demo.c:1:24: note: attribute 'noinline' is only applicable to functions
|
|
@end smallexample
|
|
|
|
@noindent
|
|
which spells out the missing information (and fixes the location
|
|
information, as discussed below).
|
|
|
|
The above example uses a note to avoid a combinatorial explosion of possible
|
|
messages.
|
|
|
|
@subsection Try the diagnostic on real-world code
|
|
|
|
It's worth testing a new warning on many instances of real-world code,
|
|
written by different people, and seeing what it complains about, and
|
|
what it doesn't complain about.
|
|
|
|
This may suggest heuristics that silence common false positives.
|
|
|
|
It may also suggest ways to improve the precision of the message.
|
|
|
|
@subsection Make mismatches clear
|
|
|
|
Many diagnostics relate to a mismatch between two different places in the
|
|
user's source code. Examples include:
|
|
@itemize @bullet
|
|
@item
|
|
a type mismatch, where the type at a usage site does not match the type
|
|
at a declaration
|
|
|
|
@item
|
|
the argument count at a call site does not match the parameter count
|
|
at the declaration
|
|
|
|
@item
|
|
something is erroneously duplicated (e.g.@: an error, due to breaking a
|
|
uniqueness requirement, or a warning, if it's suggestive of a bug)
|
|
|
|
@item
|
|
an ``opened'' syntactic construct (such as an open-parenthesis) is not
|
|
closed
|
|
|
|
@c TODO: more examples?
|
|
@end itemize
|
|
|
|
In each case, the diagnostic should indicate @strong{both} pertinent
|
|
locations (so that the user can easily see the problem and how to fix it).
|
|
|
|
The standard way to do this is with a note (via @code{inform}). For
|
|
example:
|
|
|
|
@smallexample
|
|
auto_diagnostic_group d;
|
|
if (warning_at (loc, OPT_Wduplicated_cond,
|
|
"duplicated %<if%> condition"))
|
|
inform (EXPR_LOCATION (t), "previously used here");
|
|
@end smallexample
|
|
|
|
@noindent
|
|
which leads to:
|
|
|
|
@smallexample
|
|
demo.c: In function 'test':
|
|
demo.c:5:17: warning: duplicated 'if' condition [-Wduplicated-cond]
|
|
5 | else if (flag > 3)
|
|
| ~~~~~^~~
|
|
demo.c:3:12: note: previously used here
|
|
3 | if (flag > 3)
|
|
| ~~~~~^~~
|
|
@end smallexample
|
|
|
|
@noindent
|
|
The @code{inform} call should be guarded by the return value from the
|
|
@code{warning_at} call so that the note isn't emitted when the warning
|
|
is suppressed.
|
|
|
|
For cases involving punctuation where the locations might be near
|
|
each other, they can be conditionally consolidated via
|
|
@code{gcc_rich_location::add_location_if_nearby}:
|
|
|
|
@smallexample
|
|
auto_diagnostic_group d;
|
|
gcc_rich_location richloc (primary_loc);
|
|
bool added secondary = richloc.add_location_if_nearby (secondary_loc);
|
|
error_at (&richloc, "main message");
|
|
if (!added secondary)
|
|
inform (secondary_loc, "message for secondary");
|
|
@end smallexample
|
|
|
|
@noindent
|
|
This will emit either one diagnostic with two locations:
|
|
@smallexample
|
|
demo.c:42:10: error: main message
|
|
(foo)
|
|
~ ^
|
|
@end smallexample
|
|
|
|
@noindent
|
|
or two diagnostics:
|
|
|
|
@smallexample
|
|
demo.c:42:4: error: main message
|
|
foo)
|
|
^
|
|
demo.c:40:2: note: message for secondary
|
|
(
|
|
^
|
|
@end smallexample
|
|
|
|
@subsection Location Information
|
|
@cindex diagnostics, locations
|
|
@cindex location information
|
|
@cindex source code, location information
|
|
@cindex caret
|
|
|
|
GCC's @code{location_t} type can support both ordinary locations,
|
|
and locations relating to a macro expansion.
|
|
|
|
As of GCC 6, ordinary locations changed from supporting just a
|
|
point in the user's source code to supporting three points: the
|
|
@dfn{caret} location, plus a start and a finish:
|
|
|
|
@smallexample
|
|
a = foo && bar;
|
|
~~~~^~~~~~
|
|
| | |
|
|
| | finish
|
|
| caret
|
|
start
|
|
@end smallexample
|
|
|
|
Tokens coming out of libcpp have locations of the form @code{caret == start},
|
|
such as for @code{foo} here:
|
|
|
|
@smallexample
|
|
a = foo && bar;
|
|
^~~
|
|
| |
|
|
| finish
|
|
caret == start
|
|
@end smallexample
|
|
|
|
Compound expressions should be reported using the location of the
|
|
expression as a whole, rather than just of one token within it.
|
|
|
|
For example, in @code{-Wformat}, rather than underlining just the first
|
|
token of a bad argument:
|
|
|
|
@smallexample
|
|
printf("hello %i %s", (long)0, "world");
|
|
~^ ~
|
|
%li
|
|
@end smallexample
|
|
|
|
@noindent
|
|
the whole of the expression should be underlined, so that the user can
|
|
easily identify what is being referred to:
|
|
|
|
@smallexample
|
|
printf("hello %i %s", (long)0, "world");
|
|
~^ ~~~~~~~
|
|
%li
|
|
@end smallexample
|
|
|
|
@c this was r251239
|
|
|
|
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}, and provide the most appropriate
|
|
@code{location_t} value available at that phase of the compilation. It's
|
|
possible to supply secondary @code{location_t} values via
|
|
@code{rich_location}.
|
|
|
|
@noindent
|
|
@anchor{input_location_example}
|
|
For example, in the example of imprecise wording above, generating the
|
|
diagnostic using @code{warning}:
|
|
|
|
@smallexample
|
|
// BAD: implicitly uses @code{input_location}
|
|
warning (OPT_Wattributes, "%qE attribute ignored", name);
|
|
@end smallexample
|
|
|
|
@noindent
|
|
leads to:
|
|
|
|
@smallexample
|
|
// BAD: uses @code{input_location}
|
|
demo.c:1:1: warning: 'noinline' attribute ignored [-Wattributes]
|
|
1 | int foo __attribute__((noinline));
|
|
| ^~~
|
|
@end smallexample
|
|
|
|
@noindent
|
|
which thus happened to use the location of the @code{int} token, rather
|
|
than that of the attribute. Using @code{warning_at} with the location of
|
|
the attribute, providing the location of the declaration in question
|
|
as a secondary location, and adding a note:
|
|
|
|
@smallexample
|
|
auto_diagnostic_group d;
|
|
gcc_rich_location richloc (attrib_loc);
|
|
richloc.add_range (decl_loc);
|
|
if (warning_at (OPT_Wattributes, &richloc,
|
|
"attribute %qE on variable %qE was ignored", name))
|
|
inform (attrib_loc, "attribute %qE is only applicable to functions");
|
|
@end smallexample
|
|
|
|
@noindent
|
|
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]
|
|
1 | int foo __attribute__((noinline));
|
|
| ~~~ ~~~~~~~~~~~~~~~^~~~~~~~~
|
|
demo.c:1:24: note: attribute 'noinline' is only applicable to functions
|
|
@end smallexample
|
|
|
|
@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++ front end, when comparing two types in a message, use @samp{%H}
|
|
and @samp{%I} rather than @samp{%T}, as this allows the diagnostics
|
|
subsystem to highlight differences between template-based types.
|
|
For example, rather than using @samp{%qT}:
|
|
|
|
@smallexample
|
|
// BAD: a pair of %qT used in C++ front end for type comparison
|
|
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>'
|
|
@end smallexample
|
|
|
|
@noindent
|
|
using @samp{%H} and @samp{%I} (via @samp{%qH} and @samp{%qI}):
|
|
|
|
@smallexample
|
|
// OK: compare types in C++ front end via %qH and %qI
|
|
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>'
|
|
@end smallexample
|
|
|
|
@noindent
|
|
where the @code{double} and @code{int} are colorized to highlight them.
|
|
|
|
@c %H and %I were added in r248698.
|
|
|
|
@subsection Group logically-related diagnostics
|
|
|
|
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. For example,
|
|
@option{-fdiagnostics-format=json} will treat the first diagnostic
|
|
emitted within the group as a top-level diagnostic, and all subsequent
|
|
diagnostics within the group as its children.
|
|
|
|
@subsection Quoting
|
|
Text should be quoted by either using the @samp{q} modifier in a directive
|
|
such as @samp{%qE}, or by enclosing the quoted text in a pair of @samp{%<}
|
|
and @samp{%>} directives, and never by using explicit quote characters.
|
|
The directives handle the appropriate quote characters for each language
|
|
and apply the correct color or highlighting.
|
|
|
|
The following elements should be quoted in GCC diagnostics:
|
|
|
|
@itemize @bullet
|
|
@item
|
|
Language keywords.
|
|
@item
|
|
Tokens.
|
|
@item
|
|
Boolean, numerical, character, and string constants that appear in the
|
|
source code.
|
|
@item
|
|
Identifiers, including function, macro, type, and variable names.
|
|
@end itemize
|
|
|
|
Other elements such as numbers that do not refer to numeric constants that
|
|
appear in the source code should not be quoted. For example, in the message:
|
|
|
|
@smallexample
|
|
argument %d of %qE must be a pointer type
|
|
@end smallexample
|
|
|
|
@noindent
|
|
since the argument number does not refer to a numerical constant in the
|
|
source code it should not be quoted.
|
|
|
|
@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 Fix-it hints
|
|
@cindex fix-it hints
|
|
@cindex diagnostics guidelines, fix-it hints
|
|
|
|
GCC's diagnostic subsystem can emit @dfn{fix-it hints}: small suggested
|
|
edits to the user's source code.
|
|
|
|
They are printed by default underneath the code in question. They
|
|
can also be viewed via @option{-fdiagnostics-generate-patch} and
|
|
@option{-fdiagnostics-parseable-fixits}. With the latter, an IDE
|
|
ought to be able to offer to automatically apply the suggested fix.
|
|
|
|
Fix-it hints contain code fragments, and thus they should not be marked
|
|
for translation.
|
|
|
|
Fix-it hints can be added to a diagnostic by using a @code{rich_location}
|
|
rather than a @code{location_t} - the fix-it hints are added to the
|
|
@code{rich_location} using one of the various @code{add_fixit} member
|
|
functions of @code{rich_location}. They are documented with
|
|
@code{rich_location} in @file{libcpp/line-map.h}.
|
|
It's easiest to use the @code{gcc_rich_location} subclass of
|
|
@code{rich_location} found in @file{gcc-rich-location.h}, as this
|
|
implicitly supplies the @code{line_table} variable.
|
|
|
|
For example:
|
|
|
|
@smallexample
|
|
if (const char *suggestion = hint.suggestion ())
|
|
@{
|
|
gcc_rich_location richloc (location);
|
|
richloc.add_fixit_replace (suggestion);
|
|
error_at (&richloc,
|
|
"%qE does not name a type; did you mean %qs?",
|
|
id, suggestion);
|
|
@}
|
|
@end smallexample
|
|
|
|
@noindent
|
|
which can lead to:
|
|
|
|
@smallexample
|
|
spellcheck-typenames.C:73:1: error: 'singed' does not name a type; did
|
|
you mean 'signed'?
|
|
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 work
|
|
|
|
When implementing a fix-it hint, please verify that the suggested edit
|
|
leads to fixed, compilable code. (Unfortunately, this currently must be
|
|
done by hand using @option{-fdiagnostics-generate-patch}. It would be
|
|
good to have an automated way of verifying that fix-it hints actually fix
|
|
the code).
|
|
|
|
For example, a ``gotcha'' here is to forget to add a space when adding a
|
|
missing reserved word. Consider a C++ fix-it hint that adds
|
|
@code{typename} in front of a template declaration. A naive way to
|
|
implement this might be:
|
|
|
|
@smallexample
|
|
gcc_rich_location richloc (loc);
|
|
// BAD: insertion is missing a trailing space
|
|
richloc.add_fixit_insert_before ("typename");
|
|
error_at (&richloc, "need %<typename%> before %<%T::%E%> because "
|
|
"%qT is a dependent scope",
|
|
parser->scope, id, parser->scope);
|
|
@end smallexample
|
|
|
|
@noindent
|
|
When applied to the code, this might lead to:
|
|
|
|
@smallexample
|
|
T::type x;
|
|
@end smallexample
|
|
|
|
@noindent
|
|
being ``corrected'' to:
|
|
|
|
@smallexample
|
|
typenameT::type x;
|
|
@end smallexample
|
|
|
|
@noindent
|
|
In this case, the correct thing to do is to add a trailing space after
|
|
@code{typename}:
|
|
|
|
@smallexample
|
|
gcc_rich_location richloc (loc);
|
|
// OK: note that here we have a trailing space
|
|
richloc.add_fixit_insert_before ("typename ");
|
|
error_at (&richloc, "need %<typename%> before %<%T::%E%> because "
|
|
"%qT is a dependent scope",
|
|
parser->scope, id, parser->scope);
|
|
@end smallexample
|
|
|
|
@noindent
|
|
leading to this corrected code:
|
|
|
|
@smallexample
|
|
typename T::type x;
|
|
@end smallexample
|
|
|
|
@subsubsection Express deletion in terms of deletion, not replacement
|
|
|
|
It's best to express deletion suggestions in terms of deletion fix-it
|
|
hints, rather than replacement fix-it hints. For example, consider this:
|
|
|
|
@smallexample
|
|
auto_diagnostic_group d;
|
|
gcc_rich_location richloc (location_of (retval));
|
|
tree name = DECL_NAME (arg);
|
|
richloc.add_fixit_replace (IDENTIFIER_POINTER (name));
|
|
warning_at (&richloc, OPT_Wredundant_move,
|
|
"redundant move in return statement");
|
|
@end smallexample
|
|
|
|
@noindent
|
|
which is intended to e.g.@: replace a @code{std::move} with the underlying
|
|
value:
|
|
|
|
@smallexample
|
|
return std::move (retval);
|
|
~~~~~~~~~~^~~~~~~~
|
|
retval
|
|
@end smallexample
|
|
|
|
@noindent
|
|
where the change has been expressed as replacement, replacing
|
|
with the name of the declaration.
|
|
This works for simple cases, but consider this case:
|
|
|
|
@smallexample
|
|
#ifdef SOME_CONFIG_FLAG
|
|
# define CONFIGURY_GLOBAL global_a
|
|
#else
|
|
# define CONFIGURY_GLOBAL global_b
|
|
#endif
|
|
|
|
int fn ()
|
|
@{
|
|
return std::move (CONFIGURY_GLOBAL /* some comment */);
|
|
@}
|
|
@end smallexample
|
|
|
|
@noindent
|
|
The above implementation erroneously strips out the macro and the
|
|
comment in the fix-it hint:
|
|
|
|
@smallexample
|
|
return std::move (CONFIGURY_GLOBAL /* some comment */);
|
|
~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
|
global_a
|
|
@end smallexample
|
|
|
|
@noindent
|
|
and thus this resulting code:
|
|
|
|
@smallexample
|
|
return global_a;
|
|
@end smallexample
|
|
|
|
@noindent
|
|
It's better to do deletions in terms of deletions; deleting the
|
|
@code{std::move (} and the trailing close-paren, leading to
|
|
this:
|
|
|
|
@smallexample
|
|
return std::move (CONFIGURY_GLOBAL /* some comment */);
|
|
~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
|
|
CONFIGURY_GLOBAL /* some comment */
|
|
@end smallexample
|
|
|
|
@noindent
|
|
and thus this result:
|
|
|
|
@smallexample
|
|
return CONFIGURY_GLOBAL /* some comment */;
|
|
@end smallexample
|
|
|
|
@noindent
|
|
Unfortunately, the pertinent @code{location_t} values are not always
|
|
available.
|
|
|
|
@c the above was https://gcc.gnu.org/ml/gcc-patches/2018-08/msg01474.html
|
|
|
|
@subsubsection Multiple suggestions
|
|
|
|
In the rare cases where you need to suggest more than one mutually
|
|
exclusive solution to a problem, this can be done by emitting
|
|
multiple notes and calling
|
|
@code{rich_location::fixits_cannot_be_auto_applied} on each note's
|
|
@code{rich_location}. If this is called, then the fix-it hints in
|
|
the @code{rich_location} will be printed, but will not be added to
|
|
generated patches.
|
|
|
|
|
|
@node Guidelines for Options
|
|
@section Guidelines for Options
|
|
@cindex command-line options, guidelines for
|
|
@cindex options, guidelines for
|
|
@cindex guidelines for options
|
|
|
|
@c TODO
|