From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 48950 invoked by alias); 3 May 2017 06:01:47 -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 47760 invoked by uid 89); 3 May 2017 06:00:47 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-24.1 required=5.0 tests=AWL,BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,RCVD_IN_DNSWL_NONE,RCVD_IN_SORBS_SPAM,SPF_PASS autolearn=ham version=3.3.2 spammy= X-HELO: mail-it0-f47.google.com Received: from mail-it0-f47.google.com (HELO mail-it0-f47.google.com) (209.85.214.47) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Wed, 03 May 2017 06:00:44 +0000 Received: by mail-it0-f47.google.com with SMTP id 131so12574357itz.1 for ; Tue, 02 May 2017 23:00:46 -0700 (PDT) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:mime-version:in-reply-to:references:from:date :message-id:subject:to:cc; bh=Z3iP96VnCdeJrJlywOeClUFA0W2cAqMMyHfCWP0zwJk=; b=iGHY4WyT3FoIdFWq44hGsmAtUFDTVQnvNGRW2f5NpC2qWmtwA5a93Y11m3D/TN9tla shzAQPXWBA/+Gq5VpZJmdZpsVxtZUtP0fF9yxAUpP6eFsok9+TXZpixPiAdSNs97+5P6 RL14tjrz1ZMdbTk85OQYcCyuwb1eDhaKxRYQqv8OAYkKDT1xbnBz4A3FHsBrt5DFUCBc F1jKZYmxUT+AaKYXA86wvC5wH6HGfAZ8GAUpKDx8Quq4VmxACfrpdoU+CZrNhnFFpot8 oqdnGi6chTWCc8Y0vvoi9Dtt7keZD6QJ4YLPafyk+xjo2EQZyYf+0e6dhcI/VYOaLd1W dTpA== X-Gm-Message-State: AN3rC/69XdHTrFHaHL3HPbrnhWuX5kXev6dL0PZkH29tByE7r2eJwCpM GH/aY/3vls9loNGRHdywJ7zcGHVAeQ1K X-Received: by 10.36.137.212 with SMTP id s203mr6255728itd.57.1493791245037; Tue, 02 May 2017 23:00:45 -0700 (PDT) MIME-Version: 1.0 Received: by 10.107.25.67 with HTTP; Tue, 2 May 2017 23:00:44 -0700 (PDT) In-Reply-To: <2b846ba2-2ac0-e387-9928-891bcc8d79af@gmail.com> References: <2b846ba2-2ac0-e387-9928-891bcc8d79af@gmail.com> From: Prathamesh Kulkarni Date: Wed, 03 May 2017 06:10:00 -0000 Message-ID: Subject: Re: [1/2] PR 78736: New warning -Wenum-conversion To: Martin Sebor Cc: gcc Patches , Marek Polacek , "Joseph S. Myers" Content-Type: multipart/mixed; boundary=94eb2c05f6a088317e054e9860a2 X-IsSubscribed: yes X-SW-Source: 2017-05/txt/msg00162.txt.bz2 --94eb2c05f6a088317e054e9860a2 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Content-length: 3195 On 3 May 2017 at 03:28, Martin Sebor wrote: > On 05/02/2017 11:11 AM, Prathamesh Kulkarni wrote: >> >> Hi, >> The attached patch attempts to add option -Wenum-conversion for C and >> objective-C similar to clang, which warns when an enum value of a type >> is implicitly converted to enum value of another type and is enabled >> by Wall. > > > It seems quite useful. My only high-level concern is with > the growing number of specialized warnings and options for each > and their interaction. > > I've been working on -Wenum-assign patch that complains about > assigning to an enum variables an integer constants that doesn't > match any of the enumerators of the type. Testing revealed that > the -Wenum-assign duplicated a subset of warnings already issued > by -Wconversion enabled with -Wpedantic. I'm debating whether > to suppress that part of -Wenum-assign altogether or only when > -Wconversion and -Wpedantic are enabled. > > My point is that these dependencies tend to be hard to discover > and understand, and the interactions tricky to get right (e.g., > avoid duplicate warnings for similar but distinct problems). > > This is not meant to be a negative comment on your patch, but > rather a comment about a general problem that might be worth > starting to think about. > > One comment on the patch itself: > > + warning_at_rich_loc (&loc, 0, "implicit conversion from" > + " enum type of %qT to %qT", checktype, typ= e); > > Unlike C++, the C front end formats an enumerated type E using > %qT as 'enum E' so the warning prints 'enum type of 'enum E'), > duplicating the "enum" part. > > I would suggest to simplify that to: > > warning_at_rich_loc (&loc, 0, "implicit conversion from " > "%qT to %qT", checktype, ... > Thanks for the suggestions. I have updated the patch accordingly. Hmm the issue you pointed out of warnings interaction is indeed of concern. I was wondering then if we should merge this warning with -Wconversion instead of having a separate option -Wenum-conversion ? Although that will = not really help with your example below. > Martin > > PS As an example to illustrate my concern above, consider this: > > enum __attribute__ ((packed)) E { e1 =3D 1 }; > enum F { f256 =3D 256 }; > > enum E e =3D f256; > > It triggers -Woverflow: > > warning: large integer implicitly truncated to unsigned type [-Woverflow] > enum E e =3D f256; > ^~~~ > > also my -Wenum-assign: > > warning: integer constant =E2=80=98256=E2=80=99 converted to =E2=80=980= =E2=80=99 due to limited range [0, > 255] of type =E2=80=98=E2=80=98enum E=E2=80=99=E2=80=99 [-Wassign-enum] > enum E e =3D f256; > ^~~~ > > and (IIUC) will trigger your new -Wenum-conversion. Yep, on my branch it triggered -Woverflow and -Wenum-conversion. Running the example on clang shows a single warning, which they call as -Wconstant-conversion, which I suppose is similar to your -Wassign-enum. test-eg.c:3:12: warning: implicit conversion from 'int' to 'enum E' changes value from 256 to 0 [-Wconstant-conversion] enum E e =3D f256; ~ ^~~~ Thanks, Prathamesh > > Martin --94eb2c05f6a088317e054e9860a2 Content-Type: text/plain; charset=US-ASCII; name="pr78736-4-gcc.txt" Content-Disposition: attachment; filename="pr78736-4-gcc.txt" Content-Transfer-Encoding: base64 X-Attachment-Id: f_j28k7uft0 Content-length: 5251 MjAxNy0wNS0wMiAgUHJhdGhhbWVzaCBLdWxrYXJuaSAgPHByYXRoYW1lc2gu a3Vsa2FybmlAbGluYXJvLm9yZz4KCgkqIGRvYy9pbnZva2UudGV4dDogRG9j dW1lbnQgV2VudW0tY29udmVyc2lvbi4KCSogYy1mYW1pbHkvYy5vcHQgKFdl bnVtLWNvbnZlcnNpb24pOiBOZXcgb3B0aW9uLgoJKiBjL2MtdHlwZWNrLmMg KGNvbnZlcnRfZm9yX2Fzc2lnbm1lbnQpOiBIYW5kbGUgV2VudW0tY29udmVy c2lvbi4KCnRlc3RzdWl0ZS8KCSogZ2NjLmRnL1dlbnVtLWNvbnZlcnNpb24u YzogTmV3IHRlc3QtY2FzZS4KCmRpZmYgLS1naXQgYS9nY2MvYy1mYW1pbHkv Yy5vcHQgYi9nY2MvYy1mYW1pbHkvYy5vcHQKaW5kZXggOWFkMmY2ZTFmY2Mu LmUwNDMxMmVjMjUzIDEwMDY0NAotLS0gYS9nY2MvYy1mYW1pbHkvYy5vcHQK KysrIGIvZ2NjL2MtZmFtaWx5L2Mub3B0CkBAIC00OTIsNiArNDkyLDEwIEBA IFdlbnVtLWNvbXBhcmUKIEMgT2JqQyBDKysgT2JqQysrIFZhcih3YXJuX2Vu dW1fY29tcGFyZSkgSW5pdCgtMSkgV2FybmluZyBMYW5nRW5hYmxlZEJ5KEMg T2JqQyxXYWxsIHx8IFdjKystY29tcGF0KQogV2FybiBhYm91dCBjb21wYXJp c29uIG9mIGRpZmZlcmVudCBlbnVtIHR5cGVzLgogCitXZW51bS1jb252ZXJz aW9uCitDIE9iakMgVmFyKHdhcm5fZW51bV9jb252ZXJzaW9uKSBJbml0KDAp IFdhcm5pbmcgTGFuZ0VuYWJsZWRCeShDIE9iamMsV2FsbCkKK1dhcm4gYWJv dXQgaW1wbGljaXQgY29udmVyc2lvbiBvZiBlbnVtIHR5cGVzLgorCiBXZXJy b3IKIEMgT2JqQyBDKysgT2JqQysrCiA7IERvY3VtZW50ZWQgaW4gY29tbW9u Lm9wdApkaWZmIC0tZ2l0IGEvZ2NjL2MvYy10eXBlY2suYyBiL2djYy9jL2Mt dHlwZWNrLmMKaW5kZXggNmY5OTA5YzYzOTYuLjQ4M2IyMDA4ZjdiIDEwMDY0 NAotLS0gYS9nY2MvYy9jLXR5cGVjay5jCisrKyBiL2djYy9jL2MtdHlwZWNr LmMKQEAgLTYzMDksNiArNjMwOSwyMCBAQCBjb252ZXJ0X2Zvcl9hc3NpZ25t ZW50IChsb2NhdGlvbl90IGxvY2F0aW9uLCBsb2NhdGlvbl90IGV4cHJfbG9j LCB0cmVlIHR5cGUsCiAJfQogICAgIH0KIAorICBpZiAod2Fybl9lbnVtX2Nv bnZlcnNpb24pCisgICAgeworICAgICAgdHJlZSBjaGVja3R5cGUgPSBvcmln dHlwZSAhPSBOVUxMX1RSRUUgPyBvcmlndHlwZSA6IHJoc3R5cGU7CisgICAg ICBpZiAoY2hlY2t0eXBlICE9IGVycm9yX21hcmtfbm9kZQorCSAgJiYgVFJF RV9DT0RFIChjaGVja3R5cGUpID09IEVOVU1FUkFMX1RZUEUKKwkgICYmIFRS RUVfQ09ERSAodHlwZSkgPT0gRU5VTUVSQUxfVFlQRQorCSAgJiYgVFlQRV9N QUlOX1ZBUklBTlQgKGNoZWNrdHlwZSkgIT0gVFlQRV9NQUlOX1ZBUklBTlQg KHR5cGUpKQorCXsKKwkgIGdjY19yaWNoX2xvY2F0aW9uIGxvYyAobG9jYXRp b24pOworCSAgd2FybmluZ19hdF9yaWNoX2xvYyAoJmxvYywgMCwgImltcGxp Y2l0IGNvbnZlcnNpb24gZnJvbSIKKwkJCSAgICAgICAiICVxVCB0byAlcVQi LCBjaGVja3R5cGUsIHR5cGUpOworCX0KKyAgICB9CisKICAgaWYgKFRZUEVf TUFJTl9WQVJJQU5UICh0eXBlKSA9PSBUWVBFX01BSU5fVkFSSUFOVCAocmhz dHlwZSkpCiAgICAgcmV0dXJuIHJoczsKIApkaWZmIC0tZ2l0IGEvZ2NjL2Rv Yy9pbnZva2UudGV4aSBiL2djYy9kb2MvaW52b2tlLnRleGkKaW5kZXggMGVl ZWE3YjNiODcuLjc5YjFlMTc1Mzc0IDEwMDY0NAotLS0gYS9nY2MvZG9jL2lu dm9rZS50ZXhpCisrKyBiL2djYy9kb2MvaW52b2tlLnRleGkKQEAgLTI3Myw3 ICsyNzMsNyBAQCBPYmplY3RpdmUtQyBhbmQgT2JqZWN0aXZlLUMrKyBEaWFs ZWN0c30uCiAtV2Rpc2FibGVkLW9wdGltaXphdGlvbiBAZ29sCiAtV25vLWRp c2NhcmRlZC1xdWFsaWZpZXJzICAtV25vLWRpc2NhcmRlZC1hcnJheS1xdWFs aWZpZXJzIEBnb2wKIC1Xbm8tZGl2LWJ5LXplcm8gIC1XZG91YmxlLXByb21v dGlvbiAgLVdkdXBsaWNhdGVkLWNvbmQgQGdvbAotLVdlbXB0eS1ib2R5ICAt V2VudW0tY29tcGFyZSAgLVduby1lbmRpZi1sYWJlbHMgIC1XZXhwYW5zaW9u LXRvLWRlZmluZWQgQGdvbAorLVdlbXB0eS1ib2R5ICAtV2VudW0tY29tcGFy ZSAtV2VudW0tY29udmVyc2lvbiAgLVduby1lbmRpZi1sYWJlbHMgIC1XZXhw YW5zaW9uLXRvLWRlZmluZWQgQGdvbAogLVdlcnJvciAgLVdlcnJvcj0qICAt V2V4dHJhLXNlbWkgIC1XZmF0YWwtZXJyb3JzIEBnb2wKIC1XZmxvYXQtZXF1 YWwgIC1XZm9ybWF0ICAtV2Zvcm1hdD0yIEBnb2wKIC1Xbm8tZm9ybWF0LWNv bnRhaW5zLW51bCAgLVduby1mb3JtYXQtZXh0cmEtYXJncyAgQGdvbApAQCAt Mzc1NCw2ICszNzU0LDcgQEAgT3B0aW9uc30gYW5kIEByZWZ7T2JqZWN0aXZl LUMgYW5kIE9iamVjdGl2ZS1DKysgRGlhbGVjdCBPcHRpb25zfS4KIC1XY29t bWVudCAgQGdvbAogLVdkdXBsaWNhdGUtZGVjbC1zcGVjaWZpZXIgQHJ7KEMg YW5kIE9iamVjdGl2ZS1DIG9ubHkpfSBAZ29sCiAtV2VudW0tY29tcGFyZSBA cnsoaW4gQy9PYmpDOyB0aGlzIGlzIG9uIGJ5IGRlZmF1bHQgaW4gQysrKX0g QGdvbAorLVdlbnVtLWNvbnZlcnNpb24gQHJ7aW4gQy9PYmpDO30gQGdvbAog LVdmb3JtYXQgICBAZ29sCiAtV2ludC1pbi1ib29sLWNvbnRleHQgIEBnb2wK IC1XaW1wbGljaXQgQHJ7KEMgYW5kIE9iamVjdGl2ZS1DIG9ubHkpfSBAZ29s CkBAIC01OTYxLDYgKzU5NjIsMTIgQEAgSW4gQysrIGVudW1lcmF0ZWQgdHlw ZSBtaXNtYXRjaGVzIGluIGNvbmRpdGlvbmFsIGV4cHJlc3Npb25zIGFyZSBh bHNvCiBkaWFnbm9zZWQgYW5kIHRoZSB3YXJuaW5nIGlzIGVuYWJsZWQgYnkg ZGVmYXVsdC4gIEluIEMgdGhpcyB3YXJuaW5nIGlzIAogZW5hYmxlZCBieSBA b3B0aW9uey1XYWxsfS4KIAorQGl0ZW0gLVdlbnVtLWNvbnZlcnNpb24gQHJ7 KEMsIE9iamVjdGl2ZS1DIG9ubHkpfQorQG9waW5kZXggV2VudW0tY29udmVy c2lvbgorQG9waW5kZXggV25vLWVudW0tY29udmVyc2lvbgorV2FybiB3aGVu IGFuIGVudW0gdmFsdWUgb2YgYSB0eXBlIGlzIGltcGxpY2l0bHkgY29udmVy dGVkIHRvIGFuIGVudW0gb2YKK2Fub3RoZXIgdHlwZS4gVGhpcyB3YXJuaW5n IGlzIGVuYWJsZWQgYnkgQG9wdGlvbnstV2FsbH0uCisKIEBpdGVtIC1XZXh0 cmEtc2VtaSBAcnsoQysrLCBPYmplY3RpdmUtQysrIG9ubHkpfQogQG9waW5k ZXggV2V4dHJhLXNlbWkKIEBvcGluZGV4IFduby1leHRyYS1zZW1pCmRpZmYg LS1naXQgYS9nY2MvdGVzdHN1aXRlL2djYy5kZy9XZW51bS1jb252ZXJzaW9u LmMgYi9nY2MvdGVzdHN1aXRlL2djYy5kZy9XZW51bS1jb252ZXJzaW9uLmMK bmV3IGZpbGUgbW9kZSAxMDA2NDQKaW5kZXggMDAwMDAwMDAwMDAuLjg2MDMz Mzk5YjdkCi0tLSAvZGV2L251bGwKKysrIGIvZ2NjL3Rlc3RzdWl0ZS9nY2Mu ZGcvV2VudW0tY29udmVyc2lvbi5jCkBAIC0wLDAgKzEsMjAgQEAKKy8qIHsg ZGctZG8gY29tcGlsZSB9ICovCisvKiB7IGRnLW9wdGlvbnMgIi1XZW51bS1j b252ZXJzaW9uIiB9ICovCisKK2VudW0gWCB7IHgxLCB4MiB9OworZW51bSBZ IHsgeTEsIHkyIH07CisKK2VudW0gWCBvYmogPSB5MTsgIC8qIHsgZGctd2Fy bmluZyAiaW1wbGljaXQgY29udmVyc2lvbiBmcm9tIC5lbnVtIFkuIHRvIC5l bnVtIFguIiB9ICovCitlbnVtIFkgb2JqMiA9IHkxOworCitlbnVtIFggb2Jq MzsKK3ZvaWQgZm9vKCkKK3sKKyAgb2JqMyA9IHkyOyAvKiB7IGRnLXdhcm5p bmcgImltcGxpY2l0IGNvbnZlcnNpb24gZnJvbSAuZW51bSBZLiB0byAuZW51 bSBYLiIgfSAqLworfQorCit2b2lkIGJhcihlbnVtIFgpOwordm9pZCBmKHZv aWQpCit7CisgIGJhciAoeTEpOyAvKiB7IGRnLXdhcm5pbmcgImltcGxpY2l0 IGNvbnZlcnNpb24gZnJvbSAuZW51bSBZLiB0byAuZW51bSBYLiIgfSAqLwor fQo= --94eb2c05f6a088317e054e9860a2--