[wasm] Clean up wasm-feature-flags.h

This CL removes the SEPERATOR from the FOREACH_WASM_FEATURE_FLAG macro,
which was ugly, and fortunately also unnecessary.

R=jkummerow@chromium.org

Bug: v8:9396
Change-Id: I455589331f84a011a231d6897d03e8ab0b492b45
Reviewed-on: https://chromium-review.googlesource.com/c/v8/v8/+/1738847Reviewed-by: Jakob Kummerow <jkummerow@chromium.org>
Commit-Queue: Andreas Haas <ahaas@chromium.org>
Cr-Commit-Position: refs/heads/master@{#63095}

src/flags/flag-definitions.h

@@ -708,13 +708,11 @@ DEFINE_STRING(dump_wasm_module_path, nullptr,
 // for configurability.
 #include "src/wasm/wasm-feature-flags.h"
 
-#define SPACE
 #define DECL_WASM_FLAG(feat, desc, val)      \
   DEFINE_BOOL(experimental_wasm_##feat, val, \
               "enable prototype " desc " for wasm")
-FOREACH_WASM_FEATURE_FLAG(DECL_WASM_FLAG, SPACE)
+FOREACH_WASM_FEATURE_FLAG(DECL_WASM_FLAG)
 #undef DECL_WASM_FLAG
-#undef SPACE
 
 DEFINE_BOOL(wasm_opt, false, "enable wasm optimization")
 DEFINE_BOOL(wasm_no_bounds_checks, false,

src/wasm/wasm-feature-flags.h

@@ -5,29 +5,17 @@
 #ifndef V8_WASM_WASM_FEATURE_FLAGS_H_
 #define V8_WASM_WASM_FEATURE_FLAGS_H_
 
-// The SEPARATOR argument allows generating proper comma-separated lists.
-#define FOREACH_WASM_FEATURE_FLAG(V, SEPARATOR)                       \
+#define FOREACH_WASM_FEATURE_FLAG(V)                                  \
   V(mv, "multi-value support", false)                                 \
-  SEPARATOR                                                           \
   V(eh, "exception handling opcodes", false)                          \
-  SEPARATOR                                                           \
   V(se, "sign extension opcodes", true)                               \
-  SEPARATOR                                                           \
   V(sat_f2i_conversions, "saturating float conversion opcodes", true) \
-  SEPARATOR                                                           \
   V(threads, "thread opcodes", false)                                 \
-  SEPARATOR                                                           \
   V(simd, "SIMD opcodes", false)                                      \
-  SEPARATOR                                                           \
   V(anyref, "anyref opcodes", false)                                  \
-  SEPARATOR                                                           \
   V(bigint, "JS BigInt support", false)                               \
-  SEPARATOR                                                           \
   V(bulk_memory, "bulk memory opcodes", true)                         \
-  SEPARATOR                                                           \
   V(return_call, "return call opcodes", false)                        \
-  SEPARATOR                                                           \
   V(type_reflection, "wasm type reflection in JS", false)             \
-  SEPARATOR                                                           \
   V(compilation_hints, "compilation hints section", false)
 #endif  // V8_WASM_WASM_FEATURE_FLAGS_H_

src/wasm/wasm-features.cc

@@ -11,17 +11,17 @@ namespace v8 {
 namespace internal {
 namespace wasm {
 
-#define COMMA ,
-#define SPACE
-#define DO_UNION(feat, desc, val) dst->feat |= src.feat;
-#define FLAG_REF(feat, desc, val) FLAG_experimental_wasm_##feat
 
 void UnionFeaturesInto(WasmFeatures* dst, const WasmFeatures& src) {
-  FOREACH_WASM_FEATURE(DO_UNION, SPACE);
+#define DO_UNION(feat, desc, val) dst->feat |= src.feat;
+  FOREACH_WASM_FEATURE(DO_UNION);
+#undef DO_UNION
 }
 
 WasmFeatures WasmFeaturesFromFlags() {
-  return WasmFeatures{FOREACH_WASM_FEATURE(FLAG_REF, COMMA)};
+#define FLAG_REF(feat, desc, val) FLAG_experimental_wasm_##feat,
+  return WasmFeatures(FOREACH_WASM_FEATURE(FLAG_REF){});
+#undef FLAG_REF
 }
 
 WasmFeatures WasmFeaturesFromIsolate(Isolate* isolate) {
@@ -31,10 +31,6 @@ WasmFeatures WasmFeaturesFromIsolate(Isolate* isolate) {
   return features;
 }
 
-#undef DO_UNION
-#undef FLAG_REF
-#undef SPACE
-#undef COMMA
 }  // namespace wasm
 }  // namespace internal
 }  // namespace v8

src/wasm/wasm-features.h

@@ -17,37 +17,50 @@ namespace internal {
 class Isolate;
 namespace wasm {
 
-#define COMMA ,
-#define SPACE
-#define DECL_FIELD(feat, desc, val) bool feat = false;
-#define JUST_TRUE(feat, desc, val) true
-#define JUST_FALSE(feat, desc, val) false
-#define DECL_PARAM(feat, desc, val) bool p##feat
-#define DO_INIT(feat, desc, val) feat(p##feat)
+// This is an empty type to indicate the end of the {WasmFeatures} struct. We
+// use the {end_t} type there to avoid trailing commas that get generated by
+// the macro generators. We considered the following alternatives:
+// * Add "separators" to the {FOREACH_WASM_FEATURE_FLAGS} between entries. This
+//   does not work when we want to have different kinds of flags, e.g. for
+//   experimental, staging, and shipped features.
+// * Use initialization lists, e.g. construct {WasmFeatures} with
+//   "WasmFeatures{true, true, ..., true,}". This solves the comma problem,
+//   because trailing commas are allowed here. However, we cannot
+//   default-initialize the fields of {WasmFeatures} anymore. This seems
+//   error-prone, because default-constructed {WasmFeatures} structs are already
+//   used in the code base.
+// * Avoid the use of {constexpr}. With that we would be more flexible with how
+//   we generate {kAllWasmFeatures} and {kNoWasmFeatures}. These values may be
+//   used in performance-critical code, however, e.g. in the decoder or in the
+//   interpreter.
+struct end_t {};
 
 // Enabled or detected features.
 struct WasmFeatures {
-  FOREACH_WASM_FEATURE(DECL_FIELD, SPACE)
+#define DECL_FIELD(feat, desc, val) bool feat = false;
+  FOREACH_WASM_FEATURE(DECL_FIELD)
+#undef DECL_FIELD
+  // Marker for the end of the list, see the comment at {end_t}.
+  end_t end_;
 
+#define DECL_PARAM(feat, desc, val) bool p##feat,
+#define DO_INIT(feat, desc, val) feat(p##feat),
+  explicit constexpr WasmFeatures(FOREACH_WASM_FEATURE(DECL_PARAM) end_t)
+      : FOREACH_WASM_FEATURE(DO_INIT) end_() {}
+#undef DECL_PARAM
+#undef DO_INIT
   constexpr WasmFeatures() = default;
-
-  explicit constexpr WasmFeatures(FOREACH_WASM_FEATURE(DECL_PARAM, COMMA))
-      : FOREACH_WASM_FEATURE(DO_INIT, COMMA) {}
 };
 
-static constexpr WasmFeatures kAllWasmFeatures{
-    FOREACH_WASM_FEATURE(JUST_TRUE, COMMA)};
-
-static constexpr WasmFeatures kNoWasmFeatures{
-    FOREACH_WASM_FEATURE(JUST_FALSE, COMMA)};
-
+#define JUST_TRUE(feat, desc, val) true,
+static constexpr WasmFeatures kAllWasmFeatures(
+    FOREACH_WASM_FEATURE(JUST_TRUE){});
 #undef JUST_TRUE
+
+#define JUST_FALSE(feat, desc, val) false,
+static constexpr WasmFeatures kNoWasmFeatures(
+    FOREACH_WASM_FEATURE(JUST_FALSE){});
 #undef JUST_FALSE
-#undef DECL_FIELD
-#undef DECL_PARAM
-#undef DO_INIT
-#undef COMMA
-#undef SPACE
 
 static constexpr WasmFeatures kAsmjsWasmFeatures = kNoWasmFeatures;