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

Status: RESOLVED WORKSFORME

(Core :: CSS Parsing and Computation, defect, P4)

Description

[Nathan Froyd [:froydnj]]

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

[Chris H-C :chutten]

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

[Chris H-C :chutten]

Whaddayaknow, it built. Yay!

Comment 3

[Chris Peterson [:cpeterson]]

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

Comment 4

[Emilio Cobos Álvarez (: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.

Comment 5

[Chris Peterson [:cpeterson]]

status-firefox57=wontfix unless someone thinks this bug should block 57

Comment 6

[Chris Peterson [:cpeterson]]

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.