After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 765990 - Visual Studio: Define inline only when necessary
Visual Studio: Define inline only when necessary
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: general
unspecified
Other Windows
: Normal normal
: ---
Assigned To: gtkdev
gtkdev
Depends on:
Blocks:
 
 
Reported: 2016-05-04 14:50 UTC by Fan, Chun-wei
Modified: 2016-05-04 16:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
gmacros.h: Don't define inline as __inline on C++, nor on Visual Studio 2015+ (2.01 KB, patch)
2016-05-04 14:54 UTC, Fan, Chun-wei
none Details | Review
gmacros.h: Don't define inline as __inline on C++, nor on Visual Studio 2015+ (indented) (2.07 KB, patch)
2016-05-04 16:03 UTC, Fan, Chun-wei
committed Details | Review

Description Fan, Chun-wei 2016-05-04 14:50:36 UTC
Hi,

According to this e-mail from Nacho:

---
1>C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\include\xkeycheck.h(203): warning C4005: 'inline' : macro redefinition
1>          C:\Users\nacho\Desktop\git\dcv-ng\deps\gtk-binaries\windows\x64\include\glib-2.0\glib/gmacros.h(64) : see previous definition of 'inline'
1>C:\Program Files (x86)\Microsoft Visual Studio 12.0\VC\include\xkeycheck.h(250): fatal error C1189: #error :  The C++ Standard Library forbids macroizing keywords. Enable warning C4005 to find the forbidden macro.
---

The build of GLib under C++ (and possibly Visual Studio 2015) may be broken.  It seems that the current checks in gmacros.h for whether we need to define inline as __inline is not really enough for the case on Visual Studio and/or C++.
Comment 1 Fan, Chun-wei 2016-05-04 14:54:59 UTC
Created attachment 327299 [details] [review]
gmacros.h: Don't define inline as __inline on C++, nor on Visual Studio 2015+

Hi,

This is my proposal to address this issue.

With blessings, thank you!
Comment 2 Colin Walters 2016-05-04 15:52:07 UTC
Review of attachment 327299 [details] [review]:

It seems like this is partially walking back  db2367e8782d7a39fc3e93d13f6a16f10cad04c2 / https://bugzilla.gnome.org/show_bug.cgi?id=757374
although in practice it's still relatively constrained.

Logic seems OK to me, but can you indent the conditionals so the nesting is clearer to a reader, like we do elsewhere in the code?
Comment 3 Fan, Chun-wei 2016-05-04 16:03:30 UTC
Created attachment 327306 [details] [review]
gmacros.h: Don't define inline as __inline on C++, nor on Visual Studio 2015+ (indented)

Hi Colin,

(In reply to Colin Walters from comment #2)
> Review of attachment 327299 [details] [review] [review]:
> 
> It seems like this is partially walking back 
> db2367e8782d7a39fc3e93d13f6a16f10cad04c2 /

Yup, this is life... :|  So I am trying to be a bit conservative here :)

> Logic seems OK to me, but can you indent the conditionals so the nesting is
> clearer to a reader, like we do elsewhere in the code?

Got it, so here is the new patch.

With blessings, thank you!
Comment 4 Colin Walters 2016-05-04 16:16:58 UTC
Review of attachment 327306 [details] [review]:

LGTM.  If we determine more compilers are affected, we might consider going back to the autoconf, but this seems OK to me for now.  Hopefully it's just MSVC...
Comment 5 Fan, Chun-wei 2016-05-04 16:47:21 UTC
Hi Colin,

Thanks!  I have pushed the patch as:
glib-2-48: 6a89272
master: 2ca496a

With blessings, thank you!