[llvm] r311146 - Give guidance on report_fatal_error in CodingStandards.rst and ProgrammersManual.rst

[Justin Bogner via llvm-commits]

Alex Bradbury via llvm-commits <llvm-commits at lists.llvm.org> writes:
> Author: asb
> Date: Thu Aug 17 22:29:21 2017
> New Revision: 311146
>
> URL: http://llvm.org/viewvc/llvm-project?rev=311146&view=rev
> Log:
> Give guidance on report_fatal_error in CodingStandards.rst and
> ProgrammersManual.rst
>
> The current ProgrammersManual.rst document has a lot of well-written 
> documentation on error handling thanks to @lhames. It suggests errors can be 
> split cleanly into "programmatic" and "recoverable" errors. However, the 
> reality in current LLVM seems to be there are a number of cases where a 
> non-programmatic error is not easily recoverable. Therefore, add a note to 
> indicate the existence of report_fatal_error for these cases. I've also added 
> a reminder to CodingStandards.rst in the section on assertions, to indicate 
> that llvm_unreachable and assertions should not be relied upon to report 
> errors triggered by user input.
>
> The ProgrammersManual is also silent on the use of LLVMContext::diagnose, 
> which is used in BPF+WebAssembly+AMDGPU to report some errors during 
> instruction selection. I don't address that in this patch, as it's not quite 
> clear how to fit in to the current error handling story
>
> Differential Revision: https://reviews.llvm.org/D36826
>
> Modified:
>     llvm/trunk/docs/CodingStandards.rst
>     llvm/trunk/docs/ProgrammersManual.rst
>
> Modified: llvm/trunk/docs/CodingStandards.rst
> URL:
> http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/CodingStandards.rst?rev=311146&r1=311145&r2=311146&view=diff
> ==============================================================================
>
> --- llvm/trunk/docs/CodingStandards.rst (original)
> +++ llvm/trunk/docs/CodingStandards.rst Thu Aug 17 22:29:21 2017
> @@ -1232,6 +1232,11 @@ builds), ``llvm_unreachable`` becomes a
>  code for this branch. If the compiler does not support this, it will fall back
>  to the "abort" implementation.
>  
> +Neither assertions or ``llvm_unreachable`` will abort the program on a release
> +build. If the error condition can be triggered by user input, then the
> +recoverable error mechanism described in :doc:`ProgrammersManual` or
> +``report_fatal_error`` should be used instead.
> +
>  Another issue is that values used only by assertions will produce an "unused
>  value" warning when assertions are disabled.  For example, this code will warn:
>  
>
> Modified: llvm/trunk/docs/ProgrammersManual.rst
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/docs/ProgrammersManual.rst?rev=311146&r1=311145&r2=311146&view=diff
> ==============================================================================
> --- llvm/trunk/docs/ProgrammersManual.rst (original)
> +++ llvm/trunk/docs/ProgrammersManual.rst Thu Aug 17 22:29:21 2017
> @@ -441,6 +441,14 @@ the program where they can be handled ap
>  as simple as reporting the issue to the user, or it may involve attempts at
>  recovery.
>  
> +.. note::
> +
> +   Ideally, the error handling approach described in this section would be
> +   used throughout LLVM. However, this is not yet the case. For
> +   non-programmatic errors where the ``Error`` scheme cannot easily be
> +   applied, ``report_fatal_error`` should be used to call any installed error
> +   handler and then terminate the program.

I'm not a big fan of the "should be used" wording here, especially with
the "cannot *easily* be applied" bit. While there are situations where
report_fatal_error is the best/only option, it really should be treated
as a last resort and avoided when reasonable. What do you think of
wording this more like so?

  .. note::

     While it would be ideal to use this error handling scheme throughout
     LLVM, there are places where this hasn't been practical to apply. In
     situations where you absolutely must emit a non-programmatic error and
     the ``Error`` model isn't workable you can call ``report_fatal_error``,
     which will call installed error handlers, print a message, and exit the
     program.

Feel free to rewrite that however, but I'd like to make sure we're
discouraging the use of report_fatal_error as much as possible.

> +
>  Recoverable errors are modeled using LLVM's ``Error`` scheme. This scheme
>  represents errors using function return values, similar to classic C integer
>  error codes, or C++'s ``std::error_code``. However, the ``Error`` class is
>
>
> _______________________________________________
> llvm-commits mailing list
> llvm-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits