On 3/17/22 01:51, Zhao Wei Liew wrote:
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.
+ if (TREE_CODE (expr) == NOP_EXPR
+ && resolves_to_fixed_type_p (TREE_OPERAND (expr, 0)) == -1)
Why not just pass expr itself to resolves_to_fixed_type_p?
We might as well also warn if it returns >0, e.g.
struct A { } a;
struct B: A { };
auto p = static_cast<B*>(&a); // undefined
You should also handle reference casts.
Jason
