Добро пожаловать на Pro Pawn - Портал о PAWN-скриптинге.
Показано с 1 по 4 из 4

Тема: warning 210 (?)

  1. #1
    Аватар для Daniel_Cortez
    "Это не хак, это фича"

    Статус
    Оффлайн
    Регистрация
    06.04.2013
    Адрес
    Novokuznetsk, Russia
    Сообщений
    2,192
    Репутация:
    2589 ±

    warning 210 (?)

    Привет всем участникам Pro-Pawn, да и просто "проходящим мимо". Хотел бы поделиться с вами мнением о том, что уже давно наболело в плане Pawn, а по возможности и узнать мнение других скриптеров.
    Предупрежу сразу: статья не рассчитана на новичков. Если помимо Pawn у вас не было опыта программирования на других языках (таких как, например, Pascal, C или C++), скорее всего, вы не поймёте, о чём говорится в этой статье.

    UPD (21.07): Доступен тестовый билд компилятора с warning 210, ссылки в конце поста.
    UPD (24.07): Новый тестовый билд, исправлены нежелательные срабатывания warning 210 и добавлено обнаружение неинициализированных массивов.



    Суть проблемы

    Ещё месяц назад я нашёл в своём древнем тестовом моде (который я вряд ли когда-то выложу из-за большого количества устаревшего кода) один интересный баг:
    1. CMD:gotopl(playerid, params[]) // "go to player" - телепортация к игроку
    2. {
    3. if(player_info[playerid][pAdminLevel] < ADM_MODERATOR)
    4. return 1;
    5. new targetid, Float:x, Float:y, Float:z;
    6. if(sscanf(params, "u", targetid))
    7. return SendClientMessage(playerid, COLOR_GREY, "Использование: /goto [playerid/PartOfName]");
    8. if(playerid == INVALID_PLAYER_ID)
    9. return SendClientMessage(playerid, COLOR_GREY, "Ошибка: Игрок не подключен.");
    10. if(playerid == targetid)
    11. return SendClientMessage(playerid, COLOR_GREY, "Ошибка: Вы не можете телепортироваться к самому себе.");
    12. SetPlayerPos(playerid, x, y, z);
    13. return SendClientMessage(playerid, COLOR_WHITE, "Вы телепортировались к игроку.");
    14. }

    Обратите внимание: переменные "x", "y" и "z" используются в SetPlayerPos(), но незадачливый автор кода (то бишь, я 6 лет назад) забыл сначала получить в них координаты игрока, к которому нужно телепортироваться (GetPlayerPos(targetid, x, y, z);), из-за чего админа по ошибке телепортирует на нулевые координаты (т.к. переменные по умолчанию инициализируются нулём). Этот код был написан мной ещё в 2013 году и баг обнаружился только сейчас.
    Казалось бы, ну что здесь необычного? Тестовый мод запускался только на локальном сервере и команда "/gotopl" ни разу не использовалась на других игроках, потому ошибка и оставалась незамеченной годами. Будь этот мод на реальном сервере - баг обнаружился бы гораздо быстрее. Сам же виноват, правда?

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

    Дабы не быть голословным по поводу принципов Pawn, приведу цитату из официальной документации:
      Открыть/закрыть

    Из Pawn Language Guide, приложение C "Rationale" ("Принципы языка"):
    Promote error checking: As you may have noticed, one of the foremost design criterions of the C language, "trust the programmer", is absent from my list of design criterions. Users of script languages may not be experienced programmers; and even if they are, PAWN will probably not be their primary language. Most PAWN programmers will keep learning the language as they go, and will even after years not have become experts. Enough reason, hence, to replace error prone elements from the C language (pointers) with saver, albeit less general, constructs (references).* References are copied from C++ . They are nothing else than pointers in disguise, but they are restricted in various, mostly useful, ways. Turn to a C++ book to find more justification for references.
    (...)
    * You should see this remark in the context of my earlier assertion that many PAWN pro-
    grammers will be novice programmers. In my (teaching) experience, novice programmers
    make many pointer errors, as opposed to experienced C/C++ programmers.
    Перевод:
    Обеспечение контроля над ошибками: Как вы могли заметить, один из главных принципов языка C "доверяй программисту" отсутствует в моём списке принципов дизайна PAWN. Пользователи скриптовых языков могут быть неопытными программистами; и даже если они опытные, PAWN наверняка не будет их основным языком. Многие PAWN-программисты будут изучать язык "на ходу", и даже через несколько лет не станут экспертами в нём. По этой причине опасные элементы из языка C (указатели) были заменены на более безопасные, хоть и менее общие элементы (ссылки).* Ссылки были позаимствованы из C++. Они - ни что иное, как замаскированные указатели, но они ограничены разными, в основном полезными способами. Обратитесь к книгам по C++, чтобы больше узнать, чем выгодно использование ссылок.
    (...)
    * Это замечание следует рассматривать в контексте моего более раннего заявления о том, что многие PAWN-программисты будут новичками. По своему опыту (преподавательскому) я знаю, что новички допускают много ошибок с указателями, в отличие от опытных программистов на C/C++.


    Ещё один интересный факт в том, что в компиляторе Pawn есть неиспользуемый текст:
    1. /*210*/ "possible use of symbol before initialization: \"%s\"\n",

    Это - warning 210, предупреждение, которое, судя по тексту, должно выдаваться, когда переменная где-то используется, но в неё ещё не было записано значение. Этот текст присутствует в компиляторе чуть ли не с самого момента появления языка Pawn (ещё когда он назывался Small), но предупреждение не выдаётся, т.к. с тех пор оно почему-то так и не было реализовано (и остаётся нереализованным до сих пор, даже в последних версиях Pawn 4, хотя этот текст там всё ещё присутствует).

    Кроме того, это предупреждение описано и в официальной документации:
      Открыть/закрыть

    Из Pawn Language Guide, приложение A "Error and warning messages" ("Сообщения об ошибках и предупреждениях"):
    210 possible use of symbol before initialization: identifier
    A local (uninitialized) variable appears to be read before a value is assigned to it. The compiler cannot determine the actual order of reading from and storing into variables and bases its assumption of the execution order on the physical appearance order of statements an expressions in the source file.
    Перевод:
    210 возможное использование переменной до инициализации: (идентификатор)
    Локальная неинициализированная переменная считывается до того, как ей было присвоено значение. Компилятор не умеет определять истинный порядок считывания и записи в переменные и основывает своё предположение на порядке физического расположения предложений и выражений в исходном файле.

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



    Причины

    Так почему это предупреждение не было реализовано? Вот несколько предположений, которые мне приходилось слышать ранее:
    • Это фишка языка, чтобы вместо "new x = 0;" можно было написать "new x;".
      Но если так, то почему же в официальной документации об этом ни слова? Во всех примерах кода переменные либо инициализированы при объявлении, либо значение записывается в процессе работы кода, но всегда прежде чем произойдёт считывание из переменной.

    • Это сделано для простоты языка, чтобы новичкам не нужно было задумываться об инициализации переменных нулём.
      Опять же, в официальной документации об этом ни слова. Упоминается, что переменные инициализируются нулём по умолчанию, но нигде не сказано, что на это поведение следует полагаться.
      Да и не совсем понятно, как введение отдельного правила для нуля должно упростить язык.

    Лично моё мнение, как человека, потратившего уйму времени на изучение документации и исходных кодов Pawn (и, как результат, страдающего завышенной самооценкой), что это предупреждение не было сделано по причине сложности реализации (на момент предполагаемого появления ещё в Small), или же просто из-за банальной забывчивости изначального автора Pawn (чем, кстати, объясняется и тот факт, что неиспользуемый текст warning 210 не удалён даже в последних версиях Pawn, спустя почти 20 лет с момента появления в компиляторе).
    Либо же это могла быть комбинация и сложности, и забывчивости. Замечать что-то подобное в Pawn приходится не в первый раз - уже известно несколько не до конца реализованных и просто халтурных решений:
    • Начиная с ранних выпусков Small, в ВМ были реализованы инструкции "umul", "udiv", "udiv.alt", "jless", "jleq", "jgrtr", "jgeq", "less", "leq", "grtr" и "geq" для работы с беззнаковыми целыми числами (что говорит о том, что в определённый момент поддержка таких чисел планировалась), но ни в одной из версий Small/Pawn эти инструкции не генерировались компилятором, т.е. оставались неиспользованными.

    • В том же самом Small в ВМ была реализована инструкция call.pri, позволяющая вызывать функции по указателю, которая не генерировалась компилятором. В то же время, в самом компиляторе присутствовала константа iREFFUNC, обозначавшая тип данных "функция, переданная как параметр":
      1. #define iREFFUNC 10 /* function passed as a parameter */

      что говорит о том, что были планы (а возможно и попытки, т.к. эта константа ещё и используется в нескольких местах, но нигде не присваивается) сделать поддержку передачи ссылок на функции в качестве параметров для других функций. Этой возможности не было ни в одном из выпусков Small/Pawn, но iREFFUNC до сих пор присутствует в последних версиях компилятора (4.*).

    • В ответ на жалобы об уязвимостях в виртуальной машине, в Pawn 3.3 была удалена директива #emit. И это вместо того, чтобы искать и устранять дыры в самой ВМ.

    • В Pawn 4 в одном из последних коммитов упоминается, что реализовано объединение дублирующихся константных строк в одну, но на самом деле добавлено только 3 функции для составления списка константных массивов, и эти функции нигде даже не используются.



    Решение

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

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


    Минусы:
    • (Неустранимых недостатков найти не удалось, все их можно с лёгкостью исправить или обойти.)


    Спорные вопросы:
    • В: Но ведь это нарушает один из основных принципов языка Pawn, по которому все переменные и массивы по умолчанию инициализируются нулями!
      О: Они и дальше будут инициализироваться нулями. Просто компилятор начнёт выдавать предупреждения, если переменная используется, но пользователь ещё не записал в неё значение явным образом, т.к. это может быть признаком ошибки.

    • В: Пользователи очень часто записывают циклы следущим образом:
      1. for (new i; i<n; i++)

      Разве это не приведёт к флуду ложными срабатываниями warning 210?
      О: Да, но это можно решить двумя способами:
      1. Предоставить пользователю возможность заменить все "new i;" на "new i = 0;".
        Естественно, мало кто будет доволен куче новых предупреждений, как и не были довольны в прошлом году из-за введения требований к const-корректности в релизе компилятора 3.10.9. Знаем, проходили. С другой стороны, много скриптеров тогда узнало, как пользоваться #pragma warning disable =)
      2. Ограничить случаи выдачи warning 210.
        Например, не выдавать предупреждение, если
        • переменная объявлена внутри цикла for;
          1. for (new n; n < 1000; n += 10) // Переменная 'n' не инициализирована явным образом,
          2. // но 'warning 210' для неё не отображается

        • значение переменной увеличивается/уменьшается с помощью оператора ++/--;
          1. new num_drivers;
          2. for (new i = 0; i < MAX_PLAYERS; ++i)
          3. if (GetPlayerState(i) == PLAYER_STATE_DRIVER)
          4. num_drivers++; // Переменной 'num_drivers' нигде не было присвоено значение,
          5. // но 'warning 210' для неё не отображается

        • массив передаётся в нативную функцию, аргумент-массив в которой объявлен как const (не все функции SA-MP объявлены корректно).
          1. // В <a_samp> функция 'GetPlayerName' объявлена с параметром 'name',
          2. // по ошибке помеченным как 'const'
          3. native GetPlayerName(playerid, const name[], len);
          4. // ...
          5. new pname[MAX_PLAYER_NAME + 1];
          6. GetPlayerName(playerid, pname, sizeof(pname));// 'warning 210' не выдаётся для массива 'pname'



      На мой взгляд, ограничение выдачи warning 210 в перечисленных выше случаях - идеальный компромиссный вариант, и именно так я и хотел бы сделать, если дело дойдёт до попытки предложить эту фичу ко включению в компилятор "3.10". Это сведёт количество ложных срабатываний к разумному минимуму и позволит скриптеру сконцентрироваться на настоящих ошибках, которые можно будет найти с помощью этого предупреждения.

    • В: Но даже если ограничить выдачу warning 210, ложные срабатывания всё равно останутся?
      О: Их легко исправить, достаточно лишь дописать "= 0" в объявление переменной, на которую жалуется компилятор.
      Это намного проще, чем с введением const-корректности в прошлом году (warning 214, 239), из-за которой приходилось переписывать код.

    • В: Разве инициализация нулями по умолчанию не была специально сделана без показа предупреждений, чтобы так было проще новичкам?
      О: Во-первых, пользователь (и не факт даже, что новичок!) может элементарно забыть инициализировать переменную (ниже я привёл примеры), что приведёт к багам в коде - и компилятор об этом гордо промолчит. Безусловно, отсутствие контроля над ошибками упрощает жизнь программистам, особенно новичкам! (нет)
      Во-вторых, в официальной документации (Pawn Language Guide) не сказано ни слова о том, что предупреждение 210 не показывается, равно как и почему оно не показывается.

    • В: А вдруг возникнет такая ситуация, что warning 210 срабатывает ложно и его нельзя избежать?
      О: Я пробовал компилировать несколько модов и ни разу не встречал ситуаций, когда вообще никак нельзя было устранить это предупреждение. Но даже если такая исключительная ситуация и возникнет, warning 210 всегда можно отключить:
      1. // сохраняем текущее состояние ошибок/предупреждений
      2. #pragma warning push
      3. // отключаем warning 210
      4. #pragma warning disable 210
      5.  
      6. // код, в котором ложно выдаётся warning 210
      7. // ...
      8.  
      9. // восстанавливаем сохранённое ранее состояние, warning 210 снова будет выдаваться
      10. #pragma warning pop



    Поиск скрытых ошибок

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

    Для начала, один интересный баг, встреченный мной в коде проекта NGRP.
    В оригинале функция довольно большая, поэтому для наглядности я приведу очень упрощённую версию того кода (кому интересно, можете взглянуть на оригинал):
    1. new Float:veh_speed[MAX_PLAYERS]; // Для сохранения скорости транспорта у игроков
    2. new Float:veh_health[MAX_PLAYERS]; // Для сохранения здоровья транспортного средства
    3. new bool:seatbelt_status[MAX_PLAYERS char]; // Статус ремней безопасности (пристёгнуты/не пристёгнуты)
    4.  
    5. // Таймерная функция, в которой у игрока отнимается ХП,
    6. // если он находится в машине и машина с чем-то столкнулась.
    7. forward PlayerMicroBeat(playerid);
    8. public PlayerMicroBeat(playerid)
    9. {
    10. new veh = GetPlayerVehicleID(playerid);
    11. if (veh == 0)
    12. return;
    13. new Float:vx, Float:vy, Float:vz;
    14. GetVehicleVelocity(veh, vx, vy, vz);
    15. new Float:vspeed = VectorSize(vx, vy, vz) * 100.0; // текущая скорость транспорта
    16. new Float:vhealth, Float:phealth;
    17. GetVehicleHealth(veh, vhealth);
    18. if (veh_speed[playerid] - vspeed > 50.0 && veh_health[playerid] - phealth > 0.0)
    19. {
    20. GetPlayerHealth(playerid, phealth);
    21. // Если у игрока не пристёгнуты ремни, отнимаем больше ХП
    22. phealth -= (veh_speed[playerid] - vspeed) * (seatbelt_status{playerid} ? 3.0 : 6.0);
    23. SetPlayerHealth(playerid, phealth);
    24. }
    25. veh_speed[playerid] = vspeed;
    26. veh_health[playerid] = vhealth;
    27. }

    Сможете сходу найти здесь ошибку?
    А если вот так?
      Открыть/закрыть

    1. // warning 210: possible use of symbol before initialization: "phealth"
    2. if (veh_speed[playerid] - vspeed > 50.0 && veh_health[playerid] - phealth > 0.0)

    Переменная phealth используется в выражении, но ей ещё не было присвоено значение (по умолчанию инициализирована нулём).
    Как результат, от предыдущей скорости машины отнимается не текущая скорость, а 0. Если машина ни с чем не столкнулась, а просто резко затормозила, у игрока всё равно отнимется ХП.
    Очевидно, что это опечатка, и автор кода намеревался вместо phealth использовать переменную vhealth с текущим здоровьем транспорта, т.е. код должен был выглядеть примерно так:
    1. if (veh_speed[playerid] - vspeed > 50.0 && veh_health[playerid] - vhealth > 0.0)


    К слову, этот баг присутствовует в репозитории NGRP, начиная с первого коммита от 12.01.2014 (на самом деле он был там и раньше, Git-репозиторий у них появился не сразу). И он особенно показателен тем, что даже такие крупные проекты, как NGRP, имеющие в своём распоряжении целую команду скриптеров, не застрахованы от столь элементарных ошибок.

    Ещё один небольшой баг, тоже из NGRP:
    1. forward CloseCage();
    2. public CloseCage()
    3. {
    4. // warning 210: possible use of symbol before initialization: "cage"
    5. MoveDynamicObject(cage,-773.52050781,2545.62109375,10022.29492188,2);
    6. return 1;
    7. }

    Глобальная переменная "cage" используется в "MoveDynamicObject()", но во всём моде ей нигде не было присвоено значение.
    Скорее всего, кто-то вносил изменения в маппинг и удалил строку с созданием объекта "cage = CreateDynamicObject(...)", но забыл удалить саму переменную "cage" и код перемещения объекта.

    Вот ещё примеры скрытых ошибок и недочётов, которые я нашёл, проверив несколько модов, лежащих в открытом доступе на GitHub:



    Заключение

    Если в новом релизе компилятора появится warning 210, пользователи в очередной раз увидят новые предупреждения в своих скриптах - их будет не так много, как после прошлогоднего нововведения с const-корректностью (да и исправить их будет гораздо легче - не придётся переписывать свой код, в отличие от...), но они всё же будут. В то же время, этот тип предупреждений может здорово помочь в нахождении скрытых ошибок в пользовательском коде, которые до этого целыми годами могли оставаться незамеченными. Даже самые опытные кодеры и самые прибыльные проекты не застрахованы от таких ошибок.

    В скором времени я выложу тестовую версию компилятора с реализованным warning 210, чтобы вы смогли на деле оценить её. А пока что хотелось бы узнать, что вы думаете: стоит ли пытаться довести до ума эту фичу с поиском ошибок, или же это плохая идея и нет смысла тратить на неё время?
    Буду очень рад услышать ваше мнение в комментариях.


    Тестовый выпуск компилятора с warning 210: https://www.dropbox.com/s/kqb322vsg8...-w210.zip?dl=0
    Исходный код: https://github.com/Daniel-Cortez/paw...ee/warning-210
    Индивидуально в ЛС по скриптингу не помогаю. Задавайте все свои вопросы здесь (click).

  2. Пользователь сказал cпасибо:
    DeimoS (25.07.2019)
  3. #2
    Аватар для Web
    Пользователь

    Статус
    Оффлайн
    Регистрация
    24.08.2014
    Сообщений
    67
    Репутация:
    8 ±
    Думаю стоит. При рефакторинге старого кода или поиска ошибок порой не хватает подобного функционала.

  4. #3
    Аватар для Daniel_Cortez
    "Это не хак, это фича"

    Статус
    Оффлайн
    Регистрация
    06.04.2013
    Адрес
    Novokuznetsk, Russia
    Сообщений
    2,192
    Репутация:
    2589 ±
    Тестовый выпуск компилятора с warning 210: https://www.dropbox.com/s/kqb322vsg8...-w210.zip?dl=0
    Исходный код: https://github.com/Daniel-Cortez/paw...ee/warning-210

    Касаемо ограничения выдачи warning 210: пока что сообщение не показывается в следующих случаях:
    • Переменная объявлена как счётчик внутри цикла for:
      1. for (new n; n < 1000; n += 10) // Переменная 'n' не инициализирована явным образом,
      2. // но 'warning 210' для неё не отображается


    • Значение переменной увеличивется/уменьшается с помощью оператора '++'/'--':
      1. new num_drivers;
      2. for (new i = 0; i < MAX_PLAYERS; ++i)
      3. if (GetPlayerState(i) == PLAYER_STATE_DRIVER)
      4. num_drivers++; // Переменной 'num_drivers' нигде не было присвоено значение,
      5. // но 'warning 210' для неё не отображается


    На практике это снижает количество ложных срабатываний в 3-5 раз, но эти ложные срабатывания всё равно остаются. В принципе их несложно устранить, добавив "= 0" в объявления переменных, но всё же...

    Отдельная проблема - инклуды. Понятное дело, дописывать в них "= 0" - не вариант, поэтому для обхода проблемы можно поступить следующим образом:
    1. #pragma warning push
    2. #pragma warning disable 210
    3.  
    4. // Подключение сторонних инклудов
    5. // ...
    6.  
    7. #pragma warning pop
    8.  
    9. // Подключение своих инклудов (т.е. тех, что являются частью мода)


    Надеюсь, для кого-нибудь эта фича окажется полезной.
    Индивидуально в ЛС по скриптингу не помогаю. Задавайте все свои вопросы здесь (click).

  5. #4
    Аватар для Daniel_Cortez
    "Это не хак, это фича"

    Статус
    Оффлайн
    Регистрация
    06.04.2013
    Адрес
    Novokuznetsk, Russia
    Сообщений
    2,192
    Репутация:
    2589 ±
    Новый тестовый билд: https://www.dropbox.com/s/kqb322vsg8...-w210.zip?dl=0
    Исходный код: https://github.com/Daniel-Cortez/paw...ee/warning-210

    Изменения:
    • Исправлен баг с нежелательной выдачей warning 210 из-за пре-инкремента переменной (было сделано ограничение при пост-инкременте, но с пре-инкрементом предупреждение всё ещё выдавалось).
      1. new i;
      2. ++i; // warning 210 (в новом билде warning не выдаётся)

    • Устранён нежелательный показ warning 210 после того, как переменной было присвоено значение с помощью #emit или __emit.

    • Реализовано обнаружение использования неинициализированных массивов (раньше компилятор мог обнаружить только одиночные переменные).
      Пример обнаружения ошибок, связанных с массивами:
      https://github.com/NextGenerationGam...tions.pwn#L367
      Carrier[0] используется в GetDynamicObjectPos(), однако самому массиву Carrier нигде во всём моде не было присвоено значение. Очевидно, очередной забытый кусок кода: при редактировании маппинга создание объектов (Carrier[0] = CreateDynamicObject(...); Carrier[1] = CreateDynamicObject(...); ...) убрали, а сам массив Carrier и связанный с ним код по недосмотру оставили.

    • Добавлено новое ограничение выдачи warning 210: если массив передаётся нативной функции, принимающей константный массив.
      Это было сделано из-за того, что не все функции SA-MP объявлены корректным образом.
      1. // В <a_samp> функция 'GetPlayerName' объявлена с параметром 'name',
      2. // по ошибке помеченным как 'const'
      3. native GetPlayerName(playerid, const name[], len);
      4. // ...
      5. new pname[MAX_PLAYER_NAME + 1];
      6. GetPlayerName(playerid, pname, sizeof(pname));// 'warning 210' не выдаётся для массива 'pname'
    Индивидуально в ЛС по скриптингу не помогаю. Задавайте все свои вопросы здесь (click).

 

 

Информация о теме

Пользователи, просматривающие эту тему

Эту тему просматривают: 1 (пользователей: 0 , гостей: 1)

Ваши права

  • Вы не можете создавать новые темы
  • Вы не можете отвечать в темах
  • Вы не можете прикреплять вложения
  • Вы не можете редактировать свои сообщения
  •