И снова в космос: как единорог Stellarium посещал

    За все время своего существования люди приложили колоссальное количество усилий, чтобы изучить практически всю площадь звездного неба. На сегодняшний день мы рассмотрели сотни тысяч астероидов, комет, туманностей и звезд, галактик и планет. Чтобы увидеть всю эту красоту самостоятельно, не обязательно выходить из дома и покупать себе телескоп. Можно установить на компьютер Stellarium — виртуальный планетарий, и посмотреть на ночное небо, с комфортом лежа на диване… Но с комфортом ли? Чтобы выяснить ответ на этот вопрос, проверим Stellarium на наличие ошибок в компьютерном коде.



    Немного о проекте...


    Согласно описанию на сайте Wikipedia, Stellarium — это виртуальный планетарий с открытым исходным кодом, доступный для платформ Linux, Mac OS X, Microsoft Windows, Symbian, Android и iOS, а также MeeGo. Программа использует технологии OpenGL и Qt, чтобы создавать реалистичное небо в режиме реального времени. Со Stellarium возможно увидеть то, что можно видеть средним и даже крупным телескопом. Также программа предоставляет наблюдения за солнечными затмениями и движением комет.

    Stellarium создан французским программистом Фабианом Шеро, который запустил проект летом 2001 года. Другие видные разработчики включают Роберта Спирмана, Джохэйннса Гадждозика, Мэтью Гейтса, Тимоти Ривза, Богдана Маринова и Джохана Меериса, который является ответственным за художественные работы.

    … и об анализаторе


    Анализ проекта проводился с помощью статического анализатора кода PVS-Studio. Это инструмент для выявления ошибок и потенциальных уязвимостей в исходном коде программ, написанных на языках С, C++ и C# (в скором времени и на Java!). Работает в среде Windows, Linux и macOS. Он разработан для тех, кому важно повышать качество своего кода.

    Провести анализ было достаточно просто. Сначала я скачал проект Stellarium с GitHub, после чего установил все необходимые для сборки пакеты. Так как проект собирается с помощью Qt Creator, я использовал систему отслеживания запуска компиляторов, которая является встроенной в Standalone-версию анализатора. Там же можно просмотреть готовый отчёт об анализе.

    Новые читатели и пользователи Stellarium возможно задались вопросом: почему в заголовке статьи фигурирует единорог и как он связан с анализом кода? Отвечаю: я являюсь одним из разработчиков PVS-Studio, а единорог — это наш любимый озорной маскот. Итак, вверх!


    Я надеюсь, что благодаря этой статье читатели узнают для себя что-то новое, а разработчики Stellarium смогут устранить часть ошибок и улучшить качество кода.

    Приносите себе кофе с воздушным круассаном и устраивайтесь поудобнее, ведь мы переходим к самому интересному — обзору результатов анализа и разбору ошибок!

    Подозрительные условия


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

    void QZipReaderPrivate::scanFiles()
    {
      ....
      // find EndOfDirectory header
      int i = 0;
      int start_of_directory = -1;
      EndOfDirectory eod;
      while (start_of_directory == -1) {
        const int pos = device->size() 
          - int(sizeof(EndOfDirectory)) - i;
        if (pos < 0 || i > 65535) {
          qWarning() << "QZip: EndOfDirectory not found";
          return;
        }
    
        device->seek(pos);
        device->read((char *)&eod, sizeof(EndOfDirectory));
        if (readUInt(eod.signature) == 0x06054b50)
          break;
        ++i;
      }
      ....
    }

    Предупреждение PVS-Studio: V654 The condition 'start_of_directory == — 1' of loop is always true. qzip.cpp 617

    Смогли найти ошибку? Если да, то хвалю.

    Ошибка кроется в условии цикла while. Оно всегда верно, так как переменная start_of_directory не меняется в теле цикла. Скорее всего, цикл не будет вечным, так как он содержит return и break, но выглядит такой код странно.

    Как мне кажется, в коде забыли сделать присваивание start_of_directory = pos в том месте, где идёт проверка сигнатуры. Тогда и оператор break, пожалуй, лишний. В этом случае код можно переписать так:

    int i = 0;
    int start_of_directory = -1;
    EndOfDirectory eod;
    while (start_of_directory == -1) {
      const int pos = device->size() 
        - int(sizeof(EndOfDirectory)) - i;
      if (pos < 0 || i > 65535) {
        qWarning() << "QZip: EndOfDirectory not found";
        return;
      }
    
      device->seek(pos);
      device->read((char *)&eod, sizeof(EndOfDirectory));
      if (readUInt(eod.signature) == 0x06054b50)
        start_of_directory = pos;
      ++i;
    }

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

    Еще одно странное условие:

    
    class StelProjectorCylinder : public StelProjector
    {
    public:
      ....
    protected:
      ....
      virtual bool 
      intersectViewportDiscontinuityInternal(const Vec3d& capN, 
                                             double capD) const
      {
        static const SphericalCap cap1(1,0,0);
        static const SphericalCap cap2(-1,0,0);
        static const SphericalCap cap3(0,0,-1);
        SphericalCap cap(capN, capD);
        return cap.intersects(cap1) 
            && cap.intersects(cap2) 
            && cap.intersects(cap2);
      }
    };

    Предупреждение PVS-Studio: V501 There are identical sub-expressions 'cap.intersects(cap2)' to the left and to the right of the '&&' operator. StelProjectorClasses.hpp 175

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

    Подобный тип ошибок встречается крайне часто: практически в каждом проверенном проекте мы встречали опечатки, связанные с именами вида имя1 и имя2 и им подобными. Как правило, такие ошибки связаны с copy-paste.

    Данный экземпляр кода является ярким примером ещё одного распространенного паттерна ошибок, по поводу которого мы даже проводили отдельное мини-исследование. Мой коллега Андрей Карпов назвал его "эффектом последней строки". Если вы ещё не знакомы с этим материалом, то предлагаю открыть вкладку в браузере, чтобы почитать позже, а пока продолжим.

    void BottomStelBar::updateText(bool updatePos)
    {
      ....
      updatePos = true;
      ....
      if (location->text() != newLocation || updatePos)
      {
        updatePos = true;
        ....
      }
      ....
      if (fov->text() != str)
      {
        updatePos = true;
        ....
      }
      ....
      if (fps->text() != str)
    
      {
        updatePos = true;
        ....
      }
    
      if (updatePos)
      {
        ....
      }
    }

    Предупреждения PVS-Studio:

    • V560 A part of conditional expression is always true: updatePos. StelGuiItems.cpp 732
    • V547 Expression 'updatePos' is always true. StelGuiItems.cpp 831
    • V763 Parameter 'updatePos' is always rewritten in function body before being used. StelGuiItems.cpp 690

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

    Выглядит странно, не так ли? Во всех местах, где участвует параметр updatePos, он имеет значение true. Это значит, что условия if (location->text() != newLocation || updatePos) и if (updatePos) будут всегда истинны.

    Еще один фрагмент:

    void LandscapeMgr::onTargetLocationChanged(StelLocation loc)
    {
      ....
      if (pl && flagEnvironmentAutoEnabling)
      {
        QSettings* conf = StelApp::getInstance().getSettings();
        setFlagAtmosphere(pl->hasAtmosphere() 
                        & conf->value("landscape/flag_atmosphere", true).toBool());
        setFlagFog(pl->hasAtmosphere() 
                 & conf->value("landscape/flag_fog", true).toBool());
        setFlagLandscape(true);
      }
      ....
    }

    Предупреждения PVS-Studio:

    • V792 The 'toBool' function located to the right of the operator '&' will be called regardless of the value of the left operand. Perhaps, it is better to use '&&'. LandscapeMgr.cpp 782
    • V792 The 'toBool' function located to the right of the operator '&' will be called regardless of the value of the left operand. Perhaps, it is better to use '&&'. LandscapeMgr.cpp 783

    Анализатор выявил подозрительное выражение в аргументах функций setFlagAtmosphere и setFlagFog. Действительно: по обеим сторонам от битового оператора & находятся значения, имеющие тип bool. Вместо оператора & стоит использовать оператор &&, и сейчас я объясню почему.

    Да, результат этого выражения будет всегда корректен. Перед применением побитового «и» оба операнда будут повышаться до типа int. В языке C++ такая конвертация однозначна: false конвертируется в 0, а true конвертируется в 1. Поэтому результат у данного выражения будет таким же, как если бы использовался оператор &&.

    Но есть нюанс. При подсчете результата операции && используется так называемое «ленивое вычисление». Если значение левого операнда является false, то правое значение даже не вычисляется, ведь логическое «и» в любом случае вернет false. Это сделано для экономии вычислительных ресурсов и позволяет писать более сложные конструкции. Например, можно проверить, что указатель не нулевой, и, если это так, разыменовать его для выполнения дополнительной проверки. Пример: if (ptr && ptr->foo()).

    Такое «ленивое вычисление» не производится при использовании побитового оператора &: выражения conf->value("...", true).toBool() будут вычисляться каждый раз, независимо от значения pl->hasAtmosphere().

    В редких случаях это бывает сделано специально. Например, если вычисление правого операнда имеет «побочные эффекты», результат которых используется позже. Так тоже делать не очень хорошо, потому что это затрудняет понимание кода и усложняет уход за ним. К тому же, порядок вычисления операндов & не определен, поэтому в некоторых случаях использования таких «трюков» можно получить неопределенное поведение.

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


    Переходим к следующей теме.

    Неправильная работа с памятью


    Начнем тему динамической памяти с вот такого фрагмента:

    
    /************ Basic Edge Operations ****************/
    /* __gl_meshMakeEdge creates one edge, 
     * two vertices, and a loop (face).
     * The loop consists of the two new half-edges.
     */
    GLUEShalfEdge* __gl_meshMakeEdge(GLUESmesh* mesh)
    {
      GLUESvertex* newVertex1 = allocVertex();
      GLUESvertex* newVertex2 = allocVertex();
      GLUESface* newFace = allocFace();
      GLUEShalfEdge* e;
      
      /* if any one is null then all get freed */
      if ( newVertex1 == NULL 
        || newVertex2 == NULL 
        || newFace == NULL)
      {
        if (newVertex1 != NULL)
        {
          memFree(newVertex1);
        }
        if (newVertex2 != NULL)
        {
          memFree(newVertex2);
        }
        if (newFace != NULL)
        {
          memFree(newFace);
        }
        return NULL;
      }
      
      e = MakeEdge(&mesh->eHead);
      if (e == NULL)
      {
        return NULL;
      }
      
      MakeVertex(newVertex1, e, &mesh->vHead);
      MakeVertex(newVertex2, e->Sym, &mesh->vHead);
      MakeFace(newFace, e, &mesh->fHead);
      
      return e;
    }

    Предупреждения PVS-Studio:

    • V773 The function was exited without releasing the 'newVertex1' pointer. A memory leak is possible. mesh.c 312
    • V773 The function was exited without releasing the 'newVertex2' pointer. A memory leak is possible. mesh.c 312
    • V773 The function was exited without releasing the 'newFace' pointer. A memory leak is possible. mesh.c 312

    Функция выделяет память для нескольких структур и передает её указателям newVertex1, newVertex2 (интересные имена, правда?) и newFace. Если один из них оказывается нулевым, то освобождается вся зарезервированная внутри функции память, после чего поток управления покидает функцию.

    Что же произойдет, если память под все три структуры выделится корректно, а функция MakeEdge(&mesh->eHead) вернет NULL? Поток управления достигнет второго по счету return.

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

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

    Следует отметить, что в данном примере третий return не является ошибочным. Функции MakeVertex и MakeFace передают адреса выделенной памяти в другие структуры данных, тем самым делегируя ответственность за её освобождение.

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

    void AstroCalcDialog::drawAngularDistanceGraph()
    {
      ....
      QVector<double> xs, ys;
      ....
    }

    Осталась всего одна строчка. Дам подсказку: это единственное упоминание объектов xs и ys.

    Предупреждения PVS-Studio:

    • V808 'xs' object of 'QVector' type was created but was not utilized. AstroCalcDialog.cpp 5329
    • V808 'ys' object of 'QVector' type was created but was not utilized. AstroCalcDialog.cpp 5329

    Векторы xs и ys создаются, но нигде не используются. Получается, что при каждом использовании метода drawAngularDistanceGraph происходит лишнее создание и удаление пустого контейнера. Думаю, это объявление осталось в коде после рефакторинга. Это, конечно, не ошибка, но стоит удалить лишний код.

    Странные приведения типов


    Еще один пример после небольшого форматирования выглядит вот так:

    void SatellitesDialog::updateSatelliteData()
    {
      ....
      // set default
      buttonColor = QColor(0.4, 0.4, 0.4);
      ....
    }

    Для того, чтобы понять, в чем ошибка, вам придется посмотреть на прототипы конструкторов класса Qcolor:


    Предупреждения PVS-Studio:

    • V674 The literal '0.4' of 'double' type is being implicitly cast to 'int' type while calling the 'QColor' function. Inspect the first argument. SatellitesDialog.cpp 413
    • V674 The literal '0.4' of 'double' type is being implicitly cast to 'int' type while calling the 'QColor' function. Inspect the second argument. SatellitesDialog.cpp 413
    • V674 The literal '0.4' of 'double' type is being implicitly cast to 'int' type while calling the 'QColor' function. Inspect the third argument. SatellitesDialog.cpp 413

    У класса Qcolor не существует конструкторов, принимающих тип double, поэтому аргументы в примере будут неявно преобразовываться в int. Это приводит к тому, что поля r, g, b объекта buttonColor будут иметь значения 0.

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

    Например, можно было использовать конструктор, принимающий Qrgb, написав:

    buttonColor = QColor(QColor::fromRgbF(0.4, 0.4, 0.4));

    Можно было сделать и по-другому. В Qt для обозначения RGB-цветов используются вещественные значения в диапазоне [0.0, 1.0] или целочисленные в диапазоне [0, 255].

    Поэтому программист мог перевести значения из вещественных в целочисленные, написав вот так:

    buttonColor = QColor((int)(255 * 0.4), 
                         (int)(255 * 0.4), 
                         (int)(255 * 0.4));

    или просто

    buttonColor = QColor(102, 102, 102);

    Заскучали? Не переживайте: впереди нас ждут более интересные ошибки.


    «Единорог в космосе». Вид из Stellarium.

    Прочие ошибки


    Напоследок я оставил вам еще несколько вкусняшек :) Приступим к одной из них.

    HipsTile* HipsSurvey::getTile(int order, int pix)
    {
      ....
      if (order == orderMin && !allsky.isNull())
      {
        int nbw = sqrt(12 * 1 << (2 * order));
        int x = (pix % nbw) * allsky.width() / nbw;
        int y = (pix / nbw) * allsky.width() / nbw;
        int s = allsky.width() / nbw;
        QImage image = allsky.copy(x, y, s, s);
        ....
      }
      ....
    }

    Предупреждение PVS-Studio: V634 The priority of the '*' operation is higher than that of the '<<' operation. It's possible that parentheses should be used in the expression. StelHips.cpp 271

    Ну что, смогли обнаружить ошибку? Рассмотрим подробнее выражение (12 * 1 << (2 * order)). Анализатор напоминает, что операция '*' имеет более высокий приоритет, чем операция битового сдвига '<<'. Легко понять, что умножение 12 на 1 бессмысленно, а скобки вокруг 2 * order не нужны.

    Скорее всего, программист собирался написать так:
    int nbw = sqrt(12 * (1 << 2 * order));
    В таком случае значение <i>12 </i> будет умножаться на корректное число.

    Примечание. Дополнительно хочу отметить, что если значение правого операнда '<<' больше или равно количеству битов левого операнда, то результат не определен. Так как численные литералы по умолчанию имеют тип int, который занимает 32 бита, значение параметра order не должно превышать 15. Иначе вычисление выражения может закончиться неопределенным поведением.

    Продолжаем. Приведенный ниже метод является весьма запутанным, но я уверен, что искушенный читатель справится с обнаружением ошибки :)

    /* inherits documentation from base class */
    QCPRange QCPStatisticalBox::
    getKeyRange(bool& foundRange, SignDomain inSignDomain) const
    {
      foundRange = true;
      if (inSignDomain == sdBoth)
      {
        return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5);
      }
      else if (inSignDomain == sdNegative)
      {
        if (mKey + mWidth * 0.5 < 0)
          return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5);
        else if (mKey < 0)
          return QCPRange(mKey - mWidth * 0.5, mKey);
        else
        {
          foundRange = false;
          return QCPRange();
        }
      }
      else if (inSignDomain == sdPositive)
      {
        if (mKey - mWidth * 0.5 > 0)
          return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5);
        else if (mKey > 0)
          return QCPRange(mKey, mKey + mWidth * 0.5);
        else
        {
          foundRange = false;
          return QCPRange();
        }
      }
      foundRange = false;
      return QCPRange();
    }

    Предупреждение PVS-Studio: V779 Unreachable code detected. It is possible that an error is present. qcustomplot.cpp 19512.

    Дело в том, что все ветви if...else имеют return. Поэтому поток управления никогда не дойдет до последних двух строк.

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

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

    /* inherits documentation from base class */
    QCPRange QCPStatisticalBox::
    getKeyRange(bool& foundRange, SignDomain inSignDomain) const
    {
      foundRange = true;
    
      switch (inSignDomain)
      {
      case sdBoth:
      {
        return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5);
        break;
      }
      case sdNegative:
      {
        if (mKey + mWidth * 0.5 < 0)
          return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5);
        else if (mKey < 0)
          return QCPRange(mKey - mWidth * 0.5, mKey);
        break;
      }
      case sdPositive: {
        if (mKey - mWidth * 0.5 > 0)
          return QCPRange(mKey - mWidth * 0.5, mKey + mWidth * 0.5);
        else if (mKey > 0)
          return QCPRange(mKey, mKey + mWidth * 0.5);
        break;
      }
      }
    
      foundRange = false;
      return QCPRange();
    }

    Последней в нашем обзоре будет ошибка, которая понравилась мне больше всего. Код проблемного места краток и прост:

    Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3)
      : distance(0.0f), sDistance(0.0f)
    {
      Plane(v1, v2, v3, SPolygon::CCW);
    }

    Заметили что-то подозрительное? Не каждый сможет :)

    Предупреждение PVS-Studio: V603 The object was created but it is not being used. If you wish to call constructor, 'this->Plane::Plane(....)' should be used. Plane.cpp 29

    Программист рассчитывал, что часть полей объекта будет инициализирована внутри вложенного конструктора, но получилось так: когда вызывается конструктор Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3), внутри него создается безымянный временный объект, который сразу же удаляется. В результате часть послей объекта остается неинициализированной.

    Чтобы код заработал правильно, следует использовать удобную и безопасную фичу C++11 — делегирующий конструктор:

    Plane::Plane(Vec3f& v1, Vec3f& v2, Vec3f& v3)
      : Plane(v1, v2, v3, SPolygon::CCW)
    {
      distance = 0.0f;
      sDistance = 0.0f;
    }

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

    Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3)
      : distance(0.0f), sDistance(0.0f)
    {
      this->Plane::Plane(v1, v2, v3, SPolygon::CCW);
    }

    Или так:

    Plane::Plane(Vec3f &v1, Vec3f &v2, Vec3f &v3)
      : distance(0.0f), sDistance(0.0f)
    {
      new (this) Plane(v1, v2, v3, SPolygon::CCW);
    }

    Замечу, что последние два способа являются весьма опасными. Поэтому следует быть очень осторожным и хорошо понимать, как такие способы работают.

    Заключение


    Какие выводы можно сделать о качестве кода Stellarium? Честно говоря, ошибок было не очень много. Также во всем проекте я не обнаружил ни одной ошибки, в которой код завязан на неопределенном поведении. Для opensource-проекта качество кода оказалось на высоком уровне, за что я снимаю шляпу перед разработчиками. Ребята, вы молодцы! Мне было приятно и интересно делать обзор на ваш проект.

    Что же насчет самого планетария — я пользуюсь им достаточно часто. К сожалению, живя в городе, я очень редко могу насладиться ясным ночным небом, а Stellarium позволяет мне оказаться в любой точке планеты, не вставая с дивана. Это действительно комфортно!

    Особенно мне нравится режим «Constellation art». От вида огромных фигур, застилающих все небо в странном танце, захватывает дух.


    «Странный танец». Вид из Stellarium.

    Нам, землянам, свойственно ошибаться, и нет ничего постыдного в том, что эти ошибки просачиваются в код. Для этого и разрабатываются инструменты анализа кода, такие, как PVS-Studio. Если вы один из землян — ставьте лайк предлагаю скачать и попробовать самому.

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

    Подписывайтесь на наши каналы и следите за новостями из мира программирования!



    Если хотите поделиться этой статьей с англоязычной аудиторией, то прошу использовать ссылку на перевод: George Gribkov. Into Space Again: how the Unicorn Visited Stellarium
    PVS-Studio
    709,00
    Static Code Analysis for C, C++, C# and Java
    Поделиться публикацией

    Комментарии 23

      0
      Так как проект собирается с помощью Qt Creator, я использовал систему отслеживания запуска компиляторов, которая является встроенной в Standalone-версию анализатора

      но зачем? Насколько я вижу, stellarium собирается с помощью cmake. Почему бы просто не скормить PVS сгенерированный им compile_commands.json?

        +2
        Видимо, был выбран Windows для сборки и анализа проекта. Под эту платформу CMake не умеет генерить compile_commands.json. С другой стороны, из CMakeLists можно сделать проект для Visual Studio, но это уже вопрос к автору.

        В любом случае, это уже дело вкуса, т.к. сейчас анализатор поддерживает почти всё. Я, например, предпочитаю сборочные системы для Linux, чтобы проверять проекты в Docker.
        +2
        На тему космоса :)
        NASA обновили свой статический анализатор
        github.com/NASA-SW-VnV/ikos

        Не смотрели, что за зверь?
          +2
          Всегда хотелось написать статью в стиле «Хьюстон, у нас проблема» :D. Посмотрим, что за проект.
            0
            Любопытно будет посмотреть.
              +4
              Может, в стиле «Хьюстон, у ВАС проблема»? ;)
            0
            Дело в том, что все ветви if...else имеют return

            Это на основании того что анализируемое значение enum? Частенько использую что то вроде enum class SomeId { null = 0 }. Ну и проверки вроде if(id == SomeId::null) return; Всё за такой проверкой будет считаться unreachable?

              +1
              На основании того, что там просто return. Неважно, что в условии, если в итоге выполняется return или return.

              Что касается содержимого условий (enum, не enum, неважно), есть другой тип недостижимого кода, который находит эта диагностика. После того, как Dataflow-анализ определяет Always True/False ветви, выполняется дополнительный поиск недостижимого кода уже с учётом этой информации.
                0

                Но там же if/elsif/elsif. Ветки else нет. Судить о недостижимом можно только по тому что в этих ветках проверены все значения enum.

                  –1
                  А если enum вышел из диапазона возможных значений, то это уже UB. :)
                    +1
                    Ой ли?
                    enum class byte : unsigned char {} ;

                    en.cppreference.com/w/cpp/types/byte
                    А как тут быть?
                      0
                      Это уже другая сущность. Я про классический enum. :)
                        0
                        И в классическом enum всё то же самое — за исключением того, что мы заранее не знаем какой тип будет выбран компилятором.
                          0
                          На самом деле даже этого нету. Для обычного enum-а можно указать базовый тип, если ну указать — тогда компилятор выбирает, да, но обычно это int. Надо в стандарте посмотреть, всегда ли это должен быть int.
                            0
                            Для обычного enum-а можно указать базовый тип
                            Начиная с C++11 только.

                            Надо в стандарте посмотреть, всегда ли это должен быть int.
                            Нет. Может быть любой тип куда влазят все перечисленные значения. Хотя на некоторых платформах int — минимум.
                              0
                              Неа, с ISO/IEC 14882:2003 (С++0x)
                                0
                                Вообще-то это две разные вещи.

                                C++0x — это рабочее название для C++11, когда он был в разработке.

                                А в C++03 enum-specifier — enum identifieropt { enumerator-listopt }, извините. Это ISO/IEC 14882:2003…
                      +1
                      Не совсем так. Неопределённое поведение — это когда вы выходите за границы выбранного компилятором типа. Положить в enum { hi = 2, ho = 4 } hiho; троечку не запрещено.
                    0

                    В примере с рефакторингом через switch последние две строки просто переходят в default ветку. И перестают считаться unreachable.

                  –3

                  А есть разбор pvs-studio? Что плохого pvs-studio нашел в коде самого pvs-studio?

                  +1
                  > Легко понять, что умножение 12 на 1 бессмысленно, а скобки вокруг 2 * order не нужны. 

                  Легко заметить что x << n = x * (2^(n+1)). И (x * 1) << n эквивалентно x * (1 << n). А вот вторые скобки обязательны. Чтобы не вспоминать про приоритеты.
                    0
                    часть ошибок должен подсвечивать компилятор:
                    static const SphericalCap cap3(0,0,-1); // unused variable
                    ...
                    QVector<double> xs, ys; // unused variable
                    ...
                    updatePos = true; // invariant
                    ...
                    setFlagAtmosphere(pl->hasAtmosphere() 
                                        & conf->value("landscape/flag_atmosphere", true).toBool()); // operator & on bool
                    

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

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

                    Самое читаемое