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.
Martin
PR c++/83394 - always_inline vs. noinline no longer diagnosed
PR c++/83322 - ICE: tree check: expected class âtypeâ, have âexceptionalâ
gcc/cp/ChangeLog:
PR c++/83394
PR c++/83322
* decl2.c (cplus_decl_attributes): Look up member functions
in the scope of their class.
gcc/testsuite/ChangeLog:
PR c++/83394
* g++.dg/Wattributes-3.C: New test.
* g++.dg/Wattributes-4.C: New test.
* g++.dg/Wattributes-5.C: New test.
Index: gcc/cp/decl2.c
===================================================================
--- gcc/cp/decl2.c (revision 255779)
+++ gcc/cp/decl2.c (working copy)
@@ -1432,6 +1432,67 @@ cp_omp_mappable_type (tree type)
return true;
}
+/* Return the last pushed declaration for the symbol DECL or NULL
+ when no such declaration exists. */
+
+static tree
+find_last_decl (tree decl)
+{
+ tree last_decl = NULL_TREE;
+
+ if (tree name = DECL_P (decl) ? DECL_NAME (decl) : NULL_TREE)
+ {
+ /* Look up the declaration in its scope. */
+ tree pushed_scope = NULL_TREE;
+ if (tree ctype = DECL_CONTEXT (decl))
+ pushed_scope = push_scope (ctype);
+
+ last_decl = lookup_name (name);
+
+ if (pushed_scope)
+ pop_scope (pushed_scope);
+
+ /* The declaration may be a member conversion operator
+ or a bunch of overfloads (handle the latter below). */
+ if (last_decl && BASELINK_P (last_decl))
+ last_decl = BASELINK_FUNCTIONS (last_decl);
+ }
+
+ if (!last_decl)
+ return NULL_TREE;
+
+ if (DECL_P (last_decl) || TREE_CODE (last_decl) == OVERLOAD)
+ {
+ /* A set of overloads of the same function. */
+ for (lkp_iterator iter (last_decl); iter; ++iter)
+ {
+ if (TREE_CODE (*iter) == OVERLOAD)
+ continue;
+
+ if (decls_match (decl, *iter, /*record_decls=*/false))
+ return *iter;
+ }
+ return NULL_TREE;
+ }
+
+ 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);
+ }
+
+ return NULL_TREE;
+}
+
/* Like decl_attributes, but handle C++ complexity. */
void
@@ -1483,28 +1544,7 @@ cplus_decl_attributes (tree *decl, tree attributes
}
else
{
- tree last_decl = (DECL_P (*decl) && DECL_NAME (*decl)
- ? lookup_name (DECL_NAME (*decl)) : NULL_TREE);
-
- if (last_decl && TREE_CODE (last_decl) == OVERLOAD)
- for (ovl_iterator iter (last_decl, true); ; ++iter)
- {
- if (!iter)
- {
- last_decl = NULL_TREE;
- break;
- }
-
- if (TREE_CODE (*iter) == OVERLOAD)
- continue;
-
- if (decls_match (*decl, *iter, /*record_decls=*/false))
- {
- last_decl = *iter;
- break;
- }
- }
-
+ tree last_decl = find_last_decl (*decl);
decl_attributes (decl, attributes, flags, last_decl);
}
Index: gcc/testsuite/g++.dg/Wattributes-3.C
===================================================================
--- gcc/testsuite/g++.dg/Wattributes-3.C (nonexistent)
+++ gcc/testsuite/g++.dg/Wattributes-3.C (working copy)
@@ -0,0 +1,90 @@
+// PR c++/83394 - always_inline vs. noinline no longer diagnosed
+// { dg-do compile }
+// { dg-options "-Wattributes" }
+
+#define ATTR(list) __attribute__ (list)
+
+struct A
+{
+ ATTR ((__noinline__)) operator int ();
+};
+
+ATTR ((__always_inline__))
+A::operator int () // { dg-warning "ignoring attribute .always_inline. because it conflicts with attribute .noinline." }
+{
+ return 0;
+}
+
+
+struct B
+{
+ operator char () const;
+ ATTR ((__always_inline__)) operator int () const;
+};
+
+B::operator char () const { return 0; }
+
+ATTR ((__noinline__))
+B::operator int () const // { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." }
+{
+ return 0;
+}
+
+
+struct C
+{
+ operator char ();
+ ATTR ((__always_inline__)) operator short ();
+ operator int ();
+ ATTR ((__noinline__)) operator long ();
+};
+
+C::operator char () { return 0; }
+
+ATTR ((__noinline__))
+C::operator short () // { dg-warning "ignoring attribute .noinline. because it conflicts with attribute .always_inline." }
+{ return 0; }
+
+inline ATTR ((__noinline__))
+C::operator int ()
+{ return 0; }
+
+
+ATTR ((__always_inline__))
+C::operator long () // { dg-warning "ignoring attribute .always_inline. because it conflicts with attribute .noinline." }
+{ return 0; }
+
+
+struct D
+{
+ int foo ();
+ int foo (int);
+ int ATTR ((const)) foo (int, int);
+ int ATTR ((pure)) foo (int, int, int);
+
+ int ATTR ((const)) foo (int, int, int, int);
+
+ int foo (int, int, int, int, int);
+};
+
+int ATTR ((const))
+D::foo ()
+{ return 0; }
+
+int ATTR ((pure))
+D::foo (int)
+{ return 0; }
+
+int ATTR ((pure))
+D::foo (int, int) // { dg-warning "ignoring attribute .pure. because it conflicts with attribute .const." }
+{ return 0; }
+
+int ATTR ((const))
+D::foo (int, int, int) // { dg-warning "ignoring attribute .const. because it conflicts with attribute .pure." }
+{ return 0; }
+
+int
+D::foo (int, int, int, int) { return 0; }
+
+int ATTR ((const))
+D::foo (int, int, int, int, int) { return 0; }
Index: gcc/testsuite/g++.dg/Wattributes-4.C
===================================================================
--- gcc/testsuite/g++.dg/Wattributes-4.C (nonexistent)
+++ gcc/testsuite/g++.dg/Wattributes-4.C (working copy)
@@ -0,0 +1,29 @@
+// PR c++/83322 - ICE: tree check: expected class âtypeâ, have âexceptionalâ
+// (baselink) in diag_attr_exclusions, at attribs.c:393
+// { dg-do compile }
+// { dg-options "-Wattributes" }
+
+#define ATTR(list) __attribute__ (list)
+
+// Test case from comment #0.
+struct A0
+{
+ template <class T> operator T();
+ ATTR ((always_inline)) operator int();
+};
+
+// Test case from comment #4.
+struct A1
+{
+ void foo();
+};
+
+struct B
+{
+ bool foo;
+};
+
+struct C: A1, B
+{
+ ATTR ((warn_unused_result)) int foo ();
+};
Index: gcc/testsuite/g++.dg/Wattributes-5.C
===================================================================
--- gcc/testsuite/g++.dg/Wattributes-5.C (nonexistent)
+++ gcc/testsuite/g++.dg/Wattributes-5.C (working copy)
@@ -0,0 +1,34 @@
+// { dg-do compile }
+// { dg-options "-Wattributes" }
+
+#define ATTR(list) __attribute__ (list)
+
+template <int>
+struct A
+{
+ int __attribute__ ((noinline))
+ f (); // { dg-message "previous declaration here" }
+};
+
+template <int N>
+int __attribute__ ((always_inline))
+A<N>::f () // { dg-warning "ignoring attribute .always_inline. because it conflicts with attribute .noinline." } */
+{ return 0; }
+
+
+template <int>
+struct B
+{
+ int __attribute__ ((always_inline))
+ f ();
+};
+
+template <>
+inline int __attribute__ ((always_inline))
+B<0>::f ()
+{ return 0; }
+
+template <>
+int __attribute__ ((noinline))
+B<1>::f ()
+{ return 1; }
