Thanks for the review. I've tested and uploaded a new patch v2 with
the requested changes.
On Thu, 17 Mar 2022 at 09:20, Jason Merrill <ja...@redhat.com> wrote:
>
> On 3/14/22 02:36, Zhao Wei Liew via Gcc-patches wrote:
>
> > This patch adds a warning when casting "this" to Derived* in the Base
> > class constructor and destructor. I've added the warning under the
> > -Wextra flag as I can't find a more appropriate flag for it.
>
> It's generally best to add a new warning flag if an existing one isn't a
> good match. Maybe -Wderived-cast-undefined? It seems like it could be
> part of -Wall.
Hmm, I agree. I've added a new -Wderived-cast-undefined flag for this.
> > + if (current_function_decl
> > + && (DECL_CONSTRUCTOR_P (current_function_decl)
> > + || DECL_DESTRUCTOR_P (current_function_decl))
> > + && TREE_CODE (expr) == NOP_EXPR
> > + && is_this_parameter (TREE_OPERAND (expr, 0)))
>
> I think the resolves_to_fixed_type_p function would be useful here;
> you're testing for a subset of the cases it handles.
Does the new patch look like how you intended it to? The new patch
passes all regression tests and newly added tests. However, I don't
fully understand the code for resolves_to_fixed_type_p() and
fixed_type_or_null(), except that I see that they contain logic to
return -1 when within a ctor/dtor.
Thanks!
From a8bbd4158f3311372b67d32816ce40d5613ae0d8 Mon Sep 17 00:00:00 2001
From: Zhao Wei Liew <zhaoweil...@gmail.com>
Date: Tue, 22 Feb 2022 16:03:17 +0800
Subject: [PATCH] c++: warn when casting Base* to Derived* in Base ctor/dtor
[PR96765]
Casting "this" in a base class constructor to a derived class type is
undefined behaviour, but there is no warning when doing so.
Add a warning for this.
Signed-off-by: Zhao Wei Liew <zhaoweil...@gmail.com>
PR c++/96765
gcc/c-family/ChangeLog:
* c.opt (-Wderived-cast-undefined): New option.
gcc/cp/ChangeLog:
* typeck.cc (build_static_cast_1): Add a warning when casting
Base* to Derived* in Base constructor and destructor.
gcc/testsuite/ChangeLog:
* g++.dg/warn/Wderived-cast-undefined.C: New test.
---
gcc/c-family/c.opt | 4 +++
gcc/cp/typeck.cc | 6 ++++
.../g++.dg/warn/Wderived-cast-undefined.C | 33 +++++++++++++++++++
3 files changed, 43 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/warn/Wderived-cast-undefined.C
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 9cfd2a6bc4e..1681395c529 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -598,6 +598,10 @@ C++ ObjC++ Var(warn_deprecated_enum_float_conv) Warning
Warn about deprecated arithmetic conversions on operands where one is of
enumeration
type and the other is of a floating-point type.
+Wderived-cast-undefined
+C++ Var(warn_derived_cast_undefined) Warning LangEnabledBy(C++, Wall)
+Warn about undefined casts from Base* to Derived* in the Base constructor or
destructor.
+
Wdesignated-init
C ObjC Var(warn_designated_init) Init(1) Warning
Warn about positional initialization of structs requiring designated
initializers.
diff --git a/gcc/cp/typeck.cc b/gcc/cp/typeck.cc
index 516fa574ef6..0bbbc378806 100644
--- a/gcc/cp/typeck.cc
+++ b/gcc/cp/typeck.cc
@@ -8079,6 +8079,12 @@ build_static_cast_1 (location_t loc, tree type, tree
expr, bool c_cast_p,
{
tree base;
+ if (TREE_CODE (expr) == NOP_EXPR
+ && resolves_to_fixed_type_p (TREE_OPERAND (expr, 0)) == -1)
+ warning_at (loc, OPT_Wderived_cast_undefined,
+ "invalid %<static_cast%> from type %qT to type %qT before
the latter is constructed",
+ intype, type);
+
if (processing_template_decl)
return expr;
diff --git a/gcc/testsuite/g++.dg/warn/Wderived-cast-undefined.C
b/gcc/testsuite/g++.dg/warn/Wderived-cast-undefined.C
new file mode 100644
index 00000000000..ea7388514dc
--- /dev/null
+++ b/gcc/testsuite/g++.dg/warn/Wderived-cast-undefined.C
@@ -0,0 +1,33 @@
+// PR c++/96765
+// { dg-options "-Wderived-cast-undefined" }
+
+struct Derived;
+struct Base {
+ Derived *x;
+ Derived *y;
+ Base();
+ ~Base();
+};
+
+struct Derived : Base {};
+
+Base::Base()
+ : x(static_cast<Derived *>(this)), // { dg-warning "invalid 'static_cast'"
}
+ y((Derived *)this) // { dg-warning "invalid 'static_cast'" }
+{
+ static_cast<Derived *>(this); // { dg-warning "invalid 'static_cast'" }
+ (Derived *)this; // { dg-warning "invalid 'static_cast'" }
+}
+
+Base::~Base() {
+ static_cast<Derived *>(this); // { dg-warning "invalid 'static_cast'" }
+ (Derived *)this; // { dg-warning "invalid 'static_cast'" }
+}
+
+struct Other {
+ Other() {
+ Base b;
+ static_cast<Derived *>(&b);
+ (Derived *)(&b);
+ }
+};
--
2.25.1
