From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from simark.ca (simark.ca [158.69.221.121]) by sourceware.org (Postfix) with ESMTPS id D98673858C2D for ; Fri, 12 May 2023 18:15:52 +0000 (GMT) DMARC-Filter: OpenDMARC Filter v1.4.2 sourceware.org D98673858C2D Authentication-Results: sourceware.org; dmarc=pass (p=none dis=none) header.from=simark.ca Authentication-Results: sourceware.org; spf=pass smtp.mailfrom=simark.ca Received: from [10.0.0.170] (unknown [167.248.160.41]) (using TLSv1.3 with cipher TLS_AES_128_GCM_SHA256 (128/128 bits) key-exchange X25519 server-signature RSA-PSS (2048 bits) server-digest SHA256) (No client certificate requested) by simark.ca (Postfix) with ESMTPSA id 227C81E0D6; Fri, 12 May 2023 14:15:52 -0400 (EDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/simple; d=simark.ca; s=mail; t=1683915352; bh=/lF9CWrf1V8jP6mg9DP/V59WkTI2q1V3TVCOr9g3kfw=; h=Date:Subject:To:Cc:References:From:In-Reply-To:From; b=pRgFhpKze2C/rv0KNnb3tNAF4Dr+KQtLUmQkorq3Hpo8P7foKSBbT86sOsMaAKXX8 WmKYzxkkyvtjRvCxVlHUC9qWTqvHyUEnzpYhnQ/QpiKD0tMYZ+6r7urn9ohVGLlGOz gIQo4+zG1DMt3NwD2RyHg3DFo0LdfiMM9AVjHuzg= Message-ID: Date: Fri, 12 May 2023 14:15:51 -0400 MIME-Version: 1.0 User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:102.0) Gecko/20100101 Thunderbird/102.11.0 Subject: Re: [PATCH] gdb.fortran/lbound-ubound.exp: read expected lbound and ubound from function parameters (PR 30414) To: Andrew Burgess , Simon Marchi via Gdb-patches Cc: Simon Marchi References: <20230504154829.34338-1-simon.marchi@efficios.com> <87o7mp5vq8.fsf@redhat.com> Content-Language: fr From: Simon Marchi In-Reply-To: <87o7mp5vq8.fsf@redhat.com> Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit X-Spam-Status: No, score=-5.7 required=5.0 tests=BAYES_00,DKIM_SIGNED,DKIM_VALID,DKIM_VALID_AU,DKIM_VALID_EF,NICE_REPLY_A,SPF_HELO_PASS,SPF_PASS,TXREP,T_SCC_BODY_TEXT_LINE 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 5/12/23 13:02, Andrew Burgess via Gdb-patches wrote: >> @@ -96,6 +68,17 @@ while { $test_count < 500 } { >> break >> } >> >> + set expected_lbound [get_valueof "" "lb" "get expected lbound"] > > I don't think this is doing what you expect (given the lines below). > The default value is going to be 'get expected lbound', while the > testname will be generated by the get_valueof call. You can just do > this: > > set expected_lbound [get_integer_valueof "lb" "" "get expected lbound"] You're right, I'm missing a parameter to get_valueof. It should have been: set expected_lbound [get_valueof "" "lb" "" "get expected lbound"] set expected_ubound [get_valueof "" "ub" "" "get expected ubound"] Note that I don't want to use get_integer_valueof here, because that proc specifically expects one integer, whereas I get an array of integers: print lb^M $1 = (-8, -10)^M (gdb) PASS: gdb.fortran/lbound-ubound.exp: test 0: get expected lbound print ub^M $2 = (-1, -2)^M (gdb) PASS: gdb.fortran/lbound-ubound.exp: test 0: get expected ubound Luckily, GDB outputs them in the same format as the inferior outputs them before my patch. So the rest of the test doesn't see the difference. >> + set expected_ubound [get_valueof "" "ub" "get expected ubound"] >> + >> + if { $expected_lbound == "" } { >> + error "failed to extract expected results for lbound" >> + } >> + >> + if { $expected_ubound == "" } { >> + error "failed to extract expected results for ubound" >> + } > > I was originally going to suggest dropping these checks -- having the > default expected result be the empty string (or maybe use some other > invalid string, e.g. '*INVALID*'?) will cause the following tests to > fail anyway, so the check doesn't seem to add much. > > But I see all you really did was move the checks I already had ... so I > guess that's fine. I don't think I'd write this test in this way today > though. I am fine with dropping the checks. > I do wonder why the switch from perror to error though? The original > perror would print the error but then continue to try the next test, > while error aborts the whole test script. I guess if one goes wrong > then they all go wrong... I guess it doesn't really matter much. I'm not a big fan of perror in the first place, I find it's really easy to mis-use. Like here, the test calls perror, but then return. So what is the next test that it intends to mark as unresolved? Is it going to affect the next .exp to be executed? I feel like something like that happened to me before and I chased the source of random unresolveds for a long time. I just re-read the documentation, and noticed: If the optional numeric value is 0, then there are no further side effects to calling this function, and the following test outcome doesn’t become UNRESOLVED. This can be used for errors with no known side effects. So I suppose that calling perror with 0 would be fine here. Except that I just tried it and: Running /home/smarchi/src/binutils-gdb/gdb/testsuite/gdb.fortran/lbound-ubound.exp ... ERROR: hello ... # of expected passes 5 ... $ echo $? 0 That error doesn't leave a trace in the summary and the exit status is 0, so it seems very easy to miss. So, my preference in that case is to use "error", it's impossible to miss. It means we abort the whole test, but that seems fine in this situation where something just unrecoverable happened. But again, I'm fine with not having checks at all, it means we would get a bunch of FAILs in that case, which is also impossible to miss. Simon