Почему Clang / LLVM предупреждает меня об использовании default в операторе switch, где охватываются все перечисленные случаи?

34

Рассмотрим следующий оператор enum и switch:

typedef enum {
    MaskValueUno,
    MaskValueDos
} testingMask;

void myFunction(testingMask theMask) {
    switch (theMask) {
        case MaskValueUno: {}// deal with it
        case MaskValueDos: {}// deal with it
        default: {} //deal with an unexpected or uninitialized value
    }
};

Я программист на Objective-C, но я написал это на чистом C для более широкой аудитории.

Clang / LLVM 4.1 с -Weverything предупреждает меня в строке по умолчанию:

Метка по умолчанию в switch, которая охватывает все значения перечисления

Теперь я могу понять, почему это так: в идеальном мире единственными значениями, входящими в аргумент theMask, являются перечисления, поэтому по умолчанию не требуется. Но что, если какой-то хак придет и бросит неинициализированный int в мою прекрасную функцию? Моя функция будет предоставлена ​​в виде библиотеки, и я не буду контролировать, что там будет. Использование default- очень аккуратный способ справиться с этим.

Почему боги LLVM считают такое поведение недостойным своего адского устройства? Должен ли я предшествовать этому оператору if, чтобы проверить аргумент?

Swizzlr
источник
1
Я должен сказать, моя причина для всего, чтобы сделать себя лучшим программистом. Как NSHipster говорит : "Pro tip: Try setting the -Weverything flag and checking the “Treat Warnings as Errors” box your build settings. This turns on Hard Mode in Xcode.".
Swizzlr
2
-Weverythingможет быть полезным, но будьте осторожны с изменением кода, чтобы справиться с ним. Некоторые из этих предупреждений не только бесполезны, но и неэффективны, и их лучше отключать. (Действительно, это тот случай использования -Weverything: начните с него и выключите то, что не имеет смысла.)
Стивен Фишер
Не могли бы вы немного рассказать о предупреждении для потомков? Там обычно больше, чем это.
Стивен Фишер
Прочитав ответы, я все же предпочитаю ваше первоначальное решение. Использование оператора по умолчанию ИМХО лучше, чем альтернативы, приведенные в ответах. Небольшое примечание: ответы действительно хорошие и информативные, они показывают, как решить проблему.
bbaja42 13.12.12
@ StevenFisher Это было все предупреждение. И, как указал Киллиан, если я позже изменю свое перечисление, это откроет возможность для передачи допустимых значений в реализацию по умолчанию. Кажется, это хороший шаблон дизайна (если это так).
Swizzlr

Ответы:

31

Вот версия, которая не страдает ни от сообщений о проблемном кланге, ни от того, от кого вы защищаетесь:

void myFunction(testingMask theMask) {
    assert(theMask == MaskValueUno || theMask == MaskValueDos);
    switch (theMask) {
        case MaskValueUno: {}// deal with it
        case MaskValueDos: {}// deal with it
    }
}

Киллиан уже объяснил, почему clang выдает предупреждение: если вы расширили enum, вы попадете в случай по умолчанию, что, вероятно, не то, что вы хотите. Правильнее всего будет удалить случай по умолчанию и получить предупреждения для необработанных условий.

Теперь вы обеспокоены тем, что кто-то может вызвать вашу функцию со значением, которое находится за пределами перечисления. Это похоже на неспособность выполнить предварительное условие функции: задокументировано ожидание значения из testingMaskперечисления, но программист пропустил что-то еще. Сделайте так, чтобы ошибка программиста использовалась assert()(или, NSCAssert()как вы сказали, вы используете Objective-C). Выключите вашу программу с сообщением, объясняющим, что программист делает это неправильно, если программист делает это неправильно.


источник
6
+1. В качестве небольшой модификации я предпочитаю иметь каждый случай return;и добавлять его assert(false);в конец (вместо того, чтобы повторяться, перечисляя юридические перечисления в инициале assert()и в switch).
Джош Келли
1
Что, если неправильное перечисление не передано тупым программистом, а скорее ошибкой повреждения памяти? Когда водитель нажимает на разрыв своей Тойоты, и ошибка повредила перечисление, тогда программа обработки прерываний должна аварийно завершить работу, и драйвер должен показать текст: «программист делает это неправильно, плохой программист! ». Я не совсем понимаю, как это поможет пользователю, пока он не проехал через край утеса.
2
@Lundin - это то, что делает OP, или это бессмысленный теоретический случай, который вы только что создали? Несмотря на это, «ошибка повреждения памяти» [i] является ошибкой программиста, [ii] - это не то, от чего вы можете продолжить осмысленным образом (по крайней мере, с учетом требований, изложенных в вопросе).
1
@GrahamLee Я просто говорю, что "ложись и умри" не обязательно лучший вариант, когда ты обнаруживаешь неожиданную ошибку.
1
У него есть новая проблема: вы можете позволить утверждению и случаям не синхронизироваться случайно.
user253751
9

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

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

Килиан Фот
источник
1
Но мое беспокойство - грязная переменная, и защита от этого (например, как я уже сказал, неинициализированный int). Мне кажется, что в такой ситуации оператор switch мог бы достичь дефолта.
Swizzlr
Я не знаю определение Objective-C наизусть, но я предполагал, что перечисление typdef будет выполняться компилятором. Другими словами, «неинициализированное» значение может войти в метод, только если ваша программа уже демонстрирует неопределенное поведение, и я считаю совершенно разумным, что компилятор не принимает это во внимание.
Килиан Фот
3
@KilianFoth нет, они не. Перечисления Objective C - это перечисления C, а не Java. Любое значение из базового целочисленного типа может присутствовать в аргументе функции.
2
Кроме того, перечисления могут быть установлены во время выполнения, из целых чисел. Таким образом, они могут содержать любые мыслимые значения.
ОП правильно. testingMask m; туРипсЫоп (м); скорее всего, ударил бы случай по умолчанию.
Мэтью Джеймс Бриггс
4

Clang сбит с толку, имея заявление по умолчанию, что вполне приемлемая практика, это называется защитным программированием и считается хорошей практикой программирования (1). Он часто используется в критически важных системах, хотя, возможно, не в настольном программировании.

Целью защитного программирования является обнаружение неожиданных ошибок, которые теоретически никогда бы не произошли. Такая неожиданная ошибка не обязательно означает, что программист вводит в функцию неправильный ввод или даже «злой хак». Скорее всего, это может быть вызвано поврежденной переменной: это может быть связано с переполнением буфера, переполнением стека, кодом разгона и подобными ошибками, не связанными с вашей функцией. А в случае встроенных систем переменные могут измениться из-за EMI, особенно если вы используете внешние схемы RAM.

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


(1) MISRA-C: 2004 15.3.


источник
1
Обратите внимание, что обычный программист на ПК обычно находит концепцию защитного программирования совершенно чуждой, поскольку его взгляд на программу - это абстрактная утопия, в которой на практике все, что может быть не так, пойдет не так. Поэтому вы получите довольно разнообразные ответы на свой вопрос в зависимости от того, кого вы спрашиваете.
3
Лязг не смущен; было явно предложено предоставить все предупреждения, включая те, которые не всегда полезны, такие как стилистические предупреждения. (Я лично включаю это предупреждение, потому что не хочу использовать значения по умолчанию в моих переключателях перечисления; их наличие означает, что я не получу предупреждение, если добавлю значение перечисления и не обработаю его. По этой причине я всегда обрабатываю дело с плохим значением вне перечисления.)
Jens Ayton
2
@JensAyton Это смущает. Все профессиональные инструменты статического анализатора, а также все MISRA-совместимые компиляторы будут предупреждать, если в операторе switch отсутствует оператор по умолчанию. Попробуйте сами.
4
Кодирование в MISRA-C - не единственное допустимое использование для компилятора C, и, опять же, -Weverythingвключает каждое предупреждение, а не выбор, соответствующий конкретному варианту использования. Несоответствие вашему вкусу - это не то же самое, что путаница.
Дженс Айтон
2
@Lundin: я уверен, что придерживаться MISRA-C не делает программы магически безопасными и без ошибок ... и я также уверен, что есть много безопасного кода C без ошибок от людей, которые никогда даже не слышал о МИСРА. На самом деле, это немного больше , чем чей - то (популярные, но не бесспорный) взгляд, и есть люди , которые находят некоторые из своих правил произвольными , а иногда даже вредные вне контекста , для которых они были разработаны.
Чао
4

Еще лучше:

typedef enum {
    MaskValueUno,
    MaskValueDos,

    MaskValue_count
} testingMask;

void myFunction(testingMask theMask) {
    assert(theMask >= 0 && theMask<MaskValue_count);
    switch theMask {
        case MaskValueUno: {}// deal with it
        case MaskValueDos: {}// deal with it
    }
};

Это менее подвержено ошибкам при добавлении элементов в перечисление. Вы можете пропустить тест для> = 0, если вы сделаете свои значения перечисления без знака. Этот метод работает, только если у вас нет пробелов в значениях перечисления, но это часто имеет место.

Стив Веллер
источник
2
Это загрязняет тип значениями, которые вы просто не хотите, чтобы этот тип содержал. Здесь есть сходство со старым добрым WTF: thedailywtf.com/articles/What_Is_Truth_0x3f_
Мартин Сугиоарто,
4

Но что, если какой-то хак придет и бросит неинициализированный int в мою прекрасную функцию?

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

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

DeadMG
источник
3
Как насчет того, чтобы зарегистрировать его или вместо этого выдать исключение, если молча проигнорировать это?
jgauffin
3
Действительно, многое можно сделать, например ... добавить оператор по умолчанию к коммутатору и корректно обработать ошибку. Это известно как защитное программирование . Он будет защищать не только от глупого программиста, обрабатывающего некорректные входные данные функции, но и от различных ошибок, вызванных переполнением буфера, переполнением стека, кодом разгона и т. Д. И т. Д.
1
Нет, с этим ничего не получится. Это UB. У программы есть UB в тот момент, когда функция введена, и тело функции ничего не может с этим поделать.
DeadMG
2
@DeadMG Перечисление может иметь любое значение, которое может иметь соответствующий ему целочисленный тип в реализации. В этом нет ничего неопределенного. Доступ к содержимому неинициализированной автоматической переменной - это действительно UB, но «злой хак» (который, скорее всего, является некоторой ошибкой) может с тем же успехом бросить в функцию четко определенное значение, даже если оно не является одним из значений перечислены в объявлении перечисления.
2

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

Насколько вы знаете, компилятор может скомпилировать этот ключ (по умолчанию) следующим образом:

if (theMask == MaskValueUno)
  // Execute something MaskValueUno code
else // theMask == MaskValueDos
  // Execute MaskValueDos code

Как только вы запускаете неопределенное поведение, пути назад уже нет.

Себя
источник
2
Во-первых, это неопределенное значение , а не неопределенное поведение. Это означает, что компилятор может принять другое удобное значение, но он не может превратить луну в зеленый сыр и т. Д. Во-вторых, насколько я могу судить, это относится только к C ++. В C диапазон значений enumтипа совпадает с диапазоном значений его базового целочисленного типа (который определяется реализацией, следовательно, должен быть самосогласованным).
Дженс Айтон
1
Однако экземпляр переменной enum и константы перечисления не обязательно должен иметь одинаковую ширину и подпись, они оба являются impl.defined. Но я почти уверен, что все перечисления в C вычисляются точно так же, как и их соответствующие целочисленные типы, поэтому здесь нет неопределенного или даже неопределенного поведения.
2

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

  • Особое предупреждение (или ошибка, если также выкидывание -Werror) исходит -Wcovered-switch-default( -Weverythingно не из -Wall). Если ваша моральная гибкость позволяет вам отключать определенные предупреждения (например, обрезать некоторые вещи с -Wallили -Weverything), подумайте о том, чтобы бросать -Wno-covered-switch-default(или -Wno-error=covered-switch-defaultпри использовании -Werror), и вообще -Wno-...для других предупреждений, которые вы находите неприемлемыми.
  • Для gcc(и более общего поведения clang), обратитесь к gccстранице Справочника -Wswitch, -Wswitch-enum, -Wswitch-defaultдля (разные) поведения в подобных ситуациях , перечисленных типов в операторах переключения.
  • Мне также не нравится это предупреждение в концепции, и мне не нравится его формулировка; для меня слова из предупреждения («метка по умолчанию ... охватывает все ... значения») предполагают, что default:регистр всегда будет выполняться, например

    switch (foo) {
      case 1:
        do_something(); 
        //note the lack of break (etc.) here!
      default:
        do_default();
    }

    При первом прочтении, я думал, что вы столкнулись с этим - ваше default:дело всегда будет исполняться, потому что его нет break;или что- return;то подобное. Эта концепция похожа (на мой слух) на другие комментарии в стиле няни (хотя иногда и полезные), из которых вытекает clang. Если foo == 1обе функции будут выполнены; ваш код выше имеет такое поведение. Т.е. не удастся прорваться, только если вы захотите продолжить выполнение кода из последующих случаев! Однако, похоже, это не ваша проблема.

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

  • Однако я считаю, что такое поведение (более) согласуется с агрессивной проверкой типов в других языках или компиляторах. Если, как вы предполагаете, некоторый негодяй делает попытку передать intили что - то в эту функцию, которая явно намеревающаяся потреблять свой собственный тип специфического, ваш компилятор должен защищать вас одинаково хорошо в этой ситуации с агрессивным предупреждением или ошибкой. НО это не так! (То есть, кажется, что по крайней мере gccи clangне делают enumпроверки типов, но я слышал, что это iccделает ). Поскольку вы не получаете безопасность типов , вы можете получить безопасность значений, как описано выше. В противном случае, как предлагается в TFA, рассмотрите structили что-то, что может обеспечить безопасность типов.
  • Другой обходной путь может заключаться в создании новой «ценности» в вашем, enumнапример MaskValueIllegal, и не поддерживать ее caseв вашем switch. Это будет съедено default:(в дополнение к любому другому дурацкому значению)

Да здравствует защитное кодирование!

hoc_age
источник
1

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

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

void myFunction(testingMask theMask) {
    int maskValue = int(theMask);
    switch(maskValue) {
        case MaskValueUno: {} // deal with it
        case MaskValueDos: {}// deal with it
        default: {} //deal with an unexpected or uninitialized value
    }
}

Я считаю, что это гораздо менее нежелательно, чем assert()тестирование каждого из возможных значений или даже предположение, что диапазон значений enum упорядочен, так что работает более простой тест. Это просто уродливый способ делать то, что по умолчанию делает точно и красиво.

Ричард
источник
1
этот пост довольно трудно читать (стена текста). Не могли бы вы изменить его в лучшую форму?
комнат