[PATCH] D44427: [WebAssembly] Add export/import for function pointer table

[Nicholas Wilson via Phabricator via llvm-commits]

ncw added inline comments.


================
Comment at: wasm/Driver.cpp:308
   Config->StripDebug = Args.hasArg(OPT_strip_debug);
+  Config->Table = getExpose(Args, OPT_import_table, OPT_export_table);
   errorHandler().Verbose = Args.hasArg(OPT_verbose);
----------------
ruiu wrote:
> Config members are named after their command line options, so this member should be "ImportTable" or "ExportTable" and have a boolean value.
Good point. In this case, it's a tri-state, where the Table is either imported/exported, or neither, and I want to import/export based on the last-specified of those two options.

I'll expand that into two booleans ImportTable and ExportTable, if that's more idiomatic.

Done in rLLD328700.


================
Comment at: wasm/Writer.cpp:42
 static constexpr int kInitialTableOffset = 1;
+static constexpr const char *kFunctionTableName = "__indirect_function_table";
 
----------------
ruiu wrote:
> Does it make sense to add `const` to `constexpr`?
Yes it does (in this case).

`constexpr char *` would be a const pointer to a non-const char.

`constexpr const char *` is equivalent to `const char* const` ie a const pointer to a const char.

Something like `constexpr const int` is silly/redundant, and `constexpr const char *const` would also be redundant.


Repository:
  rLLD LLVM Linker

https://reviews.llvm.org/D44427