Pull to refresh

Comments 37

UFO just landed and posted this here

По стандарту С++ тип bool имеет константы true=1 и false=0.

см. https://en.cppreference.com/w/cpp/language/implicit_conversion#Integral_conversions

If the source type is bool, the value false is converted to zero and the value true is converted to the value one of the destination type

На этом основан метод приведения - !!x гарантировано или 0 или 1.

UFO just landed and posted this here

Непрямое преобразование в bool идиоматично для C

void *result = someFunc();
if (result) { ... }

и если на каждое такое преобразование ругаться - будет много false positive.

Для C++ это гораздо более оправдано. Хочешь bool - пиши каст, хочешь идиоматичность - используй умные указатели или перегружай оператор каста.

Конечно же пример в статье с большой вероятностью указывает на ошибку (в частности потому, что можно вывести конкретное значение). Но при этом совершенно не важно, что было в значении, приведённом к bool - потому что программа обязана в дальнейшем вести себя так, как будто было присвоено значение true или false.

bool a = 4; // не делайте так!
int b = 4 + a; // b == 5

с другой стороны а как ещё-то?

int r = 4;
// какой-то путь, по которому r не меняется
bool cond1 = r;       // так плохо
bool cond2(r);        // а чем это лучше?
bool cond3 = bool(r); // много писать
bool cond4 = !!r;     // лучше откомментить, если не везде

Да, всё верно. Подтверждаю, ругаться на такие выражения как if (result) смысла нет. Собственно, PVS-Studio на них и не ругается (и для C для C++). Слишком много срабатываний.

Хотя нет, есть например, вот такой вариант аномальной проверки: if (enum_var) V768. Но эта другая история.

По поводу false possitve сам бы лучше не написал. Абсолютно согласен, что ругаться на каждое такое присвоение - тот еще идиотизм. Выделяйте тогда warning-и по степени критичности, если ваш статистический анализатор умеет в семантический анализ.

К сожалению, не понял этот комментарий и вообще эту ветку обсуждения. При чём здесь упаковка и т.д... Да, бывают "особые" boolean типы. Например, существует творческий VARIANT_BOOL, для которого истина задаётся как -1. Однако, это отдельная история, не имеющая отношения к bool.

Правила преобразования bool чётки и понятны. Однако, некоторые из таких преобразований странны и могут являться признаком наличия в коде ошибки. Такой странностью является запись в переменную bool значения отличного от 0/1/false/true. Именно аномальные преобразования и хочет обнаруживать человек, задающий вопрос.

Даже если логика кода верна, то всё равно лучше явно писать что-то в духе:

bool a1 = a != 0;
UFO just landed and posted this here

Не очень понятная дискуссия :). Я ведь не то, чтобы спорю :). "Творческий" в том плане, что провоцирует большее количество ошибок, связанных с невнимательностью. Хотя формально разницы нет, будет истина 1 или -1.

UFO just landed and posted this here

анализатор PVS-Studio: пример на сайте Compiler Explorer.

Здорово!. Не знал, что PVS-Studio привинтили к Compiler Explorer. Это полезная информация которая, по моему, вполне адекватна для ответа. Это ничем не отличается от ссылок на Compiler Explorer с использованием msvc или icc. Если эта ссылка демонстрирует, что кто-то что-то делает лучше, пусть и за деньги, то это не делает ее менее полезной в контексте рабочего примера в Compiler Explorer.

Бесплатно и пропиетарно это не антогонисты.

Согласен. Но обычно воспринимается это так.

UFO just landed and posted this here
На вопрос «я попробовал воспользоваться конкретным статическим анализатором кода, но не получил ожидаемого результата» отвечать «попробуйте воспользоваться статическими анализаторами кода» это сильно.
UFO just landed and posted this here

В первом кейсе ложноположительное срабатывание и у автора вопроса, и у автора статьи.

1) С стародавних времён любое ненулевое значение принималось за истину, и только нулевое - ложь. Это основы основ и об этом нельзя забывать.

2) Открываем справочник Герберта Шилдта "С++: полное руководство", страница 367:

"Виртуальные функции и полиформизм"

"Виртуальная функция (virtual function) - это функция-член, обявляемая в базовом классе и переопределяемая в производном. Чтобы создать виртуальную функцию, следует указать ключевое слово virtual перед её объявлением в базовом классе. Производный класс переопределяет эту функцию, приспосабливая ее для своих нужд. По существу, виртуальная функция реализует принцип "один интерфейс, несколько методов", лежащий в основе полиформизма. Виртуальная функция в базовом классе определяет вид интерфейса, т.е. способ вызова этой функции. Каждое переопределение виртуальной функции в производном классе реализует операции, присущие лишь данному классу. Иначе говоря, переопределение виртуальной функции создает конкретный метод (specific method)."

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

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

Это интересный комментарий, который я использую при случае для написания статьи. Спасибо за тему для дискуссии :).

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

Например, код вида:

if (A == A)

компилируется, однако почти наверняка он ошибочен. О наличии такого кода и должен предупредить статический анализатор кода (V501). И благодаря тому, что анализатор обращает внимание на такой, корректный с точки зрения компилятора код, можно выявить множество ошибок. Да, есть экзотическое использование такой конструкции для проверки float/double переменных на равенство NaN. Но это особый случай, про который статические анализаторы тоже в курсе (по крайней мере, про это знает PVS-Studio).

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

Вариант 1. Код ошибочек и его нужно исправить.

Вариант 2. Код корректен, но с запахом. В этом случае лучше провести рефакторинг. Например написать так:

bool a1 = a != 0;

Вариант 3. Это действительно ложное срабатывание. Его можно подавить различными способами.

В любом случае, то, что хочет искать человек, - места с сильным запахом, подверженные ошибкам. Есть смысл задавать вопрос на StackOverflow и использовать статический анализ кода для поиска таких паттернов кода.

Например, вот случай, когда явное присваивание в переменную типа bool значения, выходящего за диапазон [0..1], свидетельствует о настоящей ошибке. Я нашёл это в ОС Tizen.

typedef enum {
  BT_PROXIMITY_LINKLOSS_ALERT = 0x01,
  BT_PROXIMITY_IMMEDIATE_ALERT = 0x02,
  BT_PROXIMITY_TX_POWER = 0x04,
} bt_proximity_property_t;

typedef struct {
  bt_proximity_reporter_server_s reporter_server;
  bool services_supported;                                 // <=
  bool connected;

  const void *connection_callback;
  void *user_data;
} bt_proximity_reporter_s;

static bt_proximity_reporter_s *reporter_s = NULL;

int bt_proximity_reporter_create(....)
{
  ....
  reporter_s->services_supported =                         // <=
    BT_PROXIMITY_LINKLOSS_ALERT | BT_PROXIMITY_IMMEDIATE_ALERT;
  ....
}

Хотели записать значение в переменную reporter_server, но промахнулись. И записали в переменную services_supported типа bool.

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

Мне совершенно не нравится тактовка "потенциально опасны". В чем же опасны, если конвертация идет только в 0 или 1?

В вашей же строке:

BT_PROXIMITY_LINKLOSS_ALERT | BT_PROXIMITY_IMMEDIATE_ALERT;

В bool записывают побитовое ИЛИ. Окей, допустим это ошибка и хотели логическое ИЛИ - известный прием записи в булевые переменные результатов условий. Тогда если подразумевалось, что перечесление выше - это коды ошибок, то программист забыл нулевое значение, 0x0 - код успешного выполнения программы. И сравнивать следовало бы именно с ним.

В данном же случае переменная services_supported в структуре reporter_s всегда будет true. Компилятор посчитает, что в логике 00000001 и 00000010. Результат операции 00000011, то есть != 0, иначе говоря true. Запишет компилятор 11111111 или 10000000 или 00000001 значения не имеет. Либо логические условия if (bool) {...la-la-la...} исполнят содержимое блока только когда значение true, то есть любое ненулевое, в противном случае else {...}, если он вообще есть.

В чем здесь опасность вопрос открытый? И каким лесом здесь вообще целочисленное переполнение, если тут вообще не про числа речь?! Вы хоть сами матчасть от подделок форумных аналитиков отличайте, а то реально за державу уже обидно.

Мне совершенно не нравится трактовка "потенциально опасны". В чем же опасны, если конвертация идет только в 0 или 1?

Опасна не сама конвертация. Опасна аномальность этого действия.

Окей, допустим это ошибка, и хотели логическое ИЛИ - известный прием записи в булевые переменные результатов условий.

Любой практикующий С или С++ программист понимает, что здесь совсем другое. Видно, что хотели использовать именно побитовое ИЛИ, чтобы сформировать маску из разных бит. Обратите внимание, что именованные константы в enum являются степенями двойки.

Итак, с помощью | составляется маска из битов. Далее эту маску хотят записать в переменную типа bt_proximity_property_t. Это не очень красиво, но допустимо, так как для представления enum используется достаточное количество бит для вмещения значения 3. Подробнее про тонкости.

Но точно нет никакого смысла запихивать маску b0..0011 в переменную типа bool. Это аномалия. Да, этот код компилируется. Да, понятно, как работает неявное приведение типов. Даже можно придумать какую-то ситуацию, когда такой код оправдан. Но всё равно это аномалия, о которой сообщает анализатор. И действительно - перед нами не какая-то хитрая задумка, а опечатка.

Опасна не сама конвертация. Опасна аномальность этого действия.

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

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

А вы мне тут про маски, тонкости велосипедов по запиливаю того, что уже 20 лет существует в куда более удобном виде.

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

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

Далее. Про виртуальные функции вообще вашего юмора не понял.

Для начала следующий абзац с той же страницы 368:

"При обычном вызове виртуальные функции ничем не отличаются от остальных функций-членов. Особые свойства виртуальных функций проявляются при их вызове с помощью указателей. Как сказано в главе 13, указатели на объекты базового класса можно использовать для ссылки на объект произодных классов. Если указатель на объект базового класс устанавливается на объект производного класса, содержащий виртуальную функцию, выбор требуемой функции основывается на типе объекта, на который ссылается указатель, причем этот выбор осуществляется в ходе выполнения программы. Таким образом, если указатель ссылается на объекты разных типов, то будут вызваны разные виртуальные функции. Это относится и к ссылка на объекты базового класса. "

(И специально для вас я перепишу прекрасный наглядный пример, как это работает)

#include <iostream>
using namespace std;

class base {
  public:
    virtual void vfunc() {
      cout << "Функция vfunc() из класса base.\n";
    }
};

class derived1 : public base {
public:
  void vfunc() {
    cout << "Функция vfunc() из класса derived1.\n";
  }
};

class derived2 : public base {
public: 
  void vfunc() {
    cout << "Функция vfunc() из класса derived2.\n";
  }
};

int main()
{
  base *p, b;
  derived1 d1;
  derived2 d2;
  
  //Указатель на объект базового класса
  p = &b;
  p->vfunc(); //Вызов функции vfunc() из класса base.
  
  //Указатель на объект класса derived1
  p = &d1;
  p->vfunc(); //Вызов функции vfunc() из класса derived1
  
  //Указатель на объект класса derived2
  p = &d2;
  p->vfunc(); //Вызов функции vfunc() из класса derived2
  
  return 0;
}

Автор подчеркивает, что ключевое слово virtual используется только один раз. Далее в книге разъясняется, что оно наследуется.

А теперь студенты ответьте мне на вопрос: "Где вы увидили проблему вызова виртуальной функции в конструкторе и диструкторе класса? Ответ дать по отдельности для каждого случая".

Подразумевая, что вы оба, как неприлежные студенты, не разбираетесь в вопросе когда вызываются конструктор и диструктор класса. И в добавок совершенно упустили тему "В каком порядке определяются объекты родительских классов при определение предка, и в каком порядке они уничтожаются".

Возьмите оба луковицы и хорошенько подумайте над этим. Пересдача через две недели по расписанию у деканата кафедры. Всего хорошего.

Аргументировать прочитанной (одной?) книгой, это конечно прикольно... Но к теме выявления потенциальных ошибок, это не имеет никакого отношения.

Приведу в свою очередь описание предупреждения V1053.

Анализатор обнаружил вызов виртуальной функции в конструкторе или деструкторе класса.

Рассмотрим пример:

struct Base
{
  Base()
  {
    foo();
  }
  
  virtual ~Base() = default;
  virtual void foo() const;  
};

Сам по себе вызов виртуального метода 'foo' в конструкторе класса 'Base' может не являться ошибкой, однако проблемы могут проявить себя в производных классах.

struct Child : Base
{
  Child() = default;

  virtual ~Child() = default;
  virtual void foo() const override;
};

Во время создания объекта типа 'Child' будет вызван метод 'Base::foo()' из конструктора базового класса, но не переопределенный метод 'Child::foo()' из производного класса. Отметим, что в некоторых других языках программирования (C#, Java, ...) аналогичный код будет работать иначе: во время создания объекта 'Child' конструктором по умолчанию будет вызван конструктор по умолчанию базового класса 'Base', который затем вызовет переопределенный метод 'Child::foo()'.

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

struct Base
{
  Base()
  {
    Base::foo();
  }
  
  virtual ~Base() = default;
  virtual void foo() const;  
};

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

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

struct Base
{
  Base()
  {
    this->foo();       // bad
    this->Base::foo(); // good
  }
  virtual ~Base() = default;
  virtual void foo() const;  
};

Данная диагностика классифицируется как: CERT-OOP50-CPP.

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

Очередная copy paste ради copy paste. Я хоть заморочился, из справочника наглядный пример переписал, а вы хуже глупого студента.

Жаль, что сейчас не могу (с телефона сижу, стационарник сейчас не работает), специально для этого наглого гражданина напишу первую часть статьи, где все ваши мифы разрушу раз и навсегда. Заслужили, так заслужили.

Впечатляет. А можно для интереса узнать. Насколько у вас дружелюбная поддержка, если программист кидает вам репорт с комментариями ложноположительных срабатываний? А то к той же статье был комментарий, где у человека терпения не хватило и его первые 30 ложноположительных подряд вывели из себя.

Честно говоря, даже 10 ложноположительных подряд пережить той ещё выдержкой нужно обладать, особенно если проект очень личный.

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

Клиенты довольны. Хейтеры хейтят. Всё как везде и всегда.

Из статьи "Интеграция PVS-Studio в uVision Keil":
И началась эпическая переписка с техподдержкой, которая – исключительно по моей вине – растянулась почти на год (!). Вот честное слово — техподдержка у PVS-Studio – натурально лучшая из всех, с кем я общался, а общался я со многими, от российских производителей микросхем, где человек поздравлял меня с «днём пирожков с малиновым вареньем» (нет, это не шутка) до крупнейших зарубежных компаний, где меня месяцами футболили от человека к человеку :)
Тут же я со стыдом признаюсь, что отвечал существенно медленнее, чем отвечали мне… частично меня оправдывает необходимость заниматься основными рабочими задачами, но только частично.
Кстати, ещё одна интересная история сегодня про поддержку появилась: Слава баг-репортам, или как мы сократили время анализа проекта пользователя с 80 до 4 часов.
За основу возьмём реальное общение: больше 100 писем в переписке, исключения, анализ, который не завершается за трое суток…

Правда из вашего блога я предпочитаю разборы аномалий в известных проектах. Тут относительно недавно у людей возник спор, никак разработчики браузера Vivaldi не могут разобраться. Может вы им поможете, как независимая сторона? Заодно и полезная статья с разбором ошибок будет.

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

Я так и не дождался этой обещанной статьи, поэтому написал собственную :)

Вызов виртуальных функций в конструкторах и деструкторах (C++)

Это всё прекрасно, но механизм виртуальных функций не работает в конструкторах/деструкторах (собственно, откуда и возник исходный вопрос).
https://isocpp.org/wiki/faq/strange-inheritance#calling-virtuals-from-ctors
https://isocpp.org/wiki/faq/strange-inheritance#calling-virtuals-from-dtors

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

Не удивлюсь, если Шилдт забыл об этом упомянуть, такие зевки для него очень характерны.

А что он ожидал вызывая виртуальную функцию в своём же классе? Хочет вызвать функцию предка? Так оно делается через приведение типа к классу предку, и вызывай себе наздоровье.

Абстракции классов в виде private, public, protected, published не для красоты придумали.

Сомнительна даже сфера практического применения. Вот объясните для начала, где такое можно увидеть в реальных программах. Потому что сферических коней в вакууме я не обсуждаю.

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

Говоря о том, что их обычный вызов не отличается от других функций и разница проявляется только при вызове через указатели, Шилдт как-то забыл упомянуть, что обычный вызов из члена класса — это и есть вызов через указатель this.

Так и запишем: "Проверить вызов виртуальных функций через неявный указатель this"

Ещё пожелания есть?

P.S. У Шилдта указатель this разобран на страницах 284-285 в Главе 13. Массивы, указатели, ссылки и операторы динамического распределения памяти. И то что, внутри класса, он ссылается на него же в том числе. Просто он писал об этом в контексте, что сокращенная форма используется чаще, либо просто удобней, и пару нюансов об дружественных объектах указал.

Sign up to leave a comment.