[PATCH] D62704: [WebAssembly] Improve feature validation error messages

[Thomas Lively via Phabricator via llvm-commits]

tlively added inline comments.


================
Comment at: lld/wasm/Writer.cpp:379
       case WASM_FEATURE_PREFIX_DISALLOWED:
-        Disallowed.insert(Feature.Name);
+        Disallowed.insert({Feature.Name, FileName});
         break;
----------------
sbc100 wrote:
> So in the case the multiple files use a given feature it will show just one them?  (i.e. this insert fails after the first one?)
That's correct. I figured that printing all relevant files could get overwhelming rather quickly and that even when it would be reasonable, we still get 80% of the benefit by printing just one file.


================
Comment at: lld/wasm/Writer.cpp:419
   for (ObjFile *File : Symtab->ObjectFiles) {
+    std::string FileName(File->getName());
     SmallSet<std::string, 8> ObjectFeatures;
----------------
sbc100 wrote:
> Why not StringRef?
Switched, and also above. I'm always wary about StringRef because I've been burned by it not owning its contents one too many times. I assume when it is used to insert something into a StringMap the string value is copied, so it should be ok here.


Repository:
  rL LLVM

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D62704/new/

https://reviews.llvm.org/D62704