@c Copyright (C) 2018-2020 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 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 % 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()' from 'map' to 'map' @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()' 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 % 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 % 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