[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 Jakub Jelinek changed: What|Removed |Added Status|ASSIGNED|RESOLVED Resolution|--- |FIXED --- Comment #13 from Jakub Jelinek --- Fixed for 7.4+.
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 --- Comment #12 from Jakub Jelinek --- Author: jakub Date: Sat Mar 3 13:43:30 2018 New Revision: 258208 URL: https://gcc.gnu.org/viewcvs?rev=258208&root=gcc&view=rev Log: Backported from mainline 2018-03-02 Jakub Jelinek Richard Biener PR ipa/84628 * expr.c (expand_expr_real_1) : Don't emit diagnostics for error or warning attributes if CALL_FROM_THUNK_P is set. Formatting fixes. * gcc.dg/pr84628.c: New test. Added: branches/gcc-7-branch/gcc/testsuite/gcc.dg/pr84628.c Modified: branches/gcc-7-branch/gcc/ChangeLog branches/gcc-7-branch/gcc/expr.c branches/gcc-7-branch/gcc/testsuite/ChangeLog
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 --- Comment #11 from Jakub Jelinek --- Author: jakub Date: Fri Mar 2 16:19:43 2018 New Revision: 258140 URL: https://gcc.gnu.org/viewcvs?rev=258140&root=gcc&view=rev Log: PR ipa/84628 * expr.c (expand_expr_real_1) : Don't emit diagnostics for error or warning attributes if CALL_FROM_THUNK_P is set. Formatting fixes. * gcc.dg/pr84628.c: New test. Added: trunk/gcc/testsuite/gcc.dg/pr84628.c Modified: trunk/gcc/ChangeLog trunk/gcc/expr.c trunk/gcc/testsuite/ChangeLog
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 --- Comment #10 from Jakub Jelinek --- Created attachment 43533 --> https://gcc.gnu.org/bugzilla/attachment.cgi?id=43533&action=edit gcc8-pr84628.patch Untested fix.
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 --- Comment #9 from Andrew Pinski --- deprecated deprecated (msg) The deprecated attribute results in a warning if the function is used anywhere in the source file. This is useful when identifying functions that are expected to be removed in a future version of a program. The warning also includes the location of the declaration of the deprecated function, to enable users to easily find further information about why the function is deprecated, or what they should do instead. Note that the warnings only occurs for uses: int old_fn () __attribute__ ((deprecated)); int old_fn (); int (*fn_ptr)() = old_fn; results in a warning on line 3 but not line 2. The optional msg argument, which must be a string, is printed in the warning if present. The deprecated attribute can also be used for variables and types (see Variable Attributes, see Type Attributes.)
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 --- Comment #8 from Jay --- Aha, kinda the same thing, but before or after analysis. And this “deprecated” somewhat matches msvc - I was wondering about that but didn’t see it. It’d be nice to be able to customize the deprecated message but hopefully we’ll keep the baby despite the bathwater. (Msvc added that option later.) Thank you.
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628
--- Comment #7 from Jakub Jelinek ---
The warning/error attributes have been added for purposes like glibc memset
inline, which does:
if (__builtin_constant_p (__len) && __len == 0
&& (!__builtin_constant_p (__ch) || __ch != 0))
{
__warn_memset_zero_len ();
return __dest;
}
where __warn_memset_zero_len has the warning attribute. This only works if it
is after optimization. If you want a diagnostics whenever you reference some
function, you are looking for the deprecated attribute instead.
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 --- Comment #6 from Jay --- Misplaced comment: But, the thing is, because optimization can remove the use of such functions, people are now advocating that we noinline along with the attribute, which hypothetically is unwarranted damage. Noinline being a partial disabling of optimization, not complete. See here: https://github.com/mono/mono/pull/7353 But it isn't clear as I said, if it is worth warning/error for what the source states, vs. what the optimizer divines from the code.
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 --- Comment #5 from Jay --- I know. We just noticed and were surprised. It isn't clear if this is what users would expect or not. Warn because they wrote code that "merely looks bad", or only if the compiler decides after analysis that it really is bad? Anyway, there are two points here I think. One is far more arguable than the other. I don't think the small case shown should error.
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 --- Comment #4 from Andrew Pinski --- (In reply to Jay from comment #3) > The original case said something about "localalias" in the error, so aliases > don't seem to address it. I can dig that up probably. > > Shouldn't it warn for: > if (0) >banned_function() > ? > > I believe we want it to. > You know, strive for equal warnings/errors w/ or w/o optimization, but I > know that is impossible in general, because optimization==analysis. The documentation is explicit here though: " call to such a function is not eliminated through dead code elimination or other optimizations"
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 --- Comment #3 from Jay --- The original case said something about "localalias" in the error, so aliases don't seem to address it. I can dig that up probably. Shouldn't it warn for: if (0) banned_function() ? I believe we want it to. You know, strive for equal warnings/errors w/ or w/o optimization, but I know that is impossible in general, because optimization==analysis. I believe attribute(error) should be like pragma poison except: - easier to implement the deprecated function - can provide custom error - works with C++ overloads -- attach attribute to full declaration, not just identifier But yeah, another option is to optimize a little less, on a case by case basis. Or actually remove the attribute, or don't warn, if the compiler essentially introduced the use.
[Bug ipa/84628] attribute(warning/error) functions should not be merged together
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=84628 Jakub Jelinek changed: What|Removed |Added Status|UNCONFIRMED |ASSIGNED Last reconfirmed||2018-03-01 CC||jakub at gcc dot gnu.org Assignee|unassigned at gcc dot gnu.org |jakub at gcc dot gnu.org Ever confirmed|0 |1 --- Comment #2 from Jakub Jelinek --- Of course it has to be evaluated after optimization, that is the whole point of these attributes. In this case just IPA ICF should not try to fold error1 with error2 if it can't use aliases and the functions involved have warning or error attributes.
