1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169 170 171 172 173 174 175 176 177 178 179 180 181 182 183 184 185 186 187 188 189 190 191 192 193 194 195 196 197 198 199 200 201 202 203 204 205 206 207 208 209 210 211 212 213 214 215 216 217 218 219 220 221 222 223 224 225 226 227 228 229 230 231 232 233 234 235 236 237 238 239 240 241 242 243 244 245 246 247 248 249 250 251 252 253 254 255 256 257 258 259 260 261 262 263 264 265 266 267 268 269 270 271 272 273 274 275 276 277 278 279 280 281 282 283 284 285 286 287 288 289 290 291 292 293 294 295 296 297 298 299 300 301 302 303 304 305 306 307 308 309 310 311 312 313 314 315 316 317 318 319 320 321 322 323 324 325 326 327 328 329 330 331 332 333 334 335 336 337 338 339 340 341 342 343 344 345 346 347 348 349 350 351 352 353 354 355 356 357 358 359 360 361 362 363 364 365 366 367 368 369 370 371 372 373 374 375 376 377 378 379 380 381 382 383 384 385 386 387 388 389 390 391 392 393 394 395 396 397 398 399 400 401 402 403 404 405 406 407 408 409 410 411 412 413 414 415 416 417 418 419 420 421 422 423 424 425 426 427 428 429 430 431 432 433 434 435 436 437 438 439 440 441 442 443 444 445 446 447 448 449 450 451 452 453 454 455 456 457 458 459 460 461 462 463 464 465 466 467 468 469 470 471 472 473 474 475 476 477 478 479 480 481 482 483 484 485 486 487 488 489 490 491 492 493 494 495 496 497 498 499 500 501 502 503 504 505 506 507 508 509 510 511 512 513 514 515 516 517 518 519 520 521 522 523 524 525 526 527 528 529 530 531 532 533 534 535 536 537 538 539 540 541 542 543 544 545 546 547 548 549 550 551 552 553 554 555 556 557 558 559 560 561 562 563 564 565 566 567 568 569 570 571 572 573 574 575 576 577 578 579 580 581 582 583 584 585 586 587 588 589 590 591 592 593 594 595 596 597 598 599 600 601 602 603 604 605 606 607 608 609 610 611 612 613 614 615 616 617 618 619 620 621 622 623 624 625 626 627 628 629 630 631 632 633 634 635 636 637 638 639 640 641 642 643 644 645 646 647 648 649 650 651 652 653 654 655 656 657 658 659 660 661
|
@c Copyright (C) 2018-2022 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
|