- report tear offs using the x#y syntax as a compilation error

runtime/vm/parser.cc

Index: runtime/vm/parser.cc
diff --git a/runtime/vm/parser.cc b/runtime/vm/parser.cc
index 5964c1104ab946c366a9e9f78dafbc4f9ee79bdf..eb3454bf37a250629a3ce25037cf655e02bcd29f 100644
--- a/runtime/vm/parser.cc
+++ b/runtime/vm/parser.cc
@@ -44,7 +44,10 @@ namespace dart {
 
 DEFINE_FLAG(bool, enable_debug_break, false, "Allow use of break \"message\".");
 DEFINE_FLAG(bool, trace_parser, false, "Trace parser operations.");
-DEFINE_FLAG(bool, warn_new_tearoff_syntax, true, "Warning on new tear off.");
+DEFINE_FLAG(bool,
+            support_new_tearoff_syntax,
+            false,
+            "Support new tear off syntax.");
 // TODO(floitsch): remove the conditional-directive flag, once we publicly
 // committed to the current version.
 DEFINE_FLAG(bool,
@@ -11704,148 +11707,151 @@ AstNode* Parser::ParseSelectors(AstNode* primary, bool is_cascade) {
 
 // Closurization e#m of getter, setter, method or operator.
 AstNode* Parser::ParseClosurization(AstNode* primary) {
-  if (FLAG_warn_new_tearoff_syntax) {
-    ReportWarning(
-        "Tear-offs using the x#id syntax is a deprecated feature,"
-        "it will not be supported in the next release");
-  }
-  ExpectToken(Token::kHASH);
-  TokenPosition property_pos = TokenPos();
-  bool is_setter_name = false;
+  if (!FLAG_support_new_tearoff_syntax) {
+    ReportError("Tear-offs using the x#id syntax is a deprecated feature");

[regis, 2016/12/29 18:24:07]

I have not followed the discussions about this tear off syntax and its future,
but having to specify --new_tearoff_syntax to avoid a "deprecated" error is
confusing. Shouldn't the flag be called --old_tearoff_syntax?

[siva, 2016/12/29 20:59:56]

Good point, I renamed the flag to --deprecated_tearoff_syntax

+    return NULL;
+  } else {

[regis, 2016/12/29 18:24:07]

I would not use an else here and leave the rest of this function unindented.

[siva, 2016/12/29 20:59:56]

Done.

+    ExpectToken(Token::kHASH);
+    TokenPosition property_pos = TokenPos();
+    bool is_setter_name = false;
 
-  String& extractor_name = String::ZoneHandle(Z);
-  if (IsIdentifier()) {
-    extractor_name = CurrentLiteral()->raw();
-    ConsumeToken();
-    if (CurrentToken() == Token::kASSIGN) {
+    String& extractor_name = String::ZoneHandle(Z);
+    if (IsIdentifier()) {
+      extractor_name = CurrentLiteral()->raw();
       ConsumeToken();
-      is_setter_name = true;
+      if (CurrentToken() == Token::kASSIGN) {
+        ConsumeToken();
+        is_setter_name = true;
+      }
+    } else if (Token::CanBeOverloaded(CurrentToken())) {
+      extractor_name = Symbols::Token(CurrentToken()).raw();
+      ConsumeToken();
+    } else {
+      ReportError("identifier or operator expected");
     }
-  } else if (Token::CanBeOverloaded(CurrentToken())) {
-    extractor_name = Symbols::Token(CurrentToken()).raw();
-    ConsumeToken();
-  } else {
-    ReportError("identifier or operator expected");
-  }
-
-  if (primary->IsPrimaryNode() && primary->AsPrimaryNode()->IsSuper()) {
-    // TODO(hausner): implement super#m
-    ReportError("closurization of super method not yet supported");
-  }
-
-  // Handle closurization of top-level names from library prefixes, P#m
-  if (primary->IsLiteralNode() &&
-      primary->AsLiteralNode()->literal().IsLibraryPrefix()) {
-    const LibraryPrefix& prefix =
-        LibraryPrefix::Cast(primary->AsLiteralNode()->literal());
-    Object& obj = Object::Handle(Z);
-    const bool is_private_name =
-        (extractor_name.CharAt(0) == Library::kPrivateIdentifierStart);
-    if (!is_private_name) {
-      // Private names are not exported by libraries. The name mangling
-      // of private names with a library-specific suffix usually ensures
-      // that _x in library A is not found when looked up from library B.
-      // In the pathological case where a library imports itself with
-      // a prefix, the name mangling does not help in hiding the private
-      // name, so we explicitly prevent lookup of private names here.
-      if (is_setter_name) {
-        String& setter_name =
-            String::Handle(Z, Field::SetterName(extractor_name));
-        obj = prefix.LookupObject(setter_name);
+
+    if (primary->IsPrimaryNode() && primary->AsPrimaryNode()->IsSuper()) {
+      // TODO(hausner): implement super#m
+      ReportError("closurization of super method not yet supported");
+    }
+
+    // Handle closurization of top-level names from library prefixes, P#m
+    if (primary->IsLiteralNode() &&
+        primary->AsLiteralNode()->literal().IsLibraryPrefix()) {
+      const LibraryPrefix& prefix =
+          LibraryPrefix::Cast(primary->AsLiteralNode()->literal());
+      Object& obj = Object::Handle(Z);
+      const bool is_private_name =
+          (extractor_name.CharAt(0) == Library::kPrivateIdentifierStart);
+      if (!is_private_name) {
+        // Private names are not exported by libraries. The name mangling
+        // of private names with a library-specific suffix usually ensures
+        // that _x in library A is not found when looked up from library B.
+        // In the pathological case where a library imports itself with
+        // a prefix, the name mangling does not help in hiding the private
+        // name, so we explicitly prevent lookup of private names here.
+        if (is_setter_name) {
+          String& setter_name =
+              String::Handle(Z, Field::SetterName(extractor_name));
+          obj = prefix.LookupObject(setter_name);
+        }
+        if (obj.IsNull()) {
+          obj = prefix.LookupObject(extractor_name);
+        }
       }
-      if (obj.IsNull()) {
-        obj = prefix.LookupObject(extractor_name);
+      if (!prefix.is_loaded() && (parsed_function() != NULL) &&
+          !FLAG_load_deferred_eagerly) {
+        // Remember that this function depends on an import prefix of an
+        // unloaded deferred library.
+        parsed_function()->AddDeferredPrefix(prefix);
       }
-    }
-    if (!prefix.is_loaded() && (parsed_function() != NULL) &&
-        !FLAG_load_deferred_eagerly) {
-      // Remember that this function depends on an import prefix of an
-      // unloaded deferred library.
-      parsed_function()->AddDeferredPrefix(prefix);
-    }
 
-    if (obj.IsFunction()) {
-      const Function& func = Function::Cast(obj);
-      if (!func.IsSetterFunction() || is_setter_name) {
-        return CreateImplicitClosureNode(func, property_pos, NULL);
-      }
-    } else if (obj.IsField()) {
-      const Field& field = Field::Cast(obj);
-      if (is_setter_name && !field.is_final()) {
-        Instance& setter_closure = Instance::ZoneHandle(field.SetterClosure());
-        return new (Z) LiteralNode(property_pos, setter_closure);
-      }
-      if (!is_setter_name) {
-        Instance& getter_closure = Instance::ZoneHandle(field.GetterClosure());
-        return new (Z) LiteralNode(property_pos, getter_closure);
-      }
-    }
-    return ThrowNoSuchMethodError(
-        property_pos, current_class(), extractor_name,
-        NULL,  // No arguments.
-        InvocationMirror::kTopLevel,
-        is_setter_name ? InvocationMirror::kSetter : InvocationMirror::kMethod,
-        NULL);  // No existing function.
-  }
-
-  // Handle closurization of static properties of classes, C#n.
-  if (primary->IsPrimaryNode() &&
-      primary->AsPrimaryNode()->primary().IsClass()) {
-    const Class& cls = Class::Cast(primary->AsPrimaryNode()->primary());
-    const Field& field =
-        Field::Handle(Z, cls.LookupStaticField(extractor_name));
-    if (!field.IsNull()) {
-      if (is_setter_name) {
-        extractor_name = Field::SetterName(extractor_name);
-        if (!field.is_final()) {
+      if (obj.IsFunction()) {
+        const Function& func = Function::Cast(obj);
+        if (!func.IsSetterFunction() || is_setter_name) {
+          return CreateImplicitClosureNode(func, property_pos, NULL);
+        }
+      } else if (obj.IsField()) {
+        const Field& field = Field::Cast(obj);
+        if (is_setter_name && !field.is_final()) {
           const Instance& setter_closure =
-              Instance::ZoneHandle(Z, field.SetterClosure());
-          ASSERT(setter_closure.IsClosure());
-          // Note: the created closure is cached after it's created
-          // once. If eager compilation is desired, the compiler can
-          // be invoked here. The same applies for getters below.
+              Instance::ZoneHandle(field.SetterClosure());
           return new (Z) LiteralNode(property_pos, setter_closure);
         }
-      } else {
-        const Instance& getter_closure =
-            Instance::ZoneHandle(Z, field.GetterClosure());
-        ASSERT(getter_closure.IsClosure());
-        return new (Z) LiteralNode(property_pos, getter_closure);
+        if (!is_setter_name) {
+          const Instance& getter_closure =
+              Instance::ZoneHandle(field.GetterClosure());
+          return new (Z) LiteralNode(property_pos, getter_closure);
+        }
       }
-    } else {
-      Function& func = Function::Handle(Z);
-      if (is_setter_name) {
-        extractor_name = Field::SetterName(extractor_name);
-        func = cls.LookupStaticFunction(extractor_name);
+      return ThrowNoSuchMethodError(property_pos, current_class(),
+                                    extractor_name,
+                                    NULL,  // No arguments.
+                                    InvocationMirror::kTopLevel,
+                                    is_setter_name ? InvocationMirror::kSetter
+                                                   : InvocationMirror::kMethod,
+                                    NULL);  // No existing function.
+    }
+
+    // Handle closurization of static properties of classes, C#n.
+    if (primary->IsPrimaryNode() &&
+        primary->AsPrimaryNode()->primary().IsClass()) {
+      const Class& cls = Class::Cast(primary->AsPrimaryNode()->primary());
+      const Field& field =
+          Field::Handle(Z, cls.LookupStaticField(extractor_name));
+      if (!field.IsNull()) {
+        if (is_setter_name) {
+          extractor_name = Field::SetterName(extractor_name);
+          if (!field.is_final()) {
+            const Instance& setter_closure =
+                Instance::ZoneHandle(Z, field.SetterClosure());
+            ASSERT(setter_closure.IsClosure());
+            // Note: the created closure is cached after it's created
+            // once. If eager compilation is desired, the compiler can
+            // be invoked here. The same applies for getters below.
+            return new (Z) LiteralNode(property_pos, setter_closure);
+          }
+        } else {
+          const Instance& getter_closure =
+              Instance::ZoneHandle(Z, field.GetterClosure());
+          ASSERT(getter_closure.IsClosure());
+          return new (Z) LiteralNode(property_pos, getter_closure);
+        }
       } else {
-        func = cls.LookupStaticFunction(extractor_name);
-        if (func.IsNull()) {
-          const String& getter_name =
-              String::Handle(Z, Field::GetterName(extractor_name));
-          func = cls.LookupStaticFunction(getter_name);
+        Function& func = Function::Handle(Z);
+        if (is_setter_name) {
+          extractor_name = Field::SetterName(extractor_name);
+          func = cls.LookupStaticFunction(extractor_name);
+        } else {
+          func = cls.LookupStaticFunction(extractor_name);
+          if (func.IsNull()) {
+            const String& getter_name =
+                String::Handle(Z, Field::GetterName(extractor_name));
+            func = cls.LookupStaticFunction(getter_name);
+          }
+        }
+        if (!func.IsNull()) {
+          return CreateImplicitClosureNode(func, property_pos, NULL);
         }
       }
-      if (!func.IsNull()) {
-        return CreateImplicitClosureNode(func, property_pos, NULL);
-      }
+      return ThrowNoSuchMethodError(property_pos, cls, extractor_name,
+                                    NULL,  // No arguments.
+                                    InvocationMirror::kStatic,
+                                    is_setter_name ? InvocationMirror::kSetter
+                                                   : InvocationMirror::kMethod,
+                                    NULL);  // No existing function.
     }
-    return ThrowNoSuchMethodError(
-        property_pos, cls, extractor_name,
-        NULL,  // No arguments.
-        InvocationMirror::kStatic,
-        is_setter_name ? InvocationMirror::kSetter : InvocationMirror::kMethod,
-        NULL);  // No existing function.
-  }
 
-  // Closurization of instance getter, setter, method or operator.
-  GrowableHandlePtrArray<const String> pieces(Z, 3);
-  pieces.Add(Symbols::HashMark());
-  if (is_setter_name) {
-    pieces.Add(Symbols::SetterPrefix());
+    // Closurization of instance getter, setter, method or operator.
+    GrowableHandlePtrArray<const String> pieces(Z, 3);
+    pieces.Add(Symbols::HashMark());
+    if (is_setter_name) {
+      pieces.Add(Symbols::SetterPrefix());
+    }
+    pieces.Add(extractor_name);
+    extractor_name = Symbols::FromConcatAll(T, pieces);
+    return new (Z) InstanceGetterNode(property_pos, primary, extractor_name);
   }
-  pieces.Add(extractor_name);
-  extractor_name = Symbols::FromConcatAll(T, pieces);
-  return new (Z) InstanceGetterNode(property_pos, primary, extractor_name);
 }
 
 
@@ -13348,41 +13354,40 @@ void Parser::ParseConstructorClosurization(Function* constructor,
   // type that is loaded.
   ASSERT(prefix.IsNull() || prefix.is_loaded());
   ASSERT(!type.IsMalformed() && !type.IsTypeParameter());
-  if (FLAG_warn_new_tearoff_syntax) {
-    ReportWarning(
-        "Tear-offs using the x#id syntax is a deprecated feature,"
-        "it will not be supported in the next release");
-  }
-  ExpectToken(Token::kHASH);
-  String* named_constructor = NULL;
-  if (IsIdentifier()) {
-    named_constructor = CurrentLiteral();
-    ConsumeToken();
-  }
-  // Resolve the type and optional identifier to a constructor or factory.
-  Class& type_class = Class::Handle(Z, type.type_class());
-  String& type_class_name = String::Handle(Z, type_class.Name());
-  *type_arguments = type.arguments();
-  String& constructor_name =
-      BuildConstructorName(T, type_class_name, named_constructor);
-  *constructor = type_class.LookupConstructor(constructor_name);
-  if (constructor->IsNull()) {
-    *constructor = type_class.LookupFactory(constructor_name);
-    ASSERT(!constructor->IsNull());
-    if (constructor->IsRedirectingFactory()) {
-      ClassFinalizer::ResolveRedirectingFactory(type_class, *constructor);
-      type = constructor->RedirectionType();
-      ASSERT(!type.IsMalformedOrMalbounded());
-      if (!type.IsInstantiated()) {
-        Error& error = Error::Handle(Z);
-        type ^= type.InstantiateFrom(*type_arguments, &error,
-                                     NULL,  // instantiation_trail
-                                     NULL,  // bound_trail
-                                     Heap::kOld);
-        ASSERT(error.IsNull());
+  if (!FLAG_support_new_tearoff_syntax) {
+    ReportError("Tear-offs using the x#id syntax is a deprecated feature");
+  } else {

[regis, 2016/12/29 18:24:07]

ditto

[siva, 2016/12/29 20:59:56]

Done.

+    ExpectToken(Token::kHASH);
+    String* named_constructor = NULL;
+    if (IsIdentifier()) {
+      named_constructor = CurrentLiteral();
+      ConsumeToken();
+    }
+    // Resolve the type and optional identifier to a constructor or factory.
+    Class& type_class = Class::Handle(Z, type.type_class());
+    String& type_class_name = String::Handle(Z, type_class.Name());
+    *type_arguments = type.arguments();
+    String& constructor_name =
+        BuildConstructorName(T, type_class_name, named_constructor);
+    *constructor = type_class.LookupConstructor(constructor_name);
+    if (constructor->IsNull()) {
+      *constructor = type_class.LookupFactory(constructor_name);
+      ASSERT(!constructor->IsNull());
+      if (constructor->IsRedirectingFactory()) {
+        ClassFinalizer::ResolveRedirectingFactory(type_class, *constructor);
+        type = constructor->RedirectionType();
+        ASSERT(!type.IsMalformedOrMalbounded());
+        if (!type.IsInstantiated()) {
+          Error& error = Error::Handle(Z);
+          type ^= type.InstantiateFrom(*type_arguments, &error,
+                                       NULL,  // instantiation_trail
+                                       NULL,  // bound_trail
+                                       Heap::kOld);
+          ASSERT(error.IsNull());
+        }
+        *type_arguments = type.arguments();
+        *constructor = constructor->RedirectionTarget();
       }
-      *type_arguments = type.arguments();
-      *constructor = constructor->RedirectionTarget();
     }
   }
 }
@@ -13451,14 +13456,13 @@ AstNode* Parser::ParseNewOperator(Token::Kind op_kind) {
     if (is_const) {
       ReportError("tear-off closure not allowed with const allocation");
     }
-    if (FLAG_warn_new_tearoff_syntax) {
-      ReportWarning(
-          "Tear-offs using the x#id syntax is a deprecated feature,"
-          "and will not be supported in the next release");
-    }
-    ConsumeToken();
-    if (IsIdentifier()) {
-      named_constructor = ExpectIdentifier("name of constructor expected");
+    if (!FLAG_support_new_tearoff_syntax) {
+      ReportError("Tear-offs using the x#id syntax is a deprecated feature");
+    } else {

[regis, 2016/12/29 18:24:07]

ditto

[siva, 2016/12/29 20:59:56]

Done.

+      ConsumeToken();
+      if (IsIdentifier()) {
+        named_constructor = ExpectIdentifier("name of constructor expected");
+      }
     }
   } else if (CurrentToken() == Token::kPERIOD) {
     ConsumeToken();
@@ -13893,13 +13897,14 @@ AstNode* Parser::ParsePrimary() {
     const LibraryPrefix& prefix = LibraryPrefix::ZoneHandle(Z, ParsePrefix());
     if (!prefix.IsNull()) {
       if (CurrentToken() == Token::kHASH) {
-        if (FLAG_warn_new_tearoff_syntax) {
-          ReportWarning(
-              "Tear-offs using the x#id syntax is a deprecated feature,"
-              "it will not be supported in the next release");
+        if (!FLAG_support_new_tearoff_syntax) {
+          ReportError(
+              "Tear-offs using the x#id syntax "
+              "is a deprecated feature");
+        } else {

[regis, 2016/12/29 18:24:07]

ditto

[siva, 2016/12/29 20:59:56]

Done.

+          // Closurization of top-level entity in prefix scope.
+          return new (Z) LiteralNode(qual_ident_pos, prefix);
         }
-        // Closurization of top-level entity in prefix scope.
-        return new (Z) LiteralNode(qual_ident_pos, prefix);
       } else {
         ExpectToken(Token::kPERIOD);
       }