This is the mail archive of the gcc-patches@gcc.gnu.org mailing list for the GCC project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: C++ PATCH for c++/79817 - attribute deprecated on namespace


On 8/23/19 11:55 AM, Marek Polacek wrote:
On Thu, Aug 22, 2019 at 11:49:49AM -0700, Jason Merrill wrote:
On Thu, Aug 22, 2019 at 11:01 AM Nathan Sidwell <nathan@acm.org> wrote:

On 8/20/19 9:03 PM, Marek Polacek wrote:

and in cp_parser_nested_name_specifier_opt we simply don't know if we're
dealing with a function decl.  Calling cp_warn_deprecated_use_scopes from
cp_parser_type_specifier resulted int duplicated diagnostics so that one
is out too.  So I did the following which doesn't seem too bad.


diff --git gcc/cp/decl.c gcc/cp/decl.c
index 08b7baa40e0..46ad0271f7b 100644
--- gcc/cp/decl.c
+++ gcc/cp/decl.c
@@ -10791,6 +10791,7 @@ grokdeclarator (const cp_declarator *declarator,
     cp_warn_deprecated_use (type);
     if (type && TREE_CODE (type) == TYPE_DECL)
       {
+      cp_warn_deprecated_use_scopes (DECL_CONTEXT (type));

CP_DECL_CONTEXT would be clearer, here and elsewhere.

Oh right, I failed to adjust that.  Fixed now.

     /* Do warn about using typedefs to a deprecated class.  */
diff --git gcc/cp/decl2.c gcc/cp/decl2.c
index a32108f9d16..d6f407d7aef 100644
--- gcc/cp/decl2.c
+++ gcc/cp/decl2.c
@@ -5407,6 +5407,23 @@ cp_warn_deprecated_use (tree decl, tsubst_flags_t complain)
     return warned;
   }

+/* Like above, but takes into account outer scopes.  */
+
+void
+cp_warn_deprecated_use_scopes (tree ns)

Do we need to walk non-namespace scopes here?  can we just bail if NS is
not a namespace?  if can legitimately not be a namespace, calling it NS
is confusing :)

It seems like it can be a class in some cases, and I would want to
warn about deprecated classes named in a nested-name-specifier.  Did
we not already warn about that?

I know of at least this case:

namespace  N {
   enum [[deprecated]] E { X };
}

int i = N::E::X;

we didn't warn about the deprecated 'E' but with this patch we do.

I agree that the parameter name is confusing.

True, I didn't realize.  Fixed now.

+{
+  while (ns
+      && ns != error_mark_node
+      && ns != global_namespace)
+    {
+      cp_warn_deprecated_use (ns);
+      if (TYPE_P (ns))
... and does this ever trigger?

Yeah, it can, see the testcase above.

+     ns = CP_TYPE_CONTEXT (ns);
+      else
+     ns = CP_DECL_CONTEXT (ns);
+    }
+}

I always worry about such recursive lookups.  NAMESPACE_DECL has so many
spare flags, could we take one to say 'is, or contained in, deprecated',
and thus know whether we can bail early.  And stop at the first
deprecated one -- though not sure why someone would deprecate more than
one namespace in a nest.  thoughts?

Well, this patch uses the TREE_DEPRECATED bit for a NAMESPACE_DECL.  I'm not
exactly sure how another bit would help us, where I would set it (I guess the
innermost scope, but that doesn't have to be a NAMESPACE_DECL), and how that
would play with diagnostics -- i.e. if we'd print the correct name.

I can imagine deprecating an inner namespace and later deprecating an
outer namespace, but I don't think it's important to warn about more
than one in that case.

Agreed, and to that effect I tweaked cp_warn_deprecated_use_scopes to return
if cp_warn_deprecated_use issued a warning.

Thank you both!

Bootstrapped/regtested on x86_64-linux, ok for trunk?

OK on Monday if Nathan doesn't have more comments before then.

Jason


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]