Here's a proposed "User Experience Guidelines" section for our
internals manual
It's a mixture of proposed policy, together with notes on how to
implement the recommendations.
Thoughts?
gcc/ChangeLog:
* Makefile.in (TEXI_GCCINT_FILES): Add ux.texi.
* doc/gccint.texi: Include ux.texi and use it in top-level menu.
* doc/ux.texi: New file.
---
gcc/Makefile.in | 2 +-
gcc/doc/gccint.texi | 2 +
gcc/doc/ux.texi | 455 ++++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 458 insertions(+), 1 deletion(-)
create mode 100644 gcc/doc/ux.texi
diff --git a/gcc/Makefile.in b/gcc/Makefile.in
index 70efab7..3f05e95 100644
--- a/gcc/Makefile.in
+++ b/gcc/Makefile.in
@@ -3176,7 +3176,7 @@ TEXI_GCCINT_FILES = gccint.texi gcc-common.texi
gcc-vers.texi \
gnu.texi gpl_v3.texi fdl.texi contrib.texi languages.texi \
sourcebuild.texi gty.texi libgcc.texi cfg.texi tree-ssa.texi \
loop.texi generic.texi gimple.texi plugins.texi optinfo.texi \
- match-and-simplify.texi poly-int.texi
+ match-and-simplify.texi ux.texi poly-int.texi
TEXI_GCCINSTALL_FILES = install.texi install-old.texi fdl.texi \
gcc-common.texi gcc-vers.texi
diff --git a/gcc/doc/gccint.texi b/gcc/doc/gccint.texi
index 1a1af41..2554b31 100644
--- a/gcc/doc/gccint.texi
+++ b/gcc/doc/gccint.texi
@@ -125,6 +125,7 @@ Additional tutorial information is linked to from
* LTO:: Using Link-Time Optimization.
* Match and Simplify:: How to write expression simplification patterns for
GIMPLE and GENERIC
+* User Experience Guidelines:: Guidelines for implementing diagnostics and
options.
* Funding:: How to help assure funding for free software.
* GNU Project:: The GNU Project and GNU/Linux.
@@ -162,6 +163,7 @@ Additional tutorial information is linked to from
@include plugins.texi
@include lto.texi
@include match-and-simplify.texi
+@include ux.texi
@include funding.texi
@include gnu.texi
diff --git a/gcc/doc/ux.texi b/gcc/doc/ux.texi
new file mode 100644
index 0000000..87ff599
--- /dev/null
+++ b/gcc/doc/ux.texi
@@ -0,0 +1,455 @@
+@c Copyright (C) 2018 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/blog/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
+@cindex Guidelines for Diagnostics
+@cindex Diagnostics, Guidelines for
+
+@section Guidelines for Diagnostics
+
+@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''
+
+A good diagnostic is @emph{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 want to review the warning and think:
+
+@itemize @bullet
+@item
+``Is this a ``true'' result?''
+@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.
+
+Warnings should have a good ``signal:noise ratio'': we should have few
+``false positives'' (falsely issuing a warning when no warning is
+warranted) and few ``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.
+
+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.
+
+@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
+
+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
+
+This will emit either one diagnostic with two locations:
+@smallexample
+ demo.c:42:10: error: main message
+ (foo)
+ ~ ^
+@end smallexample
+
+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
+
+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
+``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 ``caret == start'',
+such as for ``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
+
+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}.
+
+@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).
+
+@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
+
+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
+
+@subsection Fix-it hints
+
+GCC's diagnostic subsystem can emit ``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 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}.
+
+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
+
+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 be ``applyable''
+
+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
+
+When applied to the code, this might lead to:
+
+@smallexample
+T::type x;
+@end smallexample
+
+being ``corrected'' to:
+
+@smallexample
+typenameT::type x;
+@end smallexample
+
+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
+
+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
+
+which is intended to e.g. replace a @code{std::move} with the underlying
+value:
+
+@smallexample
+ return std::move (retval);
+ ~~~~~~~~~~^~~~~~~~
+ retval
+@end smallexample
+
+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
+
+The above implementation will erroneously strip out the macro and the
+comment in the fix-it hint:
+
+@smallexample
+ return std::move (CONFIGURY_GLOBAL /* some comment */);
+ ~~~~~~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+ global_a
+@end smallexample
+
+and thus this resulting code:
+
+@smallexample
+ return global_a;
+@end smallexample
+
+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
+
+and thus this result:
+
+@smallexample
+ return CONFIGURY_GLOBAL /* some comment */;
+@end smallexample
+
+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
+@cindex Options, Guidelines for
+@cindex Guidelines for Options
+
+@section Guidelines for Options
+
+@c TODO
--
1.8.5.3
