Excessive nil pointer checks in Go

(konradreiche.com)

50 points | by ingve 3 days ago ago

52 comments

  • diarrhea 9 hours ago ago

    This is the mess a language lands on when it conflates optionality (a semantic concept) with references/pointers (purely a machine concept). In Go, the requirement "need (non-optional) a reference to an object" is simply not expressible. This is a solved problem in other languages, for example `&T` vs. `Option<&T>` in Rust.

    • Animats 9 hours ago ago

      In C++, that distinction supposedly exists. References should never be null, while pointers can be. But there's no enforcement.

          int& ref = *ptr;
      
      ought to generate a panic for a null pointer. But it doesn't. They were so close to getting it right.
      • zarzavat an hour ago ago

        There's nothing particularly special about null pointers: you can also have an invalid non-null pointer, e.g. through pointer arithmetic.

        When you write `int& ref = *ptr;` you are dereferencing the pointer with `*ptr` therefore you have promised that it's valid. The compiler doesn't need to do anything to validate ptr because it already has your assurance.

        It's really no different than if you were to write `printf("%d", *ptr);`. It's only a little weird because in `ref = *ptr;` the compiler doesn't actually emit any instruction for the dereference, but that doesn't mean that the assurance you gave doesn't exist.

        It would indeed be problematic were it to be `int& ref = ptr;` without the dereference but it's not.

      • PoignardAzur 9 hours ago ago

        For the longest time I thought this line would lead to a crash just because it seemed so obvious. So close indeed.

      • FartyMcFarter 8 hours ago ago

        > They were so close to getting it right.

        The philosophy of C++ is to not introduce unnecessary overhead, and to trust the programmer. This design choice is prevalent throughout the language. They were never going to make an exception, especially for something as prevalently used as references.

        There are countless examples of this "no unnecessary overhead and/or trust the programmer" choice:

        - primitive types and standard containers are not thread safe - it's up to the programmer to know this and use them accordingly.

        - std::unique_ptr lets you grab the underlying raw pointer, in which case it's no longer a "unique_ptr". But there are cases in which it's useful to do this (e.g. interfacing with C code), so they let you do it, and trust that you do it in a safe way. They could have made unique_ptr not support this, but then it would be less useful (or force you into copying data unnecessarily to call an API that requires a raw pointer).

        > But there's no enforcement.

        There's no strict enforcement, but it is undefined behaviour, so compilers can randomly choose to act as if it's enforced and simply crash your program or make it act weirdly.

        • VorpalWay 6 hours ago ago

          > primitive types and standard containers are not thread safe - it's up to the programmer to know this and use them accordingly.

          Which (sort of) makes sense: most types should not be used across threads. Having everything use atomics/mutexes under the hood would have significant overhead. However, the problem is that the language doesn't then protect you against using these across threads by mistake, this is one of the things that I really like about Rust.

          Funnily enough, shared_ptr in C++ is thread safe (for the reference count at least), leading to pointless overhead when not used between threads. Rust has both thread safe and non-thread safe versions (Arc and Rc respectively), and it will error if you try to send an Rc to another thread.

          • tialaramex 4 hours ago ago

            mutable XOR alias is also key here

            For a typical type Goose it's fine if two threads can both look at the Goose (via a reference, pointer or whatever), so long as nobody can mutate the Goose. If thread A finds that the Goose is Happy, thread B weighs the Goose and finds it to be Heavy, and thread A again measures the length of the Goose as 860 millimetres this is all fine, it won't matter if (by the vagaries of hardware) the weight is measured before the length or after, there's no difference.

            In Rust this is reflected in &Goose, the immutable reference to a Goose, being Send, ie a thing you can give to other threads. The mutable reference &mut Goose is not Send.

      • maccard 8 hours ago ago

        Im not entirely sure this helps with your point but;

        The contract is that the reference is still non-null, and that the error is dereferencing the pointer. There’s two big problems with defining the behaviour of the deterrence - 0 is a valid memory address on some (ancient) platforms so for better or worse the behaviour is platform dependent.

        The other is that there’s many other ways to have absolute garbage in a pointer that aren’t null.

            int& foo() { 
                int local = 42;
                return local;
            }
        
        Now, a compiler catches this case, but the point is that null isn’t the only invalid state that needs to be checked. Adding a compiler overhead of checking each pointer to every single pointer dereference wouldn’t work.

        Modern codebases ran with static analysis tools will catch these errors (honestly even valgrind will find most if not all of these).

      • 8 hours ago ago
        [deleted]
      • Hizonner 5 hours ago ago

        No, it ought to generate a compilation error unless the compiler can prove that the pointer isn't null.

        ... but that only works if you design properly from day one.

    • throwa356262 9 hours ago ago

      What the article said applies to Rust ref vs ref-option too.

      • tialaramex 8 hours ago ago

        Not really. It's possible to write this mistake but it's pretty obviously a bad idea, I've never seen someone do this and need correcting.

        Edited to expand: Sometimes it feels reasonable to have a construction function which returns Option<Goose> rather than Goose because you might be OK with getting back None, for example if you want to make a NonZeroU8 the function to do that will of course give you back Option<NonZeroU8> because you might give it a zero and that's er... not nonzero. But I've never seen people go oh, OK, I guess i'll scatter all my checks throughout the rest of my software and just pass Option<NonZeroU8> everywhere even though I need a NonZeroU8. Rust's shape encourages them to check once during creation like this article suggests.

    • Groxx 9 hours ago ago

      Don't forget mutability! Go throws that on top too.

    • 5701652400 7 hours ago ago

      it does not resolve the problem.

      you would need to check "is this value optional?" and unpacking everywhere. this is what this article saying.

      you can do unpacking/nil-checks at the root or later when it happened.

      with rust you have 2x more ways to shoot yourself in the foot.

      • bestouff 7 hours ago ago

        You check and unpack once, then the rest of the "positive" codepath can use the reference without fearing null.

        I fail to see how Rust would offer twice as many ways to shoot yourself in the foot ; this is a rather safe and picky language.

        • 5701652400 7 hours ago ago

          true, "non-nil pointers"/references will help here to avoid nil checks.

          also true, if you have optional you still need to unpack it somwhere, and your nil checks become unpacking statements. delayed conditionals and delegation to callsites far from offending code (what author says) is still present.

          and if you also have pointers, then you can do Optional<Pointer>.. and now you have to option unpakcing + nil checks. 2x more problems.

          • tialaramex 5 hours ago ago

            If you have an actual pointer type *mut P then Option<*mut P> might be None or it might be Some(null_pointer) or Some(other_pointer) that's not 2x more problems it's just a representation of a more complicated scenario - we may or may not have a pointer and, if we do have a pointer that might be null. We'd presumably have done this because we need to distinguish those cases.

            If you actually mean Option<NonNull<P>> you should write that, now we're saying this is either a non-null pointer or it's nothing. Often though you want Option<&P> either a reference or nothing, or you actually did mean a raw pointer *mut P and you're going to handle scenarios where it is null or whatever.

            Edited: Fix asterisks

      • K0nserv 6 hours ago ago

        > with rust you have 2x more ways to shoot yourself in the foot.

        The checking isn't how you shoot yourself in the foot, it's the absence of checking. Rust doesn't allow you to forget to check. This entire class of problems just disappears in Rust.

        In this if the code needs a non-null redis client to work you take `RedisClient` not `Option<RedisClient>`.

      • BoardsOfCanada 7 hours ago ago

        Obviously, in his example it would be RateLimiter not Option<RateLimiter>, so no check necessary.

        • guilhas 2 hours ago ago

          I think the author can propagate RateLimiter instead of *RateLimiter, making it exactly the same

          • diarrhea 2 hours ago ago

            No, because RateLimiter is then copied on passing it around (pass by value).

            That is problematic for two reasons: it might be a large type, so copying might be expensive. Second, more likely, it might violate invariants in your domain. For a rate limiter, this might mean accidentally copying around some internal state like a mutex, which then exists n times instead of 1 time, which can represent a problem (e.g. if you want to internally limit whole-app concurrency toward Redis).

            • guilhas 42 minutes ago ago

              No. You can see the code

              Clearly it is not large. Second the child object is a pointer so does not violate anything

              And if if if there were constraints, you can wrap it in another object, and pass that around

        • 5701652400 7 hours ago ago

          you still need to unpack that option somewhere.

          • Someone 7 hours ago ago

            _If_ you start out with an optional, and even then only once in the code path.

    • poly2it 8 hours ago ago

      It's really difficult to view Go as a serious language when fundamental design decisions such as this one have seemingly been glossed over. It's in a precarious spot, on the one hand cushioning the C it wants to resemble, but on the other hand not yielding any capable tools or abstractions which could otherwise be unlocked via the safe architecture. Go developers seem uninterested in language design.

      • the_gipsy 6 hours ago ago

        It's not that it has been glossed over, or was a mistake. It's a tradeoff in favor of simplicity (and compiler / tooling speed).

        It is difficult to view Go as a serious language because it fails to acknowledge these decisions, repeatedly. You can't really trust the language in that sense.

  • Joker_vD 6 hours ago ago

    > You may attempt to address this by pushing the problem up one layer. You now check for nil and return an error to flag the nil dependency as an invalid state.

    > It’s better, but it’s still not correct. Why not? Because we still allowed the invalid state to enter our system. A nil pointer is still being passed to our function, which puts the burden of deciding whether to trust the input on code that should have received a valid value in the first place.

    > The constructor is not where the error happened. The error happens at the initialization site:

    > Once initialization fails, we should handle that error immediately. We should not continue with a nil pointer and force the next, deeper layer to rediscover the outcome. Doing so also removes the need for the rate limiter constructor to return an error in the first place!

    But... surely it'd be better to leave this guard rail of a nil check in the rate limiter constructor, to quickly and accurately detect regressions in the very possible future where you reshuffle the code that constructs your objects?

    > The check belongs at the boundary

    Wait... is the author operating under an assumption that I control (almost) the whole of my codebase, so there is no need to have the boundaries inside of it?

  • Fire-Dragon-DoL 9 hours ago ago

    I could have forgiven nil checks, but nil checks on interfaces elevated nils to a whole new level, which is annoying, but I do get where they were going with this: you should never nil check an interface. After all,an interface could be valid for a nil value.

    There are ways to decently write go and not deal with nil, but as usual, linters defaults makes it impossible and you have to fight with your team before they will understand (we did this at some point and it was a huge improvement).

    Don't use pointers at all, always allocate structs on the stack, pass them by value.

    You pay the copy price, even with large structs, and that's fine. When there are exceptions, be very explicit about the reason: performance must be critical,not just an optimization.

    Don't ever check interfaces for nil, if you need some sort of optional parameter, make a separate function and make it pass an valid object for that interface that's a null object.

    These two did improve things substantially

    • mirekrusin 8 hours ago ago

      This suggestion fails for values that can be null, need to be mutable or need references from multiple places etc – it's not "just performance penalty".

      Go has a problem, "just remember to always do X, never Y" patterns can't be guaranteed across all libraries you use, can't be enforced, can be violated for good reasons, other patterns and as a mistake etc etc.

      Shame because otherwise it's a great language, but some mistakes are just no-go.

      So close indeed.

      They need Go 2 with *T and ?*T - that would be nice language to use.

      • pjmlp 7 hours ago ago

        The best approach is to use other programming languages with more open minded approach to modern type systems, and leave Go to the use cases where there is no alternative due to existing adoption.

        Go 2 will never happen, they will keep incrementing 1.x until end of current computing model.

  • galkk 9 hours ago ago

    I agree with first point “Nil Check on a Dependency” and disagree with 2nd point

    “Nil Check on a Dependency in the Constructor”, at least in the way it is described in article’s example.

    The _parameter_ check in the constructor is the standard practice of testing on perimeter/blundaries. You test your parameters on the public methods (that constructor obviously is), and assume valid state in private methods. And even there I can accept practice of debug build assertions (DCHECK/TCHECK in Google c++ terminology ).

  • usrnm 10 hours ago ago

    Also known as contract programming vs. defensive programming. This argument is very old, is not specific to golang, and I have found myself on both sides at different points in my carreer.

    • Sharlin 8 hours ago ago

      Fortunately we have type systems to encode many contracts at compile time, including stuff like optionality. Certainly no modern language would still repeat Hoare’s "billion dollar mistake"? Right? …Oh.

      • tialaramex 7 hours ago ago

        It's so bad that here we are in the 21st century and there are even still people who insist it wasn't a mistake e.g.: https://www.gingerbill.org/article/2026/01/02/was-it-really-...

        • bvrmn 5 hours ago ago

          It's quite a delusional take from Bill. Wow. Using non-nullable (a sane language default) pointers in Zig is liberating experience. And it's still as low level as in C but instead of ship-and-pray you could state your intention with a type system.

  • nilirl 7 hours ago ago

    Go is a very unique language in that it is the only language designed to make you understand the frustration of online dating.

    First, seduction, and then as it reveals how little it cares about you, eventual disappointment.

  • ceving 7 hours ago ago

    The problem the author overlooks is that `RateLimiter` is public, meaning no one is forced to call the constructor.

  • rzwitserloot 7 hours ago ago

    A good point and the java ecosystem makes similar mistakes. In general any:

    ``` if (x != null && !x.isEmpty()) doAThing(x); ```

    is either:

    [A] Code directly on the boundary between systems; the other system is explicitly documented to treat null and empty as semantically equivalent, which is bad, but given that the mistake lies in a system beyond the control of this programmer, they're working around it. It can exist in this boundary code and nowhere else, or

    [B] Extremely rare, but there is a real semantic difference between the notion 'x is null' and 'x is empty' but this code wants to do the same thing in both semantically separate cases, or

    [C] it's bad code.

    NPEs are better than endless defensive dealings. If code checks for null I'd expect that null has a semantically identifiable meaning, and one that isn't also covered by something else (such as some notion of 'empty', e.g. an empty string or an empty list).

  • bediger4000 3 days ago ago

    This is good advice for humans: they can quantify to decide "too many nil checks" or not. But it's not good for agentic coding, which we're entering the age of. Although agents are the worst they'll ever be right now, they're never going to be great at quantifying too many nil checks. I think we'll have to get used to far more nil checks than even bad programmers put in. But that doesn't matter to agents, they've got infinite attention spans, no cognitive bias and large working memories. Sonn we'll see no nil checks.

  • kstenerud 6 hours ago ago

    I'd really really wished, with all of the history behind us, that golang would have learned from it. All they had to do was make pointers nonnull by default.

    Immutable-by-default would also have been nice. A man can dream...

  • lenkite a day ago ago

    Delegate all `nil` and bad input checks to a validation framework and use it in all your constructor functions.

    • FridgeSeal 9 hours ago ago

      I’ll go you one better: integrate it into your language and have the compiler enforce it for you!

  • turtleyacht 3 days ago ago

    What about wrapping nil in a Maybe or Option type?

    • flowerthoughts 10 hours ago ago

      In this case, the missing piece in Go is the NonNullable hint. That would make it clear that null checks aren't needed, enforceable by the type system, and lintable.

      Option types just forces you to do the check, but doesn't remove the need for it.

      Now that we have generic types, a NonNullable intrinsic type seems doable...

      • ThePhysicist 8 hours ago ago

        It's quite easy to write a generic Maybe struct that performs most of the encapsulation that Rust's Maybe does i.e. allow unwrapping of the inner type through a function or handling the nil case through a switch like statement. I've never seen this in the wild which makes me think people don't care about it too much. And of course it's runtime based so no compile time guarantees, and just to preempt the expected replies I know it's not the same what Rust is capable off and Rust is of course a much much much much better language than Go.

        Personally I do experiment with these things as it makes code more readable, it just seems adoption for generics and what you can do with them is still quite low in the broader community. That said I do not deal with null pointer exceptions much at all, and when I do it's often relatively simply to spot and fix, so for me it's not a large issue.

      • cubefox 9 hours ago ago

        Or a set theoretic type system with union type declarations (foo|null), like in TypeScript.

      • sail0rm00n 10 hours ago ago

        References like C++, maybe?

    • aarjaneiro 10 hours ago ago

      This is more about a hard dependency which causes a function to early exit

  • guilhas 3 hours ago ago

    To avoid propagating the pointer I would change *RateLimiter to RateLimiter

    DecodeRequest can return Request instead of *Request, or error if not valid

    Also I would replace `if userID == "" {` with `if err != nil {`. If an object is not loaded successfully returning error I think is more standard

  • 5 hours ago ago
    [deleted]
  • kissgyorgy 6 hours ago ago

    This is exactly what LLMs are really bad at. They don't have the knowledge (and don't ask for) the invariants of the system and write defensive code at every step of the way, which is not just unnecessary, it's bad because if an unexpected state still get into the system, you will never notice and bad data will flow through and makes everything unpredictable.

  • glove2477 6 hours ago ago

    applies to all languages, actually. Fail fast, handle errors at place, etc. I really hate Java for its runtime exceptions, you really have no idea where and how your code will fail