6 comments

  • vismit2000 24 minutes ago
  • ansgri 2 hours ago
    Good illustration that a seemingly simple feature could require a ton of functionality under the hood. Would be nice to have this in Python.
  • shoo 3 hours ago
    If I follow, this isn't a compile time inline directive, it's a `go fix` time source transformation of client code calling the annotated function.

    Per the post, it sounds like this is most effective in closed-ecosystem internal monorepo-like contexts where an organisation has control over every instance of client code & can `go fix` all of the call sites to completely eradicate all usage of a deprecated APIs:

    > For many years now, our Google colleagues on the teams supporting Java, Kotlin, and C++ have been using source-level inliner tools like this. To date, these tools have eliminated millions of calls to deprecated functions in Google’s code base. Users simply add the directives, and wait. During the night, robots quietly prepare, test, and submit batches of code changes across a monorepo of billions of lines of code. If all goes well, by the morning the old code is no longer in use and can be safely deleted. Go’s inliner is a relative newcomer, but it has already been used to prepare more than 18,000 changelists to Google’s monorepo.

    It could still have some incremental benefit for public APIs where client code is not under centralised control, but would not allow deprecated APIs to be removed without breakage.

    • RossBencina 16 minutes ago
      I'm not sure what all of the hazards are, but I could imagine a language (or a policy) where public APIs ship with all of the inline fix directives packaged as robust transactions (some kind of "API-version usage diffs"). When the client pulls the new API version they are required to run the update transaction against their usage as part of the validation process. The catch being that this will only work if the fix is entirely semantically equivalent, which is sometimes hard to guarantee. The benefits would be huge in terms of allowing projects to refine APIs and fix bad design decisions early rather than waiting or never fixing things "because too many people already depend on the current interface".
    • avabuildsdata 2 hours ago
      yeah this is the part that got me excited honestly. we're not google-scale by any stretch but we have ~8 internal Go modules and deprecating old helper functions is always this awkward dance of "please update your imports" in slack for weeks. even if it doesn't let you delete the function immediately for external consumers, having the tooling nudge internal callers toward the replacement automatically is huge. way better than grep + manual PRs
      • shoo 2 hours ago
        it could be better than a nudge -- if you could get a mandatory `go fix` call into internal teams' CI pipelines that either fixes in place (perhaps risky) or fails the build if code isn't already identical to fixed code.
  • omoikane 3 hours ago
    I wonder why they chose to add these directives as comments as opposed to adding new syntax for them. It feels like a kludge.

    https://wiki.c2.com/?HotComments

    • kjksf 3 hours ago
      Go designers distinguish between Go language as defined by Go spec and implementation details.

      //go:fix is something understood by a particular implementation of Go. Another implementation could implement Go without implementing support for //go:fix and it would be a fully compliant implementation of Go, the language.

      If they made it part of the syntax, that would require other implementations to implement it.

      • dwattttt 2 hours ago
        If the comments impact correctness (which inlining doesn't, but I believe there are other directives that do), saying it's "an implementation detail" waves away "it's an implementation detail that everyone needs" aka part of the spec.

        The reason it feels like a kludge is that "comments" are normally understood to be non-impactful. Is a source transformation that removes all comments valid? If comments have no impact per the spec, yes. But that's not the case here.

        In practice comments in go are defined to be able to carry semantic meaning extensibly. Whether they're safe to ignore depends on what meaning is given to the directives, e.g. conditional compilation directives.

        • tptacek 2 hours ago
          There's nothing unique to Go about this kind of tooling. It exists in C, Java, Rust, Typescript, and probably dozens of other settings as well. It's the standard way of implementing "after-market" opt-in directives.
          • dwattttt 1 hour ago
            Are we referring to 'go fix' as after market tooling?

            It's certainly done in many places. JsDoc is the biggest example I can think of. But they're all walking the line of "this doesn't have an impact, except when it does".

            It being done by the language owners just makes them the ones walking the line.

            • tptacek 1 hour ago
              That's exactly how this works: it doesn't have an impact, except when you ask it to. This is an idiomatic approach to this problem.
              • dwattttt 1 hour ago
                The part I object to is overloading comments, which aren't meant to be load bearing. A tool developed outside the language has no choice but to take something that has no meaning and overload it, but language owners weren't forced to take this route; they could've made directives not comments.

                In practice, the Go language developers carved syntax out of comments, so that a comment is "anything that starts with //, unless the next characters are go:"

                • YesThatTom2 22 minutes ago
                  So how many angels can you fit on the head of a pin?
          • omoikane 1 hour ago
            In the listed examples, the compiler will emit a diagnostic upon encountering those comments:

            https://go.dev/blog/inliner#example-fixing-api-design-flaws

            So these comments carry more weight than how those comment annotations might be consumed by optional tools for other languages.

            For most of the listed examples, I think the corresponding C annotation would have been "[[deprecated]]", which has been added to the syntax as of C23.

          • ternaryoperator 1 hour ago
            It does not exist in Java. Comments in Java do not change code.
            • joshuamorton 11 minutes ago
              This also does not change th code. It is an advertisement to a linter-loke tool to take some action on the source code. Its most similar to linter directives which usually are comments.
          • Patryk27 1 hour ago
            There are no comment-based directives in Rust, are there?
            • win311fwg 6 minutes ago
              It provides the feature to use.

              It’s possible nobody has yet.

            • tptacek 1 hour ago
              Eh, you're right, they have a structured attribute system.
        • joshuamorton 2 hours ago
          > The reason it feels like a kludge is that "comments" are normally understood to be non-impactful. Is a source transformation that removes all comments valid? If comments have no impact per the spec, yes. But that's not the case here.

          This is not inlining in the compiler. It's a directive to a source transformation (refactoring) tool. So yes, this has no impact on the code. It will do things if you run `go fix` on your codebase, otherwise it won't.

          • dwattttt 2 hours ago
            And yet it still breaks "comments aren't semantic". That transformation I described is still invalid.
            • pastel8739 33 minutes ago
              I don’t understand why that wouldn’t be valid. As far as I understand if you compile code with these go:fix comments, they will be ignored. But if instead of compiling the code you run ‘go fix’, the source code will be modified to inline the function call. Only after the source code has been modified in this way would compiling reflect the inlining. Do you have a different understanding?
              • dwattttt 11 minutes ago
                I mean that directives other than inlining impact correctness. If you have a source file that only builds for one OS, stripping the build tag will break your build.
      • bheadmaster 3 hours ago
        That's such an elegant solution.

        I keep being impressed at subtle but meaningful things that Go does right.

    • Groxx 1 hour ago
      Because these are instructions for users for making tool-assisted changes to their source code, not a behavior that exists at runtime (or even compile time). A new syntax wouldn't make sense for it.

      For other things, like `//go:noinline`, this is fair criticism. `//go:fix inline` is quite different in every way.

    • 0x696C6961 3 hours ago
      The //go:xyz comments are an established pattern in the Go tooling.
      • Mond_ 3 hours ago
        This is begging the question. Yes, but why did they do that over dedicated syntax?

        (My personal theory is that early go had a somewhat misguided idea of simplicity, and preferred overloading existing concepts with special cases over introducing new keywords. Capitalization for visibility is another example of that.)

        • thwarted 2 hours ago
          //go:xyz is dedicated syntax that is compatible with both the language spec and other toolchains that don't know about it.
          • Mond_ 1 hour ago
            It's an overloaded comment. I am personally quite fine with it, I don't think it's bad. but it is an overloaded comment.
            • thwarted 1 hour ago
              I'm no longer sure what you're saying. You asked why they didn't go with dedicated syntax, I listed two advantageous aspects of the chosen syntax. We know it's an overloaded comment: that's literally one of the advantages.
  • tapirl 3 hours ago
    It looks the following code will be rewritten badly, but no ways to avoid it? If this is true, maybe the blog article should mention this.

        package main
        
        //go:fix inline
        func handle() {
            recover()
        }
        
        func foo() {
            handle()
        }
        
        func main() {
            defer foo()
            panic("bye")
        }
    • arjvik 1 hour ago
      recover()'s semantics make it so that "pointless" use like this can be inlined in a way that changes its semantics, but "correct" use remains unchanged.

      Yes, maybe some code uses recover() to check if its being called as a panic handler, and perhaps `go fix` should add a check for this ("error: function to be inlined calls recover()"), but this isn't a particularly common footgun.

    • tapirl 2 hours ago
      Another example (fixable):

          package main
      
          import "unsafe"
      
          //go:fix inline
          func foo[T any]() {
              var t T
              _ = 1 / unsafe.Sizeof(t)
          }
      
          func main() {
              foo[struct{}]()
          }
      
      Go is a language full of details: https://go101.org/details-and-tips/101.html
      • tapirl 14 minutes ago
        similar:

            package main
        
            //go:fix inline
            func foo[T [8]byte | [4]uint16]() {
                var v T
                var n byte = 1 << len(v) >> len(v)
             if n == 0 {
                 println("T is [8]byte")
             } else {
                 println("T is [4]uint16]")
             }
            }
        
            func main() {
                foo[[8]byte]()
            }
    • shoo 3 hours ago
      Great example, illustrating go1.26.1 go fix source inline transformation breaking program semantics. Raise it as a bug against go fix?
      • tapirl 3 hours ago
        As I have mentioned, no ways to fix it. Because it is hard to know whether or not the handle function is called in a deferred call.
    • arccy 3 hours ago
      Or: your buggy code is no longer buggy.
      • tapirl 3 hours ago
        You claim listens right for this specified example. :D

        It is just a demo.

  • measurablefunc 4 hours ago