From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 87224 invoked by alias); 1 Sep 2017 11:55:11 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 86733 invoked by uid 89); 1 Sep 2017 11:55:11 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-2.0 required=5.0 tests=AWL,BAYES_00,RCVD_IN_DNSWL_NONE,SPF_PASS,URIBL_RED autolearn=ham version=3.3.2 spammy=HCC:D*de, HCC:D*gmail.com X-HELO: relay1.mentorg.com Received: from relay1.mentorg.com (HELO relay1.mentorg.com) (192.94.38.131) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 01 Sep 2017 11:55:10 +0000 Received: from nat-ies.mentorg.com ([192.94.31.2] helo=svr-ies-mbx-01.mgc.mentorg.com) by relay1.mentorg.com with esmtp id 1dnkXR-0005rs-Oi from joseph_myers@mentor.com ; Fri, 01 Sep 2017 04:55:05 -0700 Received: from digraph.polyomino.org.uk (137.202.0.87) by svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) with Microsoft SMTP Server (TLS) id 15.0.1263.5; Fri, 1 Sep 2017 12:55:02 +0100 Received: from jsm28 (helo=localhost) by digraph.polyomino.org.uk with local-esmtp (Exim 4.86_2) (envelope-from ) id 1dnkXL-0003cD-Bv; Fri, 01 Sep 2017 11:54:59 +0000 Date: Fri, 01 Sep 2017 11:55:00 -0000 From: Joseph Myers To: Prathamesh Kulkarni CC: Tobias Burnus , Martin Sebor , gcc Patches , Marek Polacek Subject: Re: [1/2] PR 78736: New warning -Wenum-conversion In-Reply-To: Message-ID: References: <2b846ba2-2ac0-e387-9928-891bcc8d79af@gmail.com> <4bf960f7-440a-1305-e1ac-c744c53a50f9@gmail.com> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset="US-ASCII" X-ClientProxiedBy: svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) To svr-ies-mbx-01.mgc.mentorg.com (139.181.222.1) X-SW-Source: 2017-09/txt/msg00034.txt.bz2 On Fri, 1 Sep 2017, Prathamesh Kulkarni wrote: > > If it's an implicit conversion between different enum types, the warning > > is correct. The more important question for the review is: is it correct > > to replace the implicit conversion by an explicit one? That is, for each > > value in the source type, what enumerator of the destination type has the > > same value, and do the semantics match in whatever way is required for the > > code in question? > Well, for instance unit_sign in libgfortran/io.h is defined as: > typedef enum > { SIGN_PROCDEFINED, SIGN_SUPPRESS, SIGN_PLUS, SIGN_UNSPECIFIED } > unit_sign; > > and unit_sign_s is defined: > typedef enum > { SIGN_S, SIGN_SS, SIGN_SP } > unit_sign_s; > > Since the enum types are different, I assume warnings for implicit > conversion from unit_sign_s to unit_sign would be correct ? > And since unit_sign_s is a subset of unit_sign in terms of > value-ranges, I assume replacing implicit by explicit conversion would > be OK ? Whether an explicit conversion is OK depends on *the semantics of the individual values and the intended semantics of the code doing the conversion*. That is, for the semantics of whatever code converts unit_sign_s to unit_sign, is it indeed correct that an input of SIGN_S should result in an output of SIGN_PROCDEFINED, an input of SIGN_SS should result in an output of SIGN_SUPPRESS and an input of SIGN_SP should result in an output of SIGN_PLUS? That is not a question one should expect C front-end maintainers to have any expertise in. Thus, I strongly advise sending each patch that fixes the warning fallout for such conversions separately, CC:ing the maintainers of the relevant part of GCC, and including an explanation with each such patch of what the semantics are in the relevant code and why an explicit conversion is correct. It would also seem a good idea to me to make sure that each enum in question has a comment on its definition, saying that the values have to be kept consistent (in whatever way) with the values of another enum, because of the conversions in whatever function named in the constant, so that people editing either enum know they can't just insert values in the middle of it. -- Joseph S. Myers joseph@codesourcery.com