[VM] Implement name demangling of extension methods.

Change-Id: Id64ae1aa0de89260b6bf2d6f40260a0dc9ef5c16
Reviewed-on: https://dart-review.googlesource.com/c/sdk/+/121000
Reviewed-by: Ryan Macnak <rmacnak@google.com>
Commit-Queue: Siva Annamalai <asiva@google.com>

runtime/lib/mirrors.cc

@@ -246,7 +246,7 @@ static RawInstance* CreateMethodMirror(const Function& func,
   args.SetAt(0, MirrorReference::Handle(MirrorReference::New(func)));
 
   String& name = String::Handle(func.name());
-  name = String::ScrubNameRetainPrivate(name);
+  name = String::ScrubNameRetainPrivate(name, func.is_extension_member());
   args.SetAt(1, name);
   args.SetAt(2, owner_mirror);
   args.SetAt(3, instantiator);

runtime/tests/vm/dart/extension_names_test.dart

@@ -0,0 +1,103 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Test that ensures stack traces have user friendly names from extension
+// functions.
+
+import 'dart:core';
+import "package:expect/expect.dart";
+
+class C<T> {
+  static int tracefunc() {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('C.tracefunc'));
+      Expect.isTrue(s.toString().contains('ext.sfld'));
+    }
+    return 10;
+  }
+
+  static int ld = C.tracefunc();
+}
+
+extension ext<T> on C<T> {
+  func() {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('ext.func'));
+    }
+  }
+
+  get prop {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('ext.prop'));
+    }
+  }
+
+  set prop(value) {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('ext.prop='));
+    }
+  }
+
+  operator +(val) {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('ext.+'));
+    }
+  }
+
+  operator -(val) {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('ext.-'));
+    }
+  }
+
+  static int sfld = C.tracefunc();
+  static sfunc() {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('ext.sfunc'));
+    }
+  }
+
+  static get sprop {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('ext.sprop'));
+    }
+  }
+
+  static set sprop(value) {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('ext.sprop='));
+    }
+  }
+}
+
+main() {
+  C<int> c = new C<int>();
+  c.func();
+  c.prop;
+  c.prop = 10;
+  ext.sfunc();
+  ext.sprop;
+  ext.sprop = 10;
+  ext.sfld;
+  c + 1;
+  c - 1;
+}

runtime/tests/vm/dart/extension_unnamed_names_test.dart

@@ -0,0 +1,85 @@
+// Copyright (c) 2019, the Dart project authors.  Please see the AUTHORS file
+// for details. All rights reserved. Use of this source code is governed by a
+// BSD-style license that can be found in the LICENSE file.
+
+// Test that ensures stack traces have user friendly names from extension
+// functions.
+
+import 'dart:core';
+import "package:expect/expect.dart";
+
+class C {
+  static int tracefunc() {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('C.tracefunc'));
+      Expect.isTrue(s.toString().contains('ext.sfld'));
+    }
+    return 10;
+  }
+
+  static int ld = C.tracefunc();
+}
+
+extension on C {
+  func() {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('_extension#0.func'));
+    }
+  }
+
+  get prop {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('_extension#0.prop'));
+    }
+  }
+
+  set prop(value) {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('_extension#0.prop='));
+    }
+  }
+
+  operator +(val) {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('_extension#0.+'));
+    }
+  }
+
+  operator -(val) {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('_extension#0.-'));
+    }
+  }
+}
+
+extension on C {
+  bar() {
+    try {
+      throw "producing a stack trace";
+    } catch (e, s) {
+      Expect.isTrue(s.toString().contains('_extension#1.bar'));
+    }
+  }
+}
+
+main() {
+  C c = new C();
+  c.func();
+  c.prop;
+  c.prop = 10;
+  c + 1;
+  c - 1;
+  c.bar();
+}

runtime/vm/object.cc

@@ -252,7 +252,15 @@ RawString* String::RemovePrivateKey(const String& name) {
 //   _MyClass@6328321. -> _MyClass
 //   _MyClass@6328321.named -> _MyClass.named
 //
-RawString* String::ScrubName(const String& name) {
+// For extension methods the following demangling is done
+//   ext|func -> ext.func (instance extension method)
+//   ext|get#prop -> ext.prop (instance extension getter)
+//   ext|set#prop -> ext.prop= (instance extension setter)
+//   ext|sfunc -> ext.sfunc (static extension method)
+//   get:ext|sprop -> ext.sprop (static extension getter)
+//   set:ext|sprop -> ext.sprop= (static extension setter)
+//
+RawString* String::ScrubName(const String& name, bool is_extension) {
   Thread* thread = Thread::Current();
   Zone* zone = thread->zone();
 
@@ -266,7 +274,8 @@ RawString* String::ScrubName(const String& name) {
   const char* cname = name.ToCString();
   ASSERT(strlen(cname) == static_cast<size_t>(name.Length()));
   const intptr_t name_len = name.Length();
-  // First remove all private name mangling.
+  // First remove all private name mangling and if 'is_extension' is true
+  // substitute the first '|' character with '.'.
   intptr_t start_pos = 0;
   GrowableArray<const char*> unmangled_segments;
   intptr_t sum_segment_len = 0;
@@ -286,6 +295,15 @@ RawString* String::ScrubName(const String& name) {
       }
       start_pos = i;
       i--;  // Account for for-loop increment.
+    } else if (is_extension && cname[i] == '|') {
+      // Append the current segment to the unmangled name.
+      const intptr_t segment_len = i - start_pos;
+      AppendSubString(zone, &unmangled_segments, cname, start_pos, segment_len);
+      // Append the '.' character (replaces '|' with '.').
+      AppendSubString(zone, &unmangled_segments, ".", 0, 1);
+      start_pos = i + 1;
+      // Account for length of segments added so far.
+      sum_segment_len += (segment_len + 1);
     }
   }
 
@@ -306,12 +324,41 @@ RawString* String::ScrubName(const String& name) {
   }
 
 #if !defined(DART_PRECOMPILED_RUNTIME)
-  intptr_t len = sum_segment_len;
+  unmangled_segments.Clear();
   intptr_t start = 0;
-  intptr_t dot_pos = -1;  // Position of '.' in the name, if any.
+  intptr_t final_len = 0;
+  intptr_t len = sum_segment_len;
   bool is_setter = false;
+  if (is_extension) {
+    // First scan till we see the '.' character.
+    for (intptr_t i = 0; i < len; i++) {
+      if (unmangled_name[i] == '.') {
+        intptr_t slen = i + 1;
+        intptr_t plen = slen - start;
+        AppendSubString(zone, &unmangled_segments, unmangled_name, start, plen);
+        final_len = plen;
+        unmangled_name += slen;
+        len -= slen;
+        break;
+      } else if (unmangled_name[i] == ':') {
+        if (start != 0) {
+          // Reset and break.
+          start = 0;
+          is_setter = false;
+          break;
+        }
+        if (unmangled_name[0] == 's') {
+          is_setter = true;
+        }
+        start = i + 1;
+      }
+    }
+  }
+  intptr_t dot_pos = -1;  // Position of '.' in the name, if any.
+  start = 0;
   for (intptr_t i = start; i < len; i++) {
-    if (unmangled_name[i] == ':') {
+    if (unmangled_name[i] == ':' ||
+        (is_extension && unmangled_name[i] == '#')) {
       if (start != 0) {
         // Reset and break.
         start = 0;
@@ -320,6 +367,7 @@ RawString* String::ScrubName(const String& name) {
       }
       ASSERT(start == 0);  // Only one : is possible in getters or setters.
       if (unmangled_name[0] == 's') {
+        ASSERT(!is_setter);
         is_setter = true;
       }
       start = i + 1;
@@ -335,7 +383,7 @@ RawString* String::ScrubName(const String& name) {
     }
   }
 
-  if ((start == 0) && (dot_pos == -1)) {
+  if (!is_extension && (start == 0) && (dot_pos == -1)) {
     // This unmangled_name is fine as it is.
     return Symbols::New(thread, unmangled_name, sum_segment_len);
   }
@@ -343,9 +391,9 @@ RawString* String::ScrubName(const String& name) {
   // Drop the trailing dot if needed.
   intptr_t end = ((dot_pos + 1) == len) ? dot_pos : len;
 
-  unmangled_segments.Clear();
-  intptr_t final_len = end - start;
-  AppendSubString(zone, &unmangled_segments, unmangled_name, start, final_len);
+  intptr_t substr_len = end - start;
+  final_len += substr_len;
+  AppendSubString(zone, &unmangled_segments, unmangled_name, start, substr_len);
   if (is_setter) {
     const char* equals = Symbols::Equals().ToCString();
     const intptr_t equals_len = strlen(equals);
@@ -359,17 +407,46 @@ RawString* String::ScrubName(const String& name) {
   return Symbols::New(thread, unmangled_name);
 }
 
-RawString* String::ScrubNameRetainPrivate(const String& name) {
+RawString* String::ScrubNameRetainPrivate(const String& name,
+                                          bool is_extension) {
 #if !defined(DART_PRECOMPILED_RUNTIME)
   intptr_t len = name.Length();
   intptr_t start = 0;
   intptr_t at_pos = -1;  // Position of '@' in the name, if any.
   bool is_setter = false;
 
+  String& result = String::Handle();
+
+  // If extension strip out the leading prefix e.g" ext|func would strip out
+  // 'ext|'.
+  if (is_extension) {
+    // First scan till we see the '|' character.
+    for (intptr_t i = 0; i < len; i++) {
+      if (name.CharAt(i) == '|') {
+        result = String::SubString(name, start, (i - start));
+        result = String::Concat(result, Symbols::Dot());
+        start = i + 1;
+        break;
+      } else if (name.CharAt(i) == ':') {
+        if (start != 0) {
+          // Reset and break.
+          start = 0;
+          is_setter = false;
+          break;
+        }
+        if (name.CharAt(0) == 's') {
+          is_setter = true;
+        }
+        start = i + 1;
+      }
+    }
+  }
+
   for (intptr_t i = start; i < len; i++) {
-    if (name.CharAt(i) == ':') {
-      ASSERT(start == 0);  // Only one : is possible in getters or setters.
-      if (name.CharAt(0) == 's') {
+    if (name.CharAt(i) == ':' || (is_extension && name.CharAt(i) == '#')) {
+      // Only one : is possible in getters or setters.
+      ASSERT(is_extension || start == 0);
+      if (name.CharAt(start) == 's') {
         is_setter = true;
       }
       start = i + 1;
@@ -385,8 +462,13 @@ RawString* String::ScrubNameRetainPrivate(const String& name) {
     return name.raw();
   }
 
-  String& result =
-      String::Handle(String::SubString(name, start, (len - start)));
+  if (is_extension) {
+    const String& fname =
+        String::Handle(String::SubString(name, start, (len - start)));
+    result = String::Concat(result, fname);
+  } else {
+    result = String::SubString(name, start, (len - start));
+  }
 
   if (is_setter) {
     // Setters need to end with '='.
@@ -7889,7 +7971,7 @@ RawString* Function::UserVisibleName() const {
   if (FLAG_show_internal_names) {
     return name();
   }
-  return String::ScrubName(String::Handle(name()));
+  return String::ScrubName(String::Handle(name()), is_extension_member());
 }
 
 RawString* Function::QualifiedName(NameVisibility name_visibility) const {
@@ -8679,7 +8761,7 @@ RawString* Field::UserVisibleName() const {
   if (FLAG_show_internal_names) {
     return name();
   }
-  return String::ScrubName(String::Handle(name()));
+  return String::ScrubName(String::Handle(name()), is_extension_member());
 }
 
 intptr_t Field::guarded_list_length() const {

runtime/vm/object.h

@@ -7838,8 +7838,9 @@ class String : public Instance {
 
   static RawString* RemovePrivateKey(const String& name);
 
-  static RawString* ScrubName(const String& name);
-  static RawString* ScrubNameRetainPrivate(const String& name);
+  static RawString* ScrubName(const String& name, bool is_extension = false);
+  static RawString* ScrubNameRetainPrivate(const String& name,
+                                           bool is_extension = false);
 
   static bool EqualsIgnoringPrivateKey(const String& str1, const String& str2);
 

tests/lib_2/mirrors/method_mirror_extension_test.dart

@@ -88,9 +88,10 @@ extension ext<T> on C<T> {
   }
 }
 
-checkExtensionKind(closure, kind) {
+checkExtensionKind(closure, kind, name) {
   var closureMirror = reflect(closure) as ClosureMirror;
   var methodMirror = closureMirror.function;
+  Expect.isTrue(methodMirror.simpleName.toString().contains(name));
   Expect.equals(kind, methodMirror.isExtensionMember, "isExtension");
 }
 
@@ -99,6 +100,7 @@ void testExtension(sym, mirror) {
     var methodMirror = mirror as MethodMirror;
     if (MirrorSystem.getName(sym).startsWith('ext', 0)) {
       Expect.equals(true, methodMirror.isExtensionMember, "isExtension");
+      Expect.isTrue(methodMirror.simpleName.toString().contains('ext.'));
     } else {
       Expect.equals(false, methodMirror.isExtensionMember, "isExtension");
     }
@@ -113,11 +115,11 @@ void testExtension(sym, mirror) {
 }
 
 main() {
-  checkExtensionKind(C.tracefunc, false);
+  checkExtensionKind(C.tracefunc, false, 'tracefunc');
 
   C c = new C();
-  checkExtensionKind(c.func, true);
-  checkExtensionKind(ext.sfunc, true);
+  checkExtensionKind(c.func, true, 'ext.func');
+  checkExtensionKind(ext.sfunc, true, 'ext.sfunc');
 
   var libraryMirror = reflectClass(C).owner as LibraryMirror;
   libraryMirror.declarations.forEach(testExtension);