Por que Clang / LLVM me avisa sobre o uso padrão em uma instrução switch onde todos os casos enumerados são cobertos?

34

Considere a seguinte enumeração e instrução 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
    }
};

Sou um programador de Objective-C, mas escrevi isso em C puro para um público mais amplo.

Clang / LLVM 4.1 com -Weverything me avisa na linha padrão:

Etiqueta padrão na opção que cobre todos os valores de enumeração

Agora, eu posso ver por que isso existe: em um mundo perfeito, os únicos valores inseridos no argumento theMaskseriam enum, portanto, nenhum padrão é necessário. Mas e se algum hack surgir e lançar um int não inicializado em minha bela função? Minha função será fornecida como uma gota na biblioteca e não tenho controle sobre o que poderia acontecer lá. Usar defaulté uma maneira muito elegante de lidar com isso.

Por que os deuses LLVM consideram esse comportamento indigno de seu dispositivo infernal? Devo estar precedendo isso por uma instrução if para verificar o argumento?

Swizzlr
fonte
1
Devo dizer que minha razão para tudo é me tornar um programador melhor. Como o NSHipster diz : "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
-Weverythingpode ser útil, mas tenha cuidado ao alterar muito seu código para lidar com ele. Alguns desses avisos não são apenas inúteis, mas contraproducentes e são melhor desativados. (Na verdade, esse é o caso de uso para -Weverything: começar com ele, e desligar o que não faz sentido.)
Steven Fisher
Você poderia expandir um pouco a mensagem de aviso para a posteridade? Geralmente há mais do que isso.
22812 Steven
Depois de ler as respostas, ainda prefiro sua solução inicial. Uso da declaração padrão O IMHO é melhor do que as alternativas dadas nas respostas. Apenas uma pequena nota: as respostas são realmente boas e informativas, mostram uma maneira legal de resolver o problema.
bbaja42
@StevenFisher Esse foi o aviso inteiro. E, como Killian apontou, se eu modificar meu enum posteriormente, isso abriria a possibilidade de valores válidos passarem para a implementação padrão. Parece ser um bom padrão de design (se é que existe).
18712 Swizzlr #

Respostas:

31

Aqui está uma versão que não sofre os relatórios do clang do problema ou a que você está protegendo:

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

Killian já explicou por que o clang emite o aviso: se você estender o enum, você entraria no caso padrão que provavelmente não é o que você deseja. O correto é remover o caso padrão e receber avisos de condições não tratadas .

Agora você está preocupado que alguém possa chamar sua função com um valor que esteja fora da enumeração. Parece que falhou em atender ao pré-requisito da função: está documentado esperar um valor da testingMaskenumeração, mas o programador passou outra coisa. Portanto, cometa um erro de programador usando assert()(ou NSCAssert()como você disse que está usando Objective-C). Faça seu programa falhar com uma mensagem explicando que o programador está fazendo errado, se o programa estiver errado.


fonte
6
+1. Como uma pequena modificação, prefiro ter cada caso return;e adicionar um assert(false);ao final (em vez de me repetir listando as enumerações legais em uma inicial assert()e na switch).
Josh Kelley
1
E se o enum incorreto não for passado por um programador idiota, mas por um erro de corrupção de memória? Quando o motorista pressiona o freio do Toyota e um bug corrompe o enum, o programa de manuseio de quebra deve travar e queimar, e deve aparecer um texto para o motorista: "o programador está fazendo errado, programador ruim! " Não vejo bem como isso ajudará o usuário antes que ele atravesse a beira de um precipício.
2
@Lundin é isso que o OP está fazendo, ou esse é um caso teórico sem sentido que você acabou de construir? Independentemente disso, um "erro de corrupção de memória" [i] é um erro do programador, [ii] não é algo de que você possa continuar de maneira significativa (pelo menos não com os requisitos estabelecidos na pergunta).
1
@GrahamLee Estou apenas dizendo que "deitar e morrer" não é necessariamente a melhor opção quando você vê um erro inesperado.
1
Tem o novo problema de que você pode deixar a afirmação e os casos ficarem fora de sincronia acidentalmente.
User253751
9

Ter um defaultrótulo aqui é um indicador de que você está confuso sobre o que está esperando. Como você esgotou todos os enumvalores possíveis explicitamente, eles defaultnão podem ser executados e você também não precisa se proteger contra alterações futuras, porque se você estendeu o valor enum, a construção geraria um aviso.

Portanto, o compilador percebe que você cobriu todas as bases, mas parece estar pensando que não, e isso sempre é um mau sinal. Ao fazer o mínimo esforço possível para alterar a switchforma esperada, você demonstra ao compilador que o que parece estar fazendo é o que realmente está fazendo, e você sabe disso.

Kilian Foth
fonte
1
Mas minha preocupação é uma variável suja, e protegendo-a (como, como eu disse, um int não inicializado). Parece-me que seria possível que a instrução switch atinja o padrão em tal situação.
Swizzlr
Não conheço a definição de Objective-C de cor, mas presumi que uma enumeração typdef'd seria aplicada pelo compilador. Em outras palavras, um valor "não inicializado" só poderia inserir o método se o seu programa exibir um comportamento indefinido, e acho inteiramente razoável que um compilador não leve isso em consideração.
Kilian Foth
3
@KilianFoth não, eles não são. As enumerações Objective-C são enumerações C, não enumerações Java. Qualquer valor do tipo integral subjacente pode estar presente no argumento da função.
2
Além disso, enumerações podem ser definidas em tempo de execução, a partir de números inteiros. Para que eles possam conter qualquer valor imaginável.
OP está correto. testingMask m; minhaFunção (m); provavelmente atingiria o caso padrão.
Matthew James Briggs
4

Clang está confuso, ter uma declaração padrão de que existe uma prática perfeitamente correta, é conhecida como programação defensiva e é considerada uma boa prática de programação (1). É bastante usado em sistemas de missão crítica, embora talvez não em programação de desktop.

O objetivo da programação defensiva é detectar erros inesperados que, em teoria, nunca aconteceriam. Um erro tão inesperado não é necessariamente o programador que fornece à função uma entrada incorreta ou mesmo um "mal hack". Provavelmente, isso pode ser causado por uma variável corrompida: estouros de buffer, estouro de pilha, código descontrolado e erros semelhantes não relacionados à sua função podem estar causando isso. E, no caso de sistemas embarcados, as variáveis ​​podem mudar devido à EMI, principalmente se você estiver usando circuitos de RAM externos.

Quanto ao que escrever dentro da declaração padrão ... se você suspeitar que o programa deu errado depois que você chegou lá, precisará de algum tipo de tratamento de erros. Em muitos casos, você provavelmente pode simplesmente adicionar uma declaração vazia com um comentário: "inesperado, mas não importa" etc, para mostrar que você pensou na situação improvável.


(1) MISRA-C: 2004 15.3.


fonte
1
Por favor, não que o programador de PC comum ache o conceito de programação defensiva completamente estranho, uma vez que a visão de um programa é uma utopia abstrata em que nada que possa estar errado na teoria dará errado na prática. Portanto, você receberá respostas bastante diversas para sua pergunta, dependendo de quem você perguntar.
3
Clang não está confuso; foi solicitado explicitamente a fornecer todos os avisos, incluindo os que nem sempre são úteis, como os avisos estilísticos. (Eu pessoalmente uso esse aviso porque não quero padrões nos meus switches enum; tê-los significa que eu não recebo um aviso se eu adicionar um valor enum e não o tratar. Por esse motivo, eu sempre manipulo o caso mau valor fora do enum).
Jens Ayton
2
@JensAyton Está confuso. Todas as ferramentas profissionais de analisador de estática, bem como todos os compiladores compatíveis com MISRA, emitem um aviso se uma instrução switch não possuir uma instrução padrão. Experimente você mesmo.
4
Codificar para MISRA-C não é o único uso válido para um compilador C e, novamente, -Weverythingativa todos os avisos, não uma seleção apropriada para um caso de uso específico. Não se adequar ao seu gosto não é a mesma coisa que confusão.
Jens Ayton
2
@Lundin: Tenho certeza de que seguir o MISRA-C não torna magicamente os programas seguros e livres de erros ... e também tenho certeza de que há bastante código C seguro e livre de erros de pessoas que nunca ouviu falar de MISRA. De fato, é pouco mais do que a opinião de alguém (popular, mas não indiscutível), e há pessoas que acham algumas de suas regras arbitrárias e às vezes até prejudiciais fora do contexto para o qual foram projetadas.
cHao
4

Melhor ainda:

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
    }
};

Isso é menos propenso a erros ao adicionar itens à enumeração. Você pode pular o teste para> = 0 se você deixar seus valores de enum sem sinal. Esse método funciona apenas se você não tiver lacunas nos valores da enumeração, mas esse geralmente é o caso.

Steve Weller
fonte
2
Isso está poluindo o tipo com valores que você simplesmente não deseja que o tipo contenha. Tem semelhanças com o bom WTF velho aqui: thedailywtf.com/articles/What_Is_Truth_0x3f_
Martin Sugioarto
4

Mas e se algum hack surgir e lançar um int não inicializado em minha bela função?

Então você obtém um comportamento indefinido e sua defaultvontade não faz sentido. Não há nada que você possa fazer para melhorar isso.

Deixe-me ser mais claro. No instante em que alguém passa um não inicializado intpara sua função, é Comportamento indefinido. Sua função poderia resolver o problema da parada e isso não importaria. É UB. Não há nada que você possa fazer depois que o UB for chamado.

DeadMG
fonte
3
Que tal registrá-lo ou lançar uma exceção se ignorá-la silenciosamente?
jgauffin
3
De fato, há muito que pode ser feito, como ... adicionar uma instrução padrão ao comutador e manipular graciosamente o erro. Isso é conhecido como programação defensiva . Ele irá não só protege contra a entrega programador mudo a função entradas incorrectas, mas também contra vários erros causados por sobrecarga da memória intermédia, excesso de pilha, código fugitivo etc etc
1
Não, não vai aguentar nada. É UB. O programa possui UB no momento em que a função é inserida e o corpo da função não pode fazer nada a respeito.
precisa
2
@DeadMG Um enum pode ter qualquer valor que seu tipo inteiro correspondente na implementação possa ter. Não há nada indefinido sobre isso. Acessar o conteúdo de uma variável automática não inicializada é realmente UB, mas o "mal hack" (que é provavelmente algum tipo de bug) também pode lançar um valor bem definido para a função, mesmo que não seja um dos valores listados na declaração enum.
2

A declaração padrão não ajudaria necessariamente. Se a opção ultrapassar uma enumeração, qualquer valor que não esteja definido na enumeração acabará executando um comportamento indefinido.

Pelo que você sabe, o compilador pode compilar essa opção (com o padrão) como:

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

Depois que você aciona um comportamento indefinido, não há como voltar atrás.

Eu mesmo
fonte
2
Em primeiro lugar, é um valor não especificado , não um comportamento indefinido. Isso significa que o compilador pode assumir algum outro valor mais conveniente, mas pode não transformar a lua em queijo verde, etc. Em segundo lugar, até onde eu sei, isso só se aplica ao C ++. Em C, o intervalo de valores de um enumtipo é igual ao intervalo de valores de seu tipo inteiro subjacente (que é definido pela implementação e, portanto, deve ser autoconsistente).
Jens Ayton
1
No entanto, uma instância de uma variável de enumeração e uma constante de enumeração não precisam necessariamente ter a mesma largura e sinalidade, pois ambas são definidas. Mas tenho certeza de que todas as enumerações em C são avaliadas exatamente como seu tipo inteiro correspondente, portanto, não há comportamento indefinido ou mesmo não especificado lá.
2

Eu também prefiro ter um default:em todos os casos. Estou atrasado para a festa, como sempre, mas ... alguns outros pensamentos que não vi acima:

  • O aviso específico (ou erro se também estiver sendo lançado -Werror) é proveniente -Wcovered-switch-default(de -Weverythingmas não -Wall). Se a sua flexibilidade moral permite que você gire off certas advertências (ie, extirpar algumas coisas -Wallou -Weverything), considere jogando -Wno-covered-switch-default(ou -Wno-error=covered-switch-defaultpelo uso -Werror), e em geral -Wno-...para os outros avisos que você encontrar desagradável.
  • Para gcc(e comportamento mais genérica em clang), consulte gccpágina de manual para -Wswitch, -Wswitch-enum, -Wswitch-defaultpara o comportamento (diferente) em situações semelhantes de tipos enumerados em declarações switch.
  • Eu também não gosto desse aviso em conceito e não gosto da redação; para mim, as palavras do aviso ("etiqueta padrão ... cobre todos ... valores") sugerem que o default:caso será sempre executado, como

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

    Na primeira leitura, era para isso que eu pensava que você estava se deparando - que seu default:caso seria sempre executado porque não existe break;ou não é return;semelhante. Esse conceito é semelhante (ao meu ouvido) a outros comentários no estilo de babá (embora ocasionalmente úteis) que surjam clang. Se foo == 1, ambas as funções serão executadas; seu código acima tem esse comportamento. Ou seja, falhe apenas se você quiser continuar executando o código de casos subseqüentes! Parece, no entanto, não ser o seu problema.

Correndo o risco de ser pedante, alguns outros pensamentos para a plenitude:

  • No entanto, acho que esse comportamento é (mais) consistente com a verificação agressiva de tipos em outros idiomas ou compiladores. Se, como hipótese, alguns réprobos não tentar passar uma intou algo para esta função, que é explícita a intenção de consumir o seu próprio tipo específico, o compilador deve protegê-lo igualmente bem nessa situação com um aviso agressivo ou de erro. MAS isso não acontece! (Ou seja, parece que pelo menos gcce clangnão faz enumverificação de tipo, mas ouvi dizer que iccsim ). Como você não está obtendo o tipo -segurança, você pode obter o valor -segurança conforme discutido acima. Caso contrário, conforme sugerido no TFA, considere um structou algo que possa fornecer segurança de tipo.
  • Outra solução alternativa poderia ser criar um novo "valor" no seu enum, como MaskValueIllegalnão oferecer suporte a ele caseno seu switch. Isso seria comido pelo default:(além de qualquer outro valor maluco)

Viva a codificação defensiva!

hoc_age
fonte
1

Aqui está uma sugestão alternativa:
O OP está tentando se proteger contra o caso em que alguém passa por intonde é esperado um enum . Ou, mais provavelmente, quando alguém vinculou uma biblioteca antiga a um programa mais recente usando um cabeçalho mais novo com mais casos.

Por que não mudar o interruptor para lidar com o intcaso? A adição de uma conversão antes do valor no comutador elimina o aviso e até fornece uma certa dica sobre por que o padrão existe.

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
    }
}

Acho isso muito menos censurável do que assert()testar cada um dos valores possíveis ou até assumir que o intervalo de valores enum está bem ordenado, para que um teste mais simples funcione. Essa é apenas uma maneira feia de fazer o que o padrão faz com precisão e beleza.

Richard
fonte
1
é difícil ler este post (parede de texto). Você se importaria de editá -lo em uma forma melhor?
mosquito