Re: [C++ PATCH] Hide __for_{range,begin,end} symbols (PR c++/85515)
On 07/03/2018 08:15 AM, Jakub Jelinek wrote: And it isn't limited to that, omp_priv/omp_orig/omp_in/omp_out too (the OpenMP UDR artifical vars), also variables captured in lambdas, anon union fields, ... So I'm afraid it is not possible to ignore DECL_ARTIFICIAL VAR_DECLs in name lookup and generally it is ok that they have user accessible names. Just the range for vars are a special case. Yeah, I'd rather not have DECL_ARTIFICIAL be significant in name lookup. An unspellable name would be better, which is what we do for, eg, ctors. nathan -- Nathan Sidwell
Re: Invert sense of NO_IMPLICIT_EXTERN_C
could a global reviewer comment? This touches a lot of target-specific config files. David has kindly checked AIX is ok, the known target needing the functionality. https://gcc.gnu.org/ml/gcc-patches/2018-06/msg01568.html nathan On 06/25/2018 12:48 PM, Nathan Sidwell wrote: NO_IMPLICIT_EXTERN_C was introduced to tell the compiler that it didn't need to fake up 'extern "C" { ... }' around system header files. Over the years more and more system headers have become C++-aware, leading to more targets defining this macro. Unfortunately because of the sense of this macro, and that the requirement is based on the target-OS, whereas we partition the config directory by target-ARCH, it's become hard to know which targets still require the older functionality. There have been a few questions over the past 2 decades to figure this out, but they didn;t progress. This patch replaces the negative NO_IMPLICIT_EXTERN_C with the positive SYSTEM_IMPLICIT_EXTERN_C. Targets that previously did not define NO_IMPLICIT_EXTERN_C now need to define SYSTEM_IMPLICIT_EXTERN_C. I know of one such target -- AIX, and I'd be grateful this patch could be tried there. Going through the config files was tricky, and I may well have missed something. One suspicious file is config/sparc/openbsd64.h which did explicitly undef the macro, with the comment: /* Inherited from sp64-elf. */ sp64-elf.h does define the macro, but the other bsd's also define it, which leaves me wondering if openbsd.h has bit rotted here. Which leads me to another observation: It's quite possible the extern "C" functionality is enabled on targets that no longer need it, because their observed behaviour would not be broken. On the other hand, the failure mode of not defining its replacement (or alternatively mistakenly defining NO_IMPLICIT_EXTERN_C), would be immediate and obvious. And the fix is also simple. So, if you have a target that you think has C++-unaware system headers, please give this patch a spin and report. Blessing from a GM after a few days out there would be nice :) The lesson here is that when one has a transition, chose an enablement mechanism that makes it easy to tell when the transition is complete. nathan -- Nathan Sidwell
[PR c++/86374] Name lookup failure in enclosing template
This was a latent problem exposed by Jason's fix for 85815. There we needed to find the class in the scope stack. Unfortunately, here we'd pushed an (incomplete) instantiation list, rather than the general template list. We were previously getting away with that because we'd do the lookup in the a tsubstd scope, that had found the general template in another way. The fix is to have lookup_template call tsubst_aggr_class directly for contexts that are classes, with the entering_scope flag set. That way we figure that list is the general template. This matches the code in tsubst_aggr_class itself, when it's tsubsting the context of the class it is dealing with. A small drive-by cleanup in the parser. Committing to trunk now. will commit to gcc-8 after testing there. nathan -- Nathan Sidwell 2018-07-13 Nathan Sidwell PR c++/86374 * pt.c (lookup_template_class_1): Use tsubst_aggr_type for contexts that are classes. * parser.c (cp_parser_template_id): Combine entering_scope decl & initializer. PR c++/86374 * g++.dg/pr86374.C: New. Index: cp/parser.c === --- cp/parser.c (revision 262582) +++ cp/parser.c (working copy) @@ -15973,15 +15973,14 @@ cp_parser_template_id (cp_parser *parser else if (DECL_TYPE_TEMPLATE_P (templ) || DECL_TEMPLATE_TEMPLATE_PARM_P (templ)) { - bool entering_scope; /* In "template ... A::", A is the abstract A template (rather than some instantiation thereof) only if is not nested within some other construct. For example, in "template void f(T) { A::", A is just an instantiation of A. */ - entering_scope = (template_parm_scope_p () - && cp_lexer_next_token_is (parser->lexer, - CPP_SCOPE)); + bool entering_scope + = (template_parm_scope_p () + && cp_lexer_next_token_is (parser->lexer, CPP_SCOPE)); template_id = finish_template_type (templ, arguments, entering_scope); } Index: cp/pt.c === --- cp/pt.c (revision 262582) +++ cp/pt.c (working copy) @@ -9368,8 +9368,15 @@ lookup_template_class_1 (tree d1, tree a return found; } - context = tsubst (DECL_CONTEXT (gen_tmpl), arglist, - complain, in_decl); + context = DECL_CONTEXT (gen_tmpl); + if (context && TYPE_P (context)) + { + context = tsubst_aggr_type (context, arglist, complain, in_decl, true); + context = complete_type (context); + } + else + context = tsubst (context, arglist, complain, in_decl); + if (context == error_mark_node) return error_mark_node; Index: testsuite/g++.dg/pr86374.C === --- testsuite/g++.dg/pr86374.C (revision 0) +++ testsuite/g++.dg/pr86374.C (working copy) @@ -0,0 +1,20 @@ +// pr C++/86374 +// bogus lookup error +template +struct list { + static const int index = 1; + template struct addWithChecking {}; +}; + +template +struct find { + static const int result = 0; +}; + +template +template +struct list::addWithChecking +{ + static const int xres = +find >::result; // bogus error about index here. +};
Re: C++ patch ping
On 07/13/2018 09:49 AM, Jakub Jelinek wrote: Hi! I'd like to ping the following C++ patches: - PR c++/85515 make range for temporaries unspellable during parsing and only turn them into spellable for debug info purposes http://gcc.gnu.org/ml/gcc-patches/2018-07/msg00086.html How hard would it be to add the 6 special identifiers to the C++ global table via initialize_predefined_identifiers (decl.c) and then use them directly in the for range machinery? repeated get_identifier ("string-const") just smells bad. nathan -- Nathan Sidwell
Re: C++ patch ping
On 07/13/2018 09:49 AM, Jakub Jelinek wrote: - PR c++/3698, PR c++/86208 extern_decl_map & TREE_USED fix (plus 2 untested variants) http://gcc.gnu.org/ml/gcc-patches/2018-07/msg00084.html ok, thanks -- Nathan Sidwell
Re: [C++ Patch] Check permerror return value
On 07/16/2018 07:46 AM, Paolo Carlini wrote: Hi, over the last weeks, while working on various diagnostic issues, I noticed a few defective permerror + inform pairs. Tested x86_64-linux. ok, thanks -- Nathan Sidwell
Re: [PATCH][C++] Fix PR86523
On 07/17/2018 08:32 AM, Richard Biener wrote: The following makes sure to generate early debug for globals that end up being pushed to the backend via the write_out_vars call in c_parse_final_cleanups. rest_of_decl_compilation is confused by seeing current_function_decl set and skips the debug-hook invocation because of that. So the easy fix is to flush out globals before starting the init function. looks sensible, ok. nathan -- Nathan Sidwell
Re: [C++ PATCH] Further get_identifier ("string literal") C++ FE caching
So cool! Thanks. Sorry for the top posting nathan-- Nathan Sidwell Original message From: Jakub Jelinek Date: 7/18/18 17:04 (GMT-05:00) To: Nathan Sidwell Cc: Jason Merrill , gcc-patches@gcc.gnu.org Subject: [C++ PATCH] Further get_identifier ("string literal") C++ FE caching On Wed, Jul 18, 2018 at 12:19:31PM +0200, Jakub Jelinek wrote: > Shall I submit an incremental patch for the "abi_tag", "gnu", "begin", "end", > "get", > "tuple_size", "tuple_element" etc. identifiers? Here it is in an incremental patch. I've tried to do it only for get_identifier ("string literal") calls that can be called many times during parsing rather than just at most once, and aren't related to -fgnu-tm, -fopenmp, Obj-C++ or vtv. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2018-07-18 Jakub Jelinek * cp-tree.h (enum cp_tree_index): Add CPTI_{ABI_TAG,ALIGNED,BEGIN,END,GET,TUPLE_{ELEMENT,SIZE}}_IDENTIFIER and CPTI_{GNU,TYPE,VALUE,FUN,CLOSURE}_IDENTIFIER. (abi_tag_identifier, aligned_identifier, begin_identifier, end_identifier, get__identifier, gnu_identifier, tuple_element_identifier, tuple_size_identifier, type_identifier, value_identifier, fun_identifier, closure_identifier): Define. * decl.c (initialize_predefined_identifiers): Initialize the above identifiers. (get_tuple_size): Use tuple_size_identifier instead of get_identifier ("tuple_size") and value_identifier instead of get_identifier ("value"). (get_tuple_element_type): Use tuple_element_identifier instead of get_identifier ("tuple_element") and type_identifier instead of get_identifier ("type"). (get_tuple_decomp_init): Use get__identifier instead of get_identifier ("get"). * lambda.c (maybe_add_lambda_conv_op): Use fun_identifier instead of get_identifier ("_FUN"). * parser.c (cp_parser_lambda_declarator_opt): Use closure_identifier instead of get_identifier ("__closure"). (cp_parser_std_attribute): Use gnu_identifier instead of get_identifier ("gnu"). (cp_parser_std_attribute_spec): Likewise. Use aligned_identifier instead of get_identifier ("aligned"). * class.c (check_abi_tags, inherit_targ_abi_tags): Use abi_tag_identifier instead of get_identifier ("abi_tag"). --- gcc/cp/cp-tree.h.jj 2018-07-18 11:57:55.980529748 +0200 +++ gcc/cp/cp-tree.h2018-07-18 18:52:44.805248036 +0200 @@ -160,6 +160,18 @@ enum cp_tree_index CPTI_FOR_RANGE_IDENTIFIER, CPTI_FOR_BEGIN_IDENTIFIER, CPTI_FOR_END_IDENTIFIER, + CPTI_ABI_TAG_IDENTIFIER, + CPTI_ALIGNED_IDENTIFIER, + CPTI_BEGIN_IDENTIFIER, + CPTI_END_IDENTIFIER, + CPTI_GET_IDENTIFIER, + CPTI_GNU_IDENTIFIER, + CPTI_TUPLE_ELEMENT_IDENTIFIER, + CPTI_TUPLE_SIZE_IDENTIFIER, + CPTI_TYPE_IDENTIFIER, + CPTI_VALUE_IDENTIFIER, + CPTI_FUN_IDENTIFIER, + CPTI_CLOSURE_IDENTIFIER, CPTI_LANG_NAME_C, CPTI_LANG_NAME_CPLUSPLUS, @@ -286,6 +298,18 @@ extern GTY(()) tree cp_global_trees[CPTI #define for_range_identifier cp_global_trees[CPTI_FOR_RANGE_IDENTIFIER] #define for_begin_identifier cp_global_trees[CPTI_FOR_BEGIN_IDENTIFIER] #define for_end_identifier cp_global_trees[CPTI_FOR_END_IDENTIFIER] +#define abi_tag_identifier cp_global_trees[CPTI_ABI_TAG_IDENTIFIER] +#define aligned_identifier cp_global_trees[CPTI_ALIGNED_IDENTIFIER] +#define begin_identifier cp_global_trees[CPTI_BEGIN_IDENTIFIER] +#define end_identifier cp_global_trees[CPTI_END_IDENTIFIER] +#define get__identifier cp_global_trees[CPTI_GET_IDENTIFIER] +#define gnu_identifier cp_global_trees[CPTI_GNU_IDENTIFIER] +#define tuple_element_identifier cp_global_trees[CPTI_TUPLE_ELEMENT_IDENTIFIER] +#define tuple_size_identifier cp_global_trees[CPTI_TUPLE_SIZE_IDENTIFIER] +#define type_identifier cp_global_trees[CPTI_TYPE_IDENTIFIER] +#define value_identifier cp_global_trees[CPTI_VALUE_IDENTIFIER] +#define fun_identifier cp_global_trees[CPTI_FUN_IDENTIFIER] +#define closure_identifier cp_global_trees[CPTI_CLOSURE_IDENTIFIER] #define lang_name_ccp_global_trees[CPTI_LANG_NAME_C] #define lang_name_cplusplus cp_global_trees[CPTI_LANG_NAME_CPLUSPLUS] --- gcc/cp/decl.c.jj2018-07-18 11:59:06.220595473 +0200 +++ gcc/cp/decl.c 2018-07-18 18:52:58.676265952 +0200 @@ -4050,6 +4050,18 @@ initialize_predefined_identifiers (void) {"__for_range", &for_range_identifier, cik_normal},
Re: [C++ PATCH] Further get_identifier ("string literal") C++ FE caching
On 07/18/2018 06:43 PM, Jakub Jelinek wrote: On Wed, Jul 18, 2018 at 06:00:20PM -0400, Nathan Sidwell wrote: So cool! Thanks. Ok for both patches or just this one? both are ok, sorry for delay (vacation), thanks for doing that! nathan -- Nathan Sidwell
Re: [PATCH 01/11] Add __builtin_speculation_safe_value
On 07/27/2018 05:37 AM, Richard Earnshaw wrote: +/* Work out the size of the first argument of a call to + __builtin_speculation_safe_value. Only pointers and integral types + are permitted. Return -1 if the argument type is not supported or + the size is too large; 0 if the argument type is a pointer or the + size if it is integral. */ +static enum built_in_function +speculation_safe_value_resolve_call (tree function, vec *params) If I'm reading the expander correctly, isn't it 'pointers to integral types', not any old pointer? so the following needs some adjustment... + if (POINTER_TYPE_P (type)) +return BUILT_IN_SPECULATION_SAFE_VALUE_PTR; + + if (!INTEGRAL_TYPE_P (type)) +goto incompatible; + if (!COMPLETE_TYPE_P (type)) +goto incompatible; Are incomplete integral types a thing? (forward enum extension?) I presume resolve_overloaded_builtin already works correctly with template instantiations, but a templatatized testcase would be nice -- shout if you'd like help constructing one. nathan -- Nathan Sidwell
Re: [PATCH 01/11] Add __builtin_speculation_safe_value
On 07/27/2018 08:32 AM, Richard Earnshaw (lists) wrote: The intention is to allow pointer to anything. Oh, the speculation safe fetch is of the pointer itself, not the thing being pointed to. I'd missed that. I'm not sure I understand why that needs special casing down to the expander (why can't the resolver figure the mode?), but you're the expert in that kind of stuff. I presume resolve_overloaded_builtin already works correctly with template instantiations, but a templatatized testcase would be nice -- shout if you'd like help constructing one. Yelp! added to todo list. nathan -- Nathan Sidwell
Re: [PATCH] Fix an UBSAN error in cp/parse.c (PR c++/86653).
On 07/26/2018 04:52 AM, Martin Liška wrote: Hello. Quite simple patch that initializes a boolean value before it's used. The variable is not initialized when an error recovery happens. Ready for trunk after testing? ok, thanks nathan -- Nathan Sidwell
Re: [PATCH] c++/modules: ICE with lambda initializing local var [PR105322]
Thanks for looking at this, but your patch is essentially papering over the problem. It took me a while to figure out, but the clue was that things like 'decltype(f()).m' worked, but 'decltype(f()){0}' did not. The CONSTRUCTOR node is the exception to the rule that required an expression node's type to be streamed after the node's operands. We want the opposite for CTORS. I'll commit this once bootstrapped. nathan On 10/18/23 12:28, Patrick Palka wrote: Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for trunk? -- >8 -- For a local variable initialized by a lambda: auto f = []{}; The corresponding BLOCK_VARS contains the variable declaration first, followed by the closure type declaration, consistent with the syntactical order. This however means that a use of the closure type appears (in the variable type/initializer) before the declaration of the type. This ends up causing an ICE when streaming the BLOCK_VARS of f1 below because we stream (by value) the CONSTRUCTOR initializer of g1 -- which contains components of the closure type -- before we've streamed the declaration defining the closure type. The following comment in module.cc seems relevant: /* We want to stream the type of a expression-like nodes /after/ we've streamed the operands. The type often contains (bits of the) types of the operands, and with things like decltype and noexcept in play, we really want to stream the decls defining the type before we try and stream the type on its own. Otherwise we can find ourselves trying to read in a decl, when we're already partially reading in a component of its type. And that's bad. */ This patch narrowly fixes this issue by special casing closure type declarations in add_decl_to_level. (A loop is needed since there could be multiple variable declarations with an unprocessed initializer in light of structured bindings.) PR c++/105322 gcc/cp/ChangeLog: * name-lookup.cc (add_decl_to_level): When adding a closure type declaration to a block scope, add it before rather than after any variable declarations whose initializer we're still processing. gcc/testsuite/ChangeLog: * g++.dg/modules/lambda-5_a.C: New test. * g++.dg/modules/lambda-5_b.C: New test. --- gcc/cp/name-lookup.cc | 19 --- gcc/testsuite/g++.dg/modules/lambda-5_a.C | 23 +++ gcc/testsuite/g++.dg/modules/lambda-5_b.C | 10 ++ 3 files changed, 49 insertions(+), 3 deletions(-) create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_a.C create mode 100644 gcc/testsuite/g++.dg/modules/lambda-5_b.C diff --git a/gcc/cp/name-lookup.cc b/gcc/cp/name-lookup.cc index a8b9229b29e..bb00baaf9f4 100644 --- a/gcc/cp/name-lookup.cc +++ b/gcc/cp/name-lookup.cc @@ -391,9 +391,22 @@ add_decl_to_level (cp_binding_level *b, tree decl) gcc_assert (b->names != decl); /* We build up the list in reverse order, and reverse it later if - necessary. */ - TREE_CHAIN (decl) = b->names; - b->names = decl; + necessary. If we're adding a lambda closure type to a block + scope as part of a local variable initializer, then make sure + we declare the type before the variable; modules expects that + we see a type declaration before a use of the type. */ + tree *prev = &b->names; + if (b->kind == sk_block + && !processing_template_decl + && TREE_CODE (decl) == TYPE_DECL + && LAMBDA_TYPE_P (TREE_TYPE (decl))) +while (*prev && VAR_P (*prev) + && !DECL_EXTERNAL (*prev) + && !DECL_INITIALIZED_P (*prev)) + prev = &TREE_CHAIN (*prev); + + TREE_CHAIN (decl) = *prev; + *prev = decl; /* If appropriate, add decl to separate list of statics. We include extern variables because they might turn out to be static later. diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C new file mode 100644 index 000..6b54c8e3173 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C @@ -0,0 +1,23 @@ +// PR c++/105322 +// { dg-additional-options -fmodules-ts } +// { dg-module-cmi pr105322 } + +export module pr105322; + +struct A { }; + +export +inline void f1() { + A a; + auto g1 = [a] { }; // used to ICE here during stream out +} + +export +template +void f2() { + A a; + auto g2 = [a] { }; +} + +export +inline auto g3 = [a=A{}] { }; diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_b.C b/gcc/testsuite/g++.dg/modules/lambda-5_b.C new file mode 100644 index 000..e25a913b726 --- /dev/null +++ b/gcc/testsuite/g++.dg/modules/lambda-5_b.C @@ -0,0 +1,10 @@ +// PR c++/105322 +// { dg-additional-options -fmodules-ts } + +import pr105322; + +int main() { + f1(); + f2(); + g3(); +}
Re: [PATCH] c++/modules: fix up recent testcases
Patrick, thanks for noticing this, and this is a suitable workaround for another bug. We should either be emitting the definition of that member function in the object file of its containing function. Or it should be implicitly inline. I know in module perview the in-class defined member functions at namespace scope are /not/ implicitly inline. But I can't recall what the std says about non-namespace scope classes. Ah, it appears to be the former we should be doing: [class.mfct] If a member function is attached to the global module and is defined (9.5) in its class definition, it is inline (9.2.8). Notice we can get into the weird situation that the member functions of a local class of a module-purview inline function might not themselves be inline! On 10/25/23 14:32, Patrick Palka wrote: Tested on x86_64-pc-linux-gnu, does this look OK for trunk? Declaring get() inline seems necessary to avoid link failure: /usr/bin/ld: /tmp/ccwdv6Co.o: in function `g3@pr105322.Decltype()': decltype-1_b.C:(.text._ZW8pr105322W8Decltype2g3v[_ZW8pr105322W8Decltype2g3v]+0x18): undefined reference to `f@pr105322.Decltype()::A::get()' Not sure if that's expected? -- >8 -- This fixes some minor issues with the testcases from r14-4806-g084addf8a700fa. gcc/testsuite/ChangeLog: * g++.dg/modules/decltype-1_a.C: Add missing } to dg-module-do directive. Declare f()::A::get() inline. * g++.dg/modules/lambda-5_a.C: Add missing } to dg-module-do directive. --- gcc/testsuite/g++.dg/modules/decltype-1_a.C | 4 ++-- gcc/testsuite/g++.dg/modules/lambda-5_a.C | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/gcc/testsuite/g++.dg/modules/decltype-1_a.C b/gcc/testsuite/g++.dg/modules/decltype-1_a.C index ca66e8b598a..6512f151aae 100644 --- a/gcc/testsuite/g++.dg/modules/decltype-1_a.C +++ b/gcc/testsuite/g++.dg/modules/decltype-1_a.C @@ -1,5 +1,5 @@ // PR c++/105322 -// { dg-module-do link +// { dg-module-do link } // { dg-additional-options -fmodules-ts } // { dg-module-cmi pr105322.Decltype } @@ -7,7 +7,7 @@ export module pr105322.Decltype; auto f() { struct A { int m; -int get () { return m; } +inline int get () { return m; } }; return A{}; } diff --git a/gcc/testsuite/g++.dg/modules/lambda-5_a.C b/gcc/testsuite/g++.dg/modules/lambda-5_a.C index 6b589d4965c..37d0e77b1e1 100644 --- a/gcc/testsuite/g++.dg/modules/lambda-5_a.C +++ b/gcc/testsuite/g++.dg/modules/lambda-5_a.C @@ -1,5 +1,5 @@ // PR c++/105322 -// { dg-module-do link +// { dg-module-do link } // { dg-additional-options -fmodules-ts } // { dg-module-cmi pr105322.Lambda } -- Nathan Sidwell
c++: Stat-hack for members [PR 98530]
This was a header file that deployed the stat-hack inside a class (both a member-class and a [non-static data] member had the same name). Due to the way that's represented in name lookup we missed the class. Sadly just changing the representation globally has detrimental effects elsewhere, and this is a rare case, so just creating a new overload on the fly shouldn't be a problem. PR c++/98530 gcc/cp/ * name-lookup.c (lookup_class_binding): Rearrange a stat-hack. gcc/testsuite/ * g++.dg/modules/stat-mem-1.h: New. * g++.dg/modules/stat-mem-1_a.H: New. * g++.dg/modules/stat-mem-1_b.C: New. -- Nathan Sidwell diff --git c/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index b4b6c0b81b5..c99f2e3622d 100644 --- c/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -3926,11 +3926,16 @@ lookup_class_binding (tree klass, tree name) vec *member_vec = CLASSTYPE_MEMBER_VEC (klass); found = member_vec_binary_search (member_vec, name); - if (IDENTIFIER_CONV_OP_P (name)) + if (!found) + ; + else if (STAT_HACK_P (found)) + /* Rearrange the stat hack so that we don't need to expose that + internal detail. */ + found = ovl_make (STAT_TYPE (found), STAT_DECL (found)); + else if (IDENTIFIER_CONV_OP_P (name)) { gcc_checking_assert (name == conv_op_identifier); - if (found) - found = OVL_CHAIN (found); + found = OVL_CHAIN (found); } } else diff --git c/gcc/testsuite/g++.dg/modules/stat-mem-1.h w/gcc/testsuite/g++.dg/modules/stat-mem-1.h new file mode 100644 index 000..b5703ea2262 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/stat-mem-1.h @@ -0,0 +1,6 @@ + +struct fpu { + struct NAME { +int state; + } NAME; +}; diff --git c/gcc/testsuite/g++.dg/modules/stat-mem-1_a.H w/gcc/testsuite/g++.dg/modules/stat-mem-1_a.H new file mode 100644 index 000..6daa137be4f --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/stat-mem-1_a.H @@ -0,0 +1,5 @@ +// { dg-additional-options -fmodule-header } +// PR c++ 98530 stat-hack inside a structure +// { dg-module-cmi {} } + +#include "stat-mem-1.h" diff --git c/gcc/testsuite/g++.dg/modules/stat-mem-1_b.C w/gcc/testsuite/g++.dg/modules/stat-mem-1_b.C new file mode 100644 index 000..9b83d4e6e30 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/stat-mem-1_b.C @@ -0,0 +1,4 @@ +// { dg-additional-options "-fmodules-ts -fno-module-lazy" } + +#include "stat-mem-1.h" +import "stat-mem-1_a.H";
c++: Fix null this pointer [PR 98624]
One maynot usea null this pointer to invoke astatic member function. Thisfixes the remaining ubsan errors found with an ubsan bootstrap. PR c++/98624 gcc/cp/ * module.cc (depset::hash::find_dependencies): Add module arg. (trees_out::core_vals):Check state before calling write_location. (sort_cluster, module_state::write): Adjust find_dependencies call. -- Nathan Sidwell diff --git i/gcc/cp/module.cc w/gcc/cp/module.cc index 8f9c7940ef8..6741ae03ee7 100644 --- i/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -2567,7 +2567,7 @@ public: void add_class_entities (vec *); public: -void find_dependencies (); +void find_dependencies (module_state *); bool finalize_dependencies (); vec connect (); }; @@ -5898,7 +5898,8 @@ trees_out::core_vals (tree t) if (!DECL_TEMPLATE_PARM_P (t)) WT (t->decl_minimal.context); - state->write_location (*this, t->decl_minimal.locus); + if (state) + state->write_location (*this, t->decl_minimal.locus); } if (CODE_CONTAINS_STRUCT (code, TS_TYPE_COMMON)) @@ -6001,7 +6002,8 @@ trees_out::core_vals (tree t) if (CODE_CONTAINS_STRUCT (code, TS_EXP)) { - state->write_location (*this, t->exp.locus); + if (state) + state->write_location (*this, t->exp.locus); /* Walk in forward order, as (for instance) REQUIRES_EXPR has a bunch of unscoped parms on its first operand. It's safer to @@ -6140,9 +6142,12 @@ trees_out::core_vals (tree t) /* Miscellaneous common nodes. */ case BLOCK: - state->write_location (*this, t->block.locus); - state->write_location (*this, t->block.end_locus); - + if (state) + { + state->write_location (*this, t->block.locus); + state->write_location (*this, t->block.end_locus); + } + /* DECL_LOCAL_DECL_P decls are first encountered here and streamed by value. */ chained_decls (t->block.vars); @@ -6183,7 +6188,8 @@ trees_out::core_vals (tree t) /* The ompcode is serialized in start. */ if (streaming_p ()) WU (t->omp_clause.subcode.map_kind); - state->write_location (*this, t->omp_clause.locus); + if (state) + state->write_location (*this, t->omp_clause.locus); unsigned len = omp_clause_num_ops[OMP_CLAUSE_CODE (t)]; for (unsigned ix = 0; ix != len; ix++) @@ -6270,8 +6276,9 @@ trees_out::core_vals (tree t) WT (((lang_tree_node *)t)->lambda_expression.extra_scope); /* pending_proxies is a parse-time thing. */ gcc_assert (!((lang_tree_node *)t)->lambda_expression.pending_proxies); - state->write_location - (*this, ((lang_tree_node *)t)->lambda_expression.locus); + if (state) + state->write_location + (*this, ((lang_tree_node *)t)->lambda_expression.locus); if (streaming_p ()) { WU (((lang_tree_node *)t)->lambda_expression.default_capture_mode); @@ -6291,8 +6298,9 @@ trees_out::core_vals (tree t) case STATIC_ASSERT: WT (((lang_tree_node *)t)->static_assertion.condition); WT (((lang_tree_node *)t)->static_assertion.message); - state->write_location - (*this, ((lang_tree_node *)t)->static_assertion.location); + if (state) + state->write_location + (*this, ((lang_tree_node *)t)->static_assertion.location); break; case TEMPLATE_DECL: @@ -6324,7 +6332,8 @@ trees_out::core_vals (tree t) WT (m.binfo); WT (m.decl); WT (m.diag_decl); - state->write_location (*this, m.loc); + if (state) + state->write_location (*this, m.loc); } } } @@ -13159,9 +13168,9 @@ depset::hash::add_mergeable (depset *mergeable) entries on the same binding that need walking. */ void -depset::hash::find_dependencies () +depset::hash::find_dependencies (module_state *module) { - trees_out walker (NULL, NULL, *this); + trees_out walker (NULL, module, *this); vec unreached; unreached.create (worklist.length ()); @@ -13547,7 +13556,7 @@ sort_cluster (depset::hash *original, depset *scc[], unsigned size) gcc_checking_assert (use_lwm <= bind_lwm); dump (dumper::MERGE) && dump ("Ordering %u/%u depsets", use_lwm, size); - table.find_dependencies (); + table.find_dependencies (nullptr); vec order = table.connect (); gcc_checking_assert (order.length () == use_lwm); @@ -17571,7 +17580,7 @@ module_state::write (elf_out *to, cpp_reader *reader) } /* Now join everything up. */ - table.find_dependencies (); + table.find_dependencies (this); if (!table.finalize_dependencies ()) {
Re: driver: do not check input file existence here [PR 98452]
On 1/21/21 12:59 PM, Joseph Myers wrote: On Wed, 20 Jan 2021, Nathan Sidwell wrote: Unspecified non-existent response file: OLD: (1)devvm293:282>g++ -c @nothing g++: error: nothing: No such file or directory g++: fatal error: no input files compilation terminated. NEW: (1)devvm293:284>./xg++ -B./ -c @nothing xg++: warning: @nothing: linker input file unused because linking not done This is less clear, in that one might suppose "nothing" is meant to be a file listing C++ sources to compile, so should be processed and should result in an error for its absence. However, this particular place in the driver handling OPT_SPECIAL_input_file doesn't seem like the right place for such an error. Either the correct handling of response files is that the @file argument is silently kept as-is and so the warning after your patch is correct, or the correct handling of response files is that there should be an error if such a file cannot be found (and so users with filenames starting @ must reference them e.g. as ./@file.cc) and expandargv should have some way to report that error. Leaving it to OPT_SPECIAL_input_file as in the driver at present would mean such an error doesn't occur when @file, unexpanded, appears to be an option argument rather than an input file. Agreed. I wonder who cares about naming sources '@file.cc' or '@linkerscript' or whatever? Do you want expandargv altered alongs the lines you mention? Or a bug filed? [in order for my patch to be acceptable] nathan -- Nathan Sidwell
Re: driver: do not check input file existence here [PR 98452]
On 1/21/21 3:20 PM, Joseph Myers wrote: On Thu, 21 Jan 2021, Nathan Sidwell wrote: Do you want expandargv altered alongs the lines you mention? Or a bug filed? [in order for my patch to be acceptable] The patch is OK as-is. Filing a bug for expandargv handling of missing files might be a good idea; it seems very arbitrary that @directory produces an error, but @file for a nonexistent file, or a file without read access, or a file with an I/O error on reading, gets passed through. 98794 filed, thanks! nathan -- Nathan Sidwell
testsuite: Uniquify test names [PR 98795]
Header unit names come from the path the preprocessor determines, and thus can be absolute. This tweaks the testsuite to elide that absoluteness when embedded in a CMI name. We were also not distinguishing link and execute tests by the $std flags, so append them when necessary. PR testsuite/98795 gcc/testsuite/ * g++.dg/modules/modules.exp (module_cmi_p): Avoid embedded absolute paths. (module_do_it): Append $std to test name. -- Nathan Sidwell diff --git c/gcc/testsuite/g++.dg/modules/modules.exp w/gcc/testsuite/g++.dg/modules/modules.exp index bd88bde60dc..8c9a00e68e9 100644 --- c/gcc/testsuite/g++.dg/modules/modules.exp +++ w/gcc/testsuite/g++.dg/modules/modules.exp @@ -118,10 +118,12 @@ proc module_cmi_p { src ifs } { } set not [lindex $if_arg 2] set cmi [lindex $if_arg 3] + global srcdir + set relcmi [string map [list $srcdir "/\$srcdir"] $cmi] if { $not != [file_on_host exists $cmi] } { - pass "$src module-cmi $spec ($cmi)" + pass "$src module-cmi $spec ($relcmi)" } else { - fail "$src module-cmi $spec ($cmi)" + fail "$src module-cmi $spec ($relcmi)" set not [expr ! $not ] } if { ! $not } { @@ -210,8 +212,10 @@ proc module_do_it { do_what testcase std asm_list } { } set options { } +set ident $testcase if { $std != "" } { lappend options "additional_flags=$std" + set ident "$ident $std" } if { [llength $do_what] > 3 } { lappend options "additional_flags=[lindex $do_what 3]" @@ -222,15 +226,15 @@ proc module_do_it { do_what testcase std asm_list } { # link it verbose "Linking $asm_list" 1 if { !$ok } { - unresolved "$testcase link" + unresolved "$identlink" } else { set out [${tool}_target_compile $asm_list \ $execname executable $options] eval $xfail if { $out == "" } { - pass "$testcase link" + pass "$ident link" } else { - fail "$testcase link" + fail "$ident link" set ok 0 } } @@ -238,12 +242,12 @@ proc module_do_it { do_what testcase std asm_list } { # run it? if { !$run } { } elseif { !$ok } { - unresolved "$testcase execute" + unresolved "$ident execute" } else { set out [${tool}_load $execname "" ""] set status [lindex $out 0] eval $xfail - $status "$testcase execute" + $status "$ident execute" if { $status != "pass" } { set $ok 0 }
Re: [PATCH] config: check for sighandler_t properly
On 1/22/21 12:19 PM, Nick Alcock wrote: Searching for sighander_t is unlikely to succeed anywhere. The attempt to #include is also not working, and fixing it shows that doing an AC_DEFINE in the body of an AC_CHECK_TYPE like that is also risky: both fixed. (The purpose of this check is opaque to me: neither libcody nor GCC ever includes , and though is widely included, it is not directly included by any of the headers checking this macro... for now I've fixed things to conform to the comment, but perhaps we should be checking the non-deprecated instead, and #including it in mapper-client.h?) Thanks for noticint this. I think this is left over from earlier module-mapper predating its move (and changes) to c++tools. If you'd like to remove that entire bit of configure.ac, that'd be great (or I can do so, if you don't have time) nathan -- Nathan Sidwell
c++: cross-module __cxa_atexit use [PR 98531]
[ARM has unique ABI here, hence cc'ing Richard] Solaris tickled this bug as it has some mutex/sync/something primitive with a destructor, hence wanted to generate a __cxa_atexit call inside an inline/template function. But the problem is not solaris-specific. I tested this bootstrapping both x86_64-linux and aarch64-linux. I'll commit in a couple of days if there are no further comments. The compiler's use of lazily-declared library functions must insert said functions into a symbol table, so that they can be correctly merged across TUs at the module-level. We have too many different ways of declaring such library functions. This fixes __cxa_atexit (or its system-specific variations), pushing (or merging) the decl into the appropriate namespace. PR c++/98531 gcc/cp/ * cp-tree.h (push_abi_namespace, pop_abi_namespace): Declare. * decl.c (push_abi_namespace, pop_abi_namespace): Moved from rtti.c, add default namespace arg. (declare_global_var): Use push/pop_abi_namespace. (get_atexit_node): Push the fndecl into a namespace. * rtti.c (push_abi_namespace, pop_abi_namespace): Moved to decl.c. gcc/testsuite/ * g++.dg/modules/pr98531.h: New. * g++.dg/modules/pr98531_a.H: New. * g++.dg/modules/pr98531_b.C: New. -- Nathan Sidwell diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index 51139f4a4be..fefd870360b 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -193,7 +193,9 @@ enum cp_tree_index CPTI_MODULE_HWM, /* Nodes after here change during compilation, or should not be in - the module's global tree table. */ + the module's global tree table. Such nodes must be locatable + via name lookup or type-construction, as those are the only + cross-TU matching capabilities remaining. */ /* We must find these via the global namespace. */ CPTI_STD, @@ -6620,6 +6622,9 @@ extern tree make_typename_type (tree, tree, enum tag_types, tsubst_flags_t); extern tree build_typename_type (tree, tree, tree, tag_types); extern tree make_unbound_class_template (tree, tree, tree, tsubst_flags_t); extern tree make_unbound_class_template_raw (tree, tree, tree); +extern unsigned push_abi_namespace (tree node = abi_node); +extern void pop_abi_namespace (unsigned flags, + tree node = abi_node); extern tree build_library_fn_ptr (const char *, tree, int); extern tree build_cp_library_fn_ptr (const char *, tree, int); extern tree push_library_fn (tree, tree, tree, int); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 1a114a2e2d0..1f97bcf1e55 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -4696,6 +4696,30 @@ cxx_init_decl_processing (void) using_eh_for_cleanups (); } +/* Enter an abi node in global-module context. returns a cookie to + give to pop_abi_namespace. */ + +unsigned +push_abi_namespace (tree node) +{ + push_nested_namespace (node); + push_visibility ("default", 2); + unsigned flags = module_kind; + module_kind = 0; + return flags; +} + +/* Pop an abi namespace, FLAGS is the cookie push_abi_namespace gave + you. */ + +void +pop_abi_namespace (unsigned flags, tree node) +{ + module_kind = flags; + pop_visibility (2); + pop_nested_namespace (node); +} + /* Create the VAR_DECL for __FUNCTION__ etc. ID is the name to give the decl, LOC is the location to give the decl, NAME is the initialization string and TYPE_DEP indicates whether NAME depended @@ -8668,21 +8692,19 @@ cp_finish_decomp (tree decl, tree first, unsigned int count) static tree declare_global_var (tree name, tree type) { - tree decl; - - push_to_top_level (); - decl = build_decl (input_location, VAR_DECL, name, type); + auto cookie = push_abi_namespace (global_namespace); + tree decl = build_decl (input_location, VAR_DECL, name, type); TREE_PUBLIC (decl) = 1; DECL_EXTERNAL (decl) = 1; DECL_ARTIFICIAL (decl) = 1; - DECL_CONTEXT (decl) = FROB_CONTEXT (global_namespace); + DECL_CONTEXT (decl) = FROB_CONTEXT (current_namespace); /* If the user has explicitly declared this variable (perhaps because the code we are compiling is part of a low-level runtime library), then it is possible that our declaration will be merged with theirs by pushdecl. */ decl = pushdecl (decl); cp_finish_decl (decl, NULL_TREE, false, NULL_TREE, 0); - pop_from_top_level (); + pop_abi_namespace (cookie, global_namespace); return decl; } @@ -8727,6 +8749,7 @@ get_atexit_node (void) tree fn_ptr_type; const char *name; bool use_aeabi_atexit; + tree ctx = global_namespace; if (atexit_node) return atexit_node; @@ -8762,9 +8785,20 @@ get_atexit_node (void) argtype0, argtype1, argtype2, NULL_TREE); if (use_aeabi_atexit) - name = "__aeabi_atexit"; + { + name = "__aeabi_atexit"
Re: [PATCH] config: check for sighandler_t properly
On 1/25/21 12:50 PM, Nick Alcock wrote: On 25 Jan 2021, Nathan Sidwell uttered the following: On 1/22/21 12:19 PM, Nick Alcock wrote: Searching for sighander_t is unlikely to succeed anywhere. The attempt to #include is also not working, and fixing it shows that doing an AC_DEFINE in the body of an AC_CHECK_TYPE like that is also risky: both fixed. (The purpose of this check is opaque to me: neither libcody nor GCC ever includes , and though is widely included, it is not directly included by any of the headers checking this macro... for now I've fixed things to conform to the comment, but perhaps we should be checking the non-deprecated instead, and #including it in mapper-client.h?) Thanks for noticint this. I think this is left over from earlier module-mapper predating its move (and changes) to c++tools. If you'd like to remove that entire bit of configure.ac, that'd be great (or I can do so, if you don't have time) The sig_handler_t does appear to be still used, albeit not much: it's used by mapper-client.h for the module_client::sigpipe declaration, which is then used by the module mapper, which does appear to be still used from elsewhere in module.c. Oh, I recall now. We have to ignore SIGPIPE when using a pair of pipes to communicate between compiler and mapper. Otherwise the compiler can die awfully if the mapper goes away (though maybe that's the behaviour we'd now want?) I think you're right about checking though, not -- Nathan Sidwell
c++: header unit template alias merging [PR 98770]
Typedefs are streamed by streaming the underlying type, and then recreating the typedef. But this breaks checking a duplicate is the same as the original when it is a template alias -- we end up checking a template alias (eg __void_t) against the underlying type (void). And those are not the same template alias. This stops pretendig that the underlying type is the typedef for that checking and tells is_matching_decl 'you have a typedef', so it knows what to do. (We do not want to recreate the typedef of the duplicate, because that whole set of nodes is going to go away.) PR c++/98770 gcc/cp/ gcc/testsuite/ * g++.dg/modules/pr98770_a.C: New. * g++.dg/modules/pr98770_b.C: New. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 18f5de8724b..daf75b16007 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -3029,7 +3029,7 @@ public: bool read_definition (tree decl); private: - bool is_matching_decl (tree existing, tree decl); + bool is_matching_decl (tree existing, tree decl, bool is_typedef); static bool install_implicit_member (tree decl); bool read_function_def (tree decl, tree maybe_template); bool read_var_def (tree decl, tree maybe_template); @@ -7864,8 +7864,8 @@ trees_out::decl_value (tree decl, depset *dep) || !dep == (VAR_OR_FUNCTION_DECL_P (inner) && DECL_LOCAL_DECL_P (inner))); else if ((TREE_CODE (inner) == TYPE_DECL - && TYPE_NAME (TREE_TYPE (inner)) == inner - && !is_typedef) + && !is_typedef + && TYPE_NAME (TREE_TYPE (inner)) == inner) || TREE_CODE (inner) == FUNCTION_DECL) { bool write_defn = !dep && has_definition (decl); @@ -8088,12 +8088,6 @@ trees_in::decl_value () && TREE_CODE (inner) == TYPE_DECL && DECL_ORIGINAL_TYPE (inner) && !TREE_TYPE (inner)); - if (is_typedef) -{ - /* Frob it to be ready for cloning. */ - TREE_TYPE (inner) = DECL_ORIGINAL_TYPE (inner); - DECL_ORIGINAL_TYPE (inner) = NULL_TREE; -} existing = back_refs[~tag]; bool installed = install_entity (existing); @@ -8156,7 +8150,12 @@ trees_in::decl_value () } if (is_typedef) - set_underlying_type (inner); + { + /* Frob it to be ready for cloning. */ + TREE_TYPE (inner) = DECL_ORIGINAL_TYPE (inner); + DECL_ORIGINAL_TYPE (inner) = NULL_TREE; + set_underlying_type (inner); + } if (inner_tag) /* Set the TEMPLATE_DECL's type. */ @@ -8218,7 +8217,7 @@ trees_in::decl_value () /* Set the TEMPLATE_DECL's type. */ TREE_TYPE (decl) = TREE_TYPE (inner); - if (!is_matching_decl (existing, decl)) + if (!is_matching_decl (existing, decl, is_typedef)) unmatched_duplicate (existing); /* And our result is the existing node. */ @@ -8257,8 +8256,8 @@ trees_in::decl_value () if (inner && !NAMESPACE_SCOPE_P (inner) && ((TREE_CODE (inner) == TYPE_DECL - && TYPE_NAME (TREE_TYPE (inner)) == inner - && !is_typedef) + && !is_typedef + && TYPE_NAME (TREE_TYPE (inner)) == inner) || TREE_CODE (inner) == FUNCTION_DECL) && u ()) read_definition (decl); @@ -11088,7 +11087,7 @@ trees_in::binfo_mergeable (tree *type) decls_match because it can cause instantiations of constraints. */ bool -trees_in::is_matching_decl (tree existing, tree decl) +trees_in::is_matching_decl (tree existing, tree decl, bool is_typedef) { // FIXME: We should probably do some duplicate decl-like stuff here // (beware, default parms should be the same?) Can we just call @@ -11099,35 +11098,36 @@ trees_in::is_matching_decl (tree existing, tree decl) // can elide some of the checking gcc_checking_assert (TREE_CODE (existing) == TREE_CODE (decl)); - tree inner = decl; + tree d_inner = decl; + tree e_inner = existing; if (TREE_CODE (decl) == TEMPLATE_DECL) { - inner = DECL_TEMPLATE_RESULT (decl); - gcc_checking_assert (TREE_CODE (DECL_TEMPLATE_RESULT (existing)) - == TREE_CODE (inner)); + d_inner = DECL_TEMPLATE_RESULT (d_inner); + e_inner = DECL_TEMPLATE_RESULT (e_inner); + gcc_checking_assert (TREE_CODE (e_inner) == TREE_CODE (d_inner)); } gcc_checking_assert (!map_context_from); /* This mapping requres the new decl on the lhs and the existing entity on the rhs of the comparitors below. */ - map_context_from = inner; - map_context_to = STRIP_TEMPLATE (existing); + map_context_from = d_inner; + map_context_to = e_inner; - if (TREE_CODE (inner) == FUNCTION_DECL) + if (TREE_CODE (d_inner) == FUNCTION_DECL) { tree e_ret = fndecl_declared_return_type (existing); tree d_ret = fndecl_declared_return_type (decl); - if (decl != inner && DECL_NAME (inner) == fun_identifier - && LAMBDA_TYPE_P (DECL_CONTEXT (inner)))
Re: c++: header unit template alias merging [PR 98770]
On 1/28/21 7:54 AM, Nathan Sidwell wrote: Typedefs are streamed by streaming the underlying type, and then recreating the typedef. But this breaks checking a duplicate is the same as the original when it is a template alias -- we end up checking a template alias (eg __void_t) against the underlying type (void). And those are not the same template alias. This stops pretendig that the underlying type is the typedef for that checking and tells is_matching_decl 'you have a typedef', so it knows what to do. (We do not want to recreate the typedef of the duplicate, because that whole set of nodes is going to go away.) d'oh! PR c++/98770 gcc/cp/ * module.cc (trees_out::decl_value): Swap is_typedef & TYPE_NAME check order. (trees_in::decl_value): Do typedef frobbing only when installing a new typedef, adjust is_matching_decl call. Swap is_typedef & TYPE_NAME check. (trees_in::is_matching_decl): Add is_typedef parm. Adjust variable names and deal with typedef checking. gcc/testsuite/ * g++.dg/modules/pr98770_a.C: New. * g++.dg/modules/pr98770_b.C: New. -- Nathan Sidwell
c++: Fix unordered entity array [PR 98843]
A couple of module invariants are that the modules are always allocated in ascending order and appended to the module array. The entity array is likewise ordered, with each module having spans in that array in ascending order. Prior to header-units, this was provided by the way import declarations were encountered. With header-units we need to load the preprocessor state of header units before we parse the C++, and this can lead to incorrect ordering of the entity array. I had made the initialization of a module's language state a little too lazy. This moves the allocation of entity array spans into the initial read of a module, thus ensuring the ordering of those spans. We won't be looking in them until we've loaded the language portions of that particular module, and even if we did, we'd find NULLs there and issue a diagnostic. PR c++/98843 gcc/cp/ * module.cc (module_state_config): Add num_entities field. (module_state::read_entities): The entity_ary span is already allocated. (module_state::write_config): Write num_entities. (module_state::read_config): Read num_entities. (module_state::write): Set config's num_entities. (module_state::read_initial): Allocate the entity ary span here. (module_state::read_language): Do not set entity_lwm here. gcc/testsuite/ * g++.dg/modules/pr98843_a.C: New. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index daf75b16007..2d761452505 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -4064,6 +4064,7 @@ import_entity_module (unsigned index) /* This is an index for an exported entity. */ return (*modules)[0]; + /* Do not include the current TU (not an off-by-one error). */ unsigned pos = 1; unsigned len = modules->length () - pos; while (len) @@ -14480,6 +14481,7 @@ struct module_state_config { const char *dialect_str; unsigned num_imports; unsigned num_partitions; + unsigned num_entities; unsigned ordinary_locs; unsigned macro_locs; unsigned ordinary_loc_align; @@ -14487,7 +14489,7 @@ struct module_state_config { public: module_state_config () :dialect_str (get_dialect ()), - num_imports (0), num_partitions (0), + num_imports (0), num_partitions (0), num_entities (0), ordinary_locs (0), macro_locs (0), ordinary_loc_align (0) { } @@ -15286,9 +15288,7 @@ module_state::read_entities (unsigned count, unsigned lwm, unsigned hwm) dump () && dump ("Reading entities"); dump.indent (); - vec_safe_reserve (entity_ary, count); - unsigned ix; - for (ix = 0; ix != count; ix++) + for (binding_slot *slot = entity_ary->begin () + entity_lwm; count--; slot++) { unsigned snum = sec.u (); if (snum && (snum - lwm) >= (hwm - lwm)) @@ -15296,13 +15296,9 @@ module_state::read_entities (unsigned count, unsigned lwm, unsigned hwm) if (sec.get_overrun ()) break; - binding_slot slot; - slot.u.binding = NULL_TREE; if (snum) - slot.set_lazy (snum << 2); - entity_ary->quick_push (slot); + slot->set_lazy (snum << 2); } - entity_num = ix; dump.outdent (); if (!sec.end (from ())) @@ -17301,6 +17297,7 @@ module_state::write_config (elf_out *to, module_state_config &config, cfg.u (config.num_imports); cfg.u (config.num_partitions); + cfg.u (config.num_entities); cfg.u (config.ordinary_locs); cfg.u (config.macro_locs); @@ -17484,6 +17481,7 @@ module_state::read_config (module_state_config &config) config.num_imports = cfg.u (); config.num_partitions = cfg.u (); + config.num_entities = cfg.u (); config.ordinary_locs = cfg.u (); config.macro_locs = cfg.u (); @@ -17717,6 +17715,7 @@ module_state::write (elf_out *to, cpp_reader *reader) /* Write the entitites. None happens if we contain namespaces or nothing. */ + config.num_entities = counts[MSC_entities]; if (counts[MSC_entities]) write_entities (to, sccs, counts[MSC_entities], &crc); @@ -17818,6 +17817,21 @@ module_state::read_initial (cpp_reader *reader) gcc_checking_assert (mod == MODULE_UNKNOWN); gcc_checking_assert (this != (*modules)[0]); + { +/* Allocate space in the entities array now -- that array must be + monotionically in step with the modules array. */ +entity_lwm = vec_safe_length (entity_ary); +entity_num = config.num_entities; +gcc_checking_assert (modules->length () == 1 + || modules->last ()->entity_lwm <= entity_lwm); +vec_safe_reserve (entity_ary, config.num_entities); + +binding_slot slot; +slot.u.binding = NULL_TREE; +for (unsigned count = config.num_entities; count--;) + entity_ary->quick_push (slot); + } + /* We'll run out of other resources before we run out of module indices. */ mod = modules->
Re: c++: cross-module __cxa_atexit use [PR 98531]
As Rainer pointed out, there were some regressions in the library tests. That's because we didn't build the correct ehspec for __cxa_atexit. This adds that, but also, I realize we can use the 'hidden' flag on pushdecl to make this lazy-builtin not visible to user name lookup without them already declaring it. And I realized we should be setting the location as BUILTIN_LOCATION because that's what we're doing. Finally the duplicate_decl processing needed augmenting to allow such a lazy builtin's eh spec to differ from one already known. (The converse is already dealt with, as that's the only case we had before.) As I mentioned in the first patch, a cleanup of the 'lazily declare a builtin' API would be a nice stage-1 thing. I'll hold off applying this until next week. Rainer, I don't yet know if this resolves all of the issues you encountered in the PR. On 1/25/21 1:01 PM, Nathan Sidwell wrote: [ARM has unique ABI here, hence cc'ing Richard] Solaris tickled this bug as it has some mutex/sync/something primitive with a destructor, hence wanted to generate a __cxa_atexit call inside an inline/template function. But the problem is not solaris-specific. I tested this bootstrapping both x86_64-linux and aarch64-linux. I'll commit in a couple of days if there are no further comments. The compiler's use of lazily-declared library functions must insert said functions into a symbol table, so that they can be correctly merged across TUs at the module-level. We have too many different ways of declaring such library functions. This fixes __cxa_atexit (or its system-specific variations), pushing (or merging) the decl into the appropriate namespace. PR c++/98531 gcc/cp/ * cp-tree.h (push_abi_namespace, pop_abi_namespace): Declare. * decl.c (push_abi_namespace, pop_abi_namespace): Moved from rtti.c, add default namespace arg. (declare_global_var): Use push/pop_abi_namespace. (get_atexit_node): Push the fndecl into a namespace. * rtti.c (push_abi_namespace, pop_abi_namespace): Moved to decl.c. gcc/testsuite/ * g++.dg/modules/pr98531.h: New. * g++.dg/modules/pr98531_a.H: New. * g++.dg/modules/pr98531_b.C: New. -- Nathan Sidwell diff --git c/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index f31319904eb..bb06b6e44a9 100644 --- c/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -193,7 +193,9 @@ enum cp_tree_index CPTI_MODULE_HWM, /* Nodes after here change during compilation, or should not be in - the module's global tree table. */ + the module's global tree table. Such nodes must be locatable + via name lookup or type-construction, as those are the only + cross-TU matching capabilities remaining. */ /* We must find these via the global namespace. */ CPTI_STD, @@ -6622,6 +6624,9 @@ extern tree make_typename_type (tree, tree, enum tag_types, tsubst_flags_t); extern tree build_typename_type (tree, tree, tree, tag_types); extern tree make_unbound_class_template (tree, tree, tree, tsubst_flags_t); extern tree make_unbound_class_template_raw (tree, tree, tree); +extern unsigned push_abi_namespace (tree node = abi_node); +extern void pop_abi_namespace (unsigned flags, + tree node = abi_node); extern tree build_library_fn_ptr (const char *, tree, int); extern tree build_cp_library_fn_ptr (const char *, tree, int); extern tree push_library_fn (tree, tree, tree, int); diff --git c/gcc/cp/decl.c w/gcc/cp/decl.c index 1a114a2e2d0..edabeb989b3 100644 --- c/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -1209,7 +1209,8 @@ check_redeclaration_exception_specification (tree new_decl, all declarations, including the definition and an explicit specialization, of that function shall have an exception-specification with the same set of type-ids. */ - if (! DECL_IS_UNDECLARED_BUILTIN (old_decl) + if (!DECL_IS_UNDECLARED_BUILTIN (old_decl) + && !DECL_IS_UNDECLARED_BUILTIN (new_decl) && !comp_except_specs (new_exceptions, old_exceptions, ce_normal)) { const char *const msg @@ -4696,6 +4697,30 @@ cxx_init_decl_processing (void) using_eh_for_cleanups (); } +/* Enter an abi node in global-module context. returns a cookie to + give to pop_abi_namespace. */ + +unsigned +push_abi_namespace (tree node) +{ + push_nested_namespace (node); + push_visibility ("default", 2); + unsigned flags = module_kind; + module_kind = 0; + return flags; +} + +/* Pop an abi namespace, FLAGS is the cookie push_abi_namespace gave + you. */ + +void +pop_abi_namespace (unsigned flags, tree node) +{ + module_kind = flags; + pop_visibility (2); + pop_nested_namespace (node); +} + /* Create the VAR_DECL for __FUNCTION__ etc. ID is the name to give
Re: c++: cross-module __cxa_atexit use [PR 98531]
[Dropping Richard, as I don't think his input is necessary further] Hi Rainer, here are two patches, to be applied in sequence (replacing earlier versions of the patch). The first is merely a testsuite correction of the earlier patch to inhibit the cxa_atexit-specific tests where it matters. (I don't think it matters in the ABI ones, if the library does not provide them, we won't hit a declaration that could differ) The second fixes the atexit issue (and array entity with cxa_atexit) you encountered on solaris 11.3. With these two patches I could build the xtreme-header-2_a.ii you provided on an i386-solaris2.11 xcompiler (I can only compile there, not got a complete runtime to run the tests). Please let me know how these behave, thanks. patch 3a: c++: cross-module __cxa_atexit use [PR 98531] The compiler's use of lazily-declared library functions must insert said functions into a symbol table, so that they can be correctly merged across TUs at the module-level. We have too many different ways of declaring such library functions. This fixes __cxa_atexit (or its system-specific variations), pushing (or merging) the decl into the appropriate namespace. Because we're pushing a lazy builtin, check_redeclaration_exception_specification needed a tweak to allow a such a builtin's eh spec to differ from what the user may have already declared. (I suspect no all headers declare atexit as noexcept.) We can't test the -fno-use-cxa-atexit path with modules, as that requires a followup patch to a closely related piece (which also affects cxa_atexit targets in other circumstances). PR c++/98531 gcc/cp/ * cp-tree.h (push_abi_namespace, pop_abi_namespace): Declare. * decl.c (push_abi_namespace, pop_abi_namespace): Moved from rtti.c, add default namespace arg. (check_redeclaration_exception_specification): Allow a lazy builtin's eh spec to differ from an lready-declared user declaration. (declare_global_var): Use push/pop_abi_namespace. (get_atexit_node): Push the fndecl into a namespace. * rtti.c (push_abi_namespace, pop_abi_namespace): Moved to decl.c. gcc/testsuite/ * g++.dg/modules/pr98531-1.h: New. * g++.dg/modules/pr98531-1_a.H: New. * g++.dg/modules/pr98531-1_b.C: New. * g++.dg/abi/pr98531-1.C: New. * g++.dg/abi/pr98531-2.C: New. * g++.dg/abi/pr98531-3.C: New. * g++.dg/abi/pr98531-4.C: New. patch 3b: c++: cleanup function name [PR 98531] The next piece of 98531 is that in some cases we need to create a cleanup function to do the work (when the object is an array, or we're using regular atexit). We were not pushing that function's decl anywhere (not giving it a context) so streaming it failed. This is a partial fix. You'll notice we're naming these from a per-TU counter. I've captured that in PR98893. gcc/cp/ * decl.c (start_cleanup_fn): Push function into namespace. gcc/testsuite/ * g++.dg/modules/pr98531-2.h: New. * g++.dg/modules/pr98531-2_a.H: New. * g++.dg/modules/pr98531-2_b.C: New. * g++.dg/modules/pr98531-3.h: New. * g++.dg/modules/pr98531-3_a.H: New. * g++.dg/modules/pr98531-3_b.C: New. -- Nathan Sidwell diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h index f31319904eb..bb06b6e44a9 100644 --- a/gcc/cp/cp-tree.h +++ b/gcc/cp/cp-tree.h @@ -193,7 +193,9 @@ enum cp_tree_index CPTI_MODULE_HWM, /* Nodes after here change during compilation, or should not be in - the module's global tree table. */ + the module's global tree table. Such nodes must be locatable + via name lookup or type-construction, as those are the only + cross-TU matching capabilities remaining. */ /* We must find these via the global namespace. */ CPTI_STD, @@ -6622,6 +6624,9 @@ extern tree make_typename_type (tree, tree, enum tag_types, tsubst_flags_t); extern tree build_typename_type (tree, tree, tree, tag_types); extern tree make_unbound_class_template (tree, tree, tree, tsubst_flags_t); extern tree make_unbound_class_template_raw (tree, tree, tree); +extern unsigned push_abi_namespace (tree node = abi_node); +extern void pop_abi_namespace (unsigned flags, + tree node = abi_node); extern tree build_library_fn_ptr (const char *, tree, int); extern tree build_cp_library_fn_ptr (const char *, tree, int); extern tree push_library_fn (tree, tree, tree, int); diff --git a/gcc/cp/decl.c b/gcc/cp/decl.c index 1a114a2e2d0..edabeb989b3 100644 --- a/gcc/cp/decl.c +++ b/gcc/cp/decl.c @@ -1209,7 +1209,8 @@ check_redeclaration_exception_specification (tree new_decl, all declarations, including the defi
driver: error for nonexistent linker inputs [PR 98943]
We used to check all unknown input files, even when passing them to a compiler. But that caused problems. However, not erroring out on non-existent would-be-linker inputs confuses configure machinery that probes the compiler to see if it accepts various inputs. This restores the access check for things that are thought to be linker input files, when we're not linking. (If we are linking, we presume the linker will error out on its own accord.) We get a warning about not linking, and then an error about the file not being found. This seemed marginally better than a single error message conveying both pieces of information. devvm1702:30>./xg++ -B./ NOTAFILE -x c /dev/null -c xg++: warning: NOTAFILE: linker input file unused because linking not done xg++: error: NOTAFILE: linker input file not found: No such file or directory PR driver/98943 gcc/ * gcc.c (driver::maybe_run_linker): Check for input file accessibility if not linking. gcc/testsuite/ * c-c++-common/pr98943.c: New. booted & tested on x86_64-linux, ok? nathan -- Nathan Sidwell diff --git c/gcc/gcc.c w/gcc/gcc.c index aa5774af7e7..96381a77dee 100644 --- c/gcc/gcc.c +++ w/gcc/gcc.c @@ -9020,8 +9020,15 @@ driver::maybe_run_linker (const char *argv0) const for (i = 0; (int) i < n_infiles; i++) if (explicit_link_files[i] && !(infiles[i].language && infiles[i].language[0] == '*')) - warning (0, "%s: linker input file unused because linking not done", - outfiles[i]); + { + warning (0, "%s: linker input file unused because linking not done", + outfiles[i]); + if (access (outfiles[i], F_OK) < 0) + /* This is can be an indication the user specifed an errorneous + separated option value, (or used the wrong prefix for an + option). */ + error ("%s: linker input file not found: %m", outfiles[i]); + } } /* The end of "main". */ diff --git c/gcc/testsuite/c-c++-common/pr98943.c w/gcc/testsuite/c-c++-common/pr98943.c new file mode 100644 index 000..53d8838f242 --- /dev/null +++ w/gcc/testsuite/c-c++-common/pr98943.c @@ -0,0 +1,10 @@ +// { dg-do compile } +// PR 98943, compiler feature tests can get confused by not linking +// { dg-options "NOTAFILE" } + +int main () +{ + return 0; +} + +// { dg-regexp {[^\n:]*: warning: NOTAFILE: linker input file unused because linking not done\n[^\n:]*: error: NOTAFILE: linker input file not found: [^\n]*\n} }
c++: Fix indirect partitions [PR 98944]
The most recent reimplementation of module loading initialization changed the behaviour of setting an import's location, and broke some partition handling. PR c++/98944 gcc/cp/ * module.cc (module_state::is_rooted): Rename to ... (module_state::has_location): ... here. Adjust callers. (module_state::read_partitions): Adjust validity check. Don't overwrite a known location. gcc/testsuite/ * g++.dg/modules/pr98944_a.C: New. * g++.dg/modules/pr98944_b.C: New. * g++.dg/modules/pr98944_c.C: New. * g++.dg/modules/pr98944_d.C: New. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 41ce2011525..0749db8fe94 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -3608,8 +3608,8 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state { } public: - /* Is this not a real module? */ - bool is_rooted () const + /* Is this a real module? */ + bool has_location () const { return loc != UNKNOWN_LOCATION; } @@ -4416,7 +4416,7 @@ dumper::operator () (const char *format, ...) const char *str = "(none)"; if (module_state *m = va_arg (args, module_state *)) { - if (!m->is_rooted ()) + if (!m->has_location ()) str = "(detached)"; else str = m->get_flatname (); @@ -14441,16 +14441,17 @@ module_state::read_partitions (unsigned count) dump () && dump ("Reading elided partition %s (crc=%x)", name, crc); module_state *imp = get_module (name); - if (!imp || !imp->is_partition () || imp->is_rooted () - || get_primary (imp) != this) + if (!imp /* Partition should be ... */ + || !imp->is_partition () /* a partition ... */ + || imp->loadedness != ML_NONE /* that is not yet loaded ... */ + || get_primary (imp) != this) /* whose primary is this. */ { sec.set_overrun (); break; } - /* Attach the partition without loading it. We'll have to load - for real if it's indirectly imported. */ - imp->loc = floc; + if (!imp->has_location ()) + imp->loc = floc; imp->crc = crc; if (!imp->filename && fname[0]) imp->filename = xstrdup (fname); @@ -18857,7 +18858,7 @@ direct_import (module_state *import, cpp_reader *reader) timevar_start (TV_MODULE_IMPORT); unsigned n = dump.push (import); - gcc_checking_assert (import->is_direct () && import->is_rooted ()); + gcc_checking_assert (import->is_direct () && import->has_location ()); if (import->loadedness == ML_NONE) if (!import->do_import (reader, true)) gcc_unreachable (); @@ -18904,7 +18905,7 @@ import_module (module_state *import, location_t from_loc, bool exporting_p, linemap_module_reparent (line_table, import->loc, from_loc); } gcc_checking_assert (!import->module_p); - gcc_checking_assert (import->is_direct () && import->is_rooted ()); + gcc_checking_assert (import->is_direct () && import->has_location ()); direct_import (import, reader); } @@ -18934,7 +18935,7 @@ declare_module (module_state *module, location_t from_loc, bool exporting_p, } gcc_checking_assert (module->module_p); - gcc_checking_assert (module->is_direct () && module->is_rooted ()); + gcc_checking_assert (module->is_direct () && module->has_location ()); /* Yer a module, 'arry. */ module_kind &= ~MK_GLOBAL; diff --git c/gcc/testsuite/g++.dg/modules/pr98944_a.C w/gcc/testsuite/g++.dg/modules/pr98944_a.C new file mode 100644 index 000..9475317dc82 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr98944_a.C @@ -0,0 +1,9 @@ +// PR 98944, the example in [module.unit]/4 +// { dg-additional-options -fmodules-ts } + +// tu3 + +module A:Internals; +// { dg-module-cmi A:Internals } + +int bar(); diff --git c/gcc/testsuite/g++.dg/modules/pr98944_b.C w/gcc/testsuite/g++.dg/modules/pr98944_b.C new file mode 100644 index 000..209eafccc76 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr98944_b.C @@ -0,0 +1,8 @@ +// { dg-additional-options -fmodules-ts } + +// tu2 +export module A:Foo; +// { dg-module-cmi A:Foo } + +import :Internals; +export int foo() { return 2 * (bar() + 1); } diff --git c/gcc/testsuite/g++.dg/modules/pr98944_c.C w/gcc/testsuite/g++.dg/modules/pr98944_c.C new file mode 100644 index 000..90be60f2629 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr98944_c.C @@ -0,0 +1,8 @@ +// { dg-additional-options -fmodules-ts } + +// tu1 +export module A; +// { dg-module-cmi A } + +export import :Foo; +export int baz(); diff --git c/gcc/testsuite/g++.dg/modules/pr98944_d.C w/gcc/testsuite/g++.dg/modules/pr98944_d.C new file mode 100644 index 000..25364ab9aae --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr98944_d.C @@ -0,0 +1,8 @@ +// { dg-additional-options -fmodules-ts } + +// tu4 +module A; + +import :Internals; +int bar() { return baz() - 10; } +int baz() { return 30; }
c++: generic lambdas and local-externs from outer scopes [PR 99030]
Lambdas can refer to local externs from their enclosing scope. When the lambda's generic but the containing function is not a temploid, we'll never have tsubsted the declaring decl so won't have a local specialization. But in that case we can just use the decl we tsubsting directly -- it's not dependent. PR c++/99030 gcc/cp * pt.c (tsubst_copy) [VAR_DECL]: For a DECL_LOCAL_DECL_P T is the answer if there's no local specialization. gcc/testsuite/ * g++.dg/lookup/pr99030.C: New. -- Nathan Sidwell diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c index f73deb3aee3..d8574f649b2 100644 --- c/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -16650,11 +16650,16 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) r = tsubst (t, args, complain, in_decl); else if (DECL_LOCAL_DECL_P (t)) { - /* Local specialization will have been created when we - instantiated the DECL_EXPR_DECL. */ + /* Local specialization will usually have been created when + we instantiated the DECL_EXPR_DECL. */ r = retrieve_local_specialization (t); if (!r) - r = error_mark_node; + { + /* We're in a generic lambda referencing a local extern + from an outer block-scope of a non-template. */ + gcc_checking_assert (LAMBDA_FUNCTION_P (current_function_decl)); + r = t; + } } else if (local_variable_p (t) && uses_template_parms (DECL_CONTEXT (t))) diff --git c/gcc/testsuite/g++.dg/lookup/pr99030.C w/gcc/testsuite/g++.dg/lookup/pr99030.C new file mode 100644 index 000..080847ca11c --- /dev/null +++ w/gcc/testsuite/g++.dg/lookup/pr99030.C @@ -0,0 +1,16 @@ +// PR 99030 ICE with generic lambda accessing local extern +// { dg-do compile { target c++14 } } + +void foo () +{ + extern int a; + [] (auto b) { a; } (1); +} + +template void bar () +{ + extern T a; + [] (auto b) { a; } (1); +} + +template void bar ();
Expunge namespace-scope IDENTIFIER_TYPE_VALUE & global_type_name [PR 99039]
IDENTIFIER_TYPE_VALUE and friends is a remnant of G++'s C origins. It holds elaborated types on identifier-nodes. While this is fine for C and for local and class-scopes in C++, it fails badly for namespaces. In that case a marker 'global_type_node' was used, which essentially signified 'this is a namespace-scope type *somewhere*', and you'd have to do a regular name_lookup to find it. As the parser and substitution machinery has avanced over the last 25 years or so, there's not much outside of actual name-lookup that uses that. Amusingly the IDENTIFIER_HAS_TYPE_VALUE predicate will do an actual name-lookup and then users would repeat that lookup to find the now-known to be there type. Rather late I realized that this interferes with the lazy loading of module entities, because we were setting IDENTIFIER_TYPE_VALUE to global_type_node. But we could be inside some local scope where that identifier is bound to some local type. Not good! Rather than add more cruft to look at an identifier's shadow stack and alter that as necessary, this takes the approach of removing the existing cruft. We nuke the few places outside of name lookup that use IDENTIFIER_TYPE_VALUE. Replacing them with either proper name lookups, alternative sequences, or in some cases asserting that they (no longer) happen. Class template instantiation was calling pushtag after setting IDENTIFIER_TYPE_VALUE in order to stop pushtag creating an implicit typedef and pushing it, but to get the bookkeeping it needed. Let's just do the bookkeeping directly. Then we can stop having a 'bound at namespace-scope' marker at all, which means lazy loading won't screw up local shadow stacks. Also, it simplifies set_identifier_type_value_with_scope, as it never needs to inspect the scope stack. When developing this patch, I discovered a number of places we'd put an actual namespace-scope type on the type_value slot, rather than global_type_node. You might notice this is killing at least two 'why are we doing this?' comments. While this doesn't fix the two PRs mentioned, it is a necessary step. PR c++/99039 PR c++/99040 gcc/cp/ * cp-tree.h (CPTI_GLOBAL_TYPE): Delete. (global_type_node): Delete. (IDENTIFIER_TYPE_VALUE): Delete. (IDENTIFIER_HAS_TYPE_VALUE): Delete. (get_type_value): Delete. * name-lookup.h (identifier_type_value): Delete. * name-lookup.c (check_module_override): Don't SET_IDENTIFIER_TYPE_VALUE here. (do_pushdecl): Nor here. (identifier_type_value_1, identifier_type_value): Delete. (set_identifier_type_value_with_scope): Only SET_IDENTIFIER_TYPE_VALUE for local and class scopes. (pushdecl_nanmespace_level): Remove shadow stack nadgering. (do_pushtag): Use REAL_IDENTIFIER_TYPE_VALUE. * call.c (check_dtor_name): Use lookup_name. * decl.c (cxx_init_decl_processing): Drop global_type_node. * decl2.c (cplus_decl_attributes): Don't SET_IDENTIFIER_TYPE_VALUE here. * init.c (get_type_value): Delete. * pt.c (instantiate_class_template_1): Don't call pushtag or SET_IDENTIFIER_TYPE_VALUE here. (tsubst): Assert never an identifier. (dependent_type_p): Drop global_type_node assert. * typeck.c (error_args_num): Don't use IDENTIFIER_HAS_TYPE_VALUE to determine ctorness. gcc/testsuite/ * g++.dg/lookup/pr99039.C: New. -- Nathan Sidwell diff --git c/gcc/cp/call.c w/gcc/cp/call.c index 4744c9768ec..186feef6fe3 100644 --- c/gcc/cp/call.c +++ w/gcc/cp/call.c @@ -236,8 +236,15 @@ check_dtor_name (tree basetype, tree name) || TREE_CODE (basetype) == ENUMERAL_TYPE) && name == constructor_name (basetype)) return true; - else - name = get_type_value (name); + + /* Otherwise lookup the name, it could be an unrelated typedef + of the correct type. */ + name = lookup_name (name, LOOK_want::TYPE); + if (!name) + return false; + name = TREE_TYPE (name); + if (name == error_mark_node) + return false; } else { @@ -252,8 +259,6 @@ check_dtor_name (tree basetype, tree name) return false; } - if (!name || name == error_mark_node) -return false; return same_type_p (TYPE_MAIN_VARIANT (basetype), TYPE_MAIN_VARIANT (name)); } diff --git c/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index 4ed3936ade2..38b31e3908f 100644 --- c/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -129,7 +129,6 @@ enum cp_tree_index CPTI_VTBL_TYPE, CPTI_VTBL_PTR_TYPE, CPTI_GLOBAL, -CPTI_GLOBAL_TYPE, CPTI_ABORT_FNDECL, CPTI_AGGR_TAG, CPTI_CONV_OP_MARKER, @@ -250,7 +249,6 @@ extern GTY(()) tree
c++: Register streamed-in decls when new [PR 99040]
With modules one can have using-decls refering to their own scope. This is the way to export things from the GMF or from an import. The problem was I was using current_ns == CP_DECL_CONTEXT (decl) to determine whether a decl should be registered in a namespace level or not. But that's an inadequate check and we ended up reregistering decls and creating a circular list. We should be registering the decl when first encountered -- whether we bind it is orthogonal to that. PR c++/99040 gcc/cp/ * module.cc (trees_in::decl_value): Call add_module_namespace_decl for new namespace-scope entities. (module_state::read_cluster): Don't call add_module_decl here. * name-lookup.h (add_module_decl): Rename to ... (add_module_namespace_decl): ... this. * name-lookup.c (newbinding_bookkeeping): Move into ... (do_pushdecl): ... here. Its only remaining caller. (add_module_decl): Rename to ... (add_module_namespace_decl): ... here. Add checking-assert for circularity. Don't call newbinding_bookkeeping, just extern_c checking and incomplete var checking. gcc/testsuite/ * g++.dg/modules/pr99040_a.C: New. * g++.dg/modules/pr99040_b.C: New. * g++.dg/modules/pr99040_c.C: New. * g++.dg/modules/pr99040_d.C: New. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 0749db8fe94..37ccddc74a5 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -8162,6 +8162,12 @@ trees_in::decl_value () /* Set the TEMPLATE_DECL's type. */ TREE_TYPE (decl) = TREE_TYPE (inner); + if (NAMESPACE_SCOPE_P (decl) + && (mk == MK_named || mk == MK_unique + || mk == MK_enum || mk == MK_friend_spec) + && !(VAR_OR_FUNCTION_DECL_P (decl) && DECL_LOCAL_DECL_P (decl))) + add_module_namespace_decl (CP_DECL_CONTEXT (decl), decl); + /* The late insertion of an alias here or an implicit member (next block), is ok, because we ensured that all imports were loaded up before we started this cluster. Thus an insertion @@ -14893,20 +14899,6 @@ module_state::read_cluster (unsigned snum) : 0, decls, type, visible)) sec.set_overrun (); - - if (type - && CP_DECL_CONTEXT (type) == ns - && !sec.is_duplicate (type)) - add_module_decl (ns, name, type); - - for (ovl_iterator iter (decls); iter; ++iter) - if (!iter.using_p ()) - { - tree decl = *iter; - if (CP_DECL_CONTEXT (decl) == ns - && !sec.is_duplicate (decl)) - add_module_decl (ns, name, decl); - } } break; diff --git c/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index 8aa490db634..5aa206d40d4 100644 --- c/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -382,7 +382,8 @@ add_decl_to_level (cp_binding_level *b, tree decl) /* Make sure we don't create a circular list. xref_tag can end up pushing the same artificial decl more than once. We - should have already detected that in update_binding. */ + should have already detected that in update_binding. (This isn't a + complete verification of non-circularity.) */ gcc_assert (b->names != decl); /* We build up the list in reverse order, and reverse it later if @@ -3496,41 +3497,6 @@ implicitly_export_namespace (tree ns) } } -/* DECL has just been bound at LEVEL. finish up the bookkeeping. */ - -static void -newbinding_bookkeeping (tree name, tree decl, cp_binding_level *level) -{ - if (TREE_CODE (decl) == TYPE_DECL) -{ - tree type = TREE_TYPE (decl); - - if (type != error_mark_node) - { - if (TYPE_NAME (type) != decl) - set_underlying_type (decl); - - set_identifier_type_value_with_scope (name, decl, level); - - if (level->kind != sk_namespace - && !instantiating_current_function_p ()) - /* This is a locally defined typedef in a function that - is not a template instantation, record it to implement - -Wunused-local-typedefs. */ - record_locally_defined_typedef (decl); - } -} - else -{ - if (VAR_P (decl) && !DECL_LOCAL_DECL_P (decl)) - maybe_register_incomplete_var (decl); - - if (VAR_OR_FUNCTION_DECL_P (decl) - && DECL_EXTERN_C_P (decl)) - check_extern_c_conflict (decl); -} -} - /* DECL is a global or module-purview entity. If it has non-internal linkage, and we have a module vector, record it in the appropriate slot. We have already checked for duplicates. */ @@ -3839,12 +3805,38 @@ do_pushdecl (tree decl, bool hiding) decl = old; else { - newbinding_bookkeeping (name, decl, level); + if (TREE_CODE (decl) == TYPE_DECL) + { + tree type = TREE_TYPE (decl); + + if (type != error_mark_node) + { + if (TYPE_NAME (type) != decl) +
c++: Seed imported bindings [PR 99039]
As mentioned in 99040's fix, we can get inter-module using decls. If the using decl is the only reference to an import, we'll have failed to seed our imports leading to an assertion failure. The fix is straight-forwards, check binding contents when seeding imports. gcc/cp/ * module.cc (module_state::write_cluster): Check bindings for imported using-decls. gcc/testsuite/ * g++.dg/modules/pr99039_a.C: New. * g++.dg/modules/pr99039_b.C: New. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 37ccddc74a5..520494f2543 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -3108,7 +3108,8 @@ private: unsigned section; #if CHECKING_P int importedness; /* Checker that imports not occurring - inappropriately. */ + inappropriately. +ve imports ok, + -ve imports not ok. */ #endif public: @@ -14632,13 +14633,36 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size, { depset *dep = b->deps[jx]; - if (!dep->is_binding () - && dep->is_import () && !TREE_VISITED (dep->get_entity ())) + if (dep->is_binding ()) { - tree import = dep->get_entity (); + /* A cross-module using decl could be here. */ + for (unsigned ix = dep->deps.length (); --ix;) + { + depset *bind = dep->deps[ix]; + if (bind->get_entity_kind () == depset::EK_USING + && bind->deps[1]->is_import ()) + { + tree import = bind->deps[1]->get_entity (); + if (!TREE_VISITED (import)) + { + sec.tree_node (import); + dump (dumper::CLUSTER) + && dump ("Seeded import %N", import); + } + } + } + /* Also check the namespace itself. */ + dep = dep->deps[0]; + } - sec.tree_node (import); - dump (dumper::CLUSTER) && dump ("Seeded import %N", import); + if (dep->is_import ()) + { + tree import = dep->get_entity (); + if (!TREE_VISITED (import)) + { + sec.tree_node (import); + dump (dumper::CLUSTER) && dump ("Seeded import %N", import); + } } } } diff --git c/gcc/testsuite/g++.dg/modules/pr99039_a.C w/gcc/testsuite/g++.dg/modules/pr99039_a.C new file mode 100644 index 000..56041e948db --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99039_a.C @@ -0,0 +1,9 @@ +// PR c++/99039 +// { dg-additional-options -fmodules-ts } +export module format; +// { dg-module-cmi format } + +export namespace NS +{ +void Format (); +} diff --git c/gcc/testsuite/g++.dg/modules/pr99039_b.C w/gcc/testsuite/g++.dg/modules/pr99039_b.C new file mode 100644 index 000..6fb76f584fa --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99039_b.C @@ -0,0 +1,9 @@ +// { dg-additional-options -fmodules-ts } +export module hello; +// { dg-module-cmi hello } +import format; + +export namespace NS +{ +using NS::Format; +}
c++: directives-only preprocessing and include translation [PR 99050]
We make sure files end in \n by placing one at the limit of the buffer (just past the end of what is read). We need to do the same for buffers generated via include-translation. Fortunately they have space. libcpp/ * files.c (_cpp_stack_file): Make buffers end in unread \n. gcc/testsuite/ * g++.dg/modules/pr99050_a.H: New. * g++.dg/modules/pr99050_b.C: New. -- Nathan Sidwell diff --git c/gcc/testsuite/g++.dg/modules/pr99050_a.H w/gcc/testsuite/g++.dg/modules/pr99050_a.H new file mode 100644 index 000..137e37f567f --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99050_a.H @@ -0,0 +1,4 @@ +// PR c++/99050 ICE with directives only +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } +void f (); diff --git c/gcc/testsuite/g++.dg/modules/pr99050_b.C w/gcc/testsuite/g++.dg/modules/pr99050_b.C new file mode 100644 index 000..439e216eb16 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99050_b.C @@ -0,0 +1,7 @@ +// { dg-do preprocess } +// { dg-additional-options {-fdirectives-only -fmodules-ts} } +#include "pr99050_a.H" + +int main () {} + +// { dg-final { scan-file pr99050_b.i {import "[^\n]*99050_a.H" \[\[__translated\]\];\n} } } diff --git c/libcpp/files.c w/libcpp/files.c index 5ea3f8e1bf3..3a35f7c9743 100644 --- c/libcpp/files.c +++ w/libcpp/files.c @@ -918,13 +918,17 @@ _cpp_stack_file (cpp_reader *pfile, _cpp_file *file, include_type type, because we don't usually need that location (we're popping an include file). However in this case we do want to do the increment. So push a writable buffer of two newlines to acheive - that. */ - static uchar newlines[] = "\n\n"; + that. (We also need an extra newline, so this looks like a regular + file, which we do that to to make sure we don't fall off the end in the + middle of a line. */ + static uchar newlines[] = "\n\n\n"; cpp_push_buffer (pfile, newlines, 2, true); + size_t len = strlen (buf); + buf[len] = '\n'; /* See above */ cpp_buffer *buffer = cpp_push_buffer (pfile, reinterpret_cast (buf), - strlen (buf), true); + len, true); buffer->to_free = buffer->buf; file->header_unit = +1;
c++: ICE with header-units [PR 99071]
This ICE was caused by dereferencing the wrong pointer and not finding the expected thing there. Pointers are like that. PR c++/99071 gcc/cp/ * name-lookup.c (maybe_record_mergeable_decl): Deref the correct pointer. gcc/testsuite/ * g++.dg/modules/pr99071_a.H: New. * g++.dg/modules/pr99071_b.H: New. -- Nathan Sidwell diff --git c/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index 5aa206d40d4..fda987e9616 100644 --- c/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -3525,7 +3525,7 @@ maybe_record_mergeable_decl (tree *slot, tree name, tree decl) if (!partition) { binding_slot &orig - = BINDING_VECTOR_CLUSTER (*gslot, 0).slots[BINDING_SLOT_CURRENT]; + = BINDING_VECTOR_CLUSTER (*slot, 0).slots[BINDING_SLOT_CURRENT]; if (!STAT_HACK_P (tree (orig))) orig = stat_hack (tree (orig)); diff --git c/gcc/testsuite/g++.dg/modules/pr99071_a.H w/gcc/testsuite/g++.dg/modules/pr99071_a.H new file mode 100644 index 000..44bc7c43601 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99071_a.H @@ -0,0 +1,6 @@ +// PR 99071 ICE with global-module merging +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +template +void begin (T *); diff --git c/gcc/testsuite/g++.dg/modules/pr99071_b.H w/gcc/testsuite/g++.dg/modules/pr99071_b.H new file mode 100644 index 000..1c773d74f12 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99071_b.H @@ -0,0 +1,8 @@ +// PR 99071 ICE with global-module merging +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +import "pr99071_a.H"; + +template +void begin(T &);
c++: More set_identifier_type_value fixing [PR 99116]
My recent change looked under template_parms in two places, but that was covering up a separate problem. We were attempting to set the identifier_type_value of a template_parm into the template_parm scope. The peeking stopped us doing that, but confused poplevel, leaving an identifier value lying around. This fixes the underlying problem in do_pushtag -- we only need to set the identifier_type_value directly when we're in a template_parm scope (a later pushdecl will push the actual template_decl). for non-class non-template-parm bindings do_pushdecl already ends up manipulating identifier_type_value correctly. PR c++/99116 gcc/cp/ * name-lookup.c (do_pushdecl): Don't peek under template_parm bindings here ... (set_identifier_type_value_with_scope): ... or here. (do_pushtag): Only set_identifier_type_value_with_scope at non-class template parm scope, and use parent scope. gcc/testsuite/ * g++.dg/lookup/pr99116-1.C: New. * g++.dg/lookup/pr99116-2.C: New. -- Nathan Sidwell diff --git c/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index 5aa206d40d4..0b0fb80c1ff 100644 --- c/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -3691,14 +3691,9 @@ do_pushdecl (tree decl, bool hiding) if (match == error_mark_node) ; else if (TREE_CODE (match) == TYPE_DECL) - { - auto *l = level; - while (l->kind == sk_template_parms) - l = l->level_chain; - gcc_checking_assert (REAL_IDENTIFIER_TYPE_VALUE (name) - == (l->kind == sk_namespace - ? NULL_TREE : TREE_TYPE (match))); - } + gcc_checking_assert (REAL_IDENTIFIER_TYPE_VALUE (name) + == (level->kind == sk_namespace + ? NULL_TREE : TREE_TYPE (match))); else if (iter.hidden_p () && !hiding) { /* Unhiding a previously hidden decl. */ @@ -4756,12 +4751,13 @@ print_binding_stack (void) static void set_identifier_type_value_with_scope (tree id, tree decl, cp_binding_level *b) { - while (b->kind == sk_template_parms) -b = b->level_chain; - if (b->kind == sk_namespace) /* At namespace scope we should not see an identifier type value. */ -gcc_checking_assert (!REAL_IDENTIFIER_TYPE_VALUE (id)); +gcc_checking_assert (!REAL_IDENTIFIER_TYPE_VALUE (id) + /* We could be pushing a friend underneath a template + parm (ill-formed). */ + || (TEMPLATE_PARM_P + (TYPE_NAME (REAL_IDENTIFIER_TYPE_VALUE (id); else { /* Push the current type value, so we can restore it later */ @@ -8257,10 +8253,8 @@ do_pushtag (tree name, tree type, TAG_how how) if (decl == error_mark_node) return decl; - bool in_class = false; if (b->kind == sk_class) { - in_class = true; if (!TYPE_BEING_DEFINED (current_class_type)) /* Don't push anywhere if the class is complete; a lambda in an NSDMI is not a member of the class. */ @@ -8275,7 +8269,12 @@ do_pushtag (tree name, tree type, TAG_how how) pushdecl_class_level (decl); } else if (b->kind == sk_template_parms) - in_class = b->level_chain->kind == sk_class; + { + /* Do not push the tag here -- we'll want to push the + TEMPLATE_DECL. */ + if (b->level_chain->kind != sk_class) + set_identifier_type_value_with_scope (name, tdef, b->level_chain); + } else { decl = do_pushdecl_with_scope @@ -8293,9 +8292,6 @@ do_pushtag (tree name, tree type, TAG_how how) } } - if (!in_class) - set_identifier_type_value_with_scope (name, tdef, b); - TYPE_CONTEXT (type) = DECL_CONTEXT (decl); /* If this is a local class, keep track of it. We need this diff --git c/gcc/testsuite/g++.dg/lookup/pr99116-1.C w/gcc/testsuite/g++.dg/lookup/pr99116-1.C new file mode 100644 index 000..01b483ea915 --- /dev/null +++ w/gcc/testsuite/g++.dg/lookup/pr99116-1.C @@ -0,0 +1,25 @@ +// PR 99116 sliding hidden friends under template parm scopes + +template struct Z { + + friend struct T; // { dg-error "shadows template parameter" } +}; + +struct Y { + + template struct A {}; + + friend struct S; +}; + +struct X +{ + struct S2 {}; + + struct In + { +friend struct S2; + }; +}; + +typedef int S2; diff --git c/gcc/testsuite/g++.dg/lookup/pr99116-2.C w/gcc/testsuite/g++.dg/lookup/pr99116-2.C new file mode 100644 index 000..2a4148bade8 --- /dev/null +++ w/gcc/testsuite/g++.dg/lookup/pr99116-2.C @@ -0,0 +1,19 @@ +// PR 99116, we need to remove the namespace-scope meaning of +// IDENTIFIER_TYPE_VALUE. + +namespace __gnu_cxx +{ +template +struct char_traits +{ + static void length(); +}; + +template +void char_traits<_T>::length () +{ +} + +} + +struct char_traits;
Re: [PATCH] c++: Fix up build_zero_init_1 once more [PR99106]
On 2/17/21 4:50 AM, Jakub Jelinek wrote: Hi! My earlier build_zero_init_1 patch for flexible array members created an empty CONSTRUCTOR. As the following testcase shows, that doesn't work very well because the middle-end doesn't expect CONSTRUCTOR elements with incomplete type (that the empty CONSTRUCTOR at the end of outer CONSTRUCTOR had). The following patch just doesn't add any CONSTRUCTOR for the flexible array members, it doesn't seem to be needed. lgtm -- yes, an array bound of -1 is certainly odd :) Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2021-02-17 Jakub Jelinek PR sanitizer/99106 * init.c (build_zero_init_1): For flexible array members just return NULL_TREE instead of returning empty CONSTRUCTOR with non-complete ARRAY_TYPE. * g++.dg/ubsan/pr99106.C: New test. --- gcc/cp/init.c.jj2021-02-12 23:57:30.501141871 +0100 +++ gcc/cp/init.c 2021-02-16 09:29:24.635069944 +0100 @@ -252,7 +252,7 @@ build_zero_init_1 (tree type, tree nelts build_one_cst (TREE_TYPE (nelts))); /* Treat flexible array members like [0] arrays. */ else if (TYPE_DOMAIN (type) == NULL_TREE) - max_index = build_minus_one_cst (sizetype); + return NULL_TREE; else max_index = array_type_nelts (type); --- gcc/testsuite/g++.dg/ubsan/pr99106.C.jj 2021-02-16 09:35:50.575679899 +0100 +++ gcc/testsuite/g++.dg/ubsan/pr99106.C2021-02-16 09:35:42.904767167 +0100 @@ -0,0 +1,5 @@ +// PR sanitizer/99106 +// { dg-do compile } +// { dg-options "-fsanitize=undefined" } + +#include "../ext/flexary38.C" Jakub -- Nathan Sidwell
Re: [PATCH] c++: Fix -std=c++20 ICE on virtual method call [PR99132]
On 2/18/21 3:30 AM, Jakub Jelinek wrote: Hi! On the following testcase we ICE in C++20 mode during cp_get_callee_fndecl -> constexpr evaluation. It is only in C++20 mode on this testcase because virtual methods can't be constexpr in C++17 and earlier and so potential_constant_expression_1 rejects it earlier. And the ICE is caused by genericization changing the h PARM_DECL from having B type to B & DECL_BY_REFERENCE and the constexpr evaluation not being able to deal with that. I think this just shows that we shouldn't do the constexpr evaluation during genericization and later, and other spots e.g. during gimplification also don't call cp_get_callee_fndecl but cp_get_callee_fndecl_nofold. Agreed, we've run into this kind of thing before. After all, cp_fold has already been run and it did the folding if there was any opportunity to do so. And furthermore, what that cp_genericize_r spot does is check for any left-over immediate function calls (which can be ATM just std::source_location::current() call) and immediate functions outside of immediate functions can't have addresses leaked into the IL, so it will be always a direct call anyway. And immediate functions themselves don't make it into genericization/gimplification. Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? ok, thanks 2021-02-18 Jakub Jelinek PR c++/99132 * cp-gimplify.c (cp_genericize_r) : Use cp_get_callee_fndecl_nofold instead of cp_get_callee_fndecl to check for immediate function calls. * g++.dg/cpp2a/constexpr-virtual18.C: New test. --- gcc/cp/cp-gimplify.c.jj 2021-02-17 16:14:42.621294819 +0100 +++ gcc/cp/cp-gimplify.c2021-02-17 16:48:30.343291226 +0100 @@ -1386,7 +1386,7 @@ cp_genericize_r (tree *stmt_p, int *walk break; } - if (tree fndecl = cp_get_callee_fndecl (stmt)) + if (tree fndecl = cp_get_callee_fndecl_nofold (stmt)) if (DECL_IMMEDIATE_FUNCTION_P (fndecl)) { gcc_assert (source_location_current_p (fndecl)); --- gcc/testsuite/g++.dg/cpp2a/constexpr-virtual18.C.jj 2021-02-17 16:48:18.136421425 +0100 +++ gcc/testsuite/g++.dg/cpp2a/constexpr-virtual18.C2021-02-17 16:49:55.929378398 +0100 @@ -0,0 +1,13 @@ +// PR c++/99132 +// { dg-do compile { target c++11 } } + +template struct A { T c; }; +template struct B { + A d; + constexpr T operator-> () { return d.c; } + B (B &&); +}; +struct C { + virtual void foo (); + void bar (B h) { h->foo (); } +}; Jakub -- Nathan Sidwell
Re: [r11-7271 Regression] FAIL: g++.dg/modules/pr99023_a.H (test for excess errors) on Linux/x86_64
On 2/18/21 3:03 AM, Jakub Jelinek wrote: On Wed, Feb 17, 2021 at 02:19:11PM -0800, sunil.k.pandey via Gcc-patches wrote: On Linux/x86_64, Yeah: FAIL: g++.dg/modules/pr99023_a.H (test for excess errors) Excess errors: ../../..//src/libstdc++-v3/libsupc++/initializer_list:47:11: fatal error: definition of 'class std::initializer_list<_E>' does not match '#include ' compilation terminated. sigh, I shouldn't have picked a header that is somewhat ABI-specific nathan -- Nathan Sidwell
c++: Remove large abi-specific tests [PR 99150]
Remove the two large and incorrectly abi-specific testcases I added. Replacement tests will be forthcoming. PR c++/99150 gcc/testsuite/ * g++.dg/modules/pr99023_a.H: Delete. * g++.dg/modules/pr99023_b.H: Delete. No patch because it exceeds the ml size limit :( -- Nathan Sidwell
c++: Remove obsolete dg-module-headers [PR 99023]
PR99023's testcase is highlighting some missing functionality of the modules test harness. I did have some partial support, but it's only use in one place for a now-obsolete test. This patch expunges that support so I can add better functionality now I understand better what is necessary. PR c++/99023 gcc/testsuite/ * modules.exp: Remove dg-module-headers support * alias-2_a.H: Delete. * sys/alias-2_a.H: Delete. -- Nathan Sidwell diff --git c/gcc/testsuite/g++.dg/modules/alias-2_a.H w/gcc/testsuite/g++.dg/modules/alias-2_a.H deleted file mode 100644 index 1befe85cf49..000 --- c/gcc/testsuite/g++.dg/modules/alias-2_a.H +++ /dev/null @@ -1,9 +0,0 @@ -// { dg-additional-options "-fmodule-header -isystem [srcdir]/sys" } -// { dg-module-cmi {} } -// { dg-module-headers test sys/alias-2_a.H } -#ifndef ALIAS_2_A -#define ALIAS_2_A - -int frob (); - -#endif diff --git c/gcc/testsuite/g++.dg/modules/modules.exp w/gcc/testsuite/g++.dg/modules/modules.exp index 28d627dcb86..c17120f2c00 100644 --- c/gcc/testsuite/g++.dg/modules/modules.exp +++ w/gcc/testsuite/g++.dg/modules/modules.exp @@ -41,7 +41,6 @@ dg-init global module_do global module_cmis -global module_headers set DEFAULT_REPO "gcm.cache" @@ -132,39 +131,6 @@ proc module_cmi_p { src ifs } { return $res } -# Append required header unit names to module_headers var -proc dg-module-headers { args } { -if { [llength $args] != 3 } { - error "[lindex $args 0]: wrong number of arguments" - return -} -} - -proc do_module_headers { srcdir subdir std flags} { -global module_headers -foreach header $module_headers { - set kind [lindex $header 0] - set hdr [lindex $header 1] - verbose "Header $hdr $std" 1 - switch $kind { - test { - global module_cmis - set module_cmis {} - dg-test -keep-output $srcdir/$subdir/$hdr "$std" $flags - global mod_files - lappend mod_files [module_cmi_p $subdir/$hdr $module_cmis] - } - system - - user { - # FIXME - } - default { - error "$kind unknown header" - } - } -} -} - # link and maybe run a set of object files # dg-module-do WHAT WHEN proc dg-module-do { args } { @@ -277,8 +243,6 @@ proc srcdir {} { proc module-init { src } { set tmp [dg-get-options $src] set option_list {} -global module_headers -set module_headers {} set have_std 0 set std_prefix "-std=c++" @@ -295,12 +259,6 @@ proc module-init { src } { set have_std 1 } } - "dg-module-headers" { - set kind [lindex $op 2] - foreach header [lindex $op 3] { - lappend module_headers [list $kind $header] - } - } } } @@ -324,7 +282,6 @@ foreach test [prune [lsort [find $srcdir/$subdir {*.[CH]}]] \ set std_list [module-init $test] foreach std $std_list { - do_module_headers $srcdir $subdir $std $DEFAULT_MODFLAGS set module_cmis {} verbose "Testing $nshort $std" 1 dg-test $test "$std" $DEFAULT_MODFLAGS @@ -347,7 +304,6 @@ foreach src [lsort [find $srcdir/$subdir {*_a.[CH}]] { global module_do set module_do {"compile" "P"} set asm_list {} - do_module_headers $srcdir $subdir $std $DEFAULT_MODFLAGS foreach test $tests { if { [lindex $module_do 1] != "N" } { set module_cmis {} diff --git c/gcc/testsuite/g++.dg/modules/sys/alias-2_a.H w/gcc/testsuite/g++.dg/modules/sys/alias-2_a.H deleted file mode 100644 index 5a058089725..000 --- c/gcc/testsuite/g++.dg/modules/sys/alias-2_a.H +++ /dev/null @@ -1,9 +0,0 @@ -// { dg-additional-options "-fmodule-header -isystem [srcdir]/sys" } -// { dg-module-cmi {} } - -#ifndef ALIAS_2_A_SYS -#define ALIAS_2_A_SYS - -int frob (int); - -#endif
c++: header-unit build capability [PR 99023]
This defect really required building header-units and include translation of pieces of the standard library. This adds smarts to the modules test harness to do that -- accept .X files as the source file, but provide '-x c++-system-header $HDR' in the options. The .X file will be considered by the driver to be a linker script and ignored (with a warning). Using this we can add 2 tests that end up building list_initializer and iostream, along with a test that iostream's build include-translates list_initializer's #include. That discovered a set of issues with the -flang-info-include-translate=HDR handling, also fixed and documented here. PR c++/99023 gcc/cp/ * module.cc (canonicalize_header_name): Use cpp_probe_header_unit. (maybe_translate_include): Fix note_includes comparison. (init_modules): Fix note_includes string termination. libcpp/ * include/cpplib.h (cpp_find_header_unit): Rename to ... (cpp_probe_header_unit): ... this. * internal.h (_cp_find_header_unit): Declare. * files.c (cpp_find_header_unit): Break apart to .. (test_header_unit): ... this, and ... (_cpp_find_header_unit): ... and, or and ... (cpp_probe_header_unit): ... this. * macro.c (cpp_get_token_1): Call _cpp_find_header_unit. gcc/ * doc/invoke.texi (flang-info-include-translate): Document header lookup behaviour. gcc/testsuite/ * g++.dg/modules/modules.exp: Bail on cross-testing. Add support for .X files. * g++.dg/modules/pr99023_a.X: New. * g++.dg/modules/pr99023_b.X: New. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 064e57a29c4..e801c52069e 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -19091,7 +19091,7 @@ canonicalize_header_name (cpp_reader *reader, location_t loc, bool unquoted, buf[len] = 0; if (const char *hdr - = cpp_find_header_unit (reader, buf, str[-1] == '<', loc)) + = cpp_probe_header_unit (reader, buf, str[-1] == '<', loc)) { len = strlen (hdr); str = hdr; @@ -19185,19 +19185,11 @@ maybe_translate_include (cpp_reader *reader, line_maps *lmaps, location_t loc, else if (note_include_translate_no && xlate == 0) note = true; else if (note_includes) -{ - /* We do not expect the note_includes vector to be large, so O(N) - iteration. */ - for (unsigned ix = note_includes->length (); !note && ix--;) - { - const char *hdr = (*note_includes)[ix]; - size_t hdr_len = strlen (hdr); - if ((hdr_len == len - || (hdr_len < len && IS_DIR_SEPARATOR (path[len - hdr_len - 1]))) - && !memcmp (hdr, path + len - hdr_len, hdr_len)) - note = true; - } -} +/* We do not expect the note_includes vector to be large, so O(N) + iteration. */ +for (unsigned ix = note_includes->length (); !note && ix--;) + if (!strcmp ((*note_includes)[ix], path)) + note = true; if (note) inform (loc, xlate @@ -19570,7 +19562,7 @@ init_modules (cpp_reader *reader) 0, !delimed, hdr, len); char *path = XNEWVEC (char, len + 1); memcpy (path, hdr, len); - path[len+1] = 0; + path[len] = 0; (*note_includes)[ix] = path; } diff --git c/gcc/doc/invoke.texi w/gcc/doc/invoke.texi index e8baa545eee..c00514a6306 100644 --- c/gcc/doc/invoke.texi +++ w/gcc/doc/invoke.texi @@ -3382,7 +3382,12 @@ is used when building the C++ library.) @itemx -flang-info-include-translate=@var{header} @opindex flang-info-include-translate @opindex flang-info-include-translate-not -Diagnose include translation events. +Diagnose include translation events. The first will note accepted +include translations, the second will note declined include +translations. The @var{header} form will inform of include +translations relating to that specific header. If @var{header} is of +the form @code{"user"} or @code{} it will be resolved to a +specific user or system header using the include path. @item -stdlib=@var{libstdc++,libc++} @opindex stdlib diff --git c/gcc/testsuite/g++.dg/modules/modules.exp w/gcc/testsuite/g++.dg/modules/modules.exp index c17120f2c00..38654caf7b9 100644 --- c/gcc/testsuite/g++.dg/modules/modules.exp +++ w/gcc/testsuite/g++.dg/modules/modules.exp @@ -39,6 +39,11 @@ set MOD_STD_LIST { 17 2a } dg-init +if {[is_remote host]} { +# remote testing not functional here :( +return +} + global module_do global module_cmis @@ -274,6 +279,9 @@ proc module-init { src } { return $option_list } +# cleanup any detritus from previous run +cleanup_module_files [find $DEFAULT_REPO *.gcm] + # not grouped tests, sadly tcl doesn't have negated glob foreach test [prune [lsort [find $srcdir/$subdir {*.[CH]}]] \ "$srcdir/$subdir/*_?.\[CH\]"] { @@ -282,6 +290,7 @@ foreach test [pru
c++: Inform of CMI reads [PR 99166]
When successfully reading a module CMI, the user gets no indication of where that CMI was located. I originally didn't consider this a problem -- the read was successful after all. But it can make it difficult to interact with build systems, particularly when caching can be involved. Grovelling over internal dump files is not really useful to the user. Hence this option, which is similar to the -flang-info-include-translate variants, and allows the user to ask for all, or specific module read notification. gcc/c-family/ * c.opt (flang-info-module-read, flang-info-module-read=): New. gcc/ * doc/invoke.texi (flang-info-module-read): Document. gcc/cp/ * module.cc (note_cmis): New. (struct module_state): Add inform_read_p bit. (module_state::do_import): Inform of CMI location, if enabled. (init_modules): Canonicalize note_cmis entries. (handle_module_option): Handle -flang-info-module-read=FOO. gcc/testsuite/ * g++.dg/modules/pr99166_a.X: New. * g++.dg/modules/pr99166_b.C: New. * g++.dg/modules/pr99166_c.C: New. * g++.dg/modules/pr99166_d.C: New. -- Nathan Sidwell diff --git c/gcc/c-family/c.opt w/gcc/c-family/c.opt index b209d46d32b..3264c646ad3 100644 --- c/gcc/c-family/c.opt +++ w/gcc/c-family/c.opt @@ -1752,6 +1752,14 @@ flang-info-include-translate= C++ Joined RejectNegative MissingArgError(missing header name) Note a #include translation of a specific header. +flang-info-module-read +C++ Var(note_module_read_yes) +Note Compiled Module Interface pathnames. + +flang-info-module-read= +C++ Joined RejectNegative MissingArgError(missing module name) +Note Compiled Module Interface pathname of a specific module or header-unit. + fmax-include-depth= C ObjC C++ ObjC++ Joined RejectNegative UInteger fmax-include-depth= Set the maximum depth of the nested #include. diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index e801c52069e..691381bf995 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -317,6 +317,9 @@ version2string (unsigned version, verstr_t &out) /* Include files to note translation for. */ static vec *note_includes; +/* Modules to note CMI pathames. */ +static vec *note_cmis; + /* Traits to hash an arbitrary pointer. Entries are not deletable, and removal is a noop (removal needed upon destruction). */ template @@ -3547,9 +3550,10 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state { do it again */ bool call_init_p : 1; /* This module's global initializer needs calling. */ + bool inform_read_p : 1; /* Inform of a read. */ /* Record extensions emitted or permitted. */ unsigned extensions : SE_BITS; - /* 12 bits used, 4 bits remain */ + /* 13 bits used, 3 bits remain */ public: module_state (tree name, module_state *, bool); @@ -3782,6 +3786,8 @@ module_state::module_state (tree name, module_state *parent, bool partition) partition_p = partition; + inform_read_p = false; + extensions = 0; if (name && TREE_CODE (name) == STRING_CST) { @@ -18634,6 +18640,8 @@ module_state::do_import (cpp_reader *reader, bool outermost) { const char *file = maybe_add_cmi_prefix (filename); dump () && dump ("CMI is %s", file); + if (note_module_read_yes || inform_read_p) + inform (loc, "reading CMI %qs", file); fd = open (file, O_RDONLY | O_CLOEXEC | O_BINARY); e = errno; } @@ -19545,6 +19553,7 @@ init_modules (cpp_reader *reader) headers = BITMAP_GGC_ALLOC (); if (note_includes) +/* Canonicalize header names. */ for (unsigned ix = 0; ix != note_includes->length (); ix++) { const char *hdr = (*note_includes)[ix]; @@ -19567,6 +19576,37 @@ init_modules (cpp_reader *reader) (*note_includes)[ix] = path; } + if (note_cmis) +/* Canonicalize & mark module names. */ +for (unsigned ix = 0; ix != note_cmis->length (); ix++) + { + const char *name = (*note_cmis)[ix]; + size_t len = strlen (name); + + bool is_system = name[0] == '<'; + bool is_user = name[0] == '"'; + bool is_pathname = false; + if (!(is_system || is_user)) + for (unsigned ix = len; !is_pathname && ix--;) + is_pathname = IS_DIR_SEPARATOR (name[ix]); + if (is_system || is_user || is_pathname) + { + if (len <= (is_pathname ? 0 : 2) + || (!is_pathname && name[len-1] != (is_system ? '>' : '"'))) + { + error ("invalid header name %qs", name); + continue; + } + else + name = canonicalize_header_name (is_pathname ? nullptr : reader, + 0, is_pathname, name, len); + } + if (auto module = get_module (name)) + module->inform_read_p = 1; + else + error ("invalid module name %qs", name); + } + dump.push (NULL); /* Determine
c++: Incorrect module-number ordering [PR 98741]
One of the very strong invariants in modules is that module numbers are allocated such that (other than the current TU), all imports have lesser module numbers, and also that the binding vector is only appended to with increasing module numbers. This broke down when module-directives became a thing and the preprocessing became entirely decoupled from parsing. We'd load header units and their macros (but not symbols of course) during preprocessing. Then we'd load named modules during parsing. This could lead to the situation where a header unit appearing after a named import had a lower module number than the import. Consequently, if they both bound the same identifier, the binding vector would be misorderd and bad things happen. This patch restores a pending import queue I previously had, but in simpler form (hurrah). During preprocessing we queue all module-directives and when we meet one for a header unit we do the minimal loading for all of the queue, so they get appropriate numbering. Then we load the preprocessor state for the header unit. PR c++/98741 gcc/cp/ * module.cc (pending_imports): New. (declare_module): Adjust test condition. (name_pending_imports): New. (preprocess_module): Reimplement using pending_imports. (preprocessed_module): Move name-getting to name_pending_imports. * name-lookup.c (append_imported_binding_slot): Assert module ordering is increasing. gcc/testsuite/ * g++.dg/modules/pr98741_a.H: New. * g++.dg/modules/pr98741_b.H: New. * g++.dg/modules/pr98741_c.C: New. * g++.dg/modules/pr98741_d.C: New. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 691381bf995..3d17b8ddcdb 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -3873,6 +3873,9 @@ module_state_hash::equal (const value_type existing, /* Mapper name. */ static const char *module_mapper_name; +/* Deferred import queue (FIFO). */ +static vec *pending_imports; + /* CMI repository path and workspace. */ static char *cmi_repo; static size_t cmi_repo_length; @@ -18944,7 +18947,7 @@ declare_module (module_state *module, location_t from_loc, bool exporting_p, gcc_assert (global_namespace == current_scope ()); module_state *current = (*modules)[0]; - if (module_purview_p () || module->loadedness != ML_NONE) + if (module_purview_p () || module->loadedness > ML_CONFIG) { error_at (from_loc, module_purview_p () ? G_("module already declared") @@ -19275,6 +19278,70 @@ module_begin_main_file (cpp_reader *reader, line_maps *lmaps, } } +/* Process the pending_import queue, making sure we know the + filenames. */ + +static void +name_pending_imports (cpp_reader *reader, bool at_end) +{ + auto *mapper = get_mapper (cpp_main_loc (reader)); + + bool only_headers = (flag_preprocess_only + && !bool (mapper->get_flags () & Cody::Flags::NameOnly) + && !cpp_get_deps (reader)); + if (at_end + && (!vec_safe_length (pending_imports) || only_headers)) +/* Not doing anything. */ +return; + + timevar_start (TV_MODULE_MAPPER); + + dump.push (NULL); + dump () && dump ("Resolving direct import names"); + + mapper->Cork (); + for (unsigned ix = 0; ix != pending_imports->length (); ix++) +{ + module_state *module = (*pending_imports)[ix]; + gcc_checking_assert (module->is_direct ()); + if (!module->filename) + { + Cody::Flags flags + = (flag_preprocess_only ? Cody::Flags::None + : Cody::Flags::NameOnly); + + if (only_headers && !module->is_header ()) + ; + else if (module->module_p + && (module->is_partition () || module->exported_p)) + mapper->ModuleExport (module->get_flatname (), flags); + else + mapper->ModuleImport (module->get_flatname (), flags); + } +} + + auto response = mapper->Uncork (); + auto r_iter = response.begin (); + for (unsigned ix = 0; ix != pending_imports->length (); ix++) +{ + module_state *module = (*pending_imports)[ix]; + gcc_checking_assert (module->is_direct ()); + if (only_headers && !module->is_header ()) + ; + else if (!module->filename) + { + Cody::Packet const &p = *r_iter; + ++r_iter; + + module->set_filename (p); + } +} + + dump.pop (0); + + timevar_stop (TV_MODULE_MAPPER); +} + /* We've just lexed a module-specific control line for MODULE. Mark the module as a direct import, and possibly load up its macro state. Returns the primary module, if this is a module @@ -19322,17 +19389,22 @@ preprocess_module (module_state *module, location_t from_loc, } } + auto desired = ML_CONFIG; if (is_import - && !module->is_module () && module->is_header () - && mod
c++: cross-header-unit template definitions [PR 99153]
A member function can be defined in a different header-file than the one defining the class. In such situations we must unmark the decl as imported. When the entity is a template we failed to unmark the template_decl. Perhaps the duplication of these flags on the template_decl from the underlying decl is an error. I set on the fence about it for a long time during development, but I don't think now is the time to change that (barring catastrophic bugs). PR c++/99153 gcc/cp/ * decl.c (duplicate_decls): Move DECL_MODULE_IMPORT_P propagation to common-path. * module.cc (set_defining_module): Add assert. gcc/testsuite/ * g++.dg/modules/pr99153_a.H: New. * g++.dg/modules/pr99153_b.H: New. -- Nathan Sidwell diff --git c/gcc/cp/decl.c w/gcc/cp/decl.c index 6f3414f058e..7fa8f52d667 100644 --- c/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -2879,19 +2879,6 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) (char *) newdecl + sizeof (struct tree_common), sizeof (struct tree_decl_common) - sizeof (struct tree_common)); - if (DECL_LANG_SPECIFIC (olddecl) && DECL_TEMPLATE_INFO (olddecl)) - { - /* Repropagate the module information to the template. */ - tree tmpl = DECL_TI_TEMPLATE (olddecl); - - if (DECL_TEMPLATE_RESULT (tmpl) == olddecl) - { - DECL_MODULE_PURVIEW_P (tmpl) = DECL_MODULE_PURVIEW_P (olddecl); - gcc_checking_assert (!DECL_MODULE_IMPORT_P (olddecl)); - DECL_MODULE_IMPORT_P (tmpl) = false; - } - } - switch (TREE_CODE (newdecl)) { case LABEL_DECL: @@ -2925,6 +2912,19 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) } } + if (DECL_LANG_SPECIFIC (olddecl) && DECL_TEMPLATE_INFO (olddecl)) +{ + /* Repropagate the module information to the template. */ + tree tmpl = DECL_TI_TEMPLATE (olddecl); + + if (DECL_TEMPLATE_RESULT (tmpl) == olddecl) + { + DECL_MODULE_PURVIEW_P (tmpl) = DECL_MODULE_PURVIEW_P (olddecl); + gcc_checking_assert (!DECL_MODULE_IMPORT_P (olddecl)); + DECL_MODULE_IMPORT_P (tmpl) = false; + } +} + if (VAR_OR_FUNCTION_DECL_P (newdecl)) { if (DECL_EXTERNAL (olddecl) diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 3d17b8ddcdb..7a40be3db35 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -18516,6 +18516,7 @@ set_defining_module (tree decl) gcc_checking_assert (!use_tpl); /* Get to the TEMPLATE_DECL. */ decl = TI_TEMPLATE (ti); + gcc_checking_assert (!DECL_MODULE_IMPORT_P (decl)); } /* Record it on the class_members list. */ diff --git c/gcc/testsuite/g++.dg/modules/pr99153_a.H w/gcc/testsuite/g++.dg/modules/pr99153_a.H new file mode 100644 index 000..3eaa76bdc32 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99153_a.H @@ -0,0 +1,11 @@ +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +template +struct pair +{ + inline void Frob (); +}; + +template +inline void Widget (); diff --git c/gcc/testsuite/g++.dg/modules/pr99153_b.H w/gcc/testsuite/g++.dg/modules/pr99153_b.H new file mode 100644 index 000..5699378d317 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99153_b.H @@ -0,0 +1,15 @@ +// PR 99153 Mismatched flags on template and result +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +import "pr99153_a.H"; + +template +inline void pair<_T1>::Frob() +{ } + + +template +inline void Widget () +{ +}
c++: Recursing header imports and other duplicates [PR 99174]
The fix for 98741 introduced two issues. (a) recursive header units iced because we tried to read the preprocessor state after having failed to read the config. (b) we could have duplicate imports of named modules in our pending queue, and that would lead to duplicate requests for pathnames, which coupled with the use of a null-pathname to indicate we'd asked could lead to desynchronization with the module mapper. Fixed by adding a 'visited' flag to module state, so we ask exactly once. PR c++/99174 gcc/cp * module.cc (struct module_state): Add visited_p flag. (name_pending_imports): Use it to avoid duplicate requests. (preprocess_module): Don't read preprocessor state if we failed to load a module's config. gcc/testsuite/ * g++.dg/modules/pr99174-1_a.C: New. * g++.dg/modules/pr99174-1_b.C: New. * g++.dg/modules/pr99174-1_c.C: New. * g++.dg/modules/pr99174.H: New. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 3d17b8ddcdb..09a9ca8778b 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -3551,9 +3551,10 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state { bool call_init_p : 1; /* This module's global initializer needs calling. */ bool inform_read_p : 1; /* Inform of a read. */ + bool visited_p : 1;/* A walk-once flag. */ /* Record extensions emitted or permitted. */ unsigned extensions : SE_BITS; - /* 13 bits used, 3 bits remain */ + /* 14 bits used, 2 bits remain */ public: module_state (tree name, module_state *, bool); @@ -3787,6 +3788,7 @@ module_state::module_state (tree name, module_state *parent, bool partition) partition_p = partition; inform_read_p = false; + visited_p = false; extensions = 0; if (name && TREE_CODE (name) == STRING_CST) @@ -19304,16 +19306,16 @@ name_pending_imports (cpp_reader *reader, bool at_end) { module_state *module = (*pending_imports)[ix]; gcc_checking_assert (module->is_direct ()); - if (!module->filename) + if (!module->filename + && !module->visited_p + && (module->is_header () || !only_headers)) { - Cody::Flags flags - = (flag_preprocess_only ? Cody::Flags::None - : Cody::Flags::NameOnly); + module->visited_p = true; + Cody::Flags flags = (flag_preprocess_only + ? Cody::Flags::None : Cody::Flags::NameOnly); - if (only_headers && !module->is_header ()) - ; - else if (module->module_p - && (module->is_partition () || module->exported_p)) + if (module->module_p + && (module->is_partition () || module->exported_p)) mapper->ModuleExport (module->get_flatname (), flags); else mapper->ModuleImport (module->get_flatname (), flags); @@ -19325,15 +19327,13 @@ name_pending_imports (cpp_reader *reader, bool at_end) for (unsigned ix = 0; ix != pending_imports->length (); ix++) { module_state *module = (*pending_imports)[ix]; - gcc_checking_assert (module->is_direct ()); - if (only_headers && !module->is_header ()) - ; - else if (!module->filename) + if (module->visited_p) { - Cody::Packet const &p = *r_iter; - ++r_iter; + module->visited_p = false; + gcc_checking_assert (!module->filename); - module->set_filename (p); + module->set_filename (*r_iter); + ++r_iter; } } @@ -19443,7 +19443,8 @@ preprocess_module (module_state *module, location_t from_loc, /* Now read the preprocessor state of this particular import. */ unsigned n = dump.push (module); - if (module->read_preprocessor (true)) + if (module->loadedness == ML_CONFIG + && module->read_preprocessor (true)) module->import_macros (); dump.pop (n); diff --git c/gcc/testsuite/g++.dg/modules/pr99174-1_a.C w/gcc/testsuite/g++.dg/modules/pr99174-1_a.C new file mode 100644 index 000..c22b45bff45 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99174-1_a.C @@ -0,0 +1,8 @@ +// PR 99174 what if we import the same module twice (with no +// intervening header import)? +// { dg-additional-options -fmodules-ts } + +export module Foo; +// { dg-module-cmi Foo } + +export void Foo (); diff --git c/gcc/testsuite/g++.dg/modules/pr99174-1_b.C w/gcc/testsuite/g++.dg/modules/pr99174-1_b.C new file mode 100644 index 000..aaa0a9492ad --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99174-1_b.C @@ -0,0 +1,6 @@ +// { dg-additional-options -fmodules-ts } + +export module Bar; +// { dg-module-cmi Bar } + +export void Bar (); diff --git c/gcc/testsuite/g++.dg/modules/pr99174-1_c.C w/gcc/testsuite/g++.dg/modules/pr99174-1_c.C new file mode 100644 index 000..58936f292f8 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99174-1_c.C @@ -0,0 +1,11 @
c++: typedef for linkage [PR 99208]
Unnamed types with a typedef name for linkage were always troublesome in modules. This is the underlying cause of that trouble -- we were creating incorrect type structures. Classes have an implicit self-reference, and we created that for unnamed classes too. It turns out we make use of this member, so just not generating it turned into a rathole. This member is created using the anonymous name -- because we've not yet met the typedef name. When we retrofit the typedef name we were checking identifier matching and changing all type variants with that identifier. Which meant we ended up with a strange typedef for the self reference. This fixes things to check for DECL identity of the variants, so we don't smash the self-reference -- that continues to have the anonymous name. PR c++/99208 gcc/cp/ * decl.c (name_unnamed_type): Check DECL identity, not IDENTIFIER identity. gcc/testsuite/ * g++.dg/modules/pr99208_a.C: New. * g++.dg/modules/pr99208_b.C: New. -- Nathan Sidwell diff --git c/gcc/cp/decl.c w/gcc/cp/decl.c index 7fa8f52d667..1742e286d9f 100644 --- c/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -11081,21 +11081,18 @@ name_unnamed_type (tree type, tree decl) { gcc_assert (TYPE_UNNAMED_P (type)); - /* Replace the anonymous name with the real name everywhere. */ + /* Replace the anonymous decl with the real decl. Be careful not to + rename other typedefs (such as the self-reference) of type. */ + tree orig = TYPE_NAME (type); for (tree t = TYPE_MAIN_VARIANT (type); t; t = TYPE_NEXT_VARIANT (t)) -if (IDENTIFIER_ANON_P (TYPE_IDENTIFIER (t))) - /* We do not rename the debug info representing the unnamed - tagged type because the standard says in [dcl.typedef] that - the naming applies only for linkage purposes. */ - /*debug_hooks->set_name (t, decl);*/ +if (TYPE_NAME (t) == orig) TYPE_NAME (t) = decl; /* If this is a typedef within a template class, the nested type is a (non-primary) template. The name for the template needs updating as well. */ if (TYPE_LANG_SPECIFIC (type) && CLASSTYPE_TEMPLATE_INFO (type)) -DECL_NAME (CLASSTYPE_TI_TEMPLATE (type)) - = TYPE_IDENTIFIER (type); +DECL_NAME (CLASSTYPE_TI_TEMPLATE (type)) = DECL_NAME (decl); /* Adjust linkage now that we aren't unnamed anymore. */ reset_type_linkage (type); diff --git c/gcc/testsuite/g++.dg/modules/pr99208_a.C w/gcc/testsuite/g++.dg/modules/pr99208_a.C new file mode 100644 index 000..427c7f1b04c --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99208_a.C @@ -0,0 +1,9 @@ +// PR 99208 typedef anonymous class +// { dg-additional-options {-Wno-pedantic -fmodules-ts} } +module; +# 5 "pr99208_a.C" 1 +typedef struct {} __mbstate_t; +# 7 "" 2 +export module hello:format; +// { dg-module-cmi {hello:format} } +export __mbstate_t v; diff --git c/gcc/testsuite/g++.dg/modules/pr99208_b.C w/gcc/testsuite/g++.dg/modules/pr99208_b.C new file mode 100644 index 000..0ed68d8069a --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99208_b.C @@ -0,0 +1,4 @@ +// { dg-additional-options {-fmodules-ts} } +export module hello; +// { dg-module-cmi hello } +export import :format;
c++: modules & -fpreprocessed [PR 99072]
When we read preprocessed source, we deal with a couple of special location lines at the start of the file. These provide information about the original filename of the source and the current directory, so we can process the source in the same manner. When updating that code, I had a somewhat philosophical question: Should the line table contain evidence of the filename the user provided to the compiler? I figured to leave it there, as it did no harm. But this defect shows an issue. It's in the line table and our (non optimizing) line table serializer emits that filename. Which means if one re-preprocesses the original source to a differently-named intermediate file, the resultant CMI is different. Boo. That's a difference that doesn't matter, except the CRC matching then fails. We should elide the filename, so that one can preprocess to mktemp intermediate filenames for whatever reason. This patch takes the approach of expunging it from the line table -- so the line table will end up with exactly the same form. That seems a better bet than trying to fix up mismatching line tables in CMI emission. PR c++/99072 libcpp/ * init.c (read_original_filename): Expunge all evidence of the original filename. gcc/testsuite/ * g++.dg/modules/pr99072.H: New. -- Nathan Sidwell diff --git c/gcc/testsuite/g++.dg/modules/pr99072.H w/gcc/testsuite/g++.dg/modules/pr99072.H new file mode 100644 index 000..eda0c07f9e2 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99072.H @@ -0,0 +1,10 @@ +# 0 "REALNAME" +# 0 "" +# 0 "" +# 0 "" +# 1 "/usr/include/stdc-predef.h" 1 3 4 +# 0 "" 2 +# 1 "REALNAME" + +// { dg-additional-options {-fmodule-header -fpreprocessed -fdump-lang-module-lineno} } +// { dg-final { scan-lang-dump { 4 source file names\n Source file\[0\]=REALNAME\n Source file\[1\]=\n Source file\[2\]=\n Source file\[3\]=/usr/include/stdc-predef.h\n} module } } diff --git c/libcpp/init.c w/libcpp/init.c index 17b0d251cda..68ed2c761b9 100644 --- c/libcpp/init.c +++ w/libcpp/init.c @@ -752,6 +752,23 @@ read_original_filename (cpp_reader *pfile) if (_cpp_handle_directive (pfile, token->flags & PREV_WHITE)) { read_original_directory (pfile); + + auto *penult = &linemap_check_ordinary + (LINEMAPS_LAST_MAP (pfile->line_table, false))[-1]; + if (penult[1].reason == LC_RENAME_VERBATIM) + { + /* Expunge any evidence of the original linemap. */ + pfile->line_table->highest_location + = pfile->line_table->highest_line + = penult[0].start_location; + + penult[1].start_location = penult[0].start_location; + penult[1].reason = penult[0].reason; + penult[0] = penult[1]; + pfile->line_table->info_ordinary.used--; + pfile->line_table->info_ordinary.cache = 0; + } + return true; } }
c++: Macro location fixes [PR 98718]
This fixes some issues with macro maps. We were incorrectly calculating the number of macro expansions in a location span, and I had a workaround that partially covered that up. Further, while macro location spans are monotonic, that is not true of ordinary location spans. Thus we need to insert an indirection array when binary searching the latter. (We load ordinary locations before loading imports, but macro locations afterwards. We make sure an import location is de-macrofied, if needed.) PR c++/98718 gcc/cp/ * module.cc (ool): New indirection vector. (loc_spans::maybe_propagate): Location is not optional. (loc_spans::open): Likewise. Assert monotonically advancing. (module_for_ordinary_loc): Use ool indirection vector. (module_state::write_prepare_maps): Do not count empty macro expansions. Elide empty spans. (module_state::write_macro_maps): Skip empty expansions. (ool_cmp): New qsort comparator. (module_state::write): Create and destroy ool vector. (name_pending_imports): Fix dump push/pop. (preprocess_module): Likewise. Add more dumping. (preprocessed_module): Likewise. libcpp/ * include/line-map.h * line-map.c gcc/testsuite/ * g++.dg/modules/pr98718_a.C: New. * g++.dg/modules/pr98718_b.C: New. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 766f2ab853d..e576face0d8 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -3363,6 +3363,8 @@ public: }; static loc_spans spans; +/* Indirection to allow bsearching imports by ordinary location. */ +static vec *ool; // /* Data needed by a module during the process of loading. */ @@ -13758,13 +13760,12 @@ loc_spans::init (const line_maps *lmaps, const line_map_ordinary *map) interface and we're importing a partition. */ bool -loc_spans::maybe_propagate (module_state *import, - location_t loc = UNKNOWN_LOCATION) +loc_spans::maybe_propagate (module_state *import, location_t hwm) { bool opened = (module_interface_p () && !module_partition_p () && import->is_partition ()); if (opened) -open (loc); +open (hwm); return opened; } @@ -13772,11 +13773,8 @@ loc_spans::maybe_propagate (module_state *import, first map of the interval. */ void -loc_spans::open (location_t hwm = UNKNOWN_LOCATION) +loc_spans::open (location_t hwm) { - if (hwm == UNKNOWN_LOCATION) -hwm = MAP_START_LOCATION (LINEMAPS_LAST_ORDINARY_MAP (line_table)); - span interval; interval.ordinary.first = interval.ordinary.second = hwm; interval.macro.first = interval.macro.second @@ -13786,6 +13784,13 @@ loc_spans::open (location_t hwm = UNKNOWN_LOCATION) && dump ("Opening span %u ordinary:[%u,... macro:...,%u)", spans->length (), interval.ordinary.first, interval.macro.second); + if (spans->length ()) +{ + /* No overlapping! */ + auto &last = spans->last (); + gcc_checking_assert (interval.ordinary.first >= last.ordinary.second); + gcc_checking_assert (interval.macro.second <= last.macro.first); +} spans->safe_push (interval); } @@ -15547,13 +15552,13 @@ enum loc_kind { static const module_state * module_for_ordinary_loc (location_t loc) { - unsigned pos = 1; - unsigned len = modules->length () - pos; + unsigned pos = 0; + unsigned len = ool->length () - pos; while (len) { unsigned half = len / 2; - module_state *probe = (*modules)[pos + half]; + module_state *probe = (*ool)[pos + half]; if (loc < probe->ordinary_locs.first) len = half; else if (loc < probe->ordinary_locs.second) @@ -15565,7 +15570,7 @@ module_for_ordinary_loc (location_t loc) } } - return NULL; + return nullptr; } static const module_state * @@ -15849,31 +15854,49 @@ module_state::write_prepare_maps (module_state_config *) for (unsigned ix = loc_spans::SPAN_FIRST; ix != spans.length (); ix++) { loc_spans::span &span = spans[ix]; - line_map_ordinary const *omap - = linemap_check_ordinary (linemap_lookup (line_table, - span.ordinary.first)); - - /* We should exactly match up. */ - gcc_checking_assert (MAP_START_LOCATION (omap) == span.ordinary.first); - line_map_ordinary const *fmap = omap; - for (; MAP_START_LOCATION (omap) < span.ordinary.second; omap++) + if (span.ordinary.first != span.ordinary.second) { - /* We should never find a module linemap in an interval. */ - gcc_checking_assert (!MAP_MODULE_P (omap)); + line_map_ordinary const *omap + = linemap_check_ordinary (linemap_lookup (line_table, + span.ordinary.first)); - if (max_range < omap->m_range_bits) - max_range = omap->m_range_
c++: Fix typo in module-mapper [PR 98318]
User reported this typo: '0' and '-' are right next to each other, and as it happened I always had networking, so it went unnoticed. PR c++/98318 gcc/cp/ * mapper-client.cc (module_client::open_module_client): Fix typo of fd init. -- Nathan Sidwell diff --git c/gcc/cp/mapper-client.cc w/gcc/cp/mapper-client.cc index a72b4b70664..774e2b2b095 100644 --- c/gcc/cp/mapper-client.cc +++ w/gcc/cp/mapper-client.cc @@ -249,7 +249,7 @@ module_client::open_module_client (location_t loc, const char *o, if (port && endp != cptr + 1 && !*endp) { name[colon] = 0; - int fd = 01; + int fd = -1; #if CODY_NETWORKING fd = Cody::OpenInet6 (&errmsg, name.c_str (), port); #endif
Re: [PATCH] coroutines : Call promise CTOR with parm copies [PR97587].
On 2/24/21 3:09 PM, Iain Sandoe wrote: Hi, As the PR notes, we were calling the promise CTOR with the original function parameters, not the copy (as pointed, a previous wording of the section on this was unambiguous). Fixed thus. tested on x86_64-darwin, x86_64-linux-gnu, this is a wrong-code bug, OK for master / 10.x? thanks Iain gcc/cp/ChangeLog: PR c++/97587 * coroutines.cc (struct param_info): Track rvalue refs. (morph_fn_to_coro): Track rvalue refs, and call the promise CTOR with the frame copy of passed parms. gcc/testsuite/ChangeLog: PR c++/97587 * g++.dg/coroutines/coro1-refs-and-ctors.h: Add a CTOR with two reference parms, to distinguish the rvalue ref. variant. * g++.dg/coroutines/pr97587.C: New test. ok, thanks -- Nathan Sidwell
c++tools: Make NETWORKING define check consistent [PR 98318]
PR98318 also pointed out that the NETWORKING #define was being checked with both #if and #ifdef. Let's consistently use one form. c++tools/ * server.cc: Use #if NETWORKING not #ifdef, to be consistent with elsewhere. -- Nathan Sidwell diff --git c/c++tools/server.cc w/c++tools/server.cc index 8514ef6293b..21c7d468aa0 100644 --- c/c++tools/server.cc +++ w/c++tools/server.cc @@ -63,7 +63,7 @@ along with GCC; see the file COPYING3. If not see #include // Select or epoll -#ifdef NETWORKING +#if NETWORKING #ifdef HAVE_EPOLL /* epoll_create, epoll_ctl, epoll_pwait */ #include @@ -91,7 +91,7 @@ along with GCC; see the file COPYING3. If not see #define DIR_SEPARATOR '/' #endif -#ifdef NETWORKING +#if NETWORKING struct netmask { in6_addr addr; unsigned bits; @@ -161,7 +161,7 @@ static bool flag_xlate = false; /* Root binary directory. */ static const char *flag_root = "gcm.cache"; -#ifdef NETWORKING +#if NETWORKING static netmask_set_t netmask_set; static netmask_vec_t accept_addrs; @@ -233,7 +233,7 @@ error (const char *msg, ...) exit (1); } -#ifdef NETWORKING +#if NETWORKING /* Progress messages to the user. */ static bool ATTRIBUTE_PRINTF_1 ATTRIBUTE_COLD noisy (const char *fmt, ...) @@ -414,7 +414,7 @@ process_args (int argc, char **argv) return optind; } -#ifdef NETWORKING +#if NETWORKING /* Manipulate the EPOLL state, or do nothing, if there is epoll. */ @@ -871,7 +871,7 @@ main (int argc, char *argv[]) /* Ignore sigpipe, so read/write get an error. */ signal (SIGPIPE, SIG_IGN); #endif -#ifdef NETWORKING +#if NETWORKING #ifdef SIGINT signal (SIGINT, kill_signal); #endif @@ -935,7 +935,7 @@ main (int argc, char *argv[]) } #endif -#ifdef NETWORKING +#if NETWORKING if (sock_fd >= 0) { server (name[0] != '=', sock_fd, &r);
c++: Rename new -flang-note-module-read option [PR 99166]
I realized that the just-added flang-note-module-read option should also cover module writes, and was therefore misnamed. This addresses that, replacing it with a -flang-note-module-cmi pair of options. As this was such a recent addition, I didn't leave the old option available. PR c++/99166 gcc/c-family/ * c.opt (-flang-info-module-cmi): Renamed option. gcc/ * doc/invoke.texi (flang-info-module-cmi): Renamed option. gcc/cp/ * module.cc (module_state::inform_cmi_p): Renamed field. (module_state::do_import): Adjust. (init_modules, finish_module_processing): Likewise. (handle_module_option): Likewise. gcc/testsuite/ * g++.dg/modules/pr99166_a.X: Adjust. * g++.dg/modules/pr99166_b.C: Adjust. * g++.dg/modules/pr99166_c.C: Adjust. * g++.dg/modules/pr99166_d.C: Adjust. -- Nathan Sidwell diff --git c/gcc/c-family/c.opt w/gcc/c-family/c.opt index 3264c646ad3..64e46e7573e 100644 --- c/gcc/c-family/c.opt +++ w/gcc/c-family/c.opt @@ -1752,11 +1752,11 @@ flang-info-include-translate= C++ Joined RejectNegative MissingArgError(missing header name) Note a #include translation of a specific header. -flang-info-module-read -C++ Var(note_module_read_yes) +flang-info-module-cmi +C++ Var(note_module_cmi_yes) Note Compiled Module Interface pathnames. -flang-info-module-read= +flang-info-module-cmi= C++ Joined RejectNegative MissingArgError(missing module name) Note Compiled Module Interface pathname of a specific module or header-unit. diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index e576face0d8..0cb5bd9b644 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -3552,7 +3552,7 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state { do it again */ bool call_init_p : 1; /* This module's global initializer needs calling. */ - bool inform_read_p : 1; /* Inform of a read. */ + bool inform_cmi_p : 1; /* Inform of a read/write. */ bool visited_p : 1;/* A walk-once flag. */ /* Record extensions emitted or permitted. */ unsigned extensions : SE_BITS; @@ -3789,7 +3789,7 @@ module_state::module_state (tree name, module_state *parent, bool partition) partition_p = partition; - inform_read_p = false; + inform_cmi_p = false; visited_p = false; extensions = 0; @@ -18699,7 +18699,7 @@ module_state::do_import (cpp_reader *reader, bool outermost) { const char *file = maybe_add_cmi_prefix (filename); dump () && dump ("CMI is %s", file); - if (note_module_read_yes || inform_read_p) + if (note_module_cmi_yes || inform_cmi_p) inform (loc, "reading CMI %qs", file); fd = open (file, O_RDONLY | O_CLOEXEC | O_BINARY); e = errno; @@ -19695,7 +19695,7 @@ init_modules (cpp_reader *reader) 0, is_pathname, name, len); } if (auto module = get_module (name)) - module->inform_read_p = 1; + module->inform_cmi_p = 1; else error ("invalid module name %qs", name); } @@ -19903,6 +19903,8 @@ finish_module_processing (cpp_reader *reader) break; create_dirs (tmp_name); } + if (note_module_cmi_yes || state->inform_cmi_p) + inform (state->loc, "writing CMI %qs", path); dump () && dump ("CMI is %s", path); } @@ -19915,7 +19917,7 @@ finish_module_processing (cpp_reader *reader) if (to.begin ()) { auto loc = input_location; - /* So crashes finger point the module decl. */ + /* So crashes finger-point the module decl. */ input_location = state->loc; state->write (&to, reader); input_location = loc; @@ -20085,7 +20087,7 @@ handle_module_option (unsigned code, const char *str, int) vec_safe_push (note_includes, str); return true; -case OPT_flang_info_module_read_: +case OPT_flang_info_module_cmi_: vec_safe_push (note_cmis, str); return true; diff --git c/gcc/doc/invoke.texi w/gcc/doc/invoke.texi index ea315f1be58..546e95453c1 100644 --- c/gcc/doc/invoke.texi +++ w/gcc/doc/invoke.texi @@ -242,7 +242,7 @@ in the following sections. -fext-numeric-literals @gol -flang-info-include-translate@r{[}=@var{header}@r{]} @gol -flang-info-include-translate-not @gol --flang-info-module-read@r{[}=@var{module}@r{]} @gol +-flang-info-module-cmi@r{[}=@var{module}@r{]} @gol -stdlib=@var{libstdc++,libc++} @gol -Wabi-tag -Wcatch-value -Wcatch-value=@var{n} @gol -Wno-class-conversion -Wclass-memaccess @gol @@ -3390,9 +3390,9 @@ translations relating to that specific header. If @var{header} is of the form @code{"user"} or @code{} it will be resolved to a specific user or system header using the include path. -@item -flang-info-module-read -@itemx -flang-info-module-read=@var{module} -@opindex flang-info-module-read +@item -flang-info-module-cmi +@itemx -f
c++: Completeness of typedef structs [PR 99294]
When we read in a class definition, we use fixup_type_variants to propagate the now-completed fields of the class's TYPE to other variants. Unfortunately that doesn't propagate all of them, and in this case we had a typedef to an (incomplete) instantiation. That typedef ended up with a VOIDmode, which blew up gimple expansion as the type itself isn't VOID. Without modules, that information is propagated in finalize_type_size when laying out the class. But that doesn't happen with stream-in -- we already know the layout. There is already some overlap between the two functions, now there's a bit more. In fixup_type_variants, I pay attention to the TYPE_NAME to decide whether to override a user's TYPE_ALIGN -- variants of the main-variant typedef just copy the main-variant. Other variants recalculate. Overaligning is still permitted. I also added a TYPE_ALIGN_RAW accessor, and fixed a bug in the alignment streaming I noticed. I did not refactor TYPE_ALIGN beyond using the new accessor. (It could be written as ((1 << align_raw) >> 1), rather than use the conditional.) PR c++/99294 gcc/ * gcc/tree.h (TYPE_ALIGN_RAW): New accessor. (TYPE_ALIGN): Use it. gcc/cp/ * class.c (fixup_type_variants): Propagate mode, precision, alignment & emptiness. * module.cc (trees_out::type_node): Use TYPE_ALIGN_RAW. (trees_in::tree_node): Rematerialize alignment here. gcc/testsuite/ * g++.dg/modules/pr99294.h: New. * g++.dg/modules/pr99294_a.C: New. * g++.dg/modules/pr99294_b.C: New. -- Nathan Sidwell diff --git c/gcc/cp/class.c w/gcc/cp/class.c index 40f5fef7baa..ea007e88e6e 100644 --- c/gcc/cp/class.c +++ w/gcc/cp/class.c @@ -2005,35 +2005,45 @@ determine_primary_bases (tree t) /* Update the variant types of T. */ void -fixup_type_variants (tree t) +fixup_type_variants (tree type) { - tree variants; - - if (!t) + if (!type) return; - for (variants = TYPE_NEXT_VARIANT (t); - variants; - variants = TYPE_NEXT_VARIANT (variants)) + for (tree variant = TYPE_NEXT_VARIANT (type); + variant; + variant = TYPE_NEXT_VARIANT (variant)) { /* These fields are in the _TYPE part of the node, not in the TYPE_LANG_SPECIFIC component, so they are not shared. */ - TYPE_HAS_USER_CONSTRUCTOR (variants) = TYPE_HAS_USER_CONSTRUCTOR (t); - TYPE_NEEDS_CONSTRUCTING (variants) = TYPE_NEEDS_CONSTRUCTING (t); - TYPE_HAS_NONTRIVIAL_DESTRUCTOR (variants) - = TYPE_HAS_NONTRIVIAL_DESTRUCTOR (t); + TYPE_HAS_USER_CONSTRUCTOR (variant) = TYPE_HAS_USER_CONSTRUCTOR (type); + TYPE_NEEDS_CONSTRUCTING (variant) = TYPE_NEEDS_CONSTRUCTING (type); + TYPE_HAS_NONTRIVIAL_DESTRUCTOR (variant) + = TYPE_HAS_NONTRIVIAL_DESTRUCTOR (type); - TYPE_POLYMORPHIC_P (variants) = TYPE_POLYMORPHIC_P (t); - CLASSTYPE_FINAL (variants) = CLASSTYPE_FINAL (t); + TYPE_POLYMORPHIC_P (variant) = TYPE_POLYMORPHIC_P (type); + CLASSTYPE_FINAL (variant) = CLASSTYPE_FINAL (type); - TYPE_BINFO (variants) = TYPE_BINFO (t); + TYPE_BINFO (variant) = TYPE_BINFO (type); /* Copy whatever these are holding today. */ - TYPE_VFIELD (variants) = TYPE_VFIELD (t); - TYPE_FIELDS (variants) = TYPE_FIELDS (t); + TYPE_VFIELD (variant) = TYPE_VFIELD (type); + TYPE_FIELDS (variant) = TYPE_FIELDS (type); + + TYPE_SIZE (variant) = TYPE_SIZE (type); + TYPE_SIZE_UNIT (variant) = TYPE_SIZE_UNIT (type); + + if (!TYPE_USER_ALIGN (variant) + || TYPE_NAME (variant) == TYPE_NAME (type) + || TYPE_ALIGN_RAW (variant) < TYPE_ALIGN_RAW (type)) + { + TYPE_ALIGN_RAW (variant) = TYPE_ALIGN_RAW (type); + TYPE_USER_ALIGN (variant) = TYPE_USER_ALIGN (type); + } - TYPE_SIZE (variants) = TYPE_SIZE (t); - TYPE_SIZE_UNIT (variants) = TYPE_SIZE_UNIT (t); + TYPE_PRECISION (variant) = TYPE_PRECISION (type); + TYPE_MODE_RAW (variant) = TYPE_MODE_RAW (type); + TYPE_EMPTY_P (variant) = TYPE_EMPTY_P (type); } } diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 0cb5bd9b644..369a02604fe 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -8714,7 +8714,7 @@ trees_out::type_node (tree type) else { if (TYPE_USER_ALIGN (type)) - flags = exact_log2 (TYPE_ALIGN (type)); + flags = TYPE_ALIGN_RAW (type); } if (streaming_p ()) @@ -9510,7 +9510,7 @@ trees_in::tree_node (bool is_use) } else { - res = build_aligned_type (res, 1u << flags); + res = build_aligned_type (res, (1u << flags) >> 1); TYPE_USER_ALIGN (res) = true; } diff --git c/gcc/testsuite/g++.dg/modules/pr99294.h w/gcc/testsuite/g++.dg/modules/pr99294.h new file mode 100644 index 000..757113c0283 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99294.h @@ -0,0 +1,14 @@ + +template +class basic_string; + +typede
c++: namespace reachability [PR 99344]
This reworks namespace serializing to avoid some issues I ran into when working on 99170. In modules, (non-anonymous) namespaces are strange beasts, that always have external linkage, but may have module-specific visibility. I still don't get the latter 100% correct, but this is in the right direction. PR c++/99344 gcc/cp/ * module.cc (trees_out::decl_node): Small refactor. (depset::hash::add_binding_entity): Return true on meeting an import. Set namespace's import here. (module_state:write_namespaces): Inform of purview too. (module_state:read_namespaces): Adjust. * name-lookup.c (implicitly_export_namespace): Delete. (do_pushdecl): Don't call it. (push_namespace): Likewise, set purview. (add_imported_namespace): Reorder parms. * name-lookup.h (add_imported_namespace): Alter param ordering. gcc/testsuite/ * g++.dg/modules/namespace-2_a.C * g++.dg/modules/pr99344_a.C * g++.dg/modules/pr99344_b.C -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 369a02604fe..31172824fdd 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -2377,8 +2377,10 @@ public: } public: + /* This class-member is defined here, but the class was imported. */ bool is_member () const { +gcc_checking_assert (get_entity_kind () == EK_DECL); return get_flag_bit (); } public: @@ -8613,12 +8615,13 @@ trees_out::decl_node (tree decl, walk_kind ref) else if (TREE_CODE (ctx) != FUNCTION_DECL || TREE_CODE (decl) == TEMPLATE_DECL || (dep_hash->sneakoscope && DECL_IMPLICIT_TYPEDEF_P (decl)) - || (DECL_LANG_SPECIFIC (decl) - && DECL_MODULE_IMPORT_P (decl))) -dep = dep_hash->add_dependency (decl, -TREE_CODE (decl) == NAMESPACE_DECL -&& !DECL_NAMESPACE_ALIAS (decl) -? depset::EK_NAMESPACE : depset::EK_DECL); + || (DECL_LANG_SPECIFIC (decl) && DECL_MODULE_IMPORT_P (decl))) +{ + auto kind = (TREE_CODE (decl) == NAMESPACE_DECL + && !DECL_NAMESPACE_ALIAS (decl) + ? depset::EK_NAMESPACE : depset::EK_DECL); + dep = dep_hash->add_dependency (decl, kind); +} if (!dep) { @@ -12751,12 +12754,14 @@ struct add_binding_data bool met_namespace; }; +/* Return true if we are, or contain something that is exported. */ + bool depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_) { auto data = static_cast (data_); - if (TREE_CODE (decl) != NAMESPACE_DECL || DECL_NAMESPACE_ALIAS (decl)) + if (!(TREE_CODE (decl) == NAMESPACE_DECL && !DECL_NAMESPACE_ALIAS (decl))) { tree inner = decl; @@ -12811,7 +12816,7 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_) d->clear_hidden_binding (); if (flags & WMB_Export) OVL_EXPORT_P (d->get_entity ()) = true; - return false; + return bool (flags & WMB_Export); } } } @@ -12857,30 +12862,37 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_) /* Binding and contents are mutually dependent. */ dep->deps.safe_push (data->binding); - return true; + return (flags & WMB_Using + ? flags & WMB_Export : DECL_MODULE_EXPORT_P (decl)); } else if (DECL_NAME (decl) && !data->met_namespace) { /* Namespace, walk exactly once. */ gcc_checking_assert (TREE_PUBLIC (decl)); data->met_namespace = true; - if (data->hash->add_namespace_entities (decl, data->partitions) - || DECL_MODULE_EXPORT_P (decl)) + if (data->hash->add_namespace_entities (decl, data->partitions)) + { + /* It contains an exported thing, so it is exported. */ + gcc_checking_assert (DECL_MODULE_PURVIEW_P (decl)); + DECL_MODULE_EXPORT_P (decl) = true; + } + + if (DECL_MODULE_PURVIEW_P (decl)) { data->hash->make_dependency (decl, depset::EK_NAMESPACE); - return true; + + return DECL_MODULE_EXPORT_P (decl); } } return false; } -/* Recursively find all the namespace bindings of NS. - Add a depset for every binding that contains an export or - module-linkage entity. Add a defining depset for every such decl - that we need to write a definition. Such defining depsets depend - on the binding depset. Returns true if we contain something - explicitly exported. */ +/* Recursively find all the namespace bindings of NS. Add a depset + for every binding that contains an export or module-linkage entity. + Add a defining depset for every such decl that we need to write a + definition. Such defining depsets depend on the binding depset. + Returns true if we contain something exported. */ bool depset::hash::add_namespace_entities (tree ns, bitmap partitions) @@ -15088,36 +15100,29 @@ module_state::write_namespaces (el
c++: Defer specialization registration [PR 99170]
This defers inserting specializations into the specialization table, until we have completed their streaming. When streaming a cluster we ensure that all imports are populated before any of the cluster, so they need no visibility of other specializations. Further within the same import, we've already partitioned the graph, so no earlier cluster can be refering to a specialization in a later cluster. Inserting them early causes problems when other specializations of the same template are inserted. (This doesn't fix 99170, but is a necessary change for that PR). Earlier on, I had less deferred processing, but it has become clearer that deferred worklists are the right way of handling a few things. This patch highlights a fixme, in that we're streaming a key twice, and need not do that, but I wanted to get correctness first. Besides the second streaming will end up being a back reference, which is of course much cheaper than a by-value stream. PR c++/99170 gcc/cp/ * module.cc (trees_out::decl_value): Stream specialization keys after decl. (trees_in::decl_value): Stream them back and insert after completing the decl. (trees_out::key_mergeable): Drop some streaming here ... (trees_in::key_mergeable): ... and here. Don't insert into specialization tables. -- Nathan Sidwell diff --git i/gcc/cp/module.cc w/gcc/cp/module.cc index 31172824fdd..b068acea3bd 100644 --- i/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -7808,7 +7808,31 @@ trees_out::decl_value (tree decl, depset *dep) } if (!is_key_order ()) -tree_node (get_constraints (decl)); +{ + if (mk & MK_template_mask + || mk == MK_partial + || mk == MK_friend_spec) + { + if (mk != MK_partial) + { + // FIXME: We should make use of the merge-key by + // exposing it outside of key_mergeable. But this gets + // the job done. + auto *entry = reinterpret_cast (dep->deps[0]); + + if (streaming_p ()) + u (get_mergeable_specialization_flags (entry->tmpl, decl)); + tree_node (entry->tmpl); + tree_node (entry->args); + } + else + { + tree_node (CLASSTYPE_TI_TEMPLATE (TREE_TYPE (inner))); + tree_node (CLASSTYPE_TI_ARGS (TREE_TYPE (inner))); + } + } + tree_node (get_constraints (decl)); +} if (streaming_p ()) { @@ -8096,7 +8120,22 @@ trees_in::decl_value () || (stub_decl && !tree_node_vals (stub_decl goto bail; - tree constraints = tree_node (); + spec_entry spec; + unsigned spec_flags = 0; + if (mk & MK_template_mask + || mk == MK_partial + || mk == MK_friend_spec) +{ + if (mk == MK_partial) + spec_flags = 2; + else + spec_flags = u (); + + spec.tmpl = tree_node (); + spec.args = tree_node (); +} + /* Hold constraints on the spec field, for a short while. */ + spec.spec = tree_node (); dump (dumper::TREE) && dump ("Read:%d %C:%N", tag, TREE_CODE (decl), decl); @@ -8157,8 +8196,13 @@ trees_in::decl_value () } } - if (constraints) - set_constraints (decl, constraints); + if (spec.spec) + set_constraints (decl, spec.spec); + if (mk & MK_template_mask + || mk == MK_partial) + /* Add to specialization tables now that constraints etc are + added. */ + add_mergeable_specialization (spec.tmpl, spec.args, decl, spec_flags); if (TREE_CODE (decl) == INTEGER_CST && !TREE_OVERFLOW (decl)) { @@ -8247,6 +8291,15 @@ trees_in::decl_value () decl = existing; } + if (mk == MK_friend_spec) +{ + tree e = match_mergeable_specialization (true, &spec); + if (!e) + add_mergeable_specialization (spec.tmpl, spec.args, decl, spec_flags); + else if (e != existing) + set_overrun (); +} + if (is_typedef) { /* Insert the type into the array now. */ @@ -10415,8 +10468,6 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, tree_node (entry->tmpl); tree_node (entry->args); - if (streaming_p ()) - u (get_mergeable_specialization_flags (entry->tmpl, decl)); if (mk & MK_tmpl_decl_mask) if (flag_concepts && TREE_CODE (inner) == VAR_DECL) { @@ -10504,16 +10555,6 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, key.ret = fndecl_declared_return_type (inner); } - if (mk == MK_friend_spec) - { - gcc_checking_assert (dep && dep->is_special ()); - spec_entry *entry = reinterpret_cast (dep->deps[0]); - - tree_node (entry->tmpl); - tree_node (entry->args); - if (streaming_p ()) - u (get_mergeable_specialization_flags (entry->tmpl, decl)); - } break; case MK_field: @@ -10763,7 +10804,6 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, spec_entry
c++: Defer cloning to post-loading [PR 99170]
It turns out that cloning can cause use to load things. Specifically when checking paramter shadows (this is avoidable), and also the delete operator of a deleting dtor (not avoidable). Doing that in the middle of loading is a bad thing. This defers it to a post-load worklist. If it causes more loading at that point there is no problem, as we've completed the first set of loads, bar this bit of cleanup. Again, this doesn't fix 99170, but is a step towards a solution. PR c++/99170 gcc/cp/ * module.cc (post_load_decls): New. (lazy_snum, recursive_lazy): Move earlier. (module_state::read_cluster): Push cloning onto post_load_decls. (post_load_processing): New. Do the cloning here. (module_state::read_inits): Call post_load_processing. (module_state::read_language): Likewise. (lazy_load_binding, lazy_load_specializations): Likewise (lazy_load_members): Likewise -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index b068acea3bd..b7b9c3734f2 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -2644,6 +2644,10 @@ depset *depset::make_entity (tree entity, entity_kind ek, bool is_defn) return r; } +/* Decls that need some post processing once a batch of lazy loads has + completed. */ +vec *post_load_decls; + /* Values keyed to some unsigned integer. This is not GTY'd, so if T is tree they must be reachable via some other path. */ @@ -14023,6 +14027,21 @@ make_mapper (location_t loc) return mapper; } +static unsigned lazy_snum; + +static bool +recursive_lazy (unsigned snum = ~0u) +{ + if (lazy_snum) +{ + error_at (input_location, "recursive lazy load"); + return true; +} + + lazy_snum = snum; + return false; +} + /* If THIS is the current purview, issue an import error and return false. */ bool @@ -15016,11 +15035,7 @@ module_state::read_cluster (unsigned snum) if (abstract) ; else if (DECL_ABSTRACT_P (decl)) - { - bool cloned = maybe_clone_body (decl); - if (!cloned) - from ()->set_error (); - } + vec_safe_push (post_load_decls, decl); else { bool aggr = aggregate_value_p (DECL_RESULT (decl), decl); @@ -17267,6 +17282,33 @@ module_state::write_inits (elf_out *to, depset::hash &table, unsigned *crc_ptr) return count; } +/* We have to defer some post-load processing until we've completed + reading, because they can cause more reading. */ + +static void +post_load_processing () +{ + if (!post_load_decls) +return; + + tree old_cfd = current_function_decl; + struct function *old_cfun = cfun; + while (post_load_decls->length ()) +{ + tree decl = post_load_decls->pop (); + + dump () && dump ("Post-load processing of %N", decl); + + gcc_checking_assert (DECL_ABSTRACT_P (decl)); + /* Cloning can cause loading -- specifically operator delete for + the deleting dtor. */ + maybe_clone_body (decl); +} + + cfun = old_cfun; + current_function_decl = old_cfd; +} + bool module_state::read_inits (unsigned count) { @@ -17276,6 +17318,7 @@ module_state::read_inits (unsigned count) dump () && dump ("Reading %u initializers", count); dump.indent (); + lazy_snum = ~0u; for (unsigned ix = 0; ix != count; ix++) { /* Merely referencing the decl causes its initializer to be read @@ -17287,6 +17330,8 @@ module_state::read_inits (unsigned count) if (decl) dump ("Initializer:%u for %N", count, decl); } + lazy_snum = 0; + post_load_processing (); dump.outdent (); if (!sec.end (from ())) return false; @@ -18025,21 +18070,6 @@ module_state::read_preprocessor (bool outermost) return check_read (outermost, ok); } -static unsigned lazy_snum; - -static bool -recursive_lazy (unsigned snum = ~0u) -{ - if (lazy_snum) -{ - error_at (input_location, "recursive lazy load"); - return true; -} - - lazy_snum = snum; - return false; -} - /* Read language state. */ bool @@ -18114,16 +18144,15 @@ module_state::read_language (bool outermost) unsigned hwm = counts[MSC_sec_hwm]; for (unsigned ix = counts[MSC_sec_lwm]; ok && ix != hwm; ix++) - { - if (!load_section (ix, NULL)) - { - ok = false; - break; - } - ggc_collect (); - } - + if (!load_section (ix, NULL)) + { + ok = false; + break; + } lazy_snum = 0; + post_load_processing (); + + ggc_collect (); if (ok && CHECKING_P) for (unsigned ix = 0; ix != entity_num; ix++) @@ -18873,6 +18902,7 @@ lazy_load_binding (unsigned mod, tree ns, tree id, binding_slot *mslot) { ok = module->load_section (snum, mslot); lazy_snum = 0; + post_load_processing (); } dump.pop (n); @@ -18929,6 +18959,7 @@ lazy_load_specializations (tree tmpl) function_depth
c++: Redesign pending entity handling [PR 99170]
This patch addresses 99170. with modules (and in particular header units), one module can provide a (maybe nested) class or template and another module can provide a definition or (maybe partial) specialization of said entity, or member thereof. when both are imported into a 3rd TU, and that TU instantiates or uses the class, it needs to stream in those entities (in general). But how does it key those entities to the original? It can't /just/ use the entity index, because, when header-units and/or partitions are in play, the entity index /is not unique/. I had two complicated schemes that tried to unify that, but it failed. Here's a simpler scheme. Such pending entities are keyed to the namespace and identifier of the namespace-scope entity that contains them. Thus the final TU needs to find that entity and look in a hash table for lists of sections that need loading just before instantiating a template or looking inside a class. I would like to make this more efficient, but given the complex scheme failed, I'm shooting for correctness right now. There will be a follow up patch to complete the cleanup this enables. PR c++/99170 gcc/cp/ * cp-tree.h * lex.c (cxx_dup_lang_specific_decl): Adjust for module_attached_p rename. * module.cc (class pending_key): New. (default_hash_traits): New specialization. (pending_map_t): New typedef. (pending_table): Replace old table. (trees_out::lang_decl_bools): Adjust. (trees_in::lang_decl_bools): Adjust. (trees_in::install_entity): Drop pending member and specialization handling. (find_pending_key): New. (depset::hash::fiund_dependencies): Use it. (pendset_lazy_load): Delete. (module_state::write_cluster): Don't count pendings here. Bye Duff's device-like thing. (module_state::write_pendings): Reimplement. (module_state::read_pendings): Reimplement. (lazy_specializations_p): Delete. (module_state::write): Adjust write_pendings call. (lazy_load_pendings): New. (lazy_load_specializations): Delete. (lazy_load_members): Delete. (init_modules): Adjust. * name-lookup.c (maybe_lazily_declare): Call lazy_load_pendings not lazy_load_members. (note_pending_specializations): Delete. (load_pending_specializations): Delete. * name-lookup.h (BINDING_VECTR_PENDING_SPECIALIZATIONS_P): Delete. (BINDING_VECTOR_PENDING_MEMBERS_P): Delete. (BINDING_VECTR_PENDING_MEMBERS_P): Delete. (note_pending_specializations): Delete. (load_pending_specializations): Delete. * pt.c (lookup_template_class_1): Call lazy_load_pendings not lazy_load_specializations. (instantiate_template_class_1): Likewise. (instantiate_decl): Call lazy_load_pendings. * typeck.c (complete_type): Likewise. gcc/testsuite/ * g++.dg/modules/pr99170-1_a.H: New. * g++.dg/modules/pr99170-1_b.C: New. * g++.dg/modules/pr99170-2.h: New. * g++.dg/modules/pr99170-2_a.C: New. * g++.dg/modules/pr99170-2_b.C: New. * g++.dg/modules/pr99170-3_a.H: New. * g++.dg/modules/pr99170-3_b.C: New. * g++.dg/modules/inst-2_b.C: Adjust scan. * g++.dg/modules/inst-4_a.C: Adjust scan. * g++.dg/modules/inst-4_b.C: Adjust scan. * g++.dg/modules/member-def-1_b.C: Adjust scan. * g++.dg/modules/member-def-1_c.C: Adjust scan. * g++.dg/modules/tpl-spec-1_a.C: Adjust scan. * g++.dg/modules/tpl-spec-1_b.C: Adjust scan. * g++.dg/modules/tpl-spec-2_b.C: Adjust scan. * g++.dg/modules/tpl-spec-2_c.C: Adjust scan. * g++.dg/modules/tpl-spec-2_d.C: Adjust scan. * g++.dg/modules/tpl-spec-3_a.C: Adjust scan. * g++.dg/modules/tpl-spec-3_b.C: Adjust scan. * g++.dg/modules/tpl-spec-4_a.C: Adjust scan. * g++.dg/modules/tpl-spec-4_b.C: Adjust scan. * g++.dg/modules/tpl-spec-5_a.C: Adjust scan. * g++.dg/modules/tpl-spec-5_b.C: Adjust scan. -- Nathan Sidwell diff --git c/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index 699c50515e8..39e2ad83abd 100644 --- c/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -1678,21 +1678,10 @@ check_constraint_info (tree t) #define DECL_MODULE_ENTITY_P(NODE) \ (DECL_LANG_SPECIFIC (DECL_MODULE_CHECK (NODE))->u.base.module_entity_p) -/* True if there are unloaded specializations keyed to this template. */ -#define DECL_MODULE_PENDING_SPECIALIZATIONS_P(NODE) \ - (DECL_LANG_SPECIFIC (TEMPLATE_DECL_CHECK (NODE)) \ - ->u.base.module_pending_p) - -/* True if this class has unloaded members. These should be loaded - before we do member lookups. */ -#define DECL_MODULE_PENDING_MEMBERS_P(NODE) \ - (DECL_LANG_SPECIFIC (TYPE_DECL_CHECK (NODE)) \ - ->u.base.module_pending_p) - /* DE
Re: [PATCH] c-ppoutput: Fix preprocessing ICE on very large line number [PR99325]
On 3/4/21 4:03 AM, Jakub Jelinek wrote: Hi! In libcpp, lines are represented as linenum_type, which is unsigned int. The following testcases ICE because maybe_print_line_1 is sometimes called with UNKNOWN_LOCATION (e.g. at pragma eof) and while most of the time the && src_line >= print.src_line && src_line < print.src_line + 8 check doesn't succeed for the src_line of 0 from UNKNOWN_LOCATION, when print.src_line is from very large line numbers (UINT_MAX - 7 and above) it succeeds (with UB on the compiler side) but src_file is NULL for UNKNOWN_LOCATION and so the strcmp call ICEs. As print.src_line can easily wrap around, this patch changes its type to unsigned int to match libcpp, so that we don't invoke UB in the compiler. For print.src_line of UINT_MAX - 7 and above, src_line from UNKNOWN_LOCATION will not pass that test anymore, but when it wraps around to 0, it can, so I've also added a check for src_loc != UNKNOWN_LOCATION (or, if preferred, could be src_file != NULL). Besides fixing the ICE and UB in the compiler, I believe worst case the patch will cause printing a few more line directives in the preprocessed source around the wrapping from lines UINT_MAX - 7 to 0 (but less around the wrapping from INT_MAX to INT_MAX + 1U), but I think those are exceptional cases (sources with > 2billion lines are rare and we warn or error on #line > INT_MAX). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? 2021-03-03 Jakub Jelinek PR c/99325 * c-ppoutput.c (print): Change src_line type from int to unsigned. (token_streamer::stream) Likewise. (maybe_print_line_1): Likewise. Don't strcmp src_file if src_loc is UNKNOWN_LOCATION. ok. thanks! I think I noticed that signed/unsigned confusion when futzing around with modules, but didn't feel like diving in :) nathan -- Nathan Sidwell
c++: Post-pending redesign cleanup [PR 99170]
With pending entities reimplemented, the remaining use of uintset can just use a regular hash map -- I only used a uintset because it was there. So one adhoc hash-table/vector structure goes away. PR c++/99170 gcc/cp/ * module.cc (class uintset): Delete. (typedef attached_map_t): A hash map. (attached_table): Use attached_map_t. Adjust uses ... (trees_out::decl_value, trees_in::decl_value): ... here ... (trees_out::key_mergeable): ... here ... (trees_in::key_mergeable): ... here ... (maybe_attach_decl): ... here ... (direct_import): ... and here. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 3ee71e5211f..31bbf9776dd 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -2697,167 +2697,11 @@ pending_map_t *pending_table; completed. */ vec *post_load_decls; -/* Values keyed to some unsigned integer. This is not GTY'd, so if - T is tree they must be reachable via some other path. */ - -template -class uintset { -public: - unsigned key; /* Entity index of the other entity. */ - - /* Payload. */ - unsigned allocp2 : 5; /* log(2) allocated pending */ - unsigned num : 27;/* Number of pending. */ - - /* Trailing array of values. */ - T values[1]; - -public: - /* Even with ctors, we're very pod-like. */ - uintset (unsigned uid) -: key (uid), allocp2 (0), num (0) - { - } - /* Copy constructor, which is exciting because of the trailing - array. */ - uintset (const uintset *from) - { -size_t size = (offsetof (uintset, values) - + sizeof (uintset::values) * from->num); -memmove (this, from, size); -if (from->num) - allocp2++; - } - -public: - struct traits : delete_ptr_hash { -typedef unsigned compare_type; -typedef typename delete_ptr_hash::value_type value_type; - -/* Hash and equality for compare_type. */ -inline static hashval_t hash (const compare_type k) -{ - return hashval_t (k); -} -inline static hashval_t hash (const value_type v) -{ - return hash (v->key); -} - -inline static bool equal (const value_type v, const compare_type k) -{ - return v->key == k; -} - }; - -public: - class hash : public hash_table - { -typedef typename traits::compare_type key_t; -typedef hash_table parent; - - public: -hash (size_t size) - : parent (size) -{ -} -~hash () -{ -} - - private: -uintset **find_slot (key_t key, insert_option insert) -{ - return this->find_slot_with_hash (key, traits::hash (key), insert); -} - - public: -uintset *get (key_t key, bool extract = false); -bool add (key_t key, T value); -uintset *create (key_t key, unsigned num, T init = 0); - }; -}; - -/* Add VALUE to KEY's uintset, creating it if necessary. Returns true - if we created the uintset. */ - -template -bool -uintset::hash::add (typename uintset::hash::key_t key, T value) -{ - uintset **slot = this->find_slot (key, INSERT); - uintset *set = *slot; - bool is_new = !set; - - if (is_new || set->num == (1u << set->allocp2)) -{ - if (set) - { - unsigned n = set->num * 2; - size_t new_size = (offsetof (uintset, values) - + sizeof (uintset (0u).values) * n); - uintset *new_set = new (::operator new (new_size)) uintset (set); - delete set; - set = new_set; - } - else - set = new (::operator new (sizeof (*set))) uintset (key); - *slot = set; -} - - set->values[set->num++] = value; - - return is_new; -} - -template -uintset * -uintset::hash::create (typename uintset::hash::key_t key, unsigned num, - T init) -{ - unsigned p2alloc = 0; - for (unsigned v = num; v != 1; v = (v >> 1) | (v & 1)) -p2alloc++; - - size_t new_size = (offsetof (uintset, values) - + (sizeof (uintset (0u).values) << p2alloc)); - uintset *set = new (::operator new (new_size)) uintset (key); - set->allocp2 = p2alloc; - set->num = num; - while (num--) -set->values[num] = init; - - uintset **slot = this->find_slot (key, INSERT); - gcc_checking_assert (!*slot); - *slot = set; - - return set; -} - -/* Locate KEY's uintset, potentially removing it from the hash table */ - -template -uintset * -uintset::hash::get (typename uintset::hash::key_t key, bool extract) -{ - uintset *res = NULL; - - if (uintset **slot = this->find_slot (key, NO_INSERT)) -{ - res = *slot; - if (extract) - /* We need to remove the pendset without deleting it. */ - traits::mark_deleted (*slot); -} - - return res; -} - /* Some entities are attached to another entitity for ODR purposes. For example, at namespace scope, 'inline auto var = []{};', that lambda is attached to 'var', and follows its ODRness. */ -typedef uintset attachset; -sta
Re: [PATCH] coroutines : Adjust constraints on when to build ctors [PR98118].
On 3/4/21 2:59 PM, Iain Sandoe wrote: Hi, PR98118 shows that TYPE_NEEDS_CONSTRUCTING is a necessary, but not sufficient, condition for determining when we need to build a constructor. Use type_build_ctor_call() instead. tested on x86_64-darwin, x86_64-linux-gnu, [ ice on valid ] OK for master / 10.x? thanks Iain gcc/cp/ChangeLog: PR c++/98118 * coroutines.cc (build_co_await): Use type_build_ctor_call() to determine cases when a CTOR needs to be built. (flatten_await_stmt): Likewise. (morph_fn_to_coro): Likewise. ok, thanks -- Nathan Sidwell
Re: [PATCH] coroutines : Do not accept throwing final await expressions [PR95616].
On 3/4/21 2:54 PM, Iain Sandoe wrote: Hi, From the PR: The wording of [dcl.fct.def.coroutine]/15 states: * The expression co_await promise.final_suspend() shall not be potentially-throwing ([except.spec]). See http://eel.is/c++draft/dcl.fct.def.coroutine#15 and http://eel.is/c++draft/except.spec#6 ie. all of the following must be declared noexcept (if they form part of the await-expression): - promise_type::final_suspend() - finalSuspendObj.operator co_await() - finalSuspendAwaiter.await_ready() - finalSuspendAwaiter.await_suspend() - finalSuspendAwaiter.await_resume() - finalSuspedObj destructor - finalSuspendAwaiter destructor This implements the checks for these cases and rejects such code with a diagnostic. [ accepts invalid ] tested on x86_64-darwin, x86_64-linux-gnu, OK for master / 10.x? thanks Iain gcc/cp/ChangeLog: PR c++/95616 * coroutines.cc (coro_diagnose_throwing_fn): New helper. (coro_diagnose_throwing_final_aw_expr): New helper. (build_co_await): Diagnose throwing final await expression components. (build_init_or_final_await): Diagnose a throwing promise final_suspend() call. ok. Does this DTRT in the presence of -fno-exceptions? (i.e. use of that flag means you don't have to decorate everything with noexcept) nathan -- Nathan Sidwell
Re: [PATCH] coroutines : Handle exceptions throw before the first await_resume() [PR95615].
On 2/26/21 4:24 PM, Iain Sandoe wrote: Hi, The coroutine body is wrapped in a try-catch block which is responsible for handling any exceptions thrown by the original function body. Originally, the initial suspend expression was outside this, but an amendment to the standard places the await_resume call inside and everything else outside. This means that any exception thrown prior to the initial suspend expression await_resume() will propagate to the ramp function. However, some portion of the coroutine state will exist at that point (how much depends on where the exception is thrown from). For example, we might have some frame parameter copies, or the promise object or the return object any of which might have a non-trivial DTOR. Also the frame itself needs to be deallocated. This patch fixes the handling of these cases. tested on x86_64-darwin, x86_64-linux-gnu, OK for master / 10.x? thanks Iain gcc/cp/ChangeLog: PR c++/95615 * coroutines.cc (struct param_info): Track parameter copies that need a DTOR. (coro_get_frame_dtor): New helper function factored from build_actor(). (build_actor_fn): Use coro_get_frame_dtor(). (morph_fn_to_coro): Track parameters that need DTORs on exception, likewise the frame promise and the return object. On exception, run the DTORs for these, destroy the frame and then rethrow the exception. OK. I spotted a couple of 'VAR_DECL,get_identifier' (lack of space after comma). nathan -- Nathan Sidwell
c++: instantiating imported specializations [PR 99389]
When an incomplete class specialization is imported, and is completed by instantiation, we were failing to mark the instantiation, and thus didn't stream it out. Leading to errors in importing as we had members of an incomplete type. PR c++/99389 gcc/cp/ * pt.c (instantiate_class_template_1): Set instantiating module here. gcc/testsuite/ * g++.dg/modules/pr99389_a.H: New. * g++.dg/modules/pr99389_b.C: New. * g++.dg/modules/pr99389_c.C: New. -- Nathan Sidwell diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c index 83589101c0d..8ca3dc8ec2b 100644 --- c/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -11816,6 +11816,8 @@ instantiate_class_template_1 (tree type) input_location = DECL_SOURCE_LOCATION (TYPE_NAME (type)) = DECL_SOURCE_LOCATION (typedecl); + set_instantiating_module (TYPE_NAME (type)); + TYPE_PACKED (type) = TYPE_PACKED (pattern); SET_TYPE_ALIGN (type, TYPE_ALIGN (pattern)); TYPE_USER_ALIGN (type) = TYPE_USER_ALIGN (pattern); diff --git c/gcc/testsuite/g++.dg/modules/pr99389_a.H w/gcc/testsuite/g++.dg/modules/pr99389_a.H new file mode 100644 index 000..cbb64df7562 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99389_a.H @@ -0,0 +1,20 @@ +// PR 99389 failed to stream class specialization +// { dg-module-do link } +// { dg-additional-options "-fmodule-header" } +// { dg-module-cmi {} } +template +class basic_string_view +{ +public: + basic_string_view(const _CharT* __str) noexcept + {} + bool +empty() const noexcept + { return !_M_len; } + +private: + unsigned _M_len; +}; + +using string_view = basic_string_view; + diff --git c/gcc/testsuite/g++.dg/modules/pr99389_b.C w/gcc/testsuite/g++.dg/modules/pr99389_b.C new file mode 100644 index 000..f8d010e453d --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99389_b.C @@ -0,0 +1,12 @@ +// { dg-additional-options {-fmodules-ts -fdump-lang-module } } +export module hello; +// { dg-module-cmi hello } + +import "pr99389_a.H"; + +export inline bool Check (const string_view& n) +{ + return !n.empty (); +} + +// { dg-final { scan-lang-dump {Pending specialization '::basic_string_view' entity:. section:. keyed to '::basic_string_view'} module } } diff --git c/gcc/testsuite/g++.dg/modules/pr99389_c.C w/gcc/testsuite/g++.dg/modules/pr99389_c.C new file mode 100644 index 000..f31847dd928 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99389_c.C @@ -0,0 +1,7 @@ +// { dg-additional-options -fmodules-ts } +import hello; + +int main () +{ + return Check ("World") ? 0 : 1; +}
c++: Local instantiations of imported specializations [PR 99377]
This turned out to be the function version of the previous fix. We can import an implicit specialization declaration that we need to instantiate. We must mark the instantiation so we remember to stream it. PR c++/99377 gcc/cp/ * pt.c (instantiate_decl): Call set_instantiating_module. gcc/testsuite/ * g++.dg/modules/pr99377_a.H: New. * g++.dg/modules/pr99377_b.C: New. * g++.dg/modules/pr99377_c.C: New. -- Nathan Sidwell diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c index 8ca3dc8ec2b..1f3cd9c45f1 100644 --- c/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -26154,6 +26154,7 @@ instantiate_decl (tree d, bool defer_ok, bool expl_inst_class_mem_p) } else { + set_instantiating_module (d); if (variable_template_p (gen_tmpl)) note_variable_template_instantiation (d); instantiate_body (td, args, d, false); diff --git c/gcc/testsuite/g++.dg/modules/pr99377_a.H w/gcc/testsuite/g++.dg/modules/pr99377_a.H new file mode 100644 index 000..b5e5a3fea54 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99377_a.H @@ -0,0 +1,21 @@ +// PR 99377 failed to stream locally instantiated member +// link failure in function `Check(Widget const&)': +// bug_c.ii:(.text._Z5CheckRK6WidgetIiE[_Z5CheckRK6WidgetIiE]+0x14): undefined reference to `Widget::Second() const' +// { dg-module-do link } +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } +template +struct Widget +{ + Widget (int) { } + + bool First() const { return true; } + + bool Second () const { return true;} +}; + +inline void Frob (const Widget& w) noexcept +{ + w.First (); +} + diff --git c/gcc/testsuite/g++.dg/modules/pr99377_b.C w/gcc/testsuite/g++.dg/modules/pr99377_b.C new file mode 100644 index 000..77826379fc7 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99377_b.C @@ -0,0 +1,10 @@ +// { dg-additional-options -fmodules-ts } +export module Foo; +// { dg-module-cmi Foo } +import "pr99377_a.H"; + +export inline bool Check (const Widget& w) +{ + return w.Second (); +} + diff --git c/gcc/testsuite/g++.dg/modules/pr99377_c.C w/gcc/testsuite/g++.dg/modules/pr99377_c.C new file mode 100644 index 000..287388fa6dd --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99377_c.C @@ -0,0 +1,8 @@ +// { dg-additional-options -fmodules-ts } + +import Foo; + +int main () +{ + return Check (0) ? 0 : 1; +}
c++: Duplicate namespace bindings [PR 99245]
Header units can declare the same entity, and this can lead to one of them containing a (non-using) binding to an import. If one gets the cluster ordering just right, an assert will trigger. Relax that assert. PR c++/99245 gcc/cp/ * module.cc (module_state::write_cluster): Relax binding assert. gcc/testsuite/ * g++.dg/modules/pr99245_a.H: New. * g++.dg/modules/pr99245_b.H: New. -- Nathan Sidwell diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 31bbf9776dd..48862dd9bbc 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -14496,20 +14496,25 @@ module_state::write_cluster (elf_out *to, depset *scc[], unsigned size, gcc_unreachable (); case depset::EK_BINDING: - dump (dumper::CLUSTER) - && dump ("[%u]=%s %P", ix, b->entity_kind_name (), - b->get_entity (), b->get_name ()); - for (unsigned jx = b->deps.length (); jx--;) - { - depset *dep = b->deps[jx]; - if (jx) - gcc_checking_assert (dep->get_entity_kind () == depset::EK_USING - || TREE_VISITED (dep->get_entity ())); - else - gcc_checking_assert (dep->get_entity_kind () - == depset::EK_NAMESPACE - && dep->get_entity () == b->get_entity ()); - } + { + dump (dumper::CLUSTER) + && dump ("[%u]=%s %P", ix, b->entity_kind_name (), + b->get_entity (), b->get_name ()); + depset *ns_dep = b->deps[0]; + gcc_checking_assert (ns_dep->get_entity_kind () + == depset::EK_NAMESPACE + && ns_dep->get_entity () == b->get_entity ()); + for (unsigned jx = b->deps.length (); --jx;) + { + depset *dep = b->deps[jx]; + // We could be declaring something that is also a + // (merged) import + gcc_checking_assert (dep->is_import () + || TREE_VISITED (dep->get_entity ()) + || (dep->get_entity_kind () + == depset::EK_USING)); + } + } break; case depset::EK_DECL: diff --git c/gcc/testsuite/g++.dg/modules/pr99245_a.H w/gcc/testsuite/g++.dg/modules/pr99245_a.H new file mode 100644 index 000..94c6bf11995 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99245_a.H @@ -0,0 +1,5 @@ +// PR 99245 ICE writing out user of type_info +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } + +namespace std { class type_info {}; } diff --git c/gcc/testsuite/g++.dg/modules/pr99245_b.H w/gcc/testsuite/g++.dg/modules/pr99245_b.H new file mode 100644 index 000..548c2720ef5 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99245_b.H @@ -0,0 +1,9 @@ +// { dg-additional-options -fmodule-header } +// { dg-module-cmi {} } +namespace std { class type_info; } + +import "pr99245_a.H"; + +namespace std { + const type_info* __cxa_exception_type () noexcept; +}
c++: Incorrect specialization hash table [PR 99285]
Class template partial specializations need to be in the specialization hash, but not all of them. This defers adding streamed-in entities to the hash table, in the same way I deferred adding the instantiation and specialization lists for 99170. PR c++/99285 gcc/cp/ * cp-tree.h (match_mergeable_specialization) (add_mergeable_specialization): Adjust parms. * module.cc (trees_in::decl_value): Adjust add_mergeable_specialization calls. (trees_out::key_mergeable): Adjust match_mergeable_specialization calls. (specialization_add): Likewise. * pt.c (match_mergeable_specialization): Do not insert. (add_mergeable_specialization): Add to hash table here. gcc/testsuite/ * g++.dg/modules/pr99285_a.H: New. * g++.dg/modules/pr99285_b.H: New. -- Nathan Sidwell diff --git c/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index 39e2ad83abd..81ff375f8a5 100644 --- c/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -7239,11 +7239,10 @@ extern void walk_specializations (bool, void (*)(bool, spec_entry *, void *), void *); -extern tree match_mergeable_specialization (bool is_decl, spec_entry *, - bool insert = true); +extern tree match_mergeable_specialization (bool is_decl, spec_entry *); extern unsigned get_mergeable_specialization_flags (tree tmpl, tree spec); -extern void add_mergeable_specialization(tree tmpl, tree args, - tree spec, unsigned); +extern void add_mergeable_specialization(bool is_decl, spec_entry *, + tree outer, unsigned); extern tree add_outermost_template_args (tree, tree); extern tree add_extra_args (tree, tree); extern tree build_extra_args (tree, tree, tsubst_flags_t); diff --git c/gcc/cp/module.cc w/gcc/cp/module.cc index 48862dd9bbc..2518d73c220 100644 --- c/gcc/cp/module.cc +++ w/gcc/cp/module.cc @@ -8059,9 +8059,14 @@ trees_in::decl_value () set_constraints (decl, spec.spec); if (mk & MK_template_mask || mk == MK_partial) - /* Add to specialization tables now that constraints etc are - added. */ - add_mergeable_specialization (spec.tmpl, spec.args, decl, spec_flags); + { + /* Add to specialization tables now that constraints etc are + added. */ + bool is_decl = (mk & MK_template_mask) && (mk & MK_tmpl_decl_mask); + + spec.spec = is_decl ? inner : type; + add_mergeable_specialization (is_decl, &spec, decl, spec_flags); + } if (TREE_CODE (decl) == INTEGER_CST && !TREE_OVERFLOW (decl)) { @@ -8154,7 +8159,10 @@ trees_in::decl_value () { tree e = match_mergeable_specialization (true, &spec); if (!e) - add_mergeable_specialization (spec.tmpl, spec.args, decl, spec_flags); + { + spec.spec = inner; + add_mergeable_specialization (true, &spec, decl, spec_flags); + } else if (e != existing) set_overrun (); } @@ -10344,14 +10352,14 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, { /* Make sure we can locate the decl. */ tree existing = match_mergeable_specialization - (bool (mk & MK_tmpl_decl_mask), entry, false); + (bool (mk & MK_tmpl_decl_mask), entry); gcc_assert (existing); if (mk & MK_tmpl_decl_mask) { if (mk & MK_tmpl_alias_mask) /* It should be in both tables. */ - gcc_assert (match_mergeable_specialization (false, entry, false) + gcc_assert (match_mergeable_specialization (false, entry) == TREE_TYPE (existing)); else if (mk & MK_tmpl_tmpl_mask) if (tree ti = DECL_TEMPLATE_INFO (existing)) @@ -10659,6 +10667,7 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree decl, tree inner, if (mk & MK_template_mask) { + // FIXME: We could stream the specialization hash? spec_entry spec; spec.tmpl = tree_node (); spec.args = tree_node (); @@ -12869,7 +12878,7 @@ specialization_add (bool decl_p, spec_entry *entry, void *data_) /* Only alias templates can appear in both tables (and if they're in the type table they must also be in the decl table). */ gcc_checking_assert - (!match_mergeable_specialization (true, entry, false) + (!match_mergeable_specialization (true, entry) == (decl_p || !DECL_ALIAS_TEMPLATE_P (entry->tmpl))); } else if (VAR_OR_FUNCTION_DECL_P (entry->spec)) diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c index 81df8c88ccd..5e485f10d19 100644 --- c/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -29955,28 +29955,19 @@ walk_specializations (bool decls_p, } /* Lookup the specialization of *ELT, in the decl or type - specialization table. Return the SPEC that's already there (NULL if - nothing). If INSERT is true, and there was nothing, add the new - spec. */ + specialization table. Return the SPEC that's already there, or + NULL if nothing. */ tree -match_mergeable_specialization (bool decl_p, spec_e
c++: Poor diagnostic in header-unit [PR 99468]
We didn't specifically check for a module-decl inside a header unit. That leads to a confusing diagostic. Fixed thusly. gcc/cp/ * lex.c (module_token_filter::resume): Ignore module-decls inside header-unit. * parser.c (cp_parser_module_declaration): Reject in header-unit. gcc/testsuite/ * g++.dg/modules/pr99468.H: New. -- Nathan Sidwell diff --git c/gcc/cp/lex.c w/gcc/cp/lex.c index c83346b617d..73e14b8394c 100644 --- c/gcc/cp/lex.c +++ w/gcc/cp/lex.c @@ -515,7 +515,7 @@ struct module_token_filter { module_end:; /* End of the directive, handle the name. */ - if (import) + if (import && (is_import || !flag_header_unit)) if (module_state *m = preprocess_module (import, token_loc, module != NULL, is_import, got_export, reader)) diff --git c/gcc/cp/parser.c w/gcc/cp/parser.c index 378e4572f8b..f636bb746d4 100644 --- c/gcc/cp/parser.c +++ w/gcc/cp/parser.c @@ -13745,7 +13745,13 @@ cp_parser_module_declaration (cp_parser *parser, module_parse mp_state, parser->lexer->in_pragma = true; cp_token *token = cp_lexer_consume_token (parser->lexer); - if (mp_state == MP_FIRST && !exporting + if (flag_header_unit) +{ + error_at (token->location, + "module-declaration not permitted in header-unit"); + goto skip_eol; +} + else if (mp_state == MP_FIRST && !exporting && cp_lexer_next_token_is (parser->lexer, CPP_SEMICOLON)) { /* Start global module fragment. */ diff --git c/gcc/testsuite/g++.dg/modules/pr99468.H w/gcc/testsuite/g++.dg/modules/pr99468.H new file mode 100644 index 000..b6be0c349d5 --- /dev/null +++ w/gcc/testsuite/g++.dg/modules/pr99468.H @@ -0,0 +1,7 @@ +// PR 99468, stupidly worded diagnostic +// { dg-additional-options -fmodule-header } + +module M; // { dg-error "module-declaration not permitted" } + +int i; +// { dg-prune-output "not writing" }
C++: Enable c++2b module mode [PR 99436]
This adds support for c++23 mode to modules, and enables such testing. PR c++/99436 gcc/cp/ * name-lookup.c (get_cxx_dialect_name): Add cxx23. gcc/testsuite/ * g++.dg/modules/modules.exp (MOD_STD_LIST): Add 2b. -- Nathan Sidwell diff --git c/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index 092fa6b8768..28271ba0485 100644 --- c/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -6963,6 +6963,8 @@ get_cxx_dialect_name (enum cxx_dialect dialect) return "C++17"; case cxx20: return "C++20"; +case cxx23: + return "C++23"; } } diff --git c/gcc/testsuite/g++.dg/modules/modules.exp w/gcc/testsuite/g++.dg/modules/modules.exp index 38654caf7b9..da7afc2a226 100644 --- c/gcc/testsuite/g++.dg/modules/modules.exp +++ w/gcc/testsuite/g++.dg/modules/modules.exp @@ -35,7 +35,7 @@ if ![info exists DEFAULT_CXXFLAGS] then { set DEFAULT_CXXFLAGS " -pedantic-errors -Wno-long-long" } set DEFAULT_MODFLAGS $DEFAULT_CXXFLAGS -set MOD_STD_LIST { 17 2a } +set MOD_STD_LIST { 17 2a 2b } dg-init
i386: Fix array index in expander
I noticed a compiler warning about out-of-bound access. Fixed thusly. gcc/ * config/i386/sse.md (mov): Fix operand indices. pushed as obvious -- Nathan Sidwell diff --git i/gcc/config/i386/sse.md w/gcc/config/i386/sse.md index a728b979f01..a784346a23b 100644 --- i/gcc/config/i386/sse.md +++ w/gcc/config/i386/sse.md @@ -23491,7 +23491,7 @@ (define_expand "mov" (match_operand:MASK_DWI 1 "nonimmediate_operand"))] "TARGET_AVX512VP2INTERSECT" { - if (MEM_P (operands[1]) && MEM_P (operands[2])) + if (MEM_P (operands[0]) && MEM_P (operands[1])) operands[1] = force_reg (mode, operands[1]); })
Re: i386: Fix array index in expander
On 9/14/20 6:10 AM, Hongtao Liu wrote: On Mon, Sep 14, 2020 at 3:51 PM Richard Biener via Gcc-patches wrote: On Fri, Sep 11, 2020 at 11:19 PM Nathan Sidwell wrote: I noticed a compiler warning about out-of-bound access. Fixed thusly. gcc/ * config/i386/sse.md (mov): Fix operand indices. pushed as obvious looks like wrong-code as well so could you push to affected branches? GCC11 and GCC10 are affected. pushed to 10 -- Nathan Sidwell
c++: local externs in templates do not get template head
Now we consistently mark local externs with DECL_LOCAL_DECL_P, we can teach the template machinery not to give them a TEMPLATE_DECL head, and the instantiation machinery treat them as the local specialiations they are. (openmp UDRs also fall into this category, and are dealt with similarly.) gcc/cp/ * pt.c (push_template_decl_real): Don't attach a template head to local externs. (tsubst_function_decl): Add support for headless local extern decls. (tsubst_decl): Add support for headless local extern decls. pushed to trunk -- Nathan Sidwell diff --git i/gcc/cp/pt.c w/gcc/cp/pt.c index 0f52a9ed77d..8124efcbe24 100644 --- i/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -6071,7 +6071,11 @@ push_template_decl_real (tree decl, bool is_friend) { if (is_primary) retrofit_lang_decl (decl); - if (DECL_LANG_SPECIFIC (decl)) + if (DECL_LANG_SPECIFIC (decl) + && ((TREE_CODE (decl) != VAR_DECL + && TREE_CODE (decl) != FUNCTION_DECL) + || !ctx + || !DECL_LOCAL_DECL_P (decl))) DECL_TEMPLATE_INFO (decl) = info; } @@ -13701,14 +13705,20 @@ static tree tsubst_function_decl (tree t, tree args, tsubst_flags_t complain, tree lambda_fntype) { - tree gen_tmpl, argvec; + tree gen_tmpl = NULL_TREE, argvec = NULL_TREE; hashval_t hash = 0; tree in_decl = t; /* Nobody should be tsubst'ing into non-template functions. */ - gcc_assert (DECL_TEMPLATE_INFO (t) != NULL_TREE); + gcc_assert (DECL_TEMPLATE_INFO (t) != NULL_TREE + || DECL_LOCAL_DECL_P (t)); - if (TREE_CODE (DECL_TI_TEMPLATE (t)) == TEMPLATE_DECL) + if (DECL_LOCAL_DECL_P (t)) +{ + if (tree spec = retrieve_local_specialization (t)) + return spec; +} + else if (TREE_CODE (DECL_TI_TEMPLATE (t)) == TEMPLATE_DECL) { /* If T is not dependent, just return it. */ if (!uses_template_parms (DECL_TI_ARGS (t)) @@ -13958,6 +13968,11 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain, && !uses_template_parms (argvec)) tsubst_default_arguments (r, complain); } + else if (DECL_LOCAL_DECL_P (r)) +{ + if (!cp_unevaluated_operand) + register_local_specialization (r, t); +} else DECL_TEMPLATE_INFO (r) = NULL_TREE; @@ -14503,11 +14518,8 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) { tree argvec = NULL_TREE; tree gen_tmpl = NULL_TREE; - tree spec; tree tmpl = NULL_TREE; - tree ctx; tree type = NULL_TREE; - bool local_p; if (TREE_TYPE (t) == error_mark_node) RETURN (error_mark_node); @@ -14529,19 +14541,13 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) /* Check to see if we already have the specialization we need. */ - spec = NULL_TREE; - if (DECL_CLASS_SCOPE_P (t) || DECL_NAMESPACE_SCOPE_P (t)) + tree spec = NULL_TREE; + bool local_p = false; + tree ctx = DECL_CONTEXT (t); + if (!(VAR_P (t) && DECL_LOCAL_DECL_P (t)) + && (DECL_CLASS_SCOPE_P (t) || DECL_NAMESPACE_SCOPE_P (t))) { - /* T is a static data member or namespace-scope entity. - We have to substitute into namespace-scope variables - (not just variable templates) because of cases like: - - template void f() { extern T t; } - - where the entity referenced is not known until - instantiation time. */ local_p = false; - ctx = DECL_CONTEXT (t); if (DECL_CLASS_SCOPE_P (t)) { ctx = tsubst_aggr_type (ctx, args, @@ -14581,10 +14587,11 @@ tsubst_decl (tree t, tree args, tsubst_flags_t complain) } else { + if (!(VAR_P (t) && DECL_LOCAL_DECL_P (t))) + /* Subsequent calls to pushdecl will fill this in. */ + ctx = NULL_TREE; /* A local variable. */ local_p = true; - /* Subsequent calls to pushdecl will fill this in. */ - ctx = NULL_TREE; /* Unless this is a reference to a static variable from an enclosing function, in which case we need to fill it in now. */ if (TREE_STATIC (t))
Re: c++: local externs in templates do not get template head
On 9/14/20 12:49 PM, Marek Polacek wrote: On Mon, Sep 14, 2020 at 12:45:33PM -0400, Nathan Sidwell wrote: Now we consistently mark local externs with DECL_LOCAL_DECL_P, we can teach the template machinery not to give them a TEMPLATE_DECL head, and the instantiation machinery treat them as the local specialiations they are. (openmp UDRs also fall into this category, and are dealt with similarly.) gcc/cp/ * pt.c (push_template_decl_real): Don't attach a template head to local externs. (tsubst_function_decl): Add support for headless local extern decls. (tsubst_decl): Add support for headless local extern decls. pushed to trunk -- Nathan Sidwell diff --git i/gcc/cp/pt.c w/gcc/cp/pt.c index 0f52a9ed77d..8124efcbe24 100644 --- i/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -6071,7 +6071,11 @@ push_template_decl_real (tree decl, bool is_friend) { if (is_primary) retrofit_lang_decl (decl); - if (DECL_LANG_SPECIFIC (decl)) + if (DECL_LANG_SPECIFIC (decl) + && ((TREE_CODE (decl) != VAR_DECL + && TREE_CODE (decl) != FUNCTION_DECL) This is !VAR_OR_FUNCTION_DECL_P. Want me to "fix" that as obvious? ah, thanks -- great. I knew of VAR_P and the lack of FUNCTION_P, but not that one. (bah, who needs consistency!) -- Nathan Sidwell
Re: c++: local externs in templates do not get template head
On 9/14/20 6:47 PM, Tobias Burnus wrote: This patch cause run-time fails for g++ -fopenmp libgomp/testsuite/libgomp.c++/udr-13.C The follow-up fix does not help. Namely, in udr-3.C:115: 115 if (t.s != 11 || v.v != 9 || q != 0 || d != 3.0) abort (); (gdb) p t.s oops, I forgot the runtime tests are there -- it was so long ago! This unbreaks it, while I go develop a more robust patch. Turns out I didn't get OMP reductions correct. To address those I need to do some reorganization, so this patch just reverts the OMP-specific pieces of the local decl changes. gcc/cp/ * pt.c (push_template_decl_real): OMP reductions retain a template header. (tsubst_function_decl): Likewise. pushed to trunk nathan -- Nathan Sidwell diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c index c630ef5a070..1aea105edd5 100644 --- c/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -6072,9 +6072,11 @@ push_template_decl_real (tree decl, bool is_friend) if (is_primary) retrofit_lang_decl (decl); if (DECL_LANG_SPECIFIC (decl) - && (!VAR_OR_FUNCTION_DECL_P (decl) - || !ctx - || !DECL_LOCAL_DECL_P (decl))) + && !(VAR_OR_FUNCTION_DECL_P (decl) + && DECL_LOCAL_DECL_P (decl) + /* OMP reductions still need a template header. */ + && !(TREE_CODE (decl) == FUNCTION_DECL + && DECL_OMP_DECLARE_REDUCTION_P (decl DECL_TEMPLATE_INFO (decl) = info; } @@ -13712,7 +13714,8 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain, gcc_assert (DECL_TEMPLATE_INFO (t) != NULL_TREE || DECL_LOCAL_DECL_P (t)); - if (DECL_LOCAL_DECL_P (t)) + if (DECL_LOCAL_DECL_P (t) + && !DECL_OMP_DECLARE_REDUCTION_P (t)) { if (tree spec = retrieve_local_specialization (t)) return spec; @@ -13967,7 +13970,8 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain, && !uses_template_parms (argvec)) tsubst_default_arguments (r, complain); } - else if (DECL_LOCAL_DECL_P (r)) + else if (DECL_LOCAL_DECL_P (r) + && !DECL_OMP_DECLARE_REDUCTION_P (r)) { if (!cp_unevaluated_operand) register_local_specialization (r, t);
openmp question
Jakub, is libgomp/testsuite/libgomp.c++/udr-3.C well formed? Specifically V::baz? If you remove the templatedness and plug in typedefs for T & D on its only instantiation (as the attached does), you get the following errors: devvm293:414>g++ -std=c++11 -c -fopenmp udr-3.C udr-3.C: In member function 'void V::baz()': udr-3.C:112:27: error: no matching function for call to 'W::W(int)' 112 | initializer (omp_priv (12)) | ^ udr-3.C:88:3: note: candidate: 'W::W()' 88 | W () : v (6) {} | ^ udr-3.C:88:3: note: candidate expects 0 arguments, 1 provided udr-3.C:85:8: note: candidate: 'constexpr W::W(const W&)' 85 | struct W |^ udr-3.C:85:8: note: no known conversion for argument 1 from 'int' to 'const W&' udr-3.C:115:26: error: no matching function for call to 'W::W(int)' 115 | initializer (omp_priv (9)) | ^ udr-3.C:88:3: note: candidate: 'W::W()' 88 | W () : v (6) {} | ^ udr-3.C:88:3: note: candidate expects 0 arguments, 1 provided udr-3.C:85:8: note: candidate: 'constexpr W::W(const W&)' 85 | struct W |^ udr-3.C:85:8: note: no known conversion for argument 1 from 'int' to 'const W&' It would seem to me you should get the same missing ctor errors when it's an instantiated template. And such errors are what I observe with my WIP dealing with my recent breakage of that test. (adding the missing ctor makes the test pass) Also, it seems cp_check_omp_declare_reduction (d) doesn;t set the reduction to a suitably formed error_mark_node on failure-- so downstream consumers can see the rejected cases. Is there a reason for that? nathan -- Nathan Sidwell // { dg-do run } extern "C" void abort (); void dblinit (double *p) { *p = 2.0; } namespace NS { template struct U { void foo (U &, bool); U (); }; template struct S { int s; #pragma omp declare reduction (foo : U<0>, S : omp_out.foo (omp_in, false)) #pragma omp declare reduction (foo : int : omp_out += omp_in) \ initializer (omp_priv = N + 2) #pragma omp declare reduction (foo : double : omp_out += omp_in) \ initializer (dblinit (&omp_priv)) void baz (int v) { S s; int q = 0; if (s.s != 6 || v != 0) abort (); s.s = 20; double d = 4.0; #pragma omp parallel num_threads (4) reduction (foo : s, v, d) \ reduction (::NS::U::operator + : q) { if (s.s != 6 || q != 0 || v != N + 2 || d != 2.0) abort (); asm volatile ("" : "+m" (s.s), "+r" (q), "+r" (v)); s.s++; q++; v++; } if (s.s != 20 + q * 7 || (N + 3) * q != v || d != 4.0 + 2.0 * q) abort (); } void foo (S &x) { s += x.s; } void foo (S &x, bool y) { s += x.s; if (y) abort (); } S (const S &x) { s = x.s + 1; } S (const S &x, bool y) { s = x.s + 2; if (y) abort (); } S () { s = 6; } S (int x) { s = x; } ~S (); }; #pragma omp declare reduction (bar : S<1> : omp_out.foo (omp_in)) \ initializer (omp_priv (8)) } template NS::S::~S () { if (s < 6) abort (); s = -1; /* Ensure the above store is not DSEd. */ asm volatile ("" : : "r" (&s) : "memory"); } template struct T : public NS::S { void baz () { NS::S s; int q = 0; if (s.s != 6) abort (); #pragma omp parallel num_threads (4) reduction (foo:s) \ reduction (+: q) { if (s.s != 6 || q != 0) abort (); asm volatile ("" : "+m" (s.s), "+r" (q)); s.s += 2; q++; } if (s.s != 6 + q * 8) abort (); } }; struct W { int v; W () : v (6) {} // ADD to fix: W (int i) : v (i) {} ~W () {} }; // Remove templatedness // template struct V { using T =NS::S<0>; // Types instantiated oer using D= double; // Likewise #pragma omp declare reduction (baz: T: omp_out.s += omp_in.s) \ initializer (omp_priv (11)) #pragma omp declare reduction (baz: D: omp_out += omp_in) \ initializer (dblinit (&omp_priv)) static void dblinit (D *x) { *x = 3.0; } void baz () { T t; V v; int q = 0; D d = 4.0; if (t.s != 6 || v.v != 4) abort (); #pragma omp declare reduction (+ : V, W : omp_out.v -= omp_in.v) \ initializer (omp_priv (12)) { #pragma omp declare reduction (+ : W, V : omp_out.v += omp_in.v) \ initializer (omp_priv (9)) #pragma omp parallel num_threads (4) reduction (+: v, q) \ reduction (baz: t, d) { if (t.s != 11 || v.v != 9 || q != 0 || d != 3.0) abort (); asm volatile ("" : "+m" (t.s), "+m" (v.v), "+r" (q)); t.s += 2; v.v += 3; q++; } if (t.s != 6 + 13 * q || v.v != 4 + 12 * q || d != 4.0 + 3.0 * q) abort (); } }
c++: Break out actual instantiation from instantiate_decl
This refactors instantiate_decl, breaking out the actual instantiation work to instantiate_body. That'll allow me to address the OMP UDR issue, but it also means we have slightly neater code in instantiate_decl anyway. gcc/cp/ * pt.c (instantiate_body): New, broken out of .. (instantiate_decl): ... here. Call it. pushing to trunk -- Nathan Sidwell diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c index 1aea105edd5..e8aa950b8fa 100644 --- c/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -25447,6 +25447,158 @@ register_parameter_specializations (tree pattern, tree inst) gcc_assert (!spec_parm); } +/* Instantiate the body of D using PATTERN with ARGS. We have + already determined PATTERN is the correct template to use. */ + +static void +instantiate_body (tree pattern, tree args, tree d) +{ + gcc_checking_assert (TREE_CODE (pattern) == TEMPLATE_DECL); + + tree td = pattern; + tree code_pattern = DECL_TEMPLATE_RESULT (td); + + tree fn_context = decl_function_context (d); + if (LAMBDA_FUNCTION_P (d)) +/* tsubst_lambda_expr resolved any references to enclosing functions. */ +fn_context = NULL_TREE; + bool nested = current_function_decl != NULL_TREE; + bool push_to_top = !(nested && fn_context == current_function_decl); + + vec omp_privatization_save; + if (nested) +save_omp_privatization_clauses (omp_privatization_save); + + if (push_to_top) +push_to_top_level (); + else +{ + gcc_assert (!processing_template_decl); + push_function_context (); + cp_unevaluated_operand = 0; + c_inhibit_evaluation_warnings = 0; +} + + if (VAR_P (d)) +{ + /* The variable might be a lambda's extra scope, and that + lambda's visibility depends on D's. */ + maybe_commonize_var (d); + determine_visibility (d); +} + + /* Mark D as instantiated so that recursive calls to + instantiate_decl do not try to instantiate it again. */ + DECL_TEMPLATE_INSTANTIATED (d) = 1; + + /* Regenerate the declaration in case the template has been modified + by a subsequent redeclaration. */ + regenerate_decl_from_template (d, td, args); + + /* We already set the file and line above. Reset them now in case + they changed as a result of calling regenerate_decl_from_template. */ + input_location = DECL_SOURCE_LOCATION (d); + + if (VAR_P (d)) +{ + tree init; + bool const_init = false; + + /* Clear out DECL_RTL; whatever was there before may not be right + since we've reset the type of the declaration. */ + SET_DECL_RTL (d, NULL); + DECL_IN_AGGR_P (d) = 0; + + /* The initializer is placed in DECL_INITIAL by + regenerate_decl_from_template so we don't need to + push/pop_access_scope again here. Pull it out so that + cp_finish_decl can process it. */ + init = DECL_INITIAL (d); + DECL_INITIAL (d) = NULL_TREE; + DECL_INITIALIZED_P (d) = 0; + + /* Clear DECL_EXTERNAL so that cp_finish_decl will process the + initializer. That function will defer actual emission until + we have a chance to determine linkage. */ + DECL_EXTERNAL (d) = 0; + + /* Enter the scope of D so that access-checking works correctly. */ + bool enter_context = DECL_CLASS_SCOPE_P (d); + if (enter_context) +push_nested_class (DECL_CONTEXT (d)); + + const_init = DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (code_pattern); + cp_finish_decl (d, init, const_init, NULL_TREE, 0); + + if (enter_context) +pop_nested_class (); +} + else if (TREE_CODE (d) == FUNCTION_DECL && DECL_DEFAULTED_FN (code_pattern)) +synthesize_method (d); + else if (TREE_CODE (d) == FUNCTION_DECL) +{ + /* Set up the list of local specializations. */ + local_specialization_stack lss (push_to_top ? lss_blank : lss_copy); + tree block = NULL_TREE; + + /* Set up context. */ + if (DECL_OMP_DECLARE_REDUCTION_P (code_pattern) + && TREE_CODE (DECL_CONTEXT (code_pattern)) == FUNCTION_DECL) + block = push_stmt_list (); + else + start_preparsed_function (d, NULL_TREE, SF_PRE_PARSED); + + perform_instantiation_time_access_checks (code_pattern, args); + + /* Create substitution entries for the parameters. */ + register_parameter_specializations (code_pattern, d); + + /* Substitute into the body of the function. */ + if (DECL_OMP_DECLARE_REDUCTION_P (code_pattern)) + tsubst_omp_udr (DECL_SAVED_TREE (code_pattern), args, + tf_warning_or_error, DECL_TI_TEMPLATE (d)); + else + { + tsubst_expr (DECL_SAVED_TREE (code_pattern), args, + tf_warning_or_error, DECL_TI_TEMPLATE (d), + /*integral_constant_expression_p=*/false); + + /* Set the current input_location to the end of the function + so that finish_function knows where we are. */ + input_location + = DECL_STRUCT_FUNCTION (code_pattern)->function_end_locus; + +
c++: Avoid confusing 'nested' name
instantiate_body has a local var call 'nested', which indicates that this instantiation was caused during the body of some function -- not necessarily its containing scope. That's confusing, let's just use 'current_function_decl' directly. Then we can also simplify the push_to_top_level logic, which /does/ indicate whether this is an actual nested function. (C++ does not have nested functions, but OMP ODRs fall into that category. A follow up patch will use that more usual meaning of 'nested' wrt to functions.) gcc/cp/ * pt.c (instantiate_body): Remove 'nested' var, simplify push_to_top logic. pushing to trunk -- Nathan Sidwell diff --git i/gcc/cp/pt.c w/gcc/cp/pt.c index e8aa950b8fa..289452ab740 100644 --- i/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -25458,17 +25458,15 @@ instantiate_body (tree pattern, tree args, tree d) tree td = pattern; tree code_pattern = DECL_TEMPLATE_RESULT (td); - tree fn_context = decl_function_context (d); - if (LAMBDA_FUNCTION_P (d)) -/* tsubst_lambda_expr resolved any references to enclosing functions. */ -fn_context = NULL_TREE; - bool nested = current_function_decl != NULL_TREE; - bool push_to_top = !(nested && fn_context == current_function_decl); - vec omp_privatization_save; - if (nested) + if (current_function_decl) save_omp_privatization_clauses (omp_privatization_save); + bool push_to_top += !(current_function_decl + && !LAMBDA_FUNCTION_P (d) + && decl_function_context (d) == current_function_decl); + if (push_to_top) push_to_top_level (); else @@ -25595,7 +25593,7 @@ instantiate_body (tree pattern, tree args, tree d) else pop_function_context (); - if (nested) + if (current_function_decl) restore_omp_privatization_clauses (omp_privatization_save); }
c++: local-scope OMP UDR reductions have no template head
Jakub, are you ok with the bool return from cp_check_omp_declare_reduction? That seemed like the least invasive change. This corrects the earlier problems with removing the template header from local omp reductions. And it uncovered a latent bug. When we tsubst such a decl, we immediately tsubst its body. cp_check_omp_declare_reduction gets a success return value to gate that instantiation. udr-2.C got a further error, as the omp checking machinery doesn't appear to turn the reduction into an error mark when failing. I didn't dig into that further. udr-3.C appears to have been invalid and accidentally worked. gcc/cp/ * cp-tree.h (cp_check_omp_declare_reduction): Return bool. * semantics.c (cp_check_omp_declare_reduction): Return true on for success. * pt.c (push_template_decl_real): OMP reductions do not get a template header. (tsubst_function_decl): Remove special casing for local decl omp reductions. (tsubst_expr): Call instantiate_body for a local omp reduction. (instantiate_body): Add nested_p parm, and deal with such instantiations. (instantiate_decl): Reject FUNCTION_SCOPE entities, adjust instantiate_body call. gcc/testsuite/ * g++.dg/gomp/udr-2.C: Add additional expected error. libgomp/ * testsuite/libgomp.c++/udr-3.C: Add missing ctor. -- Nathan Sidwell diff --git i/gcc/c-family/c-opts.c w/gcc/c-family/c-opts.c index 23934416f64..24e21ce0d0c 100644 --- i/gcc/c-family/c-opts.c +++ w/gcc/c-family/c-opts.c @@ -1129,7 +1129,10 @@ c_common_post_options (const char **pfilename) input_location = UNKNOWN_LOCATION; *pfilename = this_input_filename -= cpp_read_main_file (parse_in, in_fnames[0], !cpp_opts->preprocessed); += cpp_read_main_file (parse_in, in_fnames[0], + /* We'll inject preamble pieces if this is + not preprocessed. */ + !cpp_opts->preprocessed); /* Don't do any compilation or preprocessing if there is no input file. */ if (this_input_filename == NULL) { @@ -1453,6 +1456,7 @@ c_finish_options (void) = linemap_check_ordinary (linemap_add (line_table, LC_RENAME, 0, _(""), 0)); cb_file_change (parse_in, bltin_map); + linemap_line_start (line_table, 0, 1); /* Make sure all of the builtins about to be declared have BUILTINS_LOCATION has their location_t. */ @@ -1476,6 +1480,7 @@ c_finish_options (void) = linemap_check_ordinary (linemap_add (line_table, LC_RENAME, 0, _(""), 0)); cb_file_change (parse_in, cmd_map); + linemap_line_start (line_table, 0, 1); /* All command line defines must have the same location. */ cpp_force_token_locations (parse_in, cmd_map->start_location);
Re: c++: local-scope OMP UDR reductions have no template head
On 9/16/20 2:31 PM, Jakub Jelinek wrote: On Wed, Sep 16, 2020 at 02:11:32PM -0400, Nathan Sidwell wrote: Jakub, are you ok with the bool return from cp_check_omp_declare_reduction? That seemed like the least invasive change. This corrects the earlier problems with removing the template header from local omp reductions. And it uncovered a latent bug. When we tsubst such a decl, we immediately tsubst its body. cp_check_omp_declare_reduction gets a success return value to gate that instantiation. udr-2.C got a further error, as the omp checking machinery doesn't appear to turn the reduction into an error mark when failing. I didn't dig into that further. udr-3.C appears to have been invalid and accidentally worked. The attached patch doesn't match the ChangeLog... here's the right one, problem with ordering by time or name :( nathan -- Nathan Sidwell diff --git i/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index 6e4de7d0c4b..5b727348e51 100644 --- i/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -7228,7 +7228,7 @@ extern void simplify_aggr_init_expr (tree *); extern void finalize_nrv (tree *, tree, tree); extern tree omp_reduction_id (enum tree_code, tree, tree); extern tree cp_remove_omp_priv_cleanup_stmt (tree *, int *, void *); -extern void cp_check_omp_declare_reduction (tree); +extern bool cp_check_omp_declare_reduction (tree); extern void finish_omp_declare_simd_methods (tree); extern tree finish_omp_clauses (tree, enum c_omp_region_type); extern tree push_omp_privatization_clauses (bool); diff --git i/gcc/cp/pt.c w/gcc/cp/pt.c index 289452ab740..cfe5ff4a94f 100644 --- i/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -227,6 +227,7 @@ static tree canonicalize_expr_argument (tree, tsubst_flags_t); static tree make_argument_pack (tree); static void register_parameter_specializations (tree, tree); static tree enclosing_instantiation_of (tree tctx); +static void instantiate_body (tree pattern, tree args, tree d, bool nested); /* Make the current scope suitable for access checking when we are processing T. T can be FUNCTION_DECL for instantiated function @@ -6073,10 +6074,7 @@ push_template_decl_real (tree decl, bool is_friend) retrofit_lang_decl (decl); if (DECL_LANG_SPECIFIC (decl) && !(VAR_OR_FUNCTION_DECL_P (decl) - && DECL_LOCAL_DECL_P (decl) - /* OMP reductions still need a template header. */ - && !(TREE_CODE (decl) == FUNCTION_DECL - && DECL_OMP_DECLARE_REDUCTION_P (decl + && DECL_LOCAL_DECL_P (decl))) DECL_TEMPLATE_INFO (decl) = info; } @@ -13714,8 +13712,7 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain, gcc_assert (DECL_TEMPLATE_INFO (t) != NULL_TREE || DECL_LOCAL_DECL_P (t)); - if (DECL_LOCAL_DECL_P (t) - && !DECL_OMP_DECLARE_REDUCTION_P (t)) + if (DECL_LOCAL_DECL_P (t)) { if (tree spec = retrieve_local_specialization (t)) return spec; @@ -13970,8 +13967,7 @@ tsubst_function_decl (tree t, tree args, tsubst_flags_t complain, && !uses_template_parms (argvec)) tsubst_default_arguments (r, complain); } - else if (DECL_LOCAL_DECL_P (r) - && !DECL_OMP_DECLARE_REDUCTION_P (r)) + else if (DECL_LOCAL_DECL_P (r)) { if (!cp_unevaluated_operand) register_local_specialization (r, t); @@ -18083,7 +18079,8 @@ tsubst_expr (tree t, tree args, tsubst_flags_t complain, tree in_decl, DECL_CONTEXT (decl) = global_namespace; pushdecl (decl); DECL_CONTEXT (decl) = current_function_decl; - cp_check_omp_declare_reduction (decl); + if (cp_check_omp_declare_reduction (decl)) + instantiate_body (pattern_decl, args, decl, true); } else { @@ -25448,15 +25445,24 @@ register_parameter_specializations (tree pattern, tree inst) } /* Instantiate the body of D using PATTERN with ARGS. We have - already determined PATTERN is the correct template to use. */ + already determined PATTERN is the correct template to use. + NESTED_P is true if this is a nested function, in which case + PATTERN will be a FUNCTION_DECL not a TEMPLATE_DECL. */ static void -instantiate_body (tree pattern, tree args, tree d) +instantiate_body (tree pattern, tree args, tree d, bool nested_p) { - gcc_checking_assert (TREE_CODE (pattern) == TEMPLATE_DECL); - - tree td = pattern; - tree code_pattern = DECL_TEMPLATE_RESULT (td); + tree td = NULL_TREE; + tree code_pattern = pattern; + + if (!nested_p) +{ + td = pattern; + code_pattern = DECL_TEMPLATE_RESULT (td); +} + else +/* Only OMP reductions are nested. */ +gcc_checking_assert (DECL_OMP_DECLARE_REDUCTION_P (code_pattern)); vec omp_privatization_save; if (current_function_decl) @@ -25489,9 +25495,10 @@ instantiate_body (tree pattern, tree args, tree d) instantiate_decl do not try to instantiate it again. */ DECL_TEMPLATE_INSTANTIATED (d) = 1;
C++RFC: Extending __builtin_FUNCTION
__builtin_FUNCTION () returns __FUNCTION__ of its invoker. Any thoughts about extending it to provide: a) the qualified name of the caller b) the full signature of the invoker (__PRETTY_FUNCTION__) we could do this with another __builtin, or permitting __builtin_FUNCTION to take an optional (or defaulted?) constant arg. Something like: __builtin_FUNCTION () -> current behavior __builtin_FUNCTION (0) -> as current __builtin_FUNCTION (1) -> as qualified name __builtin_FUNCTION (2) -> as __PRETTY_FUNCTION__ thoughts? nathan -- Nathan Sidwell
c++: ts_lambda is not needed
I've been forced[*] to look at the bits of name-lookup I ran away from when reimplementing namespace-scope lookup at the beginning of this modules project. Here's the first change in an expected series. We don't need ts_lambda, as IDENTIFIER_LAMBDA_P is sufficient. Killed thusly. gcc/cp/ * decl.c (xref_tag_1): Use IDENTIFIER_LAMBDA_P to detect lambdas. * lambda.c (begin_lambda_type): Use ts_current to push the tag. * name-lookup.h (enum tag_scope): Drop ts_lambda. pushed to trunk. nathan [*] I can only blame myself :) -- Nathan Sidwell diff --git i/gcc/cp/decl.c w/gcc/cp/decl.c index af796499df7..bbecebe7a62 100644 --- i/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -14857,10 +14857,10 @@ check_elaborated_type_specifier (enum tag_types tag_code, return type; } -/* Lookup NAME in elaborate type specifier in scope according to - SCOPE and issue diagnostics if necessary. - Return *_TYPE node upon success, NULL_TREE when the NAME is not - found, and ERROR_MARK_NODE for type error. */ +/* Lookup NAME of an elaborated type specifier according to SCOPE and + issue diagnostics if necessary. Return *_TYPE node upon success, + NULL_TREE when the NAME is not found, and ERROR_MARK_NODE for type + error. */ static tree lookup_and_check_tag (enum tag_types tag_code, tree name, @@ -14997,9 +14997,9 @@ xref_tag_1 (enum tag_types tag_code, tree name, /* In case of anonymous name, xref_tag is only called to make type node and push name. Name lookup is not required. */ tree t = NULL_TREE; - if (scope != ts_lambda && !IDENTIFIER_ANON_P (name)) + if (!IDENTIFIER_ANON_P (name)) t = lookup_and_check_tag (tag_code, name, scope, template_header_p); - + if (t == error_mark_node) return error_mark_node; @@ -15052,19 +15052,14 @@ xref_tag_1 (enum tag_types tag_code, tree name, error ("use of enum %q#D without previous declaration", name); return error_mark_node; } - else - { - t = make_class_type (code); - TYPE_CONTEXT (t) = context; - if (scope == ts_lambda) - { - /* Mark it as a lambda type. */ - CLASSTYPE_LAMBDA_EXPR (t) = error_mark_node; - /* And push it into current scope. */ - scope = ts_current; - } - t = pushtag (name, t, scope); - } + + t = make_class_type (code); + TYPE_CONTEXT (t) = context; + if (IDENTIFIER_LAMBDA_P (name)) + /* Mark it as a lambda type right now. Our caller will + correct the value. */ + CLASSTYPE_LAMBDA_EXPR (t) = error_mark_node; + t = pushtag (name, t, scope); } else { diff --git i/gcc/cp/lambda.c w/gcc/cp/lambda.c index c94fe8edb8e..364a3e9f6b9 100644 --- i/gcc/cp/lambda.c +++ w/gcc/cp/lambda.c @@ -135,7 +135,7 @@ begin_lambda_type (tree lambda) /* Create the new RECORD_TYPE for this lambda. */ tree type = xref_tag (/*tag_code=*/record_type, name, - /*scope=*/ts_lambda, /*template_header_p=*/false); + /*scope=*/ts_current, /*template_header_p=*/false); if (type == error_mark_node) return error_mark_node; diff --git i/gcc/cp/name-lookup.h w/gcc/cp/name-lookup.h index 723fbb0008c..a0815e1a0ac 100644 --- i/gcc/cp/name-lookup.h +++ w/gcc/cp/name-lookup.h @@ -139,7 +139,6 @@ enum tag_scope { only, for friend class lookup according to [namespace.memdef]/3 and [class.friend]/9. */ - ts_lambda = 3 /* Declaring a lambda closure. */ }; struct GTY(()) cp_class_binding {
c++: fix injected friend of template class
In working on fixing hiddenness, I discovered some suspicious code in template instantiation. I suspect it dates from when we didn't do the hidden friend injection thing at all. The xreftag finds the same class, but makes it visible to name lookup. Which is wrong. hurrah, fixing a bug by deleting code! gcc/cp/ * pt.c (instantiate_class_template_1): Do not repush and unhide injected friend. gcc/testsuite/ * g++.old-deja/g++.pt/friend34.C: Check injected friend is still invisible. pushed to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/pt.c w/gcc/cp/pt.c index 97d0c245f7e..44ca14afc4e 100644 --- i/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -12030,25 +12030,6 @@ instantiate_class_template_1 (tree type) adjust_processing_template_decl = true; --processing_template_decl; } - else if (TREE_CODE (friend_type) != BOUND_TEMPLATE_TEMPLATE_PARM - && !CLASSTYPE_USE_TEMPLATE (friend_type) - && TYPE_HIDDEN_P (friend_type)) - { - /* friend class C; - - where C hasn't been declared yet. Let's lookup name - from namespace scope directly, bypassing any name that - come from dependent base class. */ - tree ns = decl_namespace_context (TYPE_MAIN_DECL (friend_type)); - - /* The call to xref_tag_from_type does injection for friend - classes. */ - push_nested_namespace (ns); - friend_type = - xref_tag_from_type (friend_type, NULL_TREE, - /*tag_scope=*/ts_current); - pop_nested_namespace (ns); - } else if (uses_template_parms (friend_type)) /* friend class C; */ friend_type = tsubst (friend_type, args, diff --git i/gcc/testsuite/g++.old-deja/g++.pt/friend34.C w/gcc/testsuite/g++.old-deja/g++.pt/friend34.C index 5e80ab98b2e..dcd6df0ce55 100644 --- i/gcc/testsuite/g++.old-deja/g++.pt/friend34.C +++ w/gcc/testsuite/g++.old-deja/g++.pt/friend34.C @@ -6,9 +6,12 @@ template class bar { public: - friend class foo; // this is not bar::foo, it forward-declares ::foo + friend class foo; // this is not bar::foo, it injects hidden ::foo class foo {}; bar() { foo(); } // but this should refer to bar::foo }; bar<> baz; + +// We still have not made foo visible. +foo *b; // { dg-error "does not name a type" }
c++: Remove a broken error-recovery path
The remaining use of xref_tag_from_type was also suspicious. It turns out to be an error path. At parse time we diagnose that a class definition cannot appear, but we swallow the definition. This code was attempting to push it into the global scope (or find a conflict). This seems needless, just return error_mark_node. This was the simpler fix than going through the parser and figuring out how to get it to put in error_mark_node at the right point. gcc/cp/ * cp-tree.h (xref_tag_from_type): Don't declare. * decl.c (xref_tag_from_type): Delete. * pt.c (lookup_template_class_1): Erroneously located class definitions just give error_mark, don't try and inject it into the namespace. pushed to trunk -- Nathan Sidwell diff --git i/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index 71353814973..029a165a3e8 100644 --- i/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -6502,7 +6502,6 @@ extern void grok_special_member_properties (tree); extern bool grok_ctor_properties (const_tree, const_tree); extern bool grok_op_properties (tree, bool); extern tree xref_tag(enum tag_types, tree, tag_scope, bool); -extern tree xref_tag_from_type (tree, tree, tag_scope); extern void xref_basetypes (tree, tree); extern tree start_enum(tree, tree, tree, tree, bool, bool *); extern void finish_enum_value_list (tree); diff --git i/gcc/cp/decl.c w/gcc/cp/decl.c index bbecebe7a62..f3fdfe3d896 100644 --- i/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -15120,23 +15120,6 @@ xref_tag (enum tag_types tag_code, tree name, return ret; } - -tree -xref_tag_from_type (tree old, tree id, tag_scope scope) -{ - enum tag_types tag_kind; - - if (TREE_CODE (old) == RECORD_TYPE) -tag_kind = (CLASSTYPE_DECLARED_CLASS (old) ? class_type : record_type); - else -tag_kind = union_type; - - if (id == NULL_TREE) -id = TYPE_IDENTIFIER (old); - - return xref_tag (tag_kind, id, scope, false); -} - /* Create the binfo hierarchy for REF with (possibly NULL) base list BASE_LIST. For each element on BASE_LIST the TREE_PURPOSE is an access_* node, and the TREE_VALUE is the type of the base-class. diff --git i/gcc/cp/pt.c w/gcc/cp/pt.c index 44ca14afc4e..69946da09bf 100644 --- i/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -9856,12 +9856,11 @@ lookup_template_class_1 (tree d1, tree arglist, tree in_decl, tree context, && !PRIMARY_TEMPLATE_P (gen_tmpl) && !LAMBDA_TYPE_P (TREE_TYPE (gen_tmpl)) && TREE_CODE (CP_DECL_CONTEXT (gen_tmpl)) == NAMESPACE_DECL) - { - found = xref_tag_from_type (TREE_TYPE (gen_tmpl), - DECL_NAME (gen_tmpl), - /*tag_scope=*/ts_global); - return found; - } + /* This occurs when the user has tried to define a tagged type + in a scope that forbids it. We emitted an error during the + parse. We didn't complete the bail out then, so here we + are. */ + return error_mark_node; context = DECL_CONTEXT (gen_tmpl); if (context && TYPE_P (context))
c++: dependent local extern decl ICE [PR97171]
I'd missed the piece of substutution for the uses of a local extern decl. Just grab the local specialization. We need to do this regardless of dependentness because we always cloned the local extern. PR c++/97171 gcc/cp/ * pt.c (tsubst_copy) [FUNCTION_DECL,VAR_DECL]: Retrieve local specialization for DECL_LOCAL_P decls. gcc/testsuite/ * g++.dg/template/local10.C: New. pushing to trunk nathan -- Nathan Sidwell diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c index 314bd038c6d..1ec039d0793 100644 --- c/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -16531,6 +16531,14 @@ tsubst_copy (tree t, tree args, tsubst_flags_t complain, tree in_decl) case FUNCTION_DECL: if (DECL_LANG_SPECIFIC (t) && DECL_TEMPLATE_INFO (t)) r = tsubst (t, args, complain, in_decl); + else if (DECL_LOCAL_DECL_P (t)) + { + /* Local specialization will have been created when we + instantiated the DECL_EXPR_DECL. */ + r = retrieve_local_specialization (t); + if (!r) + r = error_mark_node; + } else if (local_variable_p (t) && uses_template_parms (DECL_CONTEXT (t))) { diff --git c/gcc/testsuite/g++.dg/template/local10.C w/gcc/testsuite/g++.dg/template/local10.C new file mode 100644 index 000..a2ffc1e7306 --- /dev/null +++ w/gcc/testsuite/g++.dg/template/local10.C @@ -0,0 +1,15 @@ +// PR c++/97171 +// { dg-additional-options -flto } + +template +void transform(_UnaryOperation); + +template +void Apply () +{ + extern T Maker (void); // block-scope extern with dependent type + + transform (Maker); +} + +template void Apply ();
c++: Remove some gratuitous typedefing
This is C++, we don't need 'typedef struct foo foo;'. Oh, and bool bitfields are a thing. gcc/cp/ * name-lookup.h (typedef cxx_binding): Delete tdef. (typedef cp_binding_level): Likewise. (struct cxx_binding): Flags are bools. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/name-lookup.h w/gcc/cp/name-lookup.h index a0815e1a0ac..5d2d364fc3a 100644 --- i/gcc/cp/name-lookup.h +++ w/gcc/cp/name-lookup.h @@ -47,12 +47,8 @@ struct GTY(()) binding_entry_s { extern void binding_table_foreach (binding_table, bt_foreach_proc, void *); extern binding_entry binding_table_find (binding_table, tree); -/* Datatype that represents binding established by a declaration between - a name and a C++ entity. */ -typedef struct cxx_binding cxx_binding; - /* The datatype used to implement C++ scope. */ -typedef struct cp_binding_level cp_binding_level; +struct cp_binding_level; /* Nonzero if this binding is for a local scope, as opposed to a class or namespace scope. */ @@ -62,6 +58,8 @@ typedef struct cp_binding_level cp_binding_level; currently being defined. */ #define INHERITED_VALUE_BINDING_P(NODE) ((NODE)->value_is_inherited) +/* Datatype that represents binding established by a declaration between + a name and a C++ entity. */ struct GTY(()) cxx_binding { /* Link to chain together various bindings for this name. */ cxx_binding *previous; @@ -71,8 +69,9 @@ struct GTY(()) cxx_binding { tree type; /* The scope at which this binding was made. */ cp_binding_level *scope; - unsigned value_is_inherited : 1; - unsigned is_local : 1; + + bool value_is_inherited : 1; + bool is_local : 1; }; /* Datatype used to temporarily save C++ bindings (for implicit
c++: local-decls are never member fns [PR97186]
This fixes an ICE in noexcept instantiation. It was presuming functions always have template_info, but that changed with my DECL_LOCAL_DECL_P changes. Fortunately DECL_LOCAL_DECL_P fns are never member fns, so we don't need to go fishing out a this pointer. Also I realized I'd misnamed local10.C, so renaming it local-fn3.C, and while there adding the effective-target lto that David E pointed out was missing. PR c++/97186 gcc/cp/ * pt.c (maybe_instantiate_noexcept): Local externs are never member fns. gcc/testsuite/ * g++.dg/template/local10.C: Rename ... * g++.dg/template/local-fn3.C: .. here. Require lto. * g++.dg/template/local-fn4.C: New. pushing to trunk nathan -- Nathan Sidwell diff --git c/gcc/cp/pt.c w/gcc/cp/pt.c index 1ec039d0793..62e85095bc4 100644 --- c/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -25397,15 +25397,20 @@ maybe_instantiate_noexcept (tree fn, tsubst_flags_t complain) push_deferring_access_checks (dk_no_deferred); input_location = DECL_SOURCE_LOCATION (fn); - /* If needed, set current_class_ptr for the benefit of - tsubst_copy/PARM_DECL. */ - tree tdecl = DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (fn)); - if (DECL_NONSTATIC_MEMBER_FUNCTION_P (tdecl)) + if (!DECL_LOCAL_DECL_P (fn)) { - tree this_parm = DECL_ARGUMENTS (tdecl); - current_class_ptr = NULL_TREE; - current_class_ref = cp_build_fold_indirect_ref (this_parm); - current_class_ptr = this_parm; + /* If needed, set current_class_ptr for the benefit of + tsubst_copy/PARM_DECL. The exception pattern will + refer to the parm of the template, not the + instantiation. */ + tree tdecl = DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (fn)); + if (DECL_NONSTATIC_MEMBER_FUNCTION_P (tdecl)) + { + tree this_parm = DECL_ARGUMENTS (tdecl); + current_class_ptr = NULL_TREE; + current_class_ref = cp_build_fold_indirect_ref (this_parm); + current_class_ptr = this_parm; + } } /* If this function is represented by a TEMPLATE_DECL, then diff --git c/gcc/testsuite/g++.dg/template/local10.C w/gcc/testsuite/g++.dg/template/local-fn3.C similarity index 87% rename from gcc/testsuite/g++.dg/template/local10.C rename to gcc/testsuite/g++.dg/template/local-fn3.C index a2ffc1e7306..2affe235bd3 100644 --- c/gcc/testsuite/g++.dg/template/local10.C +++ w/gcc/testsuite/g++.dg/template/local-fn3.C @@ -1,4 +1,6 @@ // PR c++/97171 + +// { dg-require-effective-target lto } // { dg-additional-options -flto } template diff --git c/gcc/testsuite/g++.dg/template/local-fn4.C w/gcc/testsuite/g++.dg/template/local-fn4.C new file mode 100644 index 000..4699012accc --- /dev/null +++ w/gcc/testsuite/g++.dg/template/local-fn4.C @@ -0,0 +1,21 @@ +// PR c++/97186 +// ICE in exception spec substitution + + +template +struct no { + static void + tg () + { +void + hk () noexcept (tg); // { dg-error "convert" } + +hk (); + } +}; + +void +os () +{ + no ().tg (); +}
c++: Cleanup some decl pushing apis
In cleaning up local decl handling, here's an initial patch that takes advantage of C++'s default args for the is_friend parm of pushdecl, duplicate_decls and push_template_decl_real and the scope & tpl_header parms of xref_tag. Then many of the calls simply not mention these. I also rename push_template_decl_real to push_template_decl, deleting the original forwarding function. This'll make my later patches changing their types less intrusive. There are 2 functional changes: 1) push_template_decl requires is_friend to be correct, it doesn't go checking for a friend function (an assert is added). 2) debug_overload prints out Hidden and Using markers for the overload set. gcc/cp/ * cp-tree.h (duplicate_decls): Default is_friend to false. (xref_tag): Default tag_scope & tpl_header_p to ts_current & false. (push_template_decl_real): Default is_friend to false. Rename to ... (push_template_decl): ... here. Delete original decl. * name-lookup.h (pushdecl_namespace_level): Default is_friend to false. (pushtag): Default tag_scope to ts_current. * coroutine.cc (morph_fn_to_coro): Drop default args to xref_tag. * decl.c (start_decl): Drop default args to duplicate_decls. (start_enum): Drop default arg to pushtag & xref_tag. (start_preparsed_function): Pass DECL_FRIEND_P to push_template_decl. (grokmethod): Likewise. * friend.c (do_friend): Rename push_template_decl_real calls. * lambda.c (begin_lamnbda_type): Drop default args to xref_tag. (vla_capture_type): Likewise. * name-lookup.c (maybe_process_template_type_declaration): Rename push_template_decl_real call. (pushdecl_top_level_and_finish): Drop default arg to pushdecl_namespace_level. * pt.c (push_template_decl_real): Assert no surprising friend functions. Rename to ... (push_template_decl): ... here. Delete original function. (lookup_template_class_1): Drop default args from pushtag. (instantiate_class_template_1): Likewise. * ptree.c (debug_overload): Print hidden and using markers. * rtti.c (init_rtti_processing): Drop refault args from xref_tag. * semantics.c (begin_class_definition): Drop default args to pushtag. gcc/objcp/ * objcp-decl.c (objcp_start_struct): Drop default args to xref_tag. (objcp_xref_tag): Likewise. libcc1/ * libcp1plugin.cc (supplement_binding): Drop default args to duplicate_decls. (safe_pushtag): Drop scope parm. Drop default args to pushtag. (safe_pushdecl_maybe_friend): Rename to ... (safe_pushdecl): ... here. Drop is_friend parm. Drop default args to pushdecl. (plugin_build_decl): Adjust safe_pushdecl & safe_pushtag calls. (plugin_build_constant): Adjust safe_pushdecl call. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/coroutines.cc w/gcc/cp/coroutines.cc index 898b88b7075..ba813454a0b 100644 --- i/gcc/cp/coroutines.cc +++ w/gcc/cp/coroutines.cc @@ -4011,7 +4011,7 @@ morph_fn_to_coro (tree orig, tree *resumer, tree *destroyer) /* 2. Types we need to define or look up. */ tree fr_name = get_fn_local_identifier (orig, "frame"); - tree coro_frame_type = xref_tag (record_type, fr_name, ts_current, false); + tree coro_frame_type = xref_tag (record_type, fr_name); DECL_CONTEXT (TYPE_NAME (coro_frame_type)) = current_scope (); tree coro_frame_ptr = build_pointer_type (coro_frame_type); tree act_des_fn_type diff --git i/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index 029a165a3e8..3ae48749b3d 100644 --- i/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -6461,7 +6461,8 @@ extern void note_iteration_stmt_body_end (bool); extern void determine_local_discriminator (tree); extern int decls_match(tree, tree, bool = true); extern bool maybe_version_functions (tree, tree, bool); -extern tree duplicate_decls (tree, tree, bool); +extern tree duplicate_decls (tree, tree, + bool is_friend = false); extern tree declare_local_label (tree); extern tree define_label (location_t, tree); extern void check_goto(tree); @@ -6501,7 +6502,9 @@ extern tree get_scope_of_declarator (const cp_declarator *); extern void grok_special_member_properties (tree); extern bool grok_ctor_properties (const_tree, const_tree); extern bool grok_op_properties (tree, bool); -extern tree xref_tag(enum tag_types, tree, tag_scope, bool); +extern tree xref_tag(tag_types, tree, + tag_scope = ts_current, + bool tpl_header_p = false); extern void xref_basetypes (tree, tree); extern tree start_enum(tree, tree, tree, tree, bool, bool *); extern void finish_enum_value_list (tree); @@ -6849,8 +6852,7 @@ extern void end_template_parm_list (void); extern void end_template_decl (void)
c++: DECL_BUILTIN_P for builtins
We currently detect builtin decls via DECL_ARTIFICIAL && !DECL_HIDDEN_FUNCTION_P, which, besides being clunky, is a problem as hiddenness is a property of the symbol table -- not the decl being hidden. This adds DECL_BUILTIN_P, which just looks at the SOURCE_LOCATION -- we have a magic one for builtins. One of the consequential changes is to make function-scope omp udrs have function context (needed because otherwise duplicate-decls thinks the types don't match at the point we check). This is also morally better, because that's what they are -- nested functions, stop lying. (That's actually my plan for all DECL_LOCAL_DECL_P decls, as they are distinct decls to the namespace-scope decl they alias.) gcc/cp/ * cp-tree.h (DECL_BUILTIN_P): New. * decl.c (duplicate_decls): Use it. Do not treat omp-udr as a builtin. * name-lookup.c (anticipated_builtin): Use it. (set_decl_context_in_fn): Function-scope OMP UDRs have function context. (do_nonmember_using_decl): Use DECL_BUILTIN_P. * parser.c (cp_parser_omp_declare_reduction): Function-scope OMP UDRs have function context. Assert we never find a valid duplicate. * pt.c (tsubst_expr): Function-scope OMP UDRs have function context. libcc1/ * libcp1plugin.cc (supplement_binding): Use DECL_BULTIN_P. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index 3ae48749b3d..bd78f00ba97 100644 --- i/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -4040,6 +4040,10 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter) #define FNDECL_USED_AUTO(NODE) \ TREE_LANG_FLAG_2 (FUNCTION_DECL_CHECK (NODE)) +/* True if NODE is a builtin decl. */ +#define DECL_BUILTIN_P(NODE) \ + (DECL_SOURCE_LOCATION(NODE) == BUILTINS_LOCATION) + /* Nonzero if NODE is a DECL which we know about but which has not been explicitly declared, such as a built-in function or a friend declared inside a class. In the latter case DECL_HIDDEN_FRIEND_P diff --git i/gcc/cp/decl.c w/gcc/cp/decl.c index 6019051ed12..1709dd9a370 100644 --- i/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -1464,9 +1464,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend) /* Check for redeclaration and other discrepancies. */ if (TREE_CODE (olddecl) == FUNCTION_DECL - && DECL_ARTIFICIAL (olddecl) - /* A C++20 implicit friend operator== uses the normal path (94462). */ - && !DECL_HIDDEN_FRIEND_P (olddecl)) + && DECL_BUILTIN_P (olddecl)) { if (TREE_CODE (newdecl) != FUNCTION_DECL) { @@ -1508,15 +1506,6 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend) "declaration %q#D", newdecl, olddecl); return NULL_TREE; } - else if (DECL_OMP_DECLARE_REDUCTION_P (olddecl)) - { - gcc_assert (DECL_OMP_DECLARE_REDUCTION_P (newdecl)); - error_at (newdecl_loc, - "redeclaration of %"); - inform (olddecl_loc, - "previous % declaration"); - return error_mark_node; - } else if (!types_match) { /* Avoid warnings redeclaring built-ins which have not been @@ -1815,6 +1804,17 @@ duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend) return error_mark_node; } } + else if (TREE_CODE (newdecl) == FUNCTION_DECL + && DECL_OMP_DECLARE_REDUCTION_P (newdecl)) +{ + /* OMP UDRs are never duplicates. */ + gcc_assert (DECL_OMP_DECLARE_REDUCTION_P (olddecl)); + error_at (newdecl_loc, + "redeclaration of %"); + inform (olddecl_loc, + "previous % declaration"); + return error_mark_node; +} else if (TREE_CODE (newdecl) == FUNCTION_DECL && ((DECL_TEMPLATE_SPECIALIZATION (olddecl) && (!DECL_TEMPLATE_INFO (newdecl) diff --git i/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index e7764abff67..dbc6cc32dd8 100644 --- i/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -2119,10 +2119,10 @@ anticipated_builtin_p (tree ovl) tree fn = OVL_FUNCTION (ovl); gcc_checking_assert (DECL_ANTICIPATED (fn)); - if (DECL_HIDDEN_FRIEND_P (fn)) -return false; + if (DECL_BUILTIN_P (fn)) +return true; - return true; + return false; } /* BINDING records an existing declaration for a name in the current scope. @@ -2857,9 +2857,12 @@ set_decl_context_in_fn (tree ctx, tree decl) { if (TREE_CODE (decl) == FUNCTION_DECL || (VAR_P (decl) && DECL_EXTERNAL (decl))) -/* Make sure local externs are marked as such. */ +/* Make sure local externs are marked as such. OMP UDRs really + are nested functions. */ gcc_checking_assert (DECL_LOCAL_DECL_P (decl) - && DECL_NAMESPACE_SCOPE_P (decl)); + && (DECL_NAMESPACE_SCOPE_P (decl) + || (TREE_CODE (decl) == FUNCTION_DECL + && DECL_OMP_DECLARE_REDUCTION_P (decl;
c++: Replace tag_scope with TAG_how
I always found tag_scope confusing, as it is not a scope, but a direction of how to lookup or insert an elaborated type tag. This replaces it with a enum class TAG_how. I also add a new value, HIDDEN_FRIEND, to distinguish the two cases of innermost-non-class insertion that we currently conflate. Also renamed 'lookup_type_scope' to 'lookup_elaborated_type', because again, we're not providing a scope to lookup in. gcc/cp/ * name-lookup.h (enum tag_scope): Replace with ... (enum class TAG_how): ... this. Add HIDDEN_FRIEND value. (lookup_type_scope): Replace with ... (lookup_elaborated_type): ... this. (pushtag): Use TAG_how, not tag_scope. * cp-tree.h (xref_tag): Parameter is TAG_how, not tag_scope. * decl.c (lookup_and_check_tag): Likewise. Adjust. (xref_tag_1, xref_tag): Likewise. adjust. (start_enum): Adjust lookup_and_check_tag call. * name-lookup.c (lookup_type_scope_1): Rename to ... (lookup_elaborated_type_1) ... here. Use TAG_how, not tag_scope. (lookup_type_scope): Rename to ... (lookup_elaborated_type): ... here. Use TAG_how, not tag_scope. (do_pushtag): Use TAG_how, not tag_scope. Adjust. (pushtag): Likewise. * parser.c (cp_parser_elaborated_type_specifier): Adjust. (cp_parser_class_head): Likewise. gcc/objcp/ * objcp-decl.c (objcp_start_struct): Use TAG_how not tag_scope. (objcp_xref_tag): Likewise. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index bd78f00ba97..321bb959120 100644 --- i/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -6507,7 +6507,7 @@ extern void grok_special_member_properties (tree); extern bool grok_ctor_properties (const_tree, const_tree); extern bool grok_op_properties (tree, bool); extern tree xref_tag(tag_types, tree, - tag_scope = ts_current, + TAG_how = TAG_how::CURRENT_ONLY, bool tpl_header_p = false); extern void xref_basetypes (tree, tree); extern tree start_enum(tree, tree, tree, tree, bool, bool *); diff --git i/gcc/cp/decl.c w/gcc/cp/decl.c index 1709dd9a370..b481bbd7b7d 100644 --- i/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -75,7 +75,7 @@ static void record_unknown_type (tree, const char *); static int member_function_or_else (tree, tree, enum overload_flags); static tree local_variable_p_walkfn (tree *, int *, void *); static const char *tag_name (enum tag_types); -static tree lookup_and_check_tag (enum tag_types, tree, tag_scope, bool); +static tree lookup_and_check_tag (enum tag_types, tree, TAG_how, bool); static void maybe_deduce_size_from_array_init (tree, tree); static void layout_var_decl (tree); static tree check_initializer (tree, tree, int, vec **); @@ -14862,11 +14862,10 @@ check_elaborated_type_specifier (enum tag_types tag_code, static tree lookup_and_check_tag (enum tag_types tag_code, tree name, - tag_scope scope, bool template_header_p) + TAG_how how, bool template_header_p) { - tree t; tree decl; - if (scope == ts_global) + if (how == TAG_how::GLOBAL) { /* First try ordinary name lookup, ignoring hidden class name injected via friend declaration. */ @@ -14879,16 +14878,16 @@ lookup_and_check_tag (enum tag_types tag_code, tree name, If we find one, that name will be made visible rather than creating a new tag. */ if (!decl) - decl = lookup_type_scope (name, ts_within_enclosing_non_class); + decl = lookup_elaborated_type (name, TAG_how::INNERMOST_NON_CLASS); } else -decl = lookup_type_scope (name, scope); +decl = lookup_elaborated_type (name, how); if (decl && (DECL_CLASS_TEMPLATE_P (decl) - /* If scope is ts_current we're defining a class, so ignore a - template template parameter. */ - || (scope != ts_current + /* If scope is TAG_how::CURRENT_ONLY we're defining a class, + so ignore a template template parameter. */ + || (how != TAG_how::CURRENT_ONLY && DECL_TEMPLATE_TEMPLATE_PARM_P (decl decl = DECL_TEMPLATE_RESULT (decl); @@ -14898,11 +14897,10 @@ lookup_and_check_tag (enum tag_types tag_code, tree name, class C { class C {}; }; */ - if (scope == ts_current && DECL_SELF_REFERENCE_P (decl)) + if (how == TAG_how::CURRENT_ONLY && DECL_SELF_REFERENCE_P (decl)) { error ("%qD has the same name as the class in which it is " - "declared", - decl); + "declared", decl); return error_mark_node; } @@ -14922,10 +14920,10 @@ lookup_and_check_tag (enum tag_types tag_code, tree name, class C *c2; // DECL_SELF_REFERENCE_P is true }; */ - t = check_elaborated_type_specifier (tag_code, - decl, - template_header_p - | DECL_SELF_REFERENCE_P (decl)); + tree t = check_elaborated_type_specifier (
c++: Adjust pushdecl/duplicate_decls API
The decl pushing APIs and duplicate_decls take an 'is_friend' parm, when what they actually mean is 'hide this from name lookup'. That conflation has gotten more anachronistic as time moved on. We now have anticipated builtins, and I plan to have injected extern decls soon. So this patch is mainly a renaming excercise. is_friend -> hiding. duplicate_decls gets an additional 'was_hidden' parm. As I've already said, hiddenness is a property of the symbol table, not the decl. Builtins are now pushed requesting hiding, and pushdecl asserts that we don't attempt to push a thing that should be hidden without asking for it to be hidden. This is the final piece of groundwork to get rid of a bunch of 'this is hidden' markers on decls and move the hiding management entirely into name lookup. gcc/cp/ * cp-tree.h (duplicate_decls): Replace 'is_friend' with 'hiding' and add 'was_hidden'. * name-lookup.h (pushdecl_namespace_level): Replace 'is_friend' with 'hiding'. (pushdecl): Likewise. (pushdecl_top_level): Drop is_friend parm. * decl.c (check_no_redeclaration_friend_default_args): Rename parm olddelc_hidden_p. (duplicate_decls): Replace 'is_friend' with 'hiding' and 'was_hidden'. Do minimal adjustments in body. (cxx_builtin_function): Pass 'hiding' to pushdecl. * friend.c (do_friend): Pass 'hiding' to pushdecl. * name-lookup.c (supplement_binding_1): Drop defaulted arg to duplicate_decls. (update_binding): Replace 'is_friend' with 'hiding'. Drop defaulted arg to duplicate_decls. (do_pushdecl): Replace 'is_friend' with 'hiding'. Assert no surprise hidhing. Adjust duplicate_decls calls to inform of old decl's hiddennes. (pushdecl): Replace 'is_friend' with 'hiding'. (set_identifier_type_value_with_scope): Adjust update_binding call. (do_pushdecl_with_scope): Replace 'is_friend' with 'hiding'. (pushdecl_outermost_localscope): Drop default arg to do_pushdecl_with_scope. (pushdecl_namespace_level): Replace 'is_friend' with 'hiding'. (pushdecl_top_level): Drop is_friend parm. * pt.c (register_specialization): Comment duplicate_decls call args. (push_template_decl): Commont pushdecl_namespace_level. (tsubst_friend_function, tsubst_friend_class): Likewise. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index 321bb959120..b7f5b6b399f 100644 --- i/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -6466,7 +6466,8 @@ extern void determine_local_discriminator (tree); extern int decls_match(tree, tree, bool = true); extern bool maybe_version_functions (tree, tree, bool); extern tree duplicate_decls (tree, tree, - bool is_friend = false); + bool hiding = false, + bool was_hidden = false); extern tree declare_local_label (tree); extern tree define_label (location_t, tree); extern void check_goto(tree); diff --git i/gcc/cp/decl.c w/gcc/cp/decl.c index b481bbd7b7d..c00b996294e 100644 --- i/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -1341,17 +1341,16 @@ check_redeclaration_no_default_args (tree decl) static void check_no_redeclaration_friend_default_args (tree olddecl, tree newdecl, - bool olddecl_hidden_friend_p) + bool olddecl_hidden_p) { - if (!olddecl_hidden_friend_p && !DECL_FRIEND_P (newdecl)) + if (!olddecl_hidden_p && !DECL_FRIEND_P (newdecl)) return; - tree t1 = FUNCTION_FIRST_USER_PARMTYPE (olddecl); - tree t2 = FUNCTION_FIRST_USER_PARMTYPE (newdecl); - - for (; t1 && t1 != void_list_node; + for (tree t1 = FUNCTION_FIRST_USER_PARMTYPE (olddecl), + t2 = FUNCTION_FIRST_USER_PARMTYPE (newdecl); + t1 && t1 != void_list_node; t1 = TREE_CHAIN (t1), t2 = TREE_CHAIN (t2)) -if ((olddecl_hidden_friend_p && TREE_PURPOSE (t1)) +if ((olddecl_hidden_p && TREE_PURPOSE (t1)) || (DECL_FRIEND_P (newdecl) && TREE_PURPOSE (t2))) { auto_diagnostic_group d; @@ -1435,10 +1434,14 @@ duplicate_function_template_decls (tree newdecl, tree olddecl) If NEWDECL is not a redeclaration of OLDDECL, NULL_TREE is returned. - NEWDECL_IS_FRIEND is true if NEWDECL was declared as a friend. */ + HIDING is true if the new decl is being hidden. WAS_HIDDEN is true + if the old decl was hidden. + + Hidden decls can be anticipated builtins, injected friends, or + (coming soon) injected from a local-extern decl. */ tree -duplicate_decls (tree newdecl, tree olddecl, bool newdecl_is_friend) +duplicate_decls (tree newdecl,
c++: Identifier type value should not update binding
This simplification removes some unneeded behaviour in set_identifier_type_value_with_scope, which was updating the namespace binding. And causing update_binding to have to deal with meeting two implicit typedefs. But the typedef is already there, and there's no other way to have two such typedef's collide (we'll already have dealt with that in lookup_elaborated_type). So, let's kill this crufty code. gcc/cp/ * name-lookup.c (update_binding): We never meet two implicit typedefs. (do_pushdecl): Adjust set_identifier_type_value_with_scope calls. (set_identifier_type_value_with_scope): Do not update binding in the namespace-case. Assert it is already there. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index 184e9c873e7..f195e81280a 100644 --- i/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -2365,33 +2365,24 @@ update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, if (old == error_mark_node) old = NULL_TREE; - if (TREE_CODE (decl) == TYPE_DECL && DECL_ARTIFICIAL (decl)) + if (DECL_IMPLICIT_TYPEDEF_P (decl)) { - tree other = to_type; - - if (old && TREE_CODE (old) == TYPE_DECL && DECL_ARTIFICIAL (old)) - other = old; - - /* Pushing an artificial typedef. See if this matches either - the type slot or the old value slot. */ - if (!other) - ; - else if (same_type_p (TREE_TYPE (other), TREE_TYPE (decl))) - /* Two artificial decls to same type. Do nothing. */ - return other; - else - goto conflict; + /* Pushing an artificial decl. We should not find another + artificial decl here already -- lookup_elaborated_type will + have already found it. */ + gcc_checking_assert (!to_type + && !(old && DECL_IMPLICIT_TYPEDEF_P (old))); if (old) { /* Slide decl into the type slot, keep old unaltered */ to_type = decl; to_val = old; - goto done; } + goto done; } - if (old && TREE_CODE (old) == TYPE_DECL && DECL_ARTIFICIAL (old)) + if (old && DECL_IMPLICIT_TYPEDEF_P (old)) { /* Slide old into the type slot. */ to_type = old; @@ -3122,7 +3113,7 @@ do_pushdecl (tree decl, bool hiding) if (TREE_CODE (decl) == NAMESPACE_DECL) /* A local namespace alias. */ - set_identifier_type_value (name, NULL_TREE); + set_identifier_type_value_with_scope (name, NULL_TREE, level); if (!binding) binding = create_local_binding (level, name); @@ -3150,10 +3141,7 @@ do_pushdecl (tree decl, bool hiding) if (TYPE_NAME (type) != decl) set_underlying_type (decl); - if (!ns) - set_identifier_type_value_with_scope (name, decl, level); - else - SET_IDENTIFIER_TYPE_VALUE (name, global_type_node); + set_identifier_type_value_with_scope (name, decl, level); } /* If this is a locally defined typedef in a function that @@ -3768,8 +3756,9 @@ identifier_type_value (tree id) } /* Push a definition of struct, union or enum tag named ID. into - binding_level B. DECL is a TYPE_DECL for the type. We assume that - the tag ID is not already defined. */ + binding_level B. DECL is a TYPE_DECL for the type. DECL has + already been pushed into its binding level. This is bookkeeping to + find it easily. */ static void set_identifier_type_value_with_scope (tree id, tree decl, cp_binding_level *b) @@ -3781,20 +3770,25 @@ set_identifier_type_value_with_scope (tree id, tree decl, cp_binding_level *b) /* Shadow the marker, not the real thing, so that the marker gets restored later. */ tree old_type_value = REAL_IDENTIFIER_TYPE_VALUE (id); - b->type_shadowed - = tree_cons (id, old_type_value, b->type_shadowed); + b->type_shadowed = tree_cons (id, old_type_value, b->type_shadowed); type = decl ? TREE_TYPE (decl) : NULL_TREE; TREE_TYPE (b->type_shadowed) = type; } else { - tree *slot = find_namespace_slot (current_namespace, id, true); gcc_assert (decl); - update_binding (b, NULL, slot, MAYBE_STAT_DECL (*slot), decl); + if (CHECKING_P) + { + tree *slot = find_namespace_slot (current_namespace, id); + gcc_checking_assert (slot + && (decl == MAYBE_STAT_TYPE (*slot) + || decl == MAYBE_STAT_DECL (*slot))); + } /* Store marker instead of real type. */ type = global_type_node; } + SET_IDENTIFIER_TYPE_VALUE (id, type); }
c++: Name lookup simplifications
Here are a few cleanups, prior to landing the hidden decl changes. 1) Clear cxx_binding flags in the allocator, not at each user of the allocator. 2) Refactor update_binding. The logic was getting too convoluted. 3) Set friendliness and anticipatedness before pushing a template decl (not after). gcc/cp/ * name-lookup.c (create_local_binding): Do not clear INHERITED_VALUE_BINDING_P here. (name_lookup::process_binding): Move done hidden-decl triage to ... (name_lookup::search_namespace_only): ... here, its only caller. (cxx_binding_make): Clear flags here. (push_binding): Not here. (pop_local_binding): RAII. (update_binding): Refactor. (do_pushdecl): Assert we're never revealing a local binding. (do_pushdecl_with_scope): Directly call do_pushdecl. (get_class_binding): Do not clear LOCAL_BINDING_P here. * pt.c (push_template_decl): Set friend & anticipated before pushing. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index f195e81280a..89f1a4c5d64 100644 --- i/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -77,7 +77,6 @@ create_local_binding (cp_binding_level *level, tree name) { cxx_binding *binding = cxx_binding_make (NULL, NULL); - INHERITED_VALUE_BINDING_P (binding) = false; LOCAL_BINDING_P (binding) = true; binding->scope = level; binding->previous = IDENTIFIER_BINDING (name); @@ -480,22 +479,17 @@ name_lookup::add_type (tree new_type) } /* Process a found binding containing NEW_VAL and NEW_TYPE. Returns - true if we actually found something noteworthy. */ + true if we actually found something noteworthy. Hiddenness has + already been handled in the caller. */ bool name_lookup::process_binding (tree new_val, tree new_type) { /* Did we really see a type? */ if (new_type - && ((want & LOOK_want::TYPE_NAMESPACE) == LOOK_want::NAMESPACE - || (!bool (want & LOOK_want::HIDDEN_FRIEND) - && DECL_LANG_SPECIFIC (new_type) - && DECL_ANTICIPATED (new_type + && (want & LOOK_want::TYPE_NAMESPACE) == LOOK_want::NAMESPACE) new_type = NULL_TREE; - if (new_val && !bool (want & LOOK_want::HIDDEN_FRIEND)) -new_val = ovl_skip_hidden (new_val); - /* Do we really see a value? */ if (new_val) switch (TREE_CODE (new_val)) @@ -544,8 +538,25 @@ name_lookup::search_namespace_only (tree scope) bool found = false; if (tree *binding = find_namespace_slot (scope, name)) -found |= process_binding (MAYBE_STAT_DECL (*binding), - MAYBE_STAT_TYPE (*binding)); +{ + tree value = *binding, type = NULL_TREE; + + if (STAT_HACK_P (value)) + { + type = STAT_TYPE (value); + value = STAT_DECL (value); + + if (!bool (want & LOOK_want::HIDDEN_FRIEND) + && DECL_LANG_SPECIFIC (type) + && DECL_ANTICIPATED (type)) + type = NULL_TREE; + } + + if (!bool (want & LOOK_want::HIDDEN_FRIEND)) + value = ovl_skip_hidden (value); + + found |= process_binding (value, type); +} return found; } @@ -1954,15 +1965,17 @@ cxx_binding_init (cxx_binding *binding, tree value, tree type) static cxx_binding * cxx_binding_make (tree value, tree type) { - cxx_binding *binding; - if (free_bindings) -{ - binding = free_bindings; - free_bindings = binding->previous; -} + cxx_binding *binding = free_bindings; + + if (binding) +free_bindings = binding->previous; else binding = ggc_alloc (); + /* Clear flags by default. */ + LOCAL_BINDING_P (binding) = false; + INHERITED_VALUE_BINDING_P (binding) = false; + cxx_binding_init (binding, value, type); return binding; @@ -2009,7 +2022,6 @@ push_binding (tree id, tree decl, cp_binding_level* level) /* Now, fill in the binding information. */ binding->previous = IDENTIFIER_BINDING (id); - INHERITED_VALUE_BINDING_P (binding) = 0; LOCAL_BINDING_P (binding) = (level != class_binding_level); /* And put it on the front of the list of bindings for ID. */ @@ -2022,8 +2034,6 @@ push_binding (tree id, tree decl, cp_binding_level* level) void pop_local_binding (tree id, tree decl) { - cxx_binding *binding; - if (id == NULL_TREE) /* It's easiest to write the loops that call this function without checking whether or not the entities involved have names. We @@ -2031,7 +2041,7 @@ pop_local_binding (tree id, tree decl) return; /* Get the innermost binding for ID. */ - binding = IDENTIFIER_BINDING (id); + cxx_binding *binding = IDENTIFIER_BINDING (id); /* The name should be bound. */ gcc_assert (binding != NULL); @@ -2356,9 +2366,16 @@ static tree update_binding (cp_binding_level *level, cxx_binding *binding, tree *slot, tree old, tree decl, bool hiding = false) { + tree old_type
c++: Hiddenness is a property of the symbol table
This patch moves the handling of decl-hiddenness entirely into the name lookup machinery, where it belongs. We need a few new flags, because pressing the existing OVL_HIDDEN_P into play for non-function decls doesn't work well. For a local binding we only need one marker, as there cannot be both a hidden implicit typedef and a hidden function. That's not true for namespace-scope, where they could both be hidden. The name-lookup machinery maintains the existing decl_hidden and co flags, and asserts have been sprinkled around to make sure they are consistent. The next series of patches will remove those old markers. (we'll need to keep one, as there are some special restrictions on redeclaring friend functions with in-class definitions or default args.) gcc/cp/ * cp-tree.h (ovl_insert): Change final parm to hidden-or-using indicator. * name-lookup.h (HIDDEN_TYPE_BINDING_P): New. (struct cxx_binding): Add type_is_hidden flag. * tree.c (ovl_insert): Change using_p parm to using_or_hidden, adjust. (ovl_skip_hidden): Assert we never see a naked hidden decl. * decl.c (xref_tag_1): Delete unhiding friend from here (moved to lookup_elaborated_type_1). * name-lookup.c (STAT_TYPE_HIDDEN_P, STAT_DECL_HIDDEN_P): New. (name_lookup::search_namespace_only): Check new hidden markers. (cxx_binding_make): Clear HIDDEN_TYPE_BINDING_P. (update_binding): Update new hidden markers. (lookup_name_1): Check HIDDEN_TYPE_BINDING_P and simplify friend ignoring. (lookup_elaborated_type_1): Use new hidden markers. Reveal the decl here. pushed to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index b7f5b6b399f..a25934e3263 100644 --- i/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -7371,7 +7371,7 @@ inline tree ovl_first(tree) ATTRIBUTE_PURE; extern tree ovl_make(tree fn, tree next = NULL_TREE); extern tree ovl_insert(tree fn, tree maybe_ovl, - bool using_p = false); + int using_or_hidden = 0); extern tree ovl_skip_hidden (tree) ATTRIBUTE_PURE; extern void lookup_mark(tree lookup, bool val); extern tree lookup_add(tree fns, tree lookup); diff --git i/gcc/cp/decl.c w/gcc/cp/decl.c index c00b996294e..617b96e02e4 100644 --- i/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -15089,22 +15089,9 @@ xref_tag_1 (enum tag_types tag_code, tree name, return error_mark_node; } - if (how != TAG_how::HIDDEN_FRIEND && TYPE_HIDDEN_P (t)) - { - /* This is no longer an invisible friend. Make it - visible. */ - tree decl = TYPE_NAME (t); - - DECL_ANTICIPATED (decl) = false; - DECL_FRIEND_P (decl) = false; - - if (TYPE_TEMPLATE_INFO (t)) - { - tree tmpl = TYPE_TI_TEMPLATE (t); - DECL_ANTICIPATED (tmpl) = false; - DECL_FRIEND_P (tmpl) = false; - } - } + gcc_checking_assert (how == TAG_how::HIDDEN_FRIEND + || !(DECL_LANG_SPECIFIC (TYPE_NAME (t)) +&& DECL_ANTICIPATED (TYPE_NAME (t; } return t; diff --git i/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index 89f1a4c5d64..bc60d343f7e 100644 --- i/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -55,6 +55,15 @@ static name_hint suggest_alternatives_for_1 (location_t location, tree name, #define MAYBE_STAT_DECL(N) (STAT_HACK_P (N) ? STAT_DECL (N) : N) #define MAYBE_STAT_TYPE(N) (STAT_HACK_P (N) ? STAT_TYPE (N) : NULL_TREE) +/* For regular (maybe) overloaded functions, we have OVL_HIDDEN_P. + But we also need to indicate hiddenness on implicit type decls + (injected friend classes), and (coming soon) decls injected from + block-scope externs. It is too awkward to press the existing + overload marking for that. If we have a hidden non-function, we + always create a STAT_HACK, and use these two markers as needed. */ +#define STAT_TYPE_HIDDEN_P(N) OVL_HIDDEN_P (N) +#define STAT_DECL_HIDDEN_P(N) OVL_DEDUP_P (N) + /* Create a STAT_HACK node with DECL as the value binding and TYPE as the type binding. */ @@ -545,14 +554,18 @@ name_lookup::search_namespace_only (tree scope) { type = STAT_TYPE (value); value = STAT_DECL (value); - - if (!bool (want & LOOK_want::HIDDEN_FRIEND) - && DECL_LANG_SPECIFIC (type) - && DECL_ANTICIPATED (type)) - type = NULL_TREE; + + if (!bool (want & LOOK_want::HIDDEN_FRIEND)) + { + if (STAT_TYPE_HIDDEN_P (*binding)) + type = NULL_TREE; + if (STAT_DECL_HIDDEN_P (*binding)) + value = NULL_TREE; + else + value = ovl_skip_hidden (value); + } } - - if (!bool (want & LOOK_want::HIDDEN_FRIEND)) + else if (!bool (want & LOOK_want::HIDDEN_FRIEND)) value = ovl_skip_hidden (value); found |= process_binding (value, type); @@ -1975,6 +1988,7 @@ cxx_binding_make (tree value, tree type) /* Clear flags by default. */ LOCAL_BIN
c++: Kill DECL_HIDDEN_FRIEND_P
Now hiddenness is managed by name-lookup, we no longer need DECL_HIDDEN_FRIEND_P. This removes it. Mainly by deleting its bookkeeping, but there are a couple of uses 1) two name lookups look at it to see if they found a hidden thing. In one we have the OVERLOAD, so can record OVL_HIDDEN_P. In the other we're repeating a lookup that failed, but asking for hidden things -- so if that succeeds we know the thing was hidden. (FWIW CWG recently discussed whether template specializations and instantiations should see such hidden templates anyway, there is compiler divergence.) 2) We had a confusing setting of KOENIG_P when building a non-dependent call. We don't repeat that lookup at instantiation time anyway. gcc/cp/ * cp-tree.h (struct lang_decl_fn): Remove hidden_friend_p. (DECL_HIDDEN_FRIEND_P): Delete. * call.c (add_function_candidate): Drop assert about anticipated decl. (build_new_op_1): Drop koenig lookup flagging for hidden friend. * decl.c (duplicate_decls): Drop HIDDEN_FRIEND_P updating. * name-lookup.c (do_pushdecl): Likewise. (set_decl_namespace): Discover hiddenness from OVL_HIDDEN_P. * pt.c (check_explicit_specialization): Record found_hidden explicitly. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/call.c w/gcc/cp/call.c index 5606389f4bd..da013e17e14 100644 --- i/gcc/cp/call.c +++ w/gcc/cp/call.c @@ -2220,11 +2220,6 @@ add_function_candidate (struct z_candidate **candidates, int viable = 1; struct rejection_reason *reason = NULL; - /* At this point we should not see any functions which haven't been - explicitly declared, except for friend functions which will have - been found using argument dependent lookup. */ - gcc_assert (!DECL_ANTICIPATED (fn) || DECL_HIDDEN_FRIEND_P (fn)); - /* The `this', `in_chrg' and VTT arguments to constructors are not considered in overload resolution. */ if (DECL_CONSTRUCTOR_P (fn)) @@ -6344,11 +6339,6 @@ build_new_op_1 (const op_location_t &loc, enum tree_code code, int flags, tree call = extract_call_expr (result); CALL_EXPR_OPERATOR_SYNTAX (call) = true; - if (processing_template_decl && DECL_HIDDEN_FRIEND_P (cand->fn)) - /* This prevents build_new_function_call from discarding this - function during instantiation of the enclosing template. */ - KOENIG_LOOKUP_P (call) = 1; - /* Specify evaluation order as per P0145R2. */ CALL_EXPR_ORDERED_ARGS (call) = false; switch (op_is_ordered (code)) diff --git i/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index a25934e3263..762a3519b7c 100644 --- i/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -2720,14 +2720,13 @@ struct GTY(()) lang_decl_fn { unsigned thunk_p : 1; unsigned this_thunk_p : 1; - unsigned hidden_friend_p : 1; unsigned omp_declare_reduction_p : 1; unsigned has_dependent_explicit_spec_p : 1; unsigned immediate_fn_p : 1; unsigned maybe_deleted : 1; unsigned coroutine_p : 1; - unsigned spare : 9; + unsigned spare : 10; /* 32-bits padding on 64-bit host. */ @@ -4067,12 +4066,6 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter) #define DECL_OMP_PRIVATIZED_MEMBER(NODE) \ (DECL_LANG_SPECIFIC (VAR_DECL_CHECK (NODE))->u.base.anticipated_p) -/* Nonzero if NODE is a FUNCTION_DECL which was declared as a friend - within a class but has not been declared in the surrounding scope. - The function is invisible except via argument dependent lookup. */ -#define DECL_HIDDEN_FRIEND_P(NODE) \ - (LANG_DECL_FN_CHECK (DECL_COMMON_CHECK (NODE))->hidden_friend_p) - /* Nonzero if NODE is an artificial FUNCTION_DECL for #pragma omp declare reduction. */ #define DECL_OMP_DECLARE_REDUCTION_P(NODE) \ diff --git i/gcc/cp/decl.c w/gcc/cp/decl.c index 617b96e02e4..14742c115ad 100644 --- i/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -2141,10 +2141,7 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) olddecl_hidden_friend = olddecl_friend && was_hidden; hidden_friend = olddecl_hidden_friend && hiding; if (!hidden_friend) - { - DECL_ANTICIPATED (olddecl) = 0; - DECL_HIDDEN_FRIEND_P (olddecl) = 0; - } + DECL_ANTICIPATED (olddecl) = false; } if (TREE_CODE (newdecl) == TEMPLATE_DECL) @@ -2892,12 +2889,9 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden) DECL_UID (olddecl) = olddecl_uid; if (olddecl_friend) -DECL_FRIEND_P (olddecl) = 1; +DECL_FRIEND_P (olddecl) = true; if (hidden_friend) -{ - DECL_ANTICIPATED (olddecl) = 1; - DECL_HIDDEN_FRIEND_P (olddecl) = 1; -} +DECL_ANTICIPATED (olddecl) = true; /* NEWDECL contains the merged attribute lists. Update OLDDECL to be the same. */ diff --git i/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index bc60d343f7e..8cd6fe38271 100644 --- i/gcc/
c++: Refactor lookup_and_check_tag
It turns out I'd already found lookup_and_check_tag's control flow confusing, and had refactored it on the modules branch. For instance, it continually checks 'if (decl &&$ condition)' before finally getting to 'else if (!decl)'. why not just check !decl first and be done? Well, it is done thusly. gcc/cp/ * decl.c (lookup_and_check_tag): Refactor. pushed to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/decl.c w/gcc/cp/decl.c index 14742c115ad..d2a8d4012ab 100644 --- i/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -14885,71 +14885,73 @@ lookup_and_check_tag (enum tag_types tag_code, tree name, else decl = lookup_elaborated_type (name, how); - if (decl - && (DECL_CLASS_TEMPLATE_P (decl) - /* If scope is TAG_how::CURRENT_ONLY we're defining a class, - so ignore a template template parameter. */ - || (how != TAG_how::CURRENT_ONLY - && DECL_TEMPLATE_TEMPLATE_PARM_P (decl -decl = DECL_TEMPLATE_RESULT (decl); - - if (decl && TREE_CODE (decl) == TYPE_DECL) -{ - /* Look for invalid nested type: - class C { - class C {}; - }; */ - if (how == TAG_how::CURRENT_ONLY && DECL_SELF_REFERENCE_P (decl)) - { - error ("%qD has the same name as the class in which it is " - "declared", decl); - return error_mark_node; - } - - /* Two cases we need to consider when deciding if a class - template is allowed as an elaborated type specifier: - 1. It is a self reference to its own class. - 2. It comes with a template header. - For example: - - template class C { - class C *c1; // DECL_SELF_REFERENCE_P is true - class D; - }; - template class C; // template_header_p is true - template class C::D { - class C *c2; // DECL_SELF_REFERENCE_P is true - }; */ - - tree t = check_elaborated_type_specifier (tag_code, - decl, - template_header_p - | DECL_SELF_REFERENCE_P (decl)); - if (template_header_p && t && CLASS_TYPE_P (t) - && (!CLASSTYPE_TEMPLATE_INFO (t) - || (!PRIMARY_TEMPLATE_P (CLASSTYPE_TI_TEMPLATE (t) - { - error ("%qT is not a template", t); - inform (location_of (t), "previous declaration here"); - if (TYPE_CLASS_SCOPE_P (t) - && CLASSTYPE_TEMPLATE_INFO (TYPE_CONTEXT (t))) - inform (input_location, - "perhaps you want to explicitly add %<%T::%>", - TYPE_CONTEXT (t)); - t = error_mark_node; - } + if (!decl) +/* We found nothing. */ +return NULL_TREE; - return t; -} - else if (decl && TREE_CODE (decl) == TREE_LIST) + if (TREE_CODE (decl) == TREE_LIST) { error ("reference to %qD is ambiguous", name); print_candidates (decl); return error_mark_node; } - else + + if (DECL_CLASS_TEMPLATE_P (decl) + /* If scope is TAG_how::CURRENT_ONLY we're defining a class, + so ignore a template template parameter. */ + || (how != TAG_how::CURRENT_ONLY && DECL_TEMPLATE_TEMPLATE_PARM_P (decl))) +decl = DECL_TEMPLATE_RESULT (decl); + + if (TREE_CODE (decl) != TYPE_DECL) +/* Found not-a-type. */ return NULL_TREE; + +/* Look for invalid nested type: + class C { + class C {}; + }; */ + if (how == TAG_how::CURRENT_ONLY && DECL_SELF_REFERENCE_P (decl)) +{ + error ("%qD has the same name as the class in which it is " + "declared", decl); + return error_mark_node; +} + + /* Two cases we need to consider when deciding if a class + template is allowed as an elaborated type specifier: + 1. It is a self reference to its own class. + 2. It comes with a template header. + + For example: + + template class C { + class C *c1; // DECL_SELF_REFERENCE_P is true + class D; + }; + template class C; // template_header_p is true + template class C::D { + class C *c2; // DECL_SELF_REFERENCE_P is true + }; */ + + tree t = check_elaborated_type_specifier (tag_code, decl, + template_header_p + | DECL_SELF_REFERENCE_P (decl)); + if (template_header_p && t && CLASS_TYPE_P (t) + && (!CLASSTYPE_TEMPLATE_INFO (t) + || (!PRIMARY_TEMPLATE_P (CLASSTYPE_TI_TEMPLATE (t) +{ + error ("%qT is not a template", t); + inform (location_of (t), "previous declaration here"); + if (TYPE_CLASS_SCOPE_P (t) + && CLASSTYPE_TEMPLATE_INFO (TYPE_CONTEXT (t))) + inform (input_location, + "perhaps you want to explicitly add %<%T::%>", + TYPE_CONTEXT (t)); + return error_mark_node; +} + + return t; } /* Get the struct, enum or union (TAG_CODE says which) with tag NAME.
c++: pushdecl_top_level must set context
I discovered pushdecl_top_level was not setting the decl's context, and we ended up with namespace-scope decls with NULL context. That broke modules. Then I discovered a couple of places where we set the context to a FUNCTION_DECL, which is also wrong. AFAICT the literals in question belong in global scope, as they're comdatable entities. But create_temporary would use current_scope for the context before we pushed it into namespace scope. This patch asserts the context is NULL and then sets it to the frobbed global_namespace. gcc/cp/ * name-lookup.c (pushdecl_top_level): Assert incoming context is null, add global_namespace context. (pushdecl_top_level_and_finish): Likewise. * pt.c (get_template_parm_object): Clear decl context before pushing. * semantics.c (finish_compound_literal): Likewise. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index 8cd6fe38271..620f3a6 100644 --- i/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -7404,6 +7404,8 @@ pushdecl_top_level (tree x) { bool subtime = timevar_cond_start (TV_NAME_LOOKUP); do_push_to_top_level (); + gcc_checking_assert (!DECL_CONTEXT (x)); + DECL_CONTEXT (x) = FROB_CONTEXT (global_namespace); x = pushdecl_namespace_level (x); do_pop_from_top_level (); timevar_cond_stop (TV_NAME_LOOKUP, subtime); @@ -7418,6 +7420,8 @@ pushdecl_top_level_and_finish (tree x, tree init) { bool subtime = timevar_cond_start (TV_NAME_LOOKUP); do_push_to_top_level (); + gcc_checking_assert (!DECL_CONTEXT (x)); + DECL_CONTEXT (x) = FROB_CONTEXT (global_namespace); x = pushdecl_namespace_level (x); cp_finish_decl (x, init, false, NULL_TREE, 0); do_pop_from_top_level (); diff --git i/gcc/cp/pt.c w/gcc/cp/pt.c index 869477f2c2e..45b18f6a5ad 100644 --- i/gcc/cp/pt.c +++ w/gcc/cp/pt.c @@ -7094,12 +7094,12 @@ get_template_parm_object (tree expr, tsubst_flags_t complain) tree type = cp_build_qualified_type (TREE_TYPE (expr), TYPE_QUAL_CONST); decl = create_temporary_var (type); + DECL_CONTEXT (decl) = NULL_TREE; TREE_STATIC (decl) = true; DECL_DECLARED_CONSTEXPR_P (decl) = true; TREE_READONLY (decl) = true; DECL_NAME (decl) = name; SET_DECL_ASSEMBLER_NAME (decl, name); - DECL_CONTEXT (decl) = global_namespace; comdat_linkage (decl); if (!zero_init_p (type)) diff --git i/gcc/cp/semantics.c w/gcc/cp/semantics.c index b0930442bda..1e42cd799c2 100644 --- i/gcc/cp/semantics.c +++ w/gcc/cp/semantics.c @@ -3030,6 +3030,7 @@ finish_compound_literal (tree type, tree compound_literal, && initializer_constant_valid_p (compound_literal, type)) { tree decl = create_temporary_var (type); + DECL_CONTEXT (decl) = NULL_TREE; DECL_INITIAL (decl) = compound_literal; TREE_STATIC (decl) = 1; if (literal_type_p (type) && CP_TYPE_CONST_NON_VOLATILE_P (type))
c++: Kill DECL_HIDDEN_P
There are only a couple of asserts remaining using this macro, and nothing using TYPE_HIDDEN_P. Killed thusly. gcc/cp/ * cp-tree.h (DECL_ANTICIPATED): Adjust comment. (DECL_HIDDEN_P, TYPE_HIDDEN_P): Delete. * tree.c (ovl_insert): Delete DECL_HIDDEN_P assert. (ovl_skip_hidden): Likewise. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index 48a4074b370..3ccd54ce24b 100644 --- i/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -4045,22 +4045,11 @@ more_aggr_init_expr_args_p (const aggr_init_expr_arg_iterator *iter) /* Nonzero if NODE is a DECL which we know about but which has not been explicitly declared, such as a built-in function or a friend - declared inside a class. In the latter case DECL_HIDDEN_FRIEND_P - will be set. */ + declared inside a class. */ #define DECL_ANTICIPATED(NODE) \ (DECL_LANG_SPECIFIC (TYPE_FUNCTION_OR_TEMPLATE_DECL_CHECK (NODE)) \ ->u.base.anticipated_p) -/* Is DECL NODE a hidden name? */ -#define DECL_HIDDEN_P(NODE) \ - (DECL_LANG_SPECIFIC (NODE) && TYPE_FUNCTION_OR_TEMPLATE_DECL_P (NODE) \ - && DECL_ANTICIPATED (NODE)) - -/* True if this is a hidden class type.*/ -#define TYPE_HIDDEN_P(NODE) \ - (DECL_LANG_SPECIFIC (TYPE_NAME (NODE)) \ - && DECL_ANTICIPATED (TYPE_NAME (NODE))) - /* True for artificial decls added for OpenMP privatized non-static data members. */ #define DECL_OMP_PRIVATIZED_MEMBER(NODE) \ diff --git i/gcc/cp/tree.c w/gcc/cp/tree.c index 0b80d8ed408..8b7c6798ee9 100644 --- i/gcc/cp/tree.c +++ w/gcc/cp/tree.c @@ -2261,8 +2261,6 @@ ovl_insert (tree fn, tree maybe_ovl, int using_or_hidden) { maybe_ovl = ovl_make (fn, maybe_ovl); - gcc_checking_assert ((using_or_hidden < 0) == DECL_HIDDEN_P (fn)); - if (using_or_hidden < 0) OVL_HIDDEN_P (maybe_ovl) = true; if (using_or_hidden > 0) @@ -2287,14 +2285,8 @@ ovl_insert (tree fn, tree maybe_ovl, int using_or_hidden) tree ovl_skip_hidden (tree ovl) { - for (; - ovl && TREE_CODE (ovl) == OVERLOAD && OVL_HIDDEN_P (ovl); - ovl = OVL_CHAIN (ovl)) -gcc_checking_assert (DECL_HIDDEN_P (OVL_FUNCTION (ovl))); - - /* We should not see a naked hidden decl. */ - gcc_checking_assert (!(ovl && TREE_CODE (ovl) != OVERLOAD - && DECL_HIDDEN_P (ovl))); + while (ovl && TREE_CODE (ovl) == OVERLOAD && OVL_HIDDEN_P (ovl)) +ovl = OVL_CHAIN (ovl); return ovl; }
c++: Simplify __FUNCTION__ creation
I had reason to wander into cp_make_fname, and noticed it's the only caller of cp_fname_init. Folding it in makes the code simpler. gcc/cp/ * cp-tree.h (cp_fname_init): Delete declaration. * decl.c (cp_fname_init): Merge into only caller ... (cp_make_fname): ... here & refactor. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/cp-tree.h w/gcc/cp/cp-tree.h index 3ccd54ce24b..aa93b11b91f 100644 --- i/gcc/cp/cp-tree.h +++ w/gcc/cp/cp-tree.h @@ -6520,7 +6520,6 @@ extern tree create_implicit_typedef (tree, tree); extern int local_variable_p (const_tree); extern tree register_dtor_fn (tree); extern tmpl_spec_kind current_tmpl_spec_kind (int); -extern tree cp_fname_init (const char *, tree *); extern tree cxx_builtin_function (tree decl); extern tree cxx_builtin_function_ext_scope (tree decl); extern tree cxx_simulate_builtin_function_decl (tree); diff --git i/gcc/cp/decl.c w/gcc/cp/decl.c index d2a8d4012ab..6b306ee4667 100644 --- i/gcc/cp/decl.c +++ w/gcc/cp/decl.c @@ -4592,38 +4592,6 @@ cxx_init_decl_processing (void) using_eh_for_cleanups (); } -/* Generate an initializer for a function naming variable from - NAME. NAME may be NULL, to indicate a dependent name. TYPE_P is - filled in with the type of the init. */ - -tree -cp_fname_init (const char* name, tree *type_p) -{ - tree domain = NULL_TREE; - tree type; - tree init = NULL_TREE; - size_t length = 0; - - if (name) -{ - length = strlen (name); - domain = build_index_type (size_int (length)); - init = build_string (length + 1, name); -} - - type = cp_build_qualified_type (char_type_node, TYPE_QUAL_CONST); - type = build_cplus_array_type (type, domain); - - *type_p = type; - - if (init) -TREE_TYPE (init) = type; - else -init = error_mark_node; - - return init; -} - /* Create the VAR_DECL for __FUNCTION__ etc. ID is the name to give the decl, LOC is the location to give the decl, NAME is the initialization string and TYPE_DEP indicates whether NAME depended @@ -4634,31 +4602,45 @@ cp_fname_init (const char* name, tree *type_p) static tree cp_make_fname_decl (location_t loc, tree id, int type_dep) { - const char * name = NULL; - bool release_name = false; + tree domain = NULL_TREE; + tree init = NULL_TREE; + if (!(type_dep && in_template_function ())) { + const char *name = NULL; + bool release_name = false; + if (current_function_decl == NULL_TREE) name = "top level"; - else if (type_dep == 1) /* __PRETTY_FUNCTION__ */ - name = cxx_printable_name (current_function_decl, 2); - else if (type_dep == 0) /* __FUNCTION__ */ + else if (type_dep == 0) { + /* __FUNCTION__ */ name = fname_as_string (type_dep); release_name = true; } else - gcc_unreachable (); + { + /* __PRETTY_FUNCTION__ */ + gcc_checking_assert (type_dep == 1); + name = cxx_printable_name (current_function_decl, 2); + } + + size_t length = strlen (name); + domain = build_index_type (size_int (length)); + init = build_string (length + 1, name); + if (release_name) + free (const_cast (name)); } - tree type; - tree init = cp_fname_init (name, &type); - tree decl = build_decl (loc, VAR_DECL, id, type); - if (release_name) -free (CONST_CAST (char *, name)); + tree type = cp_build_qualified_type (char_type_node, TYPE_QUAL_CONST); + type = build_cplus_array_type (type, domain); - /* As we're using pushdecl_with_scope, we must set the context. */ - DECL_CONTEXT (decl) = current_function_decl; + if (init) +TREE_TYPE (init) = type; + else +init = error_mark_node; + + tree decl = build_decl (loc, VAR_DECL, id, type); TREE_READONLY (decl) = 1; DECL_ARTIFICIAL (decl) = 1; @@ -4667,13 +4649,10 @@ cp_make_fname_decl (location_t loc, tree id, int type_dep) TREE_USED (decl) = 1; - if (init) -{ - SET_DECL_VALUE_EXPR (decl, init); - DECL_HAS_VALUE_EXPR_P (decl) = 1; - /* For decl_constant_var_p. */ - DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1; -} + SET_DECL_VALUE_EXPR (decl, init); + DECL_HAS_VALUE_EXPR_P (decl) = 1; + /* For decl_constant_var_p. */ + DECL_INITIALIZED_BY_CONSTANT_EXPRESSION_P (decl) = 1; if (current_function_decl) { @@ -4685,7 +4664,7 @@ cp_make_fname_decl (location_t loc, tree id, int type_dep) else { DECL_THIS_STATIC (decl) = true; - pushdecl_top_level_and_finish (decl, NULL_TREE); + decl = pushdecl_top_level_and_finish (decl, NULL_TREE); } return decl;
c++: cleanup ctor_omit_inherited_parms [PR97268]
ctor_omit_inherited_parms was being somewhat abused. What I'd missed is that it checks for a base-dtor name, before proceeding with the check. But we ended up passing it that during cloning before we'd completed the cloning. It was also using DECL_ORIGIN to get to the in-charge ctor, but we sometimes zap DECL_ABSTRACT_ORIGIN, and it ends up processing the incoming function -- which happens to work. so, this breaks out a predicate that expects to get the incharge ctor, and will tell you whether its base ctor will need to omit the parms. We call that directly during cloning. Then the original fn is essentially just a wrapper, but uses DECL_CLONED_FUNCTION to get to the in-charge ctor. That uncovered abuse in add_method, which was happily passing TEMPLATE_DECLs to it. Let's not do that. add_method itself contained a loop mostly containing an 'if (nomatch) continue' idiom, except for a final 'if (match) {...}' check, which itself contained instances of the former idiom. I refactored that to use the former idiom throughout. In doing that I found a place where we'd issue an error, but then not actually reject the new member. gcc/cp/ * cp-tree.h (base_ctor_omit_inherited_parms): Declare. * class. (add_method): Refactor main loop, only pass fns to ctor_omit_inherited_parms. (build_cdtor_clones): Rename bool parms. (clone_cdtor): Call base_ctor_omit_inherited_parms. * method.c (base_ctor_omit_inherited_parms): New, broken out of ... (ctor_omit_inherited_parms): ... here, call it with DECL_CLONED_FUNCTION. gcc/testsuite/ * g++.dg/inherit/pr97268.C: New. pushing to trunk nathan -- Nathan Sidwell diff --git c/gcc/cp/class.c w/gcc/cp/class.c index c9a1f753d56..01780fe8291 100644 --- c/gcc/cp/class.c +++ w/gcc/cp/class.c @@ -1006,10 +1006,6 @@ add_method (tree type, tree method, bool via_using) for (ovl_iterator iter (current_fns); iter; ++iter) { tree fn = *iter; - tree fn_type; - tree method_type; - tree parms1; - tree parms2; if (TREE_CODE (fn) != TREE_CODE (method)) continue; @@ -1037,10 +1033,8 @@ add_method (tree type, tree method, bool via_using) functions in the derived class override and/or hide member functions with the same name and parameter types in a base class (rather than conflicting). */ - fn_type = TREE_TYPE (fn); - method_type = TREE_TYPE (method); - parms1 = TYPE_ARG_TYPES (fn_type); - parms2 = TYPE_ARG_TYPES (method_type); + tree fn_type = TREE_TYPE (fn); + tree method_type = TREE_TYPE (method); /* Compare the quals on the 'this' parm. Don't compare the whole types, as used functions are treated as @@ -1055,137 +1049,149 @@ add_method (tree type, tree method, bool via_using) || type_memfn_rqual (fn_type) != type_memfn_rqual (method_type))) continue; - /* For templates, the return type and template parameters - must be identical. */ - if (TREE_CODE (fn) == TEMPLATE_DECL - && (!same_type_p (TREE_TYPE (fn_type), - TREE_TYPE (method_type)) - || !comp_template_parms (DECL_TEMPLATE_PARMS (fn), - DECL_TEMPLATE_PARMS (method + tree real_fn = fn; + tree real_method = method; + + /* Templates and conversion ops must match return types. */ + if ((DECL_CONV_FN_P (fn) || TREE_CODE (fn) == TEMPLATE_DECL) + && !same_type_p (TREE_TYPE (fn_type), TREE_TYPE (method_type))) continue; + + /* For templates, the template parameters must be identical. */ + if (TREE_CODE (fn) == TEMPLATE_DECL) + { + if (!comp_template_parms (DECL_TEMPLATE_PARMS (fn), +DECL_TEMPLATE_PARMS (method))) + continue; - if (! DECL_STATIC_FUNCTION_P (fn)) + real_fn = DECL_TEMPLATE_RESULT (fn); + real_method = DECL_TEMPLATE_RESULT (method); + } + + tree parms1 = TYPE_ARG_TYPES (fn_type); + tree parms2 = TYPE_ARG_TYPES (method_type); + if (! DECL_STATIC_FUNCTION_P (real_fn)) parms1 = TREE_CHAIN (parms1); - if (! DECL_STATIC_FUNCTION_P (method)) + if (! DECL_STATIC_FUNCTION_P (real_method)) parms2 = TREE_CHAIN (parms2); - /* Bring back parameters omitted from an inherited ctor. */ - if (ctor_omit_inherited_parms (fn)) - parms1 = FUNCTION_FIRST_USER_PARMTYPE (DECL_ORIGIN (fn)); - if (ctor_omit_inherited_parms (method)) - parms2 = FUNCTION_FIRST_USER_PARMTYPE (DECL_ORIGIN (method)); + /* Bring back parameters omitted from an inherited ctor. The + method and the function can have different omittedness. */ + if (ctor_omit_inherited_parms (real_fn)) + parms1 = FUNCTION_FIRST_USER_PARMTYPE (DECL_CLONED_FUNCTION (real_fn)); + if (ctor_omit_inherited_parms (real_method)) + parms2 = (FUNCTION_FIRST_USER_PARMTYPE + (DECL_CLONED_FUNCTION (real_method))); - if (compparms (parms1, parms2)
c++: Hash table iteration for namespace-member spelling suggestions
For 'no such binding' errors, we iterate over binding levels to find a close match. At the namespace level we were using DECL_ANTICIPATED to skip undeclared builtins. But (a) there are other unnameable things there and (b) decl-anticipated is about to go away. This changes the namespace scanning to iterate over the hash table, and look at non-hidden bindings. This does mean we look at fewer strings (hurrarh), but the order we meet them is somewhat 'random'. Our distance measure is not very fine grained, and a couple of testcases change their suggestion. I notice for the c/c++ common one, we now match the output of the C compiler. For the other one we think 'int' and 'int64_t' have the same distance from 'int64', and now meet the former first. That's a little unfortunate. If it's too problematic I suppose we could sort the strings via an intermediate array before measuring distance. gcc/cp/ * name-lookup.c (consider_decl): New, broken out of ... (consider_binding_level): ... here. Iterate the hash table for namespace bindings. gcc/testsuite/ * c-c++-common/spellcheck-reserved.c: Adjust diagnostic. * g++.dg/spellcheck-typenames.C: Adjust diagnostic. pushing to trunk nathan -- Nathan Sidwell diff --git i/gcc/cp/name-lookup.c w/gcc/cp/name-lookup.c index 620f3a6..4024ceaa74b 100644 --- i/gcc/cp/name-lookup.c +++ w/gcc/cp/name-lookup.c @@ -6106,6 +6106,39 @@ qualified_namespace_lookup (tree scope, name_lookup *lookup) return found; } +static void +consider_decl (tree decl, best_match &bm, + bool consider_impl_names) +{ + /* Skip compiler-generated variables (e.g. __for_begin/__for_end + within range for). */ + if (TREE_CODE (decl) == VAR_DECL && DECL_ARTIFICIAL (decl)) +return; + + tree suggestion = DECL_NAME (decl); + if (!suggestion) +return; + + /* Don't suggest names that are for anonymous aggregate types, as + they are an implementation detail generated by the compiler. */ + if (IDENTIFIER_ANON_P (suggestion)) +return; + + const char *suggestion_str = IDENTIFIER_POINTER (suggestion); + + /* Ignore internal names with spaces in them. */ + if (strchr (suggestion_str, ' ')) +return; + + /* Don't suggest names that are reserved for use by the + implementation, unless NAME began with an underscore. */ + if (!consider_impl_names + && name_reserved_for_implementation_p (suggestion_str)) +return; + + bm.consider (suggestion_str); +} + /* Helper function for lookup_name_fuzzy. Traverse binding level LVL, looking for good name matches for NAME (and BM). */ @@ -6129,54 +6162,63 @@ consider_binding_level (tree name, best_match &bm, with an underscore. */ bool consider_implementation_names = (IDENTIFIER_POINTER (name)[0] == '_'); - for (tree t = lvl->names; t; t = TREE_CHAIN (t)) -{ - tree d = t; - - /* OVERLOADs or decls from using declaration are wrapped into - TREE_LIST. */ - if (TREE_CODE (d) == TREE_LIST) - d = OVL_FIRST (TREE_VALUE (d)); - - /* Don't use bindings from implicitly declared functions, - as they were likely misspellings themselves. */ - if (TREE_TYPE (d) == error_mark_node) - continue; - - /* Skip anticipated decls of builtin functions. */ - if (TREE_CODE (d) == FUNCTION_DECL - && fndecl_built_in_p (d) - && DECL_ANTICIPATED (d)) - continue; + if (lvl->kind != sk_namespace) +for (tree t = lvl->names; t; t = TREE_CHAIN (t)) + { + tree d = t; - /* Skip compiler-generated variables (e.g. __for_begin/__for_end - within range for). */ - if (TREE_CODE (d) == VAR_DECL - && DECL_ARTIFICIAL (d)) - continue; + /* OVERLOADs or decls from using declaration are wrapped into + TREE_LIST. */ + if (TREE_CODE (d) == TREE_LIST) + d = OVL_FIRST (TREE_VALUE (d)); - tree suggestion = DECL_NAME (d); - if (!suggestion) - continue; - - /* Don't suggest names that are for anonymous aggregate types, as - they are an implementation detail generated by the compiler. */ - if (IDENTIFIER_ANON_P (suggestion)) - continue; + /* Don't use bindings from implicitly declared functions, + as they were likely misspellings themselves. */ + if (TREE_TYPE (d) == error_mark_node) + continue; - const char *suggestion_str = IDENTIFIER_POINTER (suggestion); + /* If we want a typename, ignore non-types. */ + if (kind == FUZZY_LOOKUP_TYPENAME + && TREE_CODE (STRIP_TEMPLATE (d)) != TYPE_DECL) + continue; - /* Ignore internal names with spaces in them. */ - if (strchr (suggestion_str, ' ')) - continue; + consider_decl (d, bm, consider_implementation_names); + } + else +{ + /* Iterate over the namespace hash table, that'll have fewer + entries than the