Re: [C++PATCH] [PR86379] do not use TREE_TYPE for USING_DECL_SCOPE

[Alexandre Oliva <aoliva at redhat dot com>]

On Feb  5, 2019, Jason Merrill <jason@redhat.com> wrote:

> On Tue, Feb 5, 2019 at 1:37 AM Alexandre Oliva <aoliva@redhat.com> wrote:
>> On Jan 31, 2019, Jason Merrill <jason@redhat.com> wrote:
>> 
>> > Let's use strip_using_decl instead
>> 
>> Aah, nice!  Thanks, I'll make the changes, test them, and post a new patch.
>> 
>> 
>> >> @@ -13288,7 +13295,8 @@ grok_special_member_properties (tree decl)
>> >> {
>> >> tree class_type;
>> >> -  if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
>> >> +  if (TREE_CODE (decl) != USING_DECL
>> >> +      && !DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
>> >> return;
>> 
>> > Is there a reason not to use it here, as well?
>> 
>> The using decl will take us to a member of a different class, and this
>> function takes the DECL_CONTEXT of decl and adjusts the properties of
>> that class.  If we followed USING_DECLs in decl that early, we'd adjust
>> (again?) the member properties of USING_DECL_SCOPE(original using decl),
>> rather than of DECL_CONTEXT (original using decl) as intended.

> But a little further in, copy_fn_p will also check
> DECL_NONSTATIC_MEMBER_FUNCTION_P and crash.

It would crash if I hadn't adjusted it to test for USING_DECLs, yes.

> This function used to do nothing for USING_DECL (since its TREE_TYPE
> wasn't METHOD_TYPE), right?  It should continue to do nothing.

I thought I was fixing bugs making certain functions follow USING_DECLs
rather than fail to do what it should, because the test on TREE_TYPE
happened to not pass.  This one I was not so sure about, for the reasons
above.  I guess it makes sense to return early if it really shouldn't do
anything for a USING_DECL, even if it happens to resolve to a method
that it would otherwise handle if declared directly in the class.

> Come to think of it, how about fixing DECL_NONSTATIC_MEMBER_FUNCTION_P
> to be false for non-functions rather than messing with all these
> places that use it?

I considered that at first, but it seemed to me that this would not
bring about the desired behavior in the first functions I touched
(lookup_member vs shared_member), and I thought we'd be better off
retaining some means to catch incorrect uses of this and other macros on
USING_DECLs, so that we can analyze and fix the uses instead of getting
accidental behavior, correct or not.


Here's what I'm testing now.


[PR86379] do not use TREE_TYPE for USING_DECL_SCOPE

From: Alexandre Oliva <aoliva@redhta.com>

It's too risk to reuse the type field for USING_DECL_SCOPE.
Language-independent parts of the compiler, such as location and
non-lvalue wrappers, happily take the TREE_TYPE of a USING_DECL as if
it was a type rather than an unrelated scope.

For better or worse, USING_DECLs use the non-common struct so we can
use the otherwise unused result field.  Adjust fallout, from uses of
TREE_TYPE that were supposed to be USING_DECL_SCOPE, to other
accidental uses of TREE_TYPE of a USING_DECL.


for  gcc/cp/ChangeLog

	PR c++/86379
	* cp-tree.h (USING_DECL_SCOPE): Use result rather than type.
	* name-lookup.c (strip_using_decl): Use USING_DECL_SCOPE.
	* search.c (protected_accessible_p): Follow USING_DECL_DECLS.
	(shared_member_p): Likewise.
	(lookup_member): Likewise.
	* decl.c (grok_special_member_properties): Skip USING_DECLs.
	* semantics.c (finish_omp_declare_simd_methods): Likewise.

for  gcc/testsuite/ChangeLog

	PR c++/86379
	* g++.dg/cpp0x/pr86379.C: New.
---
 gcc/cp/cp-tree.h                     |    2 
 gcc/cp/decl.c                        |    3 
 gcc/cp/name-lookup.c                 |    2 
 gcc/cp/search.c                      |   17 ++-
 gcc/cp/semantics.c                   |    3 
 gcc/testsuite/g++.dg/cpp0x/pr86379.C |  207 ++++++++++++++++++++++++++++++++++
 6 files changed, 227 insertions(+), 7 deletions(-)
 create mode 100644 gcc/testsuite/g++.dg/cpp0x/pr86379.C

diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index dada3a6aa410..44a3620a539f 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -3293,7 +3293,7 @@ struct GTY(()) lang_decl {
 #define DECL_DEPENDENT_P(NODE) DECL_LANG_FLAG_0 (USING_DECL_CHECK (NODE))
 
 /* The scope named in a using decl.  */
-#define USING_DECL_SCOPE(NODE) TREE_TYPE (USING_DECL_CHECK (NODE))
+#define USING_DECL_SCOPE(NODE) DECL_RESULT_FLD (USING_DECL_CHECK (NODE))
 
 /* The decls named by a using decl.  */
 #define USING_DECL_DECLS(NODE) DECL_INITIAL (USING_DECL_CHECK (NODE))
diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c
index 65ba812deb67..373b79b844dc 100644
--- a/gcc/cp/decl.c
+++ b/gcc/cp/decl.c
@@ -13297,7 +13297,8 @@ grok_special_member_properties (tree decl)
 {
   tree class_type;
 
-  if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
+  if (TREE_CODE (decl) == USING_DECL
+      || !DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
     return;
 
   class_type = DECL_CONTEXT (decl);
diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c
index d7b9029b0a3a..959f43b02384 100644
--- a/gcc/cp/name-lookup.c
+++ b/gcc/cp/name-lookup.c
@@ -2100,7 +2100,7 @@ strip_using_decl (tree decl)
 	     
 	 using typename :: [opt] nested-name-specifier unqualified-id ;
       */
-      decl = make_typename_type (TREE_TYPE (decl),
+      decl = make_typename_type (USING_DECL_SCOPE (decl),
 				 DECL_NAME (decl),
 				 typename_type, tf_error);
       if (decl != error_mark_node)
diff --git a/gcc/cp/search.c b/gcc/cp/search.c
index 0367e4952138..5b00a63615ed 100644
--- a/gcc/cp/search.c
+++ b/gcc/cp/search.c
@@ -623,6 +623,11 @@ protected_accessible_p (tree decl, tree derived, tree type, tree otype)
   if (!DERIVED_FROM_P (type, derived))
     return 0;
 
+  /* DECL_NONSTATIC_MEMBER_P won't work for USING_DECLs.  */
+  decl = strip_using_decl (decl);
+  if (TREE_CODE (decl) == USING_DECL)
+    return 1;
+
   /* [class.protected]
 
      When a friend or a member function of a derived class references
@@ -928,8 +933,12 @@ shared_member_p (tree t)
   if (is_overloaded_fn (t))
     {
       for (ovl_iterator iter (get_fns (t)); iter; ++iter)
-	if (DECL_NONSTATIC_MEMBER_FUNCTION_P (*iter))
-	  return 0;
+	{
+	  tree decl = strip_using_decl (*iter);
+	  if (TREE_CODE (decl) != USING_DECL
+	      && DECL_NONSTATIC_MEMBER_FUNCTION_P (decl))
+	    return 0;
+	}
       return 1;
     }
   return 0;
@@ -1177,7 +1186,9 @@ lookup_member (tree xbasetype, tree name, int protect, bool want_type,
       && !really_overloaded_fn (rval))
     {
       tree decl = is_overloaded_fn (rval) ? get_first_fn (rval) : rval;
-      if (!DECL_NONSTATIC_MEMBER_FUNCTION_P (decl)
+      decl = strip_using_decl (decl);
+      if (TREE_CODE (decl) != USING_DECL
+	  && !DECL_NONSTATIC_MEMBER_FUNCTION_P (decl)
 	  && !perform_or_defer_access_check (basetype_path, decl, decl,
 					     complain, afi))
 	rval = error_mark_node;
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 786f18ab0c8b..2009a10b4e85 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -5867,7 +5867,8 @@ finish_omp_declare_simd_methods (tree t)
 
   for (tree x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x))
     {
-      if (TREE_CODE (TREE_TYPE (x)) != METHOD_TYPE)
+      if (TREE_CODE (x) == USING_DECL
+	  || !DECL_NONSTATIC_MEMBER_FUNCTION_P (x))
 	continue;
       tree ods = lookup_attribute ("omp declare simd", DECL_ATTRIBUTES (x));
       if (!ods || !TREE_VALUE (ods))
diff --git a/gcc/testsuite/g++.dg/cpp0x/pr86379.C b/gcc/testsuite/g++.dg/cpp0x/pr86379.C
new file mode 100644
index 000000000000..82282eae8e52
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/pr86379.C
@@ -0,0 +1,207 @@
+// { dg-do compile { target c++11 } }
+
+// Reduced from Mozilla SpiderMonkey, licensed under MPL-2.0.
+
+template<typename T, unsigned N>
+class Vector
+{
+  public:
+    Vector() {}
+    unsigned length() const { return 0; }
+};
+
+class TokenStreamShared
+{
+};
+
+template<typename CharT, class AnyCharsAccess>
+class TokenStreamSpecific;
+
+class TokenStreamAnyChars
+  : public TokenStreamShared
+{
+  public:
+    TokenStreamAnyChars() {}
+};
+
+template<typename CharT>
+class SourceUnits
+{
+  public:
+    SourceUnits() {}
+
+    bool atEnd() const { return true; }
+    unsigned offset() const { return 0; }
+    bool matchCodeUnit(CharT c) { return true; }
+};
+
+class TokenStreamCharsShared
+{
+    using CharBuffer = Vector<char16_t, 32>;
+
+  protected:
+    CharBuffer charBuffer;
+
+  protected:
+    explicit TokenStreamCharsShared() {}
+};
+
+template<typename CharT>
+class TokenStreamCharsBase
+  : public TokenStreamCharsShared
+{
+  public:
+    TokenStreamCharsBase()
+      : TokenStreamCharsShared(), sourceUnits()
+    {}
+
+    using SourceUnits = ::SourceUnits<CharT>;
+
+    bool matchCodeUnit(int expect) { return true; }
+
+  protected:
+    SourceUnits sourceUnits;
+};
+
+template<typename CharT, class AnyCharsAccess>
+class GeneralTokenStreamChars
+  : public TokenStreamCharsBase<CharT>
+{
+    using CharsBase = TokenStreamCharsBase<CharT>;
+
+  protected:
+    using CharsBase::CharsBase;
+
+    TokenStreamAnyChars& anyCharsAccess();
+    const TokenStreamAnyChars& anyCharsAccess() const;
+};
+
+template<typename CharT, class AnyCharsAccess> class TokenStreamChars;
+
+template<class AnyCharsAccess>
+class TokenStreamChars<char16_t, AnyCharsAccess>
+  : public GeneralTokenStreamChars<char16_t, AnyCharsAccess>
+{
+  private:
+    using CharsBase = TokenStreamCharsBase<char16_t>;
+    using GeneralCharsBase = GeneralTokenStreamChars<char16_t, AnyCharsAccess>;
+    using Self = TokenStreamChars<char16_t, AnyCharsAccess>;
+
+  protected:
+    using GeneralCharsBase::anyCharsAccess;
+    using CharsBase::sourceUnits;
+
+    using typename GeneralCharsBase::SourceUnits;
+
+  protected:
+    using GeneralCharsBase::GeneralCharsBase;
+
+    bool getFullAsciiCodePoint(int lead, int* codePoint) {
+        if (lead == '\r') {
+            bool isAtEnd = sourceUnits.atEnd();
+            if (!isAtEnd)
+                sourceUnits.matchCodeUnit('\n');
+        } else if (lead != '\n') {
+            *codePoint = lead;
+            return true;
+        }
+
+        *codePoint = '\n';
+        return true;
+    }
+};
+
+template<typename CharT, class AnyCharsAccess>
+class TokenStreamSpecific
+  : public TokenStreamChars<CharT, AnyCharsAccess>,
+    public TokenStreamShared
+{
+  public:
+    using CharsBase = TokenStreamCharsBase<CharT>;
+    using GeneralCharsBase = GeneralTokenStreamChars<CharT, AnyCharsAccess>;
+    using SpecializedCharsBase = TokenStreamChars<CharT, AnyCharsAccess>;
+
+  public:
+    using GeneralCharsBase::anyCharsAccess;
+
+  private:
+    using typename CharsBase::SourceUnits;
+
+  private:
+    using TokenStreamCharsShared::charBuffer;
+    using CharsBase::sourceUnits;
+
+  public:
+    TokenStreamSpecific()
+      : SpecializedCharsBase()
+    {}
+
+  public:
+    bool advance(unsigned position) {
+        bool t = charBuffer.length() + 1 > 0;
+        auto offs = sourceUnits.offset();
+        return t && offs > 0;
+    }
+};
+
+class TokenStreamAnyCharsAccess
+{
+};
+
+class TokenStream final
+  : public TokenStreamAnyChars,
+    public TokenStreamSpecific<char16_t, TokenStreamAnyCharsAccess>
+{
+    using CharT = char16_t;
+
+  public:
+    TokenStream()
+    : TokenStreamAnyChars(),
+      TokenStreamSpecific<CharT, TokenStreamAnyCharsAccess>()
+    {}
+};
+
+class SyntaxParseHandler {};
+
+class ParserBase
+{
+  public:
+    TokenStreamAnyChars anyChars;
+};
+
+template<class ParseHandler, typename CharT> class GeneralParser;
+
+template <class ParseHandler>
+class PerHandlerParser : public ParserBase
+{
+};
+
+template<class Parser>
+class ParserAnyCharsAccess
+{
+};
+
+template <class ParseHandler, typename CharT>
+class Parser;
+
+template <class ParseHandler, typename CharT>
+class GeneralParser
+  : public PerHandlerParser<ParseHandler>
+{
+  public:
+    TokenStreamSpecific<CharT, ParserAnyCharsAccess<GeneralParser>> tokenStream;
+
+  public:
+    GeneralParser();
+};
+
+
+template class TokenStreamCharsBase<char16_t>;
+
+template class TokenStreamChars<char16_t, TokenStreamAnyCharsAccess>;
+
+template class
+TokenStreamChars<char16_t, ParserAnyCharsAccess<GeneralParser<SyntaxParseHandler, char16_t>>>;
+
+template class
+TokenStreamSpecific<char16_t, ParserAnyCharsAccess<GeneralParser<SyntaxParseHandler, char16_t>>>;


-- 
Alexandre Oliva, freedom fighter   https://FSFLA.org/blogs/lxo
Be the change, be Free!         FSF Latin America board member
GNU Toolchain Engineer                Free Software Evangelist
Hay que enGNUrecerse, pero sin perder la terGNUra jamás-GNUChe