| Message ID | orlg31skh4.fsf@lxoliva.fsfla.org |
|---|---|
| State | New |
| Headers | show |
| Series | [C++PATCH,PR86379] do not use TREE_TYPE for USING_DECL_SCOPE | expand |
On 1/30/19 8:07 PM, Alexandre Oliva wrote: > 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. > > Regstrapped on x86_64- and i686-linux-gnu. Ok to install? > (but see the additional patchlet below) > > > 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 (copy_fn_p): Likewise. > (grok_special_member_properties): Do not test USING_DECL for > staticness. > * 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 | 10 +- > gcc/cp/name-lookup.c | 2 > gcc/cp/search.c | 23 +++- > gcc/cp/semantics.c | 3 > gcc/testsuite/g++.dg/cpp0x/pr86379.C | 207 ++++++++++++++++++++++++++++++++++ > 6 files changed, 240 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 77e1425b4357b..053ed5ace6d42 100644 > --- a/gcc/cp/cp-tree.h > +++ b/gcc/cp/cp-tree.h > @@ -3288,7 +3288,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 79eeac177b64c..86101d3bc3b45 100644 > --- a/gcc/cp/decl.c > +++ b/gcc/cp/decl.c > @@ -13174,6 +13174,13 @@ copy_fn_p (const_tree d) > tree arg_type; > int result = 1; > > + while (TREE_CODE (d) == USING_DECL) > + { > + d = USING_DECL_DECLS (d); > + if (!d) > + return result; > + } Let's use strip_using_decl instead of writing the loop here and in the other similar places. > @@ -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? Jason
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.
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. This function used to do nothing for USING_DECL (since its TREE_TYPE wasn't METHOD_TYPE), right? It should continue to do nothing. 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? Jason
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>>>;
On 2/5/19 8:58 PM, Alexandre Oliva wrote: > 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. OK, that makes sense; it isn't always clear what the right handling of a USING_DECL is. In grok_special_member_properties we want to ignore them (as you do) because the handling there only applies to immediate members, and any inherited properties will be handled when processing the bases. In protected_accessible_p and shared_member_p, if we're left with a USING_DECL after strip_using_decl, we can't give a meaningful answer, and should probably abort; we shouldn't get here with a dependent expression. The other two changes look fine. Jason
On Feb 7, 2019, Jason Merrill <jason@redhat.com> wrote: > OK, that makes sense; it isn't always clear what the right handling of > a USING_DECL is. Indeed. Like, in shared_member_p, I'm wondering if we shouldn't recurse if the USING_DECL maps to an overload, that IIUC might contain static and non-static functions, other using decls, and maybe even types. > In protected_accessible_p and shared_member_p, if we're left with a > USING_DECL after strip_using_decl, we can't give a meaningful answer, > and should probably abort; we shouldn't get here with a dependent > expression. Ah, that's good knowledge to have. I've added asserts and comments. Here's what I'm testing. [PR86379] do not use TREE_TYPE for USING_DECL_SCOPE From: Alexandre Oliva <aoliva@redhat.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 | 19 +++ gcc/cp/semantics.c | 3 gcc/testsuite/g++.dg/cpp0x/pr86379.C | 207 ++++++++++++++++++++++++++++++++++ 6 files changed, 229 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..4c3fffda717c 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); + /* We don't expect or support dependent decls. */ + gcc_assert (TREE_CODE (decl) != USING_DECL); + /* [class.protected] When a friend or a member function of a derived class references @@ -928,8 +933,13 @@ 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); + /* We don't expect or support dependent decls. */ + gcc_assert (TREE_CODE (decl) != USING_DECL); + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl)) + return 0; + } return 1; } return 0; @@ -1177,7 +1187,10 @@ 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); + /* A dependent USING_DECL will be checked after tsubsting. */ + 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>>>;
On Feb 7, 2019, Jason Merrill <jason@redhat.com> wrote: > In protected_accessible_p and shared_member_p, if we're left with a > USING_DECL after strip_using_decl, we can't give a meaningful answer, > and should probably abort; we shouldn't get here with a dependent > expression. In g++.dg/lookup/using39.C, shared_member_p is called by cp_parser_primary_expression -> finish_id_expression -> finish_id_expression_1 -> finish_qualified_id_expr when parsing 'using A<T>::f' in B<T>. That was the only regression in the patch I posted earlier today. I'm going back to the previous version of shared_member_p.
On 2/8/19 4:07 AM, Alexandre Oliva wrote: > On Feb 7, 2019, Jason Merrill <jason@redhat.com> wrote: > >> In protected_accessible_p and shared_member_p, if we're left with a >> USING_DECL after strip_using_decl, we can't give a meaningful answer, >> and should probably abort; we shouldn't get here with a dependent >> expression. > > In g++.dg/lookup/using39.C, shared_member_p is called by > cp_parser_primary_expression -> finish_id_expression -> > finish_id_expression_1 -> finish_qualified_id_expr when parsing 'using > A<T>::f' in B<T>. Then I think finish_qualified_id_expr should check type_dependent_expression_p first. Jason
On Feb 8, 2019, Jason Merrill <jason@redhat.com> wrote: > On 2/8/19 4:07 AM, Alexandre Oliva wrote: >> On Feb 7, 2019, Jason Merrill <jason@redhat.com> wrote: >> >>> In protected_accessible_p and shared_member_p, if we're left with a >>> USING_DECL after strip_using_decl, we can't give a meaningful answer, >>> and should probably abort; we shouldn't get here with a dependent >>> expression. >> >> In g++.dg/lookup/using39.C, shared_member_p is called by >> cp_parser_primary_expression -> finish_id_expression -> >> finish_id_expression_1 -> finish_qualified_id_expr when parsing 'using >> A<T>::f' in B<T>. > Then I think finish_qualified_id_expr should check > type_dependent_expression_p first. That works, as long as we take dependent as non-shared, which, no surprise, is what the previous code would accidentally do. Doing otherwise regresses cpp1y/lambda-generic-this1a.C, and I have test results that prove it ;-) (I wonder if that won't malfunction if the name ends up resolving to a static member, or even to a type... Shall I give that a try myself, or do you know we have tests in place for that already?) Anyway, here's the patch that passed regstrapping without regressions on x86_64- and i686-linux-gnu. Ok to install? [PR86379] do not use TREE_TYPE for USING_DECL_SCOPE From: Alexandre Oliva <aoliva@redhat.com> It's too risky 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. (finish_qualified_id_expr): Do not call shared_member_p with a dependent expr. 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 | 19 +++ gcc/cp/semantics.c | 6 + gcc/testsuite/g++.dg/cpp0x/pr86379.C | 207 ++++++++++++++++++++++++++++++++++ 6 files changed, 231 insertions(+), 8 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..4c3fffda717c 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); + /* We don't expect or support dependent decls. */ + gcc_assert (TREE_CODE (decl) != USING_DECL); + /* [class.protected] When a friend or a member function of a derived class references @@ -928,8 +933,13 @@ 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); + /* We don't expect or support dependent decls. */ + gcc_assert (TREE_CODE (decl) != USING_DECL); + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (decl)) + return 0; + } return 1; } return 0; @@ -1177,7 +1187,10 @@ 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); + /* A dependent USING_DECL will be checked after tsubsting. */ + 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..89ea438ebeef 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -2096,7 +2096,8 @@ finish_qualified_id_expr (tree qualifying_class, { /* See if any of the functions are non-static members. */ /* If so, the expression may be relative to 'this'. */ - if (!shared_member_p (expr) + if ((type_dependent_expression_p (expr) + || !shared_member_p (expr)) && current_class_ptr && DERIVED_FROM_P (qualifying_class, current_nonlambda_class_type ())) @@ -5867,7 +5868,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>>>;
On 2/13/19 12:36 PM, Alexandre Oliva wrote: > On Feb 8, 2019, Jason Merrill <jason@redhat.com> wrote: > >> On 2/8/19 4:07 AM, Alexandre Oliva wrote: >>> On Feb 7, 2019, Jason Merrill <jason@redhat.com> wrote: >>> >>>> In protected_accessible_p and shared_member_p, if we're left with a >>>> USING_DECL after strip_using_decl, we can't give a meaningful answer, >>>> and should probably abort; we shouldn't get here with a dependent >>>> expression. >>> >>> In g++.dg/lookup/using39.C, shared_member_p is called by >>> cp_parser_primary_expression -> finish_id_expression -> >>> finish_id_expression_1 -> finish_qualified_id_expr when parsing 'using >>> A<T>::f' in B<T>. > >> Then I think finish_qualified_id_expr should check >> type_dependent_expression_p first. > > That works, as long as we take dependent as non-shared, which, no > surprise, is what the previous code would accidentally do. Doing > otherwise regresses cpp1y/lambda-generic-this1a.C, and I have test > results that prove it ;-) > > (I wonder if that won't malfunction if the name ends up resolving to a > static member, or even to a type... Shall I give that a try myself, or > do you know we have tests in place for that already?) > > > Anyway, here's the patch that passed regstrapping without regressions on > x86_64- and i686-linux-gnu. Ok to install? OK. Jason
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 77e1425b4357b..053ed5ace6d42 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -3288,7 +3288,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 79eeac177b64c..86101d3bc3b45 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -13174,6 +13174,13 @@ copy_fn_p (const_tree d) tree arg_type; int result = 1; + while (TREE_CODE (d) == USING_DECL) + { + d = USING_DECL_DECLS (d); + if (!d) + return result; + } + gcc_assert (DECL_FUNCTION_MEMBER_P (d)); if (TREE_CODE (d) == TEMPLATE_DECL @@ -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; class_type = DECL_CONTEXT (decl); diff --git a/gcc/cp/name-lookup.c b/gcc/cp/name-lookup.c index d7b9029b0a3a5..959f43b023844 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 0367e49521380..4e0a9b722ea64 100644 --- a/gcc/cp/search.c +++ b/gcc/cp/search.c @@ -623,6 +623,14 @@ 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. */ + while (TREE_CODE (decl) == USING_DECL) + { + decl = USING_DECL_DECLS (decl); + if (!decl) + break; + } + /* [class.protected] When a friend or a member function of a derived class references @@ -634,7 +642,7 @@ protected_accessible_p (tree decl, tree derived, tree type, tree otype) derived from that class) (_expr.ref_). If the access is to form a pointer to member, the nested-name-specifier shall name the derived class (or any class derived from that class). */ - if (DECL_NONSTATIC_MEMBER_P (decl) + if (decl && DECL_NONSTATIC_MEMBER_P (decl) && !DERIVED_FROM_P (derived, otype)) return 0; @@ -928,7 +936,10 @@ 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)) + if (TREE_CODE (*iter) != USING_DECL + ? DECL_NONSTATIC_MEMBER_FUNCTION_P (*iter) + : (USING_DECL_DECLS (*iter) + && !shared_member_p (USING_DECL_DECLS (*iter)))) return 0; return 1; } @@ -1177,7 +1188,13 @@ 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) + while (TREE_CODE (decl) == USING_DECL) + { + decl = USING_DECL_DECLS (decl); + if (!decl) + break; + } + if (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 786f18ab0c8b5..2009a10b4e85c 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 0000000000000..82282eae8e52b --- /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>>>; Now, here's an experiment I started and haven't completed yet. It's an incremental patch based on the one above, that still falls short of preventing access to TREE_TYPE of USING_DECLs, but that catches accesses of USING_DECLs in predicates intended for member FUNCTION_DECL (or TEMPLATE_DECL), and that were possibly misbehaving with the reuse of USING_DECL's TREE_TYPE to hold the target scope. It bootstraps successfully, but there are some two dozens of testsuite regressions. Is this worth pursuing? [PR86379] check for function_decl in memfn-testing predicates --- gcc/cp/cp-tree.h | 20 +++++++++++++++----- gcc/cp/search.c | 5 ++++- gcc/cp/semantics.c | 2 +- gcc/cp/typeck.c | 3 ++- 4 files changed, 22 insertions(+), 8 deletions(-) diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 053ed5ace6d42..371398d85bde9 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -568,6 +568,9 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX]; #define VAR_OR_FUNCTION_DECL_CHECK(NODE) \ TREE_CHECK2(NODE,VAR_DECL,FUNCTION_DECL) +#define FUNCTION_OR_TEMPLATE_DECL_CHECK(NODE) \ + TREE_CHECK2(NODE,FUNCTION_DECL,TEMPLATE_DECL) + #define TYPE_FUNCTION_OR_TEMPLATE_DECL_CHECK(NODE) \ TREE_CHECK3(NODE,TYPE_DECL,TEMPLATE_DECL,FUNCTION_DECL) @@ -3004,15 +3007,21 @@ struct GTY(()) lang_decl { #define DECL_BEFRIENDING_CLASSES(NODE) \ (LANG_DECL_FN_CHECK (NODE)->befriending_classes) +#define FUNCTION_OR_TEMPLATE_DECL_P(NODE) \ + (TREE_CODE (NODE) == FUNCTION_DECL \ + || TREE_CODE (NODE) == TEMPLATE_DECL) + /* Nonzero for FUNCTION_DECL means that this decl is a static member function. */ #define DECL_STATIC_FUNCTION_P(NODE) \ - (LANG_DECL_FN_CHECK (NODE)->static_function) + (LANG_DECL_FN_CHECK (FUNCTION_OR_TEMPLATE_DECL_CHECK (NODE)) \ + ->static_function) /* Nonzero for FUNCTION_DECL means that this decl is a non-static member function. */ #define DECL_NONSTATIC_MEMBER_FUNCTION_P(NODE) \ - (TREE_CODE (TREE_TYPE (NODE)) == METHOD_TYPE) + (TREE_CODE (TREE_TYPE (FUNCTION_OR_TEMPLATE_DECL_CHECK (NODE))) \ + == METHOD_TYPE) /* Nonzero for FUNCTION_DECL means that this decl is a member function (static or non-static). */ @@ -3034,9 +3043,10 @@ struct GTY(()) lang_decl { (TYPE_ARG_TYPES (TREE_TYPE (NODE)))))) /* Nonzero for a DECL means that this member is a non-static member. */ -#define DECL_NONSTATIC_MEMBER_P(NODE) \ - (DECL_NONSTATIC_MEMBER_FUNCTION_P (NODE) \ - || TREE_CODE (NODE) == FIELD_DECL) +#define DECL_NONSTATIC_MEMBER_P(NODE) \ + (TREE_CODE (NODE) == FIELD_DECL \ + || (FUNCTION_OR_TEMPLATE_DECL_P (NODE) \ + && DECL_NONSTATIC_MEMBER_FUNCTION_P (NODE))) /* Nonzero for _DECL means that this member object type is mutable. */ diff --git a/gcc/cp/search.c b/gcc/cp/search.c index 4e0a9b722ea64..f6518d52126b1 100644 --- a/gcc/cp/search.c +++ b/gcc/cp/search.c @@ -1193,8 +1193,11 @@ lookup_member (tree xbasetype, tree name, int protect, bool want_type, decl = USING_DECL_DECLS (decl); if (!decl) break; + if (is_overloaded_fn (decl)) + decl = get_first_fn (decl); } - if (decl && !DECL_NONSTATIC_MEMBER_FUNCTION_P (decl) + if (decl && (!FUNCTION_OR_TEMPLATE_DECL_P (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 2009a10b4e85c..ea147b428136b 100644 --- a/gcc/cp/semantics.c +++ b/gcc/cp/semantics.c @@ -5867,7 +5867,7 @@ finish_omp_declare_simd_methods (tree t) for (tree x = TYPE_FIELDS (t); x; x = DECL_CHAIN (x)) { - if (TREE_CODE (x) == USING_DECL + if (!FUNCTION_OR_TEMPLATE_DECL_P (x) || !DECL_NONSTATIC_MEMBER_FUNCTION_P (x)) continue; tree ods = lookup_attribute ("omp declare simd", DECL_ATTRIBUTES (x)); diff --git a/gcc/cp/typeck.c b/gcc/cp/typeck.c index ec722a360357d..b132d847a1c09 100644 --- a/gcc/cp/typeck.c +++ b/gcc/cp/typeck.c @@ -1887,7 +1887,8 @@ invalid_nonstatic_memfn_p (location_t loc, tree expr, tsubst_flags_t complain) return false; if (is_overloaded_fn (expr) && !really_overloaded_fn (expr)) expr = get_first_fn (expr); - if (DECL_NONSTATIC_MEMBER_FUNCTION_P (expr)) + if (FUNCTION_OR_TEMPLATE_DECL_P (expr) + && DECL_NONSTATIC_MEMBER_FUNCTION_P (expr)) { if (complain & tf_error) {