Closed Bug 1243331 Opened 4 years ago Closed 4 years ago

[gcc 4.8] error: typedef '_GStaticAssertCompileTimeAssertion_0' locally defined but not used [-Werror=unused-local-typedefs]

Categories

(Core :: Disability Access APIs, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: glandium, Assigned: sfink)

References

Details

Attachments

(1 file, 1 obsolete file)

Build fails on try with GCC 4.8.5 (with --enable-warnings-as-errors):

/home/worker/workspace/build/src/accessible/atk/AtkSocketAccessible.cpp:25:489: error: typedef '_GStaticAssertCompileTimeAssertion_0' locally defined but not used [-Werror=unused-local-typedefs]
/home/worker/workspace/build/src/accessible/atk/AtkSocketAccessible.cpp:25:691: error: typedef '_GStaticAssertCompileTimeAssertion_1' locally defined but not used [-Werror=unused-local-typedefs]
/home/worker/workspace/build/src/accessible/atk/AtkSocketAccessible.cpp:25:1496: error: typedef '_GStaticAssertCompileTimeAssertion_2' locally defined but not used [-Werror=unused-local-typedefs] 

https://treeherder.mozilla.org/logviewer.html#?job_id=15989654&repo=try
Fails equally with GCC 4.9.3.
Blocks: gcc-4.9
Trev, is this something you worked on recently?
Flags: needinfo?(tbsaunde+mozbugs)
(In reply to Mike Hommey [:glandium] from comment #0)
> Build fails on try with GCC 4.8.5 (with --enable-warnings-as-errors):
> 
> /home/worker/workspace/build/src/accessible/atk/AtkSocketAccessible.cpp:25:
> 489: error: typedef '_GStaticAssertCompileTimeAssertion_0' locally defined
> but not used [-Werror=unused-local-typedefs]
> /home/worker/workspace/build/src/accessible/atk/AtkSocketAccessible.cpp:25:
> 691: error: typedef '_GStaticAssertCompileTimeAssertion_1' locally defined
> but not used [-Werror=unused-local-typedefs]
> /home/worker/workspace/build/src/accessible/atk/AtkSocketAccessible.cpp:25:
> 1496: error: typedef '_GStaticAssertCompileTimeAssertion_2' locally defined
> but not used [-Werror=unused-local-typedefs] 

So this is coming from the G_DEFINE_TYPE_EXTENDED macro, which I believe comes from glib / gobject.  I can't reproduce this locally with gcc 5, so I'm guessing its fixed in newer gobject headers.  Its probably easiest to just #pragma diagnostic our way to victory.
Flags: needinfo?(tbsaunde+mozbugs)
Bug 1117259 added -Wno-unused-local-typedef to configure.in [1], but "-Wunused-local-typedef" is clang's warning name. gcc's name is "-Wunused-local-typedefs" (plural) [2]. We should use -Wunused-local-typedefs because clang accepts it as an alias [3].

Also, since both clang and gcc 4.7+ accept "-Wno-unused-local-typedefs", we no longer need the MOZ_CXX_SUPPORTS_WARNING check (assuming our minimum gcc version is 4.7).

[1] https://mxr.mozilla.org/mozilla-central/source/configure.in#1591
[2] https://gcc.gnu.org/onlinedocs/gcc-4.8.0/gcc/Warning-Options.html
[3] https://github.com/llvm-mirror/clang/blob/master/include/clang/Basic/DiagnosticGroups.td#L688
Blocks: 1117259
(In reply to Chris Peterson [:cpeterson] from comment #4)
> Bug 1117259 added -Wno-unused-local-typedef to configure.in [1], but
> "-Wunused-local-typedef" is clang's warning name. gcc's name is
> "-Wunused-local-typedefs" (plural) [2]. We should use
> -Wunused-local-typedefs because clang accepts it as an alias [3].

It seems to me it would be better to keep this warning enabled especially if this is the only problem case.
Yeah this is a typo.  I have a fix in bug 1243604.
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → DUPLICATE
Duplicate of bug: 1243604
Status: RESOLVED → REOPENED
Resolution: DUPLICATE → ---
There seems to be a fair amount of discussion about the right way to fix this (wait for upstream etc.) but I really need to upgrade gcc, so I'd like to just suppress this warning for now.
Attachment #8713761 - Flags: review?(mh+mozilla)
Assignee: nobody → sphink
Status: REOPENED → ASSIGNED
Comment on attachment 8713761 [details] [diff] [review]
Prevent G_DEFINE_TYPE_EXTENDED macro from producing a fatal warning

>-if CONFIG['CLANG_CXX']:
>-    # Suppress clang warning about unused function from gobject's RTTI macros.
>-    CXXFLAGS += ['-Wno-unused-function']

aren't you accidentally reenablingthis warning?

>+if CONFIG['CLANG_CXX'] or CONFIG['GNU_CXX']:
>+    # Used in G_DEFINE_TYPE_EXTENDED macro, probably fixed in newer glib /
>+    # gobject headers. See bug 1243331 comment 3.
>+    CXXFLAGS += ['-Wno-unused-local-typedefs']

I'd prefer #pragma just around the macro use, but I'm fine with this if you explain the unused function issue.
Comment on attachment 8713761 [details] [diff] [review]
Prevent G_DEFINE_TYPE_EXTENDED macro from producing a fatal warning

Review of attachment 8713761 [details] [diff] [review]:
-----------------------------------------------------------------

::: accessible/atk/moz.build
@@ +51,5 @@
>      CXXFLAGS += CONFIG['MOZ_DBUS_CFLAGS']
>  
>  include('/ipc/chromium/chromium-config.mozbuild')
>  
> +if CONFIG['CLANG_CXX'] or CONFIG['GNU_CXX']:

You can just use `if CONFIG['GNU_CXX']:` because clang defines both CLANG_CXX and GNU_CXX. To add clang-only flags, we have to check `if CONFIG['CLANG_CXX'] and not CONFIG['GNU_CXX']:`.
Uh, yeah, thanks! I really ought to read patches before submitting them for review.
Attachment #8713829 - Flags: review?(mh+mozilla)
Attachment #8713761 - Attachment is obsolete: true
Attachment #8713761 - Flags: review?(mh+mozilla)
(In reply to Chris Peterson [:cpeterson] from comment #9)
> Comment on attachment 8713761 [details] [diff] [review]
> Prevent G_DEFINE_TYPE_EXTENDED macro from producing a fatal warning
> 
> Review of attachment 8713761 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> ::: accessible/atk/moz.build
> @@ +51,5 @@
> >      CXXFLAGS += CONFIG['MOZ_DBUS_CFLAGS']
> >  
> >  include('/ipc/chromium/chromium-config.mozbuild')
> >  
> > +if CONFIG['CLANG_CXX'] or CONFIG['GNU_CXX']:
> 
> You can just use `if CONFIG['GNU_CXX']:` because clang defines both
> CLANG_CXX and GNU_CXX. To add clang-only flags, we have to check `if
> CONFIG['CLANG_CXX'] and not CONFIG['GNU_CXX']:`.

I'd say its better to explicitly check for gcc or clang, and that'll take care of things like clang-cl where GNUCXX is not defined.

Of course in this particular case we could probably do without the check all together, I kind of doubt anybody tries to compile this code with something other than gcc or clang.
Comment on attachment 8713829 [details] [diff] [review]
Prevent G_DEFINE_TYPE_EXTENDED macro from producing a fatal warning

I guess this technically build config, but I'll claim its a11y enough that its not unreasonable for me to review, and save glandium 5 seconds.
Attachment #8713829 - Flags: review?(mh+mozilla) → review+
https://hg.mozilla.org/mozilla-central/rev/e187d1b2244f
Status: ASSIGNED → RESOLVED
Closed: 4 years ago4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.