Revert of Refactor SharedFunctionInfo::IsBuiltin. (patchset #1 id:1 of …

…https://codereview.chromium.org/2505853003/ ) Reason for revert: Breaks layout tests: https://build.chromium.org/p/client.v8.fyi/builders/V8-Blink%20Linux%2064/builds/11394 Original issue's description: > Refactor SharedFunctionInfo::IsBuiltin. > > This method is a slight misnomer. What we actually want to know is > whether the function was defined in a user-provided script. > > Also remove redundant Script::hide_source flag. > > R=bmeurer@chromium.org, ulan@chromium.org TBR=bmeurer@chromium.org,ulan@chromium.org,yangguo@chromium.org # Skipping CQ checks because original CL landed less than 1 days ago. NOPRESUBMIT=true NOTREECHECKS=true NOTRY=true Review-Url: https://codereview.chromium.org/2512463002 Cr-Commit-Position: refs/heads/master@{#41050}
nodejs · Nov 16, 2016 · 1160e5e · 1160e5e
1 parent ca3f487
commit 1160e5e
Show file tree

Hide file tree

Showing 19 changed files with 58 additions and 46 deletions.
diff --git a/src/accessors.cc b/src/accessors.cc
@@ -1025,11 +1025,10 @@ MaybeHandle<JSFunction> FindCaller(Isolate* isolate,
     if (caller == NULL) return MaybeHandle<JSFunction>();
   } while (caller->shared()->is_toplevel());
 
-  // If caller is not user code and caller's caller is also not user code,
+  // If caller is a built-in function and caller's caller is also built-in,
   // use that instead.
   JSFunction* potential_caller = caller;
-  while (potential_caller != NULL &&
-         !potential_caller->shared()->IsUserJavaScript()) {
+  while (potential_caller != NULL && potential_caller->shared()->IsBuiltin()) {
     caller = potential_caller;
     potential_caller = it.next();
   }

diff --git a/src/api.cc b/src/api.cc
@@ -5109,7 +5109,7 @@ bool Function::IsBuiltin() const {
     return false;
   }
   auto func = i::Handle<i::JSFunction>::cast(self);
-  return !func->shared()->IsUserJavaScript();
+  return func->shared()->IsBuiltin();
 }
 
 

diff --git a/src/ast/prettyprinter.cc b/src/ast/prettyprinter.cc
@@ -14,14 +14,14 @@
 namespace v8 {
 namespace internal {
 
-CallPrinter::CallPrinter(Isolate* isolate, bool is_user_js)
+CallPrinter::CallPrinter(Isolate* isolate, bool is_builtin)
     : builder_(isolate) {
   isolate_ = isolate;
   position_ = 0;
   num_prints_ = 0;
   found_ = false;
   done_ = false;
-  is_user_js_ = is_user_js;
+  is_builtin_ = is_builtin;
   InitializeAstVisitor(isolate);
 }
 
@@ -239,11 +239,11 @@ void CallPrinter::VisitArrayLiteral(ArrayLiteral* node) {
 
 
 void CallPrinter::VisitVariableProxy(VariableProxy* node) {
-  if (is_user_js_) {
-    PrintLiteral(node->name(), false);
-  } else {
-    // Variable names of non-user code are meaningless due to minification.
+  if (is_builtin_) {
+    // Variable names of builtins are meaningless due to minification.
     Print("(var)");
+  } else {
+    PrintLiteral(node->name(), false);
   }
 }
 
@@ -279,9 +279,9 @@ void CallPrinter::VisitProperty(Property* node) {
 void CallPrinter::VisitCall(Call* node) {
   bool was_found = !found_ && node->position() == position_;
   if (was_found) {
-    // Bail out if the error is caused by a direct call to a variable in
-    // non-user JS code. The variable name is meaningless due to minification.
-    if (!is_user_js_ && node->expression()->IsVariableProxy()) {
+    // Bail out if the error is caused by a direct call to a variable in builtin
+    // code. The variable name is meaningless due to minification.
+    if (is_builtin_ && node->expression()->IsVariableProxy()) {
       done_ = true;
       return;
     }
@@ -297,9 +297,9 @@ void CallPrinter::VisitCall(Call* node) {
 void CallPrinter::VisitCallNew(CallNew* node) {
   bool was_found = !found_ && node->position() == position_;
   if (was_found) {
-    // Bail out if the error is caused by a direct call to a variable in
-    // non-user JS code. The variable name is meaningless due to minification.
-    if (!is_user_js_ && node->expression()->IsVariableProxy()) {
+    // Bail out if the error is caused by a direct call to a variable in builtin
+    // code. The variable name is meaningless due to minification.
+    if (is_builtin_ && node->expression()->IsVariableProxy()) {
       done_ = true;
       return;
     }

diff --git a/src/ast/prettyprinter.h b/src/ast/prettyprinter.h
@@ -15,7 +15,7 @@ namespace internal {
 
 class CallPrinter final : public AstVisitor<CallPrinter> {
  public:
-  explicit CallPrinter(Isolate* isolate, bool is_user_js);
+  explicit CallPrinter(Isolate* isolate, bool is_builtin);
 
   // The following routine prints the node with position |position| into a
   // string.
@@ -38,7 +38,7 @@ class CallPrinter final : public AstVisitor<CallPrinter> {
   int position_;  // position of ast node to print
   bool found_;
   bool done_;
-  bool is_user_js_;
+  bool is_builtin_;
 
   DEFINE_AST_VISITOR_SUBCLASS_MEMBERS();
 

diff --git a/src/compiler.cc b/src/compiler.cc
@@ -1447,8 +1447,10 @@ Handle<SharedFunctionInfo> Compiler::GetSharedFunctionInfoForScript(
     if (FLAG_trace_deopt) Script::InitLineEnds(script);
     if (natives == NATIVES_CODE) {
       script->set_type(Script::TYPE_NATIVE);
+      script->set_hide_source(true);
     } else if (natives == EXTENSION_CODE) {
       script->set_type(Script::TYPE_EXTENSION);
+      script->set_hide_source(true);
     }
     if (!script_name.is_null()) {
       script->set_name(*script_name);

diff --git a/src/compiler/js-inlining-heuristic.cc b/src/compiler/js-inlining-heuristic.cc
@@ -46,8 +46,8 @@ bool CanInlineFunction(Handle<JSFunction> function) {
   // Built-in functions are handled by the JSBuiltinReducer.
   if (function->shared()->HasBuiltinFunctionId()) return false;
 
-  // Only choose user code for inlining.
-  if (!function->shared()->IsUserJavaScript()) return false;
+  // Don't inline builtins.
+  if (function->shared()->IsBuiltin()) return false;
 
   // Quick check on the size of the AST to avoid parsing large candidate.
   if (function->shared()->ast_node_count() > FLAG_max_inlined_nodes) {

diff --git a/src/compiler/pipeline.cc b/src/compiler/pipeline.cc
@@ -1512,7 +1512,7 @@ bool PipelineImpl::CreateGraph() {
     // Determine the Typer operation flags.
     Typer::Flags flags = Typer::kNoFlags;
     if (is_sloppy(info()->shared_info()->language_mode()) &&
-        info()->shared_info()->IsUserJavaScript()) {
+        !info()->shared_info()->IsBuiltin()) {
       // Sloppy mode functions always have an Object for this.
       flags |= Typer::kThisIsReceiver;
     }

diff --git a/src/crankshaft/hydrogen.cc b/src/crankshaft/hydrogen.cc
@@ -7966,7 +7966,7 @@ int HOptimizedGraphBuilder::InliningAstSize(Handle<JSFunction> target) {
   if (target_shared->force_inline()) {
     return 0;
   }
-  if (!target->shared()->IsUserJavaScript()) {
+  if (target->shared()->IsBuiltin()) {
     return kNotInlinable;
   }
 

diff --git a/src/heap/objects-visiting-inl.h b/src/heap/objects-visiting-inl.h
@@ -600,8 +600,8 @@ bool StaticMarkingVisitor<StaticVisitor>::IsFlushable(
     return false;
   }
 
-  // The function must be user code.
-  if (!shared_info->IsUserJavaScript()) {
+  // The function must not be a builtin.
+  if (shared_info->IsBuiltin()) {
     return false;
   }
 

diff --git a/src/ic/ic.cc b/src/ic/ic.cc
@@ -1033,7 +1033,7 @@ bool IsCompatibleReceiver(LookupIterator* lookup, Handle<Map> receiver_map) {
     if (holder->HasFastProperties()) {
       if (getter->IsJSFunction()) {
         Handle<JSFunction> function = Handle<JSFunction>::cast(getter);
-        if (!receiver->IsJSObject() && function->shared()->IsUserJavaScript() &&
+        if (!receiver->IsJSObject() && !function->shared()->IsBuiltin() &&
             is_sloppy(function->shared()->language_mode())) {
           // Calling sloppy non-builtins with a value as the receiver
           // requires boxing.

diff --git a/src/isolate.cc b/src/isolate.cc
@@ -358,7 +358,7 @@ class StackTraceHelper {
   // Determines whether the given stack frame should be displayed in a stack
   // trace.
   bool IsVisibleInStackTrace(JSFunction* fun) {
-    return ShouldIncludeFrame(fun) && IsNotHidden(fun) &&
+    return ShouldIncludeFrame(fun) && IsNotInNativeScript(fun) &&
            IsInSameSecurityContext(fun);
   }
 
@@ -386,12 +386,12 @@ class StackTraceHelper {
     return false;
   }
 
-  bool IsNotHidden(JSFunction* fun) {
-    // Functions defined not in user scripts are not visible unless directly
+  bool IsNotInNativeScript(JSFunction* fun) {
+    // Functions defined in native scripts are not visible unless directly
     // exposed, in which case the native flag is set.
     // The --builtins-in-stack-traces command line flag allows including
     // internal call sites in the stack trace for debugging purposes.
-    if (!FLAG_builtins_in_stack_traces && !fun->shared()->IsUserJavaScript()) {
+    if (!FLAG_builtins_in_stack_traces && fun->shared()->IsBuiltin()) {
       return fun->shared()->native();
     }
     return true;

diff --git a/src/objects-inl.h b/src/objects-inl.h
@@ -5948,6 +5948,10 @@ void Script::set_compilation_type(CompilationType type) {
   set_flags(BooleanBit::set(flags(), kCompilationTypeBit,
       type == COMPILATION_TYPE_EVAL));
 }
+bool Script::hide_source() { return BooleanBit::get(flags(), kHideSourceBit); }
+void Script::set_hide_source(bool value) {
+  set_flags(BooleanBit::set(flags(), kHideSourceBit, value));
+}
 Script::CompilationState Script::compilation_state() {
   return BooleanBit::get(flags(), kCompilationStateBit) ?
       COMPILATION_STATE_COMPILED : COMPILATION_STATE_INITIAL;
@@ -6493,15 +6497,17 @@ void SharedFunctionInfo::set_disable_optimization_reason(BailoutReason reason) {
       opt_count_and_bailout_reason(), reason));
 }
 
-bool SharedFunctionInfo::IsUserJavaScript() {
+
+bool SharedFunctionInfo::IsBuiltin() {
   Object* script_obj = script();
-  if (script_obj->IsUndefined(GetIsolate())) return false;
+  if (script_obj->IsUndefined(GetIsolate())) return true;
   Script* script = Script::cast(script_obj);
-  return static_cast<Script::Type>(script->type()) == Script::TYPE_NORMAL;
+  Script::Type type = static_cast<Script::Type>(script->type());
+  return type != Script::TYPE_NORMAL;
 }
 
 bool SharedFunctionInfo::IsSubjectToDebugging() {
-  return IsUserJavaScript() && !HasAsmWasmData();
+  return !IsBuiltin() && !HasAsmWasmData();
 }
 
 bool SharedFunctionInfo::OptimizedCodeMapIsCleared() const {

diff --git a/src/objects.cc b/src/objects.cc
@@ -13301,7 +13301,8 @@ Handle<String> JSFunction::ToString(Handle<JSFunction> function) {
   Handle<SharedFunctionInfo> shared_info(function->shared(), isolate);
 
   // Check if {function} should hide its source code.
-  if (!shared_info->IsUserJavaScript()) {
+  if (!shared_info->script()->IsScript() ||
+      Script::cast(shared_info->script())->hide_source()) {
     return NativeCodeFunctionSourceString(shared_info);
   }
 

diff --git a/src/objects.h b/src/objects.h
@@ -7104,6 +7104,11 @@ class Script: public Struct {
   inline CompilationState compilation_state();
   inline void set_compilation_state(CompilationState state);
 
+  // [hide_source]: determines whether the script source can be exposed as
+  // function source. Encoded in the 'flags' field.
+  inline bool hide_source();
+  inline void set_hide_source(bool value);
+
   // [origin_options]: optional attributes set by the embedder via ScriptOrigin,
   // and used by the embedder to make decisions about the script. V8 just passes
   // this through. Encoded in the 'flags' field.
@@ -7204,7 +7209,8 @@ class Script: public Struct {
   // Bit positions in the flags field.
   static const int kCompilationTypeBit = 0;
   static const int kCompilationStateBit = 1;
-  static const int kOriginOptionsShift = 2;
+  static const int kHideSourceBit = 2;
+  static const int kOriginOptionsShift = 3;
   static const int kOriginOptionsSize = 3;
   static const int kOriginOptionsMask = ((1 << kOriginOptionsSize) - 1)
                                         << kOriginOptionsShift;
@@ -7720,8 +7726,8 @@ class SharedFunctionInfo: public HeapObject {
   // Tells whether this function should be subject to debugging.
   inline bool IsSubjectToDebugging();
 
-  // Whether this function is defined in user-provided JavaScript code.
-  inline bool IsUserJavaScript();
+  // Whether this function is defined in native code or extensions.
+  inline bool IsBuiltin();
 
   // Check whether or not this function is inlineable.
   bool IsInlineable();

diff --git a/src/runtime-profiler.cc b/src/runtime-profiler.cc
@@ -164,7 +164,7 @@ void RuntimeProfiler::AttemptOnStackReplacement(JavaScriptFrame* frame,
                                                 int loop_nesting_levels) {
   JSFunction* function = frame->function();
   SharedFunctionInfo* shared = function->shared();
-  if (!FLAG_use_osr || !function->shared()->IsUserJavaScript()) {
+  if (!FLAG_use_osr || function->shared()->IsBuiltin()) {
     return;
   }
 

diff --git a/src/runtime/runtime-debug.cc b/src/runtime/runtime-debug.cc
@@ -778,7 +778,7 @@ RUNTIME_FUNCTION(Runtime_GetFrameDetails) {
 
   // Add the receiver (same as in function frame).
   Handle<Object> receiver(it.frame()->receiver(), isolate);
-  DCHECK(function->shared()->IsUserJavaScript());
+  DCHECK(!function->shared()->IsBuiltin());
   DCHECK_IMPLIES(is_sloppy(shared->language_mode()), receiver->IsJSReceiver());
   details->set(kFrameDetailsReceiverIndex, *receiver);
 

diff --git a/src/runtime/runtime-internal.cc b/src/runtime/runtime-internal.cc
@@ -395,8 +395,7 @@ Handle<String> RenderCallSite(Isolate* isolate, Handle<Object> object) {
             ? new ParseInfo(&zone, handle(location.function()->shared()))
             : new ParseInfo(&zone, location.script()));
     if (Parser::ParseStatic(info.get())) {
-      CallPrinter printer(isolate,
-                          location.function()->shared()->IsUserJavaScript());
+      CallPrinter printer(isolate, location.function()->shared()->IsBuiltin());
       Handle<String> str = printer.Print(info->literal(), location.start_pos());
       if (str->length() > 0) return str;
     } else {

diff --git a/src/runtime/runtime-scopes.cc b/src/runtime/runtime-scopes.cc
@@ -670,9 +670,8 @@ RUNTIME_FUNCTION(Runtime_NewScriptContext) {
   // Script contexts have a canonical empty function as their closure, not the
   // anonymous closure containing the global code.  See
   // FullCodeGenerator::PushFunctionArgumentForContextAllocation.
-  Handle<JSFunction> closure(function->shared()->IsUserJavaScript()
-                                 ? native_context->closure()
-                                 : *function);
+  Handle<JSFunction> closure(
+      function->shared()->IsBuiltin() ? *function : native_context->closure());
   Handle<Context> result =
       isolate->factory()->NewScriptContext(closure, scope_info);
 

diff --git a/test/cctest/heap/test-heap.cc b/test/cctest/heap/test-heap.cc
@@ -1712,7 +1712,7 @@ static int CountOptimizedUserFunctions(v8::Local<v8::Context> context) {
   Handle<Context> icontext = v8::Utils::OpenHandle(*context);
   Object* object = icontext->get(Context::OPTIMIZED_FUNCTIONS_LIST);
   while (object->IsJSFunction() &&
-         JSFunction::cast(object)->shared()->IsUserJavaScript()) {
+         !JSFunction::cast(object)->shared()->IsBuiltin()) {
     count++;
     object = JSFunction::cast(object)->next_function_link();
   }
@@ -1854,7 +1854,7 @@ static int CountOptimizedUserFunctionsWithGC(v8::Local<v8::Context> context,
   Handle<Object> object(icontext->get(Context::OPTIMIZED_FUNCTIONS_LIST),
                         isolate);
   while (object->IsJSFunction() &&
-         Handle<JSFunction>::cast(object)->shared()->IsUserJavaScript()) {
+         !Handle<JSFunction>::cast(object)->shared()->IsBuiltin()) {
     count++;
     if (count == n)
       isolate->heap()->CollectAllGarbage(