llvm dev - Mar 2020 - [RFC] Coding standard for error/warning messages

Hi all,

This came up in a recent review. There is currently no documented style for
how to write error messages. For example, should they start with a capital
letter or end in a full stop? Consequently, there's quite a bit of
inconsistency in our diagnostics throughout the code base.

clang typically emits error messages with no leading capital letter and no
trailing full stop. For example:

C:\>clang
"clang: error: no input files"

I have suggested this approach be followed in many different reviews,
primarily in the LLVM equivalents of the GNU binutils that I typically work
on. I'd like to propose that this be followed more widely too, and
documented in the coding standards as such. Note, I am not proposing
changing existing error messages as part of this. Do people agree with this
proposal? If not, what would you prefer to see?

As well as "regular" errors you'll see in typical usage, there are
3 other
kinds of errors that are widely used, with the following output styles:

Assertion failures:

Assertion failed: false && "this is the message", file
<filepath>

llvm_unreachable failures:

this is the message
UNREACHABLE executed at <filepath>

report_fatal_error failures:

LLVM ERROR: this is the message

Looking at the existing output, and how they are used, I think
llvm_unreachable and assertions do not need standardising, since they are
purely for internal usage, whilst report_fatal_error should be standardised
to the same as other normal errors (i.e. lower-case first letter, trailing
full stop).

What do people think?

James
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200323/3b6f7e3c/attachment.html>

Sounds good to me - and I guess the LLVM coding standards would be the
place to put it, though it feels a smidge out of place since it's
about external facing functionality, where the coding standards don't
usually touch on that. It does sort of feel like it's adjacent to the
comment style documentation (though that goes the other way - full
sentences with capital/full stop - not that the difference is a problem & I
think it's fine/correct that these be as they are/as you're suggesting):
https://llvm.org/docs/CodingStandards.html#comment-formatting

On Mon, Mar 23, 2020 at 8:54 AM James Henderson via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi all,
>
> This came up in a recent review. There is currently no documented style
> for how to write error messages. For example, should they start with a
> capital letter or end in a full stop? Consequently, there's quite a bit
of
> inconsistency in our diagnostics throughout the code base.
>
> clang typically emits error messages with no leading capital letter and no
> trailing full stop. For example:
>
> C:\>clang
> "clang: error: no input files"
>
> I have suggested this approach be followed in many different reviews,
> primarily in the LLVM equivalents of the GNU binutils that I typically work
> on. I'd like to propose that this be followed more widely too, and
> documented in the coding standards as such. Note, I am not proposing
> changing existing error messages as part of this. Do people agree with this
> proposal? If not, what would you prefer to see?
>
> As well as "regular" errors you'll see in typical usage,
there are 3 other
> kinds of errors that are widely used, with the following output styles:
>
> Assertion failures:
>
> Assertion failed: false && "this is the message", file
<filepath>
>
> llvm_unreachable failures:
>
> this is the message
> UNREACHABLE executed at <filepath>
>
> report_fatal_error failures:
>
> LLVM ERROR: this is the message
>
> Looking at the existing output, and how they are used, I think
> llvm_unreachable and assertions do not need standardising, since they are
> purely for internal usage, whilst report_fatal_error should be standardised
> to the same as other normal errors (i.e. lower-case first letter, trailing
> full stop).
>
> What do people think?
>
> James
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200323/40c6956a/attachment.html>

Thanks for the feedback. I agree for the same reasons as you that the
Coding Standards don't quite feel like the right place for it, but I'm
unaware of anywhere else. I'll give this a day or two to see if there are
any more comments before looking at a patch.

On Mon, 23 Mar 2020 at 17:36, David Blaikie <dblaikie at gmail.com> wrote:
> Sounds good to me - and I guess the LLVM coding standards would be the
> place to put it, though it feels a smidge out of place since it's
> about external facing functionality, where the coding standards don't
> usually touch on that. It does sort of feel like it's adjacent to the
> comment style documentation (though that goes the other way - full
> sentences with capital/full stop - not that the difference is a problem
& I
> think it's fine/correct that these be as they are/as you're
suggesting):
> https://llvm.org/docs/CodingStandards.html#comment-formatting
>
> On Mon, Mar 23, 2020 at 8:54 AM James Henderson via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hi all,
>>
>> This came up in a recent review. There is currently no documented style
>> for how to write error messages. For example, should they start with a
>> capital letter or end in a full stop? Consequently, there's quite a
bit of
>> inconsistency in our diagnostics throughout the code base.
>>
>> clang typically emits error messages with no leading capital letter and
>> no trailing full stop. For example:
>>
>> C:\>clang
>> "clang: error: no input files"
>>
>> I have suggested this approach be followed in many different reviews,
>> primarily in the LLVM equivalents of the GNU binutils that I typically
work
>> on. I'd like to propose that this be followed more widely too, and
>> documented in the coding standards as such. Note, I am not proposing
>> changing existing error messages as part of this. Do people agree with
this
>> proposal? If not, what would you prefer to see?
>>
>> As well as "regular" errors you'll see in typical usage,
there are 3
>> other kinds of errors that are widely used, with the following output
>> styles:
>>
>> Assertion failures:
>>
>> Assertion failed: false && "this is the message",
file <filepath>
>>
>> llvm_unreachable failures:
>>
>> this is the message
>> UNREACHABLE executed at <filepath>
>>
>> report_fatal_error failures:
>>
>> LLVM ERROR: this is the message
>>
>> Looking at the existing output, and how they are used, I think
>> llvm_unreachable and assertions do not need standardising, since they
are
>> purely for internal usage, whilst report_fatal_error should be
standardised
>> to the same as other normal errors (i.e. lower-case first letter,
trailing
>> full stop).
>>
>> What do people think?
>>
>> James
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200324/59ae3162/attachment.html>

On Mon, Mar 23, 2020 at 11:54 AM James Henderson via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi all,
>
> This came up in a recent review. There is currently no documented style
> for how to write error messages. For example, should they start with a
> capital letter or end in a full stop?
>
The policy should also mention use of other end-of-sentence punctuation.
Also, for multi-sentence messages like
"err_drv_trivial_auto_var_init_zero_disabled".

> Consequently, there's quite a bit of inconsistency in our diagnostics
> throughout the code base.
>
> clang typically emits error messages with no leading capital letter and no
> trailing full stop. For example:
>
> C:\>clang
> "clang: error: no input files"
>
> I have suggested this approach be followed in many different reviews,
> primarily in the LLVM equivalents of the GNU binutils that I typically work
> on. I'd like to propose that this be followed more widely too, and
> documented in the coding standards as such. Note, I am not proposing
> changing existing error messages as part of this. Do people agree with this
> proposal? If not, what would you prefer to see?
>
> As well as "regular" errors you'll see in typical usage,
there are 3 other
> kinds of errors that are widely used, with the following output styles:
>
> Assertion failures:
>
> Assertion failed: false && "this is the message", file
<filepath>
>
> llvm_unreachable failures:
>
> this is the message
> UNREACHABLE executed at <filepath>
>
I understand the proposal is not meant to cover llvm_unreachable; however,
I do want to point out that the non-full-stop and lowercase style in this
output format made the message completely unnoticeable for me. A bit less
so for the assertion failure output.

>
> report_fatal_error failures:
>
> LLVM ERROR: this is the message
>
> Looking at the existing output, and how they are used, I think
> llvm_unreachable and assertions do not need standardising, since they are
> purely for internal usage, whilst report_fatal_error should be standardised
> to the same as other normal errors (i.e. lower-case first letter, trailing
> full stop).
>
I believe that the document would clearly indicate that the appearance of
the output differs for llvm_unreachable and assertions, making it so that
the guidance may not be suitable for those cases (and thus it does not
apply in those cases).

>
> What do people think?
>
> James
> _______________________________________________
> LLVM Developers mailing list
> llvm-dev at lists.llvm.org
> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200324/e075172f/attachment.html>

Thanks for the feedback. I've now put https://reviews.llvm.org/D76833 up
for review.

On Tue, 24 Mar 2020 at 12:57, Hubert Tong <hubert.reinterpretcast at
gmail.com>
wrote:
> On Mon, Mar 23, 2020 at 11:54 AM James Henderson via llvm-dev <
> llvm-dev at lists.llvm.org> wrote:
>
>> Hi all,
>>
>> This came up in a recent review. There is currently no documented style
>> for how to write error messages. For example, should they start with a
>> capital letter or end in a full stop?
>>
> The policy should also mention use of other end-of-sentence punctuation.
> Also, for multi-sentence messages like
> "err_drv_trivial_auto_var_init_zero_disabled".
>
>
>> Consequently, there's quite a bit of inconsistency in our
diagnostics
>> throughout the code base.
>>
>> clang typically emits error messages with no leading capital letter and
>> no trailing full stop. For example:
>>
>> C:\>clang
>> "clang: error: no input files"
>>
>> I have suggested this approach be followed in many different reviews,
>> primarily in the LLVM equivalents of the GNU binutils that I typically
work
>> on. I'd like to propose that this be followed more widely too, and
>> documented in the coding standards as such. Note, I am not proposing
>> changing existing error messages as part of this. Do people agree with
this
>> proposal? If not, what would you prefer to see?
>>
>> As well as "regular" errors you'll see in typical usage,
there are 3
>> other kinds of errors that are widely used, with the following output
>> styles:
>>
>> Assertion failures:
>>
>> Assertion failed: false && "this is the message",
file <filepath>
>>
>> llvm_unreachable failures:
>>
>> this is the message
>> UNREACHABLE executed at <filepath>
>>
> I understand the proposal is not meant to cover llvm_unreachable; however,
> I do want to point out that the non-full-stop and lowercase style in this
> output format made the message completely unnoticeable for me. A bit less
> so for the assertion failure output.
>
>
>>
>> report_fatal_error failures:
>>
>> LLVM ERROR: this is the message
>>
>> Looking at the existing output, and how they are used, I think
>> llvm_unreachable and assertions do not need standardising, since they
are
>> purely for internal usage, whilst report_fatal_error should be
standardised
>> to the same as other normal errors (i.e. lower-case first letter,
trailing
>> full stop).
>>
> I believe that the document would clearly indicate that the appearance of
> the output differs for llvm_unreachable and assertions, making it so that
> the guidance may not be suitable for those cases (and thus it does not
> apply in those cases).
>
>
>>
>> What do people think?
>>
>> James
>> _______________________________________________
>> LLVM Developers mailing list
>> llvm-dev at lists.llvm.org
>> https://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-dev
>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20200326/18fba0c3/attachment.html>