Opened 7 years ago

Closed 6 years ago

#8882 closed bug (fixed)

add gcc version check to gl.h to avoid warnings

Reported by: jstressman Owned by: kallisti5
Priority: normal Milestone: R1/beta1
Component: Kits/OpenGL Kit Version: R1/Development
Keywords: Cc:
Blocked By: Blocking:
Has a Patch: yes Platform: All


Currently (hrev44524) when building things like SDL, we get thousands of warning messages about "warning: `visibility' attribute directive ignored"

A version check to not use this attribute unless we have a higher GCC version avoids this.

The attached patch fixes the problem for me, but I'd like for someone more knowledgeable to have a look. :)

Attachments (1)

gl.h.diff (668 bytes) - added by jstressman 7 years ago.

Download all attachments as: .zip

Change History (9)

comment:1 Changed 7 years ago by jstressman

Has a Patch: set

comment:2 Changed 7 years ago by pulkomandy

The patch looks fine (did you check the actual verson when the attribute was introduced in gcc ?)

Since this is an OpenGL header, it now lives in an optional package however. We may either want to patch it in our port or upstream the change to Mesa (if it's not already done there).

comment:3 Changed 7 years ago by jstressman

I've also added a check for ELF since I believe it only makes sense in that context.

I can't seem to find a clear note on exactly when the visibility attribute was added, but I see a number of discussions stating it as >= GCC 3.3 (I accidentally made it >= 3.2 instead of just > 3.2)

There is a discussion about it on the zlib mailing list for instance:

Updated the patch with these 2 changes.

Changed 7 years ago by jstressman

Attachment: gl.h.diff added

comment:4 Changed 7 years ago by jstressman

If it matters, Mesa 7.11.2 already has a check added.

#elif (defined(__GNUC__) && __GNUC__ >= 4) || (defined(__SUNPRO_C) && (__SUNPRO_C >= 0x590))

and our line is currently as follows:

#elif defined(__GNUC__)	|| (defined(__SUNPRO_C) && (__SUNPRO_C >= 0x590))

So you could say it's already been upstreamed. I guess I just took a more complicated/specific route?

comment:5 Changed 6 years ago by korli

Owner: changed from korli to kallisti5
Status: newassigned

comment:6 Changed 6 years ago by kallisti5

I commited an adjusted mesa 7.8.2 diff and a new bep to the ports site (hrev2276)... i still need to test it though before considering this resolved.

comment:7 Changed 6 years ago by kallisti5

should be solved in hrev45296.. but i need to start removing the "--f-no-warnings" stuff to have final verification.

comment:8 Changed 6 years ago by kallisti5

Milestone: R1R1/beta1
Resolution: fixed
Status: assignedclosed

resolved in hrev45297

Note: See TracTickets for help on using tickets.