From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x443.google.com (mail-wr1-x443.google.com [IPv6:2a00:1450:4864:20::443]) by sourceware.org (Postfix) with ESMTPS id 243B63858D37 for ; Mon, 17 Aug 2020 10:40:37 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.3.2 sourceware.org 243B63858D37 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=andrew.burgess@embecosm.com Received: by mail-wr1-x443.google.com with SMTP id l2so14447861wrc.7 for ; Mon, 17 Aug 2020 03:40:37 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=date:from:to:cc:subject:message-id:references:mime-version :content-disposition:content-transfer-encoding:in-reply-to; bh=Iw7QPatNSGpJQRTru7t2ppq1VNf1FtdFvtIf48Qs3WA=; b=aFdrGUo05QLecOw/PjdcW7Ux45BXG4sKIrc5UqlWh7gmbdoNNtCoGugB0l4tnOHFnB 7QVkXCHwe/xF4ptwFav+maZ5I6GGO1cX/Pkz3lr934XQCUDEuG9GWyhs11uRCh+sm31Q hjiwZr4pkgphBOypavr2A2DyH41WwUPlLEPZMQeDwhu1Gfg52L/hQM1uLlHAymfFGlY7 h1Lzrxrt80lrCar6zgUEx4MineTZW+R3GiWLognsrVWNfK7btSkoUWQ40Lut27fh/UM9 h4aeUTgIIkxxPeLe1E62b/c4/kS3V0qGj74LrO7HWzxQWy5g+p2nX2e4fqReUHe7C4u8 cK1A== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:date:from:to:cc:subject:message-id:references :mime-version:content-disposition:content-transfer-encoding :in-reply-to; bh=Iw7QPatNSGpJQRTru7t2ppq1VNf1FtdFvtIf48Qs3WA=; b=e5IKBxf73p8+nn8hY2MqDKqNPTep5Ycj6JXam1zE+OVfFn3WmCENxJK226v8tLnHGz hJIp0ACEo90R0qGcczu8yxZMLD3ksadph7FHVCFmxd2ss0LBET4zYpBDhOoGhq1pZij6 S35OAZr2h8qyavwNRf2TOuO9RNI63JLymUdcHLu56Gxr4bk80FdNxc0Wo+4DGi5eJ8Qb cgNEFp18KEpxw24A4WMPn4Zr7vikVJsCR2LHmIPEP4FC7heLFi1pR0xnSLLT7yYbkU2U kpvV+6c9Te+NiumTKxpKzq/6g3bKlbrvUzKwxCZvB+vd/UBt3PyFxexu7a4RuXW5LFOs TGTA== X-Gm-Message-State: AOAM530pgtJgQU7yNvWIpkUk123uSdkIr/jkuO/32JuCbjz8Hvbi/oXw RfSaz+CIBDtLZyXPByG7SigrxzKDhcpOmA== X-Google-Smtp-Source: ABdhPJzfgLWPWLLSi75F25CkerFqoijE8Q7+ESrbGTGOsJe9FiEkWTihcd1Wvx+o/DYKzDMyv2pDFw== X-Received: by 2002:adf:c789:: with SMTP id l9mr15580889wrg.41.1597660836237; Mon, 17 Aug 2020 03:40:36 -0700 (PDT) Received: from localhost (host86-186-80-213.range86-186.btcentralplus.com. [86.186.80.213]) by smtp.gmail.com with ESMTPSA id o124sm28628647wmb.2.2020.08.17.03.40.34 (version=TLS1_3 cipher=TLS_AES_256_GCM_SHA384 bits=256/256); Mon, 17 Aug 2020 03:40:35 -0700 (PDT) Date: Mon, 17 Aug 2020 11:40:34 +0100 From: Andrew Burgess To: Tom Tromey Cc: gdb-patches@sourceware.org Subject: Re: [PATCH 1/8] gdbsupport: Provide global operators |=, &=, and ^= for enum bit flags Message-ID: <20200817104034.GJ853475@embecosm.com> References: <8eb7f409ca572c526de08ab23878be905f5fef42.1597319264.git.andrew.burgess@embecosm.com> <87a6yvhls1.fsf@tromey.com> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: quoted-printable In-Reply-To: <87a6yvhls1.fsf@tromey.com> X-Operating-System: Linux/5.6.15-200.fc31.x86_64 (x86_64) X-Uptime: 11:12:58 up 29 days, 19:27, X-Editor: GNU Emacs [ http://www.gnu.org/software/emacs ] X-Spam-Status: No, score=-4.9 required=5.0 tests=BAYES_00, DKIM_SIGNED, DKIM_VALID, DKIM_VALID_AU, DKIM_VALID_EF, RCVD_IN_DNSWL_NONE, SPF_HELO_NONE, SPF_PASS, TXREP autolearn=ham autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on server2.sourceware.org X-BeenThere: gdb-patches@sourceware.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: Gdb-patches mailing list List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Mon, 17 Aug 2020 10:40:38 -0000 Tom, I think I gave up on this patch too easily. I'd like to push back and argue in favour of this change. * Tom Tromey [2020-08-15 11:16:46 -0600]: > >>>>> "Andrew" =3D=3D Andrew Burgess writes: >=20 > Andrew> The current enum bit flags mechanism provides global operators &,= |, > Andrew> ^, ~, but does not provide &=3D, |=3D, ^=3D. >=20 > Andrew> The implementation for one of these, |=3D, would look like this: >=20 > Andrew> template > Andrew> typename enum_flags_type::type & > Andrew> operator|=3D (enum_type &e1, enum_type e2) >=20 > Why "enum_type &e1" and not "enum_flags_type::type &e1" > here? Unless I'm really messing up here, then when I try this I get the compiler error: enum-flags.h:217:41: error: declaration of =E2=80=98operator|=3D=E2=80=99= as non-function 217 | operator|=3D (enum_flags_type::type &e1, enum_type e2) Though I can't understand what the error is trying to tell me, I can understand why the compiler is unhappy. When you consider the enum_flags operator|=3D and the global operator|=3D they both now have the exact same function signature, so there's no way for the compiler to pick between them. >=20 > Andrew> DEF_ENUM_FLAGS_TYPE(enum some_flag, some_flags); >=20 > Andrew> enum some_flag f =3D flag_val1 | flag_val2; > Andrew> f |=3D flag_val3; >=20 > To me this example looks incorrect -- the idea behind enum flags is to > not use the "enum some_flag" type, but instead the wrapper. So it > should be: >=20 > some_flags f =3D flag_val1 | flag_val2; > f |=3D flag_val3; >=20 > Could you say why you want to use the enum type instead? That would > help me understand this patch. I agree, but... My argument would be: 1. By making the change I propose we loose nothing, but my example above will just work, so we do gain something. 2. We already have some global operators, these are only used when we run into cases like my original example, so clearly the code was originally written (I'm assuming) with the idea of supporting both use cases. 3. Instead of writing 'enum some_flag', a developer might just write 'some_flag'. In this case they still require the changes from my patch if they ever want to use operator|=3D. I think spotting the missing 's' is much harder during review, so it's easy for uses of 'some_flag' to creep into the code base, then a future developer wanting to use 'operator|=3D' will need to fix up the 'some_flag' to 'some_flags' miss-match. Though it's easy to argue that the first developer made a mistake, and we frequently have to fix the mistakes of those going before, in this case we don't have to, so why force the matter? I guess my argument would be, lets commit. We should either remove the existing global operators from enum-flags.h, fix up the fall out, and so make it much harder to developers to use the 'some_flag' version (so forcing the use of 'some_flags'), or make the change I propose which allows the full range of operators while loosing non of the protection that already exists. Thanks, Andrew