Update documentation on Symbol. Make validation match.

sdk/lib/internal/symbol.dart

Index: sdk/lib/internal/symbol.dart
diff --git a/sdk/lib/internal/symbol.dart b/sdk/lib/internal/symbol.dart
index 7a0369a9a5686a9b4fb926e632cd3eced29a11cf..5683c2ceb3ae66bba6976984f746cc8e03a6bb36 100644
--- a/sdk/lib/internal/symbol.dart
+++ b/sdk/lib/internal/symbol.dart
@@ -15,28 +15,24 @@ part of dart._internal;
 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'-|'
-                 r'unary-|'
-                 r'\[\]=|'
-                 r'~|'
+      new RegExp(r'^(?:'
+                 r'[\-+*/%&|^]|'
+                 r'\[\]=?|'
                  r'==|'
-                 r'\[\]|'
-                 r'\*|'
-                 r'/|'
-                 r'%|'
-                 r'~/|'
-                 r'\+|'
-                 r'<<|'
-                 r'>>|'
-                 r'>=|'
-                 r'>|'
-                 r'<=|'
-                 r'<|'
-                 r'&|'
-                 r'\^|'
-                 r'\|'
+                 r'~/?|'
+                 r'<[<=]?|'
+                 r'>[>=]?|'
+                 r'unary-|'
+                 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);