| Summary: | Extract StringImpl::computeHashAndFlags() function for passing to StringImplShape constructor | ||||||||
|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
| Component: | Web Template Framework | Assignee: | David Kilzer (:ddkilzer) <ddkilzer> | ||||||
| Status: | NEW --- | ||||||||
| Severity: | Normal | CC: | benjamin, cdumez, cmarcelo, darin, ews-watchlist, webkit-bug-importer | ||||||
| Priority: | P2 | Keywords: | InRadar | ||||||
| Version: | Other | ||||||||
| Hardware: | Unspecified | ||||||||
| OS: | Unspecified | ||||||||
| Bug Depends on: | 217805 | ||||||||
| Bug Blocks: | |||||||||
| Attachments: |
|
||||||||
|
Description
David Kilzer (:ddkilzer)
2020-10-16 12:22:03 PDT
Created attachment 411604 [details]
Patch v1
(In reply to David Kilzer (:ddkilzer) from comment #1) > Created attachment 411604 [details] > Patch v1 LOL...posted this patch too soon. Fix for Bug 217805 hasn't landed yet. Created attachment 411606 [details]
Patch v1 redo
Comment on attachment 411606 [details] Patch v1 redo View in context: https://bugs.webkit.org/attachment.cgi?id=411606&action=review > Source/WTF/wtf/text/StringImpl.h:213 > + static constexpr unsigned computeHashAndFlags(unsigned hashBase, StringKind stringKind, BufferOwnership bufferOwnership, unsigned extraBits = 0) Not thrilled with passing in "hashBase", which is effectively also an enumeration, as an untyped unsigned. > Source/WTF/wtf/text/StringImpl.h:1240 > + computeHashAndFlags(s_hashFlag8BitBuffer | s_hashFlagDidReportCost, stringKind, BufferInternal, (StringHasher::computeLiteralHashAndMaskTop8Bits(characters) << s_flagCount)), ConstructWithConstExpr) Don’t need those parentheses around the last argument. Needed them before because they were part of an expression, but after a comma it’s not helpful, and in fact it makes it harder to read! Comment on attachment 411606 [details] Patch v1 redo View in context: https://bugs.webkit.org/attachment.cgi?id=411606&action=review >> Source/WTF/wtf/text/StringImpl.h:213 >> + static constexpr unsigned computeHashAndFlags(unsigned hashBase, StringKind stringKind, BufferOwnership bufferOwnership, unsigned extraBits = 0) > > Not thrilled with passing in "hashBase", which is effectively also an enumeration, as an untyped unsigned. The more I stare at `enum BufferOwnership` and `enum StringKind`, the more I realize that they're really all just StringAttributes, and probably should be a single enum with an OptionSet<>. Will think about this some more before landing. >> Source/WTF/wtf/text/StringImpl.h:1240 >> + computeHashAndFlags(s_hashFlag8BitBuffer | s_hashFlagDidReportCost, stringKind, BufferInternal, (StringHasher::computeLiteralHashAndMaskTop8Bits(characters) << s_flagCount)), ConstructWithConstExpr) > > Don’t need those parentheses around the last argument. Needed them before because they were part of an expression, but after a comma it’s not helpful, and in fact it makes it harder to read! Will do. |