| Message ID | 609ab8e5-9533-1ec3-c7ed-2086acde0a13@gmail.com |
|---|---|
| State | New |
| Headers | show |
| Series | avoid -Wnonull for dynamic_cast (PR 99251) | expand |
On 2/24/21 5:25 PM, Martin Sebor wrote: > In r11-6900 (PR 98646 - static_cast confuses -Wnonnull) we decided > that issuing -Wnonnull for dereferencing the result of dynamic_cast > was helpful despite the false positives it causes when the pointer > is guaranteed not to be null because of a prior test. > > The test case in PR 99251 along with the feedback I got from Martin > Liska have convinced me it was not the right decision. > > The attached patch arranges for dynamic_cast to also suppress -Wnonnull > analogously to static_cast. Since there already is a helper function > that builds the if-not-null test (ifnonnull) and sets TREE_NO_WARNING, > I factored out the corresponding code from build_base_path that sets > the additional TREE_NO_WARNING bit for static_cast into the function > and called it from both places. I also renamed the function to make > its purpose clearer and for consistency with other build_xxx APIs. Let's call it build_if_nonnull, as it builds the COND_EXPR as well as the test. > + /* The dynamic_cast might fail but so a warning might be justified > + but not when the operand is guarded. See pr99251. */ > + if (B *q = p->bptr ()) > + dynamic_cast<C*>(q)->g (); // { dg-bogus "\\\[-Wnonnull" } This guard is no more necessary than it is for static_cast; both cases deal with null arguments. Let's not add these checks to the testcases. This guard doesn't check for the mentioned case of dynamic_cast failing because the B* does not in fact point to a C. I think we can just change the dg-warning to dg-bogus. Sure, dynamic_cast might fail, but AFAICT -Wnonnull isn't supposed to warn about arguments that *might* be null, only arguments that are *known* to be null. Jason
On 2/24/21 8:13 PM, Jason Merrill wrote: > On 2/24/21 5:25 PM, Martin Sebor wrote: >> In r11-6900 (PR 98646 - static_cast confuses -Wnonnull) we decided >> that issuing -Wnonnull for dereferencing the result of dynamic_cast >> was helpful despite the false positives it causes when the pointer >> is guaranteed not to be null because of a prior test. >> >> The test case in PR 99251 along with the feedback I got from Martin >> Liska have convinced me it was not the right decision. >> >> The attached patch arranges for dynamic_cast to also suppress -Wnonnull >> analogously to static_cast. Since there already is a helper function >> that builds the if-not-null test (ifnonnull) and sets TREE_NO_WARNING, >> I factored out the corresponding code from build_base_path that sets >> the additional TREE_NO_WARNING bit for static_cast into the function >> and called it from both places. I also renamed the function to make >> its purpose clearer and for consistency with other build_xxx APIs. > > Let's call it build_if_nonnull, as it builds the COND_EXPR as well as > the test. Done. > >> + /* The dynamic_cast might fail but so a warning might be justified >> + but not when the operand is guarded. See pr99251. */ >> + if (B *q = p->bptr ()) >> + dynamic_cast<C*>(q)->g (); // { dg-bogus >> "\\\[-Wnonnull" } > > This guard is no more necessary than it is for static_cast; both cases > deal with null arguments. Let's not add these checks to the testcases. Done. > > This guard doesn't check for the mentioned case of dynamic_cast failing > because the B* does not in fact point to a C. > > I think we can just change the dg-warning to dg-bogus. Sure, > dynamic_cast might fail, but AFAICT -Wnonnull isn't supposed to warn > about arguments that *might* be null, only arguments that are *known* to > be null. Done. I had added the 'if' to the test to make it clear why the warning isn't expected. I replaced it with a comment to that effect. FWIW, I do think a warning for dynamic_cast to a pointer would be helpful in the absence of an if statement in these cases: void f (B *p) { dynamic_cast<D*>(p)->g (); } Not because p may be null but because the result of the cast may be for a nonnull p. If it's f's precondition that D is derived from B (in addition to p being nonnull) the cast would be better written as one to a reference to D. Such a warning would need to be issued by the middle end and although it could be controlled by a new option (say -Wdynamic-cast, along with the cases discussed in PR 12277) I don't think issuing -Wnonnull in this case would be inappropriate. Anyway, that's something to consider for GCC 12. For now, please see the revised patch. Martin > > Jason >
On 3/1/21 12:10 PM, Martin Sebor wrote: > On 2/24/21 8:13 PM, Jason Merrill wrote: >> On 2/24/21 5:25 PM, Martin Sebor wrote: >>> In r11-6900 (PR 98646 - static_cast confuses -Wnonnull) we decided >>> that issuing -Wnonnull for dereferencing the result of dynamic_cast >>> was helpful despite the false positives it causes when the pointer >>> is guaranteed not to be null because of a prior test. >>> >>> The test case in PR 99251 along with the feedback I got from Martin >>> Liska have convinced me it was not the right decision. >>> >>> The attached patch arranges for dynamic_cast to also suppress -Wnonnull >>> analogously to static_cast. Since there already is a helper function >>> that builds the if-not-null test (ifnonnull) and sets TREE_NO_WARNING, >>> I factored out the corresponding code from build_base_path that sets >>> the additional TREE_NO_WARNING bit for static_cast into the function >>> and called it from both places. I also renamed the function to make >>> its purpose clearer and for consistency with other build_xxx APIs. >> >> Let's call it build_if_nonnull, as it builds the COND_EXPR as well as >> the test. > > Done. > >> >>> + /* The dynamic_cast might fail but so a warning might be justified >>> + but not when the operand is guarded. See pr99251. */ >>> + if (B *q = p->bptr ()) >>> + dynamic_cast<C*>(q)->g (); // { dg-bogus >>> "\\\[-Wnonnull" } >> >> This guard is no more necessary than it is for static_cast; both cases >> deal with null arguments. Let's not add these checks to the testcases. > > Done. > >> >> This guard doesn't check for the mentioned case of dynamic_cast >> failing because the B* does not in fact point to a C. >> >> I think we can just change the dg-warning to dg-bogus. Sure, >> dynamic_cast might fail, but AFAICT -Wnonnull isn't supposed to warn >> about arguments that *might* be null, only arguments that are *known* >> to be null. > > Done. > > I had added the 'if' to the test to make it clear why the warning > isn't expected. I replaced it with a comment to that effect. > FWIW, I do think a warning for dynamic_cast to a pointer would be > helpful in the absence of an if statement in these cases: > > void f (B *p) > { > dynamic_cast<D*>(p)->g (); > } > > Not because p may be null but because the result of the cast may be > for a nonnull p. If it's f's precondition that D is derived from B > (in addition to p being nonnull) the cast would be better written as > one to a reference to D. Agreed, or if it isn't a precondition, if (D* dp = dynamic_cast<D*>(p)) dp->g(); > Such a warning would need to be issued by the middle end and although > it could be controlled by a new option (say -Wdynamic-cast, along with > the cases discussed in PR 12277) Sounds good. > I don't think issuing -Wnonnull in this case would be inappropriate. I disagree; issuing -Wnonnull for this case would be wrong. Again, -Wnonnull is supposed to warn when an argument *is* null, not when it *might* be null. I think warning about this case is a good idea, just not as part of -Wnonnull. > Anyway, that's something to consider for GCC 12. For now, please see > the revised patch. > * rtti.c (ifnonnull): Rename... > (build_nonnull_test): ...to this. Set no-warning bit on COND_EXPR. The ChangeLog needs updating. > + /* Unlike static_cast, dynamic cast may fail for a nonnull operand but Yes, but... > + since the front end can't see if the cast is guarded against being > + invoked with a null No; my point was that whether the cast is guarded against being invoked with a null is no more relevant for dynamic_cast than it is for static_cast, as both casts give a null result for a null argument. For this test, dynamic_cast is not significantly different from static_cast. For both casts, the bug was the compiler warning about a nullptr that it introduced itself. > verify there's no warning. See also pr99251. */ Yes. > - dynamic_cast<const C*>(p->bptr ())->g (); // { dg-warning "\\\[-Wnonnull" } > + dynamic_cast<const C*>(p)->g (); // { dg-bogus "\\\[-Wnonnull" } Please put back the ->bptr()s. Jason
On 3/1/21 1:33 PM, Jason Merrill wrote: > On 3/1/21 12:10 PM, Martin Sebor wrote: >> On 2/24/21 8:13 PM, Jason Merrill wrote: >>> On 2/24/21 5:25 PM, Martin Sebor wrote: >>>> In r11-6900 (PR 98646 - static_cast confuses -Wnonnull) we decided >>>> that issuing -Wnonnull for dereferencing the result of dynamic_cast >>>> was helpful despite the false positives it causes when the pointer >>>> is guaranteed not to be null because of a prior test. >>>> >>>> The test case in PR 99251 along with the feedback I got from Martin >>>> Liska have convinced me it was not the right decision. >>>> >>>> The attached patch arranges for dynamic_cast to also suppress -Wnonnull >>>> analogously to static_cast. Since there already is a helper function >>>> that builds the if-not-null test (ifnonnull) and sets TREE_NO_WARNING, >>>> I factored out the corresponding code from build_base_path that sets >>>> the additional TREE_NO_WARNING bit for static_cast into the function >>>> and called it from both places. I also renamed the function to make >>>> its purpose clearer and for consistency with other build_xxx APIs. >>> >>> Let's call it build_if_nonnull, as it builds the COND_EXPR as well as >>> the test. >> >> Done. >> >>> >>>> + /* The dynamic_cast might fail but so a warning might be justified >>>> + but not when the operand is guarded. See pr99251. */ >>>> + if (B *q = p->bptr ()) >>>> + dynamic_cast<C*>(q)->g (); // { dg-bogus >>>> "\\\[-Wnonnull" } >>> >>> This guard is no more necessary than it is for static_cast; both >>> cases deal with null arguments. Let's not add these checks to the >>> testcases. >> >> Done. >> >>> >>> This guard doesn't check for the mentioned case of dynamic_cast >>> failing because the B* does not in fact point to a C. >>> >>> I think we can just change the dg-warning to dg-bogus. Sure, >>> dynamic_cast might fail, but AFAICT -Wnonnull isn't supposed to warn >>> about arguments that *might* be null, only arguments that are *known* >>> to be null. >> >> Done. >> >> I had added the 'if' to the test to make it clear why the warning >> isn't expected. I replaced it with a comment to that effect. > >> FWIW, I do think a warning for dynamic_cast to a pointer would be >> helpful in the absence of an if statement in these cases: >> >> void f (B *p) >> { >> dynamic_cast<D*>(p)->g (); >> } >> >> Not because p may be null but because the result of the cast may be >> for a nonnull p. If it's f's precondition that D is derived from B >> (in addition to p being nonnull) the cast would be better written as >> one to a reference to D. > > Agreed, or if it isn't a precondition, > > if (D* dp = dynamic_cast<D*>(p)) > dp->g(); > >> Such a warning would need to be issued by the middle end and although >> it could be controlled by a new option (say -Wdynamic-cast, along with >> the cases discussed in PR 12277) > > Sounds good. > >> I don't think issuing -Wnonnull in this case would be inappropriate. > > I disagree; issuing -Wnonnull for this case would be wrong. Again, > -Wnonnull is supposed to warn when an argument *is* null, not when it > *might* be null. > > I think warning about this case is a good idea, just not as part of > -Wnonnull. > >> Anyway, that's something to consider for GCC 12. For now, please see >> the revised patch. > >> * rtti.c (ifnonnull): Rename... >> (build_nonnull_test): ...to this. Set no-warning bit on COND_EXPR. > > The ChangeLog needs updating. > >> + /* Unlike static_cast, dynamic cast may fail for a nonnull operand but > > Yes, but... > >> + since the front end can't see if the cast is guarded against being >> + invoked with a null > > No; my point was that whether the cast is guarded against being invoked > with a null is no more relevant for dynamic_cast than it is for > static_cast, as both casts give a null result for a null argument. > > For this test, dynamic_cast is not significantly different from > static_cast. For both casts, the bug was the compiler warning about a > nullptr that it introduced itself. It seems like splitting hairs. The comment (much as the original if guard) is just meant to explain why -Wnonnull isn't expected in case a new warning is added that detects the bad assumption above. I want to make it clear that if/when that happens a failure here doesn't mislead the author into thinking we don't want any warning there at all. I have reworded the comments yet again. > >> verify there's no warning. See also pr99251. */ > > Yes. > >> - dynamic_cast<const C*>(p->bptr ())->g (); // { dg-warning >> "\\\[-Wnonnull" } >> + dynamic_cast<const C*>(p)->g (); // { dg-bogus >> "\\\[-Wnonnull" } > > Please put back the ->bptr()s. Done. Martin > > Jason >
On 3/1/21 7:44 PM, Martin Sebor wrote: > On 3/1/21 1:33 PM, Jason Merrill wrote: >> On 3/1/21 12:10 PM, Martin Sebor wrote: >>> On 2/24/21 8:13 PM, Jason Merrill wrote: >>>> On 2/24/21 5:25 PM, Martin Sebor wrote: >>>>> In r11-6900 (PR 98646 - static_cast confuses -Wnonnull) we decided >>>>> that issuing -Wnonnull for dereferencing the result of dynamic_cast >>>>> was helpful despite the false positives it causes when the pointer >>>>> is guaranteed not to be null because of a prior test. >>>>> >>>>> The test case in PR 99251 along with the feedback I got from Martin >>>>> Liska have convinced me it was not the right decision. >>>>> >>>>> The attached patch arranges for dynamic_cast to also suppress >>>>> -Wnonnull >>>>> analogously to static_cast. Since there already is a helper function >>>>> that builds the if-not-null test (ifnonnull) and sets TREE_NO_WARNING, >>>>> I factored out the corresponding code from build_base_path that sets >>>>> the additional TREE_NO_WARNING bit for static_cast into the function >>>>> and called it from both places. I also renamed the function to make >>>>> its purpose clearer and for consistency with other build_xxx APIs. >>>> >>>> Let's call it build_if_nonnull, as it builds the COND_EXPR as well >>>> as the test. >>> >>> Done. >>> >>>> >>>>> + /* The dynamic_cast might fail but so a warning might be justified >>>>> + but not when the operand is guarded. See pr99251. */ >>>>> + if (B *q = p->bptr ()) >>>>> + dynamic_cast<C*>(q)->g (); // { dg-bogus >>>>> "\\\[-Wnonnull" } >>>> >>>> This guard is no more necessary than it is for static_cast; both >>>> cases deal with null arguments. Let's not add these checks to the >>>> testcases. >>> >>> Done. >>> >>>> >>>> This guard doesn't check for the mentioned case of dynamic_cast >>>> failing because the B* does not in fact point to a C. >>>> >>>> I think we can just change the dg-warning to dg-bogus. Sure, >>>> dynamic_cast might fail, but AFAICT -Wnonnull isn't supposed to warn >>>> about arguments that *might* be null, only arguments that are >>>> *known* to be null. >>> >>> Done. >>> >>> I had added the 'if' to the test to make it clear why the warning >>> isn't expected. I replaced it with a comment to that effect. >> >>> FWIW, I do think a warning for dynamic_cast to a pointer would be >>> helpful in the absence of an if statement in these cases: >>> >>> void f (B *p) >>> { >>> dynamic_cast<D*>(p)->g (); >>> } >>> >>> Not because p may be null but because the result of the cast may be >>> for a nonnull p. If it's f's precondition that D is derived from B >>> (in addition to p being nonnull) the cast would be better written as >>> one to a reference to D. >> >> Agreed, or if it isn't a precondition, >> >> if (D* dp = dynamic_cast<D*>(p)) >> dp->g(); >> >>> Such a warning would need to be issued by the middle end and although >>> it could be controlled by a new option (say -Wdynamic-cast, along with >>> the cases discussed in PR 12277) >> >> Sounds good. >> >>> I don't think issuing -Wnonnull in this case would be inappropriate. >> >> I disagree; issuing -Wnonnull for this case would be wrong. Again, >> -Wnonnull is supposed to warn when an argument *is* null, not when it >> *might* be null. >> >> I think warning about this case is a good idea, just not as part of >> -Wnonnull. >> >>> Anyway, that's something to consider for GCC 12. For now, please see >>> the revised patch. >> >>> * rtti.c (ifnonnull): Rename... >>> (build_nonnull_test): ...to this. Set no-warning bit on COND_EXPR. >> >> The ChangeLog needs updating. >> >>> + /* Unlike static_cast, dynamic cast may fail for a nonnull operand >>> but >> >> Yes, but... >> >>> + since the front end can't see if the cast is guarded against being >>> + invoked with a null >> >> No; my point was that whether the cast is guarded against being >> invoked with a null is no more relevant for dynamic_cast than it is >> for static_cast, as both casts give a null result for a null argument. >> >> For this test, dynamic_cast is not significantly different from >> static_cast. For both casts, the bug was the compiler warning about a >> nullptr that it introduced itself. > > It seems like splitting hairs. The comment (much as the original > if guard) is just meant to explain why -Wnonnull isn't expected in > case a new warning is added that detects the bad assumption above. > I want to make it clear that if/when that happens a failure here > doesn't mislead the author into thinking we don't want any warning > there at all. > > I have reworded the comments yet again. > >> >>> verify there's no warning. See also pr99251. */ >> >> Yes. >> >>> - dynamic_cast<const C*>(p->bptr ())->g (); // { dg-warning >>> "\\\[-Wnonnull" } >>> + dynamic_cast<const C*>(p)->g (); // { dg-bogus >>> "\\\[-Wnonnull" } >> >> Please put back the ->bptr()s. > > Done. OK, thanks. Jason
PR c++/99251 - inconsistent -Wnonnull warning behaviour with dynamic_cast gcc/cp/ChangeLog: PR c++/99251 * class.c (build_base_path): Call build_nonnull_test. * cp-tree.h (build_nonnull_test): Declare. * rtti.c (ifnonnull): Rename... (build_nonnull_test): ...to this. Set no-warning bit on COND_EXPR. (build_dynamic_cast_1): Adjust to name change. gcc/testsuite/ChangeLog: PR c++/99251 * g++.dg/warn/Wnonnull9.C: Expect no warnings. * g++.dg/warn/Wnonnull12.C: New test. diff --git a/gcc/cp/class.c b/gcc/cp/class.c index 40f5fef7baa..6c6e0564bf9 100644 --- a/gcc/cp/class.c +++ b/gcc/cp/class.c @@ -402,16 +402,9 @@ build_base_path (enum tree_code code, if (TREE_SIDE_EFFECTS (expr) && (null_test || virtual_access)) expr = save_expr (expr); - /* Now that we've saved expr, build the real null test. */ + /* Store EXPR and build the real null test just before returning. */ if (null_test) - { - tree zero = cp_convert (TREE_TYPE (expr), nullptr_node, complain); - null_test = build2_loc (input_location, NE_EXPR, boolean_type_node, - expr, zero); - /* This is a compiler generated comparison, don't emit - e.g. -Wnonnull-compare warning for it. */ - TREE_NO_WARNING (null_test) = 1; - } + null_test = expr; /* If this is a simple base reference, express it as a COMPONENT_REF. */ if (code == PLUS_EXPR && !virtual_access @@ -516,14 +509,8 @@ build_base_path (enum tree_code code, out: if (null_test) - { - expr = fold_build3_loc (input_location, COND_EXPR, target_type, null_test, - expr, build_zero_cst (target_type)); - /* Avoid warning for the whole conditional expression (in addition - to NULL_TEST itself -- see above) in case the result is used in - a nonnull context that the front end -Wnonnull checks. */ - TREE_NO_WARNING (expr) = 1; - } + /* Wrap EXPR in a null test. */ + expr = build_nonnull_test (null_test, expr, complain); return expr; } diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 38b31e3908f..8c6cda8d1a6 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -7271,6 +7271,7 @@ extern void emit_support_tinfos (void); extern bool emit_tinfo_decl (tree); extern unsigned get_pseudo_tinfo_index (tree); extern tree get_pseudo_tinfo_type (unsigned); +extern tree build_nonnull_test (tree, tree, tsubst_flags_t); /* in search.c */ extern tree get_parent_with_private_access (tree decl, tree binfo); diff --git a/gcc/cp/rtti.c b/gcc/cp/rtti.c index b41d95469c6..84482743392 100644 --- a/gcc/cp/rtti.c +++ b/gcc/cp/rtti.c @@ -121,7 +121,6 @@ vec<tree, va_gc> *unemitted_tinfo_decls; and are generated as needed. */ static GTY (()) vec<tinfo_s, va_gc> *tinfo_descs; -static tree ifnonnull (tree, tree, tsubst_flags_t); static tree tinfo_name (tree, bool); static tree build_dynamic_cast_1 (location_t, tree, tree, tsubst_flags_t); static tree throw_bad_cast (void); @@ -529,16 +528,23 @@ get_typeid (tree type, tsubst_flags_t complain) /* Check whether TEST is null before returning RESULT. If TEST is used in RESULT, it must have previously had a save_expr applied to it. */ -static tree -ifnonnull (tree test, tree result, tsubst_flags_t complain) +tree +build_nonnull_test (tree test, tree result, tsubst_flags_t complain) { - tree cond = build2 (NE_EXPR, boolean_type_node, test, - cp_convert (TREE_TYPE (test), nullptr_node, complain)); + tree null_ptr = cp_convert (TREE_TYPE (test), nullptr_node, complain); + tree cond = build2 (NE_EXPR, boolean_type_node, test, null_ptr); + /* This is a compiler generated comparison, don't emit e.g. -Wnonnull-compare warning for it. */ TREE_NO_WARNING (cond) = 1; - return build3 (COND_EXPR, TREE_TYPE (result), cond, result, - cp_convert (TREE_TYPE (result), nullptr_node, complain)); + + null_ptr = cp_convert (TREE_TYPE (result), nullptr_node, complain); + cond = build3 (COND_EXPR, TREE_TYPE (result), cond, result, null_ptr); + + /* Likewise, don't emit -Wnonnull for using the result to call + a member function. */ + TREE_NO_WARNING (cond) = 1; + return cond; } /* Execute a dynamic cast, as described in section 5.2.6 of the 9/93 working @@ -671,7 +677,7 @@ build_dynamic_cast_1 (location_t loc, tree type, tree expr, expr1 = build_headof (expr); if (TREE_TYPE (expr1) != type) expr1 = build1 (NOP_EXPR, type, expr1); - return ifnonnull (expr, expr1, complain); + return build_nonnull_test (expr, expr1, complain); } else { @@ -786,7 +792,7 @@ build_dynamic_cast_1 (location_t loc, tree type, tree expr, /* Now back to the type we want from a void*. */ result = cp_convert (type, result, complain); - return ifnonnull (expr, result, complain); + return build_nonnull_test (expr, result, complain); } } else diff --git a/gcc/testsuite/g++.dg/warn/Wnonnull12.C b/gcc/testsuite/g++.dg/warn/Wnonnull12.C new file mode 100644 index 00000000000..7b2606302f5 --- /dev/null +++ b/gcc/testsuite/g++.dg/warn/Wnonnull12.C @@ -0,0 +1,29 @@ +/* PR c++/99251 - inconsistent -Wnonnull warning behaviour with dynamic_cast + { dg-do compile } + { dg-options "-Wall" } */ + +struct A +{ + virtual ~A (); +}; + +struct B: A +{ + int f (int); +}; + +int f1 (A *p) +{ + if (!p) + return 0; + + return (dynamic_cast<B *>(p))->f (1); +} + +int f2 (A *p) +{ + if (!p) + return 0; + + return dynamic_cast<B *>(p)->f (2); // { dg-bogus "\\\[-Wnonnull" } +} diff --git a/gcc/testsuite/g++.dg/warn/Wnonnull9.C b/gcc/testsuite/g++.dg/warn/Wnonnull9.C index b6135c4a624..e7c0dda2502 100644 --- a/gcc/testsuite/g++.dg/warn/Wnonnull9.C +++ b/gcc/testsuite/g++.dg/warn/Wnonnull9.C @@ -38,13 +38,16 @@ void static_cast_const_C_ptr (B *p) void dynamic_cast_C_ptr (B *p) { - // The dynamic_cast might fail so a warning is justified. - dynamic_cast<C*>(p->bptr ())->g (); // { dg-warning "\\\[-Wnonnull" } + /* The dynamic_cast might fail but so a warning might be justified + but not when the operand is guarded. See pr99251. */ + if (B *q = p->bptr ()) + dynamic_cast<C*>(q)->g (); // { dg-bogus "\\\[-Wnonnull" } } void dynamic_cast_const_C_ptr (B *p) { - dynamic_cast<const C*>(p->bptr ())->g (); // { dg-warning "\\\[-Wnonnull" } + if (B *q = p->bptr ()) + dynamic_cast<const C*>(q)->g (); // { dg-bogus "\\\[-Wnonnull" } } @@ -107,11 +110,14 @@ void static_cast_const_D_ptr (B *p) void dynamic_cast_D_ptr (B *p) { - // The dynamic_cast might fail so a warning is justified. - dynamic_cast<D*>(p->bptr ())->g (); // { dg-warning "\\\[-Wnonnull" } + /* The dynamic_cast might fail but so a warning might be justified + but not when the operand is guarded. See pr99251. */ + if (B *q = p->bptr ()) + dynamic_cast<D*>(q)->g (); // { dg-bogus "\\\[-Wnonnull" } } void dynamic_cast_const_D_ptr (B *p) { - dynamic_cast<const D*>(p->bptr ())->g (); // { dg-warning "\\\[-Wnonnull" } + if (B *q = p->bptr ()) + dynamic_cast<const D*>(q)->g (); // { dg-bogus "\\\[-Wnonnull" } }