[NFC] Refactor Feature::All to match FeatureSet.setAll() (#4557)

As we recently noted in #4555, that Feature::All and FeatureSet.setAll()
are different is potentially confusing...

I think the best thing is to make them identical. This does that, and adds a
new Feature::AllPossible which is everything possible and not just the
set of all features that are enabled by -all.

This undoes part of #4555 as now the old/simpler code works properly.

2 files changed, 13 insertions, 9 deletions

diff --git a/src/tools/wasm-shell.cpp b/src/tools/wasm-shell.cpp
index 2d0fbf14c..0de93c69e 100644
--- a/src/tools/wasm-shell.cpp
+++ b/src/tools/wasm-shell.cpp
@@ -134,7 +134,7 @@ protected:
     lastModule = module->name;
     builders[moduleName] = builder;
     modules[moduleName].swap(module);
-    modules[moduleName]->features.setAll();
+    modules[moduleName]->features = FeatureSet::All;
     bool valid = WasmValidator().validate(*modules[moduleName]);
     if (!valid) {
       std::cout << *modules[moduleName] << '\n';
@@ -237,7 +237,7 @@ protected:
 
   void parseModuleAssertion(Element& s) {
     Module wasm;
-    wasm.features.setAll();
+    wasm.features = FeatureSet::All;
     std::unique_ptr<SExpressionWasmBuilder> builder;
     auto id = s[0]->str();
 
@@ -358,7 +358,7 @@ protected:
       "memory", spectest->memory.name, ExternalKind::Memory));
 
     modules["spectest"].swap(spectest);
-    modules["spectest"]->features.setAll();
+    modules["spectest"]->features = FeatureSet::All;
     instantiate(modules["spectest"].get());
     linkedInstances["spectest"] = instances["spectest"];
     // print_* functions are handled separately, no need to define here.
diff --git a/src/wasm-features.h b/src/wasm-features.h
index df4397e78..64d831ecc 100644
--- a/src/wasm-features.h
+++ b/src/wasm-features.h
@@ -43,7 +43,11 @@ struct FeatureSet {
     GCNNLocals = 1 << 13,
     RelaxedSIMD = 1 << 14,
     ExtendedConst = 1 << 15,
-    All = (1 << 16) - 1
+    // GCNNLocals are opt-in: merely asking for "All" does not apply them. To
+    // get all possible values use AllPossible. See setAll() below for more
+    // details.
+    All = ((1 << 16) - 1) & ~GCNNLocals,
+    AllPossible = (1 << 16) - 1,
   };
 
   static std::string toString(Feature f) {
@@ -88,7 +92,7 @@ struct FeatureSet {
   std::string toString() const {
     std::string ret;
     uint32_t x = 1;
-    while (x & Feature::All) {
+    while (x & Feature::AllPossible) {
       if (features & x) {
         if (!ret.empty()) {
           ret += ", ";
@@ -126,7 +130,7 @@ struct FeatureSet {
   bool hasGCNNLocals() const { return (features & GCNNLocals) != 0; }
   bool hasRelaxedSIMD() const { return (features & RelaxedSIMD) != 0; }
   bool hasExtendedConst() const { return (features & ExtendedConst) != 0; }
-  bool hasAll() const { return (features & All) != 0; }
+  bool hasAll() const { return (features & AllPossible) != 0; }
 
   void set(FeatureSet f, bool v = true) {
     features = v ? (features | f) : (features & ~f);
@@ -161,17 +165,17 @@ struct FeatureSet {
     // --enable-gc-nn-locals -all work (that is, if we enable the feature,
     // then -all does not disable it; it simply does not enable it by itself).
     auto oldGCNNLocals = hasGCNNLocals();
-    features = All;
+    features = AllPossible;
     setGCNNLocals(oldGCNNLocals);
   }
 
   void enable(const FeatureSet& other) { features |= other.features; }
   void disable(const FeatureSet& other) {
-    features = features & ~other.features & All;
+    features = features & ~other.features & AllPossible;
   }
 
   template<typename F> void iterFeatures(F f) const {
-    for (uint32_t feature = MVP + 1; feature < All; feature <<= 1) {
+    for (uint32_t feature = MVP + 1; feature < AllPossible; feature <<= 1) {
       if (has(feature)) {
         f(static_cast<Feature>(feature));
       }