stylo: -Wreturn-type-c-linkage warning in ServoBindings.h

RESOLVED WORKSFORME

Status

()

Core
CSS Parsing and Computation
P4
normal
RESOLVED WORKSFORME
9 months ago
7 months ago

People

(Reporter: froydnj, Unassigned)

Tracking

Trunk
Points:
---

Firefox Tracking Flags

(firefox57 wontfix)

Details

(Reporter)

Description

9 months ago
9:40 AM <chutten> Now to work on why const nsTArray<nsString>& being incompatible with C matters _at all_
9:47 AM <froydnj> chutten: "incompatible with C"?
10:02 AM <chutten> home/chutten/mozilla-central/obj-debug-x86_64-pc-linux-gnu/dist/include/mozilla/ServoBindings.h:338:27: warning: 'Gecko_CounterStyle_GetSymbols' has C-linkage specified, but returns user-defined type 'const nsTArray<nsString> &' which is incompatible with C [-Wreturn-type-c-linkage], err: false
10:03 AM <chutten> froydnj: ^ Happening when trying to build debug linux 64 for me. 
10:03 AM <froydnj> chutten: what compiler and version?
10:03 AM <chutten> Welll
10:04 AM <chutten> It complained about my version of clang,llvm so I grabbed 3.9 but am not 100% sure it's using it properly
10:06 AM <froydnj> ok
10:07 AM <froydnj> ah, so this is a reasonably new error, code is like a week old
10:07 AM <froydnj> I'm a little surprised this error doesn't cause problems in automation

It's not obvious to me why this sort of problem isn't coming up in automation.  We turn off -Wreturn-type-c-linkage already, but maybe those flags aren't getting passed to bindgen somehow?

Should be an easy fix, we just need to change the return type of Gecko_CounterStyle_GetSymbols to a pointer, rather than a reference.  The return type is already a pointer on the Rust side, so we shouldn't even need Servo changes for this.

Comment 1

9 months ago
My mozconfig:
ac_add_options --enable-warnings-as-errors
ac_add_options --enable-debug
ac_add_options --enable-logrefcnt
ac_add_options --disable-optimize
mk_add_options MOZ_OBJDIR=@TOPSRCDIR@/obj-debug-@CONFIG_GUESS@
mk_add_options AUTOCLOBBER=1

export MOZ_ENABLE_BACKGROUND_HANG_MONITOR=1
export MOZILLA_OFFICIAL=1
export MOZ_TELEMETRY_REPORTING=1
export MOZ_CRASHREPORTER=1
export MOZ_DISABLE_CONTENT_SANDBOX=1


I commented out --enable-warnings-as-errors and have my fingers crossed. So far so good.

Comment 2

9 months ago
Whaddayaknow, it built. Yay!
Priority: -- → P4
Emilio, does Gecko_CounterStyle_GetSymbols() still return a reference (to const nsTArray<nsString>) instead of a pointer? Looks like we're not passing _WARNINGS_CXXFLAGS (that suppress -Wreturn-type-c-linkage warnings) to bindgen.

https://searchfox.org/mozilla-central/search?q=_WARNINGS_CXXFLAGS&case=true
Flags: needinfo?(emilio)
I see no Gecko_CounterStyle_GetSymbols anymore in the tree... In any case if something returning a reference is causing trouble making it return a pointer is trivial and fine IMHO.
Flags: needinfo?(emilio)
status-firefox57=wontfix unless someone thinks this bug should block 57
status-firefox57: affected → wontfix
I'm going to resolve this bug as WORKSFORME because the Gecko_CounterStyle_GetSymbols function no longer exists and I don't see any Gecko_* functions that return a reference. A few Gecko_ functions do return pointers to nsTArray<> objects.
Status: NEW → RESOLVED
Last Resolved: 7 months ago
Resolution: --- → WORKSFORME
You need to log in before you can comment on or make changes to this bug.