А что, если…

Эдсгер Дейкстра писал:

На экономический вопрос «Почему программное обеспечение такое дорогое» столь же экономическим ответом был бы такой: «Потому что его пытаются получить при помощи дешёвого труда». А почему пытаются? Да потому, что присущие ему трудности повсеместно сильно недооцениваются.

Если продолжить эту мысль, то использование GenAI – очередная попытка создавать проекты максимально дёшево, игнорируя вытекающие из этого проблемы. Навскидку:
• больше кода, меньше контроля со стороны человека, проблемы безопасности и качества;
• непонятно, как будут появляться новые эксперты, которые могут понять сгенерированный код (проблема обучения);
• некому найти исправить сложные ошибки – бесконечная перегенерация фрагментов проекта в надежде, что ошибки исчезнут;
• новые модели обучаются на плохом коде предыдущих моделей (сингулярность говновайбкода);
• и т.д.

А что, если внедрение GenAI в итоге сделает программное обеспечение дороже, а не дешевле?

Давненько не постил про обзоры C++ проектов с помощью статического анализа. Исправляюсь - Нарезка кода: что скрывает OrcaSlicer.

Как все так ловко ИИ пользуются... Обзоры кода делают...

Не знаю, то ли я туповат, то ли невезучий, то ли кругом сказочники. Пытаюсь исследовать, какие новые интересные виды ошибок может находить ИИ в С++ коде, о которых не знает PVS-Studio. Claude Opus то нормально мне про интересные баги рассказывает, то на ровном месте с ума сходит.

Даю файл на вход, а он тревожно:

В присланном файле есть места, где содержимое обрезано/повреждено вставкой `</context>`, поэтому их я не рассматриваю как «баги кода» — это артефакты форматирования. Разберу реальные логические ошибки.

Шо? Какой context? Там максимум — #include "context.hxx". Ну ладно, давай ошибки посмотрим.

Так, парочка сообщений по делу. Например, найден лишний аргумент при использовании функции форматирования.

addErrorMessage(&sciErr, API_ERROR_CREATE_ZDOUBLE_IN_NAMED_LIST,
    _("%s: Unable to create list item #%d in Scilab memory"),
    "createComplexZMatrixOfDoubleInNamedList", _iItemPos + 1, _pstName);

В строке форматирования два спецификатора %s и %d, а передаются три аргумента: funcName, _iItemPos+1, _pstName. Да, это баг. Про подобные паттерны PVS-Studio уже знает, хотя, возможно, нужно подсказать, что addErrorMessage — это printf-подобная функция. В целом, тут неинтересно, идём дальше.

Ооо, и тут начинается...

`checkListItemPosition` — неправильное сравнение границы:

// check a valid item
if (_iItemPos > iItem)
{
    addErrorMessage(...);
    return sciErr;
}

Здесь проверка «позиция валидна» использует `>`, хотя позиции 1-based, а `iItem` — количество элементов. Это выглядит как граничный дефект, но по сути `_iItemPos == iItem` допустимо, `_iItemPos > iItem` — ошибка. Это в порядке — правильная граница. Не баг. (Указываю, чтобы было понятно, что я это проверил.)

Так баг или не баг? Я три раза прочитал текст и посмотрел функцию с кодом. В итоге я не только не понимаю, есть ли здесь ошибка, но и не понимаю, что Claude пытается мне сказать.

Дальше ещё хуже шарики за ролики зашли.

Баг 5 (реальный): утечка в readCommonMatrixOfPolyInNamedList и др. — нет, это API отдаёт указатели наружу. Не баг.

Это всё описание пятого бага, больше ничего нет. Комментировать — только портить.

Я не понимаю, как на всём этом можно строить хоть сколько-то надёжные процессы автоматических обзоров кода, про которые в последнее время из разных утюгов вещают.

На сегодня мне ИИ достаточно. Пойду лучше пройдусь по парку, пока погода хорошая.

69 поводов, когда IT-нику хочется что-то отметить
Наверняка многие не в курсе, что у нас есть вот такая подборка – День, когда Скайнет обрёл сознание, и ещё 69 поводов собраться с коллегами! Часть 1, Часть 2.

Короче и быстрей (часть №1 из 5) – Второй цикл

Попалась функция на языке С++. На её примере прям просится показать, что, делая рефакторинг, можно не только эстетично сократить код, но и оптимизировать его. Давайте разомнём мозги, они нам ещё пригодятся, несмотря на эпоху вайб-кодинга. Кто-то ведь должен понимать, как делать надо, а как не надо.

Приведённый ниже код я встретил в вайб-код проекте VibeTensor. Я исследую подобные проекты в качестве натуралиста. Мне интересен генезис новых видов дефектов и недостатков в коде.

Одно из наблюдений – генерированный код более "пухлый", что затрудняет его восприятие человеком и оптимизацию компиляторами. Следующий фрагмент кода как раз это хорошо демонстрирует.

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  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;
  }
  ....
}

С одной стороны, размер и скорость этого кода некритичны, так как он относится к тестам.

Однако этот код размножен по 9 файлам. Оставим за скобками, что такого по-хорошему вообще быть не должно. Но раз код размножается почкованием, желательно чтобы он был бы тогда по возможности компактным.

А ещё важно, что генерируется С++ код, и он обязан быть оптимальным. Само назначение языка – высокоэффективные приложения. В рассматриваемом месте – это просто медленный тест. Но в другом месте что-то подобное приведёт к существенному замедлению приложения. Или скажется кумулятивный эффект множества неудачных фрагментов сгенерированного кода.

Медленный С++ код – это противоестественно. Поэтому по-прежнему полезно развивать свою экспертность в понимании, удачным ли получился код и как его можно улучшить, сократить, оптимизировать. Или сгенерировать снова, используя уточнения.

В общем давайте потренируемся и проведём рефакторинг. Начнём с этого фрагмента:

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;
}

Здесь перемножаются все элементы массива. Если встретится 0, то цикл прервётся, чтобы зря не обрабатывать оставшиеся элементы массива. Всё равно ведь ноль получится.

Чтобы обнулить переменную, где хранится произведение, используется флаг any_zero. Этот подход избыточен, можно проще.

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

Когда встретится 0, обнулятся ne и цикл завершится. Можно продолжить упрощение. Современные процессоры быстро выполняют операции умножения. Поэтому можно в начале перемножить, а потом уже проверить.

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

Функциональность кода не изменилась, но он стал короче и, на мой взгляд, даже понятнее.

Короче и быстрей (часть №2 из 5) – Первый цикл

Теперь вернёмся к началу функции.

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  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);
  }

Из-за static_cast код выглядит тяжеловесным. Первое приведение типа нужно, чтобы какие-то компиляторы/статические анализаторы не ругались на странные арифметические игры.

std::ptrdiff_t i = static_cast<std::ptrdiff_t>(nd) - 1;

Рассмотрим, что будет, если убрать static_cast, а входной массив окажется пустым:

1. Если массив пуст, то nd = 0;
2. Вычтя единицу из беззнакового нуля, мы получим SIZE_MAX, т.е. очень большое положительное беззнаковое число.
3. Значение SIZE_MAX типа size_t неявно преобразуется в тип ptrdiff_t и записывается в переменную i. Получается, что i = -1, как и было задумано. Но вот тут как раз могут быть выданы предупреждения. Ведь мы инициализируем ptrdiff_t числом, которое больше диапазона максимально вмещаемого числа. Поведение в такой ситуации до C++20 определяется реализацией – implementation defined behavior. После C++20 поведение определено.

Итого: первый static_cast лучше оставить на месте. Про остальные такого сказать нельзя. В них нет никакого смысла.

Оператор [] в классе vector принимает аргумент типа size_type (этот тип является синонимом size_t). Значение переменной i автоматически будет преобразовано в size_t, и в этом нет чего-то странного, опасного или подозрительного. Явное приведение типов только загромождает код, и от него лучше избавиться.

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

Первый шаг сделан. Можно теперь всё-таки упростить длинную строку с циклом? Давайте подумаем. Хочется написать как-то так:

for (auto sz : std::ranges::views::reverse(sizes)) {
  strides[???? i ????] = acc;
  acc *= (sz == 0 ? 1 : sz);
}

Всё равно требуется переменная i для обхода массива strides, начиная с конца. Так что совсем упростить код и избавиться от i не получается. Поэтому сделаем так:

int64_t acc = 1;
size_t i = nd;
for (auto sz : std::ranges::views::reverse(sizes)) {
  strides[--i] = acc;
  acc *= (sz == 0 ? 1 : sz);
}

Если честно, мне не нравится, что из-за использования --i код стал сложнее. Теперь требуется вникнуть, почему переменная в начале уменьшается, а уже затем используется для обращения к элементу массива.

С другой стороны, кода стало меньше и его можно быстрее просмотреть глазами. Так что, наверное, когнитивная сложность кода осталась в итоге такой же. Т.е. понимать код стало не проще, но и не сложнее. В любом случае код стал покороче, так что ok.

Короче и быстрей (часть №3 из 5) – Объединяем циклы

Взглянем целиком на вариант кода, который получился к текущему моменту.

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  const std::size_t nd = sizes.size();
  std::vector<int64_t> strides(nd, 0);
  int64_t acc = 1;
  size_t i = nd;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--i] = acc;
    acc *= (sz == 0 ? 1 : sz);
  }
   int64_t ne = 1;
 
  for (auto s : sizes) {
    ne *= s;
    if (ns == 0) {
      break;
    }
  }
  ....
}

Теперь, когда кода меньше, становится очевидным, что второй цикл избыточен. В первом цикле мы перебираем все элементы. Так почему бы их сразу не перемножить?

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  const std::size_t nd = sizes.size();
  std::vector<int64_t> strides(nd, 0);
  int64_t acc = 1;
  size_t i = nd;
  int64_t ne = 1;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--i] = acc;
    acc *= (sz == 0 ? 1 : sz);
    ne *= sz;
  }
  ....
}

Красота. Мы перемножаем все элементы, несмотря на то, что один из них может оказаться нулевым? Нестрашно. Микропроцессоры сейчас быстро умножают. Можно потерять больше на повторном доступе ко всем элементам во втором цикле.

Что ещё осталось? Не требуется изначально обнулять контейнер strides нулями. Всё равно все его элементы будут перезаписаны.

std::vector<int64_t> strides(nd, 0); // надо убрать второй аргумент

В принципе, мы закончили. Но можно сделать ещё одно косметическое изменение, избавившись от переменной nd. Она ни здесь, ни в последующем коде не нужна. Итоговый код:

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  auto q = sizes.size();
  std::vector<int64_t> strides(q);
  int64_t acc = 1;
  int64_t ne = 1;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--q] = acc;
    acc *= (sz == 0 ? 1 : sz);
    ne *= sz;
  }
  ....
}

Код сократился в два раза: с 20 до 9 строк!

P.S. Если захотите и напишите комментарий, я приведу ассемблерный код, чтобы показать как он сократился и оптимизировался.

Короче и быстрей (часть №3 из 5) – Объединяем циклы - Заглянем в ассемблерный код

Текст функции стал короче, но скажется ли это положительно на производительности? Да. Посмотрим на основной фрагмент ассемблерного кода для изначального варианта функции. Используется Clang с ключом -O2.

.LBB1_7:
        mov     rax, rsi
        mov     qword ptr [rbx + 8*r12 - 16], rsi
        mov     rsi, qword ptr [r13 + 8*r12 - 16]
        cmp     rsi, 1
        adc     rsi, 0
        imul    rsi, rax
        dec     r12
        cmp     r12, 1
        ja      .LBB1_7
        mov     r12d, 1
        cmp     rbp, r13
        je      .LBB1_9
.LBB1_4:
        mov     rax, qword ptr [r13]
        test    rax, rax
        je      .LBB1_5
        imul    r12, rax
        add     r13, 8
        cmp     r13, rbp
        jne     .LBB1_4
        jmp     .LBB1_9

Достаточно многословный ассемблерный код с двумя циклами. Теперь посмотрим, что сгенерировано для последнего сокращённого кода.

.LBB1_10:
        mov     rcx, rsi
        mov     rdx, qword ptr [rbp - 8]
        add     rbp, -8
        mov     qword ptr [rax], rsi
        cmp     rdx, 1
        mov     rsi, rdx
        adc     rsi, 0
        imul    rsi, rcx
        imul    r12, rdx
        add     rax, -8
        cmp     rbp, r13
        jne     .LBB1_10

Красивое. Один цикл. Коротко и быстро.

Короче и быстрей (часть №4 из 5) – Альтернативный подход

Есть какой-то другой альтернативный подход? Есть. Не знаю, догадался ли я до него сам или нет. К сожалению, после написанию своего варианта кода, я поспешил поспрашивать, какие варианты могут предложить ИИ.

Варианты от DeepSeek были не лучше, а некоторые даже хуже исходного. Например, он сократил одну из реализаций кода ценой переворачивания массива.

// Разворачиваем strides обратно
std::reverse(strides.begin(), strides.end());

А вот одна из альтернативных реализаций от Claude Opus заслуживает внимания. Он обратил внимание, что можно выполнять не две, а одну последовательность умножений!

Если все элементы входного массива не нулевые, то по завершению цикла acc == ne. А если хотя бы один элемент был нулевой, то в конце можно просто поменять значение ne на ноль. Происходит вновь возвращение к флагу any_zero, но в более умном варианте. Используя эту идею, можно написать следующий код:

static TensorImpl make_contiguous_tensor(const std::vector<int64_t>& sizes) {
  auto q = sizes.size();
  std::vector<int64_t> strides(q);
  int64_t acc = 1;
  bool any_zero = false;
  for (auto sz : std::ranges::views::reverse(sizes)) {
    strides[--q] = acc;
    acc *= (sz == 0 ? 1 : sz);
    any_zero |= sz == 0;
  }
  const int64_t ne = any_zero ? 0 : acc;
  ....
}

Ассемблерный код:

.LBB1_7:
        mov     rdx, rsi
        mov     rsi, qword ptr [r13 - 8]
        add     r13, -8
        mov     qword ptr [rcx], rdx
        test    rsi, rsi
        sete    dil
        cmp     rsi, 1
        adc     rsi, 0
        imul    rsi, rdx
        or      al, dil
        add     rcx, -8
        cmp     r13, rbp
        jne     .LBB1_7
        xor     r13d, r13d
        test    al, 1
        cmove   r13, rsi
        mov     r14, r8

Будет ли код, в котором выполняется в два раза меньше умножений, более быстрым? Моё предсказание – необязательно. Процессор может выполнить два невзаимосвязанных умножения одновременно на разных конвейерах. Я делаю ставку, что мой вариант и вариант от Claude будут работать с практически одинаковой скоростью.

Короче и быстрей (часть №5 из 5) – Замер скорости работы и заключение

Давайте проверим замерами, как оно будет на самом деле в реальности. Пришлось немного заморочиться с тестами. Во-первых, надо было подавать такие данные, чтобы не возникало переполнение знаковых 64-битных переменных, так как это UB и замерам не будет доверия. Во-вторых, для честности, надо чередовать данные с нулём и без. В-третьих, цепочки входных данных должны быть разной длины.

Не утверждаю, что измерения выверены, но общее представление они дают. Скорость работы первого изначального алгоритма взята за единицу отсчёта. Использовал Clang с ключом -O2.

1.      Изначальный вариант c двумя циклами: 1.
2.      Мой вариант одним циклом: 0.88.
3.      Вариант, подсказанный Claude, с одним умножением: 0.92.

Рефакторинг дал ускорение в среднем на 10 %. Я ждал, что будет побольше, в районе 20—30 %. Но 10 % — это тоже хорошо, учитывая, что они достигнуты упрощением, а не усложнением кода.

Заключение

На этом разбор кода и его рефакторинг закончен. Был ли в этом смысл?

С точки зрения улучшения конкретно этого кода – нет. Это сгенерированные тесты. Некритично, что код длиннее и медленнее, чем он может быть.

С образовательной точки зрения – да. Неважно, создаётся код человеком или генерируется с помощью ИИ. Нужны эксперты, которые могут отличить хороший код от плохого. Быстрый он или нет. Безопасный он или нет.

Кому-то достаточно, что код работает, и неважно, что там. В некоторых случаях, например для прототипа, это нормально. Однако те, кто не интересуется архитектурой, оптимизацией, безопасностью, скорее всего, просто не видят и не понимают картину разработки и сопровождения больших программных продуктов. Нас ещё ждут новости о маленьких и больших провалах таких проектов.

ИИ — это мощный инструмент, но не волшебная палочка. Когда Claude подсказал мне альтернативный вариант алгоритма, это хороший пример, что ИИ можно использовать для исследований и усиления возможностей человека. Но точно также он легко нанесёт вред при бездумном использовании.

P.S. Кстати, про волшебную палочку подметил кое-что интересное, завтра напишу.