From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 63609 invoked by alias); 5 Nov 2016 02:47:43 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 63587 invoked by uid 89); 5 Nov 2016 02:47:42 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.2 required=5.0 tests=AWL,BAYES_00,SPF_HELO_PASS,SPF_SOFTFAIL autolearn=no version=3.3.2 spammy=H*u:Webmail, H*UA:Webmail, H*UA:Roundcube, H*u:Roundcube X-HELO: simark.ca Received: from simark.ca (HELO simark.ca) (158.69.221.121) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Sat, 05 Nov 2016 02:47:41 +0000 Received: by simark.ca (Postfix, from userid 33) id B410A1E18F; Fri, 4 Nov 2016 22:47:39 -0400 (EDT) To: Pedro Alves Subject: Re: [PATCH 2/3] enum_flags: Fix problems and add comprehensive unit tests X-PHP-Originating-Script: 33:rcube.php MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII; format=flowed Content-Transfer-Encoding: 7bit Date: Sat, 05 Nov 2016 02:47:00 -0000 From: Simon Marchi Cc: gdb-patches@sourceware.org In-Reply-To: <1478229738-24469-3-git-send-email-palves@redhat.com> References: <1478229738-24469-1-git-send-email-palves@redhat.com> <1478229738-24469-3-git-send-email-palves@redhat.com> Message-ID: <213333898f243221ef2d584fba50daf7@polymtl.ca> X-Sender: simon.marchi@polymtl.ca User-Agent: Roundcube Webmail/1.2.2 X-IsSubscribed: yes X-SW-Source: 2016-11/txt/msg00105.txt.bz2 On 2016-11-03 23:22, Pedro Alves wrote: > This patch starts by adding comprehensive unit tests for enum_flags. > > It adds: > > - tests of normal expected uses of the API. > > - checks that _invalid_ uses of the API would fail to compile. I.e., > it validates that enum_flags really is a strong type, and that > incorrect mixing of enum types would be caught at compile time. It > pulls that off making use of SFINEA and C++11's decltype/constexpr. > > This revealed many holes in the enum_flags API. For example, the f1 > assignment below currently incorrectly fails to compile: > > enum_flags f1 = FLAG1; > enum_flags f2 = FLAG2 | f1; > > This hole and more are all plugged by this patch, by reworking how the > enum_flags operators are implemented, and making use of C++11's > feature of being able to delete methods/functions. > > This makes most of the enum_flags operators constexpr. Beyond > enabling more compiler optimizations and enabling the new unit tests, > this has other advantages, like making it possible to use operator| > with enum_flags values in switch cases, where only compile-time > constants are allowed: > > enum_flags f = FLAG1 | FLAG2; > switch (f) > { > case FLAG1 | FLAG2: > break; > } > > Currently that fails to compile. > > This adds a STATIC_SELF_TEST macro to selftest.h, which is a variant > of SELF_TEST, but uses C++11's static_assert to do checking at compile > time. > > To avoid potential ODR violations, since the tests add enums with > short names that could easily conflict with other names, the new > selftests are placed in a namespace (selftests::enum_flags_tests). I > think that's a good practice that we should start following. This > required splitting the global operator overload enablement out of the > DEF_ENUM_FLAGS_TYPE macro into a separate macro, because > DEF_ENUM_FLAGS_TYPE wants to create the enum flags typedef in the > global namespace too. > > Tested with gcc 4.8, 4.9, 5.3, 7 (trunk) and clang 3.7. Yay, more unit tests! I don't understand much about the internal implementation, but as long as it makes the class easy to use correctly and hard to use incorrectly, I think it's a win. As a side-note, there's still the C implementation at the bottom of this file, it can probably go away. Thanks, Simon