[module] Track script "module code" status

test/cctest/test-api.cc

 // Copyright 2012 the V8 project authors. All rights reserved.
 // Redistribution and use in source and binary forms, with or without
 // modification, are permitted provided that the following conditions are
 // met:
 //
 //     * Redistributions of source code must retain the above copyright
 //       notice, this list of conditions and the following disclaimer.
 //     * Redistributions in binary form must reproduce the above
 //       copyright notice, this list of conditions and the following
 //       disclaimer in the documentation and/or other materials provided
@@ skipping 23586 matching lines @@
   CHECK(script1->GetPositionInfo(9, &info, script1->NO_OFFSET));
   CHECK_EQ(1, info.line);
   CHECK_EQ(0, info.column);
   CHECK_EQ(9, info.line_start);
   CHECK_EQ(17, info.line_end);
 
   // Fail when position is larger than script size.
   CHECK(!script1->GetPositionInfo(220384, &info, script1->NO_OFFSET));
 }
 
+TEST(ScriptIsModule) {
+  LocalContext env;
+  v8::Isolate* isolate = env->GetIsolate();
+  v8::HandleScope scope(isolate);
+  const char* url = "http://www.foo.com/foo.js";
+  v8::ScriptOrigin origin(v8_str(url), v8::Integer::New(isolate, 1));
+  Local<Script> localScript;
+  i::Handle<i::SharedFunctionInfo> fnInfo;
+
+  v8::ScriptCompiler::Source script_source1(v8_str("void 1;\n"), origin);
+  localScript = v8::ScriptCompiler::Compile(env.local(), &script_source1)
+                    .ToLocalChecked();
+  fnInfo = i::Handle<i::SharedFunctionInfo>::cast(
+      v8::Utils::OpenHandle(*localScript->GetUnboundScript()));
+  i::Handle<i::Script> globalScript(i::Script::cast(fnInfo->script()));
+  CHECK_EQ(false, globalScript->is_module());
+
+  v8::ScriptCompiler::Source script_source2(v8_str("void 2;\n"), origin);

[mike3, 2016/06/12 19:54:48]

A distinct Source object for each test case is necessary to avoid hitting the
compilation cache. I expect this is an indication that CompilationCache should
also be aware of the `is_module` flag, but I wanted to confirm with you before
making that change.

[vogelheim, 2016/06/14 15:20:50]

This is a good catch! I hadn't thought of that... After discussing with Adam:
Yes, the CompilationCache needs to depend on is_module.

Since, at present, this isn't being used, the easiest implementation would be to
just check for !is_module() when the compilation cache is queried - that'd
preserve correctness. We could then add that flag to the compilation cache when
modules are fully implemented.

--------------

[We briefly discussed how to add it, whether we could either merge that flag
into the LanguageMode, or if we should introduce some sort of
AllFlagsThatAffectCompilationStruct. The agreement was that the former doesn't
make sense since those are orthogonal; the latter might make sense if we
expected more of these flags in the future, but we don't. Hence just adding a
boolean to the compilation cache lookup would likely be best & easiest.]

[mike3, 2016/06/19 16:59:00]

Much as I enjoy seeing my commit count increase (petty, I know), I don't mind
putting in the extra effort to get it right the first time.

When it comes to LanguageMode, I don't believe "non-strict", "strict" and
"module" are orthogonal. "module" is a superset of "strict" in terms of
capabilities, so even though the specification terminology makes a distinction,
there isn't a practical benefit in allowing "strictness" to vary independently
from "moduleness". In fact, it is likely more convenient to couple the
definitions like this because it means you never have to worry about entering
an invalid "non-strict module" state.

Implementing this might not be so bad, either. The global `is_strict` function
seems to be the primary interface for checking strictness, so if that were
updated to return `true` for module code, then that might cover most cases.

Of course, I remain ignorant of huge sections of the code base, and I expect
I'm over-simplifying things.

+  localScript = v8::ScriptCompiler::CompileModule(env.local(), &script_source2)
+                    .ToLocalChecked();
+  fnInfo = i::Handle<i::SharedFunctionInfo>::cast(
+      v8::Utils::OpenHandle(*localScript->GetUnboundScript()));
+  i::Handle<i::Script> moduleScript(i::Script::cast(fnInfo->script()));
+  CHECK_EQ(true, moduleScript->is_module());
+}
+
 void CheckMagicComments(Local<Script> script, const char* expected_source_url,
                         const char* expected_source_mapping_url) {
   if (expected_source_url != NULL) {
     v8::String::Utf8Value url(script->GetUnboundScript()->GetSourceURL());
     CHECK_EQ(0, strcmp(expected_source_url, *url));
   } else {
     CHECK(script->GetUnboundScript()->GetSourceURL()->IsUndefined());
   }
   if (expected_source_mapping_url != NULL) {
     v8::String::Utf8Value url(
@@ skipping 1723 matching lines @@
 }
 
 TEST(PrivateForApiIsNumber) {
   LocalContext context;
   v8::Isolate* isolate = CcTest::isolate();
   v8::HandleScope scope(isolate);
 
   // Shouldn't crash.
   v8::Private::ForApi(isolate, v8_str("42"));
 }