On Fri, May 07, 2021 at 06:09:35PM +0200, David Lamparter wrote:
> The TYPE_MAIN_VARIANT() here was, for casts to a typedef'd type name,
> resulting in all information about the typedef's involvement getting
> lost. This drops necessary information for warnings and can make them
> confusing or even misleading. It also makes specialized warnings for
> unspecified-size system types (pid_t, uid_t, ...) impossible.
Any comments on this? Anything I can do to move this forward? Or is it
not suitable to be picked up?
Cheers,
-David
> [...]
>
> diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
> index fdc7bb6125c2..ba6014726a4b 100644
> --- a/gcc/c/c-typeck.c
> +++ b/gcc/c/c-typeck.c
> @@ -5876,6 +5876,7 @@ c_safe_function_type_cast_p (tree t1, tree t2)
> tree
> build_c_cast (location_t loc, tree type, tree expr)
> {
> + tree res_type, walk_type;
> tree value;
>
> bool int_operands = EXPR_INT_CONST_OPERANDS (expr);
> @@ -5896,7 +5897,39 @@ build_c_cast (location_t loc, tree type, tree expr)
> if (objc_is_object_ptr (type) && objc_is_object_ptr (TREE_TYPE (expr)))
> return build1 (NOP_EXPR, type, expr);
>
> + /* Want to keep typedef information, but at the same time we need to strip
> + qualifiers for a proper rvalue. Unfortunately, we don't know if any
> + qualifiers on a typedef are part of the typedef or were locally
> supplied.
> + So grab the original typedef and use that only if it has no qualifiers.
> + cf. cast-typedef testcase */
> +
> + res_type = NULL;
> +
> + for (walk_type = type;
> + walk_type && TYPE_NAME (walk_type)
> + && TREE_CODE (TYPE_NAME (walk_type)) == TYPE_DECL;
> + walk_type = DECL_ORIGINAL_TYPE (TYPE_NAME (walk_type)))
> + {
> + tree walk_unqual, orig_type, orig_decl;
> +
> + walk_unqual = build_qualified_type (walk_type, TYPE_UNQUALIFIED);
> +
> + orig_decl = lookup_name (TYPE_IDENTIFIER (walk_type));
> + if (!orig_decl || TREE_CODE (orig_decl) != TYPE_DECL)
> + continue;
> + orig_type = TREE_TYPE (orig_decl);
> +
> + if (walk_unqual == orig_type)
> + {
> + res_type = walk_unqual;
> + break;
> + }
> + }
> +
> type = TYPE_MAIN_VARIANT (type);
> + if (!res_type)
> + res_type = type;
> + gcc_assert (TYPE_MAIN_VARIANT (res_type) == type);
>
> if (TREE_CODE (type) == ARRAY_TYPE)
> {
> @@ -5924,7 +5957,7 @@ build_c_cast (location_t loc, tree type, tree expr)
> "ISO C forbids casting nonscalar to the same type");
>
> /* Convert to remove any qualifiers from VALUE's type. */
> - value = convert (type, value);
> + value = convert (res_type, value);
> }
> else if (TREE_CODE (type) == UNION_TYPE)
> {
> @@ -6078,7 +6111,7 @@ build_c_cast (location_t loc, tree type, tree expr)
> " from %qT to %qT", otype, type);
>
> ovalue = value;
> - value = convert (type, value);
> + value = convert (res_type, value);
>
> /* Ignore any integer overflow caused by the cast. */
> if (TREE_CODE (value) == INTEGER_CST && !FLOAT_TYPE_P (otype))
> @@ -6114,7 +6147,7 @@ build_c_cast (location_t loc, tree type, tree expr)
> && INTEGRAL_TYPE_P (TREE_TYPE (expr)))
> || TREE_CODE (expr) == REAL_CST
> || TREE_CODE (expr) == COMPLEX_CST)))
> - value = build1 (NOP_EXPR, type, value);
> + value = build1 (NOP_EXPR, res_type, value);
>
> /* If the expression has integer operands and so can occur in an
> unevaluated part of an integer constant expression, ensure the
> diff --git a/gcc/testsuite/gcc.dg/cast-typedef.c
> b/gcc/testsuite/gcc.dg/cast-typedef.c
> new file mode 100644
> index 000000000000..3058e5a0b190
> --- /dev/null
> +++ b/gcc/testsuite/gcc.dg/cast-typedef.c
> @@ -0,0 +1,35 @@
> +/* Test cast <> typedef interactions */
> +/* Origin: David Lamparter <[email protected]> */
> +/* { dg-do compile } */
> +/* { dg-options "-Wconversion" } */
> +
> +typedef int typedefname;
> +typedef volatile int qual1;
> +typedef volatile typedefname qual2;
> +
> +extern int val;
> +extern void f2(unsigned char arg);
> +
> +void
> +f (void)
> +{
> + /* -Wconversion just used to print out the actual type */
> +
> + f2 ((typedefname) val); /* { dg-warning "typedefname" } */
> + f2 ((volatile typedefname) val); /* { dg-warning "typedefname" } */
> + f2 ((qual1) val); /* { dg-warning "int" } */
> + f2 ((qual2) val); /* { dg-warning "typedefname" } */
> +
> + /* { dg-bogus "volatile" "qualifiers should be stripped" { target {
> "*-*-*" } } 19 } */
> + /* { dg-bogus "volatile" "qualifiers should be stripped" { target {
> "*-*-*" } } 20 } */
> + /* { dg-bogus "volatile" "qualifiers should be stripped" { target {
> "*-*-*" } } 21 } */
> +
> + /* { dg-bogus "qual1" "typedef with qualifier should not be used" { target
> { "*-*-*" } } 20 } */
> + /* { dg-bogus "qual2" "typedef with qualifier should not be used" { target
> { "*-*-*" } } 21 } */
> +
> + /* shadow "typedefname" & make sure it's not used */
> + typedef short typedefname;
> + f2 ((qual2) val); /* { dg-warning "int" } */
> +
> + /* { dg-bogus "typedefname" "retaining a shadowed typedef would be
> confusing" { target { "*-*-*" } } 32 } */
> +}
