From mboxrd@z Thu Jan  1 00:00:00 1970
Return-Path: <v.shpilevoy@tarantool.org>
Received: from smtp47.i.mail.ru (smtp47.i.mail.ru [94.100.177.107])
 (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits))
 (No client certificate requested)
 by dev.tarantool.org (Postfix) with ESMTPS id B4BAF469710
 for <tarantool-patches@dev.tarantool.org>;
 Sat, 30 May 2020 01:34:48 +0300 (MSK)
References: <cover.1590622225.git.v.shpilevoy@tarantool.org>
 <de14e939dcfeadde645197e34ffb185c5f36177b.1590622225.git.v.shpilevoy@tarantool.org>
 <20200529062445.5vs7tgq4yceu4hie@tarantool.org>
From: Vladislav Shpilevoy <v.shpilevoy@tarantool.org>
Message-ID: <8a5bb207-536e-c243-794b-a1be8d5197bc@tarantool.org>
Date: Sat, 30 May 2020 00:34:45 +0200
MIME-Version: 1.0
In-Reply-To: <20200529062445.5vs7tgq4yceu4hie@tarantool.org>
Content-Type: text/plain; charset="utf-8"
Content-Language: en-US
Content-Transfer-Encoding: 8bit
Subject: Re: [Tarantool-patches] [PATCH v2 02/10] cmake: ignore warnings on
 alignof() and offsetof()
List-Id: Tarantool development patches <tarantool-patches.dev.tarantool.org>
List-Unsubscribe: <https://lists.tarantool.org/mailman/options/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=unsubscribe>
List-Archive: <https://lists.tarantool.org/pipermail/tarantool-patches/>
List-Post: <mailto:tarantool-patches@dev.tarantool.org>
List-Help: <mailto:tarantool-patches-request@dev.tarantool.org?subject=help>
List-Subscribe: <https://lists.tarantool.org/mailman/listinfo/tarantool-patches>, 
 <mailto:tarantool-patches-request@dev.tarantool.org?subject=subscribe>
To: Kirill Yukhin <kyukhin@tarantool.org>
Cc: tarantool-patches@dev.tarantool.org

Hi! Thanks for the review!

On 29/05/2020 08:24, Kirill Yukhin wrote:
> Hello,
> 
> On 28 май 01:32, Vladislav Shpilevoy wrote:
>> --- a/cmake/compiler.cmake
>> +++ b/cmake/compiler.cmake
>> @@ -276,11 +276,12 @@ macro(enable_tnt_compile_flags)
>>          add_compile_flags("C;CXX" "-Wno-format-truncation")
>>      endif()
>>  
>> -    if (CMAKE_COMPILER_IS_GNUCXX)
>> +    if (CMAKE_COMPILER_IS_CLANG OR CMAKE_COMPILER_IS_GNUCXX)
>>          # G++ bug. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31488
>>          add_compile_flags("CXX"
>>              "-Wno-invalid-offsetof"
>>          )
> 
> So, you're disabling the warning for clang? According to comment it
> was disabled for GCC because of bug in it. Update comment? Enable for
> clang?

I disable it for clang, because invalid-offsetof is not only
about the thing mentioned in the GCC bug (va_list, which is a
part of struct fiber, which uses offsetof via rlist). It is a
warning about offsetof used on a non-POD type. We use plenty
of non-POD offsetof, every time when use rlist and some other
data structures. Both clang++ and g++ complain about it.

Here is what I get for clang++:

	tarantool/src/box/alter.cc:1148:2: error: offset of on non-standard-layout type 'typeof (*(op))' (aka 'AlterSpaceOp') [-Werror,-Winvalid-offsetof]
	        rlist_foreach_entry(op, &alter->ops, link)

I added a more descriptive comment:

====================
diff --git a/cmake/compiler.cmake b/cmake/compiler.cmake
index ce3e7e506..9ad968d90 100644
--- a/cmake/compiler.cmake
+++ b/cmake/compiler.cmake
@@ -278,6 +278,11 @@ macro(enable_tnt_compile_flags)
 
     if (CMAKE_COMPILER_IS_CLANG OR CMAKE_COMPILER_IS_GNUCXX)
         # G++ bug. http://gcc.gnu.org/bugzilla/show_bug.cgi?id=31488
+        # Also offsetof() is widely used in Tarantool source code
+        # for classes and structs to implement intrusive lists and
+        # some other data structures. G++ and clang++ both
+        # complain about classes, having a virtual table. They
+        # complain fair, but this can't be fixed for now.
         add_compile_flags("CXX"
             "-Wno-invalid-offsetof"
         )