Bloated C++ code

There's an old joke among programmers that you should never pay them by the line of code, because they'll end up writing long, pointless code and leaning hard on copy-paste. These days that joke writes itself, except now the "programmer" is GenAI, and it actually gets paid per line. Ironic.

One of my hobbies is picking apart generated C++ code, just to get a sense of how the software development industry is evolving: which problems are fading out, and which new ones are cropping up. After reading my post Real-world C++ projects built with GenAI: do they exist? someone suggested I check out the VibeTensor project, so I did.

VibeTensor: System Software for Deep Learning, Fully Generated by AI Agents

I checked it with the PVS-Studio static analyzer and also read through the C++ code with my own eyes. I was curious how many errors a classic code review versus static analysis would each turn up.

Turns out I still can't answer that. The code is bloated to the point of being unreadable, and that bloat is the whole problem. Wading through it felt like slogging through a swamp, and the analyzer got just as bogged down as I did.

Don't get your hopes up for a big error count either. There's barely any code in this project actually worth analyzing. And that's despite the project not being small at all, over 400 C++ files, around 100,000 lines total.

That size is deceptive though. Most of the project isn't really there to be looked at or analyzed. The core issue here is just verbosity. You see it in code snippets that repeat over and over, and in plain pointless busywork that bloats the code for no reason.

In the past, people would've called this copy-paste coding. That's not quite what happened here, but code generation gets you to the same place. Instead of extracting shared logic into functions, new code just gets generated again and again to solve nearly identical problems.

Scroll through the files for a while and you'll start getting déjà vu, as the same blocks of code keep popping up. They're sort of different, sort of not. Here's what I mean:

Like I mentioned in my article C++: Write, shorten, optimize, this exact block of code shows up nine times across different tests:

const std::size_t nd = sizes.size();
std::vector<int64_t> strides(nd, 0);
int64_t acc = 1;
for (std::ptrdiff_t i = static_cast<std::ptrdiff_t>(nd) - 1; i >= 0; --i) {
  strides[static_cast<std::size_t>(i)] = acc;
  const auto sz = sizes[static_cast<std::size_t>(i)];
  acc *= (sz == 0 ? 1 : sz);
}

int64_t ne = 1;
bool any_zero = false;
for (auto s : sizes) {
  if (s == 0) {
    any_zero = true;
    break;
  }
  ne *= s;
}
if (any_zero) {
  ne = 0;
}

But that's just the tip of it. PVS-Studio keeps spitting out warnings about redundant code, one after another. Sometimes it's the small stuff:

for (int i = 0; i < dl.ndim; ++i) {
  int64_t n = (dl.ndim == 0) ? 1 : dl.shape[i];
  int64_t d = n > 0 ? (n - 1) : 0;
  if (d == 0) continue;
  int64_t st = (dl.ndim == 0) ? 1 : strides[static_cast<std::size_t>(i)];

PVS-Studio issues the same warning twice: V547 Expression 'dl.ndim == 0' is always false. And it's right, if the loop runs at all, dl.ndim can't be zero at that point. The code simplifies down to:

for (int i = 0; i < dl.ndim; ++i) {
  int64_t d = std::max(0ll, dl.shape[i] - 1);
  if (d == 0) continue;
  int64_t st = strides[i];

In other spots, you can't call the bloat minor anymore. Right there, PVS-Studio fires off whole groups of warnings at once:

• V547 [CWE-570] Expression 'is_empty' is always false. tensor_bindings.cc 3007
• V547 [CWE-570] Expression 'print_size' is always false. tensor_bindings.cc 3015
• V547 [CWE-571] Expression '!parts.empty()' is always true. tensor_bindings.cc 3023

The code that triggered these warnings seems fairly slick on the surface: an array here, a loop there... Take a closer look, though, and it's garbage.

bool is_empty = false; // handled above; always false here
bool print_size = is_empty && (self.sizes().size() != 1);
bool suppress_dtype_non_empty = (!is_empty) &&
  (self.dtype() == ScalarType::Float32 ||
   self.dtype() == ScalarType::Int64 ||
   self.dtype() == ScalarType::Bool);
bool print_dtype = !suppress_dtype_non_empty;
if (is_empty) {
  // For empty tensors, only print dtype when dtype != default float32
  print_dtype = (self.dtype() != ScalarType::Float32);
}

std::string out = "tensor(";
out += body;
std::vector<std::string> parts;
if (print_size) {
  parts.push_back(std::string("size=") + format_sizes(self.sizes()));
}
if (print_dtype) {
  parts.push_back(std::string("dtype=") + dtype_name(self.dtype()));
}
// Always include device suffix for CUDA tensors
parts.push_back(std::string("device='cuda:") +
                std::to_string((int)self.device().index) + "'");
if (!parts.empty()) {
  out += ", ";
  for (std::size_t i = 0; i < parts.size(); ++i) {
    if (i) out += ", ";
    out += parts[i];
  }
}
out += ")";
return out;

At the very least, that manual loop for building the output string can be swapped out right away for:

return std::format("tensor({})", parts | std::views::join_with(", "sv));

If you take a closer look, you can actually cut all this bloated mess down to a third of its size:

std::string out = "tensor(" + body + ", ";

if (self.dtype() != ScalarType::Float32 &&
    self.dtype() != ScalarType::Int64 &&
    self.dtype() != ScalarType::Bool)
{
  out += std::string("dtype=") + dtype_name(self.dtype()) + ", ";
}
out += "device='cuda:" + std::to_string((int)self.device().index) + "')";
return out;

Here's the interesting part: look at the analyzer's output for the code as it stands, and it seems like there aren't any real errors. The code is complex, but it works correctly, no out-of-bounds array access. Credit where it's due, AI.

But once you look past the surface complexity to what the code is actually doing, you realize there's nowhere left for it to go wrong. It's all the same, just stretched out over more lines. Not much credit left after that.

Bottom line: there aren't really 100,000 lines of C++ code in this project. Move the duplicates into functions, refactor, and I'd guess the code shrinks by a factor of 5. A 20,000-line project isn't worth taking seriously. What you're looking at here isn't errors, just bloated code and analyzer warnings about a pile of always-true and always-false conditions.

Sure, the code is longer, so what? That's the thinking, anyway: it's not meant to be refactored by a person, if you need something different, you just generate a new version.

And if your goal is selling GenAI and you don't actually care what happens to the project down the line, fine, that logic holds. But if you actually need this thing to be maintained as a real project, the actual "cost per line of code" runs a lot higher than it looks.

A few things bloated code actually costs:

1. More lines of code mean a higher cost to generate them.
2. If another agent is the one reviewing your pull requests, you're paying that agent more too.
3. Bloated functions cost more to generate unit tests for.
4. Any AI-assisted code change costs more, since more lines have to be read in and written out.
5. More context means a higher chance of errors when making changes (you might miss a fix in one of a hundred nearly identical spots).
6. If a person actually has to edit the code or hunt down a bug themselves, wading through that pile of redundant entities will make their eyes cross.
7. When code is more complex than it needs to be, the compiler has a harder time optimizing it.
8. It's easier to generate yet another near-duplicate function than to track down and rework the dozens of existing ones just like it.
9. Unnecessary code doesn't just slow people down, it makes it harder for static analyzers to catch errors too.
10. It takes longer to compile.
11. Verbosity raises the odds of running into undefined behavior, or code that just doesn't do what it was supposed to.
12. I'm sure you could keep adding to this list.

By "verbosity" I also mean using words you don't actually understand, because they sound nice. The GenAI behind this code clearly has no idea what noexcept actually means, it just throws it in because it thinks the code looks better that way. And that's exactly why exceptions end up firing in places they never should.

vt_status vt_tensor_iter_binary_cpu_host(const vt_iter_config* cfg,
                                         vt_tensor out_h,
                                         vt_tensor a_h,
                                         vt_tensor b_h,
                                         vt_tensor_iter_loop1d_fn loop,
                                         void* user_ctx) noexcept {

  ....
  if (effective.check_mem_overlap != VT_ITER_OVERLAP_DISABLE &&
      effective.check_mem_overlap != VT_ITER_OVERLAP_ENABLE) {
    throw std::invalid_argument(
        "vt_tensor_iter_binary_cpu: invalid vt_iter_overlap_mode");
    }
  ....
}

And here's the thing: the bloated code problem falls on whoever's using the tool, not the AI vendor selling it. They're not the ones footing the token bill.

So what do you actually do? I don't have a ready-made fix. But at least knowing about it puts you ahead.

I'm leaning more and more toward the idea that PVS-Studio's static analyzer needs to get better at spotting similar code fragments. Do that, and you could close the loop between GenAI and PVS-Studio. Code would only count as done once the analyzer stays quiet on bugs and finds no duplicated functionality either.

This isn't a PVS-Studio roadmap yet, but a picture is starting to take shape, one that shows the new problems out there and how the tool could help tackle them.

Additional links:

1. Let's dig into some vibe code.
2. Let's check vibe code that acts like optimized C++ but is actually a mess.

Comments

[Matthew Faithfull]

I've had this theory for a long time that there are two kinds of good code:

There's the incredibly clever but very short 1-3 liner. Like one of those math tricks for fast sqrt in Quake or that new way of multiplying a vector. There's no denying that those things are good code but part of being good is that they fit on a half screen. You can read them in seconds and no matter how complex what they're achieving is, you can ultimately work it out.

Then there's the really straight forward, long, simple linear code. It may be hundreds of lines but each step follows the last. There's nothing tricky going on, just small functions or small step after small step. Each part trivially easy to understand but it adds up to getting the job done correctly and without waste. You can just read it and while it may be boring, it's obviously right. That's also good code.

The problem is that almost all real world code is a compromise between these two and that compromise destroys what's good about each and leaves you with code that's too complex to just read and still too long to hold in your head. What's worse is it often looks a lot like the long simple code but it's actually hiding complexity and inevitable bugs.

It seems that AI has fallen somewhat into the same hole. Failing to judge the correct level of abstraction for what it's trying to achieve. Failing to break down the problem, not just into a list of parts but into a hierarchy of reusable parts that can themselves be categorised and abstracted in other dimensions. It's maybe trying to 'keep it simple' but without a sense of human scale it fails to understand what simple is. Simple is the hard part that comes after solving the problem.