From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from mail-wr1-x436.google.com (mail-wr1-x436.google.com [IPv6:2a00:1450:4864:20::436]) by sourceware.org (Postfix) with ESMTPS id 7E45C3858C83 for ; Sat, 29 Oct 2022 15:58:15 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.1 sourceware.org 7E45C3858C83 Authentication-Results: sourceware.org; dmarc=none (p=none dis=none) header.from=embecosm.com Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=embecosm.com Received: by mail-wr1-x436.google.com with SMTP id j15so10232818wrq.3 for ; Sat, 29 Oct 2022 08:58:15 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=embecosm.com; s=google; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:from:to:cc:subject:date:message-id:reply-to; bh=jBknqOUGIS1n09fskArKvtQt/FXryAcmlmDSqcsrWwM=; b=M3Ul6Pz2g9mpEk3aEUEuMLdlNMdpn9seEFyH2kDxQtZ309lrna/E9LNZNyTw28jYo6 +5GS+6SP9yVCz7mE7oRhitCWknHiG3VU2w7Paba4mDlg0KbDYaKC3D2fXKmgz9ia53WO LET39M5/eXl/ozf/lMFG0SbnLsszuEdR/ZoOXCSE0UNO/Rj8MDXwVqkH3FZ6IoUMUUHX iC5+LG5rMO92xgLvJnXk4CrOP63ARChtEH2oEuDrIm+fb/gX0tWCQyq/XYcqnfX33BPF S1JNkHqd+FP46l787SprmvTqQsd+Gp1hwTlXuRlTBtw8JIzoqKTP1hQkOXKKHIhb+hBQ 5UbQ== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20210112; h=mime-version:user-agent:references:message-id:in-reply-to:subject :cc:to:from:date:x-gm-message-state:from:to:cc:subject:date :message-id:reply-to; bh=jBknqOUGIS1n09fskArKvtQt/FXryAcmlmDSqcsrWwM=; b=jY28BVmlN9Ic9ZMpmZcJ6uYdoDOQsUmc+JNkqow8V/zLNU2xhPXNdP24oxY6W1thcm 0BB3jkpEP67W7gJNNUuTtPUPuNcQe+1wTE9/o5qhm4b4fNVks8MlYUr2l002u5bX8NZ2 Q99YUAn2+jVHKHhHIJvsLHi9TskhvZ2RV07u2evaNG/B+Y0Ip3ksfP2anI2u5/2BtRcO f9Qe2UI4GRSzAVDRyOeBBDQ9S1SF3f7WKT139pqmW6ROQWfZveOyhWGvqImNUnGBnTs9 bGANWW2SM2fpNjwmtOwNZJ/zU5D5AWAQp0BPrN4TH37GtWYyTR+Phty2/7zNq0V1Kjdo NGRg== X-Gm-Message-State: ACrzQf2drs7gfNSogohBwhFrXXzuTpdTsDY12AodmO7E86jheon5f/ZI 3ZzhdNqJ3eogaoJdbWG+XZhWAA== X-Google-Smtp-Source: AMsMyM6LfPzvMukdPjXu9wQ2ANp3RkPDC87uhIQog2YyvpCKyVVINL2AFhvn/BEw1zIxFcFg/JlVFA== X-Received: by 2002:a5d:58f5:0:b0:236:2a1f:9dd1 with SMTP id f21-20020a5d58f5000000b002362a1f9dd1mr2667808wrd.637.1667059094234; Sat, 29 Oct 2022 08:58:14 -0700 (PDT) Received: from tpp.orcam.me.uk (tpp.orcam.me.uk. [2001:8b0:154:0:ea6a:64ff:fe24:f2fc]) by smtp.gmail.com with ESMTPSA id jb1-20020a05600c54e100b003c6b874a0dfsm2263389wmb.14.2022.10.29.08.58.12 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sat, 29 Oct 2022 08:58:13 -0700 (PDT) Date: Sat, 29 Oct 2022 16:58:12 +0100 (BST) From: "Maciej W. Rozycki" To: Simon Marchi cc: gdb-patches@sourceware.org, Andrew Burgess , Tom Tromey , Simon Sobisch Subject: Re: [PATCH v6 5/8] GDB/Python: Use None for `var_zuinteger_unlimited' value set to `unlimited' In-Reply-To: <0cf90df8-6943-212a-b4c5-bd5301b44369@polymtl.ca> Message-ID: References: <0cf90df8-6943-212a-b4c5-bd5301b44369@polymtl.ca> User-Agent: Alpine 2.20 (DEB 67 2015-01-07) MIME-Version: 1.0 Content-Type: text/plain; charset=US-ASCII X-Spam-Status: No, score=-2.6 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.6 X-Spam-Checker-Version: SpamAssassin 3.4.6 (2021-04-09) on server2.sourceware.org List-Id: On Mon, 17 Oct 2022, Simon Marchi wrote: > > Consistently with parameters of the PARAM_UINTEGER and PARAM_INTEGER > > types, return the special value of `None' for a PARAM_ZUINTEGER_UNLIMITED > > I had trouble parsing this sentence, I think adding a comma between > "types" and "return" would help. Fixed in v7. > > parameter set to `unlimited', fixing an inconsistency introduced with > > commit 0489430a0e1a ("Handle var_zuinteger and var_zuinteger_unlimited > > from Python"); cf. PR python/20084. Adjust the testsuite accordingly. > > > > This makes all the three parameter types consistent with each other as > > far as the use of `None' is concerned, and similar to the Guile/Scheme > > interface where the `#:unlimited' keyword is likewise used. We have a > > precedent already documented for a similar API correction: > > > > -- Function: gdb.breakpoints () > > Return a sequence holding all of GDB's breakpoints. *Note > > Breakpoints In Python::, for more information. In GDB version 7.11 > > and earlier, this function returned 'None' if there were no > > breakpoints. This peculiarity was subsequently fixed, and now > > 'gdb.breakpoints' returns an empty sequence in this case. > > > > made in the past. > > > > And then we have documentation not matching the interface as far as the > > value of `None' already returned for parameters of the PARAM_UINTEGER > > and PARAM_INTEGER types is concerned, and the case of an incorrect > > assertion for PARAM_UINTEGER and PARAM_ZUINTEGER_UNLIMITED parameters in > > the sibling Guile/Scheme module making such parameters unusable that has > > never been reported, both indicating that these features may indeed not > > be heavily used, and therefore that the risk from such an API change is > > likely lower than the long-term burden of the inconsistency. > > To rephrase, just to make sure I understand right, this is the > inconsistency you'd like to fix: > > (gdb) set an-integer unlimited > (gdb) set an-uinteger unlimited > (gdb) set a-zuinteger-unlimited unlimited > (gdb) python print(gdb.parameter('an-integer')) > None > (gdb) python print(gdb.parameter('an-uinteger')) > None > (gdb) python print(gdb.parameter('a-zuinteger-unlimited')) > -1 Correct. Please note that the -1 value for PARAM_ZUINTEGER_UNLIMITED was also only added when `None' had been already used for PARAM_UINTEGER and PARAM_ZINTEGER types, so it was wrong right from the beginning. > I'm hesitant about this kind of of breaking changes. I don't agree with > your reasoning leading you to claim that these features are not heavily > used. We had and have all kinds of inconsistencies in our MI and Python > API, where the actual API doesn't match the doc, and people usually just > silently work around them. Also, the fact that Guile was broken doesn't > mean people don't use the equivalent in Python. Fair enough, especially on the last point. My overall impression of the Python technical culture has sadly been that things largely happen on an ad-hoc basis there, with little structure or consistency. I guess this is an unfortunate side effect resulting from the low entry barrier, not by itself bad, Python presents to newcomers to computer programming. > The closest thing to empirical data we can have if searching to > occurences on Github. I just did this search, and there are no hits: > > https://github.com/search?q=PARAM_ZUINTEGER_UNLIMITED+extension%3Apy&type=Code&ref=advsearch&l=&l= > > That is, searching for PARAM_ZUINTEGER_UNLIMITED in all the .py files on > Github. As opposed to PARAM_ZINTEGER, for instance: > > https://github.com/search?q=PARAM_ZINTEGER+extension%3Apy&type=Code&ref=advsearch&l=&l= > > Of course, that's not all the code in the world, but that gives an idea. Thank you for doing this research. I wasn't aware of this Github's feature (I don't usually use Github). > And I do agree that fixing the API once will reduce the long-term costs > for everybody (us and users bumping in the inconsistency and losing > time). So, I am fine with fixing PARAM_ZUINTEGER_UNLIMITED in this > case. Good. Given that you are in favour to making this change and Andrew was against, do you think we need another opinion? Have we reached consensus? > However, I am unable to apply your patch locally in order to properly > review it. Can you send an updated version of the series, after perhaps > pushing the already approved patches that make sense on their own? I'm not sure what happened here, I had no conflicts in the rebase (but then maybe I rebased already after I posted v6 and forgot about it). I have now posted the remaining parts of the series rebased against current master. Thank you for your review. Maciej