FAQ
Hi Russ,

Surely it would not be ideal to have to discard return values in cases like
this. However, I would trust the Go team to be smarter than to add such
annotations onto functions where it is not needed. But there are definitely
cases where it is essential that the error be checked.

I would suggest to add a keyword or annotation (let's called it "checked"
for the purposes of this discussion) in front of the error return part of
the function signature, to indicate to the compiler to enforce that an
error be received by the caller.

The benefit of getting a compiler error for missing error handling on
certain important functions by far outweighs the "overhead" imposed on us
by some (hopefully few) poorly designed APIs. And actually, if some poorly
designed API function turned out to have been incorrectly marked, it would
not break an existing code base to remove the checked annotation in a
future release.

:) Hein
On Wednesday, November 2, 2011 10:04:42 PM UTC+1, Russ Cox wrote:
On Wed, Nov 2, 2011 at 16:59, Jonathan Amsterdam wrote:
What are its problems? I've been lovin' it ever since I learned about
it a few months ago.
For one thing, someone decided it was a good idea to
tag perfectly reasonable C library calls like write(2) with
the warn_unused_result attribute, which is a royal pain.
I am expecting the next Ubuntu release to tag exit(2).

Russ

--

Search Discussions

  • Minux at Nov 4, 2012 at 5:57 am

    On Sun, Nov 4, 2012 at 8:29 AM, Hein Meling wrote:

    Surely it would not be ideal to have to discard return values in cases
    like this. However, I would trust the Go team to be smarter than to add
    such annotations onto functions where it is not needed. But there are
    definitely cases where it is essential that the error be checked.

    I would suggest to add a keyword or annotation (let's called it "checked"
    for the purposes of this discussion) in front of the error return part of
    the function signature, to indicate to the compiler to enforce that an
    error be received by the caller.

    The benefit of getting a compiler error for missing error handling on
    certain important functions by far outweighs the "overhead" imposed on us
    by some (hopefully few) poorly designed APIs. And actually, if some poorly
    designed API function turned out to have been incorrectly marked, it would
    not break an existing code base to remove the checked annotation in a
    future release.
    Don't do that. The compiler can't make guarantee that the error is
    correctly handled.

    for example, given the gcc warn_unused_result attribute for function f, you
    can do the following
    to trick gcc (-Wall -Wextra compiled without warning):
    __attribute__((__warn_unused_result__)) int f(int x) { return x * 2; }
    void g() {
    __typeof__(f(1)) y;
    y = f(1);
    }

    However, this surely isn't what the f()'s author expects -- he/she wants
    the user to act on the
    return value, but this is not the case.

    --
  • Hein Meling at Nov 4, 2012 at 5:18 pm

    On Sunday, November 4, 2012 6:57:45 AM UTC+1, minux wrote:

    On Sun, Nov 4, 2012 at 8:29 AM, Hein Meling <hein....@gmail.com<javascript:>
    wrote:
    Surely it would not be ideal to have to discard return values in cases
    like this. However, I would trust the Go team to be smarter than to add
    such annotations onto functions where it is not needed. But there are
    definitely cases where it is essential that the error be checked.

    I would suggest to add a keyword or annotation (let's called it "checked"
    for the purposes of this discussion) in front of the error return part of
    the function signature, to indicate to the compiler to enforce that an
    error be received by the caller.

    The benefit of getting a compiler error for missing error handling on
    certain important functions by far outweighs the "overhead" imposed on us
    by some (hopefully few) poorly designed APIs. And actually, if some poorly
    designed API function turned out to have been incorrectly marked, it would
    not break an existing code base to remove the checked annotation in a
    future release.
    Don't do that. The compiler can't make guarantee that the error is
    correctly handled.
    IMO, that's beside the point. The compiler cannot reveal all kinds of other
    bugs either. The point is to let the programmer know that you really should
    handle this particular error. Not to prevent the programmer from ignoring
    it, if that is what the programmer wants. And by using _ to ignore such
    errors, the overhead is marginal for those cases.

    for example, given the gcc warn_unused_result attribute for function f,
    you can do the following
    to trick gcc (-Wall -Wextra compiled without warning):
    __attribute__((__warn_unused_result__)) int f(int x) { return x * 2; }
    void g() {
    __typeof__(f(1)) y;
    y = f(1);
    }
    I'm for sure not suggesting to use such a horrible notation with __ and ((
    )) all over...

    However, this surely isn't what the f()'s author expects -- he/she wants
    the user to act on the
    return value, but this is not the case.
    Again, the point is not to prevent the programmer from doing something
    stupid, but to let that be a conscious decision made by the programmer,
    e.g. by using _ to discard the error. However, if the compiler does not
    check/warn about this, it is quite easy to forget!!

    All the best,

    :) Hein

    --
  • Kevin Gillette at Nov 4, 2012 at 2:15 pm
    I think this is an excellent job for a separate tool; when the compiler
    doesn't produce any warnings whatsoever right now, it takes exponentially
    more momentum and justification to add the first warning, compared to if
    you suggested the gcc team add yet another warning to their already long
    list. Beyond that, it doesn't seem like a hard problem to me:

    A separate tool can:

    1) Parse any number of configured or option-passed whitelist/blacklist
    files, of the form:

    allow:
    fmt

    deny:
    fmt Scan*

    This would have a syntax similar to hgignore and gitignore files (using
    globs or regexes -- it doesn't really matter). The above will configure the
    tool to not report any unchecked errors for calls into the fmt package,
    unless it's a fmt.Scan* func being called. There are few enough
    white-listable cases that patterns may not be useful. White/blacklisting
    can happen on the package, type, or func/method level, and could borrow
    godoc's commandline syntax (which would hopefully be extended to handle
    methods).

    When resolving funcs or methods, the most specific applicable rule
    (func/method, then type, then package) takes precedence, and where
    conflicting rules of the same specificity are present, blacklisting takes
    precedence. Default would be to blacklist anything without a rule, as that
    is aligned with the common philosophy to, when in doubt, check every error.

    2) Scan requested source files and find all calls that return an error type
    that is ignored, and for each, unless the most applicable rule is to allow
    the error to go unchecked, report that file/line to stdout.

    This design doesn't account for other potentially pernicious cases, like
    ignoring the isPrefix return value from `bufio *Reader ReadLine` (which in
    Go syntax would be `(*bufio.Reader).ReadLine`, I think.

    --
  • Hein Meling at Nov 4, 2012 at 5:32 pm
    Well, I'd much rather have the standard compiler help us with this... And
    it should not be a warning. It should err on you, much like if you forget
    to *use* an import or declare a variable and don't use it.

    Just to clarify my suggestion with an example:

    func RegisterChannel(ch interface{}) checked error {


    }

    (I use the keyword checked here, but that is not important... it can be
    anything.)

    Using it would be like:

    err := RegisterChannel(myCh)


    Or, to ignore the error (here you are required to use some minor boilerplate to consciously ignore the err):

    _ := RegisterChannel(myCh)


    But the compiler would prevent you from doing just this:

    RegisterChannel(myCh)


    All the best,

    :) Hein
    On Sunday, November 4, 2012 3:15:07 PM UTC+1, Kevin Gillette wrote:

    I think this is an excellent job for a separate tool; when the compiler
    doesn't produce any warnings whatsoever right now, it takes exponentially
    more momentum and justification to add the first warning, compared to if
    you suggested the gcc team add yet another warning to their already long
    list. Beyond that, it doesn't seem like a hard problem to me:

    A separate tool can:

    1) Parse any number of configured or option-passed whitelist/blacklist
    files, of the form:

    allow:
    fmt

    deny:
    fmt Scan*

    This would have a syntax similar to hgignore and gitignore files (using
    globs or regexes -- it doesn't really matter). The above will configure the
    tool to not report any unchecked errors for calls into the fmt package,
    unless it's a fmt.Scan* func being called. There are few enough
    white-listable cases that patterns may not be useful. White/blacklisting
    can happen on the package, type, or func/method level, and could borrow
    godoc's commandline syntax (which would hopefully be extended to handle
    methods).

    When resolving funcs or methods, the most specific applicable rule
    (func/method, then type, then package) takes precedence, and where
    conflicting rules of the same specificity are present, blacklisting takes
    precedence. Default would be to blacklist anything without a rule, as that
    is aligned with the common philosophy to, when in doubt, check every error.

    2) Scan requested source files and find all calls that return an error
    type that is ignored, and for each, unless the most applicable rule is to
    allow the error to go unchecked, report that file/line to stdout.

    This design doesn't account for other potentially pernicious cases, like
    ignoring the isPrefix return value from `bufio *Reader ReadLine` (which in
    Go syntax would be `(*bufio.Reader).ReadLine`, I think.
    --
  • Kamil Kisiel at Nov 4, 2012 at 6:05 pm
    What would be the criteria for deciding if an error needs to be checked or
    not? Everyone's opinion on what should be a checked error will differ and
    there would be no consistency between different bodies of code. You
    wouldn't be gaining very much for the expense of complicating the language.

    I think this functionality is much better suited for a tool like go vet and
    configurable with a whitelist.
    On Sunday, November 4, 2012 9:32:13 AM UTC-8, Hein Meling wrote:

    Well, I'd much rather have the standard compiler help us with this... And
    it should not be a warning. It should err on you, much like if you forget
    to *use* an import or declare a variable and don't use it.

    Just to clarify my suggestion with an example:

    func RegisterChannel(ch interface{}) checked error {


    }

    (I use the keyword checked here, but that is not important... it can be
    anything.)

    Using it would be like:

    err := RegisterChannel(myCh)


    Or, to ignore the error (here you are required to use some minor boilerplate to consciously ignore the err):

    _ := RegisterChannel(myCh)


    But the compiler would prevent you from doing just this:

    RegisterChannel(myCh)


    All the best,

    :) Hein
    On Sunday, November 4, 2012 3:15:07 PM UTC+1, Kevin Gillette wrote:

    I think this is an excellent job for a separate tool; when the compiler
    doesn't produce any warnings whatsoever right now, it takes exponentially
    more momentum and justification to add the first warning, compared to if
    you suggested the gcc team add yet another warning to their already long
    list. Beyond that, it doesn't seem like a hard problem to me:

    A separate tool can:

    1) Parse any number of configured or option-passed whitelist/blacklist
    files, of the form:

    allow:
    fmt

    deny:
    fmt Scan*

    This would have a syntax similar to hgignore and gitignore files (using
    globs or regexes -- it doesn't really matter). The above will configure the
    tool to not report any unchecked errors for calls into the fmt package,
    unless it's a fmt.Scan* func being called. There are few enough
    white-listable cases that patterns may not be useful. White/blacklisting
    can happen on the package, type, or func/method level, and could borrow
    godoc's commandline syntax (which would hopefully be extended to handle
    methods).

    When resolving funcs or methods, the most specific applicable rule
    (func/method, then type, then package) takes precedence, and where
    conflicting rules of the same specificity are present, blacklisting takes
    precedence. Default would be to blacklist anything without a rule, as that
    is aligned with the common philosophy to, when in doubt, check every error.

    2) Scan requested source files and find all calls that return an error
    type that is ignored, and for each, unless the most applicable rule is to
    allow the error to go unchecked, report that file/line to stdout.

    This design doesn't account for other potentially pernicious cases, like
    ignoring the isPrefix return value from `bufio *Reader ReadLine` (which in
    Go syntax would be `(*bufio.Reader).ReadLine`, I think.
    --

Related Discussions

Discussion Navigation
viewthread | post
Discussion Overview
groupgolang-nuts @
categoriesgo
postedNov 4, '12 at 12:29a
activeNov 4, '12 at 6:05p
posts6
users4
websitegolang.org

People

Translate

site design / logo © 2022 Grokbase