Merge pull request #8157 from hercules-ci/switch-bugs

Fix current and future `switch` bugs

local.mk

@@ -1,6 +1,8 @@
 clean-files += Makefile.config
 
-GLOBAL_CXXFLAGS += -Wno-deprecated-declarations
+GLOBAL_CXXFLAGS += -Wno-deprecated-declarations -Werror=switch
+# Allow switch-enum to be overridden for files that do not support it, usually because of dependency headers.
+ERROR_SWITCH_ENUM = -Werror=switch-enum
 
 $(foreach i, config.h $(wildcard src/lib*/*.hh), \
   $(eval $(call install-file-in, $(i), $(includedir)/nix, 0644)))

mk/patterns.mk

@@ -1,10 +1,10 @@
 $(buildprefix)%.o: %.cc
 	@mkdir -p "$(dir $@)"
-	$(trace-cxx) $(CXX) -o $@ -c $< $(CPPFLAGS) $(GLOBAL_CXXFLAGS_PCH) $(GLOBAL_CXXFLAGS) $(CXXFLAGS) $($@_CXXFLAGS) -MMD -MF $(call filename-to-dep, $@) -MP
+	$(trace-cxx) $(CXX) -o $@ -c $< $(CPPFLAGS) $(GLOBAL_CXXFLAGS_PCH) $(GLOBAL_CXXFLAGS) $(CXXFLAGS) $($@_CXXFLAGS) $(ERROR_SWITCH_ENUM) -MMD -MF $(call filename-to-dep, $@) -MP
 
 $(buildprefix)%.o: %.cpp
 	@mkdir -p "$(dir $@)"
-	$(trace-cxx) $(CXX) -o $@ -c $< $(CPPFLAGS) $(GLOBAL_CXXFLAGS_PCH) $(GLOBAL_CXXFLAGS) $(CXXFLAGS) $($@_CXXFLAGS) -MMD -MF $(call filename-to-dep, $@) -MP
+	$(trace-cxx) $(CXX) -o $@ -c $< $(CPPFLAGS) $(GLOBAL_CXXFLAGS_PCH) $(GLOBAL_CXXFLAGS) $(CXXFLAGS) $($@_CXXFLAGS) $(ERROR_SWITCH_ENUM) -MMD -MF $(call filename-to-dep, $@) -MP
 
 $(buildprefix)%.o: %.c
 	@mkdir -p "$(dir $@)"

src/libcmd/repl.cc

@@ -1024,6 +1024,8 @@ std::ostream & NixRepl::printValue(std::ostream & str, Value & v, unsigned int m
         str << v.fpoint;
         break;
 
+    case nThunk:
+    case nExternal:
     default:
         str << ANSI_RED "«unknown»" ANSI_NORMAL;
         break;

src/libexpr/eval.cc

@@ -239,6 +239,9 @@ std::string_view showType(ValueType type)
 
 std::string showType(const Value & v)
 {
+    // Allow selecting a subset of enum values
+    #pragma GCC diagnostic push
+    #pragma GCC diagnostic ignored "-Wswitch-enum"
     switch (v.internalType) {
         case tString: return v.string.context ? "a string with context" : "a string";
         case tPrimOp:
@@ -252,16 +255,21 @@ std::string showType(const Value & v)
     default:
         return std::string(showType(v.type()));
     }
+    #pragma GCC diagnostic pop
 }
 
 PosIdx Value::determinePos(const PosIdx pos) const
 {
+    // Allow selecting a subset of enum values
+    #pragma GCC diagnostic push
+    #pragma GCC diagnostic ignored "-Wswitch-enum"
     switch (internalType) {
         case tAttrs: return attrs->pos;
         case tLambda: return lambda.fun->pos;
         case tApp: return app.left->determinePos(pos);
         default: return pos;
     }
+    #pragma GCC diagnostic pop
 }
 
 bool Value::isTrivial() const
@@ -2337,6 +2345,7 @@ bool EvalState::eqValues(Value & v1, Value & v2, const PosIdx pos, std::string_v
         case nFloat:
             return v1.fpoint == v2.fpoint;
 
+        case nThunk: // Must not be left by forceValue
         default:
             error("cannot compare %1% with %2%", showType(v1), showType(v2)).withTrace(pos, errorCtx).debugThrow<EvalError>();
     }

src/libexpr/flake/flake.cc

@@ -125,6 +125,9 @@ static FlakeInput parseFlakeInput(EvalState & state,
                 follows.insert(follows.begin(), lockRootPath.begin(), lockRootPath.end());
                 input.follows = follows;
             } else {
+                // Allow selecting a subset of enum values
+                #pragma GCC diagnostic push
+                #pragma GCC diagnostic ignored "-Wswitch-enum"
                 switch (attr.value->type()) {
                     case nString:
                         attrs.emplace(state.symbols[attr.name], attr.value->string.s);
@@ -139,6 +142,7 @@ static FlakeInput parseFlakeInput(EvalState & state,
                         throw TypeError("flake input attribute '%s' is %s while a string, Boolean, or integer is expected",
                             state.symbols[attr.name], showType(*attr.value));
                 }
+                #pragma GCC diagnostic pop
             }
         } catch (Error & e) {
             e.addTrace(

src/libexpr/local.mk

@@ -46,3 +46,5 @@ $(foreach i, $(wildcard src/libexpr/flake/*.hh), \
 $(d)/primops.cc: $(d)/imported-drv-to-derivation.nix.gen.hh $(d)/primops/derivation.nix.gen.hh $(d)/fetchurl.nix.gen.hh
 
 $(d)/flake/flake.cc: $(d)/flake/call-flake.nix.gen.hh
+
+src/libexpr/primops/fromTOML.o:	ERROR_SWITCH_ENUM =

src/libexpr/primops.cc

@@ -577,6 +577,9 @@ struct CompareValues
                 return v1->integer < v2->fpoint;
             if (v1->type() != v2->type())
                 state.error("cannot compare %s with %s", showType(*v1), showType(*v2)).debugThrow<EvalError>();
+            // Allow selecting a subset of enum values
+            #pragma GCC diagnostic push
+            #pragma GCC diagnostic ignored "-Wswitch-enum"
             switch (v1->type()) {
                 case nInt:
                     return v1->integer < v2->integer;
@@ -599,6 +602,7 @@ struct CompareValues
                     }
                 default:
                     state.error("cannot compare %s with %s; values of that type are incomparable", showType(*v1), showType(*v2)).debugThrow<EvalError>();
+            #pragma GCC diagnostic pop
             }
         } catch (Error & e) {
             if (!errorCtx.empty())

src/libstore/build-result.hh

@@ -53,6 +53,7 @@ struct BuildResult
                 case LogLimitExceeded: return "LogLimitExceeded";
                 case NotDeterministic: return "NotDeterministic";
                 case ResolvesToAlreadyValid: return "ResolvesToAlreadyValid";
+                case NoSubstituters: return "NoSubstituters";
                 default: return "Unknown";
             };
         }();

src/libstore/filetransfer.cc

@@ -407,6 +407,10 @@ struct curlFileTransfer : public FileTransfer
                     err = Misc;
                 } else {
                     // Don't bother retrying on certain cURL errors either
+
+                    // Allow selecting a subset of enum values
+                    #pragma GCC diagnostic push
+                    #pragma GCC diagnostic ignored "-Wswitch-enum"
                     switch (code) {
                         case CURLE_FAILED_INIT:
                         case CURLE_URL_MALFORMAT:
@@ -427,6 +431,7 @@ struct curlFileTransfer : public FileTransfer
                         default: // Shut up warnings
                             break;
                     }
+                    #pragma GCC diagnostic pop
                 }
 
                 attempt++;

src/libstore/nar-accessor.cc

@@ -275,6 +275,7 @@ json listNar(ref<FSAccessor> accessor, const Path & path, bool recurse)
         obj["type"] = "symlink";
         obj["target"] = accessor->readLink(path);
         break;
+    case FSAccessor::Type::tMissing:
     default:
         throw Error("path '%s' does not exist in NAR", path);
     }

src/libutil/logging.cc

@@ -65,9 +65,10 @@ public:
             switch (lvl) {
             case lvlError: c = '3'; break;
             case lvlWarn: c = '4'; break;
-            case lvlInfo: c = '5'; break;
+            case lvlNotice: case lvlInfo: c = '5'; break;
             case lvlTalkative: case lvlChatty: c = '6'; break;
-            default: c = '7';
+            case lvlDebug: case lvlVomit: c = '7';
+            default: c = '7'; break; // should not happen, and missing enum case is reported by -Werror=switch-enum
             }
             prefix = std::string("<") + c + ">";
         }

src/nix-store/nix-store.cc

@@ -277,17 +277,17 @@ static void printTree(const StorePath & path,
 static void opQuery(Strings opFlags, Strings opArgs)
 {
     enum QueryType
-        { qDefault, qOutputs, qRequisites, qReferences, qReferrers
+        { qOutputs, qRequisites, qReferences, qReferrers
         , qReferrersClosure, qDeriver, qBinding, qHash, qSize
         , qTree, qGraph, qGraphML, qResolve, qRoots };
-    QueryType query = qDefault;
+    std::optional<QueryType> query;
     bool useOutput = false;
     bool includeOutputs = false;
     bool forceRealise = false;
     std::string bindingName;
 
     for (auto & i : opFlags) {
-        QueryType prev = query;
+        std::optional<QueryType> prev = query;
         if (i == "--outputs") query = qOutputs;
         else if (i == "--requisites" || i == "-R") query = qRequisites;
         else if (i == "--references") query = qReferences;
@@ -312,15 +312,15 @@ static void opQuery(Strings opFlags, Strings opArgs)
         else if (i == "--force-realise" || i == "--force-realize" || i == "-f") forceRealise = true;
         else if (i == "--include-outputs") includeOutputs = true;
         else throw UsageError("unknown flag '%1%'", i);
-        if (prev != qDefault && prev != query)
+        if (prev && prev != query)
             throw UsageError("query type '%1%' conflicts with earlier flag", i);
     }
 
-    if (query == qDefault) query = qOutputs;
+    if (!query) query = qOutputs;
 
     RunPager pager;
 
-    switch (query) {
+    switch (*query) {
 
         case qOutputs: {
             for (auto & i : opArgs) {