Those changes look fine to me. None of those typedefs make sense with JNIEXPORT since they're only used locally, and most are scoped to a single function…
I agree with the __has_attribute comment. The next step would be to try setting -fvisibility=hidden and -fvisibility-inlines-hidden :) I encountered a few issues trying to get that working. -DrD- > More changes. I discovered that with JNIEXPORT defined to use the > attribute, typedefs that included JNIEXPORT would generate a gcc > -Wattribute warning > warning: visibility attribute ignored. > These new warnings are annoying, but arguably, the use of JNIEXPORT in a > typedef was a bug all along - JNIEXPORT should be all about actual symbols, > not types. > (But I couldn't find any actual spec for JNIEXPORT; could somebody please > fix that?). > So I removed all occurences of JNIEXPORT inside a typedef that caused a > -Wattribute warning. > > http://cr.openjdk.java.net/~martin/webrevs/openjdk8/JNIEXPORT/ > > Martin > > On Tue, Mar 5, 2013 at 5:45 PM, Martin Buchholz <marti...@google.com> wrote: > >> Another rev of this change. >> I added the condition check for gcc > 4.2 >> (I don't much care either way) >> >> I just ran "clang" for the first time, and was surprised that clang has >> values of __GNUC__ == 4 and __GNUC_MINOR__ == 2 >> >> I think we should keep the __has_attribute check, because it seems clearly >> correct and recommended by >> http://clang.llvm.org/docs/LanguageExtensions.html#feature-checking-macros >> and with luck, __has_attribute will become a minor industry standard from >> the innovators in llvm-land. >> I really like the design of their extension mechanisms. >> >> >> >> On Wed, Feb 27, 2013 at 3:07 PM, Martin Buchholz <marti...@google.com>wrote: >> >>> Here's the latest version of the proposed patch: >>> >>> http://cr.openjdk.java.net/~martin/webrevs/openjdk8/JNIEXPORT/ >>> >>> that has been tested, but only with gcc 4.6, >>> but is also written to work with llvm. >>> >> >>
