From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-13.6 required=3.0 tests=BAYES_00,DKIM_INVALID, DKIM_SIGNED,HEADER_FROM_DIFFERENT_DOMAINS,INCLUDES_CR_TRAILER,INCLUDES_PATCH, MAILING_LIST_MULTI,SPF_HELO_NONE,SPF_PASS,URIBL_BLOCKED autolearn=ham autolearn_force=no version=3.4.0 Received: from mail.kernel.org (mail.kernel.org [198.145.29.99]) by smtp.lore.kernel.org (Postfix) with ESMTP id 9B2D8C433ED for ; Thu, 15 Apr 2021 07:02:30 +0000 (UTC) Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by mail.kernel.org (Postfix) with ESMTPS id 238826117A for ; Thu, 15 Apr 2021 07:02:30 +0000 (UTC) DMARC-Filter: OpenDMARC Filter v1.3.2 mail.kernel.org 238826117A Authentication-Results: mail.kernel.org; dmarc=fail (p=none dis=none) header.from=redhat.com Authentication-Results: mail.kernel.org; spf=pass smtp.mailfrom=qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Received: from localhost ([::1]:45156 helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1lWw1F-0002Do-3u for qemu-devel@archiver.kernel.org; Thu, 15 Apr 2021 03:02:29 -0400 Received: from eggs.gnu.org ([2001:470:142:3::10]:54200) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lWvzx-0001hy-KF for qemu-devel@nongnu.org; Thu, 15 Apr 2021 03:01:11 -0400 Received: from us-smtp-delivery-124.mimecast.com ([216.205.24.124]:54809) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1lWvzt-0003OW-G6 for qemu-devel@nongnu.org; Thu, 15 Apr 2021 03:01:08 -0400 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1618470063; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=6aCYtcEHu+Hy3kbg68qhg3RgmW6MbsX6bLuHuelOcVY=; b=Ui4mN6Ft9fW5sCKW1+c/P7pndEa3QiGha0IbOWqoY2HUWoQle6nhpVh4M58nhF0erQzfkT X+YTNdJZPzRnepYavmKja2tFao4y5whQZGMwVeUXv3Yu1zlaIS+nTKup+ocpQdItMmkJnx VG5mTN/PanicXFgtpnH5m11JNKnU0CA= Received: from mimecast-mx01.redhat.com (mimecast-mx01.redhat.com [209.132.183.4]) (Using TLS) by relay.mimecast.com with ESMTP id us-mta-63-9VJkhQ7fNgK4-HX2fQrf3w-1; Thu, 15 Apr 2021 03:01:01 -0400 X-MC-Unique: 9VJkhQ7fNgK4-HX2fQrf3w-1 Received: from smtp.corp.redhat.com (int-mx04.intmail.prod.int.phx2.redhat.com [10.5.11.14]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mimecast-mx01.redhat.com (Postfix) with ESMTPS id 342BF1883522; Thu, 15 Apr 2021 07:01:00 +0000 (UTC) Received: from blackfin.pond.sub.org (ovpn-114-17.ams2.redhat.com [10.36.114.17]) by smtp.corp.redhat.com (Postfix) with ESMTPS id DAAB65D9DE; Thu, 15 Apr 2021 07:00:59 +0000 (UTC) Received: by blackfin.pond.sub.org (Postfix, from userid 1000) id 5E5F9113525D; Thu, 15 Apr 2021 09:00:58 +0200 (CEST) From: Markus Armbruster To: John Snow Subject: Re: [PATCH v2 4/8] qapi/error: Change assertion References: <20210330171844.1197918-1-jsnow@redhat.com> <20210330171844.1197918-5-jsnow@redhat.com> Date: Thu, 15 Apr 2021 09:00:58 +0200 In-Reply-To: (John Snow's message of "Thu, 8 Apr 2021 11:31:25 -0400") Message-ID: <87blagghg5.fsf@dusky.pond.sub.org> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/27.1 (gnu/linux) MIME-Version: 1.0 X-Scanned-By: MIMEDefang 2.79 on 10.5.11.14 Authentication-Results: relay.mimecast.com; auth=pass smtp.auth=CUSA124A263 smtp.mailfrom=armbru@redhat.com X-Mimecast-Spam-Score: 0 X-Mimecast-Originator: redhat.com Content-Type: text/plain Received-SPF: pass client-ip=216.205.24.124; envelope-from=armbru@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -27 X-Spam_score: -2.8 X-Spam_bar: -- X-Spam_report: (-2.8 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H4=0.001, RCVD_IN_MSPIKE_WL=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.23 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Cc: Peter Maydell , Eduardo Habkost , Michael Roth , Michael Roth , qemu-devel@nongnu.org, Cleber Rosa Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: "Qemu-devel" John Snow writes: > On 3/30/21 1:18 PM, John Snow wrote: > > Realizing now that this commit topic is wrong :) > > A prior version modified the assertion, I decided it was less churn to > simply add one. > > I think ideally we'd have no assertions here and we'd rely on the type > hints, but I don't think I can prove that this is safe until after part > 6, so I did this for now instead. > >> Eventually, we'll be able to prove that 'info.line' must be an int and >> is never None at static analysis time, and this assert can go >> away. Until then, it's a type error to assume that self.info is not >> None. >> >> Signed-off-by: John Snow >> --- >> scripts/qapi/error.py | 1 + >> 1 file changed, 1 insertion(+) >> >> diff --git a/scripts/qapi/error.py b/scripts/qapi/error.py >> index d179a3bd0c..d0bc7af6e7 100644 >> --- a/scripts/qapi/error.py >> +++ b/scripts/qapi/error.py >> @@ -25,6 +25,7 @@ def __init__(self, info, msg, col=None): >> self.col = col >> >> def __str__(self): >> + assert self.info is not None >> loc = str(self.info) >> if self.col is not None: >> assert self.info.line is not None >> Please show us the revised commit message. I'm asking because the patch's purpose isn't quite clear to me. Peeking ahead at PATCH 7, I see that info becomes Optional[QAPISourceInfo]. This means something passes None for info (else you wouldn't make it optional). None info Works (in the sense of "doesn't crash") as long as col is also None. After the patch, it doesn't. I'm not saying that's bad, only that I don't quite understand what you're trying to accomplish here.