Update documentation on Symbol. Make validation match.

sdk/lib/internal/symbol.dart

 // Copyright (c) 2013, 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.
 
 part of dart._internal;
 
 /**
  * Implementation of [core.Symbol].  This class uses the same name as
  * a core class so a user can't tell the difference.
  *
  * The purpose of this class is to hide [_name] from user code, but
  * make it accessible to Dart platform code via the static method
  * [getName].
  */
 class Symbol implements core.Symbol {
   final String _name;
 
+  // Reserved words are not allowed as identifiers.
+  static const String reservedWord =
+      r'assert|break|c(?:a(?:se|tch)|lass|on(?:st|tinue))|d(?:efault|o)|'

[floitsch, 2014/01/31 14:37:24]

I would have just written them out. It would be almost 30chars shorter, and a
good regexp-engine should just transform it into a KMP or Boyer-Moore search.

[Lasse Reichstein Nielsen, 2014/01/31 18:19:26]

This is being used as part of another regexp, so no searching will be done. 
A good regexp engine might be able to make something just as fast anyway. I
should do a benchmark :)

[floitsch, 2014/01/31 18:23:06]

That's not necessarily the case. A good regexp engine could definitely use one
of the good algorithms in the middle of a bigger regexp. But apparently they
don't do yet. (You should know ;)

[Lasse Reichstein Nielsen, 2014/01/31 20:00:43]

They could use, e.g., KMP or BMH for searching (and they don't), but in this
case we are not *searching* (there is no .* or similar before). It's just being
used to do a single (negative) match at an already found position.

[floitsch, 2014/01/31 20:07:48]

Right.
But that makes it even easier. A good regexp should do exactly what you did by
hand.

+      r'e(?:lse|num|xtends)|f(?:alse|inal(?:ly)?|or)|i[fns]|n(?:ew|ull)|'
+      r'ret(?:hrow|urn)|s(?:uper|witch)|t(?:h(?:is|row)|r(?:ue|y))|'
+      r'v(?:ar|oid)|w(?:hile|ith)';

[Lasse Reichstein Nielsen, 2014/01/31 23:17:04]

Did a benchmark. This version is ~14% faster than the plain version in
Chrome/v8.

[floitsch, 2014/01/31 23:41:44]

I guess that means that we should file a bug. It clearly shouldn't.

+
   static final RegExp validationPattern =
-      new RegExp(r'^(?:[a-zA-Z$][a-zA-Z$0-9_]*\.)*(?:[a-zA-Z$][a-zA-Z$0-9_]*=?|'
-                 r'-|'
+      new RegExp(r'^(?:'
+                 r'[\-+*/%&|^]|'
+                 r'\[\]=?|'
+                 r'==|'
+                 r'~/?|'
+                 r'<[<=]?|'
+                 r'>[>=]?|'
                  r'unary-|'
-                 r'\[\]=|'
-                 r'~|'
-                 r'==|'
-                 r'\[\]|'
-                 r'\*|'
-                 r'/|'
-                 r'%|'
-                 r'~/|'
-                 r'\+|'
-                 r'<<|'
-                 r'>>|'
-                 r'>=|'
-                 r'>|'
-                 r'<=|'
-                 r'<|'
-                 r'&|'
-                 r'\^|'
-                 r'\|'
+                 r'(?!(?:''$reservedWord'r')\b)[a-zA-Z$][\w$]*'

[floitsch, 2014/01/31 14:37:24]

this is hard to read (the ''...'r'). Maybe use "+", or add spaces, or new lines.

[floitsch, 2014/01/31 14:37:24]

You have to explain how negative lookahead works.
In general: I'm pretty good at regexps, and it took me some time to understand
this line. You really have to document it better.

Also use indentation to show where parenthesis start/end.

[Lasse Reichstein Nielsen, 2014/01/31 18:19:26]

Spaces or newlines will do. String plus is not a compile-time constant
operation, so I try to avoid it in expressions that would otherwise be constant.

[Lasse Reichstein Nielsen, 2014/01/31 18:19:26]

Ok, will add more comments.

+                 r'(?:=?|(?:\.(?!(?:''$reservedWord'r')\b)[a-zA-Z_$][\w$]*)*)'
                  r')$');
 
   external const Symbol(String name);
 
   /**
    * Platform-private method used by the mirror system to create
    * otherwise invalid names.
    */
   const Symbol.unvalidated(this._name);
 
@@ skipping 18 matching lines @@
     if (name.startsWith('_')) {
       throw new ArgumentError('"$name" is a private identifier');
     }
     if (!validationPattern.hasMatch(name)) {
       throw new ArgumentError(
           '"$name" is not an identifier or an empty String');
     }
     return name;
   }
 }