¿Por qué Clang / LLVM me advierte sobre el uso de valores predeterminados en una declaración de cambio donde se cubren todos los casos enumerados?

34

Considere la siguiente instrucción enum y 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
    }
};

Soy un programador de Objective-C, pero he escrito esto en C puro para un público más amplio.

Clang / LLVM 4.1 con -Weverything me advierte en la línea predeterminada:

Etiqueta predeterminada en el interruptor que cubre todos los valores de enumeración

Ahora, puedo ver por qué esto está ahí: en un mundo perfecto, los únicos valores que entran en el argumento theMaskestarían en la enumeración, por lo que no es necesario ningún valor predeterminado. Pero, ¿qué pasa si aparece algún truco y arroja un int no inicializado a mi hermosa función? Mi función se proporcionará como una caída en la biblioteca, y no tengo control sobre lo que podría entrar allí. Usar defaultes una forma muy ordenada de manejar esto.

¿Por qué los dioses LLVM consideran este comportamiento indigno de su dispositivo infernal? ¿Debería estar precediendo esto por una declaración if para verificar el argumento?

Swizzlr
fuente
1
Debo decir que mi razón para -Todo es hacerme un mejor programador. A medida que el NSHipster dice : "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
-Weverythingpuede ser útil, pero tenga cuidado al mutar demasiado su código para lidiar con él. Algunas de esas advertencias no solo son inútiles sino contraproducentes, y es mejor desactivarlas. (De hecho, ese es el caso de uso -Weverything: comience con él y apague lo que no tiene sentido.)
Steven Fisher
¿Podría ampliar un poco el mensaje de advertencia para la posteridad? Usualmente hay más para ellos que eso.
Steven Fisher
Después de leer las respuestas, todavía prefiero su solución inicial. El uso de la declaración predeterminada en mi humilde opinión es mejor que las alternativas dadas en las respuestas. Solo una pequeña nota: las respuestas son realmente buenas e informativas, muestran una manera genial de cómo resolver el problema.
bbaja42
@StevenFisher Esa fue toda la advertencia. Y como Killian señaló si modifico mi enumeración más tarde, se abriría la posibilidad de que los valores válidos pasen a la implementación predeterminada. Parece ser un buen patrón de diseño (si es así).
Swizzlr

Respuestas:

31

Aquí hay una versión que no sufre ni del informe del problema ni del que estás protegiendo:

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

Killian ya ha explicado por qué el sonido metálico emite la advertencia: si extendió la enumeración, caería en el caso predeterminado que probablemente no sea lo que desea. Lo correcto es eliminar el caso predeterminado y obtener advertencias para condiciones no controladas .

Ahora le preocupa que alguien pueda llamar a su función con un valor que esté fuera de la enumeración. Eso suena como no cumplir con el requisito previo de la función: está documentado esperar un valor de la testingMaskenumeración, pero el programador ha pasado algo más. Entonces haga que sea un error de programador usando assert()(o NSCAssert()como dijo que está usando Objective-C). Haga que su programa se bloquee con un mensaje que explica que el programador lo está haciendo mal, si el programador lo hace mal.


fuente
66
+1. Como pequeña modificación, prefiero tener cada caso return;y agregar un assert(false);al final (en lugar de repetirme enumerando las enumeraciones legales en una inicial assert()y en el switch).
Josh Kelley
1
¿Qué pasa si la enumeración incorrecta no es pasada por un programador tonto, sino por un error de corrupción de memoria? Cuando el conductor presiona el freno de su Toyota y un error ha corrompido la enumeración, entonces el programa de manejo del freno debería fallar y quemarse, y debería mostrarse un texto al conductor: "¡el programador lo está haciendo mal, mal programador! ". No entiendo cómo esto ayudará al usuario, antes de conducir por el borde de un acantilado.
2
@Lundin, ¿es eso lo que está haciendo el OP, o es un caso teórico sin sentido que acabas de construir? Independientemente, un "error de corrupción de memoria" [i] es un error del programador, [ii] no es algo de lo que pueda continuar de manera significativa (al menos no con los requisitos establecidos en la pregunta).
1
@GrahamLee Solo digo que "acostarse y morir" no es necesariamente la mejor opción cuando detecta un error inesperado.
1
Tiene el nuevo problema de que puede dejar que la afirmación y los casos se desincronicen accidentalmente.
user253751
9

Tener una defaultetiqueta aquí es un indicador de que estás confundido acerca de lo que estás esperando. Dado que ha agotado todos los enumvalores posibles explícitamente, defaultposiblemente no se pueda ejecutar, y tampoco lo necesita para protegerse contra cambios futuros, porque si extendió el enum, la construcción ya generaría una advertencia.

Entonces, el compilador nota que ha cubierto todas las bases pero parece estar pensando que no lo ha hecho, y eso siempre es una mala señal. Al hacer un esfuerzo mínimo para cambiar la switchforma esperada, demuestra al compilador que lo que parece estar haciendo es lo que realmente está haciendo, y lo sabe.

Kilian Foth
fuente
1
Pero mi preocupación es una variable sucia, y protegerme de eso (como, como dije, un int no inicializado). Me parece que sería posible que la instrucción switch alcance el valor predeterminado en tal situación.
Swizzlr
No sé la definición de Objective-C de memoria, pero supuse que el compilador impondría una enumeración típica. En otras palabras, un valor "no inicializado" solo podría ingresar al método si su programa ya exhibe un comportamiento indefinido, y me parece completamente razonable que un compilador no lo tenga en cuenta.
Kilian Foth
3
@KilianFoth no, no lo son. Las enumeraciones Objective-C son enumeraciones C, no enumeraciones Java. Cualquier valor del tipo integral subyacente podría estar presente en el argumento de la función.
2
Además, las enumeraciones se pueden establecer en tiempo de ejecución, a partir de enteros. Para que puedan contener cualquier valor imaginable.
OP es correcto. testingMask m; myFunction (m); probablemente golpearía el caso predeterminado.
Matthew James Briggs el
4

Clang está confundido, teniendo una declaración predeterminada, hay una práctica perfectamente buena, se conoce como programación defensiva y se considera una buena práctica de programación (1). Se usa mucho en sistemas de misión crítica, aunque quizás no en la programación de escritorio.

El propósito de la programación defensiva es detectar errores inesperados que en teoría nunca sucederían. Tal error inesperado no es necesariamente que el programador le dé a la función una entrada incorrecta, o incluso un "hack malvado". Lo más probable es que pueda deberse a una variable corrupta: desbordamientos de búfer, desbordamiento de pila, código desbocado y errores similares no relacionados con su función podrían estar causando esto. Y en el caso de los sistemas embebidos, las variables podrían cambiar debido a EMI, particularmente si está utilizando circuitos RAM externos.

En cuanto a qué escribir dentro de la declaración predeterminada ... si sospecha que el programa se volvió loco una vez que terminó allí, entonces necesita algún tipo de manejo de errores. En muchos casos, es probable que simplemente agregue una declaración vacía con un comentario: "inesperado pero no importa", etc., para mostrar que ha pensado en la situación poco probable.


(1) MISRA-C: 2004 15.3.


fuente
1
Tenga en cuenta que el programador de PC promedio generalmente encuentra que el concepto de programación defensiva es completamente extraño, ya que su visión de un programa es una utopía abstracta en la que nada de lo que puede estar mal en teoría saldrá mal en la práctica. Por lo tanto, obtendrá respuestas muy diversas a su pregunta dependiendo de a quién le pregunte.
3
Clang no está confundido; se le ha pedido explícitamente que proporcione todas las advertencias, que incluyen las que no siempre son útiles, como las advertencias estilísticas. (Personalmente opto por esta advertencia porque no quiero valores predeterminados en mis modificadores de enumeración; tenerlos significa que no recibo una advertencia si agrego un valor de enumeración y no lo manejo. Por esta razón, siempre manejo el caso de mal valor fuera de la enumeración.)
Jens Ayton
2
@JensAyton Está confundido. Todas las herramientas profesionales de analizador estático, así como todos los compiladores compatibles con MISRA darán una advertencia si una declaración de interruptor carece de una declaración predeterminada. Pruebe uno usted mismo.
44
La codificación a MISRA-C no es el único uso válido para un compilador de C y, de nuevo, -Weverythingactiva todas las advertencias, no una selección apropiada para un caso de uso en particular. No ajustarse a su gusto no es lo mismo que la confusión.
Jens Ayton
2
@Lundin: estoy bastante seguro de que apegarse a MISRA-C no hace que los programas sean mágicamente seguros y libres de errores ... y estoy igualmente seguro de que hay un montón de código C seguro y libre de errores de personas que nunca han siquiera He oído hablar de MISRA. De hecho, es poco más que la opinión de alguien (popular, pero no indiscutible), y hay personas que encuentran algunas de sus reglas arbitrarias y, a veces, incluso perjudiciales fuera del contexto para el que fueron diseñadas.
cHao
4

Mejor aún:

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

Esto es menos propenso a errores al agregar elementos a la enumeración. Puede omitir la prueba para> = 0 si hace que sus valores de enumeración no estén firmados. Este método solo funciona si no tiene huecos en los valores de su enumeración, pero ese suele ser el caso.

Steve Weller
fuente
2
Esto está contaminando el tipo con valores que simplemente no desea que contenga el tipo. Tiene similitudes con el buen viejo WTF aquí: thedailywtf.com/articles/What_Is_Truth_0x3f_
Martin Sugioarto
4

Pero, ¿qué pasa si aparece algún truco y arroja un int no inicializado a mi hermosa función?

Entonces obtienes Comportamiento Indefinido , y tu defaultvoluntad no tendrá sentido. No hay nada que puedas hacer para mejorar esto.

Déjame ser más claro. En el momento en que alguien pasa un no inicializado inta su función, es Comportamiento indefinido. Su función podría resolver el problema de detención y no importaría. Es UB. No hay nada que puedas hacer una vez que UB ha sido invocado.

DeadMG
fuente
3
¿Qué tal si lo registra o lanza una excepción si lo ignora en silencio?
jgauffin
3
De hecho, hay mucho que se puede hacer, como ... agregar una declaración predeterminada al switch y manejar el error con gracia. Esto se conoce como programación defensiva . No sólo protegerá contra entrega programador tonto las entradas incorrectas de función, sino también contra varios errores causados por el desbordamiento del búfer, desbordamiento de pila, código fuera de control, etc, etc
1
No, no manejará nada. Es UB. El programa tiene UB en el momento en que se ingresa la función, y el cuerpo de la función no puede hacer nada al respecto.
DeadMG
2
@DeadMG Una enumeración puede tener cualquier valor que pueda tener su tipo entero correspondiente en la implementación. No hay nada indefinido al respecto. Acceder al contenido de una variable automática no inicializada es, de hecho, UB, pero el "malvado truco" (que es más probable que sea algún tipo de error) también podría arrojar un valor bien definido a la función, aunque no sea uno de los valores enumerado en la declaración de enumeración.
2

La declaración predeterminada no necesariamente ayudaría. Si el cambio está sobre una enumeración, cualquier valor que no esté definido en la enumeración terminará ejecutando un comportamiento indefinido.

Por lo que sabes, el compilador puede compilar ese interruptor (con el valor predeterminado) como:

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

Una vez que desencadena un comportamiento indefinido, no hay vuelta atrás.

Mí mismo
fuente
2
En primer lugar, es un valor no especificado , no un comportamiento indefinido. Esto significa que el compilador puede asumir algún otro valor que sea más conveniente, pero no puede convertir la luna en queso verde, etc. En segundo lugar, por lo que puedo decir, eso solo se aplica a C ++. En C, el rango de valores de un enumtipo es el mismo que el rango de valores de su tipo entero subyacente (que está definido por la implementación, por lo tanto, debe ser autoconsistente).
Jens Ayton
1
Sin embargo, una instancia de una variable enum y una constante de enumeración no necesariamente tienen que tener el mismo ancho y firma, ambas están definidas implícitamente. Pero estoy bastante seguro de que todas las enumeraciones en C se evalúan exactamente como su tipo entero correspondiente, por lo que no hay un comportamiento indefinido o incluso no especificado allí.
2

También prefiero tener un default:en todos los casos. Llego tarde a la fiesta como de costumbre, pero ... algunos otros pensamientos que no vi arriba:

  • La advertencia particular (o error si también se lanza -Werror) proviene de -Wcovered-switch-default(de -Weverythingpero no -Wall). Si su flexibilidad moral le permite desactivar ciertas advertencias (es decir, eliminar algunas cosas de -Wallo -Weverything), considere tirar -Wno-covered-switch-default(o -Wno-error=covered-switch-defaultal usar -Werror) y, en general, -Wno-...otras advertencias que considere desagradables.
  • Para gcc(y un comportamiento más genérico clang), consulte gccla página de manual de -Wswitch, -Wswitch-enum, -Wswitch-defaultde (diferentes) el comportamiento en situaciones similares de los tipos enumerados en los estados de conmutación.
  • Tampoco me gusta esta advertencia en concepto, y no me gusta su redacción; para mí, las palabras de la advertencia ("etiqueta predeterminada ... cubre todos los ... valores") sugieren que el default:caso siempre se ejecutará, como

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

    En una primera lectura, esto es lo que pensé que se estaba ejecutando en - que su default:caso siempre sería ejecutado porque no hay es break;o return;o similar. Este concepto es similar (a mi oído) a otros comentarios de estilo de niñera (aunque ocasionalmente útiles) que se desprenden clang. Si foo == 1, ambas funciones se ejecutarán; Su código anterior tiene este comportamiento. Es decir, ¡no se separe solo si desea seguir ejecutando código de casos posteriores! Esto parece, sin embargo, no ser su problema.

A riesgo de ser pedante, algunos otros pensamientos para completar:

  • Sin embargo, creo que este comportamiento es (más) consistente con la verificación agresiva de tipos en otros idiomas o compiladores. Si, como usted formular la hipótesis, algunos réprobos no intente pasar una into algo para esta función, que es la intención explícita de consumir su propio tipo específico, el compilador debe protegerte igualmente bien en esa situación con una advertencia o error agresiva. PERO no lo hace! (Es decir, parece que al menos gccy clangno enumverificas el tipo, pero escuché que icc ). Como no obtiene el tipo -safety, puede obtener el valor -safety como se discutió anteriormente. De lo contrario, como se sugiere en TFA, considere uno structo algo que pueda proporcionar seguridad de tipo.
  • Otra solución podría ser la creación de un nuevo "valor" en su enumcomo MaskValueIllegal, y no admitir eso caseen su switch. Eso sería comido por el default:(además de cualquier otro valor loco)

¡Viva la codificación defensiva!

hoc_age
fuente
1

Aquí hay una sugerencia alternativa:
el OP está tratando de protegerse contra el caso en el que alguien pasa y se espera intuna enumeración . O, más probablemente, cuando alguien ha vinculado una biblioteca antigua con un programa más nuevo utilizando un encabezado más nuevo con más casos.

¿Por qué no cambiar el interruptor para manejar el intcaso? Agregar un reparto delante del valor en el interruptor elimina la advertencia e incluso proporciona un cierto indicio sobre por qué existe el valor predeterminado.

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

Me parece mucho menos objetable que assert()probar cada uno de los valores posibles o incluso suponer que el rango de valores de enumeración está bien ordenado para que funcione una prueba más simple. Esta es solo una forma fea de hacer lo que el predeterminado hace con precisión y belleza.

Ricardo
fuente
1
esta publicación es bastante difícil de leer (muro de texto). ¿Te importaría editarlo en una mejor forma?
mosquito