Andrey2008 20 мар 2019 в 22:17

Вред макросов для C++ кода

6 мин

28K

Блог компании PVS-StudioСовершенный код*C++*C*

Язык C++ открывает обширные возможности для того, чтобы обходиться без макросов. Так давайте попробуем использовать макросы как можно реже!

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

BEGIN_MESSAGE_MAP(efcDialog, EFCDIALOG_PARENT )
  //{{AFX_MSG_MAP(efcDialog)
  ON_WM_CREATE()
  ON_WM_DESTROY()
  //}}AFX_MSG_MAP
END_MESSAGE_MAP()

Существуют такие макросы, да и ладно. Они действительно были созданы для упрощения программирования.

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

Примечание. Этот текст писался как гостевой пост для блога «Simplify C++». Русский вариант статьи решил опубликовать здесь. Собственно, пишу это примечание для того, чтобы избежать вопрос от невнимательных читателей, почему статья не помечена как «перевод» :). А вот, собственно, гостевой пост на английском языке: "Macro Evil in C++ Code".

Первое: код с макросами притягивает к себе баги

Я не знаю, как объяснить причины этого явления с философской точки зрения, но это так. Более того, баги, связанные с макросами, часто очень сложно заметить, проводя code review.

Такие случаи я неоднократно описывал в своих статьях. Например, подмена функции isspace вот таким макросом:

#define isspace(c) ((c)==' ' || (c) == '\t')

Программист, использовавший isspace, полагал, что использует настоящую функцию, которая считает пробельными символами не только пробелы и табы, но также и LF, CR и т.д. В результате получается, что одно из условий всегда истинно и код работает не так, как предполагалось. Эта ошибка из Midnight Commander описана здесь.

Или как вам вот такое сокращение написания функции std::printf?

#define sprintf std::printf

Думаю, читатель догадывается, что это был весьма неудачный макрос. Найден он был, кстати, в проекте StarEngine. Подробнее об этом можно прочитать здесь.

Можно возразить, что в этих ошибках виноваты программисты, а не макросы. Это так. Естественно, в ошибках всегда виноваты программисты :).

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

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

Библиотека ATL предоставляет для конвертации строк такие макросы, как A2W, T2W и так далее. Однако мало кто знает, что эти макросы очень опасно использовать внутри циклов. Внутри макроса происходит вызов функции alloca, которая на каждой итерации цикла будет вновь и вновь выделять память на стеке. Программа может делать вид, что корректно работает. Стоит только программе начать обрабатывать длинные строки или увеличится количество итераций в цикле, так стек может взять и закончиться в самый неожиданный момент. Подробнее про это можно прочитать в этой мини-книге (см. главу «Не вызывайте функцию alloca() внутри циклов»).

Макросы, такие как A2W, прячут зло. Они выглядят, как функции, но, на самом деле, имеют побочные эффекты, которые сложно заметить.

Не могу я пройти и мимо подобных попыток сокращать код с помощью макросов:

void initialize_sanitizer_builtins (void)
{
  ....
  #define DEF_SANITIZER_BUILTIN(ENUM, NAME, TYPE, ATTRS) \
  decl = add_builtin_function ("__builtin_" NAME, TYPE, ENUM, \
             BUILT_IN_NORMAL, NAME, NULL_TREE);  \
  set_call_expr_flags (decl, ATTRS);          \
  set_builtin_decl (ENUM, decl, true);

  #include "sanitizer.def"

  if ((flag_sanitize & SANITIZE_OBJECT_SIZE)
      && !builtin_decl_implicit_p (BUILT_IN_OBJECT_SIZE))
    DEF_SANITIZER_BUILTIN (BUILT_IN_OBJECT_SIZE, "object_size",
         BT_FN_SIZE_CONST_PTR_INT,
         ATTR_PURE_NOTHROW_LEAF_LIST)
  ....
}

Только первая строка макроса относится к оператору if. Остальные строки будут выполняться независимо от условия. Можно сказать, что эта ошибка из мира C, так как она была найдена мною с помощью диагностики V640 внутри компилятора GCC. Код GCC написан в основном на C, а в этом языке без макросов обходиться тяжело. Однако согласитесь, что этот не тот случай. Здесь вполне можно было сделать настоящую функцию.

Второе: усложняется чтение кода

Если вы сталкивались с проектом, который весь пестрит макросами, состоящими из других макросов, то вы понимаете, какой это ад — разбираться в подобном проекте. Если не сталкивались, то поверьте на слово, это грустно. В качестве примера тяжёлого для восприятия кода могу привести уже упомянутый ранее компилятор GCC.

По легенде, компания Apple вложилась в развитие проекта LLVM как альтернативного варианта GCC по причине слишком большой сложности кода GCC из-за этих самых макросов. Где я читал про это, я не помню, поэтому proof-ов не будет.

Третье: писать макросы сложно

Легко написать плохой макрос. Я их повсюду встречаю с соответствующими последствиями. А вот написать хороший и надёжный макрос часто сложнее, чем написать аналогичную функцию.

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

#define MIN(X, Y) (((X) < (Y)) ? (X) : (Y))
m = MIN(ArrayA[i++], ArrayB[j++]);

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

#define MAX(a,b) \
   ({ __typeof__ (a) _a = (a); \
       __typeof__ (b) _b = (b); \
     _a > _b ? _a : _b; })

Только вопрос, а нужно ли нам всё это в C++? Нет, в C++ есть шаблоны и другие способы построить эффективный код. Так почему я продолжаю встречать подобные макросы в C++ программах?

Четвёртое: усложняется отладка

Есть мнение, что отладка — это для слабаков :). Это, конечно, интересно обсудить, но с практической точки зрения отладка полезна и помогает находить ошибки. Макросы усложняют этот процесс и однозначно замедляют поиск ошибок.

Пятое: ложные срабатывания статических анализаторов

Очень многие макросы в силу специфики своего устройства порождают множественные ложные срабатывания статических анализаторов кода. Могу смело утверждать, что большинство ложных срабатывания при проверке C и C++ кода связано именно с макросами.

Беда с макросами в том, что анализаторы просто не могут отличить корректный хитрый код от ошибочного кода. В статье про проверку Chromium описан один из таких макросов.

Что делать?

Давайте не использовать макросы в C++ программах без крайней на то необходимости!

C++ предоставляет богатый инструментарий, такой как шаблонные функции, автоматический вывод типов (auto, decltype), constexpr functions.

Почти всегда вместо макроса можно написать обыкновенную функцию. Часто этого не делают по причине обыкновенной лени. Эта лень вредна, и с ней надо бороться. Небольшое дополнительное время, потраченное на написание полноценной функции, окупится с лихвой. Код будет проще читать и сопровождать. Уменьшится вероятность отстрелить себе ногу, а компиляторы и статические анализаторы будут выдавать меньше ложных срабатываний.

Кто-то может возразить, что код с функцией менее эффективен. Это тоже только «отмазка».

Компиляторы сейчас отлично инлайнят код, даже если вы не написали ключевое слово inline.

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

Поясню на примере. Перед вами классическая ошибка в макросе, который я позаимствовал из кода FreeBSD Kernel.

#define ICB2400_VPOPT_WRITE_SIZE 20

#define  ICB2400_VPINFO_PORT_OFF(chan) \
  (ICB2400_VPINFO_OFF +                \
   sizeof (isp_icb_2400_vpinfo_t) +    \
  (chan * ICB2400_VPOPT_WRITE_SIZE))          // <=

static void
isp_fibre_init_2400(ispsoftc_t *isp)
{
  ....
  if (ISP_CAP_VP0(isp))
    off += ICB2400_VPINFO_PORT_OFF(chan);
  else
    off += ICB2400_VPINFO_PORT_OFF(chan - 1); // <=
  ....
}

Аргумент chan используется в макросе без обёртывания в круглые скобки. В результате, на константу ICB2400_VPOPT_WRITE_SIZE умножается не выражение (chan — 1), а только единица.

Ошибка не появилась бы, если вместо макроса была написана обыкновенная функция.

size_t ICB2400_VPINFO_PORT_OFF(size_t chan)
{
  return   ICB2400_VPINFO_OFF
         + sizeof(isp_icb_2400_vpinfo_t)
         + chan * ICB2400_VPOPT_WRITE_SIZE;
}

С большой вероятностью современный C и C++ компилятор самостоятельно выполнит подстановку (inlining) функции, и код будет столь же эффективен, как и в случае макроса.

При этом код стал более читаемым, а также избавленным от ошибки.

Если известно, что входным значением всегда является константа, то можно добавить constexpr и быть уверенным, что все вычисления произойдут на этапе компиляции. Представим, что это язык C++ и что chan — это всегда некая константа. Тогда функцию ICB2400_VPINFO_PORT_OFF полезно объявить так:

constexpr size_t ICB2400_VPINFO_PORT_OFF(size_t chan)
{
  return   ICB2400_VPINFO_OFF
         + sizeof(isp_icb_2400_vpinfo_t)
         + chan * ICB2400_VPOPT_WRITE_SIZE;
}

Profit!

Надеюсь, мне удалось вас убедить. Желаю удачи и поменьше макросов в коде!

Теги:

Хабы: