fix misplaced declarations of mozilla::jni::Context<>::name for clang's benefit

RESOLVED FIXED in Firefox 50

Status

()

RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: froydnj, Assigned: jchen)

Tracking

Trunk
mozilla50
All
Android
Points:
---

Firefox Tracking Flags

(firefox49 affected, firefox50 fixed)

Details

Attachments

(3 attachments)

(Reporter)

Description

2 years ago
Generated Android bindings have code like:

namespace mozilla {
namespace widget {
namespace sdk {

template<> const char mozilla::jni::Context<Rect, jobject>::name[] =
        "android/graphics/Rect";

...

clang complains about this:

/opt/build/froydnj/build-android/widget/android/bindings/AndroidRect.cpp:14:61: error: cannot define or redeclare 'name' here because namespace 'sdk' does not enclose namespace 'Context<mozilla::widget::sdk::Rect, _jobject *>'
template<> const char mozilla::jni::Context<Rect, jobject>::name[] =
                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^
/opt/build/froydnj/build-android/widget/android/bindings/AndroidRect.cpp:556:62: error: cannot define or redeclare 'name' here because namespace 'sdk' does not enclose namespace 'Context<mozilla::widget::sdk::RectF, _jobject *>'
template<> const char mozilla::jni::Context<RectF, jobject>::name[] =
                      ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~^

So I guess we need to move these out a couple of levels.
(Assignee)

Updated

2 years ago
Assignee: nobody → nchen
Status: NEW → ASSIGNED
(Assignee)

Comment 1

2 years ago
Created attachment 8759731 [details] [diff] [review]
Move JNI class name out of Context (v1)

Move the class name strings into ObjectBase, so that the strings can be
overridden by derived classes in other namespaces.
Attachment #8759731 - Flags: review?(snorp)
(Assignee)

Comment 2

2 years ago
Created attachment 8759732 [details] [diff] [review]
Update auto-generated bindings (v1)
Attachment #8759732 - Flags: review+
(Assignee)

Comment 3

2 years ago
Created attachment 8759733 [details] [diff] [review]
Don't use anonymous namespace in headers (v1)

Trivial patch to not use anonymous namespace in headers, which can
introduce multiple copies of the same code in different compilation
units.
Attachment #8759733 - Flags: review+
(Reporter)

Comment 4

2 years ago
For the record, these patches do fix the compilation errors with clang.
Attachment #8759731 - Flags: review?(snorp) → review+
(Reporter)

Comment 5

2 years ago
Jim, can you land these?
Flags: needinfo?(nchen)

Comment 6

2 years ago
Pushed by nchen@mozilla.com:
https://hg.mozilla.org/integration/mozilla-inbound/rev/91ca677e4496
Move JNI class name out of Context; r=snorp
https://hg.mozilla.org/integration/mozilla-inbound/rev/6f2a56ca83e7
Update auto-generated bindings; r=me
https://hg.mozilla.org/integration/mozilla-inbound/rev/5c6ee7ceb0a9
Don't use anonymous namespace in headers; r=me
(Assignee)

Updated

2 years ago
Flags: needinfo?(nchen)

Comment 7

2 years ago
bugherder
https://hg.mozilla.org/mozilla-central/rev/91ca677e4496
https://hg.mozilla.org/mozilla-central/rev/6f2a56ca83e7
https://hg.mozilla.org/mozilla-central/rev/5c6ee7ceb0a9
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
status-firefox50: --- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla50
You need to log in before you can comment on or make changes to this bug.