On 12/18/2017 05:25 PM, Martin Sebor wrote:
On 12/13/2017 02:38 PM, Jason Merrill wrote:
On Wed, Dec 13, 2017 at 12:54 PM, Martin Sebor <mse...@gmail.com> wrote:
The attached update also fixes both instances of the ICE
reported in bug 83322 and supersedes Jakub's patch for that
bug (https://gcc.gnu.org/ml/gcc-patches/2017-12/msg00765.html).
This passes bootstrap on x86_64 with no new regressions (there
are an increasing number of failures on trunk at the moment
but, AFAICS, none caused by this patch).
Jason, I'm still trying to come up with a test case for templates
that would illustrate the issue you're concerned about. If you
have one that would be great (preferably one showing a regression).
I looked at the case I was concerned about, and found that it isn't an
issue because in that case we call duplicate_decls before applying
attributes.
But it looks like we'll still get this testcase wrong, because the
code assumes that if the old decl is a single _DECL, it must match.
[[gnu::noinline]] void f() { }
[[gnu::always_inline]] void f(int) { } // OK, not the same function
I think the answer is to use Nathan's new iterators unconditionally,
probably lkp_iterator.
Thanks for the test case. You're right that this problem still
exists. I thought a complete fix for it would be simple enough
to include in this patch but after running into issues with
assumptions about how inline/noinline conflicts are resolved
I think it's best to fix the ICE alone in this patch and deal
with the pre-existing bug above in a follow up. Apparently
(according to comment #6 on pr83322) the ICE is causing some
anxiety about the timely availability of a fix, so I'd like
to avoid spending more time on it than is necessary.
Attached is an updated patch. It handles the overloads above
correctly but it doesn't fix the latent problem and so they
are still diagnosed, same as in GCC 7.
I still share Jakub's uneasiness with this approach; looking up a
redeclaration works rather differently from a normal lookup, and we
already have code for handling that. I still think that handling this
stuff by extending diagnose_mismatched_attributes is a better way to go.
That said, it's good to fix the ICE as a stopgap.
+ if (TREE_CODE (last_decl) == TREE_LIST)
+ {
+ /* The list contains a mix of symbols with the same name
+ (e.g., functions and data members defined in different
+ base classes). */
+ do
+ {
+ if (decls_match (decl, TREE_VALUE (last_decl)))
+ return TREE_VALUE (last_decl);
+
+ last_decl = TREE_CHAIN (last_decl);
+ }
+ while (last_decl);
+ }
We shouldn't need to handle TREE_LIST at all, as getting that result
should indicate that there isn't any declaration in the scope we care
about; decls from base classes will never match.
OK with this block removed.
Jason
