toolkit/components/typeheadfind - compiler warnings on mac

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
8 years ago
8 years ago

People

(Reporter: joey, Assigned: atulagrwl)

Tracking

(Blocks 1 bug)

Firefox Tracking Flags

(Not tracked)

Details

(Whiteboard: [build_warning])

Attachments

(2 obsolete attachments)

% uname -a
Darwin banshee.local 10.7.4 Darwin Kernel Version 10.7.4: Mon Apr 18 21:24:17 PDT 2011; root:xnu-1504.14.12~3/RELEASE_X86_64 x86_64

/mozilla/sandbox/gml/toolkit/components/typeaheadfind/nsTypeAheadFind.cpp:567: warning: enumeral mismatch in conditional expression: 'nsITypeAheadFind::<anonymous enum>' vs 'nsITypeAheadFind::<anonymous enum>'
/mozilla/sandbox/gml/toolkit/components/places/History.cpp:1282: warning: ignoring return value of 'bool mozilla::dom::PContentParent::SendNotifyVisited(const IPC::URI&)', declared with attribute warn_unused_result
Whiteboard: [build_warnings]
Whiteboard: [build_warnings] → [build_warning]
Posted patch Patch v1 (obsolete) — Splinter Review
Patch for first issue. This cannot be fixed in any better way (in my knowledge) as the enums are generated from idl files. Enums needs to be clubbed together to fix this issue.
Second issue does not occur now.
Assignee: nobody → atulagrwl
Attachment #556871 - Flags: review?(Ms2ger)
Can we not do this change and wait for bug 585099 instead?
Definitely, I didn't know that such a bug exists. If we can fix the root cause, there is no point in fixing the implications :).
Comment on attachment 556871 [details] [diff] [review]
Patch v1

You shouldn't introduce tabs, and the typical way to fix this warning is

      *aResult = hasWrapped
                  ? static_cast<PRUint16>(FIND_WRAPPED)
                  : static_cast<PRUint16>(FIND_FOUND);

And please ask a toolkit peer: <https://wiki.mozilla.org/Modules/Toolkit#Toolkit> to review the patch.
Attachment #556871 - Flags: review?(Ms2ger) → review-
Posted patch Patch v2 (obsolete) — Splinter Review
Changes suggested.
Attachment #556871 - Attachment is obsolete: true
Attachment #558557 - Flags: review?(dolske)
Err, isn't the implication from comment 2 that the about-to-land bug 585099 will fix this without code changes?
Depends on: 585099
Comment on attachment 558557 [details] [diff] [review]
Patch v2

Clearing review on the assumption(?) 585099 eliminates the need for this.
Attachment #558557 - Flags: review?(dolske)
Attachment #558557 - Attachment is obsolete: true
Marking the bug as Resolved/Fixed as the dependent bug fix has been checked-in.
Status: NEW → RESOLVED
Closed: 8 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.