On 1/11/19 2:27 PM, Joseph Myers wrote:
On Fri, 11 Jan 2019, Martin Sebor wrote:
gcc/testsuite/ChangeLog:
PR c/88718
* gcc.dg/inline-40.c: New test.
* gcc.dg/inline-41.c: New test.
We already have tests inline-40.c and inline-41.c; these need to be
renumbered accordingly.
Done.
+ if (add)
+ {
+ if (csi->function == func
+ || csi->function)
+ continue;
Since func is non-NULL, this is equivalent to "if (csi->function)".
Yes. Done.
Don't you want to break, not continue, in the case where csi->function (as
all the tentative entries come first)? As-is, this looks like it would
have quadratic cost if a file has many inline functions each referencing
some file-scope static. (The "Avoid adding another tentative record for
this DECL if one already exists." also looks quadratic, but having very
many functions in a file seems more likely than very many references to
statics in a single declaration that is not a definition.)
Yes, good catch!
@@ -5165,6 +5259,11 @@ finish_decl (tree decl, location_t init_loc, tree
set_user_assembler_name (decl, asmspec);
}
+ /* Remove any tentative record of a non-static inline function
+ referencing a static decl made a DECL that is not a definition. */
+ if (TREE_CODE (decl) == FUNCTION_DECL)
+ update_tentative_inline_static (decl);
+
You also need to do the update when the declaration wasn't a function
declaration at all. The present patch produces spurious warnings for:
static int i;
int a[sizeof i];
inline void f (void) {}
because the reference to i in the declaration of a gets treated as if it
were in the definition of f.
Done.
Also, you need to distinguish the case of updating for a function
definition, and updating for a declaration that's not a definition, for a
function that was previously defined. E.g.
inline void f (int a[sizeof (int)]) {}
static int i;
inline void f (int a[sizeof i]);
must not diagnose a reference to i in f (you have such tests with the
declaration before the definition, but none that I can see with the
declaration after the definition). I think this means the caller of
update_tentative_inline_static should specify whether to add or remove,
rather than update_tentative_inline_static trying to deduce it from
properties of a DECL (that don't say whether the particular declaration in
question is a definition or not, just whether a definition has been seen).
Also true. Done.
Attached is a revision with these fixes.
Thanks for the careful review!
Martin
PR c/88718 - Strange inconsistency between old style and new style definitions of inline functions.
gcc/c/ChangeLog:
PR c/88718
* c-decl.c (update_tentative_inline_static): New function.
(check_inline_statics): Handle tentative records for inline
declarations without definitions.
Print static declaration location.
(finish_decl): Add tentative records of references to statics.
(finish_function): Same.
* c-typeck.c (build_external_ref): Handle all references to statics.
gcc/testsuite/ChangeLog:
PR c/88718
* gcc.dg/inline-42.c: New test.
* gcc.dg/inline-43.c: New test.
diff --git a/gcc/c/c-decl.c b/gcc/c/c-decl.c
index 57049f80073..ddc09abb669 100644
--- a/gcc/c/c-decl.c
+++ b/gcc/c/c-decl.c
@@ -826,9 +826,11 @@ c_finish_incomplete_decl (tree decl)
}
}
-/* Record that inline function FUNC contains a reference (location
- LOC) to static DECL (file-scope or function-local according to
- TYPE). */
+/* Record that inline function FUNC either does contain or may contain
+ a reference (location LOC) to static DECL (file-scope or function-local
+ according to TYPE). For a null FUNC, a tentative record is created that
+ reflects a reference in the function signature and that is either updated
+ or removed when the function declaration is complete. */
void
record_inline_static (location_t loc, tree func, tree decl,
@@ -843,6 +845,51 @@ record_inline_static (location_t loc, tree func, tree decl,
c_inline_statics = csi;
}
+/* Update tentative records of references to static declarations in
+ an inline declaration function DECL, or remove them if DECL isn't
+ a function declared inline or when ADD is false. A tentative
+ record is one whose FUNCTION is null. */
+
+static void
+update_tentative_inline_static (tree decl, bool add)
+{
+ gcc_assert (decl);
+
+ if (!c_inline_statics)
+ return;
+
+ /* True to associate DECL with the tentative records, false to remove
+ them. */
+ add = (add
+ && TREE_CODE (decl) == FUNCTION_DECL
+ && DECL_DECLARED_INLINE_P (decl)
+ && DECL_EXTERNAL (decl)
+ && DECL_INITIAL (decl));
+
+ /* Iterate over tentative records (all those at the head of the list
+ with a null FUNCTION) and either associate them with DECL when ADD
+ is set or remove them from it otherwise. */
+ for (c_inline_static *csi = c_inline_statics, *last = csi;
+ csi; csi = csi->next)
+ {
+ if (csi->function)
+ break;
+
+ if (add)
+ {
+ if (csi->static_decl)
+ csi->function = decl;
+ }
+ else
+ {
+ if (last == c_inline_statics)
+ c_inline_statics = last = csi->next;
+ else
+ last->next = csi->next;
+ }
+ }
+}
+
/* Check for references to static declarations in inline functions at
the end of the translation unit and diagnose them if the functions
are still inline definitions. */
@@ -853,22 +900,41 @@ check_inline_statics (void)
struct c_inline_static *csi;
for (csi = c_inline_statics; csi; csi = csi->next)
{
- if (DECL_EXTERNAL (csi->function))
- switch (csi->type)
- {
- case csi_internal:
- pedwarn (csi->location, 0,
- "%qD is static but used in inline function %qD "
- "which is not static", csi->static_decl, csi->function);
- break;
- case csi_modifiable:
- pedwarn (csi->location, 0,
- "%q+D is static but declared in inline function %qD "
- "which is not static", csi->static_decl, csi->function);
- break;
- default:
- gcc_unreachable ();
- }
+ /* FUNCTION is unset for a declaration whose signature references
+ a static variable and for which a definition wasn't provided. */
+ if (!csi->function)
+ continue;
+
+ /* Only extern functions are of interest. STATIC_DECL is null
+ for inline functions that have been defined that contain
+ no references to statics (but whose subsequent declarations
+ might). */
+ if (!DECL_EXTERNAL (csi->function)
+ || !csi->static_decl)
+ continue;
+
+ bool warned;
+ switch (csi->type)
+ {
+ case csi_internal:
+ warned = pedwarn (csi->location, 0,
+ "%qD is static but used in inline function %qD "
+ "which is not static",
+ csi->static_decl, csi->function);
+ break;
+ case csi_modifiable:
+ warned = pedwarn (csi->location, 0,
+ "%q+D is static but declared in inline function %qD "
+ "which is not static",
+ csi->static_decl, csi->function);
+ break;
+ default:
+ gcc_unreachable ();
+ }
+
+ if (warned)
+ inform (DECL_SOURCE_LOCATION (csi->static_decl),
+ "%qD declared here", csi->static_decl);
}
c_inline_statics = NULL;
}
@@ -5166,6 +5232,11 @@ finish_decl (tree decl, location_t init_loc, tree init,
set_user_assembler_name (decl, asmspec);
}
+ /* Remove any tentative records of references to static declarations
+ made in the declaration of DECL, which can be a fuction or some
+ other entity. */
+ update_tentative_inline_static (decl, false);
+
if (DECL_FILE_SCOPE_P (decl))
{
if (DECL_INITIAL (decl) == NULL_TREE
@@ -9637,6 +9708,11 @@ finish_function (void)
}
}
+ /* Associate any tentative records of references to static variables
+ with this function declaration if it is inline and not static, or
+ remove them otherwise. */
+ update_tentative_inline_static (fndecl, true);
+
if (!decl_function_context (fndecl))
undef_nested_function = false;
diff --git a/gcc/c/c-typeck.c b/gcc/c/c-typeck.c
index 63d177f7a6f..f05519d3896 100644
--- a/gcc/c/c-typeck.c
+++ b/gcc/c/c-typeck.c
@@ -52,6 +52,8 @@ along with GCC; see the file COPYING3. If not see
#include "stringpool.h"
#include "attribs.h"
#include "asan.h"
+#include "c-family/c-pragma.h"
+#include "c-parser.h"
/* Possible cases of implicit bad conversions. Used to select
diagnostic messages in convert_for_assignment. */
@@ -2818,19 +2820,26 @@ build_external_ref (location_t loc, tree id, bool fun, tree *type)
if (context != NULL_TREE && context != current_function_decl)
DECL_NONLOCAL (ref) = 1;
}
- /* C99 6.7.4p3: An inline definition of a function with external
- linkage ... shall not contain a reference to an identifier with
- internal linkage. */
- else if (current_function_decl != NULL_TREE
+ else if (VAR_OR_FUNCTION_DECL_P (ref)
+ && (!VAR_P (ref) || TREE_STATIC (ref))
+ && ! TREE_PUBLIC (ref))
+ {
+ /* C99 6.7.4p3: An inline definition of a function with external
+ linkage ... shall not contain a reference to an identifier with
+ internal linkage. */
+ if ((current_function_decl
&& DECL_DECLARED_INLINE_P (current_function_decl)
&& DECL_EXTERNAL (current_function_decl)
- && VAR_OR_FUNCTION_DECL_P (ref)
- && (!VAR_P (ref) || TREE_STATIC (ref))
- && ! TREE_PUBLIC (ref)
&& DECL_CONTEXT (ref) != current_function_decl)
- record_inline_static (loc, current_function_decl, ref,
- csi_internal);
-
+ /* If the function declaration is not available yet at this
+ point make a record of the reference if it isn't associated
+ with any context yet and either fill in the details or
+ discard the record when the declaration has been completed. */
+ || (!current_function_decl
+ && !DECL_CONTEXT (ref)))
+ record_inline_static (loc, current_function_decl, ref,
+ csi_internal);
+ }
return ref;
}
diff --git a/gcc/testsuite/gcc.dg/inline-42.c b/gcc/testsuite/gcc.dg/inline-42.c
new file mode 100644
index 00000000000..1e61d820895
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/inline-42.c
@@ -0,0 +1,156 @@
+/* PR c/88718 - Strange inconsistency between old style and new style
+ definitions of inline functions
+ { dg-do compile }
+ { dg-options "-Wall -Wno-unused" } */
+
+extern int global;
+extern const int cst_global;
+
+static int local;
+static const int cst_local;
+
+/* Verify that inline declarations that aren't definitions are accepted
+ without a warning even if their argument lists reference static
+ variables. */
+
+inline void global_decl_arg_ref_global (int (*)[sizeof global]);
+static inline void local_decl_arg_ref_global (int (*)[sizeof global]);
+
+inline void global_decl_arg_ref_cst_global (int (*)[sizeof cst_global]);
+static inline void local_decl_arg_ref_cst_global (int (*)[sizeof cst_global]);
+
+inline void global_decl_arg_ref_local (int (*)[sizeof local]);
+static inline void local_decl_arg_ref_local (int (*)[sizeof local]);
+
+inline void global_decl_arg_ref_cst_local (int (*)[sizeof cst_local]);
+static inline void local_decl_arg_ref_cst_local (int (*)[sizeof cst_local]);
+
+/* Verify that implicitly extern inline definitions trigger a warning
+ when their argument lists reference static (but not extern) variables. */
+
+inline void
+global_decl_arg_ref_global (int (*p)[sizeof global]) { (void)&p; }
+
+inline void
+global_decl_arg_ref_cst_global (int (*p)[sizeof cst_global]) { (void)&p; }
+
+inline void
+global_decl_arg_ref_local (int (*p)[sizeof local])
+/* { dg-warning ".local. is static but used in inline function .global_decl_arg_ref_local." "" { target *-*-* } .-1 } */
+{
+ (void)&p;
+}
+
+inline void
+global_decl_arg_ref_cst_local (int (*p)[sizeof cst_local])
+/* { dg-warning ".cst_local. is static but used in inline function .global_decl_arg_ref_cst_local." "" { target *-*-* } .-1 } */
+{
+ (void)&p;
+}
+
+/* Verify that static inline definitions don't trigger a warning
+ when their argument lists reference static or extern variables. */
+
+static inline void
+local_decl_arg_ref_global (int (*p)[sizeof global]) { (void)&p; }
+static inline void
+local_decl_arg_ref_cst_global (int (*p)[sizeof cst_global]) { (void)&p; }
+static inline void
+local_decl_arg_ref_local (int (*p)[sizeof local]) { (void)&p; }
+static inline void
+local_decl_arg_ref_cst_local (int (*p)[sizeof cst_local]) { (void)&p; }
+
+
+/* Verify that a function declaration that references a static that
+ follows a definition of the same function that doesn't reference
+ any variables is not diagnosed. */
+
+inline void
+global_def_arg_ref_none (int (*p)[sizeof (int)]) { (void)&p; }
+
+static int prior_local;
+
+inline void
+global_def_arg_ref_none (int (*p)[sizeof prior_local]);
+
+
+/* Verify that a function declaration that references a static that
+ follows a definition of the same function that refernces a global
+ is not diagnosed. */
+
+inline void
+global_def_arg_ref_cst_global (int (*p)[sizeof cst_global]) { (void)&p; }
+
+inline void
+global_def_arg_ref_cst_global (int (*p)[sizeof cst_local]);
+
+
+/* Same as above but with return types. */
+
+extern void abort (void);
+
+/* Verify that inline declarations that aren't definitions are accepted
+ without a warning even if their return types reference static
+ variables. */
+
+inline struct { int a[sizeof global]; }
+ global_decl_ret_ref_global (void);
+
+static inline struct { int a[sizeof global]; }
+ local_decl_ret_ref_global (void);
+
+inline struct { int a[sizeof cst_global]; }
+ global_decl_ret_ref_cst_global (void);
+
+static inline struct { int a[sizeof cst_global]; }
+ local_decl_ret_ref_cst_global (void);
+
+inline struct { int a[sizeof local]; }
+ global_decl_ret_ref_local (void);
+static inline struct { int a[sizeof local]; }
+ local_decl_ret_ref_local (void);
+
+inline struct { int a[sizeof cst_local]; }
+ global_decl_ret_ref_cst_local (void);
+
+static inline struct { int a[sizeof global]; }
+ local_decl_ret_ref_cst_local (void);
+
+
+/* Verify that implicitly extern inline definitions trigger a warning
+ when their return types reference static (but not extern) variables. */
+
+inline struct { int a[sizeof global]; }
+ global_def_ret_ref_global (void) { abort (); }
+
+inline struct { int a[sizeof cst_global]; }
+ global_def_ret_ref_cst_global (void) { abort (); }
+
+inline struct { int a[sizeof local]; }
+ global_def_ret_ref_local (void)
+ /* { dg-warning ".local. is static but used in inline function .global_def_ret_ref_local." "" { target *-*-* } .-2 } */
+ {
+ abort ();
+ }
+
+inline struct { int a[sizeof cst_local]; }
+ global_def_ret_ref_cst_local (void)
+ /* { dg-warning ".cst_local. is static but used in inline function .global_def_ret_ref_cst_local." "" { target *-*-* } .-2 } */
+ {
+ abort ();
+ }
+
+/* Verify that static inline definitions don't trigger a warning
+ when their return types reference static or extern variables. */
+
+static inline struct { int a[sizeof global]; }
+ local_def_ret_ref_global (void) { abort (); }
+static inline struct { int a[sizeof cst_global]; }
+ local_def_ret_ref_cst_global (void) { abort (); }
+static inline struct { int a[sizeof local]; }
+ local_def_ret_ref_local (void) { abort (); }
+static inline struct { int a[sizeof cst_local]; }
+ local_def_ret_ref_cst_local (void) { abort (); }
+
+
+/* { dg-prune-output "declared but never defined" } */
diff --git a/gcc/testsuite/gcc.dg/inline-43.c b/gcc/testsuite/gcc.dg/inline-43.c
new file mode 100644
index 00000000000..47709959fcf
--- /dev/null
+++ b/gcc/testsuite/gcc.dg/inline-43.c
@@ -0,0 +1,59 @@
+/* PR c/88718 - Strange inconsistency between old style and new style
+ definitions of inline functions
+ Verify that warnings for references to multiple static variables in
+ inline function signatures are all diagnosed and that the locations
+ of the diagnostics, including the notes, are correct.
+ { dg-do compile }
+ { dg-options "-Wall -Wno-unused" } */
+
+extern const int e1;
+extern const int e2;
+
+static const int s1 = 1; /* { dg-message ".s1. declared here" } */
+static const int s2 = 2; /* { dg-message ".s2. declared here" } */
+
+/* Verify that variable declarations don't cause bogus warnings
+ in subsequent inline function declarations. */
+int extarr[sizeof e1][sizeof e2][sizeof s1][sizeof s2];
+static int locarr[sizeof e1][sizeof e2][sizeof s1][sizeof s2];
+inline void fv (void) { }
+
+inline void fes (int (*)[sizeof e1][sizeof s2]);
+
+
+inline void
+fes (int (*p)
+ [sizeof e1]
+ [sizeof s2]) /* { dg-warning ".s2. is static but used in inline function .fes. which is not static" } */
+{ }
+
+
+inline void fse (int (*)[sizeof s1][sizeof e2]);
+
+inline void
+fse (int (*p)
+ [sizeof s1] /* { dg-warning ".s1. is static but used in inline function .fse. which is not static" } */
+ [sizeof e2])
+{ }
+
+
+static int s3 = 3; /* { dg-message ".s3. declared here" } */
+static int s4 = 4; /* { dg-message ".s4. declared here" } */
+
+/* Use s1 and s2 here and verify that the warnings mention s3 and s4
+ referenced in the definition. */
+inline void fss (int (*)[sizeof s1][sizeof s2]);
+
+inline void
+fss (int (*p)
+ [sizeof s3] /* { dg-warning ".s3. is static but used in inline function .fss. which is not static" } */
+ [sizeof s4]) /* { dg-warning ".s4. is static but used in inline function .fss. which is not static" } */
+{ }
+
+
+/* Use s1 and s2 in the declaration and verify that no warnings are
+ emitted for the definition that doesn't reference any statics. */
+inline void fee (int (*)[sizeof s1][sizeof s2]);
+
+inline void
+fee (int (*p)[sizeof e1][sizeof e2]) { }
