On Sun, Jan 21, 2024 at 2:33 PM Hans-Peter Nilsson <h...@axis.com> wrote:
>
> Tested x86_64-linux-gnu. Ok to commit?
>
> Or, does the message need more tweaking?
> (If so, suggestions from native speakers?)
> FWIW, I found no PR for just the message being bad.
>
> -- >8 --
> When you're not regularly exposed to this warning, it is
> easy to be misled by its wording, believing that there's
> something else in the function that stops it from being
> inlined, than the lack of also being *declared* inline.
>
> It's just a warning: without the inline directive, there has
> to be a secondary reasons for the function to be inlined,
> than the always_inline attribute, reasons that may be in
> effect despite the warning.
>
> Whenever the text is quoted in inline-related bugzilla
> entries, there seems to often have been an initial step of
> confusion that has to be cleared, for example in PR55830.
> The powerpc test-case in this patch where a comment is
> adjusted, seems to be another example, and I testify as the
> first-hand third "experience". The wording has been the
> same since the warning was added.
>
> Let's just tweak the wording, adding the cause, so that the
> reason for the warning is clearer. This hopefully stops the
> user from immediately asking "'Might'? Because why?" and
> then going off looking at the function body - or grepping
> the gcc source or documentation, or enter a bug-report
> subsequently closed as resolved/invalid.
>
> gcc:
> * cgraphunit.cc (process_function_and_variable_attributes): Tweak
> the warning for an attribute-always_inline without inline declaration.
>
> gcc/testsuite:
> * g++.dg/Wattributes-3.C: Adjust expected warning.
> * gcc.dg/fail_always_inline.c: Ditto.
> * gcc.target/powerpc/vec-extract-v16qiu-v2.h: Adjust
> quoted warning in comment.
> ---
> gcc/cgraphunit.cc | 3 ++-
> gcc/testsuite/g++.dg/Wattributes-3.C | 4 ++--
> gcc/testsuite/gcc.dg/fail_always_inline.c | 2 +-
> gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h | 2 +-
> 4 files changed, 6 insertions(+), 5 deletions(-)
>
> diff --git a/gcc/cgraphunit.cc b/gcc/cgraphunit.cc
> index 38052674aaa5..89dc1af522a4 100644
> --- a/gcc/cgraphunit.cc
> +++ b/gcc/cgraphunit.cc
> @@ -918,7 +918,8 @@ process_function_and_variable_attributes (cgraph_node
> *first,
> /* redefining extern inline function makes it DECL_UNINLINABLE. */
> && !DECL_UNINLINABLE (decl))
> warning_at (DECL_SOURCE_LOCATION (decl), OPT_Wattributes,
> - "%<always_inline%> function might not be inlinable");
> + "%<always_inline%> function is not always inlined"
> + " unless also declared %<inline%>");
I don't like the "is not always inlined", maybe simply reword to
"%<always_inline%> function might not be inlinable"
" unless also declared %<inline%>"
?
>
> process_common_attributes (node, decl);
> }
> diff --git a/gcc/testsuite/g++.dg/Wattributes-3.C
> b/gcc/testsuite/g++.dg/Wattributes-3.C
> index 208ec6696551..4adf0944cd4f 100644
> --- a/gcc/testsuite/g++.dg/Wattributes-3.C
> +++ b/gcc/testsuite/g++.dg/Wattributes-3.C
> @@ -26,7 +26,7 @@ B::operator char () const { return 0; }
>
> ATTR ((__noinline__))
> B::operator int () const // { dg-warning "ignoring attribute .noinline.
> because it conflicts with attribute .always_inline." }
> -// { dg-warning "function might not be inlinable" "" { target *-*-* } .-1 }
> +// { dg-warning "function is not always inlined unless also declared
> .inline." "" { target *-*-* } .-1 }
>
> {
> return 0;
> @@ -45,7 +45,7 @@ C::operator char () { return 0; }
>
> ATTR ((__noinline__))
> C::operator short () // { dg-warning "ignoring attribute
> .noinline. because it conflicts with attribute .always_inline." }
> -// { dg-warning "function might not be inlinable" "" { target *-*-* } .-1 }
> +// { dg-warning "function is not always inlined unless also declared
> .inline." "" { target *-*-* } .-1 }
> { return 0; }
>
> inline ATTR ((__noinline__))
> diff --git a/gcc/testsuite/gcc.dg/fail_always_inline.c
> b/gcc/testsuite/gcc.dg/fail_always_inline.c
> index 86645b850de8..2f48d7f5c6be 100644
> --- a/gcc/testsuite/gcc.dg/fail_always_inline.c
> +++ b/gcc/testsuite/gcc.dg/fail_always_inline.c
> @@ -2,7 +2,7 @@
> /* { dg-add-options bind_pic_locally } */
>
> extern __attribute__ ((always_inline)) void
> - bar() { } /* { dg-warning "function might not be inlinable" } */
> + bar() { } /* { dg-warning "function is not always inlined unless also
> declared .inline." } */
>
> void
> f()
> diff --git a/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h
> b/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h
> index d1157599ee7c..b9800e1c950d 100644
> --- a/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h
> +++ b/gcc/testsuite/gcc.target/powerpc/vec-extract-v16qiu-v2.h
> @@ -179,7 +179,7 @@ get_auto_15 (vector TYPE a)
> #ifdef DISABLE_INLINE_OF_GET_AUTO_N
> __attribute__ ((__noinline__))
> #else
> -/* gcc issues warning: always_inline function might not be inlinable
> +/* gcc issues warning: always_inline function is not always inlined unless
> also declared inline
>
> __attribute__ ((__always_inline__))
> */
> --
> 2.30.2
>
