Closed Bug 1199775 Opened 4 years ago Closed 4 years ago

mozilla::Tokenizer improvements, vol 2

Categories

(Core :: XPCOM, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla43
Tracking Status
firefox43 --- fixed

People

(Reporter: mayhemer, Assigned: mayhemer)

References

Details

Attachments

(1 file, 2 obsolete files)

No description provided.
Blocks: 1165269
Attached patch wip1 (obsolete) — Splinter Review
Assignee: nobody → honzab.moz
Status: NEW → ASSIGNED
Attached patch v1 (obsolete) — Splinter Review
- tokens returned by Next() and Check() now expose "fragment" which is a substring (reference, no copy!) from the input buffer representing the token
- introduced ReadChar(classifier_function) that works the same way as CheckChar(classifier_function) but also returns the value of the character read
- introduced Claim() overload that works with nsDependentCSubstring to store the result -> no memcopy


https://treeherder.mozilla.org/#/jobs?repo=try&revision=490f1b4c09f8
Attachment #8654334 - Attachment is obsolete: true
Attachment #8655441 - Flags: review?(nfroyd)
one more:
- internal cursors and state flags made protected: only to allow derived classes do more job more conveniently
Attached patch v1.1Splinter Review
slightly improved:
- HasFailed() was not in two cases reverted to false after a successful parse operation
- test enhanced according that

https://treeherder.mozilla.org/#/jobs?repo=try&revision=4079a4002f27
Attachment #8655441 - Attachment is obsolete: true
Attachment #8655441 - Flags: review?(nfroyd)
Attachment #8655972 - Flags: review?(nfroyd)
Comment on attachment 8655972 [details] [diff] [review]
v1.1

Review of attachment 8655972 [details] [diff] [review]:
-----------------------------------------------------------------

::: xpcom/ds/Tokenizer.h
@@ +79,5 @@
>      char AsChar() const;
>      nsDependentCSubstring AsString() const;
>      uint64_t AsInteger() const;
> +
> +    nsDependentCSubstring Fragment() const { return mFragment; }

I wonder if AsString and Fragment should return |const nsDependentCSubstring|...if such values are even feasible to use, depending on how const-correct ns*String's API is.  Not something to fix in this bug, but something to think about for later.  File a bug, please?
Attachment #8655972 - Flags: review?(nfroyd) → review+
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/3f38aafec98d
Status: ASSIGNED → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla43
You need to log in before you can comment on or make changes to this bug.