Hacker News Reader

Comment by xiphias2 | Hacker News Reader

I’m not sure how it’s done with the Linux Kernel (and git definately makes it hard), but at Google (using Perfoce model) there are tools for mass renaming that try to guarantee that there are no regressions. Of course in Linux the amount of C macros make it much harder to work on the syntax trees, but comparision after the preprocessor step should be possible.

View on HN

The point is that the change is not a simple rename:

    struct foo *iterator;

    list_for_each_entry(iterator, &foo_list, list) {
     do_something_with(iterator);
    }

While the new version would be something like:

    list_for_each_entry_v2(iterator, &foo_list, list) {
     do_something_with(iterator);
    }

So the ’iterator’ variable may be removed, but as this is c, might also be reused multiple times, or come from some struct member, union, or whatever. So i presume something like this would have to be fixed case by case.

You can do these kinds of changes with clang-tidy, if necessary. Clang-tidy is extensible and you can write your own rules for transforming source code.

Mind you, there are not a ton of people around who have the time to learn how to write custom clang-tidy rules. It might be done for 15,000 changes that potentially fix kernel vulnerabilities, though.

Getting code analysis/refactoring tools to understand huge, complicated projects like the Linux kernel is far from trivial.

In practice it is not that hard. The refactoring tool may only need to tackle one translation unit at a time. You create a pattern which matches some usage of a macro, and create some logic to write out some changes to the AST. It can involve a surprisingly short amount of code—because clang-tidy has good tools for writing patterns and modifying ASTs!

You can brows the clang-tidy source code yourself. There are existing checks specific to the Linux kernel in there already. Well, there’s one check. But if you use clang-tidy, you’ll discover that automatic refactoring of extremely large code bases is within reach.

https://github.com/llvm/llvm-project/tree/main/clang-tools-e...

The only question is whether you would want to spend the staff hours working on a clang-tidy check. Large code bases are exactly where the tradeoff makes sense.

Kernel developers have previously used a tool called coccinelle to do these kinds of mechanical mass changes. It's pretty nifty.

Unless something has changed in the last month, clang-tidy is not extensible. You can't write your own rules without forking the whole project.

I’ve worked at a couple companies that had custom clang-tidy checks. This is well-documented enough at this point, but it is still a bit arcane.

http://bbannier.github.io/blog/2015/05/02/Writing-a-basic-cl...

Yes, it involves forking. Forking isn’t that big a deal. We even had checks that were specific to internal libraries.

At the start forking is like no deal at all. But how do you get new clang tidy features from the main branch later into your fork; I think this is the only instance which I would really call technical debt.

“Git pull” works pretty well to bring new clang-tidy features downstream. If you write a custom check, it goes in its own set of files, so there will be no merge conflicts. The only thing that would break is when the internal clang-tidy API changes, but clang-tidy checks tend to be fairly short.

Really… I have worked at two companies that had extended clang-tidy to add custom checks for their code base. The whole point of clang-tidy is to automate code fixes in other parts of your code base. When done well, use of clang-tide pays down technical debt faster rather than adding to it.

Forking to add new checks only causes problems with pulling from upstream when APIs you are using are changed upstream, which happens rarely. Forking to _modify_ existing checks would cause you a lot of pain, but the solution there is to copy the existing check instead.

You can also write your own standalone LibTooling [0] app, which accomplishes something similar [1].

[0] https://clang.llvm.org/docs/LibTooling.html

[1] https://clang.llvm.org/docs/LibASTMatchersTutorial.html

IIRC Google works the same way. The automated refactoring tools (Rosie?) may make it go faster, but you generally don't fix up 15,000 call sites in a single change... you break it up into smaller batches spread across the tree.

It's been a long time since I was there, but I thought there were plenty of 15,000 call site refactorings done in a single final CL. Not that the Linux kernel should do the same!

When I was there (several years ago) it was rare to do that across many projects. You don't want to have to roll back everyone due to a problem that affects one project. Also, there's a risk that changes that are happening in the meantime might mean the patch doesn't apply.

For a global change? These days that 'risk' is more of a certainty.

It may take a lot of time to get all the approvals from all the code owners. One of the big advantages of a tool like Rosie is precisely that it splits up the update into smaller CLs and has automation that nags all necessary reviewers and reruns the transformation once head moves.

What does git or Perforce have to do with refactoring? Why would whatever version control you're using matter?

> tools for mass renaming that try to guarantee that there are no regressions

beyond sed or whatever a fancy IDE already does? What kind of voodoo magic are we talking about here? Did Google solve the halting problem and I just haven't noticed yet...

Disclaimer: I haven't worked at Google, but you can easily read their papers like https://dl.acm.org/doi/pdf/10.1145/2854146 and understand what's going on. Refactoring tooling is integrated with the revision control system because in a giant codebase with thousands of contributors, you need to split a refactoring patch into smaller chunks and ideally use semantic rather than text-based diff tooling, or the changes will never get reviewed or merged before they conflict with more changes.

As for the actual renaming, yes, definitely beyond what sed does but probably around what the fanciest IDE does. Imagine a big global symbol dependency graph produced by the entire build toolchain all at once and cached, I think?

Also, this isn't the halting problem unless your codebase allows for fully dynamic invocation :)

Aren't all function pointers dynamic invocation ? Or is "fully" one of those words that i'm not grasping in context.

> Aren't all function pointers dynamic invocation?

Given that compilers can be smart-enough to detect "non-dynamic" function-pointer invocation (e.g. when an execution-trace proves that a function-pointer parameter always points to the same function address), it's not safe to say that "all" function pointer invocations are dynamic.

Another case to consider is when one implements (Smalltalk-style) OOP with message-passing: in many cases it's possible to build that without needing to use function-pointers at all.

Also you have a type system. And even in C function pointers have types.

So you can exclude many things that would in-principle be possible.

> C function pointers have types.

I'm not 100% sure, but i remember that i could have have both a two and three function argument be called with the same function pointer, its probably UB however i think that as long as any dereferenced functions adhered to the standard argument behavior of the platform specific calling convention it worked, and i dont think gcc, clang or msvc complained, but my memory might be wrong.

The rename has to be atomic, which is one of the things your revision control system gives you, unless it's like CVS or something worse†. So, that's a necessary part of the solution space.

You want there to be two states: State A where the thing was called old_name and State B where the thing is called new_name. State AB where some code thinks it is called old_name but other code thinks it is called new_name is broken, and must not exist.

† This might seem outrageous to modern programmers, but CVS thinks in terms of files, so from its point of view it's fine if out of sixty files you tried to commit, 48 of them succeeded and 12 failed. Good luck fixing the resulting mess.

I remember having to go backwards from SVN to CVS for a project a long time ago, and I was so shaken by the fact that CVS couldn’t do atomic commits with multiple files!

Yeah, my favorite model of how git operates is "rewind all the way back to RCS, add atomic commits and take it from there".

SVN, baz/bzr and friends are what you get if you add networking before atomic commits. Git is what you get if you start with the proper data model and only then add networking.

I believe the commenter is referring to system where you take a giant “git sed” and automate a rather complicated process: 1. break up one huge diff into a set of many (hundreds or thousands) patches, where each individual patch only touches files in a particular subsystem. 2. send all those patches to the appropriate system owners and run the appropriate tests. 3. manage the actual merging of successful patches, as well as communicating to the original author any individual patches that might need more attention e.g. based on patch review feedback.

as you might imagine, it’s probably a pretty involved process to touch thousands of lines of code in a complicated system.

There are multiple branches being actively worked on. It would be very easy for someone to murge in an unrefacatored branch and now half the code base is using the old call.

You'd want something like https://bors.tech/ to avoid that.

Bors works for git. Google has similar tooling for their system. (I think it's called the 'train'.)

Linux doesn't even have consistent testing let alone the kind of tooling or will that Google does.

The Linux kernel developers have pretty good tooling for refactoring. For example, they have Coccinelle and Sparse.

1. https://en.wikipedia.org/wiki/Coccinelle_(software) 2. https://en.wikipedia.org/wiki/Sparse

Linux definitely has testing. Some helpful person added our repo base to the kernel-test-robot list, and now I regularly get e-mails when something I'm working on in some random WIP branch broke the build on some other architecture (often in COMPILE_TEST mode), or some other random driver. Didn't even have to do anything myself. It's great for avoiding "this broke the build on $obscure_config" issues.

I don't see how Perforce makes this easier than Git?

Reply on news.ycombinator.com