[NFC] Clean up pick<HeapType>() calls in fuzzer (v2) (#7475)

* Add some assertions on features being properly enabled.
* Simplify code to avoid FeatureOptions when unnecessary (feature known to be enabled).
* Add comments on the definition of upTo/oneIn of zero.
* Add a FeatureOptions overload to handle an option without a feature.

Author: kripken

src/tools/fuzzing/fuzzing.cpp

@@ -3426,13 +3426,9 @@ Expression* TranslateToFuzzReader::makeBasicRef(Type type) {
       // Choose a subtype we can materialize a constant for. We cannot
       // materialize non-nullable refs to func or i31 in global contexts.
       Nullability nullability = getSubType(type.getNullability());
-      auto subtypeOpts = FeatureOptions<HeapType>().add(
-        FeatureSet::ReferenceTypes | FeatureSet::GC,
-        HeapType::i31,
-        HeapType::struct_,
-        HeapType::array);
-      auto subtype = pick(subtypeOpts).getBasic(share);
-      return makeConst(Type(subtype, nullability));
+      assert(wasm.features.hasGC());
+      auto subtype = pick(HeapTypes::i31, HeapTypes::struct_, HeapTypes::array);
+      return makeConst(Type(subtype.getBasic(share), nullability));
     }
     case HeapType::eq: {
       if (!wasm.features.hasGC()) {
@@ -5406,16 +5402,18 @@ HeapType TranslateToFuzzReader::getSubType(HeapType type) {
     switch (type.getBasic(Unshared)) {
       case HeapType::func:
         // TODO: Typed function references.
+        assert(wasm.features.hasReferenceTypes());
         return pick(FeatureOptions<HeapType>()
-                      .add(FeatureSet::ReferenceTypes, HeapType::func)
-                      .add(FeatureSet::GC, HeapType::nofunc))
+                      .add(HeapTypes::func)
+                      .add(FeatureSet::GC, HeapTypes::nofunc))
           .getBasic(share);
       case HeapType::cont:
         return pick(HeapTypes::cont, HeapTypes::nocont).getBasic(share);
       case HeapType::ext: {
+        assert(wasm.features.hasReferenceTypes());
         auto options = FeatureOptions<HeapType>()
-                         .add(FeatureSet::ReferenceTypes, HeapType::ext)
-                         .add(FeatureSet::GC, HeapType::noext);
+                         .add(HeapTypes::ext)
+                         .add(FeatureSet::GC, HeapTypes::noext);
         if (share == Unshared) {
           // Shared strings not yet supported.
           options.add(FeatureSet::Strings, HeapType::string);
@@ -5425,14 +5423,13 @@ HeapType TranslateToFuzzReader::getSubType(HeapType type) {
       case HeapType::any: {
         assert(wasm.features.hasReferenceTypes());
         assert(wasm.features.hasGC());
-        auto options = FeatureOptions<HeapType>().add(FeatureSet::GC,
-                                                      HeapType::any,
-                                                      HeapType::eq,
-                                                      HeapType::i31,
-                                                      HeapType::struct_,
-                                                      HeapType::array,
-                                                      HeapType::none);
-        return pick(options).getBasic(share);
+        return pick(HeapTypes::any,
+                    HeapTypes::eq,
+                    HeapTypes::i31,
+                    HeapTypes::struct_,
+                    HeapTypes::array,
+                    HeapTypes::none)
+          .getBasic(share);
       }
       case HeapType::eq:
         assert(wasm.features.hasReferenceTypes());

src/tools/fuzzing/heap-types.cpp

@@ -424,6 +424,9 @@ struct HeapTypeGeneratorImpl {
           return HeapTypes::none.getBasic(share);
         }
       }
+      // If we had no candidates then the oneIn() above us should have returned
+      // true, since oneIn(0) => true.
+      assert(!candidates.empty());
       return rand.pick(candidates);
     } else {
       // This is not a constructed type, so it must be a basic type.

src/tools/fuzzing/random.h

@@ -22,6 +22,7 @@
 #include <vector>
 
 #include "wasm-features.h"
+#include "wasm-type.h"
 
 namespace wasm {
 
@@ -53,8 +54,9 @@ class Random {
   double getDouble();
 
   // Choose an integer value in [0, x). This doesn't use a perfectly uniform
-  // distribution, but it's fast and reasonable.
+  // distribution, but it's fast and reasonable. upTo(0) is defined as 0.
   uint32_t upTo(uint32_t x);
+  // Returns true with probability 1 in x. oneIn(0) is defined as true.
   bool oneIn(uint32_t x) { return upTo(x) == 0; }
 
   // Apply upTo twice, generating a skewed distribution towards
@@ -114,6 +116,23 @@ class Random {
 #endif
 
   template<typename T> struct FeatureOptions {
+    FeatureOptions<T>& add(HeapType::BasicHeapType option) {
+      // Using FeatureOptions with BasicHeapTypes is risky as BasicHeapType
+      // is an enum, which can convert into FeatureSet implicitly (which would
+      // then be ambiguous with add(FeatureSet) below). Use HeapType instead.
+      //
+      // Use a weird static assert on something other than |false| because of
+      // https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2023/p2593r1.html
+      // (older compilers may error without this workaround).
+      static_assert(sizeof(T) == 0);
+    }
+
+    // An option without a feature is applied in all cases.
+    FeatureOptions<T>& add(T option) {
+      options[FeatureSet::MVP].push_back(option);
+      return *this;
+    }
+
     template<typename... Ts>
     FeatureOptions<T>& add(FeatureSet feature, T option, Ts... rest) {
       options[feature].push_back(option);