On 10/11/19 6:31 PM, Jason Merrill wrote:
> On 10/10/19 2:06 PM, Bernd Edlinger wrote:
>> On 10/10/19 7:49 PM, Jason Merrill wrote:
>>> On 10/10/19 10:42 AM, Bernd Edlinger wrote:
>>>> Hi,
>>>>
>>>> this fixes a crash when -Wshadow=compatible-local is
>>>> enabled in the testcase g++.dg/parse/crash68.C
>>>
>>> Why does that flag cause this crash?
>>>
>>
>> gcc/cp/name-lookup.c:
>>
>> if (warn_shadow)
>> warning_code = OPT_Wshadow;
>> else if (warn_shadow_local)
>> warning_code = OPT_Wshadow_local;
>> else if (warn_shadow_compatible_local
>> && (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
>> || (!dependent_type_p (TREE_TYPE (decl))
>> && !dependent_type_p (TREE_TYPE (old))
>> /* If the new decl uses auto, we don't yet know
>> its type (the old type cannot be using auto
>> at this point, without also being
>> dependent). This is an indication we're
>> (now) doing the shadow checking too
>> early. */
>> && !type_uses_auto (TREE_TYPE (decl))
>> && can_convert (TREE_TYPE (old), TREE_TYPE (decl),
>> tf_none))))
>> warning_code = OPT_Wshadow_compatible_local;
>>
>> if -Wshadow=compatible-local is used, the can_convert function crashes
>> in instantiate_class_template_1.
>
> Right, checking can_convert is problematic here, as it can cause template
> instantiations that change the semantics of the program. Or, in this case,
> crash.
>
So I try to make C++ behave more consistently with the code in c-decl.c,
thus dependent on warn_shadow but not on warn_shadow_local and/or
warn_shadow_compatible_local:
if (warn_shadow)
warning_code = OPT_Wshadow;
else if (comptypes (TREE_TYPE (old_decl), TREE_TYPE (new_decl)))
warning_code = OPT_Wshadow_compatible_local;
else
warning_code = OPT_Wshadow_local;
warned = warning_at (DECL_SOURCE_LOCATION (new_decl), warning_code,
"declaration of %qD shadows a parameter",
new_decl);
I cannot remove the if (warn_shadow) since this breaks gcc.dg/pr48062.c
which uses:
#pragma GCC diagnostic ignored "-Wshadow"
to disable a -Wshadow=compatible-local warning, but while -Wno-shadow on the
command line disables also dependent warnings the pragma does not (always) do
that.
So instead I'd like to adjust the doc of -Wshadow to reflect the implementation
and remove the if(warn_shadow_local) to have C and C++ behave identical and
hopefully now in sync with the doc.
Bootstrapped and reg-tested on x86_64-pc-linux-gnu.
Is it OK for trunk?
Thanks
Bernd.
2019-10-12 Bernd Edlinger <bernd.edlin...@hotmail.de>
* doc/invoke.texi (-Wshadow, -Wshadow=global
-Wshadow=local, -Wshadow=compatible-local): Update documentation.
cp:
2019-10-12 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR c++/92024
* name-lookup.c (check_local_shadow): Shadowing TYPE_DECLs
is always a -Wshadow=compatible-local warning, unless
-Wshadow is used.
testsuite:
2019-10-12 Bernd Edlinger <bernd.edlin...@hotmail.de>
PR c++/92024
* g++.dg/parse/crash70.C: New test.
* c-c++-common/Wshadow-1.c: New test.
Index: gcc/cp/name-lookup.c
===================================================================
--- gcc/cp/name-lookup.c (revision 276893)
+++ gcc/cp/name-lookup.c (working copy)
@@ -2750,29 +2750,31 @@ check_local_shadow (tree decl)
parameter, more than often they would use the variable
thinking (mistakenly) it's still the parameter. It would be
rare that users would use the variable in the place that
- expects the parameter but thinking it's a new decl. */
+ expects the parameter but thinking it's a new decl.
+ If either object is a TYPE_DECL, '-Wshadow=compatible-local'
+ warns regardless of whether one of the types involved
+ is a subclass of the other, since that is never okay. */
enum opt_code warning_code;
if (warn_shadow)
warning_code = OPT_Wshadow;
- else if (warn_shadow_local)
- warning_code = OPT_Wshadow_local;
- else if (warn_shadow_compatible_local
- && (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
- || (!dependent_type_p (TREE_TYPE (decl))
- && !dependent_type_p (TREE_TYPE (old))
- /* If the new decl uses auto, we don't yet know
- its type (the old type cannot be using auto
- at this point, without also being
- dependent). This is an indication we're
- (now) doing the shadow checking too
- early. */
- && !type_uses_auto (TREE_TYPE (decl))
- && can_convert (TREE_TYPE (old), TREE_TYPE (decl),
- tf_none))))
+ else if (same_type_p (TREE_TYPE (old), TREE_TYPE (decl))
+ || TREE_CODE (decl) == TYPE_DECL
+ || TREE_CODE (old) == TYPE_DECL
+ || (!dependent_type_p (TREE_TYPE (decl))
+ && !dependent_type_p (TREE_TYPE (old))
+ /* If the new decl uses auto, we don't yet know
+ its type (the old type cannot be using auto
+ at this point, without also being
+ dependent). This is an indication we're
+ (now) doing the shadow checking too
+ early. */
+ && !type_uses_auto (TREE_TYPE (decl))
+ && can_convert (TREE_TYPE (old), TREE_TYPE (decl),
+ tf_none)))
warning_code = OPT_Wshadow_compatible_local;
else
- return;
+ warning_code = OPT_Wshadow_local;
const char *msg;
if (TREE_CODE (old) == PARM_DECL)
Index: gcc/doc/invoke.texi
===================================================================
--- gcc/doc/invoke.texi (revision 276893)
+++ gcc/doc/invoke.texi (working copy)
@@ -342,7 +342,7 @@ Objective-C and Objective-C++ Dialects}.
-Wno-pragmas -Wno-prio-ctor-dtor -Wredundant-decls @gol
-Wrestrict -Wno-return-local-addr @gol
-Wreturn-type -Wsequence-point -Wshadow -Wno-shadow-ivar @gol
--Wshadow=global, -Wshadow=local, -Wshadow=compatible-local @gol
+-Wshadow=global -Wshadow=local -Wshadow=compatible-local @gol
-Wshift-overflow -Wshift-overflow=@var{n} @gol
-Wshift-count-negative -Wshift-count-overflow -Wshift-negative-value @gol
-Wsign-compare -Wsign-conversion -Wfloat-conversion @gol
@@ -6308,7 +6308,7 @@ can be used to suppress such a warning.
@opindex Wno-discarded-array-qualifiers
@opindex Wdiscarded-array-qualifiers
Do not warn if type qualifiers on arrays which are pointer targets
-are being discarded. Typically, the compiler warns if a
+are being discarded. Typically, the compiler warns if a
@code{const int (*)[]} variable is passed to a function that
takes a @code{int (*)[]} parameter. This option can be used to
suppress such a warning.
@@ -6507,9 +6507,13 @@ allowed in GCC@. It is not supported by ISO C90.
@opindex Wno-shadow
Warn whenever a local variable or type declaration shadows another
variable, parameter, type, class member (in C++), or instance variable
-(in Objective-C) or whenever a built-in function is shadowed. Note
+(in Objective-C) or whenever a built-in function is shadowed. Note
that in C++, the compiler warns if a local variable shadows an
explicit typedef, but not if it shadows a struct/class/enum.
+If this warning is enabled, it includes also all instances of
+local shadowing. This means that @option{-Wno-shadow=local}
+and @option{-Wno-shadow=compatible-local} are ignored when
+@option{-Wshadow} is used.
Same as @option{-Wshadow=global}.
@item -Wno-shadow-ivar @r{(Objective-C only)}
@@ -6520,20 +6524,19 @@ Objective-C method.
@item -Wshadow=global
@opindex Wshadow=global
-The default for @option{-Wshadow}. Warns for any (global) shadowing.
-This warning is enabled by @option{-Wshadow=global}.
+Warn for any shadowing.
+Same as @option{-Wshadow}.
@item -Wshadow=local
@opindex Wshadow=local
Warn when a local variable shadows another local variable or parameter.
-This warning is enabled by @option{-Wshadow=local}.
@item -Wshadow=compatible-local
@opindex Wshadow=compatible-local
Warn when a local variable shadows another local variable or parameter
-whose type is compatible with that of the shadowing variable. In C++,
+whose type is compatible with that of the shadowing variable. In C++,
type compatibility here means the type of the shadowing variable can be
-converted to that of the shadowed variable. The creation of this flag
+converted to that of the shadowed variable. The creation of this flag
(in addition to @option{-Wshadow=local}) is based on the idea that when
a local variable shadows another one of incompatible type, it is most
likely intentional, not a bug or typo, as shown in the following example:
@@ -6552,16 +6555,15 @@ for (SomeIterator i = SomeObj.begin(); i != SomeOb
@end smallexample
Since the two variable @code{i} in the example above have incompatible types,
-enabling only @option{-Wshadow=compatible-local} will not emit a warning.
+enabling only @option{-Wshadow=compatible-local} does not emit a warning.
Because their types are incompatible, if a programmer accidentally uses one
-in place of the other, type checking will catch that and emit an error or
-warning. So not warning (about shadowing) in this case will not lead to
-undetected bugs. Use of this flag instead of @option{-Wshadow=local} can
+in place of the other, type checking is expected to catch that and emit an
+error or warning. Use of this flag instead of @option{-Wshadow=local} can
possibly reduce the number of warnings triggered by intentional shadowing.
-Note that this does also mean that shadowing @code{const char *i} by
-@code{char *i} will not emit a warning.
+Note that this also means that shadowing @code{const char *i} by
+@code{char *i} does not emit a warning.
-This warning is enabled by @option{-Wshadow=compatible-local}.
+This warning is also enabled by @option{-Wshadow=local}.
@item -Wlarger-than=@var{byte-size}
@opindex Wlarger-than=
Index: gcc/testsuite/c-c++-common/Wshadow-1.c
===================================================================
--- gcc/testsuite/c-c++-common/Wshadow-1.c (revision 0)
+++ gcc/testsuite/c-c++-common/Wshadow-1.c (working copy)
@@ -0,0 +1,24 @@
+/* { dg-do-compile } */
+/* { dg-additional-options "-Wshadow=local -Wno-shadow=compatible-local" } */
+int c;
+void foo(int *c, int *d) /* { dg-bogus "Wshadow" } */
+{
+ int *e = d;
+ {
+ int d = 0; /* { dg-warning "Wshadow=local" } */
+ }
+ {
+ int *e = 0; /* { dg-bogus "Wshadow=compatible-local" } */
+ }
+}
+#pragma GCC diagnostic warning "-Wshadow"
+void bar(int *c, int *d) /* { dg-warning "Wshadow" } */
+{
+ int *e = d;
+ {
+ int d = 0; /* { dg-warning "Wshadow" } */
+ }
+ {
+ int *e = 0; /* { dg-warning "Wshadow" } */
+ }
+}
Index: gcc/testsuite/g++.dg/parse/crash70.C
===================================================================
--- gcc/testsuite/g++.dg/parse/crash70.C (revision 0)
+++ gcc/testsuite/g++.dg/parse/crash70.C (working copy)
@@ -0,0 +1,14 @@
+// PR c++/92024
+// { dg-additional-options "-Wshadow=compatible-local" }
+
+template<typename>
+struct S {
+ S () {
+ struct c;
+ {
+ struct c {}; // { dg-warning "Wshadow=compatible-local" }
+ }
+ }
+};
+
+S<int> s;
