On Sun, 18 Aug 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote: > On Sun, Aug 18 2013, Jani Nikula <jani@nikula.org> wrote: > >> Long story short, fix build on recent (3.2+) clang. >> >> The long story for posterity follows. >> >> gcc 4.6 added new warnings about structs with greater visibility than >> their fields. The warnings were silenced by adjusting visibility in >> >> commit d5523ead90b6be2b07d4af745b8ed9b980a6b9f1 >> Author: Carl Worth <cworth@cworth.org> >> Date: Wed May 11 13:23:13 2011 -0700 >> >> Mark some structures in the library interface with visibility=default attribute. >> >> Later on, >> >> commit 3b76adf9e2c026dd03b820f4c6eab50e25444113 >> Author: Austin Clements <amdragon@MIT.EDU> >> Date: Sat Jan 14 19:17:33 2012 -0500 >> >> lib: Add support for automatically excluding tags from queries >> >> changed visibility of struct _notmuch_string_list for the same reason, and >> >> commit 1a53f9f116fa7c460cda3df532be921baaafb082 >> Author: Mark Walters <markwalters1009@gmail.com> >> Date: Thu Mar 1 22:30:38 2012 +0000 >> >> lib: Add the exclude flag to notmuch_query_search_threads >> >> split the struct _notmuch_string_list and its typedef >> notmuch_string_list_t as a way to make a forward declaration for >> _notmuch_thread_create(). >> >> The subtle difference was that the struct definition now had 'visible' >> in it, while the typedef didn't, and it was within the #pragma GCC >> visibility push(hidden) block. This went unnoticed, as the then common >> versions of clang didn't care about this. >> >> A later change in clang (I did not dig into when this change was >> introduced) caused the following error: >> >> CXX -O2 lib/database.o >> In file included from lib/database.cc:21: >> In file included from ./lib/database-private.h:33: >> ./lib/notmuch-private.h:479:8: error: visibility does not match previous declaration >> struct visible _notmuch_string_list { >> ^ >> ./lib/notmuch-private.h:67:33: note: expanded from macro 'visible' >> ^ >> ./lib/notmuch-private.h:52:13: note: previous attribute is here >> ^ >> 1 error generated. >> make: *** [lib/database.o] Error 1 >> >> This is slightly misleading due to the reference to the #pragma. The >> real culprit is the typedef within the #pragma. >> >> We could just add 'visible' to the typedef, or move the typedef >> outside of the #pragma, and be done with it, but juggle the >> declarations a bit to accommodate moving the typedef back with the >> struct, and keep the visibility attribute in one place. >> >> The problem was originally reported by Simonas Kazlauskas >> <s@kazlauskas.me> in id:20130418102507.GA23688@godbox but I was only >> able to reproduce and investigate now that I upgraded clang. >> --- >> lib/notmuch-private.h | 28 +++++++++++++--------------- >> 1 file changed, 13 insertions(+), 15 deletions(-) >> >> diff --git a/lib/notmuch-private.h b/lib/notmuch-private.h >> index eced03e..af185c7 100644 >> --- a/lib/notmuch-private.h >> +++ b/lib/notmuch-private.h >> @@ -162,8 +162,6 @@ typedef enum _notmuch_find_flags { >> >> typedef struct _notmuch_doc_id_set notmuch_doc_id_set_t; > > Looks good to me (except log message is a bit excessive (?) maybe > it is reasonable to have it, though (TL;DR ;)... >> >> -typedef struct _notmuch_string_list notmuch_string_list_t; >> - > > anyway, would > > typedef struct visible _notmuch_string_list notmuch_string_list_t; > > have helped here (if yes is your resolution nicer ?? :D ) Yes, it would have. Read the commit message! :p Jani. > > Tomi > > >> /* database.cc */ >> >> /* Lookup a prefix value by name. >> @@ -228,17 +226,6 @@ _notmuch_directory_create (notmuch_database_t *notmuch, >> unsigned int >> _notmuch_directory_get_document_id (notmuch_directory_t *directory); >> >> -/* thread.cc */ >> - >> -notmuch_thread_t * >> -_notmuch_thread_create (void *ctx, >> - notmuch_database_t *notmuch, >> - unsigned int seed_doc_id, >> - notmuch_doc_id_set_t *match_set, >> - notmuch_string_list_t *excluded_terms, >> - notmuch_exclude_t omit_exclude, >> - notmuch_sort_t sort); >> - >> /* message.cc */ >> >> notmuch_message_t * >> @@ -476,11 +463,11 @@ typedef struct _notmuch_string_node { >> struct _notmuch_string_node *next; >> } notmuch_string_node_t; >> >> -struct visible _notmuch_string_list { >> +typedef struct visible _notmuch_string_list { >> int length; >> notmuch_string_node_t *head; >> notmuch_string_node_t **tail; >> -}; >> +} notmuch_string_list_t; >> >> notmuch_string_list_t * >> _notmuch_string_list_create (const void *ctx); >> @@ -509,6 +496,17 @@ notmuch_filenames_t * >> _notmuch_filenames_create (const void *ctx, >> notmuch_string_list_t *list); >> >> +/* thread.cc */ >> + >> +notmuch_thread_t * >> +_notmuch_thread_create (void *ctx, >> + notmuch_database_t *notmuch, >> + unsigned int seed_doc_id, >> + notmuch_doc_id_set_t *match_set, >> + notmuch_string_list_t *excluded_terms, >> + notmuch_exclude_t omit_exclude, >> + notmuch_sort_t sort); >> + >> NOTMUCH_END_DECLS >> >> #ifdef __cplusplus >> -- >> 1.7.10.4 >> >> _______________________________________________ >> notmuch mailing list >> notmuch@notmuchmail.org >> http://notmuchmail.org/mailman/listinfo/notmuch