From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 2019E421163 for ; Sun, 18 Aug 2013 11:39:05 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: 0 X-Spam-Level: X-Spam-Status: No, score=0 tagged_above=-999 required=5 tests=[none] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 728nWLq3xUFk for ; Sun, 18 Aug 2013 11:39:00 -0700 (PDT) Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34]) by olra.theworths.org (Postfix) with ESMTP id 49D47431FDB for ; Sun, 18 Aug 2013 11:39:00 -0700 (PDT) Received: from guru.guru-group.fi (localhost [IPv6:::1]) by guru.guru-group.fi (Postfix) with ESMTP id E5515100086; Sun, 18 Aug 2013 21:38:55 +0300 (EEST) From: Tomi Ollila To: Jani Nikula , notmuch@notmuchmail.org Subject: Re: [PATCH] lib: fix clang build In-Reply-To: <1376775001-16528-1-git-send-email-jani@nikula.org> References: <1376775001-16528-1-git-send-email-jani@nikula.org> User-Agent: Notmuch/0.16+3~g340c058 (http://notmuchmail.org) Emacs/24.3.1 (x86_64-unknown-linux-gnu) X-Face: HhBM'cA~ MIME-Version: 1.0 Content-Type: text/plain Cc: Simonas Kazlauskas X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 18 Aug 2013 18:39:05 -0000 On Sun, Aug 18 2013, Jani Nikula 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 > 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 > 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 > 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 > 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 ) 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