On 08/09/2017 11:13 AM, Joseph Myers wrote:
On Wed, 9 Aug 2017, Martin Sebor wrote:
diff --git a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
index 146b76c..58a4742 100644
--- a/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
+++ b/gcc/testsuite/gcc.dg/tree-ssa/ssa-ccp-2.c
@@ -113,7 +113,7 @@ int test9 (int *intarr)
int test99 (int *intarr)
{
- extern int foo9 (int) __attribute__ ((pure));
+ extern int foo9 (int) __attribute__ ((const));
int h, v;
g9 = 9;
h = foo9 (g9);
And why this? I'd avoid modifying existing tests like that unless
it's directly related to the new diagnostic.
The same function is declared earlier on in the file with attribute
const and the redeclaration with attribute pure triggers a warning
due to the two being considered mutually exclusive.
const + pure is a *redundant* combination, but I think it should always
have the meaning of const regardless of the order in which they appear or
whether they appear on separate declarations. That's different from
combinations that are actually nonsensical.
To a large degree I agree with this characterization. I think
of one as a subset of the other. FWIW, my initial approach was
to introduce the concept of attribute subsets (as in pure being
a subset of the restrictions of const). But in discussing it
with Marek we felt it was too complicated and that mutual
exclusivity was good enough here. I'd be fine with reintroducing
the subset/superset distinction, or any other that makes sense
and helps find potential bugs that might result from redeclaring
the same function with the other of this pair attributes. I'd
also be okay with not diagnosing this combination if I could
convince myself that it's safe (or can be made safe) and treated
consistently.
It's not clear to me that
const + pure should be diagnosed by default any more than declaring the
same function twice, with the const attribute both time, should be
diagnosed.
The problem whose subset the diagnostic detects is declaring
the function const and calling it before the pure declaration
that corresponds to its definition is seen. I.e., the inverse
of what the ssa-ccp-2.c test does. I think a mismatch between
the attributes is as suspect as a mismatch in the function's
signature.
In any event, it sounds to me that conceptually, it might be
useful to be able to specify which of a pair of mutually
exclusive (or redundant) attributes to retain and/or accept:
the first or the second, and not just whether to accept or
drop the most recent one. That will make it possible to make
the most appropriate decision about each specific pair of
attributes without impacting any of the others.
If this sounds right to you (and others) let me make that
change and post an updated patch.
But If I misunderstood and this solution wouldn't help please
let me know.
Martin
PS I've also wondered if -Wattributes is the right option to use
for these sorts of conflicts/redundancies. It is the only option
used now but the manual makes it sound that -Wignored-attributes
should be expected. AFAICS, -Wignored-attributes is only used by
the C++ front end for templates. Should it be extended to these
cases as well?
