Closed
Bug 780680
Opened 12 years ago
Closed 12 years ago
gfxHarfBuzzShaper.cpp:1197:21: warning: variable ‘prevGlyphCharIndex’ set but not used [-Wunused-but-set-variable]
Categories
(Core :: Graphics: Text, defect)
Core
Graphics: Text
Tracking
()
RESOLVED
FIXED
mozilla17
People
(Reporter: dholbert, Assigned: dholbert)
References
(Blocks 1 open bug)
Details
Attachments
(1 file)
1.46 KB,
patch
|
jfkthame
:
review+
|
Details | Diff | Splinter Review |
../../../mozilla/gfx/thebes/gfxHarfBuzzShaper.cpp: In member function ‘nsresult gfxHarfBuzzShaper::SetGlyphsFromRun(gfxContext*, gfxShapedWord*, hb_buffer_t*)’: ../../../mozilla/gfx/thebes/gfxHarfBuzzShaper.cpp:1197:21: warning: variable ‘prevGlyphCharIndex’ set but not used [-Wunused-but-set-variable] This is from bug 249159 part 4: https://hg.mozilla.org/mozilla-central/rev/1dbfff7cf0ab which removed the last usage of this variable.
Assignee | ||
Comment 1•12 years ago
|
||
Assignee | ||
Comment 2•12 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #0) > This is from bug 249159 part 4: > https://hg.mozilla.org/mozilla-central/rev/1dbfff7cf0ab > which removed the last usage of this variable. (Incidentally, gfxCoreTextShaper.cpp has some veeeerry similar- https://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxCoreTextShaper.cpp#376
Severity: normal → trivial
OS: Linux → All
Hardware: x86_64 → All
Assignee | ||
Comment 3•12 years ago
|
||
(In reply to Daniel Holbert [:dholbert] from comment #2) > (Incidentally, gfxCoreTextShaper.cpp has some veeeerry similar- > https://mxr.mozilla.org/mozilla-central/source/gfx/thebes/gfxCoreTextShaper. > cpp#376 (erm, probably ignore that -- didn't mean to post it. I was starting to compose a comment about very-similar code (prevGlyphCharIndex, which is used to set "inOrder", which is used to in a call to g.SetComplex()), and I was wondering if that code might've wanted the same treatment that 1dbfff7cf0ab provided to gfxHarfBuzzShaper.cpp. But I'm guessing it wasn't missed & this is a just a HarfBuzz vs. CoreText difference...?)
Comment 4•12 years ago
|
||
Comment on attachment 649343 [details] [diff] [review] fix Yes, that variable is clearly obsolete here; let's kill it. As for the similar-looking code in gfxCoreTextShaper - it's possible that we could/should make a comparable change there, but I wouldn't want to attempt that without careful re-reading of the code and creating testcases specifically designed to hit the various edge cases. (This stuff is a bit hairy, and the underlying shapers don't necessarily behave in exactly the same way...) Let's leave well alone unless someone runs across a specific issue that prompts us to revisit it.
Attachment #649343 -
Flags: review?(jfkthame) → review+
Assignee | ||
Comment 5•12 years ago
|
||
Sounds good, thanks! I'll land this once m-i reopens.
Assignee | ||
Comment 6•12 years ago
|
||
https://hg.mozilla.org/integration/mozilla-inbound/rev/759748513d64
Target Milestone: --- → mozilla17
Assignee | ||
Updated•12 years ago
|
Summary: gfxHarfBuzzShaper.cpp:1197:21: warning: variable ‘pr evGlyphCharIndex’ set but not used [-Wunused-but-set-variable] → gfxHarfBuzzShaper.cpp:1197:21: warning: variable ‘prevGlyphCharIndex’ set but not used [-Wunused-but-set-variable]
Comment 7•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/759748513d64
Status: ASSIGNED → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
You need to log in
before you can comment on or make changes to this bug.
Description
•