[wasm] Differentiate unnamed and empty names

src/js/messages.js

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 // -------------------------------------------------------------------
 
 (function(global, utils) {
 
 %CheckIsBootstrapping();
 
@@ skipping 614 matching lines @@
 function CallSiteGetScriptNameOrSourceURL() {
   CheckCallSite(this, "getScriptNameOrSourceURL");
   return %CallSiteGetScriptNameOrSourceUrlRT(this);
 }
 
 function CallSiteGetFunctionName() {
   // See if the function knows its own name
   CheckCallSite(this, "getFunctionName");
   if (HAS_PRIVATE(this, callSiteWasmObjectSymbol)) {
     var wasm = GET_PRIVATE(this, callSiteWasmObjectSymbol);
     var func_index = GET_PRIVATE(this, callSiteWasmFunctionIndexSymbol);

[Yang, 2016/05/13 07:41:10]

CallSiteGetFunctionName is accessible from the stack trace API. This here
changes the behavior, and we could be returning undefined for the function name.
Do we really want to expose that?

What would be nicer imo is to hide that, and return <WASM UNNAMED> directly. In
fact, we should do that in GetWasmFunctionName or GetWasmFunctionNameFromTable.

In another CL you commented that
"We cannot completely hide the logic inside %WasmGetFunctionName, since the
string representation differs in some cases from the information returned by
CallSite.getFunctionName()."

I still haven't understood when that is the case, and why we can't simply have
CallSiteGetFunctionName return what %WasmGetFunctionName returns.

I also looked at CallSite::GetFunctionName(). We have a wasm code path there as
well, whcih again calls GetWasmFunctionName. How come we have two different, yet
similar code paths? CallSite::GetFunctionName() converts undefined to null,
which we don't do here. Why the difference?

[Clemens Hammacher, 2016/05/17 17:43:24]

I was assuming that it would be nice for users to be able to tell that a
function was unnamed. The string "<WASM UNNAMED>" would only be used for
printing such unnamed functions, e.g. on the stack trace.

As discussed offline, I now changed it to return null on
CallSite::getFunctionName (just as anonymous JavaScript function), and print it
as "<WASM UNNAMED>" on CallSite::toString().
OK, this is not about %WasmGetFunctionName vs. CallSite.getFunctionName(), but
about those two vs. CallSite.toString(). There needs to be some (tiny) logic in
messages.js to translate null (previously undefined) to "<WASM UNNAMED>".
Oh, you are right. If I see it correctly, CallSite::GetFunctionName was never
called for wasm functions, so that code path was dead. I now removed the special
handling from messages.js, and always call CallSite::GetFunctionName, which does
the right thing and returns either null or the function name as string.

-    if (IS_UNDEFINED(wasm)) return "<WASM>";
     return %WasmGetFunctionName(wasm, func_index);
   }
   return %CallSiteGetFunctionNameRT(this);
 }
 
 function CallSiteGetMethodName() {
   // See if we can find a unique property on the receiver that holds
   // this function.
   CheckCallSite(this, "getMethodName");
   return %CallSiteGetMethodNameRT(this);
@@ skipping 25 matching lines @@
 function CallSiteIsConstructor() {
   CheckCallSite(this, "isConstructor");
   return %CallSiteIsConstructorRT(this);
 }
 
 function CallSiteToString() {
   if (HAS_PRIVATE(this, callSiteWasmObjectSymbol)) {
     var funName = this.getFunctionName();
     var funcIndex = GET_PRIVATE(this, callSiteWasmFunctionIndexSymbol);
     var pos = this.getPosition();
-    return funName + " (<WASM>:" + funcIndex + ":" + pos + ")";
+    if (IS_UNDEFINED(funName)) funName = "<WASM UNNAMED>";
+    return funName + " (<WASM>[" + funcIndex + "]+" + pos + ")";

[Yang, 2016/05/13 07:41:10]

So we could get a return value like "<WASM UNNAMED> (<WASM>[0]+44)". Is that
intended?

[Clemens Hammacher, 2016/05/17 17:43:24]

Yes.

   }
 
   var fileName;
   var fileLocation = "";
   if (this.isNative()) {
     fileLocation = "native";
   } else {
     fileName = this.getScriptNameOrSourceURL();
     if (!fileName && this.isEval()) {
       fileLocation = this.getEvalOrigin();
@@ skipping 352 matching lines @@
 utils.Export(function(to) {
   to.ErrorToString = ErrorToString;
   to.MakeError = MakeError;
   to.MakeRangeError = MakeRangeError;
   to.MakeSyntaxError = MakeSyntaxError;
   to.MakeTypeError = MakeTypeError;
   to.MakeURIError = MakeURIError;
 });
 
 });