Summary: | JSC: bug fixes and enhancements for OfflineASM annotation system | ||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Mark Lam <mark.lam> | ||||||||
Component: | JavaScriptCore | Assignee: | Nobody <webkit-unassigned> | ||||||||
Status: | RESOLVED FIXED | ||||||||||
Severity: | Normal | CC: | fpizlo, ggaren, webkit.review.bot | ||||||||
Priority: | P2 | ||||||||||
Version: | 528+ (Nightly build) | ||||||||||
Hardware: | Unspecified | ||||||||||
OS: | Unspecified | ||||||||||
Attachments: |
|
Description
Mark Lam
2012-07-18 16:17:04 PDT
Created attachment 153132 [details]
Fix.
Comment on attachment 153132 [details] Fix. View in context: https://bugs.webkit.org/attachment.cgi?id=153132&action=review > Source/JavaScriptCore/offlineasm/parser.rb:94 > + annotationType = whitespaceFound ? "@L" : "@G" # local or global annotation It is customary to use symbols (:thing) rather than strings ("thing") for enumerating in ruby. > Source/JavaScriptCore/offlineasm/parser.rb:99 > - result << Token.new(CodeOrigin.new(fileName, lineNumber), "@" + annotation) > + result << Token.new(CodeOrigin.new(fileName, lineNumber), > + "#{annotationType}#{annotation}") I don't like the idea of using strings with magic as the contents of a token. Tokens are supposed to contain the string that was parsed. We already have a slightly ugly hack for numbers (we convert the string to a number then back to a decimal string) but I don't want that to spread. Created attachment 153151 [details]
rev 1: addressed Filip's concern. No longer play tricks with string encoding.
Created attachment 153203 [details]
rev 2: removed an unneeded OOPS comment in the last patch.
Comment on attachment 153203 [details] rev 2: removed an unneeded OOPS comment in the last patch. Clearing flags on attachment: 153203 Committed r123147: <http://trac.webkit.org/changeset/123147> All reviewed patches have been landed. Closing bug. |