Warum warnt mich Clang / LLVM vor der Verwendung von default in einer switch-Anweisung, in der alle aufgezählten Fälle behandelt werden?

34

Betrachten Sie die folgende enum- und switch-Anweisung:

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

Ich bin ein Objective-C-Programmierer, aber ich habe dies für ein breiteres Publikum in reinem C geschrieben.

Clang / LLVM 4.1 mit -Alles warnt mich in der Standardzeile:

Standardbeschriftung im Schalter, die alle Aufzählungswerte abdeckt

Jetzt kann ich irgendwie nachvollziehen, warum dies so ist: In einer perfekten Welt würden die einzigen Werte, die in das Argument theMaskeingegeben werden, in der Aufzählung stehen, sodass keine Standardwerte erforderlich sind. Aber was ist, wenn ein Hack auftaucht und ein nicht initialisiertes Int in meine schöne Funktion wirft? Meine Funktion wird als Ablage in der Bibliothek bereitgestellt, und ich habe keine Kontrolle darüber, was dort hineingelangen könnte. Verwenden defaultist eine sehr ordentliche Art, damit umzugehen.

Warum halten die LLVM-Götter dieses Verhalten für ihres teuflischen Geräts unwürdig? Sollte ich dem eine if-Anweisung voranstellen, um das Argument zu überprüfen?

Swizzlr
quelle
1
Ich sollte sagen, mein Grund für -Alles ist, mich selbst zu einem besseren Programmierer zu machen. Wie der NSHipster sagt : "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
-Weverythingkann nützlich sein, aber seien Sie vorsichtig, wenn Sie Ihren Code zu stark verändern, um damit umzugehen. Einige dieser Warnungen sind nicht nur wertlos, sondern auch kontraproduktiv und sollten am besten deaktiviert werden. (In der Tat, das ist der Anwendungsfall für -Weverything: Beginnen Sie damit und deaktivieren Sie, was keinen Sinn ergibt.)
Steven Fisher
Könnten Sie die Warnmeldung für die Nachwelt etwas näher erläutern? Normalerweise steckt noch mehr dahinter.
Steven Fisher
Nach dem Lesen der Antworten bevorzuge ich immer noch Ihre ursprüngliche Lösung. Verwendung der Standardaussage IMHO ist besser als die in den Antworten angegebenen Alternativen. Nur eine kleine Anmerkung: Die Antworten sind wirklich gut und informativ. Sie zeigen auf coole Weise, wie das Problem gelöst werden kann.
bbaja42
@StevenFisher Das war die gesamte Warnung. Und wie Killian betonte, würde es die Möglichkeit eröffnen, gültige Werte an die Standardimplementierung zu übergeben, wenn ich meine Enumeration später ändere. Es scheint ein gutes Designmuster zu sein (wenn es so ist).
Swizzlr

Antworten:

31

Hier ist eine Version, die weder unter der Meldung des Problem-Clangs noch unter der Version leidet, vor der Sie sich schützen:

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

Killian hat bereits erklärt, warum Clang die Warnung ausgibt: Wenn Sie die Aufzählung verlängern, fallen Sie in den Standardfall, der wahrscheinlich nicht Ihren Wünschen entspricht. Das Richtige ist, den Standardfall zu entfernen und Warnungen für nicht behandelte Bedingungen abzurufen .

Jetzt befürchten Sie, dass jemand Ihre Funktion mit einem Wert aufrufen könnte, der außerhalb der Aufzählung liegt. Das hört sich so an, als würde die Funktionsvoraussetzung nicht erfüllt: Es ist dokumentiert, dass ein Wert von der testingMaskAufzählung erwartet wird, aber der Programmierer hat etwas anderes übergeben. Machen Sie also einen Programmierfehler mit assert()(oder NSCAssert()wie Sie sagten, Sie verwenden Objective-C). Lassen Sie Ihr Programm mit einer Meldung abstürzen, die erklärt, dass der Programmierer es falsch macht, wenn der Programmierer es falsch macht.


quelle
6
+1. Als kleine Modifikation bevorzuge ich es, jeden Fall zu haben return;und einen assert(false);am Ende hinzuzufügen (anstatt mich selbst zu wiederholen, indem ich die gesetzlichen Aufzählungen in einem Anfangsbuchstaben assert()und in dem aufführe switch).
Josh Kelley
1
Was ist, wenn die falsche Enumeration nicht von einem dummen Programmierer, sondern von einem Speicherfehler übergeben wird? Wenn der Fahrer die Pause seines Toyota drückt und ein Fehler die Aufzählung beschädigt hat, sollte das Pausenbehandlungsprogramm abstürzen und brennen, und dem Fahrer sollte ein Text angezeigt werden: "Der Programmierer macht es falsch, schlechter Programmierer! ". Ich sehe nicht ganz ein, wie das dem Benutzer helfen wird, bevor er über den Rand einer Klippe fährt.
2
@Lundin ist das, was das OP tut, oder ist das ein sinnloser theoretischer Randfall, den Sie gerade konstruiert haben? Ungeachtet dessen ist ein "Memory Corruption Bug" [i] ein Programmiererfehler, [ii] ist nichts, von dem aus Sie auf sinnvolle Weise fortfahren können (zumindest nicht mit den in der Frage angegebenen Anforderungen).
1
@ AbrahamLee Ich sage nur, dass "sich hinlegen und sterben" nicht unbedingt die beste Option ist, wenn Sie einen unerwarteten Fehler entdecken.
1
Es gibt das neue Problem, dass die Behauptung und die Fälle möglicherweise versehentlich nicht mehr synchronisiert werden.
user253751
9

Ein defaultEtikett hier ist ein Indikator dafür, dass Sie sich nicht sicher sind, was Sie erwarten. Da Sie alle möglichen enumWerte explizit ausgeschöpft haben, defaultkann das nicht ausgeführt werden, und Sie brauchen es auch nicht, um sich vor zukünftigen Änderungen zu schützen, denn wenn Sie das erweitern enum, würde das Konstrukt bereits eine Warnung erzeugen.

Der Compiler merkt also, dass Sie alle Grundlagen abgedeckt haben, denkt aber anscheinend, dass Sie dies nicht getan haben, und das ist immer ein schlechtes Zeichen. Indem switchSie das Formular mit minimalem Aufwand in das erwartete Format ändern , zeigen Sie dem Compiler, dass Sie anscheinend genau das tun, was Sie tatsächlich tun, und Sie wissen es.

Kilian Foth
quelle
1
Aber mein Anliegen ist eine schmutzige Variable, die sich davor hütet (wie ich schon sagte, ein nicht initialisiertes int). Es scheint mir, dass es in einer solchen Situation möglich ist, dass die switch-Anweisung in Verzug gerät.
Swizzlr
Ich kenne die Definition für Objective-C nicht auswendig, aber ich ging davon aus, dass der Compiler eine typdef-Aufzählung erzwingen würde. Mit anderen Worten, ein "nicht initialisierter" Wert kann nur dann in die Methode eingegeben werden, wenn Ihr Programm bereits ein undefiniertes Verhalten aufweist, und ich halte es für völlig vernünftig, dass ein Compiler dies nicht berücksichtigt.
Kilian Foth
3
@KilianFoth nein, das sind sie nicht. Objective-C-Aufzählungen sind C-Aufzählungen, keine Java-Aufzählungen. Jeder Wert aus dem zugrundeliegenden integralen Typ konnte in dem Funktionsargument vorhanden sein.
2
Außerdem können in der Laufzeit Aufzählungen aus ganzen Zahlen festgelegt werden. Sie können also jeden erdenklichen Wert enthalten.
OP ist richtig. testingMask m; myFunction (m); würde höchstwahrscheinlich den Standardfall treffen.
Matthew James Briggs
4

Clang ist verwirrt und hat eine Standardaussage, dass es eine sehr gute Übung gibt. Sie wird als defensive Programmierung bezeichnet und gilt als gute Programmierpraxis (1). Es wird häufig in unternehmenskritischen Systemen verwendet, möglicherweise jedoch nicht in der Desktop-Programmierung.

Der Zweck der defensiven Programmierung besteht darin, unerwartete Fehler aufzufangen, die theoretisch niemals auftreten würden. Ein solcher unerwarteter Fehler ist nicht notwendigerweise der Programmierer, der der Funktion eine falsche Eingabe oder sogar einen "bösen Hack" gibt. Es ist wahrscheinlicher, dass dies durch eine beschädigte Variable verursacht wurde: Pufferüberläufe, Stapelüberlauf, außer Kontrolle geratener Code und ähnliche Fehler, die nicht mit Ihrer Funktion zusammenhängen, könnten dies verursachen. Bei eingebetteten Systemen können sich Variablen möglicherweise aufgrund von EMI ändern, insbesondere wenn Sie externe RAM-Schaltkreise verwenden.

Was Sie in die Standardanweisung schreiben sollten ... Wenn Sie den Verdacht haben, dass das Programm am Ende durcheinander geraten ist, benötigen Sie eine Art Fehlerbehandlung. In vielen Fällen können Sie wahrscheinlich einfach eine leere Anweisung mit einem Kommentar hinzufügen: "Unerwartet, aber egal" usw., um zu zeigen, dass Sie über die unwahrscheinliche Situation nachgedacht haben.


(1) MISRA-C: 2004 15.3.


quelle
1
Bitte beachten Sie, dass der durchschnittliche PC-Programmierer das Konzept der defensiven Programmierung normalerweise als völlig fremd empfindet, da seine Sichtweise auf ein Programm eine abstrakte Utopie ist, bei der in der Praxis nichts schief gehen kann, was theoretisch falsch sein kann. Daher erhalten Sie sehr unterschiedliche Antworten auf Ihre Frage, je nachdem, wen Sie fragen.
3
Clang ist nicht verwirrt; Es wurde ausdrücklich darum gebeten, alle Warnungen anzugeben, einschließlich solcher, die nicht immer nützlich sind, wie z. B. stilistische Warnungen. (Ich persönlich stimme dieser Warnung zu, weil ich keine Standardeinstellungen für meine Enum-Schalter möchte. Wenn ich sie habe, wird keine Warnung angezeigt, wenn ich einen Enum-Wert hinzufüge und nicht damit umgehe. Aus diesem Grund behandle ich immer die Fall mit schlechtem Wert außerhalb der Aufzählung.)
Jens Ayton
2
@ JensAyton Es ist verwirrt. Alle professionellen statischen Analysetools sowie alle MISRA-kompatiblen Compiler geben eine Warnung aus, wenn eine switch-Anweisung keine Standardanweisung enthält. Probieren Sie es selbst aus.
4
Die Codierung nach MISRA-C ist nicht die einzige gültige Verwendung für einen C-Compiler und -Weverythingaktiviert auch hier jede Warnung und nicht eine Auswahl, die für einen bestimmten Anwendungsfall geeignet ist. Nicht deinem Geschmack zu entsprechen ist nicht das Gleiche wie Verwirrung.
Jens Ayton
2
@Lundin: Ich bin mir ziemlich sicher, dass das Festhalten an MISRA-C Programme nicht auf magische Weise sicher und fehlerfrei macht ... und ich bin mir ebenso sicher, dass es jede Menge sicheren, fehlerfreien C-Code von Leuten gibt, die es noch nie gegeben haben habe von MISRA gehört. Tatsächlich ist es kaum mehr als die Meinung von jemandem (populär, aber nicht unbestritten), und es gibt Leute, die einige seiner Regeln für willkürlich und manchmal sogar schädlich halten, außerhalb des Kontexts, für den sie entworfen wurden.
cHao
4

Besser noch:

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

Dies ist weniger fehleranfällig beim Hinzufügen von Elementen zur Aufzählung. Sie können den Test für> = 0 überspringen, wenn Sie Ihre Enum-Werte vorzeichenlos machen. Diese Methode funktioniert nur, wenn Ihre Aufzählungswerte keine Lücken aufweisen. Dies ist jedoch häufig der Fall.

Steve Weller
quelle
2
Dies verschmutzt den Typ mit Werten, die der Typ einfach nicht enthalten soll. Es hat Ähnlichkeiten mit der guten alten WTF hier: thedailywtf.com/articles/What_Is_Truth_0x3f_
Martin Sugioarto
4

Aber was ist, wenn ein Hack auftaucht und ein nicht initialisiertes Int in meine schöne Funktion wirft?

Dann bekommst du Undefiniertes Verhalten und dein defaultWille ist bedeutungslos. Sie können möglicherweise nichts tun, um dies zu verbessern.

Lass mich klarer sein. Sobald jemand eine nicht initialisierte intFunktion in Ihre Funktion übergibt , ist dies undefiniertes Verhalten. Ihre Funktion könnte das Problem des Anhaltens lösen, und es spielt keine Rolle. Es ist UB. Es gibt nichts, was Sie tun können, wenn UB einmal aufgerufen wurde.

DeadMG
quelle
3
Wie wäre es, wenn Sie es protokollieren oder stattdessen eine Ausnahme auslösen, wenn Sie es unbemerkt ignorieren?
Jgauffin
3
In der Tat kann eine Menge getan werden, z. B. ... eine Standardanweisung zum Switch hinzuzufügen und den Fehler ordnungsgemäß zu behandeln. Dies wird als defensive Programmierung bezeichnet . Es schützt nicht nur dumme Programmierer vor falschen Eingaben, sondern auch vor verschiedenen Fehlern, die durch Pufferüberlauf, Stapelüberlauf, Runaway-Code usw. verursacht werden.
1
Nein, es wird nichts aushalten. Es ist UB. Das Programm hat UB, sobald die Funktion eingegeben wird, und der Funktionskörper kann nichts dagegen tun.
DeadMG
2
@DeadMG Eine Enumeration kann einen beliebigen Wert haben, den der entsprechende Integer-Typ in der Implementierung möglicherweise hat. Es gibt nichts Unbestimmtes. Der Zugriff auf den Inhalt einer nicht initialisierten Auto-Variablen ist in der Tat UB, aber der "böse Hack" (was eher ein Fehler ist) kann der Funktion auch einen genau definierten Wert zuweisen, auch wenn er keiner der Werte ist in der Enum-Deklaration aufgeführt.
2

Die Standardanweisung würde nicht unbedingt helfen. Wenn sich der Schalter über einer Aufzählung befindet, führt jeder Wert, der nicht in der Aufzählung definiert ist, undefiniertes Verhalten aus.

Nach allem, was Sie wissen, kann der Compiler diesen Switch (mit der Standardeinstellung) wie folgt kompilieren:

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

Sobald Sie undefiniertes Verhalten auslösen, gibt es kein Zurück mehr.

Mich selber
quelle
2
Erstens handelt es sich um einen nicht angegebenen Wert , kein undefiniertes Verhalten. Dies bedeutet, dass der Compiler möglicherweise einen anderen Wert annimmt, der bequemer ist, aber den Mond möglicherweise nicht in grünen Käse usw. verwandelt. Zweitens gilt dies, soweit ich das beurteilen kann, nur für C ++. In C entspricht der Wertebereich eines enumTyps dem Wertebereich seines zugrunde liegenden Integer-Typs (der implementierungsdefiniert ist und daher selbstkonsistent sein muss).
Jens Ayton
1
Eine Instanz einer Aufzählungsvariablen und eine Aufzählungskonstante müssen jedoch nicht unbedingt die gleiche Breite und Vorzeichen haben. Sie sind beide implizit definiert. Ich bin mir jedoch ziemlich sicher, dass alle Aufzählungen in C genauso ausgewertet werden wie der entsprechende Integer-Typ, sodass dort kein undefiniertes oder gar nicht angegebenes Verhalten vorliegt.
2

Ich bevorzuge es auch, default:in allen Fällen eine zu haben . Ich bin wie immer zu spät zur Party, aber ... ein paar andere Gedanken, die ich oben nicht gesehen habe:

  • Die spezielle Warnung (oder der Fehler, wenn auch geworfen wird -Werror) kommt von -Wcovered-switch-default(von, -Weverythingaber nicht von -Wall). Wenn Ihre moralische Flexibilität es Ihnen erlaubt, bestimmte Warnungen auszuschalten (z. B. bestimmte Dinge aus -Walloder herauszuschneiden -Weverything), erwägen Sie, sie zu werfen -Wno-covered-switch-default(oder zu -Wno-error=covered-switch-defaultverwenden -Werror), und im Allgemeinen -Wno-...für andere Warnungen, die Sie als unangenehm empfinden.
  • Für gcc(und allgemeineres Verhalten in clang), konsultiert gccManpage -Wswitch, -Wswitch-enum, -Wswitch-defaultfür (verschiedene) Verhalten in ähnlichen Situationen von Aufzählungstypen in switch - Anweisungen.
  • Ich mag diese Warnung auch nicht im Konzept, und ich mag ihren Wortlaut nicht; Für mich deuten die Wörter aus der Warnung ("Standardbeschriftung ... deckt alle ... Werte ab") darauf hin, dass der default:Fall immer ausgeführt wird, wie z

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

    Auf dem ersten Lesung, ist das , was ich dachte , dass Sie in ausgeführt wurden - , dass Ihr default:Fall immer dann ausgeführt werden würde , weil es keine gibt break;oder return;oder ähnliches. Dieses Konzept ähnelt (meinem Ohr) anderen (wenn auch gelegentlich hilfreichen) Kommentaren im Nanny-Stil, aus denen es hervorgeht clang. Wenn foo == 1, werden beide Funktionen ausgeführt; Ihr Code oben hat dieses Verhalten. Das heißt, nicht ausbrechen, nur wenn Sie möglicherweise weiterhin Code aus nachfolgenden Fällen ausführen möchten! Dies scheint jedoch nicht Ihr Problem zu sein.

Auf die Gefahr, pedantisch zu sein, einige andere Gedanken zur Vollständigkeit:

  • Ich denke jedoch, dass dieses Verhalten (mehr) mit aggressiven Typüberprüfungen in anderen Sprachen oder Compilern vereinbar ist. Wenn, wie Sie theoretisieren, einige reprobate nicht versuchen , eine weitergeben intoder etwas zu dieser Funktion, die ausdrücklich sind eigene Art beabsichtigt , zu konsumieren, sollten Sie Ihre Compiler Sie schützen gleich gut in dieser Situation mit einer aggressiven Warnung oder einen Fehler. ABER das tut es nicht! (Das heißt, es scheint, dass zumindest gccund clangnicht die enumTypüberprüfung, aber ich höre, dass dies der iccFall ist ). Da Sie nicht bekommen Typ -Sicherheit, könnten Sie erhalten Wert -safety wie oben erörtert. Andernfalls sollten Sie, wie in TFA vorgeschlagen, ein structoder etwas in Betracht ziehen , das Typensicherheit bietet.
  • Eine andere Problemumgehung könnte darin bestehen, einen neuen "Wert" in Ihrem enumNamen zu erstellen MaskValueIllegalund diesen casein Ihrem Namen nicht zu unterstützen switch. Das würde gegessen von default:(zusätzlich zu jedem anderen verrückten Wert)

Es lebe die defensive Codierung!

hoc_age
quelle
1

Hier ist ein alternativer Vorschlag:
Das OP versucht, sich gegen den Fall zu schützen, dass jemand in einem Fall vorbeikommt, in intdem eine Aufzählung erwartet wird. Oder eher dort, wo jemand eine alte Bibliothek mit einem neueren Programm verknüpft hat, indem er einen neueren Header mit mehr Fällen verwendet.

Warum nicht den Schalter wechseln, um das intGehäuse zu handhaben ? Das Hinzufügen eines Cast vor dem Wert in der Option beseitigt die Warnung und gibt sogar einen Hinweis darauf, warum die Standardeinstellung existiert.

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

Ich finde das viel weniger verwerflich als das assert()Testen jedes der möglichen Werte oder sogar die Annahme, dass der Bereich der Aufzählungswerte geordnet ist, damit ein einfacherer Test funktioniert. Dies ist nur eine hässliche Methode, um genau und schön das zu tun, was default tut.

Richard
quelle
1
Dieser Beitrag ist ziemlich schwer zu lesen (Textwand). Hätten Sie etwas dagegen bearbeiten sie in eine bessere Form ing?
gnat