[PATCH] lib: fix clang build

[PATCH] lib: fix clang build

[Jani Nikula]

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;
 
-typedef struct _notmuch_string_list notmuch_string_list_t;
-
 /* 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

Re: [PATCH] lib: fix clang build

[Tomi Ollila]

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 )

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

Re: [PATCH] lib: fix clang build

[Jani Nikula]

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

Re: [PATCH] lib: fix clang build

[Tomi Ollila]

On Sun, Aug 18 2013, Jani Nikula <jani@nikula.org> wrote:

> On Sun, 18 Aug 2013, Tomi Ollila <tomi.ollila@iki.fi> wrote:
>>>  
>>> -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

OK! LGTM.

>
> Jani.
>
>> Tomi

Tomi

Re: [PATCH] lib: fix clang build

[David Bremner]

Jani Nikula <jani@nikula.org> writes:

> Long story short, fix build on recent (3.2+) clang.
>

pushed.

d