Hi Max,
On 11/30/2016 01:44 PM, Max Kanat-Alexander wrote:
For what it's worth, this looks good to me too. :-)
The name "mark" is a bit confusing, as are all the methods with the
word "mark" in it; I have to think for a moment about what each of
them do. Perhaps identifierStart would be better or something, along
with setIdentifierStart()?
I borrowed the name from java.io.InputStream::mark() which likewise
"remembers" current read position of the stream. The InputStream uses
the remembered position so it can reset() current position to it, while
the parser uses the remembered position so it can return the subsequence
of input from markToCurrent().
Also, "skipIdentifier" is a bit confusing, perhaps that means
advanceToNextIdentifier?
The semantics of the method is to parse the following input as an
identifier, but not give anything back as a result besides moving
current position past the parsed identifier (hence the prefix "skip").
After this method returns, the parser is typically not positioned at the
start of next identifier, but at some delimiter, so
"advanceToNextIdentifier" would mean something completely different. I
think the name is right for what it means.
Regards, Peter
On Wed, Nov 30, 2016 at 6:41 AM, Claes Redestad
<[email protected] <mailto:[email protected]>> wrote:
+1
Found a reference to getNext in a comment that should be removed,
no re-review required.
/Claes
On 2016-11-30 12:36, Peter Levart wrote:
Here's a webrev incorporating both suggestions. All 73
java/jang/reflect jtreg tests are passing...
http://cr.openjdk.java.net/~plevart/jdk9-dev/8035424_SignatureParser.performance/webrev.03/
<http://cr.openjdk.java.net/%7Eplevart/jdk9-dev/8035424_SignatureParser.performance/webrev.03/>
Regards, Peter
