[c++] Implement DR 976
DR 976 affects conversion operator deduction, swapping reference stripping and
cv-qual removal. This allows 'Y::operator P const & ()' to deduce T against a
call wanting plain A (previously that would fail as 'P const' cannot be deduced
from 'A').
It also affects deductions for array- or function-producing conversions, which I
suspect is rarer.
pushed to trunk
nathan
--
Nathan SidwellFrom 80f075b410125bddb31459428760645baba1a69f Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Sat, 10 Jun 2023 12:42:17 -0400
Subject: [PATCH] c++: Adjust conversion deduction [PR61663][DR976]
Drop the return type's reference before doing cvqual and related decays.
gcc/cp/
PR c++/61663
* pt.cc (maybe_adjust_types_for_deduction): Implement DR976.
gcc/testsuite/
* g++.dg/template/pr61663.C: New.
---
gcc/cp/pt.cc| 11 +++--
gcc/testsuite/g++.dg/template/pr61663.C | 63 +
2 files changed, 69 insertions(+), 5 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/template/pr61663.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 6b20c58ce66..6a2cf2c123f 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -22725,10 +22725,16 @@ maybe_adjust_types_for_deduction (tree tparms,
break;
case DEDUCE_CONV:
+ /* [temp.deduct.conv] First remove a reference type on parm.
+ DRs 322 & 976 affected this. */
+ if (TYPE_REF_P (*parm))
+ *parm = TREE_TYPE (*parm);
+
/* Swap PARM and ARG throughout the remainder of this
function; the handling is precisely symmetric since PARM
will initialize ARG rather than vice versa. */
std::swap (parm, arg);
+
break;
case DEDUCE_EXACT:
@@ -22795,11 +22801,6 @@ maybe_adjust_types_for_deduction (tree tparms,
result |= UNIFY_ALLOW_OUTER_MORE_CV_QUAL;
}
- /* DR 322. For conversion deduction, remove a reference type on parm
- too (which has been swapped into ARG). */
- if (strict == DEDUCE_CONV && TYPE_REF_P (*arg))
-*arg = TREE_TYPE (*arg);
-
return result;
}
diff --git a/gcc/testsuite/g++.dg/template/pr61663.C b/gcc/testsuite/g++.dg/template/pr61663.C
new file mode 100644
index 000..2964fa6c309
--- /dev/null
+++ b/gcc/testsuite/g++.dg/template/pr61663.C
@@ -0,0 +1,63 @@
+// { dg-do compile { target c++11 } }
+// PR c++/61663
+// DR 976, strip ref from conv op return type before doing
+// fn and ary decay or CV qual removal
+
+struct F
+{
+ template
+ operator const T&();
+};
+
+void Foo ()
+{
+ F f;
+ int i = f;
+}
+
+template struct X {};
+
+struct Y
+{
+ template operator X () &&; // #3
+ template operator X const & () const &; // #4
+};
+
+void Use (X);
+Y Val ();
+Y const &Ref ();
+
+// { dg-final { scan-assembler "_Z5Frob3v:.*_ZNO1Ycv1XIT_EIvEEv.*_Z3Use1XIvE" } }
+void Frob3 ()
+{
+ Use (Val ()); // #3
+}
+
+// { dg-final { scan-assembler "_Z5Frob4v:.*_ZNKR1YcvRK1XIT_EIvEEv.*_Z3Use1XIvE" } }
+void Frob4 ()
+{
+ Use (Ref ()); // #4
+}
+
+struct Z
+{
+ template using FnRef = void (&) (T);
+ template using AryRef = T (&)[];
+
+ template operator FnRef ();
+ template operator AryRef ();
+};
+
+// { dg-final { scan-assembler "_Z5Frob5R1Z:.*_ZN1ZcvRFvT_EIiEEv.*_ZN1ZcvRA_T_IiEEv" } }
+void Frob5 (Z &z)
+{
+ void (*fnptr)(int) = z;
+ int *iptr = z;
+}
+
+// { dg-final { scan-assembler "_Z5Frob6R1Z:.*_ZN1ZcvRFvT_EIfEEv.*_ZN1ZcvRA_T_IfEEv" } }
+void Frob6 (Z &z)
+{
+ void (&fnref)(float) = z;
+ float (&aryref)[] = z;
+}
--
2.40.1
Fix templated conversion operator demangling
I came across this when working on the conversion operator deduction fix. We'd
successfully demangle an instantiation of 'template operator X &
()', but fail for 'template operator X ()'. The demangle printer
was trying to specially handle the instantiation in the latter case -- seeing
the template inst of X. That code appears to be completely unnecessary. Added a
bunch of conversion operator demangling tests.
nathan
--
Nathan SidwellFrom 5a897036187468d4ded330b90b2abdaff5061ed6 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 12 Jun 2023 19:37:04 -0400
Subject: [PATCH] c++: Fix templated convertion operator demangling
Instantiations of templated conversion operators failed to demangle
for cases such as 'operator X', but worked for 'operator X
&', due to thinking the template instantiation of X was the
instantiation of the conversion operator itself.
libiberty/
* cp-demangle.c (d_print_conversion): Remove incorrect
template instantiation handling.
* testsuite/demangle-expected: Add testcases.
---
libiberty/cp-demangle.c | 28 +++
libiberty/testsuite/demangle-expected | 27 ++
2 files changed, 30 insertions(+), 25 deletions(-)
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 18ab28fd028..3bd303a7544 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -6660,32 +6660,10 @@ d_print_conversion (struct d_print_info *dpi, int options,
dpt.template_decl = dpi->current_template;
}
- if (d_left (dc)->type != DEMANGLE_COMPONENT_TEMPLATE)
-{
- d_print_comp (dpi, options, d_left (dc));
- if (dpi->current_template != NULL)
- dpi->templates = dpt.next;
-}
- else
-{
- d_print_comp (dpi, options, d_left (d_left (dc)));
+ d_print_comp (dpi, options, d_left (dc));
- /* For a templated cast operator, we need to remove the template
- parameters from scope after printing the operator name,
- so we need to handle the template printing here. */
- if (dpi->current_template != NULL)
- dpi->templates = dpt.next;
-
- if (d_last_char (dpi) == '<')
- d_append_char (dpi, ' ');
- d_append_char (dpi, '<');
- d_print_comp (dpi, options, d_right (d_left (dc)));
- /* Avoid generating two consecutive '>' characters, to avoid
- the C++ syntactic ambiguity. */
- if (d_last_char (dpi) == '>')
- d_append_char (dpi, ' ');
- d_append_char (dpi, '>');
-}
+ if (dpi->current_template != NULL)
+dpi->templates = dpt.next;
}
/* Initialize the information structure we use to pass around
diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected
index 52dff883a18..0acd2d635db 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -1662,3 +1662,30 @@ X::F()::{lambda(int)#1}::operator()(int) const
_Z1fIiEv1AIXnxtlT_EEE
void f(A)
+
+_ZNO1Ycv1XEv
+Y::operator X() &&
+
+_ZNO1Ycv1XIT_EIvEEv
+Y::operator X() &&
+
+_ZNO1Y3bobEv
+Y::bob() &&
+
+_ZNR1Y3bobEv
+Y::bob() &
+
+_ZNKR1YcvRK1XIT_EIvEEv
+Y::operator X const&() const &
+
+_ZZN1XIiEcviEvE1y
+X::operator int()::y
+
+_ZZN1XIiEcv1ZIiEEvE1y
+X::operator Z()::y
+
+_ZZN1Xcv1ZIT_EIiEEvE1y
+X::operator Z()::y
+
+_ZZN1XIfEcv1ZIT_EIiEEvE1y
+X::operator Z()::y
--
2.40.1
Re: [WIP RFC] Add support for keyword-based attributes
On 7/14/23 11:56, Richard Sandiford wrote: Summary: We'd like to be able to specify some attributes using keywords, rather than the traditional __attribute__ or [[...]] syntax. Would that be OK? In more detail: We'd like to add some new target-specific attributes for Arm SME. These attributes affect semantics and code generation and so they can't simply be ignored. Traditionally we've done this kind of thing by adding GNU attributes, via TARGET_ATTRIBUTE_TABLE in GCC's case. The problem is that both GCC and Clang have traditionally only warned about unrecognised GNU attributes, rather than raising an error. Older compilers might therefore be able to look past some uses of the new attributes and still produce object code, even though that object code is almost certainly going to be wrong. (The compilers will also emit a default-on warning, but that might go unnoticed when building a big project.) There are some existing attributes that similarly affect semantics in ways that cannot be ignored. vector_size is one obvious example. But that doesn't make it a good thing. :) Also, C++ says this for standard [[...]] attributes: For an attribute-token (including an attribute-scoped-token) not specified in this document, the behavior is implementation-defined; any such attribute-token that is not recognized by the implementation is ignored. which doubles down on the idea that attributes should not be used for necessary semantic information. There;s been quite a bit of discussion about the practicalities of that. As you say, there are existing, std-specified attributes, [[no_unique_address]] for instance, that affect user-visible object layout if ignored. Further, my understanding is that implementation-specific attributes are permitted to affect program semantics -- they're implementatin extensions. IMHO, attributes are the accepted mechanism for what you're describing. Compilers already have a way of dealing with them -- both parsing and, in general, representing them. I would be wary of inventing a different mechanism. Have you approached C or C++ std bodies for input? One of the attributes we'd like to add provides a new way of compiling existing code. The attribute doesn't require SME to be available; it just says that the code must be compiled so that it can run in either of two modes. This is probably the most dangerous attribute of the set, since compilers that ignore it would just produce normal code. That code might work in some test scenarios, but it would fail in others. The feeling from the Clang community was therefore that these SME attributes should use keywords instead, so that the keywords trigger an error with older compilers. However, it seemed wrong to define new SME-specific grammar rules, since the underlying problem is pretty generic. We therefore proposed having a type of keyword that can appear exactly where a standard [[...]] attribute can appear and that appertains to exactly what a standard [[...]] attribute would appertain to. No divergence or cherry-picking is allowed. For example: [[arm::foo]] would become: __arm_foo and: [[arm::bar(args)]] would become: __arm_bar(args) It wouldn't be possible to retrofit arguments to a keyword that previously didn't take arguments, since that could lead to parsing ambiguities. So when a keyword is first added, a binding decision would need to be made whether the keyword always takes arguments or is always standalone. For that reason, empty argument lists are allowed for keywords, even though they're not allowed for [[...]] attributes. The argument-less version was accepted into Clang, and I have a follow-on patch for handling arguments. Would the same thing be OK for GCC, in both the C and C++ frontends? The patch below is a proof of concept for the C frontend. It doesn't bootstrap due to warnings about uninitialised fields. And it doesn't have tests. But I did test it locally with various combinations of attribute_spec and it seemed to work as expected. The impact on the C frontend seems to be pretty small. It looks like the impact on the C++ frontend would be a bit bigger, but not much. The patch contains a logically unrelated change: c-common.h set aside 16 keywords for address spaces, but of the in-tree ports, the maximum number of keywords used is 6 (for amdgcn). The patch therefore changes the limit to 8 and uses 8 keywords for the new attributes. This keeps the number of reserved ids <= 256. A real, non-proof-of-concept patch series would: - Change the address-space keywords separately, and deal with any fallout. - Clean up the way that attributes are specified, so that it isn't necessary to update all definitions when adding a new field. - Allow more precise attribute requirements, such as "function decl only". - Add tests :) WDYT? Does this approach look OK in principle, or is it a non-starter? If it is a non-starter, the fallback would be to predefi
Re: [PATCH v3] Introduce attribute reverse_alias
Not commenting on the semantics, but the name seems unfortunate (hello
bikeshed). The documentation starts with 'attribute causes @var{name} to be
emitted as an alias to the definition'. So not emitting a 'reverse alias',
whatever that might be. It doesn;t seem to mention how reverse alias differs
from 'alias'. Why would 'alias' not DTRT?
Is is emitting a an additiona symbol -- ie, something like 'altname'. Or is it
something else? Is that symbol known in the current TU, or other TUs?
nathan
On 7/14/23 21:08, Alexandre Oliva wrote:
This patch introduces an attribute to add extra aliases to a symbol
when its definition is output. The main goal is to ease interfacing
C++ with Ada, as C++ mangled names have to be named, and in some cases
(e.g. when using stdint.h typedefs in function arguments) the symbol
names may vary across platforms.
The attribute is usable in C and C++, presumably in all C-family
languages. It can be attached to global variables and functions. In
C++, it can also be attached to namespace-scoped variables and
functions, static data members, member functions, explicit
instantiations and specializations of template functions, members and
classes.
When applied to constructors or destructor, additional reverse_aliases
with _Base and _Del suffixes are defined for variants other than
complete-object ones. This changes the assumption that clones always
carry the same attributes as their abstract declarations, so there is
now a function to adjust them.
C++ also had a bug in which attributes from local extern declarations
failed to be propagated to a preexisting corresponding
namespace-scoped decl. I've fixed that, and adjusted acc tests that
distinguished between C and C++ in this regard.
Applying the attribute to class types is only valid in C++, and the
effect is to attach the alias to the RTTI object associated with the
class type.
Regstrapped on x86_64-linux-gnu. Ok to install?
This is refreshed and renamed from earlier versions that named the
attribute 'exalias', and that AFAICT got stuck in name bikeshedding.
https://gcc.gnu.org/pipermail/gcc-patches/2020-August/551614.html
for gcc/ChangeLog
* attribs.cc: Include cgraph.h.
(decl_attributes): Allow late introduction of reverse_alias in
types.
(create_reverse_alias_decl, create_reverse_alias_decls): New.
* attribs.h: Declare them.
(FOR_EACH_REVERSE_ALIAS): New macro.
* cgraph.cc (cgraph_node::create): Create reverse_alias decls.
* varpool.cc (varpool_node::get_create): Create reverse_alias
decls.
* cgraph.h (symtab_node::remap_reverse_alias_target): New.
* symtab.cc (symtab_node::remap_reverse_alias_target):
Define.
* cgraphunit.cc (cgraph_node::analyze): Create alias_target
node if needed.
(analyze_functions): Fixup visibility of implicit alias only
after its node is analyzed.
* doc/extend.texi (reverse_alias): Document for variables,
functions and types.
for gcc/ada/ChangeLog
* doc/gnat_rm/interfacing_to_other_languages.rst: Mention
attribute reverse_alias to give RTTI symbols mnemonic names.
* doc/gnat_ugn/the_gnat_compilation_model.rst: Mention
attribute reverse_alias. Fix incorrect ref to C1 ctor variant.
for gcc/c-family/ChangeLog
* c-ada-spec.cc (pp_asm_name): Use first reverse_alias if
available.
* c-attribs.cc (handle_reverse_alias_attribute): New.
(c_common_attribute_table): Add reverse_alias.
(handle_copy_attribute): Do not copy reverse_alias.
for gcc/c/ChangeLog
* c-decl.cc (duplicate_decls): Remap reverse_alias target.
for gcc/cp/ChangeLog
* class.cc (adjust_clone_attributes): New.
(copy_fndecl_with_name, build_clone): Call it.
* cp-tree.h (adjust_clone_attributes): Declare.
(update_reverse_alias_interface): Declare.
(update_tinfo_reverse_alias): Declare.
* decl.cc (duplicate_decls): Remap reverse_alias target.
Adjust clone attributes.
(grokfndecl): Tentatively create reverse_alias decls after
adding attributes in e.g. a template member function explicit
instantiation.
* decl2.cc (cplus_decl_attributes): Update tinfo
reverse_alias.
(copy_interface, update_reverse_alias_interface): New.
(determine_visibility): Update reverse_alias interface.
(tentative_decl_linkage, import_export_decl): Likewise.
* name-lookup.cc: Include target.h and cgraph.h.
(push_local_extern_decl_alias): Merge attributes with
namespace-scoped decl, and drop duplicate reverse_alias.
* optimize.cc (maybe_clone_body): Re-adjust attributes after
cloning them. Update reverse_alias interface.
* rtti.cc: Include attribs.h and cgraph.h.
(get_tinfo_decl): Copy reverse_alias attributes from type to
tinfo decl. Create re
Re: [PATCH v5 4/5] c++modules: report imported CMI files as dependencies
On 7/18/23 16:52, Jason Merrill wrote: On 6/25/23 12:36, Ben Boeckel wrote: On Fri, Jun 23, 2023 at 08:12:41 -0400, Nathan Sidwell wrote: On 6/22/23 22:45, Ben Boeckel wrote: On Thu, Jun 22, 2023 at 17:21:42 -0400, Jason Merrill wrote: On 1/25/23 16:06, Ben Boeckel wrote: They affect the build, so report them via `-MF` mechanisms. Why isn't this covered by the existing code in preprocessed_module? It appears as though it is neutered in patch 3 where `write_make_modules_deps` is used in `make_write` (or will use that name Why do you want to record the transitive modules? I would expect just noting the ones with imports directly in the TU would suffice (i.e check the 'outermost' arg) FWIW, only GCC has "fat" modules. MSVC and Clang both require the transitive closure to be passed. The idea there is to minimize the size of individual module files. If GCC only reads the "fat" modules, then only those should be recorded. If it reads other modules, they should be recorded as well. Please explain what you mean by fat modules. There seems to be confusion. But wouldn't the transitive modules be dependencies of the direct imports, so (re)building the direct imports would first require building the transitive modules anyway? Expressing the transitive closure of dependencies for each importer seems redundant when it can be easily derived from the direct dependencies of each module. Jason -- Nathan Sidwell
Re: [PATCH v5 4/5] c++modules: report imported CMI files as dependencies
On 7/18/23 20:01, Ben Boeckel wrote: On Tue, Jul 18, 2023 at 16:52:44 -0400, Jason Merrill wrote: On 6/25/23 12:36, Ben Boeckel wrote: On Fri, Jun 23, 2023 at 08:12:41 -0400, Nathan Sidwell wrote: On 6/22/23 22:45, Ben Boeckel wrote: On Thu, Jun 22, 2023 at 17:21:42 -0400, Jason Merrill wrote: On 1/25/23 16:06, Ben Boeckel wrote: They affect the build, so report them via `-MF` mechanisms. Why isn't this covered by the existing code in preprocessed_module? It appears as though it is neutered in patch 3 where `write_make_modules_deps` is used in `make_write` (or will use that name Why do you want to record the transitive modules? I would expect just noting the ones with imports directly in the TU would suffice (i.e check the 'outermost' arg) FWIW, only GCC has "fat" modules. MSVC and Clang both require the transitive closure to be passed. The idea there is to minimize the size of individual module files. If GCC only reads the "fat" modules, then only those should be recorded. If it reads other modules, they should be recorded as well. For clarification, given: * a.cppm ``` export module a; ``` * b.cppm ``` export module b; import a; ``` * use.cppm ``` import b; ``` in a "fat" module setup, `use.cppm` only needs to be told about `b.cmi` because it contains everything that an importer needs to know about the `a` module (reachable types, re-exported bits, whateve > With the "thin" modules, `a.cmi` must be specified when compiling `use.cppm` to satisfy anything that may be required transitively (e.g., a return GCC is neither of these descriptions. a CMI does not contain the transitive closure of its imports. It contains an import table. That table lists the transitive closure of its imports (it needs that closure to do remapping), and that table contains the CMI pathnames of the direct imports. Those pathnames are absolute, if the mapper provded an absolute pathm or relative to the CMI repo. The rationale here is that if you're building a CMI, Foo, which imports a bunch of modules, those imported CMIs will have the same (relative) location in this compilation and in compilations importing Foo (why would you move them?) Note this is NOT inhibiting relocatable builds, because of the CMI repo. Maybe I'm missing how this *actually* works in GCC as I've really only interacted with it through the command line, but I've not needed to mention `a.cmi` when compiling `use.cppm`. Is `a.cmi` referenced and read through some embedded information in `b.cmi` or does `b.cmi` include enough information to not need to read it at all? If the former, distributed builds are going to have a problem knowing what files to send just from the command line (I'll call this "implicit thin"). If the latter, that is the "fat" CMI that I'm thinking of. please don't use perjorative terms like 'fat' and 'thin'. But wouldn't the transitive modules be dependencies of the direct imports, so (re)building the direct imports would first require building the transitive modules anyway? Expressing the transitive closure of dependencies for each importer seems redundant when it can be easily derived from the direct dependencies of each module. I'm not concerned whether it is transitive or not, really. If a file is read, it should be reported here regardless of the reason. Note that caching mechanisms may skip actually *doing* the reading, but the dependencies should still be reported from the cached results as-if the real work had been performed. --Ben -- Nathan Sidwell
Re: [PATCH v5 4/5] c++modules: report imported CMI files as dependencies
On 7/19/23 20:47, Ben Boeckel wrote: On Wed, Jul 19, 2023 at 17:11:08 -0400, Nathan Sidwell wrote: GCC is neither of these descriptions. a CMI does not contain the transitive closure of its imports. It contains an import table. That table lists the transitive closure of its imports (it needs that closure to do remapping), and that table contains the CMI pathnames of the direct imports. Those pathnames are absolute, if the mapper provded an absolute pathm or relative to the CMI repo. The rationale here is that if you're building a CMI, Foo, which imports a bunch of modules, those imported CMIs will have the same (relative) location in this compilation and in compilations importing Foo (why would you move them?) Note this is NOT inhibiting relocatable builds, because of the CMI repo. But it is inhibiting distributed builds because the distributing tool would need to know: - what CMIs are actually imported (here, "read the module mapper file" (in CMake's case, this is only the modules that are needed; a single massive mapper file for an entire project would have extra entries) or "act as a proxy for the socket/program specified" for other approaches); This information is in the machine (& human) README section of the CMI. - read the CMIs as it sends to the remote side to gather any other CMIs that may be needed (recursively); Contrast this with the MSVC and Clang (17+) mechanism where the command line contains everything that is needed and a single bolus can be sent. um, the build system needs to create that command line? Where does the build system get that information? IIUC it'll need to read some file(s) to do that. And relocatable is probably fine. How does it interact with reproducible builds? Or are GCC CMIs not really something anyone should consider for installation (even as a "here, maybe this can help consumers" mechanism)? Module CMIs should be considered a cacheable artifact. They are neither object files nor source files. On 7/18/23 20:01, Ben Boeckel wrote: Maybe I'm missing how this *actually* works in GCC as I've really only interacted with it through the command line, but I've not needed to mention `a.cmi` when compiling `use.cppm`. Is `a.cmi` referenced and read through some embedded information in `b.cmi` or does `b.cmi` include enough information to not need to read it at all? If the former, distributed builds are going to have a problem knowing what files to send just from the command line (I'll call this "implicit thin"). If the latter, that is the "fat" CMI that I'm thinking of. please don't use perjorative terms like 'fat' and 'thin'. Sorry, I was internally analogizing to "thinLTO". --Ben -- Nathan Sidwell
Re: [PATCH v5 4/5] c++modules: report imported CMI files as dependencies
On 7/21/23 10:57, Ben Boeckel wrote: On Thu, Jul 20, 2023 at 17:00:32 -0400, Nathan Sidwell wrote: On 7/19/23 20:47, Ben Boeckel wrote: But it is inhibiting distributed builds because the distributing tool would need to know: - what CMIs are actually imported (here, "read the module mapper file" (in CMake's case, this is only the modules that are needed; a single massive mapper file for an entire project would have extra entries) or "act as a proxy for the socket/program specified" for other approaches); This information is in the machine (& human) README section of the CMI. Ok. That leaves it up to distributing build tools to figure out at least. - read the CMIs as it sends to the remote side to gather any other CMIs that may be needed (recursively); Contrast this with the MSVC and Clang (17+) mechanism where the command line contains everything that is needed and a single bolus can be sent. um, the build system needs to create that command line? Where does the build system get that information? IIUC it'll need to read some file(s) to do that. It's chained through the P1689 information in the collator as needed. No extra files need to be read (at least with CMake's approach); certainly not CMI files. It occurs to me that the model I am envisioning is similar to CMake's object libraries. Object libraries are a convenient name for a bunch of object files. IIUC they're linked by naming the individual object files (or I think the could be implemented as a static lib linked with --whole-archive path/to/libfoo.a -no-whole-archive. But for this conversation consider them a bunch of separate object files with a convenient group name. Consider also that object libraries could themselves contain object libraries (I don't know of they can, but it seems like a useful concept). Then one could create an object library from a collection of object files and object libraries (recursively). CMake would handle the transitive gtaph. Now, allow an object library to itself have some kind of tangible, on-disk representation. *BUT* not like a static library -- it doesn't include the object files. Now that immediately maps onto modules. CMI: Object library Direct imports: Direct object libraries of an object library This is why I don't understand the need explicitly indicate the indirect imports of a CMI. CMake knows them, because it knows the graph. And relocatable is probably fine. How does it interact with reproducible builds? Or are GCC CMIs not really something anyone should consider for installation (even as a "here, maybe this can help consumers" mechanism)? Module CMIs should be considered a cacheable artifact. They are neither object files nor source files. Sure, cachable sounds fine. What about the installation? --Ben -- Nathan Sidwell
c++: Prune ordinary locations
Like macro locations, we only need to emit ordinary location
information for locations emitted into the CMI. This adds a hash table
noting which ordinary lines are needed. These are then sorted and
(sufficiently) adjacent lines are coalesced to a single map. There is
a tradeoff here, allowing greater separation reduces the number of
line maps, but increases the number of locations. It appears allowing
2 or 3 intervening lines is the sweet spot, and this patch chooses 2.
Compiling a hello-world #includeing in it's GMF gives a
reduction in number of locations of 5 fold, but an increase in number
of maps about 4 fold. Examining one of the xtreme-header tests we
halve the number of locations and increase the number of maps by 9
fold.
Module interfaces that emit no entities (or macros, if a header-unit),
will now have no location tables.
nathan
--
Nathan SidwellFrom 47794da8d8ea61ea8f6a0e21d3c1731a56d0cff3 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 24 Jun 2022 05:57:42 -0700
Subject: [PATCH] c++: Prune ordinary locations
Like macro locations, we only need to emit ordinary location
information for locations emitted into the CMI. This adds a hash table
noting which ordinary lines are needed. These are then sorted and
(sufficiently) adjacent lines are coalesced to a single map. There is
a tradeoff here, allowing greater separation reduces the number of
line maps, but increases the number of locations. It appears allowing
2 or 3 intervening lines is the sweet spot, and this patch chooses 2.
Compiling a hello-world #includeing in it's GMF gives a
reduction in number of locations of 5 fold, but an increase in number
of maps about 4 fold. Examining one of the xtreme-header tests we
halve the number of locations and increase the number of maps by 9
fold.
Module interfaces that emit no entities (or macros, if a header-unit),
will now have no location tables.
gcc/cp/
* module.cc
(struct ord_loc_info, ord_loc_traits): New.
(ord_loc_tabke, ord_loc_remap): New globals.
(struct location_map_info): Delete.
(struct module_state_config): Rename ordinary_loc_align to
loc_range_bits.
(module_for_ordinary_loc): Adjust.
(module_state::note_location): Note ordinary locations,
return bool.
(module_state::write_location): Adjust ordinary location
streaming.
(module_state::read_location): Likewise.
(module_state::write_init_maps): Allocate ord_loc_table.
(module_state::write_prepare_maps): Reimplement ordinary
map preparation.
(module_state::read_prepare_maps): Adjust.
(module_state::write_ordinary_maps): Reimplement.
(module_state::write_macro_maps): Adjust.
(module_state::read_ordinary_maps): Reimplement.
(module_state::write_macros): Adjust.
(module_state::write_config): Adjust.
(module_state::read_config): Adjust.
(module_state::write_begin): Adjust.
(module_state::read_initial): Adjust.
gcc/testsuite/
* g++.dg/modules/loc-prune-1.C: Adjust.
* g++.dg/modules/loc-prune-4.C: New.
* g++.dg/modules/pr98718_a.C: Adjust.
* g++.dg/modules/pr98718_b.C: Adjust.
* g++.dg/modules/pr99072.H: Adjust.
---
gcc/cp/module.cc | 625 -
gcc/testsuite/g++.dg/modules/loc-prune-1.C | 2 +-
gcc/testsuite/g++.dg/modules/loc-prune-4.C | 22 +
gcc/testsuite/g++.dg/modules/pr98718_a.C | 2 +-
gcc/testsuite/g++.dg/modules/pr98718_b.C | 2 +-
gcc/testsuite/g++.dg/modules/pr99072.H | 4 +-
6 files changed, 373 insertions(+), 284 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/loc-prune-4.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 238a5eb74d2..f27f4d091e5 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3240,6 +3240,66 @@ public:
static loc_spans spans;
+/* Information about ordinary locations we stream out. */
+struct ord_loc_info
+{
+ const line_map_ordinary *src; // line map we're based on
+ unsigned offset; // offset to this line
+ unsigned span; // number of locs we span
+ unsigned remap; // serialization
+
+ static int compare (const void *a_, const void *b_)
+ {
+auto *a = static_cast (a_);
+auto *b = static_cast (b_);
+
+if (a->src != b->src)
+ return a->src < b->src ? -1 : +1;
+
+// Ensure no overlap
+gcc_checking_assert (a->offset + a->span <= b->offset
+ || b->offset + b->span <= a->offset);
+
+gcc_checking_assert (a->offset != b->offset);
+return a->offset < b->offset ? -1 : +1;
+ }
+};
+struct ord_loc_traits
+{
+ typedef ord_loc_info value_type;
+ typedef value_type compare_type;
+
+ static const bool empty_zero_p = false;
+
+ static hashval_t hash (const value_type &v)
+ {
+auto h = pointer_hash::hash (v.src);
+return iterative_hash_hashval_t (v.offset, h);
+ }
+ static bool equal (const value_type &v, const compare_type p)
+ {
+return v.src == p.src && v.offset == p.offset;
+ }
+
+ static void mark_empty (value_type &v)
+ {
+v.src = nullptr;
+ }
+ static bool is_empty (value_type &v)
+ {
+return !v.src;
C++: add -std={c,gnu}++{current,future}
Inspired by a user question. Jason, thoughts?
Since C++ is such a moving target, Microsoft have /std:c++latest
(AFAICT clang does not), to select the currently implemented version
of the working paper. But the use of 'std:latest' is somewhat
ambiguous -- the current std is C++20 -- that's the latest std, the
next std will more than likely but not necessarily be C++23. So this
adds:
-std=c++current -- the current std (c++20)
-std=c++future -- the working paper (c++2b)
also adds gnu++current and gnu++future to select the gnu-extended
variants.
nathan
--
Nathan SidwellFrom 9671f4d5e7efa130280b6d50fb4e9e8492d5b587 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 13 Jul 2022 12:11:40 -0700
Subject: [PATCH] C++: add -std={c,gnu}++{current,future}
Since C++ is such a moving target, Microsoft have /std:c++latest
(AFAICT clang does not), to select the currently implemented version
of the working paper. But the use of 'std:latest' is somewhat
ambiguous -- the current std is C++20 -- that's the latest std, the
next std will more than likely but not necessarily be C++23. So this
adds:
-std=c++current -- the current std (c++20)
-std=c++future -- the working paper (c++2b)
also adds gnu++current and gnu++future to select the gnu-extended
variants.
gcc/
* doc/invoke.texi (-std=): Document new c++ current & future
options.
gcc/c-family/
* c.opt (-std={c,gnu}++{current,future}: New alias options.
gcc/testsuite/
* g++.dg/gnu-current.C: New.
* g++.dg/gnu-future.C: New.
* g++.dg/std-current.C: New.
* g++.dg/std-future.C: New.
---
gcc/c-family/c.opt | 16
gcc/doc/invoke.texi| 23 +++
gcc/testsuite/g++.dg/gnu-current.C | 7 +++
gcc/testsuite/g++.dg/gnu-future.C | 7 +++
gcc/testsuite/g++.dg/std-current.C | 11 +++
gcc/testsuite/g++.dg/std-future.C | 8
6 files changed, 72 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/gnu-current.C
create mode 100644 gcc/testsuite/g++.dg/gnu-future.C
create mode 100644 gcc/testsuite/g++.dg/std-current.C
create mode 100644 gcc/testsuite/g++.dg/std-future.C
diff --git a/gcc/c-family/c.opt b/gcc/c-family/c.opt
index 44e1a60ce24..9292029a967 100644
--- a/gcc/c-family/c.opt
+++ b/gcc/c-family/c.opt
@@ -2321,6 +2321,14 @@ std=c++23
C++ ObjC++ Undocumented
Conform to the ISO 2023 C++ draft standard (experimental and incomplete support).
+std=c++current
+C++ ObjC++ Alias(std=c++20) Undocumented
+Conform to the current ISO C++ standard (C++20).
+
+std=c++future
+C++ ObjC++ Alias(std=c++23) Undocumented
+Conform to a future ISO C++ standard (C++2b, experimentatl and incomplete support).
+
std=c11
C ObjC
Conform to the ISO 2011 C standard.
@@ -2407,6 +2415,14 @@ std=gnu++23
C++ ObjC++ Undocumented
Conform to the ISO 2023 C++ draft standard with GNU extensions (experimental and incomplete support).
+std=gnu++current
+C++ ObjC++ Alias(std=gnu++20) Undocumented
+Conform to the current ISO C++ standard with GNU extensions (C++20).
+
+std=gnu++future
+C++ ObjC++ Alias(std=gnu++23) Undocumented
+Conform to a future ISO C++ standard with GNU extensions (C++2b, experimentatl and incomplete support).
+
std=gnu11
C ObjC
Conform to the ISO 2011 C standard with GNU extensions.
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index d5ff1018372..1c0edb9df68 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -2462,6 +2462,17 @@ GNU dialect of @option{-std=c++17}.
This is the default for C++ code.
The name @samp{gnu++1z} is deprecated.
+@item gnu++current
+@itemx gnu++current
+GNU dialect of the current C++ standard, currently @option{-std=gnu++20}.
+The C++ version selected by this option is a moving target.
+
+@item gnu++future
+@itemx gnu++future
+GNU dialect of the next C++ standard, currently @option{-std=gnu++2b}.
+The C++ version selected by this option is a moving target (as are the
+semantics of that proposed version).
+
@item c++20
@itemx c++2a
The 2020 ISO C++ standard plus amendments.
@@ -2487,6 +2498,18 @@ change in incompatible ways in future releases.
GNU dialect of @option{-std=c++2b}. Support is highly experimental,
and will almost certainly change in incompatible ways in future
releases.
+
+@item c++current
+@itemx c++current
+The current C++ standard, currently @option{-std=gnu++20}.
+The C++ version selected by this option is a moving target.
+
+@item c++future
+@itemx c++future
+The next C++ standard, currently @option{-std=gnu++2b}.
+The C++ version selected by this option is a moving target (as are the
+semantics of that proposed version).
+
@end table
@item -aux-info @var{filename}
diff --git a/gcc/testsuite/g++.dg/gnu-current.C b/gcc/testsuite/g++.dg/gnu-current.C
new file mode 100644
index 000..c95c56d3ad8
--- /dev/null
+++ b/gcc/testsuite/g++.dg/gnu-current.C
@@ -0,0 +1,7 @@
+// { dg-do compile }
+// { dg-options -std=gnu++current }
+
+static_assert (__cplusplus == 202002L, "time has moved o
Re: [PATCH v5 4/5] c++modules: report imported CMI files as dependencies
On 6/22/23 22:45, Ben Boeckel wrote: On Thu, Jun 22, 2023 at 17:21:42 -0400, Jason Merrill wrote: On 1/25/23 16:06, Ben Boeckel wrote: They affect the build, so report them via `-MF` mechanisms. Why isn't this covered by the existing code in preprocessed_module? It appears as though it is neutered in patch 3 where `write_make_modules_deps` is used in `make_write` (or will use that name Why do you want to record the transitive modules? I would expect just noting the ones with imports directly in the TU would suffice (i.e check the 'outermost' arg) nathan -- Nathan Sidwell
Re: C++ modules and AAPCS/ARM EABI clash on inline key methods
On 2/21/23 11:31, Richard Earnshaw wrote:
I started looking at this a few weeks back, but I was a bit confused by the
testcase and then never got around to following up.
The Arm C++ binding rules normally exclude using an inline function definition
from being chosen as the key function because this not uncommonly appears in a
header file; instead a later function in the class is defined to take that role,
if such a function exists (in effect an inline function is treated the same way
as if the function definition appeared within the class definition itself).
But in this class we have only the one function, so in effect this testcase
appears to fall back to the 'no key function' rule and as such I'd expect the
class impedimenta to be required in all instances of the function. That doesn't
seem to be happening, so either there's something I'm missing, or there's
something the compiler is doing wrong for this case.
Nathan, your insights would be appreciated here.
Right in the ARM ABI we'll be emitting the vtables etc in any TU that might need
them. IIUC that's any TU that creates (or destroys?) an object of type Visitor
(or derived from there). That source file does not do that.
I see I didn't add a testcase where the only vfunc was declared inline in the
class itself. Thus there's no generic-abi equivalent of ARM's behaviour.
I don't think arm's behavior should be an xfail, instead restrict the checks to
!arm-eabi
nathan
R.
for gcc/testsuite/ChangeLog
PR c++/105224
* g++.dg/modules/virt-2_a.C: XFAIL syms on arm*-*-*.
---
gcc/testsuite/g++.dg/modules/virt-2_a.C | 6 +++---
1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/gcc/testsuite/g++.dg/modules/virt-2_a.C
b/gcc/testsuite/g++.dg/modules/virt-2_a.C
index 580552be5a0d8..b265515e2c7fd 100644
--- a/gcc/testsuite/g++.dg/modules/virt-2_a.C
+++ b/gcc/testsuite/g++.dg/modules/virt-2_a.C
@@ -22,6 +22,6 @@ export int Visit (Visitor *v)
}
// Emit here
-// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} } }
-// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} } }
-// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} } }
+// { dg-final { scan-assembler {_ZTVW3foo7Visitor:} { xfail arm*-*-* } } }
+// { dg-final { scan-assembler {_ZTIW3foo7Visitor:} { xfail arm*-*-* } } }
+// { dg-final { scan-assembler {_ZTSW3foo7Visitor:} { xfail arm*-*-* } } }
--
Nathan Sidwell
[patch] Allow plugin-specific dumps
PR 99451 is about the inability to name tree and rtl dumps by plugin name. And
includes a patch. But then I worked around the problem and forgot about it.
Here it is again, retested against trunk.
ok?
nathan
--
Nathan SidwellFrom e54518bc5e59ef5cdc21c652ceac41bd0c0f436c Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 17 May 2023 19:27:13 -0400
Subject: [PATCH] Allow plugin dumps
Defer dump option parsing until plugins are initialized. This allows one to
use plugin names for dumps.
PR other/99451
gcc/
* opts.h (handle_deferred_dump_options): Declare.
* opts-global.cc (handle_common_deferred_options): Do not handle
dump options here.
(handle_deferred_dump_options): New.
* toplev.cc (toplev::main): Call it after plugin init.
---
gcc/opts-global.cc | 20 +++-
gcc/opts.h | 1 +
gcc/toplev.cc | 4
3 files changed, 24 insertions(+), 1 deletion(-)
diff --git a/gcc/opts-global.cc b/gcc/opts-global.cc
index 054169158b1..a61c701621d 100644
--- a/gcc/opts-global.cc
+++ b/gcc/opts-global.cc
@@ -401,7 +401,7 @@ handle_common_deferred_options (void)
break;
case OPT_fdump_:
- g->get_dumps ()->dump_switch_p (opt->arg);
+ /* Deferred until plugins initialized. */
break;
case OPT_fopt_info_:
@@ -494,3 +494,21 @@ handle_common_deferred_options (void)
}
}
}
+
+/* Handle deferred dump options. */
+
+void
+handle_deferred_dump_options (void)
+{
+ unsigned int i;
+ cl_deferred_option *opt;
+ vec v;
+
+ if (common_deferred_options)
+v = *((vec *) common_deferred_options);
+ else
+v = vNULL;
+ FOR_EACH_VEC_ELT (v, i, opt)
+if (opt->opt_index == OPT_fdump_)
+ g->get_dumps ()->dump_switch_p (opt->arg);
+}
diff --git a/gcc/opts.h b/gcc/opts.h
index 9959a440ca1..00f377f9ca7 100644
--- a/gcc/opts.h
+++ b/gcc/opts.h
@@ -425,6 +425,7 @@ extern void control_warning_option (unsigned int opt_index, int kind,
extern char *write_langs (unsigned int mask);
extern void print_ignored_options (void);
extern void handle_common_deferred_options (void);
+extern void handle_deferred_dump_options (void);
unsigned int parse_sanitizer_options (const char *, location_t, int,
unsigned int, int, bool);
diff --git a/gcc/toplev.cc b/gcc/toplev.cc
index d53b5e78ae3..c606a0697b7 100644
--- a/gcc/toplev.cc
+++ b/gcc/toplev.cc
@@ -2253,6 +2253,10 @@ toplev::main (int argc, char **argv)
initialize_plugins ();
+ /* Handle the dump options now that plugins have had a chance to install new
+ passes. */
+ handle_deferred_dump_options ();
+
if (version_flag)
print_version (stderr, "", true);
--
2.40.1
Re: [PATCH] c++: modules ICE with typename friend declaration
Thanks, this looks right. Sigh templates can mess up ones mental invariants!
The test case should really be a foo_[ab].C kind, to test both sides of the
streaming. Bonus points for using the template after importing. And you
need the dg-module-cmi annotation to check /and then delete/ the gcm file
produced.
nathan
On Thu, Sep 15, 2022, 22:16 Patrick Palka wrote:
> A couple of xtreme-header-* modules tests began ICEing in C++23 mode
> ever since r13-2650-g5d84a4418aa962 introduced into the
> dependently scoped friend declaration
>
> friend /* typename */ _OuterIter::value_type;
>
> ultimately because the streaming code assumes a TYPE_P friend must
> be a class type, but here it's a TYPENAME_TYPE, which doesn't have
> a TEMPLATE_INFO or CLASSTYPE_BEFRIENDING_CLASSES. This patch tries
> to correct this in a minimal way.
>
> Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
>
> gcc/cp/ChangeLog:
>
> * module.cc (friend_from_decl_list): Don't consider
> CLASSTYPE_TEMPLATE_INFO for a TYPENAME_TYPE friend.
> (trees_in::read_class_def): Don't add to
> CLASSTYPE_BEFRIENDING_CLASSES for a TYPENAME_TYPE friend.
>
> gcc/testsuite/ChangeLog:
>
> * g++.dg/modules/typename-friend.C: New test.
> ---
> gcc/cp/module.cc | 5 +++--
> gcc/testsuite/g++.dg/modules/typename-friend.C | 9 +
> 2 files changed, 12 insertions(+), 2 deletions(-)
> create mode 100644 gcc/testsuite/g++.dg/modules/typename-friend.C
>
> diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
> index f27f4d091e5..1a1ff5be574 100644
> --- a/gcc/cp/module.cc
> +++ b/gcc/cp/module.cc
> @@ -4734,7 +4734,8 @@ friend_from_decl_list (tree frnd)
>if (TYPE_P (frnd))
> {
> res = TYPE_NAME (frnd);
> - if (CLASSTYPE_TEMPLATE_INFO (frnd))
> + if (CLASS_TYPE_P (frnd)
> + && CLASSTYPE_TEMPLATE_INFO (frnd))
> tmpl = CLASSTYPE_TI_TEMPLATE (frnd);
> }
>else if (DECL_TEMPLATE_INFO (frnd))
> @@ -12121,7 +12122,7 @@ trees_in::read_class_def (tree defn, tree
> maybe_template)
> {
> tree f = TREE_VALUE (friend_classes);
>
> - if (TYPE_P (f))
> + if (CLASS_TYPE_P (f))
> {
> CLASSTYPE_BEFRIENDING_CLASSES (f)
> = tree_cons (NULL_TREE, type,
> diff --git a/gcc/testsuite/g++.dg/modules/typename-friend.C
> b/gcc/testsuite/g++.dg/modules/typename-friend.C
> new file mode 100644
> index 000..d8faf7955c3
> --- /dev/null
> +++ b/gcc/testsuite/g++.dg/modules/typename-friend.C
> @@ -0,0 +1,9 @@
> +// { dg-additional-options "-fmodules-ts" }
> +
> +export module x;
> +
> +template
> +struct A {
> + friend typename T::type;
> + friend void f(A) { }
> +};
> --
> 2.37.3.662.g36f8e7ed7d
>
>
Re: [PATCH] c++: modules ICE with typename friend declaration
On 9/16/22 11:54, Patrick Palka wrote: On Fri, 16 Sep 2022, Nathan Sidwell wrote: Thanks, this looks right. Sigh templates can mess up ones mental invariants! The test case should really be a foo_[ab].C kind, to test both sides of the streaming. Bonus points for using the template after importing. And you need the dg-module-cmi annotation to check /and then delete/ the gcm file produced. Aha, thanks very much for the pointers, I redid the testcase using lang-3_[abc].C as an example. How does the following look? yes, that's right, thanks! nathan -- Nathan Sidwell
Re: [PATCH] c++: stream PACK_EXPANSION_EXTRA_ARGS [PR106761]
On 9/19/22 09:52, Patrick Palka wrote:
It looks like some xtreme-header-* tests are failing after the libstdc++
change r13-2158-g02f6b405f0e9dc ultimately because we're neglecting
to stream PACK_EXPANSION_EXTRA_ARGS, which leads to false equivalences
of different partial instantiations of _TupleConstraints::__constructible.
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
PR c++/106761
gcc/cp/ChangeLog:
* module.cc (trees_out::type_node) :
Stream PACK_EXPANSION_EXTRA_ARGS.
(trees_in::tree_node) : Likewise.
Looks good, I wonder why I missed that. (I guess extracting a testcase
out of the headers was too tricky?)
nathan
---
gcc/cp/module.cc | 3 +++
1 file changed, 3 insertions(+)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 1a1ff5be574..9a9ef4e3332 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -8922,6 +8922,7 @@ trees_out::type_node (tree type)
if (streaming_p ())
u (PACK_EXPANSION_LOCAL_P (type));
tree_node (PACK_EXPANSION_PARAMETER_PACKS (type));
+ tree_node (PACK_EXPANSION_EXTRA_ARGS (type));
break;
case TYPENAME_TYPE:
@@ -9455,12 +9456,14 @@ trees_in::tree_node (bool is_use)
{
bool local = u ();
tree param_packs = tree_node ();
+ tree extra_args = tree_node ();
if (!get_overrun ())
{
tree expn = cxx_make_type (TYPE_PACK_EXPANSION);
SET_TYPE_STRUCTURAL_EQUALITY (expn);
PACK_EXPANSION_PATTERN (expn) = res;
PACK_EXPANSION_PARAMETER_PACKS (expn) = param_packs;
+ PACK_EXPANSION_EXTRA_ARGS (expn) = extra_args;
PACK_EXPANSION_LOCAL_P (expn) = local;
res = expn;
}
--
Nathan Sidwell
Re: [PATCH] c++: stream PACK_EXPANSION_EXTRA_ARGS [PR106761]
On 9/20/22 10:08, Patrick Palka wrote:
On Tue, 20 Sep 2022, Nathan Sidwell wrote:
On 9/19/22 09:52, Patrick Palka wrote:
It looks like some xtreme-header-* tests are failing after the libstdc++
change r13-2158-g02f6b405f0e9dc ultimately because we're neglecting
to stream PACK_EXPANSION_EXTRA_ARGS, which leads to false equivalences
of different partial instantiations of _TupleConstraints::__constructible.
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
PR c++/106761
gcc/cp/ChangeLog:
* module.cc (trees_out::type_node) :
Stream PACK_EXPANSION_EXTRA_ARGS.
(trees_in::tree_node) : Likewise.
Looks good, I wonder why I missed that. (I guess extracting a testcase out of
the headers was too tricky?)
Many thanks. I managed to produce a small testcase which mirrors the
format of the xtreme-header-2* testcase. Does the following look OK?
yup, thanks for extracting that!
nathan
-- >8 --
PR c++/106761
gcc/cp/ChangeLog:
* module.cc (trees_out::type_node) :
Stream PACK_EXPANSION_EXTRA_ARGS.
(trees_in::tree_node) : Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr106761.h: New test.
* g++.dg/modules/pr106761_a.H: New test.
* g++.dg/modules/pr106761_b.C: New test.
---
gcc/cp/module.cc | 3 +++
gcc/testsuite/g++.dg/modules/pr106761.h | 22 ++
gcc/testsuite/g++.dg/modules/pr106761_a.H | 5 +
gcc/testsuite/g++.dg/modules/pr106761_b.C | 7 +++
4 files changed, 37 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/pr106761.h
create mode 100644 gcc/testsuite/g++.dg/modules/pr106761_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/pr106761_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 1a1ff5be574..9a9ef4e3332 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -8922,6 +8922,7 @@ trees_out::type_node (tree type)
if (streaming_p ())
u (PACK_EXPANSION_LOCAL_P (type));
tree_node (PACK_EXPANSION_PARAMETER_PACKS (type));
+ tree_node (PACK_EXPANSION_EXTRA_ARGS (type));
break;
case TYPENAME_TYPE:
@@ -9455,12 +9456,14 @@ trees_in::tree_node (bool is_use)
{
bool local = u ();
tree param_packs = tree_node ();
+ tree extra_args = tree_node ();
if (!get_overrun ())
{
tree expn = cxx_make_type (TYPE_PACK_EXPANSION);
SET_TYPE_STRUCTURAL_EQUALITY (expn);
PACK_EXPANSION_PATTERN (expn) = res;
PACK_EXPANSION_PARAMETER_PACKS (expn) = param_packs;
+ PACK_EXPANSION_EXTRA_ARGS (expn) = extra_args;
PACK_EXPANSION_LOCAL_P (expn) = local;
res = expn;
}
diff --git a/gcc/testsuite/g++.dg/modules/pr106761.h
b/gcc/testsuite/g++.dg/modules/pr106761.h
new file mode 100644
index 000..9f22a22a45d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr106761.h
@@ -0,0 +1,22 @@
+// PR c++/106761
+
+template
+struct __and_;
+
+template
+struct is_convertible;
+
+template
+struct _TupleConstraints {
+ template
+ using __constructible = __and_...>;
+};
+
+template
+struct tuple {
+ template
+ using __constructible
+= typename _TupleConstraints::template __constructible;
+};
+
+tuple t;
diff --git a/gcc/testsuite/g++.dg/modules/pr106761_a.H
b/gcc/testsuite/g++.dg/modules/pr106761_a.H
new file mode 100644
index 000..8ad116412af
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr106761_a.H
@@ -0,0 +1,5 @@
+// PR c++/106761
+// { dg-additional-options -fmodule-header }
+
+// { dg-module-cmi {} }
+#include "pr106761.h"
diff --git a/gcc/testsuite/g++.dg/modules/pr106761_b.C
b/gcc/testsuite/g++.dg/modules/pr106761_b.C
new file mode 100644
index 000..336cb12757e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr106761_b.C
@@ -0,0 +1,7 @@
+// PR c++/106761
+// { dg-additional-options -fmodules-ts }
+
+#include "pr106761.h"
+import "pr106761_a.H";
+
+tuple u = t;
--
Nathan Sidwell
Re: [PATCH 1/2] c++: modules and non-dependent auto deduction
On 9/20/22 15:54, Patrick Palka wrote:
The modules streaming code seems to rely on the invariant that a
TEMPLATE_DECL and its DECL_TEMPLATE_RESULT have the same TREE_TYPE.
It does indeed.
But for a templated VAR_DECL with deduced non-dependent type, the two
TREE_TYPEs end up diverging: cp_finish_decl deduces the type of the
initializer ahead of time and updates the TREE_TYPE of the VAR_DECL, but
neglects to update the corresponding TEMPLATE_DECL as well, which leads
to a "conflicting global module declaration" error for each of the
__phase_alignment decls in the below testcase (and for the xtreme-header
testcases if we try including ).
This patch makes cp_finish_decl update the TREE_TYPE of the corresponding
TEMPLATE_DECL so that the invariant is maintained >
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
Ok, thanks
gcc/cp/ChangeLog:
* decl.cc (cp_finish_decl): After updating the deduced type of a
VAR_DECL, also update the corresponding TEMPLATE_DECL if there
is one.
gcc/testsuite/ChangeLog:
* g++.dg/modules/auto-3.h: New test.
* g++.dg/modules/auto-3_a.H: New test.
* g++.dg/modules/auto-3_b.C: New test.
---
gcc/cp/decl.cc | 6 ++
gcc/testsuite/g++.dg/modules/auto-3.h | 10 ++
gcc/testsuite/g++.dg/modules/auto-3_a.H | 4
gcc/testsuite/g++.dg/modules/auto-3_b.C | 4
4 files changed, 24 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/auto-3.h
create mode 100644 gcc/testsuite/g++.dg/modules/auto-3_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/auto-3_b.C
diff --git a/gcc/cp/decl.cc b/gcc/cp/decl.cc
index 070f673c3a2..80467c19254 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -8180,6 +8180,12 @@ cp_finish_decl (tree decl, tree init, bool
init_const_expr_p,
return;
}
cp_apply_type_quals_to_decl (cp_type_quals (type), decl);
+
+ /* Update the type of the corresponding TEMPLATE_DECL to match. */
+ if (DECL_LANG_SPECIFIC (decl)
+ && DECL_TEMPLATE_INFO (decl)
+ && DECL_TEMPLATE_RESULT (DECL_TI_TEMPLATE (decl)) == decl)
+ TREE_TYPE (DECL_TI_TEMPLATE (decl)) = type;
}
if (ensure_literal_type_for_constexpr_object (decl) == error_mark_node)
diff --git a/gcc/testsuite/g++.dg/modules/auto-3.h
b/gcc/testsuite/g++.dg/modules/auto-3.h
new file mode 100644
index 000..f129433cbcb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-3.h
@@ -0,0 +1,10 @@
+template
+struct __tree_barrier {
+ static const auto __phase_alignment_1 = 0;
+
+ template
+ static const auto __phase_alignment_2 = 0;
+};
+
+template
+inline auto __phase_alignment_3 = 0;
diff --git a/gcc/testsuite/g++.dg/modules/auto-3_a.H
b/gcc/testsuite/g++.dg/modules/auto-3_a.H
new file mode 100644
index 000..25a7a73e73e
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-3_a.H
@@ -0,0 +1,4 @@
+// { dg-additional-options "-fmodule-header" }
+// { dg-module-cmi {} }
+
+#include "auto-3.h"
diff --git a/gcc/testsuite/g++.dg/modules/auto-3_b.C
b/gcc/testsuite/g++.dg/modules/auto-3_b.C
new file mode 100644
index 000..03b6d46f476
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/auto-3_b.C
@@ -0,0 +1,4 @@
+// { dg-additional-options "-fmodules-ts -fno-module-lazy" }
+
+#include "auto-3.h"
+import "auto-3_a.H";
--
Nathan Sidwell
Re: [PATCH 2/2] c++: xtreme-header modules tests cleanups
On 9/20/22 15:54, Patrick Palka wrote: This adds some recently implemented C++20/23 library headers to the xtreme-header tests as appropriate. Also, it looks like we can safely re-add and remove the NO_ASSOCIATED_LAMBDA workaround. Tested on x86_64-pc-linux-gnu, does this look OK for trunk? cool, more bits working. thanks! gcc/testsuite/ChangeLog: * g++.dg/modules/xtreme-header-2.h: Include . * g++.dg/modules/xtreme-header-6.h: Include , , , and . * g++.dg/modules/xtreme-header.h: Likewise. Remove NO_ASSOCIATED_LAMBDA workaround. Include implemented C++23 library headers. --- .../g++.dg/modules/xtreme-header-2.h | 3 +- .../g++.dg/modules/xtreme-header-6.h | 10 ++-- gcc/testsuite/g++.dg/modules/xtreme-header.h | 60 +++ 3 files changed, 29 insertions(+), 44 deletions(-) diff --git a/gcc/testsuite/g++.dg/modules/xtreme-header-2.h b/gcc/testsuite/g++.dg/modules/xtreme-header-2.h index ded093e533c..dfe94aa6988 100644 --- a/gcc/testsuite/g++.dg/modules/xtreme-header-2.h +++ b/gcc/testsuite/g++.dg/modules/xtreme-header-2.h @@ -1,8 +1,7 @@ // Everything that transitively includes #include -// FIXME: PR 97549 -// #include +#include #include #include #include diff --git a/gcc/testsuite/g++.dg/modules/xtreme-header-6.h b/gcc/testsuite/g++.dg/modules/xtreme-header-6.h index 85894b2b20a..8d024b69bac 100644 --- a/gcc/testsuite/g++.dg/modules/xtreme-header-6.h +++ b/gcc/testsuite/g++.dg/modules/xtreme-header-6.h @@ -1,22 +1,22 @@ // C++20 headers #if __cplusplus > 201703 #include +#include #include #include #include #if __cpp_coroutines #include #endif +#include #include +#include +#include #include #include +#include #if 0 // Unimplemented -#include #include -#include -#include -#include -#include #endif #endif diff --git a/gcc/testsuite/g++.dg/modules/xtreme-header.h b/gcc/testsuite/g++.dg/modules/xtreme-header.h index 41302c780b5..124e2f82277 100644 --- a/gcc/testsuite/g++.dg/modules/xtreme-header.h +++ b/gcc/testsuite/g++.dg/modules/xtreme-header.h @@ -1,17 +1,8 @@ // All the headers! -#if __cplusplus > 201703L -// FIXME: if we include everything, something goes wrong with location -// information. We used to not handle lambdas attached to global -// vars, and this is a convienient flag to stop including everything. -#define NO_ASSOCIATED_LAMBDA 1 -#endif - // C++ 17 and below #if 1 -#if !NO_ASSOCIATED_LAMBDA #include -#endif #include #include #include @@ -26,19 +17,12 @@ #include #include #include -#if !NO_ASSOCIATED_LAMBDA -// FIXME: PR 97549 -//#include -#endif +#include #include #include #include -#if !NO_ASSOCIATED_LAMBDA #include -#endif -#if !NO_ASSOCIATED_LAMBDA #include -#endif #include #include #include @@ -49,12 +33,8 @@ #include #include #include -#if !NO_ASSOCIATED_LAMBDA #include -#endif -#if !NO_ASSOCIATED_LAMBDA #include -#endif #include #include #include @@ -63,12 +43,8 @@ #include #include #include -#if !NO_ASSOCIATED_LAMBDA #include -#endif -#if !NO_ASSOCIATED_LAMBDA #include -#endif #include #include #include @@ -78,9 +54,7 @@ #include #include #include -#if !NO_ASSOCIATED_LAMBDA #include -#endif #include #include #include @@ -88,9 +62,7 @@ #include #include #include -#if !NO_ASSOCIATED_LAMBDA #include -#endif #include #include #endif @@ -119,26 +91,40 @@ #if __cplusplus > 201703 #if 1 #include +#include #include #include #include #if __cpp_coroutines #include #endif -#if !NO_ASSOCIATED_LAMBDA -#include -#endif +#include #include +#include +#include +#include #include #include +#include #if 0 // Unimplemented -#include #include -#include -#include -#include -#include #endif #endif #endif + +// C++23 +#if __cplusplus > 202002L +#include +#include +#include +#if 0 +// Unimplemented +#include +#include +#include +#include +#include +#endif +#endif + -- Nathan Sidwell
Re: [PATCH] c++ modules: partial variable template specializations [PR106826]
On 9/21/22 12:16, Patrick Palka wrote:
With partial variable template specializations, it looks like we
stream the VAR_DECL (i.e. the DECL_TEMPLATE_RESULT of the corresponding
TEMPLATE_DECL) since process_partial_specialization adds it to the
specializations table, but end up never streaming the corresponding
TEMPLATE_DECL itself that appears only in the primary template's
DECL_TEMPLATE_SPECIALIZATIONS list, which leads to the list being
incomplete on stream-in.
The modules machinery already has special logic for streaming partial
specializations of class templates; this patch generalizes it to handle
those of variable templates as well.
looks good, I didn't realize template vars had partial specializations.
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
PR c++/106826
gcc/cp/ChangeLog:
* module.cc (trees_out::decl_value): Use get_template_info in
the MK_partial case.
(trees_out::key_mergeable): Likewise.
(trees_in::key_mergeable): Likewise.
(has_definition): Consider DECL_INITIAL of a partial variable
template specialization.
(depset::hash::make_dependency): Introduce a dependency of
partial variable template specializations too.
gcc/testsuite/ChangeLog:
* g++.dg/modules/partial-2_a.C: New test.
* g++.dg/modules/partial-2_b.C: New test.
---
gcc/cp/module.cc | 32 +---
gcc/testsuite/g++.dg/modules/partial-2_a.C | 43 ++
gcc/testsuite/g++.dg/modules/partial-2_b.C | 21 +++
3 files changed, 82 insertions(+), 14 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 9a9ef4e3332..334bde99b0f 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -7789,8 +7789,9 @@ trees_out::decl_value (tree decl, depset *dep)
}
else
{
- tree_node (CLASSTYPE_TI_TEMPLATE (TREE_TYPE (inner)));
- tree_node (CLASSTYPE_TI_ARGS (TREE_TYPE (inner)));
+ tree ti = get_template_info (inner);
+ tree_node (TI_TEMPLATE (ti));
+ tree_node (TI_ARGS (ti));
}
}
tree_node (get_constraints (decl));
@@ -10626,8 +10627,9 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree
decl, tree inner,
case MK_partial:
{
key.constraints = get_constraints (inner);
- key.ret = CLASSTYPE_TI_TEMPLATE (TREE_TYPE (inner));
- key.args = CLASSTYPE_TI_ARGS (TREE_TYPE (inner));
+ tree ti = get_template_info (inner);
+ key.ret = TI_TEMPLATE (ti);
+ key.args = TI_ARGS (ti);
}
break;
}
@@ -10866,8 +10868,8 @@ trees_in::key_mergeable (int tag, merge_kind mk, tree
decl, tree inner,
spec; spec = TREE_CHAIN (spec))
{
tree tmpl = TREE_VALUE (spec);
- if (template_args_equal (key.args,
- CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl)))
+ tree ti = get_template_info (tmpl);
+ if (template_args_equal (key.args, TI_ARGS (ti))
&& cp_tree_equal (key.constraints,
get_constraints
(DECL_TEMPLATE_RESULT (tmpl
@@ -11381,8 +11383,7 @@ has_definition (tree decl)
case VAR_DECL:
if (DECL_LANG_SPECIFIC (decl)
- && DECL_TEMPLATE_INFO (decl)
- && DECL_USE_TEMPLATE (decl) < 2)
+ && DECL_TEMPLATE_INFO (decl))
return DECL_INITIAL (decl);
else
{
@@ -12498,11 +12499,14 @@ depset::hash::make_dependency (tree decl, entity_kind
ek)
if (!dep)
{
- if (DECL_IMPLICIT_TYPEDEF_P (decl)
- /* ... not an enum, for instance. */
- && RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl))
- && TYPE_LANG_SPECIFIC (TREE_TYPE (decl))
- && CLASSTYPE_USE_TEMPLATE (TREE_TYPE (decl)) == 2)
+ if ((DECL_IMPLICIT_TYPEDEF_P (decl)
+ /* ... not an enum, for instance. */
+ && RECORD_OR_UNION_TYPE_P (TREE_TYPE (decl))
+ && TYPE_LANG_SPECIFIC (TREE_TYPE (decl))
+ && CLASSTYPE_USE_TEMPLATE (TREE_TYPE (decl)) == 2)
+ || (VAR_P (decl)
+ && DECL_LANG_SPECIFIC (decl)
+ && DECL_USE_TEMPLATE (decl) == 2))
{
/* A partial or explicit specialization. Partial
specializations might not be in the hash table, because
@@ -12515,7 +12519,7 @@ depset::hash::make_dependency (tree decl, entity_kind
ek)
dep_hash, and then convert the dep we just found into a
redirect. */
- tree ti = TYPE_TEMPLATE_INFO (TREE_TYPE (decl));
+ tree ti = get_template_info (decl);
tree tmpl = TI_TEMPLATE (ti);
tree partial = NULL_TREE;
for (tree s
Re: [PATCH] c++ modules: ICE with class NTTP argument [PR100616]
On 9/22/22 14:25, Patrick Palka wrote:
index 80467c19254..722b64793ed 100644
--- a/gcc/cp/decl.cc
+++ b/gcc/cp/decl.cc
@@ -18235,9 +18235,11 @@ maybe_register_incomplete_var (tree var)
{
/* When the outermost open class is complete we can resolve any
pointers-to-members. */
- tree context = outermost_open_class ();
- incomplete_var iv = {var, context};
- vec_safe_push (incomplete_vars, iv);
+ if (tree context = outermost_open_class ())
+ {
+ incomplete_var iv = {var, context};
+ vec_safe_push (incomplete_vars, iv);
+ }
My immediate thought here is eek! during stream in, the
outermost_open_class could be anything -- to do with the context that
wanted to lookup of the thing being streamed in, right? So, the above
change is I think just papering over a problem in this case.
not sure how to approach this.
nathan
--
Nathan Sidwell
Re: [PATCH] c++ modules: ICE with class NTTP argument [PR100616]
On 9/23/22 09:32, Patrick Palka wrote:
Judging by the two commits that introduced/modified this part of
maybe_register_incomplete_var, r196852 and r214333, ISTM the code
is really only concerned with constexpr static data members (whose
initializer may contain a pointer-to-member for a currently open class).
So maybe we ought to restrict the branch like so, which effectively
disables this part of maybe_register_incomplete_var during stream-in, and
guarantees that outermost_open_class doesn't return NULL if the branch is
taken?
I think the problem is that we're streaming these VAR_DECLs as regular
VAR_DECLS, when we should be handling them as a new kind of object
fished out from the template they're instantiating. (I'm guessing
that'll just be a new tag, a type and an initializer?)
Then on stream-in we can handle them in the same way as a non-modules
compilation handles such redeclarations. I.e. how does:
template struct C { };
struct A { };
C c1; // #1
C c2; // #2
work. Presumably at some point #2's A{} gets unified such that we find
the instantation that occurred at #1?
I notice the template arg for C is a var decl mangled as
_ZTAXtl1AEE, which is a 'template paramete object for A{}'. I see
that's a special mangler 'mangle_template_parm_object', called from
get_template_parm_object. Perhaps these VAR_DECLs need an additional
in-tree flag that the streamer can check for?
nathan
--
Nathan Sidwell
Re: [PATCH] c++ modules: ICE with class NTTP argument [PR100616]
On 9/26/22 10:08, Nathan Sidwell wrote:
On 9/23/22 09:32, Patrick Palka wrote:
Judging by the two commits that introduced/modified this part of
maybe_register_incomplete_var, r196852 and r214333, ISTM the code
is really only concerned with constexpr static data members (whose
initializer may contain a pointer-to-member for a currently open class).
So maybe we ought to restrict the branch like so, which effectively
disables this part of maybe_register_incomplete_var during stream-in, and
guarantees that outermost_open_class doesn't return NULL if the branch is
taken?
I think the problem is that we're streaming these VAR_DECLs as regular
VAR_DECLS, when we should be handling them as a new kind of object
fished out from the template they're instantiating. (I'm guessing
that'll just be a new tag, a type and an initializer?)
Then on stream-in we can handle them in the same way as a non-modules
compilation handles such redeclarations. I.e. how does:
template struct C { };
struct A { };
C c1; // #1
C c2; // #2
work. Presumably at some point #2's A{} gets unified such that we find
the instantation that occurred at #1?
I notice the template arg for C is a var decl mangled as
_ZTAXtl1AEE, which is a 'template paramete object for A{}'. I see
that's a special mangler 'mangle_template_parm_object', called from
get_template_parm_object. Perhaps these VAR_DECLs need an additional
in-tree flag that the streamer can check for?
I wonder if we're setting the module attachment for these variables
sanely? They should be attached to the global module. My guess is the
pushdecl_top_level_and_finish call in get_templatE_parm_object is not
doing what is needed (as well as the other issues).
--
Nathan Sidwell
Re: [PATCH] c++ modules: variable template partial spec fixes [PR107033]
On 9/26/22 10:36, Patrick Palka wrote:
In r13-2775-g32d8123cd6ce87 I overlooked that we need to adjust the
call to add_mergeable_specialization in the MK_partial case to correctly
handle variable template partial specializations (it currently assumes
we're always dealing with one for a class template). This fixes an ICE
when converting the testcase from that commit to use an importable
header instead of a named module.
looks good, thanks
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
PR c++/107033
gcc/cp/ChangeLog:
* module.cc (trees_in::decl_value): In the MK_partial case
for a variable template partial specialization, pass decl_p=true
to add_mergeable_specialization and set spec to the VAR_DECL
not the TEMPLATE_DECL.
* pt.cc (add_mergeable_specialization): For a variable template
partial specialization, set the TREE_TYPE of the new
DECL_TEMPLATE_SPECIALIZATIONS node to the TREE_TYPE of the
VAR_DECL not the VAR_DECL itself.
gcc/testsuite/ChangeLog:
* g++.dg/modules/partial-2.cc, g++.dg/modules/partial-2.h: New
files, factored out from ...
* g++.dg/modules/partial-2_a.C, g++.dg/modules/partial-2_b.C: ...
here.
* g++.dg/modules/partial-2_c.H: New test.
* g++.dg/modules/partial-2_d.C: New test.
---
gcc/cp/module.cc | 17 ++
gcc/cp/pt.cc | 2 +-
gcc/testsuite/g++.dg/modules/partial-2.cc | 17 ++
gcc/testsuite/g++.dg/modules/partial-2.h | 38 +
gcc/testsuite/g++.dg/modules/partial-2_a.C | 39 +-
gcc/testsuite/g++.dg/modules/partial-2_b.C | 18 +-
gcc/testsuite/g++.dg/modules/partial-2_c.H | 5 +++
gcc/testsuite/g++.dg/modules/partial-2_d.C | 8 +
8 files changed, 82 insertions(+), 62 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/partial-2.cc
create mode 100644 gcc/testsuite/g++.dg/modules/partial-2.h
create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_c.H
create mode 100644 gcc/testsuite/g++.dg/modules/partial-2_d.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index f23832cb56a..7496df5e843 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -8185,13 +8185,18 @@ trees_in::decl_value ()
/* Set the TEMPLATE_DECL's type. */
TREE_TYPE (decl) = TREE_TYPE (inner);
- if (mk & MK_template_mask
- || mk == MK_partial)
+ /* Add to specialization tables now that constraints etc are
+added. */
+ if (mk == MK_partial)
{
- /* Add to specialization tables now that constraints etc are
-added. */
- bool is_type = mk == MK_partial || !(mk & MK_tmpl_decl_mask);
-
+ bool is_type = TREE_CODE (inner) == TYPE_DECL;
+ spec.spec = is_type ? type : inner;
+ add_mergeable_specialization (!is_type, false,
+ &spec, decl, spec_flags);
+ }
+ else if (mk & MK_template_mask)
+ {
+ bool is_type = !(mk & MK_tmpl_decl_mask);
spec.spec = is_type ? type : mk & MK_tmpl_tmpl_mask ? inner : decl;
add_mergeable_specialization (!is_type,
!is_type && mk & MK_tmpl_alias_mask,
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index db4e808adec..1f088fe281e 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -31010,7 +31010,7 @@ add_mergeable_specialization (bool decl_p, bool
alias_p, spec_entry *elt,
/* A partial specialization. */
tree cons = tree_cons (elt->args, decl,
DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl));
- TREE_TYPE (cons) = elt->spec;
+ TREE_TYPE (cons) = decl_p ? TREE_TYPE (elt->spec) : elt->spec;
DECL_TEMPLATE_SPECIALIZATIONS (elt->tmpl) = cons;
}
}
diff --git a/gcc/testsuite/g++.dg/modules/partial-2.cc
b/gcc/testsuite/g++.dg/modules/partial-2.cc
new file mode 100644
index 000..1316bf5e1c5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/partial-2.cc
@@ -0,0 +1,17 @@
+static_assert(is_reference_v);
+static_assert(is_reference_v);
+static_assert(!is_reference_v);
+
+static_assert(A::is_reference_v);
+static_assert(A::is_reference_v);
+static_assert(!A::is_reference_v);
+
+#if __cpp_concepts
+static_assert(concepts::is_reference_v);
+static_assert(concepts::is_reference_v);
+static_assert(!concepts::is_reference_v);
+
+static_assert(concepts::A::is_reference_v);
+static_assert(concepts::A::is_reference_v);
+static_assert(!concepts::A::is_reference_v);
+#endif
diff --git a/gcc/testsuite/g++.dg/modules/partial-2.h
b/gcc/testsuite/g++.dg/modules/partial-2.h
new file mode 100644
index 000..afcfce791b3
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/partial-2.h
@@ -0,0 +1,38 @@
+template constexpr bool is_reference_v = false;
+template constexpr bool is_reference_v = true;
+template constexpr bool is_reference_v = true;
+
Re: [PATCH] c++ modules: ICE with class NTTP argument [PR100616]
On 9/26/22 15:05, Patrick Palka wrote:
On Mon, 26 Sep 2022, Patrick Palka wrote:
On Mon, 26 Sep 2022, Nathan Sidwell wrote:
On 9/26/22 10:08, Nathan Sidwell wrote:
On 9/23/22 09:32, Patrick Palka wrote:
Judging by the two commits that introduced/modified this part of
maybe_register_incomplete_var, r196852 and r214333, ISTM the code
is really only concerned with constexpr static data members (whose
initializer may contain a pointer-to-member for a currently open class).
So maybe we ought to restrict the branch like so, which effectively
disables this part of maybe_register_incomplete_var during stream-in, and
guarantees that outermost_open_class doesn't return NULL if the branch is
taken?
I think the problem is that we're streaming these VAR_DECLs as regular
VAR_DECLS, when we should be handling them as a new kind of object fished
out from the template they're instantiating. (I'm guessing that'll just be a
new tag, a type and an initializer?)
Then on stream-in we can handle them in the same way as a non-modules
compilation handles such redeclarations. I.e. how does:
template struct C { };
struct A { };
C c1; // #1
C c2; // #2
work. Presumably at some point #2's A{} gets unified such that we find the
instantation that occurred at #1?
This works because the lookup in get_template_parm_object for #2's A{}
finds and reuses the VAR_DECL created for #1's A{}.
But IIUC this lookup (performed via get_global_binding) isn't
import-aware, which I suppose explains why we don't find the VAR_DECL
from another TU.
I notice the template arg for C is a var decl mangled as _ZTAXtl1AEE,
which is a 'template paramete object for A{}'. I see that's a special
mangler 'mangle_template_parm_object', called from
get_template_parm_object. Perhaps these VAR_DECLs need an additional
in-tree flag that the streamer can check for?
I wonder if we're setting the module attachment for these variables sanely?
They should be attached to the global module. My guess is the
pushdecl_top_level_and_finish call in get_templatE_parm_object is not doing
what is needed (as well as the other issues).
This is a bit of a shot in the dark, but the following seems to work:
when pushing the VAR_DECL, we need to call set_originating_module to
attach it to the global module, and when looking it up, we need to do so
in an import-aware way. Hopefully something like this is sufficient
to properly handle these VAR_DECLs and we don't need to stream them
specially?
Err, rather than changing the behavior of get_namespace_binding (which
has many unrelated callers), I guess we could just use the already
import-aware lookup_qualified_name instead where appropriate. WDYT of
the following? (testing in progress)
I'm going to have to think further. Morally these VAR_DECLs are like
the typeinfo objects -- which we have to handle specially. Reminding
myself, I see rtti.cc does the pushdecl_top_level stuff creating them --
so they go into the slot for the current TU. But the streaming code
writes tt_tinfo_var/tt_tinfo_typedef records, and recreates the typeinfo
on stream in, using the same above pushdec_top_level path. So even
though the tinfo decls might seem attached to the current module, that
doesn;t confuse the streaming, nor create collisions on read back. Nor
do we stream out tinfo decls that are not reachable through the streamed
AST (if we treated then as normal decls, we'd stream them cos they're
inthe current TU in the symbol table. I have the same fear for these
NTTPs.)
It looks like TREE_LANG_FLAG_5 can be used to note these VAR_DECLs are
NTTPs, and then the streaming can deal with them. Let me look further.
@@ -7307,6 +7307,7 @@ get_template_parm_object (tree expr, tsubst_flags_t
complain)
hash_map_safe_put (tparm_obj_values, decl, copy);
}
+ set_originating_module (decl);
pushdecl_top_level_and_finish (decl, expr);
this is wrong. You're attaching the decl to the current module. which
will mean conflicts when reading in such VAR_DECLs for the same NTTP
from different modules. Your test case might be hiding that as you have
an interface and implementation unit from the same module (but you
should be getting some kind of multiple definition error anyway?)
return decl;
@@ -29150,9 +29151,10 @@ finish_concept_definition (cp_expr id, tree init)
static tree
listify (tree arg)
{
- tree std_init_list = get_namespace_binding (std_node, init_list_identifier);
+ tree std_init_list = lookup_qualified_name (std_node, init_list_identifier);
- if (!std_init_list || !DECL_CLASS_TEMPLATE_P (std_init_list))
+ if (std_init_list == error_mark_node
+ || !DECL_CLASS_TEMPLATE_P (std_init_list))
{
gcc_rich_location richloc (input_location);
maybe_add_include_fixit (&richloc, "", false);
diff --git a/gcc/testsuite/g++.dg/modules/pr100616_a.C
b/gcc/testsuite/g++.dg/modules/pr100616_a.C
new file mode 100644
index 000..788af2eb533
--- /dev/nu
c++: Add DECL_NTTP_OBJECT_P lang flag
VAR_DECLs for NTTPs need to be handled specially by module streaming,
in the same manner to type info decls. This reworks their handling to
allow that work to drop in. We use DECL_LANG_FLAG_5 to indicate such
decls (I didn't notice template_parm_object_p, which looks at the
mangled name -- anyway a bit flag on the node is better, IMHO). We
break apart the creation routine, so there's now an entry point the
module machinery can use directly.
nathan
--
Nathan SidwellFrom 50888e70c984da9cd9676d3986f68222581884b3 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 28 Sep 2022 09:20:27 -0700
Subject: [PATCH] c++: Add DECL_NTTP_OBJECT_P lang flag
VAR_DECLs for NTTPs need to be handled specially by module streaming,
in the same manner to type info decls. This reworks their handling to
allow that work to drop in. We use DECL_LANG_FLAG_5 to indicate such
decls (I didn't notice template_parm_object_p, which looks at the
mangled name -- anyway a bit flag on the node is better, IMHO). We
break apart the creation routine, so there's now an entry point the
module machinery can use directly.
gcc/cp/
* cp-tree.h (DECL_NTTP_OBJECT_P): New.
(template_parm_object_p): Delete.
(build_template_parm_object): Declare.
* cxx-pretty-print.cc (pp_cx_template_argument_list): Use DECL_NTTP_OBJECT_P.
* error.cc (dump_simple_decl): Likewise.
* mangle.cc (write_template_arg): Likewise.
* pt.cc (template_parm_object_p): Delete.
(create_template_parm_object): Separated out checking from ...
(get_template_parm_object): ... this, new external entry point.
---
gcc/cp/cp-tree.h | 7 ++-
gcc/cp/cxx-pretty-print.cc | 2 +-
gcc/cp/error.cc| 2 +-
gcc/cp/mangle.cc | 2 +-
gcc/cp/pt.cc | 35 +--
5 files changed, 26 insertions(+), 22 deletions(-)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 19bbfbc557f..d0f1b18b015 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -518,6 +518,7 @@ extern GTY(()) tree cp_global_trees[CPTI_MAX];
CALL_EXPR_REVERSE_ARGS (in CALL_EXPR, AGGR_INIT_EXPR)
CONSTRUCTOR_PLACEHOLDER_BOUNDARY (in CONSTRUCTOR)
OVL_EXPORT_P (in OVERLOAD)
+ DECL_NTTP_OBJECT_P (in VAR_DECL)
6: TYPE_MARKED_P (in _TYPE)
DECL_NONTRIVIALLY_INITIALIZED_P (in VAR_DECL)
RANGE_FOR_IVDEP (in RANGE_FOR_STMT)
@@ -3548,6 +3549,10 @@ struct GTY(()) lang_decl {
#define DECL_TINFO_P(NODE) \
TREE_LANG_FLAG_4 (TREE_CHECK2 (NODE,VAR_DECL,TYPE_DECL))
+/* true iff VAR_DECL node NODE is a NTTP object decl. */
+#define DECL_NTTP_OBJECT_P(NODE) \
+ TREE_LANG_FLAG_5 (TREE_CHECK (NODE,VAR_DECL))
+
/* 1 iff VAR_DECL node NODE is virtual table or VTT. We forward to
DECL_VIRTUAL_P from the common code, as that has the semantics we
need. But we want a more descriptive name. */
@@ -7414,7 +7419,7 @@ extern bool alias_type_or_template_p(tree);
enum { nt_opaque = false, nt_transparent = true };
extern tree alias_template_specialization_p (const_tree, bool);
extern tree dependent_alias_template_spec_p (const_tree, bool);
-extern bool template_parm_object_p (const_tree);
+extern tree get_template_parm_object (tree expr, tree mangle);
extern tree tparm_object_argument (tree);
extern bool explicit_class_specialization_p (tree);
extern bool push_tinst_level(tree);
diff --git a/gcc/cp/cxx-pretty-print.cc b/gcc/cp/cxx-pretty-print.cc
index e18143e39a9..bbd51bb562a 100644
--- a/gcc/cp/cxx-pretty-print.cc
+++ b/gcc/cp/cxx-pretty-print.cc
@@ -1956,7 +1956,7 @@ pp_cxx_template_argument_list (cxx_pretty_printer *pp, tree t)
if (TYPE_P (arg) || (TREE_CODE (arg) == TEMPLATE_DECL
&& TYPE_P (DECL_TEMPLATE_RESULT (arg
pp->type_id (arg);
- else if (template_parm_object_p (arg))
+ else if (TREE_CODE (arg) == VAR_DECL && DECL_NTTP_OBJECT_P (arg))
pp->expression (DECL_INITIAL (arg));
else
pp->expression (arg);
diff --git a/gcc/cp/error.cc b/gcc/cp/error.cc
index 0389f35d731..53904e3669d 100644
--- a/gcc/cp/error.cc
+++ b/gcc/cp/error.cc
@@ -1129,7 +1129,7 @@ dump_global_iord (cxx_pretty_printer *pp, tree t)
static void
dump_simple_decl (cxx_pretty_printer *pp, tree t, tree type, int flags)
{
- if (template_parm_object_p (t))
+ if (TREE_CODE (t) == VAR_DECL && DECL_NTTP_OBJECT_P (t))
return dump_expr (pp, DECL_INITIAL (t), flags);
if (flags & TFF_DECL_SPECIFIERS)
diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index 00d283fff8c..1a455858827 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -3672,7 +3672,7 @@ write_template_arg (tree node)
}
}
- if (template_parm_object_p (node))
+ if (TREE_CODE (node) == VAR_DECL && DECL_NTTP_OBJECT_P (node))
/* We want to mangle the argument, not the var we stored it in. */
node = tparm_object_argument (node);
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 2d83dfd6954..c7adaef997d 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -711
Re: [PATCH] c++ modules: ICE with class NTTP argument [PR100616]
On 9/28/22 10:42, Patrick Palka wrote:
On Tue, 27 Sep 2022, Nathan Sidwell wrote:
On 9/26/22 15:05, Patrick Palka wrote:
On Mon, 26 Sep 2022, Patrick Palka wrote:
On Mon, 26 Sep 2022, Nathan Sidwell wrote:
return decl;
@@ -29150,9 +29151,10 @@ finish_concept_definition (cp_expr id, tree init)
static tree
listify (tree arg)
{
- tree std_init_list = get_namespace_binding (std_node,
init_list_identifier);
+ tree std_init_list = lookup_qualified_name (std_node,
init_list_identifier);
- if (!std_init_list || !DECL_CLASS_TEMPLATE_P (std_init_list))
+ if (std_init_list == error_mark_node
+ || !DECL_CLASS_TEMPLATE_P (std_init_list))
{
gcc_rich_location richloc (input_location);
maybe_add_include_fixit (&richloc, "", false);
What do you think about this independent change to use
lookup_qualified_name instead of get_namespace_binding in listify so
that the lookup for std::initializer_list is import-aware, which seems
to fix PR102576?
Yes, that looks right to me, thanks! (I think it'll also fix a
potential future problem if we ever have:
namespace std {
inline namespace v2 {
template class initializer_list {...};
}}
diff --git a/gcc/testsuite/g++.dg/modules/pr100616_a.C
b/gcc/testsuite/g++.dg/modules/pr100616_a.C
new file mode 100644
index 000..788af2eb533
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr100616_a.C
@@ -0,0 +1,8 @@
+// PR c++/100616
+// { dg-additional-options "-std=c++20 -fmodules-ts" }
+// { dg-module-cmi pr100616 }
+export module pr100616;
+
+template struct C { };
+struct A { };
+C c1;
diff --git a/gcc/testsuite/g++.dg/modules/pr100616_b.C
b/gcc/testsuite/g++.dg/modules/pr100616_b.C
new file mode 100644
index 000..fc89cd08ac5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr100616_b.C
@@ -0,0 +1,8 @@
+// PR c++/100616
+// { dg-additional-options "-std=c++20 -fmodules-ts" }
+module pr100616;
+
+C c2;
+
+using type = decltype(c1);
+using type = decltype(c2);
diff --git a/gcc/testsuite/g++.dg/modules/pr102576_a.H
b/gcc/testsuite/g++.dg/modules/pr102576_a.H
new file mode 100644
index 000..87ba9b52031
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr102576_a.H
@@ -0,0 +1,5 @@
+// PR c++/102576
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+#include
diff --git a/gcc/testsuite/g++.dg/modules/pr102576_b.C
b/gcc/testsuite/g++.dg/modules/pr102576_b.C
new file mode 100644
index 000..10251ed5304
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr102576_b.C
@@ -0,0 +1,9 @@
+// PR c++/102576
+// { dg-additional-options -fmodules-ts }
+
+import "pr102576_a.H";
+
+int main() {
+ for (int i : {1, 2, 3})
+;
+}
--
Nathan Sidwell
--
Nathan Sidwell
c++: import/export NTTP objects
This adds smarts to the module machinery to handle NTTP object
VAR_DECLs. Like typeinfo objects, these must be ignored in the symbol
table, streamed specially and recreated on stream in.
Patrick, thanks for the testcase, I don't know how to attribute that to
you in the changelog anymore.
nathan
--
Nathan SidwellFrom a1f7f9541c2b20eb44750b9c15cd831c62d67f21 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 28 Sep 2022 09:21:14 -0700
Subject: [PATCH] c++: import/export NTTP objects
This adds smarts to the module machinery to handle NTTP object
VAR_DECLs. Like typeinfo objects, these must be ignored in the symbol
table, streamed specially and recreated on stream in.
gcc/cp/
PR c++/100616
* module.cc (enum tree_tag): Add tt_nttp_var.
(trees_out::decl_node): Handle NTTP objects.
(trees_in::tree_node): Handle tt_nttp_var.
(depset::hash::add_binding_entry): Skip NTTP objects.
gcc/testsuite/
PR c++/100616
* g++.dg/modules/100616_a.H: New.
* g++.dg/modules/100616_b.C: New.
* g++.dg/modules/100616_c.C: New.
* g++.dg/modules/100616_d.C: New.
---
gcc/cp/module.cc| 35 +
gcc/testsuite/g++.dg/modules/100616_a.H | 5
gcc/testsuite/g++.dg/modules/100616_b.C | 7 +
gcc/testsuite/g++.dg/modules/100616_c.C | 7 +
gcc/testsuite/g++.dg/modules/100616_d.C | 10 +++
5 files changed, 64 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/100616_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/100616_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/100616_c.C
create mode 100644 gcc/testsuite/g++.dg/modules/100616_d.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index d965017940a..cbf3a77de01 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2737,6 +2737,7 @@ enum tree_tag {
tt_tinfo_var, /* Typeinfo object. */
tt_tinfo_typedef, /* Typeinfo typedef. */
tt_ptrmem_type, /* Pointer to member type. */
+ tt_nttp_var, /* NTTP_OBJECT VAR_DECL. */
tt_parm, /* Function parameter or result. */
tt_enum_value, /* An enum value. */
@@ -8548,6 +8549,21 @@ trees_out::decl_node (tree decl, walk_kind ref)
}
return false;
}
+
+ if (DECL_NTTP_OBJECT_P (decl))
+ {
+ /* A NTTP parm object. */
+ if (streaming_p ())
+ i (tt_nttp_var);
+ tree_node (tparm_object_argument (decl));
+ tree_node (DECL_NAME (decl));
+ int tag = insert (decl);
+ if (streaming_p ())
+ dump (dumper::TREE)
+ && dump ("Wrote nttp object:%d %N", tag, DECL_NAME (decl));
+ return false;
+ }
+
break;
case TYPE_DECL:
@@ -9627,6 +9643,21 @@ trees_in::tree_node (bool is_use)
}
break;
+case tt_nttp_var:
+ /* An NTTP object. */
+ {
+ tree init = tree_node ();
+ tree name = tree_node ();
+ if (!get_overrun ())
+ {
+ res = get_template_parm_object (init, name);
+ int tag = insert (res);
+ dump (dumper::TREE)
+ && dump ("Created nttp object:%d %N", tag, name);
+ }
+ }
+ break;
+
case tt_enum_value:
/* An enum const value. */
{
@@ -12760,6 +12791,10 @@ depset::hash::add_binding_entity (tree decl, WMB_Flags flags, void *data_)
/* Ignore TINFO things. */
return false;
+ if (TREE_CODE (decl) == VAR_DECL && DECL_NTTP_OBJECT_P (decl))
+ /* Ignore NTTP objects. */
+ return false;
+
if (!(flags & WMB_Using) && CP_DECL_CONTEXT (decl) != data->ns)
{
/* A using that lost its wrapper or an unscoped enum
diff --git a/gcc/testsuite/g++.dg/modules/100616_a.H b/gcc/testsuite/g++.dg/modules/100616_a.H
new file mode 100644
index 000..9bc42bcc05b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/100616_a.H
@@ -0,0 +1,5 @@
+// { dg-additional-options {-std=c++20 -fmodule-header} }
+// { dg-module-cmi {} }
+
+template struct C { };
+struct A { };
diff --git a/gcc/testsuite/g++.dg/modules/100616_b.C b/gcc/testsuite/g++.dg/modules/100616_b.C
new file mode 100644
index 000..416fd524b2c
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/100616_b.C
@@ -0,0 +1,7 @@
+// { dg-additional-options {-std=c++20 -fmodules-ts} }
+
+export module pr100616_b;
+// { dg-module-cmi pr100616_b }
+
+import "100616_a.H";
+export C c1;
diff --git a/gcc/testsuite/g++.dg/modules/100616_c.C b/gcc/testsuite/g++.dg/modules/100616_c.C
new file mode 100644
index 000..5c79f5eef68
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/100616_c.C
@@ -0,0 +1,7 @@
+// { dg-additional-options {-std=c++20 -fmodules-ts} }
+
+export module pr100616_c;
+// { dg-module-cmi pr100616_c }
+
+import "100616_a.H";
+export C c2;
diff --git a/gcc/testsuite/g++.dg/modules/100616_d.C b/gcc/testsuite/g++.dg/modules/100616_d.C
new file mode 100644
index 000..d9515db1140
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/100616_d.C
@@ -0,0 +1,10 @@
+// { dg-additional-options {-std=c++20 -fmodules-ts} }
+
+import "100616_a.H";
+import pr100616_b;
+import pr100616_c;
+
+C c0;
+using type = decltype(c0);
+using type = decltype(c
Re: [PATCH] testsuite: Colon is reserved on Windows
On 9/30/22 04:18, Torbjörn SVENSSON wrote:
The ':' is reserved in filenames on Windows.
Can't find any specification for this, but when there is no filename
defined in the map file, GCC will replace the ':' with a '-' in the
generated filename for the module.
Correct (and the specification is in the source code, there's no
requirement for any particular mapping, bu I was at least cognizant of
windows' dislike of : :)
Without this patch, the test case failes with:
.../ben-1_a.C:4:8: error: failed to write compiled module: Invalid argument
.../ben-1_a.C:4:8: note: compiled module file is 'partitions/module:import.mod'
gcc/testsuite:
* g++.dg/modules/ben-1.map: Replace the colon with dash.
* g++.dg/modules/ben-1_a.C: Likewise
ok, thanks
Co-Authored-By: Yvan ROUX
Signed-off-by: Torbjörn SVENSSON
---
gcc/testsuite/g++.dg/modules/ben-1.map | 2 +-
gcc/testsuite/g++.dg/modules/ben-1_a.C | 2 +-
2 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/g++.dg/modules/ben-1.map
b/gcc/testsuite/g++.dg/modules/ben-1.map
index 182183ad089..ad84c11397d 100644
--- a/gcc/testsuite/g++.dg/modules/ben-1.map
+++ b/gcc/testsuite/g++.dg/modules/ben-1.map
@@ -1,3 +1,3 @@
$root .
-module:import partitions/module:import.mod
+module:import partitions/module-import.mod
module module.mod
diff --git a/gcc/testsuite/g++.dg/modules/ben-1_a.C
b/gcc/testsuite/g++.dg/modules/ben-1_a.C
index 7e9b5661026..f1562eb2c5a 100644
--- a/gcc/testsuite/g++.dg/modules/ben-1_a.C
+++ b/gcc/testsuite/g++.dg/modules/ben-1_a.C
@@ -2,7 +2,7 @@
// { dg-additional-files ben-1.map }
export module module:import;
-// { dg-module-cmi =partitions/module:import.mod }
+// { dg-module-cmi =partitions/module-import.mod }
export int b() {
return 0;
--
Nathan Sidwell
C++ ABI
Hi, I've discovered some mangling problems with lambdas. (a) divergence from clang and (b) manglings that incorrectly demangle. With #a I'm not yet sure who is correct. for #b g++ is definitely wrong. From the docs, it doesn't appear to have been bumped this cycle. Is that correct, and I should bump it to 18? nathan -- Nathan Sidwell
Re: C++ ABI
On 9/30/22 09:43, Nathan Sidwell wrote: Hi, I've discovered some mangling problems with lambdas. (a) divergence from clang and (b) manglings that incorrectly demangle. With #a I'm not yet sure who is correct. for #b g++ is definitely wrong. From the docs, it doesn't appear to have been bumped this cycle. Is that correct, and I should bump it to 18? oops, 'it' == -fabi-version nathan -- Nathan Sidwell
Re: [PATCH] c++ modules: static var in inline function [PR104433]
On 10/6/22 12:19, Patrick Palka wrote:
The below testcase fails to link with the error
undefined reference to `f()::y'
ultimately because during stream out for the static VAR_DECL y we
override DECL_EXTERNAL to true, which later during IPA confuses
symbol_table::remove_unreachable_nodes into thinking it's safe
to not emit the symbol.
The streaming code in question already avoids overriding DECL_EXTERNAL
here for DECL_VAR_DECLARED_INLINE_P vars, so it seems natural to do the
same for static vars from an DECL_DECLARED_INLINE_P function scope.
After this patch (and r13-3134-g09df0d8b14dda6), the following now
links:
import ;
int main() { std::make_shared(); }
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
yeah, that's correct -- these are just as inline
PR c++/104433
gcc/cp/ChangeLog:
* module.cc (trees_out::core_bools): Don't override
DECL_EXTERNAL to true for static variables from an inline
function.
gcc/testsuite/ChangeLog:
* g++.dg/modules/static-2_a.H: New test.
* g++.dg/modules/static-2_b.C: New test.
---
gcc/cp/module.cc | 3 +++
gcc/testsuite/g++.dg/modules/static-2_a.H | 8
gcc/testsuite/g++.dg/modules/static-2_b.C | 9 +
3 files changed, 20 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/static-2_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/static-2_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 79cbb346ffa..11f68794cd2 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -5397,6 +5397,9 @@ trees_out::core_bools (tree t)
case VAR_DECL:
if (TREE_PUBLIC (t)
+ && !(TREE_STATIC (t)
+ && DECL_FUNCTION_SCOPE_P (t)
+ && DECL_DECLARED_INLINE_P (DECL_CONTEXT (t)))
&& !DECL_VAR_DECLARED_INLINE_P (t))
is_external = true;
break;
diff --git a/gcc/testsuite/g++.dg/modules/static-2_a.H
b/gcc/testsuite/g++.dg/modules/static-2_a.H
new file mode 100644
index 000..b4546932a12
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/static-2_a.H
@@ -0,0 +1,8 @@
+// PR c++/104433
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+inline int* f() {
+ static int y = 0;
+ return &y;
+}
diff --git a/gcc/testsuite/g++.dg/modules/static-2_b.C
b/gcc/testsuite/g++.dg/modules/static-2_b.C
new file mode 100644
index 000..bfd35b0fc15
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/static-2_b.C
@@ -0,0 +1,9 @@
+// PR c++/104433
+// { dg-additional-options -fmodules-ts }
+// { dg-do link }
+
+import "static-2_a.H";
+
+int main() {
+ f();
+}
--
Nathan Sidwell
c++: Lambda context mangling
VAR and FIELD decls can become part of a lambda context, when the
lambda is 'attached' to that entity (It's a C++20 ODR thing that was
discovered with modules, but is actually separate.) We were not
marking those decls as substitution candidates, leading to demangling
failures and compiler variance.
The alternative I chose is to bump the ABI, and add them to the
substitution table. I think this is the intent of the ABI.
The other alternative would be for the demangler to pop off the most
recent substitution when it saw the 'M'. But we'd have to change [at
least] the libiberty demangler[*] and the clang demangler and the clang
mangler, which seems a bigger challenge.
I'll commit this next week, unless y'all have comments.
We have other divergence from clang, with templated lambdas. Clang
added new manglings [Ty, Tn, Tt] for the lambda's template head (Richard
has a pull request, but it's not merged
https://github.com/itanium-cxx-abi/cxx-abi/issues/31). Without that, we
consider all the template args to be auto when demangling (which is
going to look odd for non-type template args). This divergence in
signature also affects the lambda numbering, as gcc & clang differ in
their opinions about 'same lamda signature'. thoughts on addressing
that to in this cycle?
I have some demangler fixes coming up.
nathan
[*] the demangler already seems to have a bug with 'M' handling.
--
Nathan SidwellFrom 6e943ec67fbe1f2bd09325eb6a2dc6405edfc00f Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 30 Sep 2022 08:43:10 -0700
Subject: [PATCH 1/3] c++: Lambda context mangling
VAR and FIELD decls can become part of a lambda context, when the
lambda is 'attached' to that entity (It's a C++20 ODR thing that was
discovered with modules, but is actually separate.) We were not
marking those decls as substitution candidates, leading to demangling
failures and variance from other compilers.
The alternative I chose is to bump the ABI, and add them to the
substitution table. I think this is the intent of the ABI.
The othe alternative would be for the demangler to pop off the most
recent substitition when it saw the 'M'. But we'd have to change [at
least] the libiberty demangler and the clang demangler and the clang
mangler, which seems a bigger challenge.
gcc/
* common.opt (-fabi-version=): Document 18.
* doc/invoke.texi (-fabi-version): Document 18.
gcc/c-family/
* c-opts.cc (c_common_post_options): Bump abi to 18.
gcc/cp/
* mangle.cc (write_prefix): Add VAR_DECL & FIELD_DECL to
substitution table under abi=18. Note possible mismatch.
gcc/testsuite/
* g++.dg/abi/lambda-ctx1-17.C: New.
* g++.dg/abi/lambda-ctx1-18.C: New.
* g++.dg/abi/lambda-ctx1-18vs17.C: New.
* g++.dg/abi/lambda-ctx1.h: New.
* g++.dg/abi/lambda-vis.C: Adjust expected mangles.
* g++.dg/abi/macro0.C: Adjust.
---
gcc/c-family/c-opts.cc| 2 +-
gcc/common.opt| 3 +++
gcc/cp/mangle.cc | 9 -
gcc/doc/invoke.texi | 3 +++
gcc/testsuite/g++.dg/abi/lambda-ctx1-17.C | 10 ++
gcc/testsuite/g++.dg/abi/lambda-ctx1-18.C | 11 ++
gcc/testsuite/g++.dg/abi/lambda-ctx1-18vs17.C | 9 +
gcc/testsuite/g++.dg/abi/lambda-ctx1.h| 20 +++
gcc/testsuite/g++.dg/abi/lambda-vis.C | 8 +---
gcc/testsuite/g++.dg/abi/macro0.C | 2 +-
10 files changed, 71 insertions(+), 6 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-ctx1-17.C
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-ctx1-18.C
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-ctx1-18vs17.C
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-ctx1.h
diff --git a/gcc/c-family/c-opts.cc b/gcc/c-family/c-opts.cc
index babaa2fc157..55cebf68f9c 100644
--- a/gcc/c-family/c-opts.cc
+++ b/gcc/c-family/c-opts.cc
@@ -975,7 +975,7 @@ c_common_post_options (const char **pfilename)
/* Change flag_abi_version to be the actual current ABI level, for the
benefit of c_cpp_builtins, and to make comparison simpler. */
- const int latest_abi_version = 17;
+ const int latest_abi_version = 18;
/* Generate compatibility aliases for ABI v13 (8.2) by default. */
const int abi_compat_default = 13;
diff --git a/gcc/common.opt b/gcc/common.opt
index 58dc1a0a780..c65efeeb8fc 100644
--- a/gcc/common.opt
+++ b/gcc/common.opt
@@ -1004,6 +1004,9 @@ Driver Undocumented
; member initializers in C++14 and up.
; Default in G++ 12.
;
+; 18: Corrects errors in mangling of lambdas with additional context.
+; Default in G++ 13.
+;
; Additional positive integers will be assigned as new versions of
; the ABI become the default version of the ABI.
fabi-version=
diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index f051e76466a..1215463089b 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -1252,7 +1252,14 @@ write_prefix (const tree node)
{
/* := M */
libiberty: Demangle variadic template lambdas
Now we have templated lambdas, we can have variadic template lambdas,
and this leads to lambda signatures containing parameter packs. But
just like 'auto' inside such a signature, we don't have a containing
template, and thus fail. The fix is to check is_lambda_arg, just as
for a template parameter. This allows us to demangle g++'s manglings
of such lambdas.
It's not a totally accurate demangling, because we don't mangle the
template head (that's a separate issue), but it is better than failing
to demangle.
Due to the way we print subexprs, we add an unnecessary parens around
the argument of the pack. That's an orthogonal problem, for which the
solution is to have better knowledge of operator precedence.
nathan
--
Nathan SidwellFrom 55bd808527e8c1947cf2d2b42769528087b687ef Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 30 Sep 2022 12:11:42 -0700
Subject: [PATCH] libiberty: Demangle variadic template lambdas
Now we have templated lambdas, we can have variadic template lambdas,
and this leads to lambda signatures containing parameter packs. But
just like 'auto' inside such a signature, we don't have a containing
template, and thus fail. The fix is to check is_lambda_arg, just as
for a template parameter. This allows us to demangle g++'s manglings
of such lambdas.
It's not a totally accurate demangling, because we don't mangle the
template head (that's a separate issue), but it is better than failing
to demangle.
Due to the way we print subexprs, we add an unnecessary parens around
the argument of the pack. That's an orthogonal problem, for which the
solution is to have better knowledge of operator precedence.
libiberty/
* cp-demangle.c (d_print_comp_inner): Allow parameter packs
in a lambda signature.
* testsuite/demangle-expected: Add tests.
---
libiberty/cp-demangle.c | 30 +++
libiberty/testsuite/demangle-expected | 7 +++
2 files changed, 24 insertions(+), 13 deletions(-)
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 7ff225ec1aa..303bfbf709e 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -348,7 +348,7 @@ struct d_print_info
be bigger than MAX_RECURSION_COUNT. */
int recursion;
/* Non-zero if we're printing a lambda argument. A template
- parameter reference actually means 'auto'. */
+ parameter reference actually means 'auto', a pack expansion means T... */
int is_lambda_arg;
/* The current index into any template argument packs we are using
for printing, or -1 to print the whole pack. */
@@ -5930,9 +5930,10 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
case DEMANGLE_COMPONENT_PACK_EXPANSION:
{
- int len;
- int i;
- struct demangle_component *a = d_find_pack (dpi, d_left (dc));
+ struct demangle_component *a = NULL;
+
+ if (!dpi->is_lambda_arg)
+ a = d_find_pack (dpi, d_left (dc));
if (a == NULL)
{
/* d_find_pack won't find anything if the only packs involved
@@ -5940,17 +5941,20 @@ d_print_comp_inner (struct d_print_info *dpi, int options,
case, just print the pattern and "...". */
d_print_subexpr (dpi, options, d_left (dc));
d_append_string (dpi, "...");
- return;
}
-
- len = d_pack_length (a);
- dc = d_left (dc);
- for (i = 0; i < len; ++i)
+ else
{
- dpi->pack_index = i;
- d_print_comp (dpi, options, dc);
- if (i < len-1)
- d_append_string (dpi, ", ");
+ int len = d_pack_length (a);
+ int i;
+
+ dc = d_left (dc);
+ for (i = 0; i < len; ++i)
+ {
+ if (i)
+ d_append_string (dpi, ", ");
+ dpi->pack_index = i;
+ d_print_comp (dpi, options, dc);
+ }
}
}
return;
diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected
index 8fad6893ae7..90dd4a13945 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -1574,3 +1574,10 @@ initializer for module Foo.Bar
_ZGIW3FooWP3BarW3Baz
initializer for module Foo:Bar.Baz
+
+_ZZ2L1vENKUlDpT_E_clIJiPiEEEvS0_
+void L1()::{lambda((auto:1)...)#1}::operator()(int, int*) const
+
+_ZZ2L1vENKUlDpT_E_clIJiPiEEEDaS0_
+auto L1()::{lambda((auto:1)...)#1}::operator()(int, int*) const
+
--
2.30.2
Re: [PATCH] c++ modules: ICE with bitfield member in class template
On 10/7/22 11:09, Patrick Palka wrote:
According to grokbitfield, DECL_BITFIELD_REPRESENTATIVE may "temporarily"
contain the width of the bitfield until we layout the class type (after
which it'll contain a FIELD_DECL). But for a class template, it'll always
be the width since we don't/can't layout dependent types.
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
ok, but could you add a comment on why it might not be a decl?
gcc/cp/ChangeLog:
* module.cc (trees_out::mark_class_def): Guard against
DECL_BITFIELD_REPRESENTATIVE not being a decl.
gcc/testsuite/ChangeLog:
* g++.dg/modules/bfield-3.H: New test.
---
gcc/cp/module.cc| 3 ++-
gcc/testsuite/g++.dg/modules/bfield-3.H | 8
2 files changed, 10 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/modules/bfield-3.H
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index cb1929bc5d5..172a72e92b9 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -11919,7 +11919,8 @@ trees_out::mark_class_def (tree defn)
mark_class_member (member);
if (TREE_CODE (member) == FIELD_DECL)
if (tree repr = DECL_BIT_FIELD_REPRESENTATIVE (member))
- mark_declaration (repr, false);
+ if (DECL_P (repr))
+ mark_declaration (repr, false);
}
/* Mark the binfo hierarchy. */
diff --git a/gcc/testsuite/g++.dg/modules/bfield-3.H
b/gcc/testsuite/g++.dg/modules/bfield-3.H
new file mode 100644
index 000..4fd4db7116a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/bfield-3.H
@@ -0,0 +1,8 @@
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+template
+struct A {
+ int x : 1;
+ int y : N;
+};
--
Nathan Sidwell
Re: [PATCH] c++ modules: lazy loading from within template [PR99377]
On 10/4/22 13:36, Patrick Palka wrote:
Here when lazily loading the binding for f at parse time from the
template g, processing_template_decl is set and thus the call to
note_vague_linkage_fn from module_state::read_cluster has no effect,
and we never push f onto deferred_fns and end up never emitting its
definition.
ISTM the behavior of the lazy loading machinery shouldn't be sensitive
to whether we're inside a template, and therefore we should probably be
clearing processing_template_decl somewhere e.g in lazy_load_binding.
This is sufficient to fix the testcase.
yeah, I remember hitting issues with this, but thought I'd got rid of the need
to override processing_template_decl. Do you also need to override it in
lazy_load_pendings though? that's a lazy loader and my suspicion is it might be
susceptible to the same issues.
But it also seems the processing_template_decl test in
note_vague_linkage_fn, added by r8-7539-g977bc3ee11383e for PR84973, is
perhaps too strong: if the intent is to avoid deferring output for
uninstantiated templates, we should make sure that DECL in question is
an uninstantiated template by checking e.g. value_dependent_expression_p.
This too is sufficient to fix the testcase (since f isn't a template)
and survives bootstrap and regtest.
I think this is an orthogonal issue -- can we remove it from this patch?
Does one or the other approach look like the correct fix for this PR?
PR c++/99377
gcc/cp/ChangeLog:
* decl2.cc (note_vague_linkage_fn): Relax processing_template_decl
test to value_dependent_expression_p.
* module.cc (lazy_load_binding): Clear processing_template_decl.
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr99377-2_a.C: New test.
* g++.dg/modules/pr99377-2_b.C: New test.
---
gcc/cp/decl2.cc| 2 +-
gcc/cp/module.cc | 2 ++
gcc/testsuite/g++.dg/modules/pr99377-2_a.C | 5 +
gcc/testsuite/g++.dg/modules/pr99377-2_b.C | 6 ++
4 files changed, 14 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/modules/pr99377-2_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/pr99377-2_b.C
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 9f18466192f..5af4d17ee3b 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -876,7 +876,7 @@ check_classfn (tree ctype, tree function, tree
template_parms)
void
note_vague_linkage_fn (tree decl)
{
- if (processing_template_decl)
+ if (value_dependent_expression_p (decl))
return;
DECL_DEFER_OUTPUT (decl) = 1;
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 500ac06563a..79cbb346ffa 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -19074,6 +19074,8 @@ lazy_load_binding (unsigned mod, tree ns, tree id,
binding_slot *mslot)
timevar_start (TV_MODULE_IMPORT);
+ processing_template_decl_sentinel ptds;
+
/* Stop GC happening, even in outermost loads (because our caller
could well be building up a lookup set). */
function_depth++;
diff --git a/gcc/testsuite/g++.dg/modules/pr99377-2_a.C
b/gcc/testsuite/g++.dg/modules/pr99377-2_a.C
new file mode 100644
index 000..26e2bccbbbe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99377-2_a.C
@@ -0,0 +1,5 @@
+// PR c++/99377
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr99377 }
+export module pr99377;
+export inline void f() { }
diff --git a/gcc/testsuite/g++.dg/modules/pr99377-2_b.C
b/gcc/testsuite/g++.dg/modules/pr99377-2_b.C
new file mode 100644
index 000..69571952c8a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99377-2_b.C
@@ -0,0 +1,6 @@
+// PR c++/99377
+// { dg-additional-options -fmodules-ts }
+// { dg-do link }
+import pr99377;
+template void g() { f(); }
+int main() { g(); }
--
Nathan Sidwell
libiberty: Demangling 'M' prefixes
The grammar for a lambda context can include 'M', and we
were adding the component that generated to the substitution table
twice. Just ignore the 'M' completely -- we'll already have done the
checks we need when we saw its predecessor. A prefix cannot be the
last component of a nested name, so we do not need to check for that
case (although we could if we wanted to be more lenient).
nathan
--
Nathan SidwellFrom 0fa35c7e2974a22b2107fa378895c3069fe07ff3 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 30 Sep 2022 09:43:30 -0700
Subject: [PATCH] libiberty: Demangling 'M' prefixes
The grammar for a lambda context can include 'M', and we
were adding the component that generated to the substitution table
twice. Just ignore the 'M' completely -- we'll already have done the
checks we need when we saw its predecessor. A prefix cannot be the
last component of a nested name, so we do not need to check for that
case (although we could if we wanted to be more lenient).
libiberty/
* cp-demangle.c (d_prefix): 'M' components are not
(re-)added to the substitution table.
* testsuite/demangle-expected: Add tests.
---
libiberty/cp-demangle.c | 8 +++-
libiberty/testsuite/demangle-expected | 21 +
2 files changed, 24 insertions(+), 5 deletions(-)
diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c
index 303bfbf709e..4beb4d257bb 100644
--- a/libiberty/cp-demangle.c
+++ b/libiberty/cp-demangle.c
@@ -1609,12 +1609,10 @@ d_prefix (struct d_info *di, int substable)
}
else if (peek == 'M')
{
- /* Initializer scope for a lambda. We don't need to represent
- this; the normal code will just treat the variable as a type
- scope, which gives appropriate output. */
- if (ret == NULL)
- return NULL;
+ /* Initializer scope for a lambda. We already added it as a
+ substitution candidate, don't do that again. */
d_advance (di, 1);
+ continue;
}
else
{
diff --git a/libiberty/testsuite/demangle-expected b/libiberty/testsuite/demangle-expected
index 90dd4a13945..bd92b12076b 100644
--- a/libiberty/testsuite/demangle-expected
+++ b/libiberty/testsuite/demangle-expected
@@ -1581,3 +1581,24 @@ void L1()::{lambda((auto:1)...)#1}::operator()(int, int*) const
_ZZ2L1vENKUlDpT_E_clIJiPiEEEDaS0_
auto L1()::{lambda((auto:1)...)#1}::operator()(int, int*) const
+_Z7captureIN4gvarMUlvE_EE7WrapperIT_EOS3_
+Wrapper capture(gvar::{lambda()#1}&&)
+
+_ZNK2L2MUlT_T0_E_clIifEEvS_S0_
+void L2::{lambda(auto:1, auto:2)#1}::operator()(L2, int) const
+
+_ZNK1C1fMUlT_E_clIMS_iEEDaS1_
+auto C::f::{lambda(auto:1)#1}::operator()(int C::*) const
+
+_ZNK2L2MUlT_T0_E_clIifEEvS0_S1_
+void L2::{lambda(auto:1, auto:2)#1}::operator()(int, float) const
+
+_ZNK1B2L3MUlT_T0_E_clIjdEEvS1_S2_
+void B::L3::{lambda(auto:1, auto:2)#1}::operator()(unsigned int, double) const
+
+_Z3fooIN1qMUlvE_ENS0_UlvE0_EEiOT_OT0_
+int foo(q::{lambda()#1}&&, q::{lambda()#2}&&)
+
+_ZNK2L1MUlDpT_E_clIJiPiEEEvS1_
+void L1::{lambda((auto:1)...)#1}::operator()(int, int*) const
+
--
2.30.2
Re: [PATCH] c++ modules: lazy loading from within template [PR99377]
On 10/11/22 10:58, Patrick Palka wrote:
On Mon, 10 Oct 2022, Nathan Sidwell wrote:
On 10/4/22 13:36, Patrick Palka wrote:
Here when lazily loading the binding for f at parse time from the
template g, processing_template_decl is set and thus the call to
note_vague_linkage_fn from module_state::read_cluster has no effect,
and we never push f onto deferred_fns and end up never emitting its
definition.
ISTM the behavior of the lazy loading machinery shouldn't be sensitive
to whether we're inside a template, and therefore we should probably be
clearing processing_template_decl somewhere e.g in lazy_load_binding.
This is sufficient to fix the testcase.
yeah, I remember hitting issues with this, but thought I'd got rid of the need
to override processing_template_decl. Do you also need to override it in
lazy_load_pendings though? that's a lazy loader and my suspicion is it might
be susceptible to the same issues.
Hmm yeah, looks like we need to override it in lazy_load_pendings too:
I ran the testsuite with gcc_assert (!processing_template_decl) added to
module_state::read_cluster and if we don't also override it in
lazy_load_binding then the assert triggers for pr99425-2_b.X.
But it also seems the processing_template_decl test in
note_vague_linkage_fn, added by r8-7539-g977bc3ee11383e for PR84973, is
perhaps too strong: if the intent is to avoid deferring output for
uninstantiated templates, we should make sure that DECL in question is
an uninstantiated template by checking e.g. value_dependent_expression_p.
This too is sufficient to fix the testcase (since f isn't a template)
and survives bootstrap and regtest.
I think this is an orthogonal issue -- can we remove it from this patch?
Done.
-- >8 --
Subject: [PATCH] c++ modules: lazy loading from within template [PR99377]
Here when lazily loading the binding for f at parse time from the
template g, processing_template_decl is set and thus the call to
note_vague_linkage_fn from module_state::read_cluster has no effect,
and we never push f onto deferred_fns and end up never emitting its
definition.
ISTM the behavior of the lazy loading machinery shouldn't be sensitive
to whether we're inside a template, and therefore we should be clearing
processing_template_decl in the entrypoints lazy_load_binding and
lazy_load_pendings.
PR c++/99377
gcc/cp/ChangeLog:
* module.cc (lazy_load_binding): Clear processing_template_decl.
(lazy_load_pendings): Likewise.
ok, thanks
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr99377-2_a.C: New test.
* g++.dg/modules/pr99377-2_b.C: New test.
---
gcc/cp/module.cc | 8
gcc/testsuite/g++.dg/modules/pr99377-2_a.C | 5 +
gcc/testsuite/g++.dg/modules/pr99377-2_b.C | 6 ++
3 files changed, 19 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/pr99377-2_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/pr99377-2_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 94fbee85225..7c48602136c 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -19081,6 +19081,10 @@ lazy_load_binding (unsigned mod, tree ns, tree id,
binding_slot *mslot)
timevar_start (TV_MODULE_IMPORT);
+ /* Make sure lazy loading from a template context behaves as if
+ from a non-template context. */
+ processing_template_decl_sentinel ptds;
+
/* Stop GC happening, even in outermost loads (because our caller
could well be building up a lookup set). */
function_depth++;
@@ -19129,6 +19133,10 @@ lazy_load_binding (unsigned mod, tree ns, tree id,
binding_slot *mslot)
void
lazy_load_pendings (tree decl)
{
+ /* Make sure lazy loading from a template context behaves as if
+ from a non-template context. */
+ processing_template_decl_sentinel ptds;
+
tree key_decl;
pending_key key;
key.ns = find_pending_key (decl, &key_decl);
diff --git a/gcc/testsuite/g++.dg/modules/pr99377-2_a.C
b/gcc/testsuite/g++.dg/modules/pr99377-2_a.C
new file mode 100644
index 000..26e2bccbbbe
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99377-2_a.C
@@ -0,0 +1,5 @@
+// PR c++/99377
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr99377 }
+export module pr99377;
+export inline void f() { }
diff --git a/gcc/testsuite/g++.dg/modules/pr99377-2_b.C
b/gcc/testsuite/g++.dg/modules/pr99377-2_b.C
new file mode 100644
index 000..69571952c8a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr99377-2_b.C
@@ -0,0 +1,6 @@
+// PR c++/99377
+// { dg-additional-options -fmodules-ts }
+// { dg-do link }
+import pr99377;
+template void g() { f(); }
+int main() { g(); }
--
Nathan Sidwell
Re: [PATCH] c++ modules: ICE with templated friend and std namespace [PR100134]
On 10/11/22 11:35, Patrick Palka wrote:
IIUC the function depset::hash::add_binding_entity has an assert
verifying that if a namespace contains an exported entity, then
the namespace must have been opened in the module purview:
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;
}
We're tripping over this assert in the below testcase because by
instantiating and exporting std::A, we end up in turn defining
and exporting the hidden friend std::f without ever having opening
the enclosing namespace std within the module purview and thus
DECL_MODULE_PURVIEW_P (std_node) is false.
Note that it's important that the enclosing namespace is std here: if we
use a different namespace then the ICE disappears. This probably has
something to do with the fact that we predefine std via push_namespace
from cxx_init_decl_processing (which makes it look like we've opened the
namespace in the TU), whereas with another namespace we would instead
lazily obtain the NAMESPACE_DECL from add_imported_namespace.
Since templated frined functions are special in that they give us a way
to declare a new namespace-scope function without having to explicitly
open the namespace, this patch proposes to fix this issue by propagating
DECL_MODULE_PURVIEW_P from a friend function to the enclosing namespace
when instantiating the friend.
ouch. This is ok, but I think we have a bug -- what is the module
ownership of the friend introduced by the instantiation?
Haha, there's a note on 13.7.5/3 -- the attachment is to the same module
as the befriending class.
That means we end up creating and writing out entities that exist in the
symbol table (albeit hidden) whose module ownership is neither the
global module or the tu's module. That's not something the module
machinery anticipates. We'll get the mangling wrong for starters. Hmm.
These are probably rare. Thinking about the right solution though ...
nathan
Tested on x86_64-pc-linux-gnu, does this look like the right fix? Other
solutions that seem to work are to set DECL_MODULE_PURVIEW_P on std_node
after the fact from declare_module, or simply to suppress the assert for
std_node.
PR c++/100134
gcc/cp/ChangeLog:
* pt.cc (tsubst_friend_function): Propagate DECL_MODULE_PURVIEW_P
from the new declaration to the enclosing namespace scope.
gcc/testsuite/ChangeLog:
* g++.dg/modules/tpl-friend-8_a.H: New test.
* g++.dg/modules/tpl-friend-8_b.C: New test.
---
gcc/cp/pt.cc | 7 +++
gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H | 9 +
gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C | 8
3 files changed, 24 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 5b9fc588a21..9e3085f3fa6 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -11448,6 +11448,13 @@ tsubst_friend_function (tree decl, tree args)
by duplicate_decls. */
new_friend = old_decl;
}
+
+ /* We've just added a new namespace-scope entity to the purview without
+necessarily having opened the enclosing namespace, so make sure the
+enclosing namespace is in the purview now too. */
+ if (TREE_CODE (DECL_CONTEXT (new_friend)) == NAMESPACE_DECL)
+ DECL_MODULE_PURVIEW_P (DECL_CONTEXT (new_friend))
+ |= DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (new_friend));
}
else
{
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H
b/gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H
new file mode 100644
index 000..bd2290460b5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H
@@ -0,0 +1,9 @@
+// PR c++/100134
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+namespace std {
+ template struct A {
+friend void f(A) { }
+ };
+}
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C
b/gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C
new file mode 100644
index 000..76d7447c2eb
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C
@@ -0,0 +1,8 @@
+// PR c++/100134
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr100134 }
+export module pr100134;
+
+import "tpl-friend-8_a.H";
+
+export std::A a;
--
Nathan Sidwell
Re: [PATCH] c++ modules: ICE with dynamic_cast [PR106304]
On 10/13/22 15:04, Patrick Palka wrote:
The FUNCTION_DECL we build for __dynamic_cast has an empty DECL_CONTEXT,
but trees_out::tree_node expects all FUNCTION_DECLs to have non-empty
DECL_CONTEXT thus we crash when streaming out the dynamic_cast in the
below testcase.
This patch naively fixes this by setting DECL_CONTEXT for __dynamic_cast
appropriately. Like for __cxa_atexit which is similarly lazily declared,
I suppose we should push it into the namespace too.
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
This is correct, there were a bunch of similar issues, surprising these
never triggered.
PR c++/106304
gcc/cp/ChangeLog:
* constexpr.cc (cxx_dynamic_cast_fn_p): Check for abi_node
instead of global_namespace.
* rtti.cc (build_dynamic_cast_1): Set DECL_CONTEXT and
DECL_SOURCE_LOCATION on dynamic_cast_node, and push it
into the namespace.
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr106304_a.C: New test.
* g++.dg/modules/pr106304_b.C: New test.
---
gcc/cp/constexpr.cc | 2 +-
gcc/cp/rtti.cc| 4
gcc/testsuite/g++.dg/modules/pr106304_a.C | 12
gcc/testsuite/g++.dg/modules/pr106304_b.C | 8
4 files changed, 25 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/modules/pr106304_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/pr106304_b.C
diff --git a/gcc/cp/constexpr.cc b/gcc/cp/constexpr.cc
index 06dcd71c926..5939d2882f8 100644
--- a/gcc/cp/constexpr.cc
+++ b/gcc/cp/constexpr.cc
@@ -2108,7 +2108,7 @@ cxx_dynamic_cast_fn_p (tree fndecl)
{
return (cxx_dialect >= cxx20
&& id_equal (DECL_NAME (fndecl), "__dynamic_cast")
- && CP_DECL_CONTEXT (fndecl) == global_namespace);
+ && CP_DECL_CONTEXT (fndecl) == abi_node);
}
/* Often, we have an expression in the form of address + offset, e.g.
diff --git a/gcc/cp/rtti.cc b/gcc/cp/rtti.cc
index f5b43ec0fb2..a85c7b56409 100644
--- a/gcc/cp/rtti.cc
+++ b/gcc/cp/rtti.cc
@@ -787,6 +787,10 @@ build_dynamic_cast_1 (location_t loc, tree type, tree expr,
NULL_TREE));
dcast_fn = (build_library_fn_ptr
(fn_name, fn_type, ECF_LEAF | ECF_PURE |
ECF_NOTHROW));
+ /* As with __cxa_atexit in get_atexit_node. */
+ DECL_CONTEXT (dcast_fn) = FROB_CONTEXT (current_namespace);
+ DECL_SOURCE_LOCATION (dcast_fn) = BUILTINS_LOCATION;
+ dcast_fn = pushdecl (dcast_fn, /*hiding=*/true);
pop_abi_namespace (flags);
dynamic_cast_node = dcast_fn;
}
diff --git a/gcc/testsuite/g++.dg/modules/pr106304_a.C
b/gcc/testsuite/g++.dg/modules/pr106304_a.C
new file mode 100644
index 000..b999eeccf4a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr106304_a.C
@@ -0,0 +1,12 @@
+// PR c++/106304
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr106304 }
+
+export module pr106304;
+
+struct A { virtual ~A() = default; };
+struct B : A { };
+
+inline const B* as_b(const A& a) {
+ return dynamic_cast(&a);
+}
diff --git a/gcc/testsuite/g++.dg/modules/pr106304_b.C
b/gcc/testsuite/g++.dg/modules/pr106304_b.C
new file mode 100644
index 000..e8333909c8d
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr106304_b.C
@@ -0,0 +1,8 @@
+// PR c++/106304
+// { dg-additional-options -fmodules-ts }
+
+module pr106304;
+
+void f(A& a) {
+ as_b(a);
+}
--
Nathan Sidwell
Re: [PATCH] c++ modules: ICE with templated friend and std namespace [PR100134]
On 10/13/22 11:27, Jason Merrill wrote:
On 10/11/22 13:40, Nathan Sidwell wrote:
On 10/11/22 11:35, Patrick Palka wrote:
IIUC the function depset::hash::add_binding_entity has an assert
verifying that if a namespace contains an exported entity, then
the namespace must have been opened in the module purview:
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;
}
We're tripping over this assert in the below testcase because by
instantiating and exporting std::A, we end up in turn defining
and exporting the hidden friend std::f without ever having opening
the enclosing namespace std within the module purview and thus
DECL_MODULE_PURVIEW_P (std_node) is false.
Note that it's important that the enclosing namespace is std here: if we
use a different namespace then the ICE disappears. This probably has
something to do with the fact that we predefine std via push_namespace
from cxx_init_decl_processing (which makes it look like we've opened the
namespace in the TU), whereas with another namespace we would instead
lazily obtain the NAMESPACE_DECL from add_imported_namespace.
Since templated frined functions are special in that they give us a way
to declare a new namespace-scope function without having to explicitly
open the namespace, this patch proposes to fix this issue by propagating
DECL_MODULE_PURVIEW_P from a friend function to the enclosing namespace
when instantiating the friend.
ouch. This is ok, but I think we have a bug -- what is the module
ownership of the friend introduced by the instantiation?
Haha, there's a note on 13.7.5/3 -- the attachment is to the same
module as the befriending class.
That means we end up creating and writing out entities that exist in
the symbol table (albeit hidden) whose module ownership is neither the
global module or the tu's module. That's not something the module
machinery anticipates. We'll get the mangling wrong for starters. Hmm.
I suspect we're writing out the friend definition, regardless of whether
the class instantiation is actually referenced from a written-out
entity. That's wrong, but it may simply be an inefficiency, rather than
an error.
These are probably rare. Thinking about the right solution though ...
This seems closely connected to
https://cplusplus.github.io/CWG/issues/2588.html
ah, I had gotten myself confused, I don't think there's an ODR[*]
problem in the std, this is 'just' a horrible surprise. One of the
reasons I disliked the (original) TS's cross-module extern declaration.
Not sure of the spelling, but something like:
extern export other.module int entity ();
was that it had the same requirements as this friend instantiation
needs. (this got killed when ATOM ws merged in, and thus never needed
to be implemented)
[*] The instantiated friend definition is just as temploidy as a
non-inline member function of the templated class. And therefore must
be just as well-formed to have multiple definitions reachable. Hm, I
thought that such in-template-class-defined friends could have no other
declaration, but I cannot find the wording for that.
Tested on x86_64-pc-linux-gnu, does this look like the right fix? Other
solutions that seem to work are to set DECL_MODULE_PURVIEW_P on std_node
after the fact from declare_module, or simply to suppress the assert for
std_node.
PR c++/100134
gcc/cp/ChangeLog:
* pt.cc (tsubst_friend_function): Propagate DECL_MODULE_PURVIEW_P
from the new declaration to the enclosing namespace scope.
gcc/testsuite/ChangeLog:
* g++.dg/modules/tpl-friend-8_a.H: New test.
* g++.dg/modules/tpl-friend-8_b.C: New test.
---
gcc/cp/pt.cc | 7 +++
gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H | 9 +
gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C | 8
3 files changed, 24 insertions(+)
create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/tpl-friend-8_b.C
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index 5b9fc588a21..9e3085f3fa6 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -11448,6 +11448,13 @@ tsubst_friend_function (tree decl, tree args)
by duplicate_decls. */
new_friend = old_decl;
}
+
+ /* We've just added a new namespace-scope entity to the
purview without
+ necessarily having opened the enclosing namespace, so make sure
the
+ enclosing namespace is in the purview now too. */
+ if (TREE_CODE (DECL_CONTEXT (new_friend)) == NAMESPACE_DECL)
+ DECL_MODULE_PURVIEW_P (DECL_CONTEXT (new_friend))
+ |= DECL_MODULE_PURVIEW_P (STRIP_TEMPLATE (new_friend));
}
else
{
diff --git a/gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H
b/gcc/testsuite/g++.dg/modules/tpl-friend-8_a.H
new file mod
Re: [PATCH] c++ modules: streaming constexpr_fundef [PR101449]
On 10/14/22 13:00, Patrick Palka wrote:
IIUC we currently avoid streaming the RESULT_DECL and PARM_DECLs
of a constexpr_fundef entry under the assumption that they're just
copies of the DECL_RESULT and DECL_ARGUMENTS of the FUNCTION_DECL.
Thus we can just make new copies of DECL_RESULT and DECL_ARGUMENTs
on stream in rather than separately streaming them.
Unfortunately this assumption isn't true generally: the FUNCTION_DECL
contains genericized trees, whereas the constexpr_fundef entry contains
pre-GENERIC trees. So in particular DECL_RESULT and DECL_ARGUMENTs
may have been turned into invisiref parms which we don't handle during
during constexpr evaluation and so we ICE in the below testcase.
This patch fixes this by faithfully streaming the RESULT_DECL and
PARM_DECLs of a constexpr_fundef entry.
Hm, the reason for the complexity was that I wanted to recreate the tree graph
where the fndecl came from one TU and the defn came from another one -- we need
the definition to refer to argument decls from the already-read decl. However,
it seems that for constexpr fns here, that is not needed, resulting in a
significant simplification.
ok.
nathan
PR c++/101449
gcc/cp/ChangeLog:
* module.cc (trees_out::write_function_def): Stream the
parms and result of the constexpr_fundef entry.
(trees_in::read_function_def): Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/modules/cexpr-3_a.C: New test.
* g++.dg/modules/cexpr-3_b.C: New test.
---
gcc/cp/module.cc | 59
gcc/testsuite/g++.dg/modules/cexpr-3_a.C | 14 ++
gcc/testsuite/g++.dg/modules/cexpr-3_b.C | 7 +++
3 files changed, 29 insertions(+), 51 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/cexpr-3_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/cexpr-3_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 7ffeefa7c1f..999ff3faafc 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -11553,34 +11553,13 @@ trees_out::write_function_def (tree decl)
tree_node (DECL_FRIEND_CONTEXT (decl));
constexpr_fundef *cexpr = retrieve_constexpr_fundef (decl);
- int tag = 0;
- if (cexpr)
-{
- if (cexpr->result == error_mark_node)
- /* We'll stream the RESULT_DECL naturally during the
- serialization. We never need to fish it back again, so
- that's ok. */
- tag = 0;
- else
- tag = insert (cexpr->result);
-}
+
if (streaming_p ())
+u (cexpr != nullptr);
+ if (cexpr)
{
- i (tag);
- if (tag)
- dump (dumper::TREE)
- && dump ("Constexpr:%d result %N", tag, cexpr->result);
-}
- if (tag)
-{
- unsigned ix = 0;
- for (tree parm = cexpr->parms; parm; parm = DECL_CHAIN (parm), ix++)
- {
- tag = insert (parm);
- if (streaming_p ())
- dump (dumper::TREE)
- && dump ("Constexpr:%d parm:%u %N", tag, ix, parm);
- }
+ chained_decls (cexpr->parms);
+ tree_node (cexpr->result);
tree_node (cexpr->body);
}
@@ -11613,32 +11592,10 @@ trees_in::read_function_def (tree decl, tree maybe_template)
tree maybe_dup = odr_duplicate (maybe_template, DECL_SAVED_TREE (decl));
bool installing = maybe_dup && !DECL_SAVED_TREE (decl);
- if (int wtag = i ())
+ if (u ())
{
- int tag = 1;
- cexpr.result = error_mark_node;
-
- cexpr.result = copy_decl (result);
- tag = insert (cexpr.result);
-
- if (wtag != tag)
- set_overrun ();
- dump (dumper::TREE)
- && dump ("Constexpr:%d result %N", tag, cexpr.result);
-
- cexpr.parms = NULL_TREE;
- tree *chain = &cexpr.parms;
- unsigned ix = 0;
- for (tree parm = DECL_ARGUMENTS (maybe_dup ? maybe_dup : decl);
- parm; parm = DECL_CHAIN (parm), ix++)
- {
- tree p = copy_decl (parm);
- tag = insert (p);
- dump (dumper::TREE)
- && dump ("Constexpr:%d parm:%u %N", tag, ix, p);
- *chain = p;
- chain = &DECL_CHAIN (p);
- }
+ cexpr.parms = chained_decls ();
+ cexpr.result = tree_node ();
cexpr.body = tree_node ();
cexpr.decl = decl;
}
diff --git a/gcc/testsuite/g++.dg/modules/cexpr-3_a.C
b/gcc/testsuite/g++.dg/modules/cexpr-3_a.C
new file mode 100644
index 000..be24bb43a7b
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/cexpr-3_a.C
@@ -0,0 +1,14 @@
+// PR c++/101449
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr101449 }
+
+export module pr101449;
+
+struct X {
+ bool b = true;
+ constexpr X() { }
+ constexpr X(const X&) { }
+};
+
+export constexpr X f() { return {}; }
+export constexpr bool g(X x) { return x.b; }
diff --git a/gcc/testsuite/g++.dg/modules/cexpr-3_b.C
b/gcc/testsuite/g++.dg/modules/cexpr-3_b.C
new file mode 100644
index 000..cbf3be4fcab
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/cexpr-3_b.C
@@ -0,0
Re: [PATCH] c++ modules: stream non-trailing default targs [PR105045]
On 10/18/22 10:13, Patrick Palka wrote:
This fixes the below testcase in which we neglect to stream the default
argument for T only because the subsequent parameter U doesn't also have
a default argument.
ok
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
PR c++/105045
gcc/cp/ChangeLog:
* module.cc (trees_out::tpl_parms_fini): Don't assume default
template arguments are all trailing.
(trees_in::tpl_parms_fini): Likewise.
gcc/testsuite/ChangeLog:
* g++.dg/modules/pr105045_a.C: New test.
* g++.dg/modules/pr105045_b.C: New test.
---
gcc/cp/module.cc | 20 ++--
gcc/testsuite/g++.dg/modules/pr105045_a.C | 7 +++
gcc/testsuite/g++.dg/modules/pr105045_b.C | 6 ++
3 files changed, 19 insertions(+), 14 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/pr105045_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/pr105045_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 999ff3faafc..2c2f9a9a8cb 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -10034,15 +10034,11 @@ trees_out::tpl_parms_fini (tree tmpl, unsigned
tpl_levels)
tree vec = TREE_VALUE (parms);
tree_node (TREE_TYPE (vec));
- tree dflt = error_mark_node;
for (unsigned ix = TREE_VEC_LENGTH (vec); ix--;)
{
tree parm = TREE_VEC_ELT (vec, ix);
- if (dflt)
- {
- dflt = TREE_PURPOSE (parm);
- tree_node (dflt);
- }
+ tree dflt = TREE_PURPOSE (parm);
+ tree_node (dflt);
if (streaming_p ())
{
@@ -10072,19 +10068,15 @@ trees_in::tpl_parms_fini (tree tmpl, unsigned
tpl_levels)
tpl_levels--; parms = TREE_CHAIN (parms))
{
tree vec = TREE_VALUE (parms);
- tree dflt = error_mark_node;
TREE_TYPE (vec) = tree_node ();
for (unsigned ix = TREE_VEC_LENGTH (vec); ix--;)
{
tree parm = TREE_VEC_ELT (vec, ix);
- if (dflt)
- {
- dflt = tree_node ();
- if (get_overrun ())
- return false;
- TREE_PURPOSE (parm) = dflt;
- }
+ tree dflt = tree_node ();
+ if (get_overrun ())
+ return false;
+ TREE_PURPOSE (parm) = dflt;
tree decl = TREE_VALUE (parm);
if (TREE_CODE (decl) == TEMPLATE_DECL)
diff --git a/gcc/testsuite/g++.dg/modules/pr105045_a.C
b/gcc/testsuite/g++.dg/modules/pr105045_a.C
new file mode 100644
index 000..597f9294185
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr105045_a.C
@@ -0,0 +1,7 @@
+// PR c++/105045
+// { dg-additional-options -fmodules-ts }
+// { dg-module-cmi pr105045 }
+
+export module pr105045;
+
+export template void f(U) { }
diff --git a/gcc/testsuite/g++.dg/modules/pr105045_b.C
b/gcc/testsuite/g++.dg/modules/pr105045_b.C
new file mode 100644
index 000..77c94d4c473
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/pr105045_b.C
@@ -0,0 +1,6 @@
+// PR c++/105045
+// { dg-additional-options -fmodules-ts }
+
+import pr105045;
+
+int main() { f(0); }
--
Nathan Sidwell
Re: [PATCH] c++ modules: handle CONCEPT_DECL in node_template_info [PR102963]
On 10/20/22 10:07, Patrick Palka wrote:
Here node_template_info is overlooking that CONCEPT_DECL has TEMPLATE_INFO
too, which makes get_originating_module_decl for the CONCEPT_DECL fail to
return the corresponding TEMPLATE_DECL, which leads to an ICE from
import_entity_index while pretty printing the CONCEPT_DECL's module
suffix as part of the failed static assert diagnostic.
ok
Tested on x86_64-pc-linux-gnu, does this look OK for trunk?
PR c++/102963
gcc/cp/ChangeLog:
* module.cc (node_template_info): Handle CONCEPT_DECL.
gcc/testsuite/ChangeLog:
* g++.dg/modules/concept-7_a.C: New test.
* g++.dg/modules/concept-7_b.C: New test.
---
gcc/cp/module.cc | 3 ++-
gcc/testsuite/g++.dg/modules/concept-7_a.C | 7 +++
gcc/testsuite/g++.dg/modules/concept-7_b.C | 7 +++
3 files changed, 16 insertions(+), 1 deletion(-)
create mode 100644 gcc/testsuite/g++.dg/modules/concept-7_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/concept-7_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index bb406a5cf01..dfed0a5ef89 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -4046,7 +4046,8 @@ node_template_info (tree decl, int &use)
|| TREE_CODE (decl) == TYPE_DECL
|| TREE_CODE (decl) == FUNCTION_DECL
|| TREE_CODE (decl) == FIELD_DECL
- || TREE_CODE (decl) == TEMPLATE_DECL))
+ || TREE_CODE (decl) == TEMPLATE_DECL
+ || TREE_CODE (decl) == CONCEPT_DECL))
{
use_tpl = DECL_USE_TEMPLATE (decl);
ti = DECL_TEMPLATE_INFO (decl);
diff --git a/gcc/testsuite/g++.dg/modules/concept-7_a.C
b/gcc/testsuite/g++.dg/modules/concept-7_a.C
new file mode 100644
index 000..a39b31bf7f0
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/concept-7_a.C
@@ -0,0 +1,7 @@
+// PR c++/102963
+// { dg-additional-options "-fmodules-ts -fconcepts" }
+// { dg-module-cmi pr102963 }
+
+export module pr102963;
+
+export template concept C = __is_same(T, int);
diff --git a/gcc/testsuite/g++.dg/modules/concept-7_b.C
b/gcc/testsuite/g++.dg/modules/concept-7_b.C
new file mode 100644
index 000..1f81208ebd5
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/concept-7_b.C
@@ -0,0 +1,7 @@
+// PR c++/102963
+// { dg-additional-options "-fmodules-ts -fconcepts" }
+
+import pr102963;
+
+static_assert(C);
+static_assert(C); // { dg-error "static assert" }
--
Nathan Sidwell
Re: [PATCH] c++ modules: verify_type failure with typedef enum [PR106848]
On 10/19/22 09:55, Patrick Palka wrote:
On Wed, 19 Oct 2022, Richard Biener wrote:
On Tue, Oct 18, 2022 at 8:26 PM Patrick Palka wrote:
On Fri, 14 Oct 2022, Richard Biener wrote:
On Thu, Oct 13, 2022 at 5:40 PM Patrick Palka via Gcc-patches
wrote:
Here during stream in we end up having created a type variant for the enum
before we read the enum's definition, and thus the variant inherited stale
TYPE_VALUES and TYPE_MIN/MAX_VALUES, which leads to an ICE (with -g). The
stale variant got created from set_underlying_type during earlier stream in
of the (redundant) typedef for the enum.
This patch works around this by setting TYPE_VALUES and TYPE_MIN/MAX_VALUES
for all variants when reading in an enum definition. Does this look like
the right approach? Or perhaps we need to arrange that we read the enum
definition before reading in the typedef decl? Note that seems to be an
issue only when the typedef name and enum names are the same (thus the
typedef is redundant), otherwise we seem to read the enum definition first
as desired.
PR c++/106848
gcc/cp/ChangeLog:
* module.cc (trees_in::read_enum_def): Set the TYPE_VALUES,
TYPE_MIN_VALUE and TYPE_MAX_VALUE of all type variants.
gcc/testsuite/ChangeLog:
* g++.dg/modules/enum-9_a.H: New test.
* g++.dg/modules/enum-9_b.C: New test.
---
gcc/cp/module.cc| 9 ++---
gcc/testsuite/g++.dg/modules/enum-9_a.H | 5 +
gcc/testsuite/g++.dg/modules/enum-9_b.C | 6 ++
3 files changed, 17 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/enum-9_a.H
create mode 100644 gcc/testsuite/g++.dg/modules/enum-9_b.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 7ffeefa7c1f..97fb80bcd44 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -12303,9 +12303,12 @@ trees_in::read_enum_def (tree defn, tree
maybe_template)
if (installing)
{
- TYPE_VALUES (type) = values;
- TYPE_MIN_VALUE (type) = min;
- TYPE_MAX_VALUE (type) = max;
+ for (tree t = type; t; t = TYPE_NEXT_VARIANT (t))
+ {
+ TYPE_VALUES (t) = values;
+ TYPE_MIN_VALUE (t) = min;
+ TYPE_MAX_VALUE (t) = max;
+ }
it's definitely somewhat ugly but at least type_hash_canon doesn't hash
these for ENUMERAL_TYPE (but it does compare them! which in principle
means it could as well hash them ...)
I think that if you read both from the same module that you should arrange
to read what you refer to first? But maybe that's not the actual issue here.
*nod* reading in the enum before reading in the typedef seems like
the most direct solution, though not sure how to accomplish that :/
For LTO streaming we DFS walk tree edges from all entries into the tree
graph we want to stream, collecting and streaming SCCs. Not sure if
doing similar for module streaming would help this case though.
FWIW I managed to obtain a more interesting reduction for this ICE, one
that doesn't use a typedef bound to the same name as the enum:
$ cat 106848_a.H
template
struct pair {
using type = void(*)(const _T1&);
};
struct _ScannerBase {
enum _TokenT { _S_token_anychar };
pair<_TokenT> _M_token_tbl;
};
$ cat 106848_b.C
import "106848_a.H";
using type = _ScannerBase;
$ g++ -fmodules-ts -g 106848_a.H 106848_b.C
106848_b.C:3:14: error: type variant differs by TYPE_MAX_VALUE
Like in the less interesting testcase, the problem is ultimately that we
create a variant of the enum (as part of reading in pair<_TokenT>::type)
before reading the enum's definition, thus the variant inherits stale
TYPE_MIN/MAX_VALUE.
Perhaps pair<_TokenT>::type should indirectly depend on the definition
of _TokenT -- but IIUC we generally don't require a type to be defined
in order to refer to it, so enforcing such a dependency would be a
pessimization I think.
So ISTM this isn't a dependency issue (pair<_TokenT>::type already
implicitly depends on the ENUMERAL_TYPE, just not also the enum's
defining TYPE_DECL), and the true issue is that we're streaming
TYPE_MIN/MAX_VALUE only as part of an enum's definition, which the
linked patch fixes.
Thanks for the explanation, it's a situation I didn;t anticipate and your fix is
good. Could you add a comment about why you need to propagate the values though?
nathan
A somewhat orthogonal issue (that incidentally fixes this testcase) is
that we stream TYPE_MIN/MAX_VALUE only for enums with a definition, but
the frontend sets these fields even for opaque enums. If we make sure
to stream these fields for all ENUMERAL_TYPEs, then we won't have to
worry about these fields being stale for variants that may have been
created before reading in the enum definition (their TYPE_VALUES field
will still be stale I guess, but verify_type doesn't worry about that
it seems, so we avoid the ICE).
patch to that effect is at
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603831.html
Richard.
rest_of_type_c
Re: [PATCH] c++ modules: verify_type failure with typedef enum [PR106848]
On 10/21/22 09:11, Patrick Palka wrote:
On Fri, 21 Oct 2022, Nathan Sidwell wrote:
Thanks for the explanation, it's a situation I didn;t anticipate and your fix
is good. Could you add a comment about why you need to propagate the values
though?
Thanks a lot, will do. Just to make sure since there are multiple
solutions proposed, do you prefer to go with
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603487.html
or
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603831.html ?
Both solutions fix the PR106848 issue (empty TYPE_MIN/MAX_VALUE on an
enum type variant), but the latter also fixes the related PR102600
(empty TYPE_MIN/MAX_VALUE on the main variant of an enum with no
enumerators). (We could maybe even combine the two solutions: stream
TYPE_MIN/MAX_VALUE as part of ENUMERAL_TYPE, and also update TYPE_VALUES
of each variant during trees_in::read_enum_def)
Oh, let's go with the latter:
* module.cc (trees_out::core_vals): Stream TYPE_MAX_VALUE and
TYPE_MIN_VALUE of ENUMERAL_TYPE.
(trees_in::core_vals): Likewise.
(trees_out::write_enum_def): Don't stream them here.
(trees_in::read_enum_def): Likewise.
but, again, some comments -- at the new streaming point, and in the defn
streamer were we no longer stream them.
thanks.
nathan
A somewhat orthogonal issue (that incidentally fixes this testcase) is
that we stream TYPE_MIN/MAX_VALUE only for enums with a definition, but
the frontend sets these fields even for opaque enums. If we make sure
to stream these fields for all ENUMERAL_TYPEs, then we won't have to
worry about these fields being stale for variants that may have been
created before reading in the enum definition (their TYPE_VALUES field
will still be stale I guess, but verify_type doesn't worry about that
it seems, so we avoid the ICE).
patch to that effect is at
https://gcc.gnu.org/pipermail/gcc-patches/2022-October/603831.html
Richard.
rest_of_type_compilation (type, DECL_NAMESPACE_SCOPE_P
(defn));
}
diff --git a/gcc/testsuite/g++.dg/modules/enum-9_a.H
b/gcc/testsuite/g++.dg/modules/enum-9_a.H
new file mode 100644
index 000..fb7d10ad3b6
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/enum-9_a.H
@@ -0,0 +1,5 @@
+// PR c++/106848
+// { dg-additional-options -fmodule-header }
+// { dg-module-cmi {} }
+
+typedef enum memory_order { memory_order_seq_cst } memory_order;
diff --git a/gcc/testsuite/g++.dg/modules/enum-9_b.C
b/gcc/testsuite/g++.dg/modules/enum-9_b.C
new file mode 100644
index 000..63e81675d0a
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/enum-9_b.C
@@ -0,0 +1,6 @@
+// PR c++/106848
+// { dg-additional-options "-fmodules-ts -g" }
+
+import "enum-9_a.H";
+
+memory_order x = memory_order_seq_cst;
--
2.38.0.68.ge85701b4af
--
Nathan Sidwell
--
Nathan Sidwell
c++: Adjust synthetic template parm creation
We intend to mark synthetic template parameters (coming from use of auto
parms), as DECL_VIRTUAL_P. The API of process_template_parm is
awkwardly confusing, and we were marking the previous template parm
(unless this was the first parm). process_template_parm returns the list
of parms, when most (all?) users really want the newly-added final node.
That's a bigger change, so let's not do it right now. With this, we
correctly mark such synthetic parms DECL_VIRTUAL_P.
I fell over this trying to update template lambda mangling. The API of
process_template_parm is somewhat awkaward, as it returns the whole list, and we
end up doing a lot of tree_last calls, to get to the newly added node. I think
all (or nearly all?) uses could be adapted to it returning the newly added list
node, but I didn't want to go there right now.
nathan
--
Nathan SidwellFrom 43e654afeba484d75fbee080262a038c1da00ad5 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Tue, 25 Oct 2022 09:39:00 -0400
Subject: [PATCH] c++: Adjust synthetic template parm creation
We intend to mark synthetic template parameters (coming from use of auto
parms), as DECL_VIRTUAL_P. The API of process_template_parm is
awkwardly confusing, and we were marking the previous template parm
(unless this was the first parm). process_template_parm returns the list
of parms, when most (all?) users really want the newly-added final node.
That's a bigger change, so let's not do it right now. With this, we
correctly mark such synthetic parms DECL_VIRTUAL_P.
gcc/cp/
* parser.cc (synthesize_implicit_template_parm): Fix thinko about
mark the new parm DECL_VIRTUAL_P. Avoid unneccessary tree_last call.
---
gcc/cp/parser.cc | 26 +++---
1 file changed, 15 insertions(+), 11 deletions(-)
diff --git a/gcc/cp/parser.cc b/gcc/cp/parser.cc
index a39c5f0d24b..e685f190b3d 100644
--- a/gcc/cp/parser.cc
+++ b/gcc/cp/parser.cc
@@ -48996,12 +48996,11 @@ synthesize_implicit_template_parm (cp_parser *parser, tree constr)
tree proto = constr ? DECL_INITIAL (constr) : NULL_TREE;
tree synth_id = make_generic_type_name ();
- tree synth_tmpl_parm;
bool non_type = false;
/* Synthesize the type template parameter. */
gcc_assert(!proto || TREE_CODE (proto) == TYPE_DECL);
- synth_tmpl_parm = finish_template_type_parm (class_type_node, synth_id);
+ tree synth_tmpl_parm = finish_template_type_parm (class_type_node, synth_id);
if (become_template)
current_template_parms = tree_cons (size_int (current_template_depth + 1),
@@ -49016,22 +49015,27 @@ synthesize_implicit_template_parm (cp_parser *parser, tree constr)
node,
/*non_type=*/non_type,
/*param_pack=*/false);
+ // Process_template_parm returns the list of parms, and
+ // parser->implicit_template_parms holds the final node of the parm
+ // list. We really want to manipulate the newly appended element.
+ gcc_checking_assert (!parser->implicit_template_parms
+ || parser->implicit_template_parms == new_parm);
+ if (parser->implicit_template_parms)
+new_parm = TREE_CHAIN (new_parm);
+ gcc_checking_assert (!TREE_CHAIN (new_parm));
+
+ // Record the last implicit parm node
+ parser->implicit_template_parms = new_parm;
/* Mark the synthetic declaration "virtual". This is used when
comparing template-heads to determine if whether an abbreviated
function template is equivalent to an explicit template.
- Note that DECL_ARTIFICIAL is used elsewhere for template parameters. */
+ Note that DECL_ARTIFICIAL is used elsewhere for template
+ parameters. */
if (TREE_VALUE (new_parm) != error_mark_node)
DECL_VIRTUAL_P (TREE_VALUE (new_parm)) = true;
- // Chain the new parameter to the list of implicit parameters.
- if (parser->implicit_template_parms)
-parser->implicit_template_parms
- = TREE_CHAIN (parser->implicit_template_parms);
- else
-parser->implicit_template_parms = new_parm;
-
tree new_decl = get_local_decls ();
if (non_type)
/* Return the TEMPLATE_PARM_INDEX, not the PARM_DECL. */
@@ -49059,7 +49063,7 @@ synthesize_implicit_template_parm (cp_parser *parser, tree constr)
/* If the new parameter was constrained, we need to add that to the
constraints in the template parameter list. */
- if (tree req = TEMPLATE_PARM_CONSTRAINTS (tree_last (new_parm)))
+ if (tree req = TEMPLATE_PARM_CONSTRAINTS (new_parm))
{
tree reqs = TEMPLATE_PARMS_CONSTRAINTS (current_template_parms);
reqs = combine_constraint_expressions (reqs, req);
--
2.37.3
Re: [PATCH] c++: Fix ICE on g++.dg/modules/adl-3_c.C [PR107379]
On 10/27/22 04:17, Jakub Jelinek wrote: Hi! As mentioned in the PR, apparently my r13-2887 P1467R9 changes regressed these tests on powerpc64le-linux with IEEE quad by default. I believe my changes just uncovered a latent bug. The problem is that push_namespace calls find_namespace_slot, which does: tree *slot = DECL_NAMESPACE_BINDINGS (ns) ->find_slot_with_hash (name, name ? IDENTIFIER_HASH_VALUE (name) : 0, create_p ? INSERT : NO_INSERT); In the ns case, slot is non-NULL above with a binding_vector in it. Then pushdecl is called and this does: slot = find_namespace_slot (ns, name, ns == current_namespace); where ns == current_namespace (ns is :: and name is details) is true. So this again calls tree *slot = DECL_NAMESPACE_BINDINGS (ns) ->find_slot_with_hash (name, name ? IDENTIFIER_HASH_VALUE (name) : 0, create_p ? INSERT : NO_INSERT); but this time with create_p and so INSERT. At this point we reach if (insert == INSERT && m_size * 3 <= m_n_elements * 4) expand (); and when we are unlucky and the occupancy of the hash table just reached 3/4, expand () is called and the hash table is reallocated. But when that happens, it means the slot pointer in the pushdecl caller (push_namespace) points to freed memory and so any accesses to it in make_namespace_finish will be UB. that's unfortunate, oh well. The following patch fixes it by calling find_namespace_slot again even if it was non-NULL, just doesn't assert it is *slot == ns in that case (because it often is not). Bootstrapped/regtested on x86_64-linux and i686-linux, ok for trunk? ok. thanks nathan -- Nathan Sidwell
c++: Templated lambda mangling
(Explicitly) Templated lambdas have a different signature to
implicitly templated lambdas -- '[] (T) {}' is not the
same as '[](auto) {}'. This should be reflected in the mangling. The
ABI captures this as
https://github.com/itanium-cxx-abi/cxx-abi/issues/31, and clang has
implemented such additions.
It's relatively straight forwards to write out the non-synthetic
template parms, and note if we need to issue an ABI warning.
I did find a couple of bugs on the way -- one is a failure to parse thhe pack
expansion in :
inline auto l_var2 = [] (int (&...)[I]) {}
the other was clang miscounting substitutions,
https://github.com/llvm/llvm-project/issues/58631, always a good idea to have
multiple implementations :)
the remaining change to do is lambda sequence numbering, as that is affected by
lambda signature.
nathan
--
Nathan SidwellFrom bf6e972b65c56c615682f712f785d0f0541ac77b Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 24 Oct 2022 17:39:55 -0400
Subject: [PATCH] c++: Templated lambda mangling
(Explicitly) Templated lambdas have a different signature to
implicitly templated lambdas -- '[] (T) {}' is not the
same as '[](auto) {}'. This should be reflected in the mangling. The
ABI captures this as
https://github.com/itanium-cxx-abi/cxx-abi/issues/31, and clang has
implemented such additions.
It's relatively straight forwards to write out the non-synthetic
template parms, and note if we need to issue an ABI warning.
gcc/cp/
* mangle.cc (write_closure_template_head): New.
(write_closure_type_name): Call it.
gcc/testsuite/
* g++.dg/abi/lambda-ctx1-18.C: Adjust.
* g++.dg/abi/lambda-ctx1-18vs17.C: Adjust.
* g++.dg/abi/lambda-tpl1-17.C: New.
* g++.dg/abi/lambda-tpl1-18.C: New.
* g++.dg/abi/lambda-tpl1-18vs17.C: New.
* g++.dg/abi/lambda-tpl1.h: New.
---
gcc/cp/mangle.cc | 68 +++
gcc/testsuite/g++.dg/abi/lambda-ctx1-18.C | 4 +-
gcc/testsuite/g++.dg/abi/lambda-ctx1-18vs17.C | 4 +-
gcc/testsuite/g++.dg/abi/lambda-tpl1-17.C | 20 ++
gcc/testsuite/g++.dg/abi/lambda-tpl1-18.C | 25 +++
gcc/testsuite/g++.dg/abi/lambda-tpl1-18vs17.C | 16 +
gcc/testsuite/g++.dg/abi/lambda-tpl1.h| 59
7 files changed, 192 insertions(+), 4 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-tpl1-17.C
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-tpl1-18.C
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-tpl1-18vs17.C
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-tpl1.h
diff --git a/gcc/cp/mangle.cc b/gcc/cp/mangle.cc
index 1215463089b..e39621876ef 100644
--- a/gcc/cp/mangle.cc
+++ b/gcc/cp/mangle.cc
@@ -1727,6 +1727,66 @@ write_unnamed_type_name (const tree type)
write_compact_number (discriminator);
}
+// A template head, for templated lambdas.
+// ::= Tp* Ty
+// Tp* Tn
+// Tp* Tt E
+// New in ABI=18. Returns true iff we emitted anything -- used for ABI
+// version warning.
+
+static bool
+write_closure_template_head (tree tmpl)
+{
+ bool any = false;
+
+ // We only need one level of template parms
+ tree inner = INNERMOST_TEMPLATE_PARMS (DECL_TEMPLATE_PARMS (tmpl));
+
+ for (int ix = 0, len = TREE_VEC_LENGTH (inner); ix != len; ix++)
+{
+ tree parm = TREE_VEC_ELT (inner, ix);
+ if (parm == error_mark_node)
+ continue;
+ parm = TREE_VALUE (parm);
+
+ if (DECL_VIRTUAL_P (parm))
+ // A synthetic parm, we're done.
+ break;
+
+ any = true;
+ if (abi_version_at_least (18))
+ {
+ if (TREE_CODE (parm) == PARM_DECL
+ ? TEMPLATE_PARM_PARAMETER_PACK (DECL_INITIAL (parm))
+ : TEMPLATE_TYPE_PARAMETER_PACK (TREE_TYPE (parm)))
+ write_string ("Tp");
+
+ switch (TREE_CODE (parm))
+ {
+ default:
+ gcc_unreachable ();
+
+ case TYPE_DECL:
+ write_string ("Ty");
+ break;
+
+ case PARM_DECL:
+ write_string ("Tn");
+ write_type (TREE_TYPE (parm));
+ break;
+
+ case TEMPLATE_DECL:
+ write_string ("Tt");
+ write_closure_template_head (parm);
+ write_string ("E");
+ break;
+ }
+ }
+}
+
+ return any;
+}
+
/* ::= Ul E [ ] _
::= + # Parameter types or "v" if the lambda has no parameters */
@@ -1740,6 +1800,14 @@ write_closure_type_name (const tree type)
MANGLE_TRACE_TREE ("closure-type-name", type);
write_string ("Ul");
+
+ if (auto ti = maybe_template_info (fn))
+if (write_closure_template_head (TI_TEMPLATE (ti)))
+ // If there were any explicit template parms, we may need to
+ // issue a mangling diagnostic.
+ if (abi_warn_or_compat_version_crosses (18))
+ G.need_abi_warning = true;
+
write_method_parms (parms, /*method_p=*/1, fn);
write_char ('E');
write_compact_number (LAMBDA_EXPR_DISCRIMINATOR (lambda));
diff --git a/gcc/testsuite/g++.dg/abi/lambda-ctx1-18.C b/gcc/testsuite/g++.dg/abi/lambda-ctx1-18.C
index c1c9e274d7f..3dd68a4bed2 100644
--- a/gcc
c++: Reorganize per-scope lambda discriminators
We currently use a per-extra-scope counter to discriminate multiple
lambdas in a particular such scope. This is not ABI compliant. This
patch merely refactors the existing code to make it easier to drop in
a conformant mangling -- there's no functional change here. I rename
the LAMBDA_EXPR_DISCIMINATOR to LAMBDA_EXPR_SCOPE_ONLY_DISCRIMINATOR,
foreshadowing that there'll be a new discriminator. To provide ABI
warnings we'll need to calculate both, and that requires some
repacking of the lambda_expr's fields. Finally, although we end up
calling the discriminator setter and the scope recorder (nearly)
always consecutively, it's clearer to handle it as two separate
operations. That also allows us to remove the instantiation
special-case for a null extra-scope.
nathan
--
Nathan SidwellFrom 0122faae30fe1ad1dfa8c69f3d3f0428b996b600 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 31 Oct 2022 06:11:28 -0400
Subject: [PATCH] c++: Reorganize per-scope lambda discriminators
We currently use a per-extra-scope counter to discriminate multiple
lambdas in a particular such scope. This is not ABI compliant. This
patch merely refactors the existing code to make it easier to drop in
a conformant mangling -- there's no functional change here. I rename
the LAMBDA_EXPR_DISCIMINATOR to LAMBDA_EXPR_SCOPE_ONLY_DISCRIMINATOR,
foreshadowing that there'll be a new discriminator. To provide ABI
warnings we'll need to calculate both, and that requires some
repacking of the lambda_expr's fields. Finally, although we end up
calling the discriminator setter and the scope recorder (nearly)
always consecutively, it's clearer to handle it as two separate
operations. That also allows us to remove the instantiation
special-case for a null extra-scope.
gcc/cp/
* cp-tree.h (LAMBDA_EXPR_DISCRIMINATOR): Rename to ...
(LAMBDA_EXPR_SCOPE_ONLY_DISCRIMINATOR): ... here.
(struct tree_lambda_expr): Make default_capture_mode &
discriminator_scope bitfields.
(record_null_lambda_scope) Delete.
(record_lambda_scope_discriminator): Declare.
* lambda.cc (struct lambda_discriminator): New struct.
(lambda_scope, lambda_scope_stack): Adjust types.
(lambda_count): Delete.
(struct tree_int): Delete.
(start_lambda_scope, finish_lambda_scope): Adjust.
(record_lambda_scope): Only record the scope.
(record_lambda_scope_discriminator): New.
* mangle.cc (write_closure_type_name): Adjust.
* module.cc (trees_out::core_vals): Likewise,
(trees_in::core_vals): Likewise.
* parser.cc (cp_parser_lambda_expression): Call
record_lambda_scope_discriminator.
* pt.cc (tsubst_lambda_expr): Adjust record_lambda_scope caling. Call
record_lambda_scope_discriminator. Commonize control flow on tsubsting
the operator function.
libcc1/
* libcp1plugin.cc (plugin_start_closure): Adjust.
gcc/testsuite/
* g++.dg/abi/lambda-sig1-17.C: New.
* g++.dg/abi/lambda-sig1.h: New.
* g++.dg/cpp1y/lambda-mangle-1.C: Extracted to ...
* g++.dg/cpp1y/lambda-mangle-1.h: ... here.
* g++.dg/cpp1y/lambda-mangle-1-11.C: New
* g++.dg/cpp1y/lambda-mangle-1-17.C
---
gcc/cp/cp-tree.h | 17 +--
gcc/cp/lambda.cc | 114 +-
gcc/cp/mangle.cc | 2 +-
gcc/cp/module.cc | 4 +-
gcc/cp/parser.cc | 5 +-
gcc/cp/pt.cc | 50 +++-
gcc/testsuite/g++.dg/abi/lambda-sig1-17.C | 26
gcc/testsuite/g++.dg/abi/lambda-sig1.h| 42 +++
.../g++.dg/cpp1y/lambda-mangle-1-11.C | 25
.../g++.dg/cpp1y/lambda-mangle-1-17.C | 25
.../{lambda-mangle-1.C => lambda-mangle-1.h} | 3 +-
libcc1/libcp1plugin.cc| 2 +-
12 files changed, 208 insertions(+), 107 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-sig1-17.C
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-sig1.h
create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-mangle-1-11.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-mangle-1-17.C
rename gcc/testsuite/g++.dg/cpp1y/{lambda-mangle-1.C => lambda-mangle-1.h} (98%)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 6d84514e4c0..4c0bacb91da 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1500,9 +1500,10 @@ enum cp_lambda_default_capture_mode_type {
#define LAMBDA_EXPR_EXTRA_SCOPE(NODE) \
(((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->extra_scope)
-/* If EXTRA_SCOPE, this is the number of the lambda within that scope. */
-#define LAMBDA_EXPR_DISCRIMINATOR(NODE) \
- (((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->discriminator)
+/* Lambdas in the same extra scope might need a discriminating count.
+ This is a single per-scope count. */
+#define LAMBDA_EXPR_SCOPE_ONLY_DISCRIMINATOR(NODE) \
+ (((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->discriminator_scope)
/* During parsing of the lambda, a vector of capture proxies which nee
c++: per-scope, per-signature lambda discriminators
This implements ABI-compliant lambda discriminators. Not only do we
have per-scope counters, but we also distinguish by lambda signature.
Only lambdas with the same signature will need non-zero
discriminators. As the discriminator is signature-dependent, we have
to process the lambda function's declaration before we can determine
it. For templated and generic lambdas the signature is that of the
uninstantiated lambda -- not separate for each instantiation.
With this change, gcc and clang now produce the same lambda manglings
for all these testcases.
nathan
--
Nathan SidwellFrom 2b0e81d5cc2f7e1d773f6c502bd65b097f392675 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 31 Oct 2022 06:11:28 -0400
Subject: [PATCH] c++: per-scope, per-signature lambda discriminators
This implements ABI-compliant lambda discriminators. Not only do we
have per-scope counters, but we also distinguish by lambda signature.
Only lambdas with the same signature will need non-zero
discriminators. As the discriminator is signature-dependent, we have
to process the lambda function's declaration before we can determine
it. For templated and generic lambdas the signature is that of the
uninstantiated lambda -- not separate for each instantiation.
With this change, gcc and clang now produce the same lambda manglings
for all these testcases.
gcc/cp/
* cp-tree.h (LAMBDA_EXPR_SCOPE_SIG_DISCRIMINATOR): New.
(struct tree_lambda_expr): Add discriminator_sig bitfield.
(recrd_lambda_scope_sig_discriminator): Declare.
* lambda.cc (struct lambda_sig_count): New.
(lambda_discriminator): Add signature vector.
(start_lambda_scope): Adjust.
(compare_lambda_template_head, compare_lambda_sig): New.
(record_lambda_scope_sig_discriminator): New.
* mangle.cc (write_closure_type): Use the scope-sig discriminator for
ABI >= 18. Emit abi mangling warning if needed.
* module.cc (trees_out::core_vals): Stream the new discriminator.
(trees_in::core_vals): Likewise.
* parser.cc (cp_parser_lambda_declarator_opt): Call
record_lambda_scope_sig_discriminator.
* pt.cc (tsubst_lambda_expr): Likewise.
libcc1/
* libcp1plugin.cc (plugin_start_lambda_closure_class_type):
Initialize the per-scope, per-signature discriminator.
gcc/testsuite/
* g++.dg/abi/lambda-sig1-18.C: New.
* g++.dg/abi/lambda-sig1-18vs17.C: New.
* g++.dg/cpp1y/lambda-mangle-1-18.C: New.
---
gcc/cp/cp-tree.h | 7 +-
gcc/cp/lambda.cc | 148 +-
gcc/cp/mangle.cc | 8 +-
gcc/cp/module.cc | 2 +
gcc/cp/parser.cc | 1 +
gcc/cp/pt.cc | 1 +
gcc/testsuite/g++.dg/abi/lambda-sig1-18.C | 34
gcc/testsuite/g++.dg/abi/lambda-sig1-18vs17.C | 40 +
.../g++.dg/cpp1y/lambda-mangle-1-18.C | 26 +++
libcc1/libcp1plugin.cc| 1 +
10 files changed, 265 insertions(+), 3 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-sig1-18.C
create mode 100644 gcc/testsuite/g++.dg/abi/lambda-sig1-18vs17.C
create mode 100644 gcc/testsuite/g++.dg/cpp1y/lambda-mangle-1-18.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index 4c0bacb91da..d13bb3d4c0e 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -1501,9 +1501,12 @@ enum cp_lambda_default_capture_mode_type {
(((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->extra_scope)
/* Lambdas in the same extra scope might need a discriminating count.
- This is a single per-scope count. */
+ For ABI 17, we have single per-scope count, for ABI 18, we have
+ per-scope, per-signature numbering. */
#define LAMBDA_EXPR_SCOPE_ONLY_DISCRIMINATOR(NODE) \
(((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->discriminator_scope)
+#define LAMBDA_EXPR_SCOPE_SIG_DISCRIMINATOR(NODE) \
+ (((struct tree_lambda_expr *)LAMBDA_EXPR_CHECK (NODE))->discriminator_sig)
/* During parsing of the lambda, a vector of capture proxies which need
to be pushed once we're done processing a nested lambda. */
@@ -1533,6 +1536,7 @@ struct GTY (()) tree_lambda_expr
location_t locus;
enum cp_lambda_default_capture_mode_type default_capture_mode : 2;
unsigned discriminator_scope : 15; // Per-scope discriminator
+ unsigned discriminator_sig : 15; // Per-scope, per-signature discriminator
};
/* Non-zero if this template specialization has access violations that
@@ -7783,6 +7787,7 @@ extern void start_lambda_scope (tree decl);
extern void finish_lambda_scope (void);
extern void record_lambda_scope (tree lambda);
extern void record_lambda_scope_discriminator (tree lambda);
+extern void record_lambda_scope_sig_discriminator (tree lambda, tree fn);
extern tree start_lambda_function (tree fn, tree lambda_expr);
extern void finish_lambda_function (tree body);
extern bool regenerated_lambda_fn_p (tree);
diff --git a/gcc/cp/lambda.cc b/gcc/cp/lambda.cc
index d2673e2cee
Re: [PATCH v2] c++: Use in-process client when networking is disabled
On 11/3/22 05:37, Torbjörn SVENSSON wrote:
v1 -> v2:
Updated expression in bad-mapper-3.C
Ok for trunk?
---
Without the patch, the output for bad-mapper-3.C would be:
/src/gcc/gcc/testsuite/g++.dg/modules/bad-mapper-3.C:2:1: error: unknown
Compiled Module Interface: no such module
As this line is unexpected, the test case would fail.
The same problem can also be seen for g++.dg/modules/bad-mapper-2.C.
gcc/cp/ChangeLog:
* mapper-client.cc: Use in-process client when networking is
disabled.
gcc/testsuite/ChangeLog:
* g++.dg/modules/bad-mapper-3.C: Update dg-error pattern.
Tested on Windows with arm-none-eabi for Cortex-M3 in gcc-11 tree.
Co-Authored-By: Yvan ROUX
Signed-off-by: Torbjörn SVENSSON
---
gcc/cp/mapper-client.cc | 4
gcc/testsuite/g++.dg/modules/bad-mapper-3.C | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/gcc/cp/mapper-client.cc b/gcc/cp/mapper-client.cc
index fe9544b5ba4..4dcb3a03660 100644
--- a/gcc/cp/mapper-client.cc
+++ b/gcc/cp/mapper-client.cc
@@ -227,6 +227,8 @@ module_client::open_module_client (location_t loc, const
char *o,
int fd = -1;
#if CODY_NETWORKING
fd = Cody::OpenLocal (&errmsg, name.c_str () + 1);
+#else
+ errmsg = "CODY_NETWORKING disabled";
CODY_NETWORKING is implementor speak. I think just "disabled" is sufficient
here?
#endif
if (fd >= 0)
c = new module_client (fd, fd);
@@ -254,6 +256,8 @@ module_client::open_module_client (location_t loc, const
char *o,
int fd = -1;
#if CODY_NETWORKING
fd = Cody::OpenInet6 (&errmsg, name.c_str (), port);
+#else
+ errmsg = "CODY_NETWORKING disabled";
#endif
name[colon] = ':';
diff --git a/gcc/testsuite/g++.dg/modules/bad-mapper-3.C b/gcc/testsuite/g++.dg/modules/bad-mapper-3.C
index 9dab332ccb2..c91bb4e260c 100644
--- a/gcc/testsuite/g++.dg/modules/bad-mapper-3.C
+++ b/gcc/testsuite/g++.dg/modules/bad-mapper-3.C
@@ -1,6 +1,6 @@
// { dg-additional-options "-fmodules-ts
-fmodule-mapper=localhost:172477262" }
import unique3.bob;
-// { dg-error {failed connecting mapper 'localhost:172477262'} "" { target
*-*-* } 0 }
+// { dg-error {failed (connecting|CODY_NETWORKING disabled) mapper
'localhost:172477262'} "" { target *-*-* } 0 }
// { dg-prune-output "fatal error:" }
// { dg-prune-output "failed to read" }
// { dg-prune-output "compilation terminated" }
--
Nathan Sidwell
Re: [PATCH v2] c++: Use in-process client when networking is disabled
On 11/3/22 09:48, Torbjorn SVENSSON wrote:
Hello Nathan,
On 2022-11-03 14:13, Nathan Sidwell wrote:
On 11/3/22 05:37, Torbjörn SVENSSON wrote:
v1 -> v2:
Updated expression in bad-mapper-3.C
Ok for trunk?
---
Without the patch, the output for bad-mapper-3.C would be:
/src/gcc/gcc/testsuite/g++.dg/modules/bad-mapper-3.C:2:1: error: unknown
Compiled Module Interface: no such module
As this line is unexpected, the test case would fail.
The same problem can also be seen for g++.dg/modules/bad-mapper-2.C.
gcc/cp/ChangeLog:
* mapper-client.cc: Use in-process client when networking is
disabled.
gcc/testsuite/ChangeLog:
* g++.dg/modules/bad-mapper-3.C: Update dg-error pattern.
Tested on Windows with arm-none-eabi for Cortex-M3 in gcc-11 tree.
Co-Authored-By: Yvan ROUX
Signed-off-by: Torbjörn SVENSSON
---
gcc/cp/mapper-client.cc | 4
gcc/testsuite/g++.dg/modules/bad-mapper-3.C | 2 +-
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/gcc/cp/mapper-client.cc b/gcc/cp/mapper-client.cc
index fe9544b5ba4..4dcb3a03660 100644
--- a/gcc/cp/mapper-client.cc
+++ b/gcc/cp/mapper-client.cc
@@ -227,6 +227,8 @@ module_client::open_module_client (location_t loc, const
char *o,
int fd = -1;
#if CODY_NETWORKING
fd = Cody::OpenLocal (&errmsg, name.c_str () + 1);
+#else
+ errmsg = "CODY_NETWORKING disabled";
CODY_NETWORKING is implementor speak. I think just "disabled" is sufficient
here?
Ok for trunk if I change to just "disabled" here and in the other places below?
yes, thanks
Kind regards,
Torbjörn
#endif
if (fd >= 0)
c = new module_client (fd, fd);
@@ -254,6 +256,8 @@ module_client::open_module_client (location_t loc, const
char *o,
int fd = -1;
#if CODY_NETWORKING
fd = Cody::OpenInet6 (&errmsg, name.c_str (), port);
+#else
+ errmsg = "CODY_NETWORKING disabled";
#endif
name[colon] = ':';
diff --git a/gcc/testsuite/g++.dg/modules/bad-mapper-3.C
b/gcc/testsuite/g++.dg/modules/bad-mapper-3.C
index 9dab332ccb2..c91bb4e260c 100644
--- a/gcc/testsuite/g++.dg/modules/bad-mapper-3.C
+++ b/gcc/testsuite/g++.dg/modules/bad-mapper-3.C
@@ -1,6 +1,6 @@
// { dg-additional-options "-fmodules-ts
-fmodule-mapper=localhost:172477262" }
import unique3.bob;
-// { dg-error {failed connecting mapper 'localhost:172477262'} "" { target
*-*-* } 0 }
+// { dg-error {failed (connecting|CODY_NETWORKING disabled) mapper
'localhost:172477262'} "" { target *-*-* } 0 }
// { dg-prune-output "fatal error:" }
// { dg-prune-output "failed to read" }
// { dg-prune-output "compilation terminated" }
--
Nathan Sidwell
Re: [PATCH] c++: Allow module name to be a single letter on Windows
On 10/28/22 05:15, Torbjörn SVENSSON wrote:
On Windows, the ':' character is special and when the module name is
a single character, like 'A', then the flatname would be (for
example) 'A:Foo'. On Windows, 'A:Foo' is treated as an absolute
path by the module loader and is likely not found.
Without this patch, the test case pr98944_c.C fails with:
In module imported at /src/gcc/testsuite/g++.dg/modules/pr98944_b.C:7:1,
of module A:Foo, imported at /src/gcc/testsuite/g++.dg/modules/pr98944_c.C:7:
A:Internals: error: header module expected, module 'A:Internals' found
A:Internals: error: failed to read compiled module: Bad file data
A:Internals: note: compiled module file is 'gcm.cache/A-Internals.gcm'
In module imported at /src/gcc/testsuite/g++.dg/modules/pr98944_c.C:7:8:
A:Foo: error: failed to read compiled module: Bad import dependency
A:Foo: note: compiled module file is 'gcm.cache/A-Foo.gcm'
A:Foo: fatal error: returning to the gate for a mechanical issue
compilation terminated.
include/ChangeLog:
* filenames.h: Added IS_REAL_ABSOLUTE_PATH macro to check if
path is absolute and not semi-absolute on Windows.
Hm, this is unfortunate. The current IS_ABSOLUTE_PATH, is really 'not relative
to cwd', and even then that's untrue if the drive letter there is the drive
letter of cwd, right?
It's awkward to have a new macro for just this purpose and the new name isn't
very indicative of the difference to the current IS_ABSOLUTE_PATH.
Would it be better to not deal with drive letters here? How prevalent are they
these days in windows? Would something like
if (IS_DIR_SEPARATOR (ptr[ptr[0] == '.'])
suffice?
or, failing that perhaps put some explicit WINDOWS-specific #ifdef'd code there?
It's a real corner case.
nathan
gcc/cp/ChangeLog:
* module.cc: Use IS_REAL_ABSOLUTE_PATH macro.
Co-Authored-By: Yvan ROUX
Signed-off-by: Torbjörn SVENSSON
---
gcc/cp/module.cc| 2 +-
include/filenames.h | 4
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 9957df510e6..84680e183b7 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13958,7 +13958,7 @@ get_module (tree name, module_state *parent, bool
partition)
static module_state *
get_module (const char *ptr)
{
- if (ptr[0] == '.' ? IS_DIR_SEPARATOR (ptr[1]) : IS_ABSOLUTE_PATH (ptr))
+ if (ptr[0] == '.' ? IS_DIR_SEPARATOR (ptr[1]) : IS_REAL_ABSOLUTE_PATH (ptr))
/* A header name. */
return get_module (build_string (strlen (ptr), ptr));
diff --git a/include/filenames.h b/include/filenames.h
index 6c72c422edd..d04fccfed64 100644
--- a/include/filenames.h
+++ b/include/filenames.h
@@ -43,6 +43,7 @@ extern "C" {
# define HAS_DRIVE_SPEC(f) HAS_DOS_DRIVE_SPEC (f)
# define IS_DIR_SEPARATOR(c) IS_DOS_DIR_SEPARATOR (c)
# define IS_ABSOLUTE_PATH(f) IS_DOS_ABSOLUTE_PATH (f)
+# define IS_REAL_ABSOLUTE_PATH(f) IS_DOS_REAL_ABSOLUTE_PATH (f)
#else /* not DOSish */
# if defined(__APPLE__)
#ifndef HAVE_CASE_INSENSITIVE_FILE_SYSTEM
@@ -52,6 +53,7 @@ extern "C" {
# define HAS_DRIVE_SPEC(f) (0)
# define IS_DIR_SEPARATOR(c) IS_UNIX_DIR_SEPARATOR (c)
# define IS_ABSOLUTE_PATH(f) IS_UNIX_ABSOLUTE_PATH (f)
+# define IS_REAL_ABSOLUTE_PATH(f) IS_ABSOLUTE_PATH (f)
#endif
#define IS_DIR_SEPARATOR_1(dos_based, c)\
@@ -67,6 +69,8 @@ extern "C" {
#define IS_DOS_DIR_SEPARATOR(c) IS_DIR_SEPARATOR_1 (1, c)
#define IS_DOS_ABSOLUTE_PATH(f) IS_ABSOLUTE_PATH_1 (1, f)
+#define IS_DOS_REAL_ABSOLUTE_PATH(f) \
+ ((f)[0] && (f)[1] == ':' && ((f)[2] == '/' || (f)[2] == '\\'))
#define HAS_DOS_DRIVE_SPEC(f) HAS_DRIVE_SPEC_1 (1, f)
#define IS_UNIX_DIR_SEPARATOR(c) IS_DIR_SEPARATOR_1 (0, c)
--
Nathan Sidwell
Re: Where in C++ module streaming to handle a new bitfield added in "tree_decl_common"
On 8/2/22 10:44, Qing Zhao wrote:
Hi, Nathan,
I am adding a new bitfield “decl_not_flexarray” in “tree_decl_common”
(gcc/tree-core.h) for the new gcc feature -fstrict-flex-arrays.
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index ea9f281f1cc..458c6e6ceea 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1813,7 +1813,10 @@ struct GTY(()) tree_decl_common {
TYPE_WARN_IF_NOT_ALIGN. */
unsigned int warn_if_not_align : 6;
- /* 14 bits unused. */
+ /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */
+ unsigned int decl_not_flexarray : 1;
Is it possible to invert the meaning here -- set the flag if it /IS/ a
flexible array? negated flags can be confusing, and I see your patch
sets it to '!is_flexible_array (...)' anyway?
+
+ /* 13 bits unused. */
/* UID for points-to sets, stable over copying from inlining. */
unsigned int pt_uid;
(Please refer to the following for details:
https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598556.html
https://gcc.gnu.org/pipermail/gcc-patches/2022-July/598965.html
)
Richard mentioned the following:
"I've not seen it so you are probably missing it - the bit has to be
streamed in tree-streamer-{in,out}.cc to be usable from LTO. Possibly
C++ module streaming also needs to handle it.”
I have figured out that where to add the handling of the bit in
“tree-streamer-{in, out}.cc,
However, it’s quite difficult for me to locate where should I add the handling
of this new bit in
C++ module streaming, could you please help me on this?
add it in to trees_{in,out}::core_bools. You could elide streaming for
non-FIELD_DECL decls.
Hope that helps.
nathan
Thanks a lot for your help.
Qing
--
Nathan Sidwell
c++: Fix module line no testcase
Not all systems have the same injected headers, leading to line
location table differences that are immaterial to the test. Fix the
regexp more robustly.
nathan
--
Nathan SidwellFrom af088b32def1c56538f0f3aaea16f013e9292d64 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 15 Aug 2022 07:19:36 -0700
Subject: [PATCH] c++: Fix module line no testcase
Not all systems have the same injected headers, leading to line
location table differences that are immaterial to the test. Fix the
regexp more robustly.
gcc/testsuite/
* g++.dg/modules/loc-prune-4.C: Adjust regexp
---
gcc/testsuite/g++.dg/modules/loc-prune-4.C | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/gcc/testsuite/g++.dg/modules/loc-prune-4.C b/gcc/testsuite/g++.dg/modules/loc-prune-4.C
index 765c378e51e..aa8f248b52b 100644
--- a/gcc/testsuite/g++.dg/modules/loc-prune-4.C
+++ b/gcc/testsuite/g++.dg/modules/loc-prune-4.C
@@ -18,5 +18,5 @@ int baz (int);
// { dg-final { scan-lang-dump {Ordinary maps:2 locs:12288 range_bits:5} module } }
// { dg-final { scan-lang-dump { 1 source file names\n Source file...=[^\n]*loc-prune-4.C\n} module } }
-// { dg-final { scan-lang-dump { Span:0 ordinary \[2.\+12288,\+4096\)->\[0,\+4096\)} module } }
-// { dg-final { scan-lang-dump { Span:1 ordinary \[2.\+40960,\+8192\)->\[4096,\+8192\)} module } }
+// { dg-final { scan-lang-dump { Span:0 ordinary \[[0-9]+\+12288,\+4096\)->\[0,\+4096\)} module } }
+// { dg-final { scan-lang-dump { Span:1 ordinary \[[0-9]+\+40960,\+8192\)->\[4096,\+8192\)} module } }
--
2.30.2
Re: Where in C++ module streaming to handle a new bitfield added in "tree_decl_common"
On 8/15/22 10:03, Richard Biener wrote:
On Mon, Aug 15, 2022 at 3:29 PM Nathan Sidwell via Gcc-patches
wrote:
On 8/2/22 10:44, Qing Zhao wrote:
Hi, Nathan,
I am adding a new bitfield “decl_not_flexarray” in “tree_decl_common”
(gcc/tree-core.h) for the new gcc feature -fstrict-flex-arrays.
diff --git a/gcc/tree-core.h b/gcc/tree-core.h
index ea9f281f1cc..458c6e6ceea 100644
--- a/gcc/tree-core.h
+++ b/gcc/tree-core.h
@@ -1813,7 +1813,10 @@ struct GTY(()) tree_decl_common {
TYPE_WARN_IF_NOT_ALIGN. */
unsigned int warn_if_not_align : 6;
- /* 14 bits unused. */
+ /* In FIELD_DECL, this is DECL_NOT_FLEXARRAY. */
+ unsigned int decl_not_flexarray : 1;
Is it possible to invert the meaning here -- set the flag if it /IS/ a
flexible array? negated flags can be confusing, and I see your patch
sets it to '!is_flexible_array (...)' anyway?
The issue is it's consumed by the middle-end but set by a single (or two)
frontends and the conservative setting is having the bit not set. That works
nicely together with touching just the frontends that want stricter behavior
than currently ...
Makes sense, but is the comment incomplete? I'm guessing this flag is
for FIELD_DECLs /of array type/, and not just any old FIELD_DECL? After
all a field of type int is not a flexible array, but presumably doesn't
need this flag setting?
nathan
--
Nathan Sidwell
Re: Ping [PATCH V2] libcpp: Optimize #pragma once with a hash table [PR58770]
On 8/19/22 16:27, Paul Hollinsky wrote:
Hi all,
Would love some feedback on this patch!
Thanks,
Paul
On Mon, Aug 01, 2022 at 05:18:40AM +, Paul Hollinsky wrote:
Rather than traversing the all_files linked list for every include,
this factors out the quick idempotency checks (modification time
and size) to be the keys in a hash table so we can find matching
files quickly.
I'm probably confused, but why is the existing idempotency logic
insufficiently optimal? Can it be improved directly?
WRT using modification time as a key. I don't see how that provides
sufficient additional randomization to file size? Groups of headers are
often installed with the same mtime.
The hash table value type is a linked list, in case more than one
file matches the quick check.
The table is only built if a once-only file is seen, so include
guard performance is not affecte
My laptop would previously complete Ricardo's benchmark from the
PR in ~1.1s using #pragma once, and ~0.35s using include guards.
After this change, both benchmarks now complete in ~0.35s. I did
have to randomize the modification dates on the benchmark headers
so the files did not all end up in the same hash table list, but
that would likely not come up outside of the contrived benchmark.
I bootstrapped and ran the testsuite on x86_64 Darwin, as well as
ppc64le and aarch64 Linux.
libcpp/ChangeLog:
PR preprocessor/58770
* internal.h: Add hash table for #pragma once
* files.cc: Optimize #pragma once with the hash table
Signed-off-by: Paul Hollinsky
---
libcpp/files.cc | 116 +++---
libcpp/internal.h | 3 ++
2 files changed, 112 insertions(+), 7 deletions(-)
diff --git a/libcpp/files.cc b/libcpp/files.cc
index 24208f7b0f8..d4ffd77578e 100644
--- a/libcpp/files.cc
+++ b/libcpp/files.cc
@@ -167,6 +167,33 @@ struct file_hash_entry_pool
struct cpp_file_hash_entry pool[FILE_HASH_POOL_SIZE];
};
+/* A set of attributes designed to quickly identify obviously different files
+ in a hashtable. Just in case there are collisions, we still maintain a
+ list. These sub-lists can then be checked for #pragma once rather than
+ interating through all_files. */
+struct file_quick_idempotency_attrs
+{
+ file_quick_idempotency_attrs(const _cpp_file *f)
+: mtime(f->st.st_mtime), size(f->st.st_size) {}
+
+ time_t mtime;
+ off_t size;
+
+ static hashval_t hash (/* _cpp_file* */ const void *p);
+};
+
+/* Sub-list of very similar files kept in a hashtable to check for #pragma
+ once. */
+struct file_sublist
+{
+ _cpp_file *f;
+ file_sublist *next;
+
+ static int eq (/* _cpp_file* */ const void *p,
+/* file_sublist* */ const void *q);
+ static void del (/* file_sublist* */ void *p);
+};
+
static bool open_file (_cpp_file *file);
static bool pch_open_file (cpp_reader *pfile, _cpp_file *file,
bool *invalid_pch);
@@ -849,17 +876,17 @@ has_unique_contents (cpp_reader *pfile, _cpp_file *file,
bool import,
if (!pfile->seen_once_only)
return true;
- /* We may have read the file under a different name. Look
- for likely candidates and compare file contents to be sure. */
- for (_cpp_file *f = pfile->all_files; f; f = f->next_file)
+ /* We may have read the file under a different name. We've kept
+ similar looking files in this lists under this hash table, so
+ check those more thoroughly. */
+ void* ent = htab_find(pfile->pragma_once_files, file);
+ for (file_sublist *e = static_cast (ent); e; e = e->next)
{
+ _cpp_file *f = e->f;
if (f == file)
continue; /* It'sa me! */
- if ((import || f->once_only)
- && f->err_no == 0
- && f->st.st_mtime == file->st.st_mtime
- && f->st.st_size == file->st.st_size)
+ if ((import || f->once_only) && f->err_no == 0)
{
_cpp_file *ref_file;
@@ -895,6 +922,38 @@ has_unique_contents (cpp_reader *pfile, _cpp_file *file, bool import,
return true;
}
+/* Add the given file to the #pragma once table so it can be
+ quickly identified and excluded the next time it's seen. */
+static void
+update_pragma_once_table (cpp_reader *pfile, _cpp_file *file)
+{
+ void **slot = htab_find_slot (pfile->pragma_once_files, file, INSERT);
+ if (slot)
+{
+ if (!*slot)
+ *slot = xcalloc (1, sizeof (file_sublist));
+
+ file_sublist *e = static_cast (*slot);
+ while (e->f)
+ {
+ if (!e->next)
+ {
+ void *new_sublist = xcalloc(1, sizeof (file_sublist));
+ e->next = static_cast (new_sublist);
+ }
+ e = e->next;
+ }
+
+ e->f = file;
+}
+ else
+{
+ cpp_error (pfile, CPP_DL_ERROR,
+"Unable to create #pragma once table space for %s",
+_cpp_get_file_name (file));
+}
+}
+
/* Place the file referenced by FILE into a new buffer on th
Re: Ping [PATCH V2] libcpp: Optimize #pragma once with a hash table [PR58770]
On 8/22/22 13:39, Paul Hollinsky wrote:
On Mon, Aug 22, 2022 at 09:19:29AM -0400, Nathan Sidwell wrote:
On 8/19/22 16:27, Paul Hollinsky wrote:
Hi all,
Would love some feedback on this patch!
Thanks,
Paul
On Mon, Aug 01, 2022 at 05:18:40AM +, Paul Hollinsky wrote:
Rather than traversing the all_files linked list for every include,
this factors out the quick idempotency checks (modification time
and size) to be the keys in a hash table so we can find matching
files quickly.
I'm probably confused, but why is the existing idempotency logic
insufficiently optimal? Can it be improved directly?
The idempotency logic breaks down to a couple integer comparisons, I doubt
it can be made much faster. Usually a file will be skipped after the very
first idempotency check. The reason it's suboptimal right now is that it's
O(n^2) behavior -- for each include that's seen we follow the entire linked
list and do those integeger comparisons.
Traversing a list is O(N) -- why are you encountering N^2 here? (Are
you summing over all includes?)
The cost of hashing, performing a lookup against the hash table, and then
traversing the list of results (any hash collisions) is much lower than
traversing the full list of includes. I suspect that has something to do
with cache locality, or lack thereof, when accessing each member of the
linked list. It's likely also just far fewer instructions executed.
I don't see why a hash table cannot be applied to all idempotent files,
not just #import/pragma once.
WRT using modification time as a key. I don't see how that provides
sufficient additional randomization to file size? Groups of headers are
often installed with the same mtime.
This is a fair point, and I was going to say something here about the
codebases being compiled, but git also doesn't preserve mtime.
The real reason mtime is because that's what was done in the previous
implementation, and I felt it best not to mess with.
But, if there are better attributes that could be used without adding
much overhead we could use those instead. I was personally worried that
reading the file to perform any sort of hash would be expensive.
doing anything other than compare the pathname is going to mean going to
disk. That's likely cached by the OS from the first read. I suppose
one could encounter horrible LRU behaviour, but you're probably going to
lose in that case anyway. Hm, why not just key on the inode/drive
number? that way one even avoids the inode read (oh, I think stat isn't
that fine grained is it)
I suppose one could have two hashtables -- one keyed by pathname, which
one could check before the inode, which would deal with the majority of
non-symlink-farm cases. Then one keyed by inode/m_time or whatever to
deal with symlinks.
nathan
In any case, thank you very much for the feedback!
--Paul
The hash table value type is a linked list, in case more than one
file matches the quick check.
The table is only built if a once-only file is seen, so include
guard performance is not affecte
My laptop would previously complete Ricardo's benchmark from the
PR in ~1.1s using #pragma once, and ~0.35s using include guards.
After this change, both benchmarks now complete in ~0.35s. I did
have to randomize the modification dates on the benchmark headers
so the files did not all end up in the same hash table list, but
that would likely not come up outside of the contrived benchmark.
I bootstrapped and ran the testsuite on x86_64 Darwin, as well as
ppc64le and aarch64 Linux.
libcpp/ChangeLog:
PR preprocessor/58770
* internal.h: Add hash table for #pragma once
* files.cc: Optimize #pragma once with the hash table
Signed-off-by: Paul Hollinsky
---
libcpp/files.cc | 116 +++---
libcpp/internal.h | 3 ++
2 files changed, 112 insertions(+), 7 deletions(-)
diff --git a/libcpp/files.cc b/libcpp/files.cc
index 24208f7b0f8..d4ffd77578e 100644
--- a/libcpp/files.cc
+++ b/libcpp/files.cc
@@ -167,6 +167,33 @@ struct file_hash_entry_pool
struct cpp_file_hash_entry pool[FILE_HASH_POOL_SIZE];
};
+/* A set of attributes designed to quickly identify obviously different files
+ in a hashtable. Just in case there are collisions, we still maintain a
+ list. These sub-lists can then be checked for #pragma once rather than
+ interating through all_files. */
+struct file_quick_idempotency_attrs
+{
+ file_quick_idempotency_attrs(const _cpp_file *f)
+: mtime(f->st.st_mtime), size(f->st.st_size) {}
+
+ time_t mtime;
+ off_t size;
+
+ static hashval_t hash (/* _cpp_file* */ const void *p);
+};
+
+/* Sub-list of very similar files kept in a hashtable to check for #pragma
+ once. */
+struct file_sublist
+{
+ _cpp_file *f;
+ file_sublist *next;
+
+ static int eq (/* _cpp_file* */ const void *p,
+/* file_sublist* */ const void *q);
+ static void del (/* file_sublist* */ void *p);
+};
+
c++: refactor duplicate decls
A couple of paths in duplicate decls dealing with templates and
builtins were overly complicated. Fixing thusly.
gcc/cp/
* decl.c (duplicate_decls): Refactor some template & builtin
handling.
applying to trunk
nathan
--
Nathan Sidwell : Facebook
diff --git i/gcc/cp/decl.c w/gcc/cp/decl.c
index 39f56b81275..3846e823671 100644
--- i/gcc/cp/decl.c
+++ w/gcc/cp/decl.c
@@ -2471,22 +2471,27 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden)
DECL_NOT_REALLY_EXTERN (newdecl) |= DECL_NOT_REALLY_EXTERN (olddecl);
DECL_COMDAT (newdecl) |= DECL_COMDAT (olddecl);
}
- DECL_TEMPLATE_INSTANTIATED (newdecl)
- |= DECL_TEMPLATE_INSTANTIATED (olddecl);
- DECL_ODR_USED (newdecl) |= DECL_ODR_USED (olddecl);
- /* If the OLDDECL is an instantiation and/or specialization,
- then the NEWDECL must be too. But, it may not yet be marked
- as such if the caller has created NEWDECL, but has not yet
- figured out that it is a redeclaration. */
- if (!DECL_USE_TEMPLATE (newdecl))
- DECL_USE_TEMPLATE (newdecl) = DECL_USE_TEMPLATE (olddecl);
+ if (TREE_CODE (newdecl) != TYPE_DECL)
+ {
+ DECL_TEMPLATE_INSTANTIATED (newdecl)
+ |= DECL_TEMPLATE_INSTANTIATED (olddecl);
+ DECL_ODR_USED (newdecl) |= DECL_ODR_USED (olddecl);
+
+ /* If the OLDDECL is an instantiation and/or specialization,
+ then the NEWDECL must be too. But, it may not yet be marked
+ as such if the caller has created NEWDECL, but has not yet
+ figured out that it is a redeclaration. */
+ if (!DECL_USE_TEMPLATE (newdecl))
+ DECL_USE_TEMPLATE (newdecl) = DECL_USE_TEMPLATE (olddecl);
+
+ DECL_INITIALIZED_IN_CLASS_P (newdecl)
+ |= DECL_INITIALIZED_IN_CLASS_P (olddecl);
+ }
/* Don't really know how much of the language-specific
values we should copy from old to new. */
DECL_IN_AGGR_P (newdecl) = DECL_IN_AGGR_P (olddecl);
- DECL_INITIALIZED_IN_CLASS_P (newdecl)
- |= DECL_INITIALIZED_IN_CLASS_P (olddecl);
if (LANG_DECL_HAS_MIN (newdecl))
{
@@ -2646,19 +2651,20 @@ duplicate_decls (tree newdecl, tree olddecl, bool hiding, bool was_hidden)
if (DECL_BUILT_IN_CLASS (newdecl) == BUILT_IN_NORMAL)
{
enum built_in_function fncode = DECL_FUNCTION_CODE (newdecl);
- switch (fncode)
+ if (builtin_decl_explicit_p (fncode))
{
- /* If a compatible prototype of these builtin functions
- is seen, assume the runtime implements it with the
- expected semantics. */
- case BUILT_IN_STPCPY:
- if (builtin_decl_explicit_p (fncode))
- set_builtin_decl_implicit_p (fncode, true);
- break;
- default:
- if (builtin_decl_explicit_p (fncode))
- set_builtin_decl_declared_p (fncode, true);
- break;
+ /* A compatible prototype of these builtin functions
+ is seen, assume the runtime implements it with
+ the expected semantics. */
+ switch (fncode)
+ {
+ case BUILT_IN_STPCPY:
+ set_builtin_decl_implicit_p (fncode, true);
+ break;
+ default:
+ set_builtin_decl_declared_p (fncode, true);
+ break;
+ }
}
copy_attributes_to_builtin (newdecl);
Re: [PATCH] c++tools: Fix compilation of server.cc on hpux
On 1/7/23 14:12, John David Anglin wrote: Tested on trunk and gcc-12 with hppa64-hp-hpux11.11. ah, I see that is the use that was unprotected, ok. Okay? Dave --- Fix compilation of server.cc on hpux. Select and FD_ISSET are declared in sys/time.h on most versions of hpux. As a result, HAVE_PSELECT and HAVE_SELECT can be 0. 2023-01-07 John David Anglin c++tools/ChangeLog: PR c++tools/107616 * server.cc (server): Don't call FD_ISSET when HAVE_PSELECT and HAVE_SELECT are zero. diff --git a/c++tools/server.cc b/c++tools/server.cc index 00154a05925..693aec6820a 100644 --- a/c++tools/server.cc +++ b/c++tools/server.cc @@ -753,8 +753,10 @@ server (bool ipv6, int sock_fd, module_resolver *resolver) } } +#if defined (HAVE_PSELECT) || defined (HAVE_SELECT) if (active < 0 && sock_fd >= 0 && FD_ISSET (sock_fd, &readers)) active = -1; +#endif } if (active >= 0) -- Nathan Sidwell
testsuite:missed testcase
I discovered I'd missed applying a testcase when fixing up the EOF token
location a while back.
gcc/testsuite/
* c-c++-common/cpp/pragma-eof.c: New
--
Nathan Sidwell : Facebook
diff --git c/gcc/testsuite/c-c++-common/cpp/pragma-eof.c w/gcc/testsuite/c-c++-common/cpp/pragma-eof.c
new file mode 100644
index 000..c72be8042b5
--- /dev/null
+++ w/gcc/testsuite/c-c++-common/cpp/pragma-eof.c
@@ -0,0 +1,6 @@
+/* { dg-additional-options -fopenmp } */
+
+/* { dg-error "expected" "" { target *-*-* } 6 } */
+/* Make sure we see pragma_eol even though lacking new line. *
+/* no newline at end of file. */
+#pragma omp parallel
\ No newline at end of file
preprocessor: line-map tidying
I found the linemap logic dealing with running out of column numbers
confusing. There's no need for completely separate code blocks there,
as we can rely on the masking operations working all the way down to
zero bits. The two binary searches for linemap lookups could do with
modernization of placing the var decls at their initialization point.
(These two searches work in opposite directions, and while lower_bound
would work there, the caching got in the way and I decided to be
conservative.)
libcpp/
* line-map.c (linemap_add): Simplify column overflow
calculation.
Add comment about range and column bit init.
(linemap_ordinary_map_lookup): Refactor for RAII
(linemap_macro_map_lookup): Likewise.
pushed
--
Nathan Sidwell : Facebook
diff --git i/libcpp/line-map.c w/libcpp/line-map.c
index 8a390d0857b..a8d52861dee 100644
--- i/libcpp/line-map.c
+++ w/libcpp/line-map.c
@@ -462,17 +462,12 @@ linemap_add (line_maps *set, enum lc_reason reason,
{
/* Generate a start_location above the current highest_location.
If possible, make the low range bits be zero. */
- location_t start_location;
- if (set->highest_location < LINE_MAP_MAX_LOCATION_WITH_COLS)
-{
- start_location = set->highest_location + (1 << set->default_range_bits);
- if (set->default_range_bits)
- start_location &= ~((1 << set->default_range_bits) - 1);
- linemap_assert (0 == (start_location
- & ((1 << set->default_range_bits) - 1)));
-}
- else
-start_location = set->highest_location + 1;
+ location_t start_location = set->highest_location + 1;
+ unsigned range_bits = 0;
+ if (start_location < LINE_MAP_MAX_LOCATION_WITH_COLS)
+range_bits = set->default_range_bits;
+ start_location += (1 << range_bits) - 1;
+ start_location &= ~((1 << range_bits) - 1);
linemap_assert (!LINEMAPS_ORDINARY_USED (set)
|| (start_location
@@ -537,8 +532,9 @@ linemap_add (line_maps *set, enum lc_reason reason,
map->to_file = to_file;
map->to_line = to_line;
LINEMAPS_ORDINARY_CACHE (set) = LINEMAPS_ORDINARY_USED (set) - 1;
- map->m_column_and_range_bits = 0;
- map->m_range_bits = 0;
+ /* Do not store range_bits here. That's readjusted in
+ linemap_line_start. */
+ map->m_range_bits = map->m_column_and_range_bits = 0;
set->highest_location = start_location;
set->highest_line = start_location;
set->max_column_hint = 0;
@@ -954,19 +950,16 @@ linemap_lookup (const line_maps *set, location_t line)
static const line_map_ordinary *
linemap_ordinary_map_lookup (const line_maps *set, location_t line)
{
- unsigned int md, mn, mx;
- const line_map_ordinary *cached, *result;
-
if (IS_ADHOC_LOC (line))
line = get_location_from_adhoc_loc (set, line);
if (set == NULL || line < RESERVED_LOCATION_COUNT)
return NULL;
- mn = LINEMAPS_ORDINARY_CACHE (set);
- mx = LINEMAPS_ORDINARY_USED (set);
+ unsigned mn = LINEMAPS_ORDINARY_CACHE (set);
+ unsigned mx = LINEMAPS_ORDINARY_USED (set);
- cached = LINEMAPS_ORDINARY_MAP_AT (set, mn);
+ const line_map_ordinary *cached = LINEMAPS_ORDINARY_MAP_AT (set, mn);
/* We should get a segfault if no line_maps have been added yet. */
if (line >= MAP_START_LOCATION (cached))
{
@@ -981,7 +974,7 @@ linemap_ordinary_map_lookup (const line_maps *set, location_t line)
while (mx - mn > 1)
{
- md = (mn + mx) / 2;
+ unsigned md = (mn + mx) / 2;
if (MAP_START_LOCATION (LINEMAPS_ORDINARY_MAP_AT (set, md)) > line)
mx = md;
else
@@ -989,7 +982,7 @@ linemap_ordinary_map_lookup (const line_maps *set, location_t line)
}
LINEMAPS_ORDINARY_CACHE (set) = mn;
- result = LINEMAPS_ORDINARY_MAP_AT (set, mn);
+ const line_map_ordinary *result = LINEMAPS_ORDINARY_MAP_AT (set, mn);
linemap_assert (line >= MAP_START_LOCATION (result));
return result;
}
@@ -1002,21 +995,18 @@ linemap_ordinary_map_lookup (const line_maps *set, location_t line)
static const line_map_macro *
linemap_macro_map_lookup (const line_maps *set, location_t line)
{
- unsigned int md, mn, mx;
- const struct line_map_macro *cached, *result;
-
if (IS_ADHOC_LOC (line))
line = get_location_from_adhoc_loc (set, line);
linemap_assert (line >= LINEMAPS_MACRO_LOWEST_LOCATION (set));
- if (set == NULL)
+ if (set == NULL)
return NULL;
- mn = LINEMAPS_MACRO_CACHE (set);
- mx = LINEMAPS_MACRO_USED (set);
- cached = LINEMAPS_MACRO_MAP_AT (set, mn);
-
+ unsigned mn = LINEMAPS_MACRO_CACHE (set);
+ unsigned mx = LINEMAPS_MACRO_USED (set);
+ const struct line_map_macro *cached = LINEMAPS_MACRO_MAP_AT (set, mn);
+
if (line >= MAP_START_LOCATION (cached))
{
if (mn == 0 || line < MAP_START_LOCATION (&cached[-1]))
@@ -1027,7 +1017,7 @@ linemap_macro_map_lookup (const line_maps *set, location_t line)
while (mn < mx)
{
- md = (mx + mn) / 2;
+ unsigned md = (mx + mn) / 2;
if (MAP_START_LOCATION (LINEMA
C++: Template lambda mangling testcases
I found some additional cases when working on the demangler. May as
well check their mangling. Since I managed to confuse myself.
nathan
--
Nathan SidwellFrom 51d567d4d15e78b42d2ca83f229c98fff2aec9fa Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 7 Nov 2022 11:08:21 -0500
Subject: [PATCH] C++: Template lambda mangling testcases
I found some additional cases when working on the demangler. May as
well check their mangling.
gcc/testsuite/
* g++.dg/abi/lambda-tpl1.h: Add more cases.
* g++.dg/abi/lambda-tpl1-17.C: Add checks.
* g++.dg/abi/lambda-tpl1-18.C: Likewise.
* g++.dg/abi/lambda-tpl1-18vs17.C: Likewise.
---
gcc/testsuite/g++.dg/abi/lambda-tpl1-17.C | 6 ++
gcc/testsuite/g++.dg/abi/lambda-tpl1-18.C | 6 ++
gcc/testsuite/g++.dg/abi/lambda-tpl1-18vs17.C | 5 +
gcc/testsuite/g++.dg/abi/lambda-tpl1.h| 11 +++
4 files changed, 28 insertions(+)
diff --git a/gcc/testsuite/g++.dg/abi/lambda-tpl1-17.C b/gcc/testsuite/g++.dg/abi/lambda-tpl1-17.C
index b61aaf98cd0..010f6222151 100644
--- a/gcc/testsuite/g++.dg/abi/lambda-tpl1-17.C
+++ b/gcc/testsuite/g++.dg/abi/lambda-tpl1-17.C
@@ -18,3 +18,9 @@
// { dg-final { scan-assembler {_ZNK6l_var3MUlRT_IJXspT0__clI1XJLi1ELi2ELi3DaS1_:} } }
// { dg-final { scan-assembler {_ZNK6l_var4MUlR1YIJDpT_EEE_clIJ1US6_EEEDaS3_:} } }
// { dg-final { scan-assembler {_ZZ2FnILi1EEvvENKUlT_E_clIiEEDaS0_:} } }
+
+// { dg-final { scan-assembler {_ZZ1fvENKUlT_E_clIcLc0EEEDaS_:} } }
+// { dg-final { scan-assembler {_ZZ1fvENKUlT_E_clIiLi0EEEDaS_:} } }
+// { dg-final { scan-assembler {_ZZZ1fvENKUlT_E_clIcLc0EEEDaS_ENKUlcS_E_clIcEEDacS_:} } }
+// { dg-final { scan-assembler {_ZZZ1fvENKUlT_E_clIiLi0EEEDaS_ENKUliS_E_clIiEEDaiS_:} } }
+// { dg-final { scan-assembler {_ZZ1fvENKUlP1UIT_Lj0EEPS_IiLj0EEE0_clIcEEDaS2_S4_:} } }
diff --git a/gcc/testsuite/g++.dg/abi/lambda-tpl1-18.C b/gcc/testsuite/g++.dg/abi/lambda-tpl1-18.C
index dbeea407651..66cce9aa266 100644
--- a/gcc/testsuite/g++.dg/abi/lambda-tpl1-18.C
+++ b/gcc/testsuite/g++.dg/abi/lambda-tpl1-18.C
@@ -23,3 +23,9 @@
// https://github.com/llvm/llvm-project/issues/58631
// { dg-final { scan-assembler {_ZZ2FnILi1EEvvENKUlTyT_E_clIiEEDaS0_:} } }
+
+// { dg-final { scan-assembler {_ZZ1fvENKUlTyTnT_S_E_clIcLc0EEEDaS_:} } }
+// { dg-final { scan-assembler {_ZZ1fvENKUlTyTnT_S_E_clIiLi0EEEDaS_:} } }
+// { dg-final { scan-assembler {_ZZZ1fvENKUlTyTnT_S_E_clIcLc0EEEDaS_ENKUlTycS_E_clIcEEDacS_:} } }
+// { dg-final { scan-assembler {_ZZZ1fvENKUlTyTnT_S_E_clIiLi0EEEDaS_ENKUlTyiS_E_clIiEEDaiS_:} } }
+// { dg-final { scan-assembler {_ZZ1fvENKUlTyP1UIT_Lj0EEPS_IiLj0EEE_clIcEEDaS2_S4_:} } }
diff --git a/gcc/testsuite/g++.dg/abi/lambda-tpl1-18vs17.C b/gcc/testsuite/g++.dg/abi/lambda-tpl1-18vs17.C
index 8bead7393c7..6489db9e442 100644
--- a/gcc/testsuite/g++.dg/abi/lambda-tpl1-18vs17.C
+++ b/gcc/testsuite/g++.dg/abi/lambda-tpl1-18vs17.C
@@ -14,3 +14,8 @@
// { dg-regexp {[^\n]*lambda-tpl1.h:[:0-9]* warning: the mangled name [^\n]* \('_ZNK10l_tpl_autoMUlT_T0_E_clIiiEEDaS_S0_'\) and '-fabi-version=18' \('_ZNK10l_tpl_autoMUlTyT_T0_E_clIiiEEDaS0_S1_'\) [^\n]*\n} }
// { dg-regexp {[^\n]*lambda-tpl1.h:[:0-9]* warning: the mangled name [^\n]* \('_ZNK5l_tplMUlT_E_clIiEEDaS_'\) and '-fabi-version=18' \('_ZNK5l_tplMUlTyT_E_clIiEEDaS0_'\) [^\n]*\n} }
// { dg-regexp {[^\n]*lambda-tpl1.h:[:0-9]* warning: the mangled name [^\n]* \('_ZNK6l_autoMUlT_E_clIiEEDaS_'\) and '-fabi-version=18' \('_ZNK6l_autoMUlT_E_clIiEEDaS0_'\) [^\n]*\n} }
+// { dg-regexp {[^\n]*lambda-tpl1.h:[:0-9]* warning: the mangled name [^\n]* \('_ZZ1fvENKUlT_E_clIiLi0EEEDaS_'\) and '-fabi-version=18' \('_ZZ1fvENKUlTyTnT_S_E_clIiLi0EEEDaS_'\) [^\n]*\n} }
+// { dg-regexp {[^\n]*lambda-tpl1.h:[:0-9]* warning: the mangled name [^\n]* \('_ZZZ1fvENKUlT_E_clIiLi0EEEDaS_ENKUliS_E_clIiEEDaiS_'\) and '-fabi-version=18' \('_ZZZ1fvENKUlTyTnT_S_E_clIiLi0EEEDaS_ENKUlTyiS_E_clIiEEDaiS_'\) [^\n]*\n} }
+// { dg-regexp {[^\n]*lambda-tpl1.h:[:0-9]* warning: the mangled name [^\n]* \('_ZZ1fvENKUlT_E_clIcLc0EEEDaS_'\) and '-fabi-version=18' \('_ZZ1fvENKUlTyTnT_S_E_clIcLc0EEEDaS_'\) [^\n]*\n} }
+// { dg-regexp {[^\n]*lambda-tpl1.h:[:0-9]* warning: the mangled name [^\n]* \('_ZZZ1fvENKUlT_E_clIcLc0EEEDaS_ENKUlcS_E_clIcEEDacS_'\) and '-fabi-version=18' \('_ZZZ1fvENKUlTyTnT_S_E_clIcLc0EEEDaS_ENKUlTycS_E_clIcEEDacS_'\) [^\n]*\n} }
+// { dg-regexp {[^\n]*lambda-tpl1.h:[:0-9]* warning: the mangled name [^\n]* \('_ZZ1fvENKUlP1UIT_Lj0EEPS_IiLj0EEE0_clIcEEDaS2_S4_'\) and '-fabi-version=18' \('_ZZ1fvENKUlTyP1UIT_Lj0EEPS_IiLj0EEE_clIcEEDaS2_S4_'\) [^\n]*\n} }
diff --git a/gcc/testsuite/g++.dg/abi/lambda-tpl1.h b/gcc/testsuite/g++.dg/abi/lambda-tpl1.h
index 5d6fe5e1d0a..376c3f6a2f4 100644
--- a/gcc/testsuite/g++.dg/abi/lambda-tpl1.h
+++ b/gcc/testsuite/g++.dg/abi/lambda-tpl1.h
@@ -56,4 +56,15 @@ void f ()
l_var4 (y);
Fn<1> ();
+
+ auto l1 = [] (T a) {
+auto l2 = [] (T a, U b) {};
+
+l2 (a, v);
+ };
+ auto l3 = [](U *, U *) {};
+
+ l1 (1);
+ l1 ('1'
Re: [PATCH] c++: Allow module name to be a single letter on Windows
On 11/3/22 11:06, Torbjorn SVENSSON wrote:
On 2022-11-03 15:17, Nathan Sidwell wrote:
On 10/28/22 05:15, Torbjörn SVENSSON wrote:
On Windows, the ':' character is special and when the module name is
a single character, like 'A', then the flatname would be (for
example) 'A:Foo'. On Windows, 'A:Foo' is treated as an absolute
path by the module loader and is likely not found.
Without this patch, the test case pr98944_c.C fails with:
In module imported at /src/gcc/testsuite/g++.dg/modules/pr98944_b.C:7:1,
of module A:Foo, imported at /src/gcc/testsuite/g++.dg/modules/pr98944_c.C:7:
A:Internals: error: header module expected, module 'A:Internals' found
A:Internals: error: failed to read compiled module: Bad file data
A:Internals: note: compiled module file is 'gcm.cache/A-Internals.gcm'
In module imported at /src/gcc/testsuite/g++.dg/modules/pr98944_c.C:7:8:
A:Foo: error: failed to read compiled module: Bad import dependency
A:Foo: note: compiled module file is 'gcm.cache/A-Foo.gcm'
A:Foo: fatal error: returning to the gate for a mechanical issue
compilation terminated.
include/ChangeLog:
* filenames.h: Added IS_REAL_ABSOLUTE_PATH macro to check if
path is absolute and not semi-absolute on Windows.
Hm, this is unfortunate. The current IS_ABSOLUTE_PATH, is really 'not
relative to cwd', and even then that's untrue if the drive letter there is the
drive letter of cwd, right?
It's awkward to have a new macro for just this purpose and the new name isn't
very indicative of the difference to the current IS_ABSOLUTE_PATH.
Would it be better to not deal with drive letters here? How prevalent are
they these days in windows? Would something like
if (IS_DIR_SEPARATOR (ptr[ptr[0] == '.'])
suffice?
I don't think you can ignore the drive letter part... see below.
#include
#include "include/filenames.h"
#define TF(x) ((x) ? "true" : "false")
int main(int argc, char *argv[]) {
const char *test[] = {
/* absolute */ "c:\\foo", "c:/foo", "/foo", "\\foo",
/* semi-absolute */ "c:foo",
/* relative */ "foo", "./foo", ".\\foo",
};
for (int i = 0; i < sizeof(test) / sizeof(test[0]); i++) {
const char *ptr = test[i];
printf("\nptr: %s\n", ptr);
printf(" IS_DOS_ABSOLUTE_PATH: %s\n",
TF(IS_DOS_ABSOLUTE_PATH(ptr)));
printf(" IS_DOS_REAL_ABSOLUTE_PATH: %s\n",
TF(IS_DOS_REAL_ABSOLUTE_PATH(ptr)));
printf(" IS_DIR_SEPARATOR: %s\n",
TF(IS_DIR_SEPARATOR(ptr[ptr[0] == '.'])));
}
return 0;
}
The output is:
ptr: c:\foo
IS_DOS_ABSOLUTE_PATH: true
IS_DOS_REAL_ABSOLUTE_PATH: true
IS_DIR_SEPARATOR: false
ptr: c:/foo
IS_DOS_ABSOLUTE_PATH: true
IS_DOS_REAL_ABSOLUTE_PATH: true
IS_DIR_SEPARATOR: false
ptr: /foo
IS_DOS_ABSOLUTE_PATH: true
IS_DOS_REAL_ABSOLUTE_PATH: true
IS_DIR_SEPARATOR: true
ptr: \foo
IS_DOS_ABSOLUTE_PATH: true
IS_DOS_REAL_ABSOLUTE_PATH: true
IS_DIR_SEPARATOR: false
ptr: c:foo
IS_DOS_ABSOLUTE_PATH: true
IS_DOS_REAL_ABSOLUTE_PATH: false
IS_DIR_SEPARATOR: false
ptr: foo
IS_DOS_ABSOLUTE_PATH: false
IS_DOS_REAL_ABSOLUTE_PATH: false
IS_DIR_SEPARATOR: false
ptr: ./foo
IS_DOS_ABSOLUTE_PATH: false
IS_DOS_REAL_ABSOLUTE_PATH: false
IS_DIR_SEPARATOR: true
ptr: .\foo
IS_DOS_ABSOLUTE_PATH: false
IS_DOS_REAL_ABSOLUTE_PATH: false
IS_DIR_SEPARATOR: false
or, failing that perhaps put some explicit WINDOWS-specific #ifdef'd code
there? It's a real corner case.
Would you rather have something like this in module.cc?
if (ptr[0] == '.')
{
if IS_DIR_SEPARATOR (ptr[1]))
return get_module (build_string (strlen (ptr), ptr));
}
else
{
#if HAVE_DOS_BASED_FILE_SYSTEM
if (HAS_DRIVE_SPEC (ptr) && IS_DIR_SEPARATOR (ptr[2]))
#else
if (IS_ABSOLUTE_PATH (ptr))
#endif
return get_module (build_string (strlen (ptr), ptr));
}
Yes, something like the above, but I think you're missing "/bob' in the
DOS_BASED case? shouldn't that also be a pathname?
if (IS_DIR_SEPARATOR (ptr[ptr[0] == '.']) // ./FOO or /FOO
#if HAVE_DOS_BASED_FILE_SYSTEM
// DOS-FS IS_ABSOLUTE_PATH thinks 'A:B' is absolute, but we need to consider
// that as a module:partition.
|| (HAS_DRIVE_SPEC (ptr) && IS_DIR_SEPARATOR (ptr[2])) // A:/FOO
#endif
|| false)
return
Does (something like) that work?
nathan
Let me know what you prefer.
Kind regards,
Torbjörn
nathan
gcc/cp/ChangeLog:
* module.cc: Use IS_REAL_ABSOLUTE_PATH macro.
Co-Authored-By: Yvan ROUX
Signed-off-by: Torbjörn SVENSSON
---
gcc/cp/module.cc | 2 +-
include/filenames.h | 4
2 files changed, 5 insertions(+), 1 deletion(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 9957df510e6..84680e183b7 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13958,7 +13958,7 @@ get_module (tree name, module_state *parent, bool
partition)
static module_state *
get_module (const char *ptr)
{
- if (ptr[0] ==
Re: [PATCH] c++: Allow module name to be a single letter on Windows
On 11/8/22 05:18, Torbjorn SVENSSON wrote: Hi Nathan, On 2022-11-08 00:03, Nathan Sidwell wrote: Yes, something like the above, but I think you're missing "/bob' in the DOS_BASED case? shouldn't that also be a pathname? if (IS_DIR_SEPARATOR (ptr[ptr[0] == '.']) // ./FOO or /FOO #if HAVE_DOS_BASED_FILE_SYSTEM // DOS-FS IS_ABSOLUTE_PATH thinks 'A:B' is absolute, but we need to consider // that as a module:partition. || (HAS_DRIVE_SPEC (ptr) && IS_DIR_SEPARATOR (ptr[2])) // A:/FOO #endif || false) return Does (something like) that work? I tested it and your solution appears to work. Are you okay with me pushing that solution or do you want me to send a v2 with it first? I think it needs a better introductory comment than the one I slapped in there. More explanation of the drive vs partition distinction. Something along the lines of 'things that clearly start as pathnames are header-names, everything else is treated as a (possibly malformed) named module. Feel free to just go with it, or iterate here nathan -- Nathan Sidwell
demangler: Templated lambda demangling
Templated lambdas have a template-head, which is part of their signature. GCC ABI 18 mangles that into the lambda name. This adds support to the demangler. We have to introduce artificial template parameter names, as we need to refer to them from later components of the lambda signature. We use $T:n, $N:n and $TT:n for type, non-type and template parameters. Non-type parameter names are not shown in the strictly correct location -- for instance 'int (&NT) ()' would be shown as 'int (&) $N:n'. That's unfortunate, but an orthogonal issue. The 'is_lambda_arg' field is now repurposed as indicating the number of explicit template parameters (1-based). I'll commit in a few days. nathan -- Nathan SidwellFrom b7f0ba90011b8c9cae7b7278463f609ba21cd44b Mon Sep 17 00:00:00 2001 From: Nathan Sidwell Date: Mon, 7 Nov 2022 11:24:14 -0500 Subject: [PATCH] demangler: Templated lambda demangling Templated lambdas have a template-head, which is part of their signature. GCC ABI 18 mangles that into the lambda name. This adds support to the demangler. We have to introduce artificial template parameter names, as we need to refer to them from later components of the lambda signature. We use $T:n, $N:n and $TT:n for type, non-type and template parameters. Non-type parameter names are not shown in the strictly correct location -- for instance 'int (&NT) ()' would be shown as 'int (&) $N:n'. That's unfortunate, but an orthogonal issue. The 'is_lambda_arg' field is now repurposed as indicating the number of explicit template parameters (1-based). include/ * demangle.h (enum demangle_component_type): Add DEMANGLE_COMPONENT_TEMPLATE_HEAD, DEMANGLE_COMPONENT_TEMPLATE_TYPE_PARM, DEMANGLE_COMPONENT_TEMPLATE_NON_TYPE_PARM, DEMANGLE_COMPONENT_TEMPLATE_TEMPLATE_PARM, DEMANGLE_COMPONENT_TEMPLATE_PACK_PARM. libiberty/ * cp-demangle.c (struct d_print_info): Rename is_lambda_arg to lambda_tpl_parms. Augment semantics. (d_make_comp): Add checks for new components. (d_template_parm, d_template_head): New. (d_lambda): Add templated lambda support. (d_print_init): Adjust. (d_print_lambda_parm_name): New. (d_print_comp_inner): Support templated lambdas, * testsuite/demangle-expected: Add testcases. --- include/demangle.h| 6 + libiberty/cp-demangle.c | 260 +++--- libiberty/testsuite/demangle-expected | 53 ++ 3 files changed, 295 insertions(+), 24 deletions(-) diff --git a/include/demangle.h b/include/demangle.h index 81d4353a86f..66637ebdc16 100644 --- a/include/demangle.h +++ b/include/demangle.h @@ -458,6 +458,12 @@ enum demangle_component_type DEMANGLE_COMPONENT_MODULE_ENTITY, DEMANGLE_COMPONENT_MODULE_INIT, + DEMANGLE_COMPONENT_TEMPLATE_HEAD, + DEMANGLE_COMPONENT_TEMPLATE_TYPE_PARM, + DEMANGLE_COMPONENT_TEMPLATE_NON_TYPE_PARM, + DEMANGLE_COMPONENT_TEMPLATE_TEMPLATE_PARM, + DEMANGLE_COMPONENT_TEMPLATE_PACK_PARM, + /* A builtin type with argument. This holds the builtin type information. */ DEMANGLE_COMPONENT_EXTENDED_BUILTIN_TYPE diff --git a/libiberty/cp-demangle.c b/libiberty/cp-demangle.c index 8413dcdc785..ad533f6085e 100644 --- a/libiberty/cp-demangle.c +++ b/libiberty/cp-demangle.c @@ -347,9 +347,9 @@ struct d_print_info /* Number of times d_print_comp was recursively called. Should not be bigger than MAX_RECURSION_COUNT. */ int recursion; - /* Non-zero if we're printing a lambda argument. A template - parameter reference actually means 'auto', a pack expansion means T... */ - int is_lambda_arg; + /* 1 more than the number of explicit template parms of a lambda. Template + parm references >= are actually 'auto'. */ + int lambda_tpl_parms; /* The current index into any template argument packs we are using for printing, or -1 to print the whole pack. */ int pack_index; @@ -491,6 +491,10 @@ static struct demangle_component *d_local_name (struct d_info *); static int d_discriminator (struct d_info *); +static struct demangle_component *d_template_parm (struct d_info *, int *bad); + +static struct demangle_component *d_template_head (struct d_info *, int *bad); + static struct demangle_component *d_lambda (struct d_info *); static struct demangle_component *d_unnamed_type (struct d_info *); @@ -1028,6 +1032,10 @@ d_make_comp (struct d_info *di, enum demangle_component_type type, case DEMANGLE_COMPONENT_TPARM_OBJ: case DEMANGLE_COMPONENT_STRUCTURED_BINDING: case DEMANGLE_COMPONENT_MODULE_INIT: +case DEMANGLE_COMPONENT_TEMPLATE_HEAD: +case DEMANGLE_COMPONENT_TEMPLATE_NON_TYPE_PARM: +case DEMANGLE_COMPONENT_TEMPLATE_TEMPLATE_PARM: +case DEMANGLE_COMPONENT_TEMPLATE_PACK_PARM: if (left == NULL) return NULL; break; @@ -1050,6 +1058,7 @@ d_make_comp (struct d_info *di, enum demangle_component_type type, case DEMANGLE_COMPONENT_CONST: case DEMANGLE_COMPONENT_ARGLIST: case DEMANGLE_COMPONENT_TEMPLATE_ARGLIST: +ca
Re: [PATCH v2] testsuite/C++: cope with IPv6 being unavailable
On 4/25/23 11:00, Jan Beulich wrote:
When IPv6 is disabled in the kernel, the error message coming back from
Cody::OpenInet6() is different from the sole so far expected one.
ok -- i couldn't find such a system :)
---
v2: Re-base.
--- a/gcc/testsuite/g++.dg/modules/bad-mapper-3.C
+++ b/gcc/testsuite/g++.dg/modules/bad-mapper-3.C
@@ -1,6 +1,6 @@
// { dg-additional-options "-fmodules-ts
-fmodule-mapper=localhost:172477262" }
import unique3.bob;
-// { dg-error {failed (connecting|disabled) mapper 'localhost:172477262'} "" {
target *-*-* } 0 }
+// { dg-error {failed (socket|connecting|disabled) mapper 'localhost:172477262'}
"" { target *-*-* } 0 }
// { dg-prune-output "fatal error:" }
// { dg-prune-output "failed to read" }
// { dg-prune-output "compilation terminated" }
--
Nathan Sidwell
Re: Ping: [PATCH] testsuite/C++: suppress filename canonicalization in module tests
On 4/25/23 11:04, Jan Beulich wrote:
On 28.06.2022 16:06, Jan Beulich wrote:
The pathname underneath gcm.cache/ is determined from the effective name
used for the main input file of a particular module. When modules are
built, no canonicalization occurs for the main input file. Hence the
module file wouldn't be found if a different (the canonicalized) file
name was used when importing that same module. (This is an effect of
importing happening in the preprocessor, just like #include handling.)
Since it doesn't look easy to make module generation use libcpp's
maybe_shorter_path() (in fact I'd consider this a layering violation,
while cloning the logic would - at least in principle - be prone to both
going out of sync), simply suppress system header path canonicalization
for the respective tests.
Ping: This still looks to apply as is.
ok -- I was unaware of this. might be sensible to file a defect about this?
Thanks, Jan
---
Strictly speaking it could be necessary to also suppress
canonicalization when generating the modules, but for now they're self-
contained, i.e. don't include any "real" system headers. IOW at the
moment the tests aren't susceptible to the issue at generation time.
--- a/gcc/testsuite/g++.dg/modules/alias-1_b.C
+++ b/gcc/testsuite/g++.dg/modules/alias-1_b.C
@@ -1,4 +1,4 @@
-// { dg-additional-options "-fmodules-ts -fdump-lang-module -isystem [srcdir]"
}
+// { dg-additional-options "-fmodules-ts -fdump-lang-module -isystem [srcdir]
-fno-canonical-system-headers" }
// Alias at the header file. We have one CMI file
import "alias-1_a.H";
--- a/gcc/testsuite/g++.dg/modules/alias-1_d.C
+++ b/gcc/testsuite/g++.dg/modules/alias-1_d.C
@@ -1,4 +1,4 @@
-// { dg-additional-options "-fmodules-ts -isystem [srcdir]" }
+// { dg-additional-options "-fmodules-ts -isystem [srcdir]
-fno-canonical-system-headers" }
// { dg-module-cmi kevin }
export module kevin;
--- a/gcc/testsuite/g++.dg/modules/alias-1_e.C
+++ b/gcc/testsuite/g++.dg/modules/alias-1_e.C
@@ -1,4 +1,4 @@
-// { dg-additional-options "-fmodules-ts -isystem [srcdir]" }
+// { dg-additional-options "-fmodules-ts -isystem [srcdir]
-fno-canonical-system-headers" }
import bob;
import kevin;
--- a/gcc/testsuite/g++.dg/modules/alias-1_f.C
+++ b/gcc/testsuite/g++.dg/modules/alias-1_f.C
@@ -1,4 +1,4 @@
-// { dg-additional-options "-fmodules-ts -fdump-lang-module -isystem [srcdir]"
}
+// { dg-additional-options "-fmodules-ts -fdump-lang-module -isystem [srcdir]
-fno-canonical-system-headers" }
import kevin;
import bob;
--- a/gcc/testsuite/g++.dg/modules/cpp-6_c.C
+++ b/gcc/testsuite/g++.dg/modules/cpp-6_c.C
@@ -1,5 +1,5 @@
// { dg-do preprocess }
-// { dg-additional-options "-fmodules-ts -isystem [srcdir]" }
+// { dg-additional-options "-fmodules-ts -isystem [srcdir]
-fno-canonical-system-headers" }
#define empty
#define nop(X) X
--- a/gcc/testsuite/g++.dg/modules/dir-only-2_b.C
+++ b/gcc/testsuite/g++.dg/modules/dir-only-2_b.C
@@ -1,5 +1,5 @@
// { dg-do preprocess }
-// { dg-additional-options "-fmodules-ts -fdirectives-only -isystem [srcdir]" }
+// { dg-additional-options "-fmodules-ts -fdirectives-only -isystem [srcdir]
-fno-canonical-system-headers" }
// a comment
module; // line
frob
--
Nathan Sidwell
Re: [PATCH] c++ modules: uninstantiated template friend class [PR104234]
That might be sufficient for this case, but temploid friends violate an
assumption of the implementation -- namely that module A cannot create an entity
that belongs in module B's symbol table. This causes a bunch of excitement,
particularly around handling (well formed) duplicatd instantions.
I'm not sure of the way to handle that, but I suspect something along the lines
of a flag on such decls and a new hash table to hold these exceptions.
nathan
On 1/25/23 15:16, Patrick Palka wrote:
Here we're not clearing DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P for
the instantiated/injected template friend class B, which confuses a
later call to get_originating_module_decl for B. This patch fixes this
by clearing the flag in tsubst_friend_class (as is already done for
template friend functions by r11-5730-gf7aeb823d9b0de).
After fixing that, we still fail to compile the testcase, rejecting the
later definition of B with
friend-6_a.C:10:26: error: cannot declare ‘struct B’ in a different module
ultimately because DECL_MODULE_ATTACH_P wasn't set on the original
(injected) declaration of B. This patch fixes this by calling
set_originating_module in tsubst_friend_class, but for that to work it
seems we need to relax the assert in this latter function since
get_originating_module_decl when called on the TYPE_DECL for B returns
the corresponding TEMPLATE_DECL.
(Alternatively we can instead call set_originating_module on the
TYPE_DECL B as soon as it's created in lookup_template_class (which is
what pushtag does), which doesn't need this assert change because at
this point the TYPE_DECL doesn't have any TEMPLATE_INFO so
get_originating_module_decl becomes a no-op. Would that be preferable?)
Bootstrapped and regtested on x86_64-pc-linux-gnu, does this look OK for
trunk?
PR c++/104234
gcc/cp/ChangeLog:
* module.cc (set_originating_module): Document default argument.
Relax assert to look through DECL_TEMPLATE_RESULT in the result
of get_originating_module_decl.
* pt.cc (tsubst_friend_class): Clear
DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P and call
set_originating_module on the instantiated template friend class.
gcc/testsuite/ChangeLog:
* g++.dg/modules/friend-6_a.C: New test.
---
gcc/cp/module.cc | 8 ++--
gcc/cp/pt.cc | 3 +++
gcc/testsuite/g++.dg/modules/friend-6_a.C | 10 ++
3 files changed, 19 insertions(+), 2 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/friend-6_a.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 7133009dba5..234ce43b70f 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -18843,14 +18843,18 @@ set_defining_module (tree decl)
}
void
-set_originating_module (tree decl, bool friend_p ATTRIBUTE_UNUSED)
+set_originating_module (tree decl, bool friend_p /* = false */)
{
set_instantiating_module (decl);
if (!DECL_NAMESPACE_SCOPE_P (decl))
return;
- gcc_checking_assert (friend_p || decl == get_originating_module_decl (decl));
+ if (!friend_p)
+{
+ tree o = get_originating_module_decl (decl);
+ gcc_checking_assert (STRIP_TEMPLATE (o) == decl);
+}
if (module_attach_p ())
{
diff --git a/gcc/cp/pt.cc b/gcc/cp/pt.cc
index cbe5898b553..f2ee74025e7 100644
--- a/gcc/cp/pt.cc
+++ b/gcc/cp/pt.cc
@@ -11520,6 +11520,9 @@ tsubst_friend_class (tree friend_tmpl, tree args)
CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl))
= INNERMOST_TEMPLATE_ARGS (CLASSTYPE_TI_ARGS (TREE_TYPE (tmpl)));
+ DECL_UNINSTANTIATED_TEMPLATE_FRIEND_P (tmpl) = false;
+ set_originating_module (DECL_TEMPLATE_RESULT (tmpl));
+
/* Substitute into and set the constraints on the new declaration. */
if (tree ci = get_constraints (friend_tmpl))
{
diff --git a/gcc/testsuite/g++.dg/modules/friend-6_a.C
b/gcc/testsuite/g++.dg/modules/friend-6_a.C
new file mode 100644
index 000..97017e4ee78
--- /dev/null
+++ b/gcc/testsuite/g++.dg/modules/friend-6_a.C
@@ -0,0 +1,10 @@
+// PR c++/104234
+// { dg-additional-options "-fmodules-ts" }
+// { dg-module-cmi pr104234 }
+export module pr104234;
+
+template struct A {
+ template friend struct B;
+};
+A a;
+template struct B { };
--
Nathan Sidwell
c++: Adjust module initializer calling emission
We special-case emitting the calls of module initializer functions.
It's simpler to just emit a static fn do do that, and add it onto the
front of the global init fn chain. We can also move the calculation of
the set of initializers to call to the point of use.
nathan
--
Nathan SidwellFrom 8834d2d35fcc229c00e2e06e8be8b052c803d8cd Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 10 Jun 2022 05:22:21 -0700
Subject: [PATCH] c++: Adjust module initializer calling emission
We special-case emitting the calls of module initializer functions. It's
simpler to just emit a static fn do do that, and add it onto the front of
the global init fn chain. We can also move the calculation of the set of
initializers to call to the point of use.
gcc/cp/
* cp-tree.h (module_has_import_init): Rename to ...
(module_determined_import_inits): ... here.
* decl2.cc (start_objects): Do not handle module initializers
here.
(c_parse_final_cleanups): Generate a separate module
initializer calling function and add it to the list. Shrink
the c-lang region.
* module.cc (num_init_calls_needed): Delete.
(module_has_import_init): Rename to ...
(module_determined_import_inits): ... here. Do the
calculation here ...
(finish_module_processing): ... rather than here.
(module_add_import_initializers): Reformat.
gcc/testsuite/
* g++.dg/modules/init-3_a.C: New.
* g++.dg/modules/init-3_b.C: New.
* g++.dg/modules/init-3_c.C: New.
---
gcc/cp/cp-tree.h| 2 +-
gcc/cp/decl2.cc | 47 +-
gcc/cp/module.cc| 110 +++-
gcc/testsuite/g++.dg/modules/init-3_a.C | 17
gcc/testsuite/g++.dg/modules/init-3_b.C | 6 ++
gcc/testsuite/g++.dg/modules/init-3_c.C | 17
6 files changed, 117 insertions(+), 82 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/init-3_a.C
create mode 100644 gcc/testsuite/g++.dg/modules/init-3_b.C
create mode 100644 gcc/testsuite/g++.dg/modules/init-3_c.C
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index a5d93282167..f1294dac7d5 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7180,7 +7180,7 @@ extern module_state *get_module (tree name, module_state *parent = NULL,
extern bool module_may_redeclare (tree decl);
extern bool module_global_init_needed ();
-extern bool module_has_import_inits ();
+extern bool module_determine_import_inits ();
extern void module_add_import_initializers ();
/* Where the namespace-scope decl was originally declared. */
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 9de9a7a4f8a..ff1c36745cf 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -3903,8 +3903,7 @@ start_objects (bool initp, unsigned priority, bool has_body)
tree body = begin_compound_stmt (BCS_FN_BODY);
- bool has_import_inits = default_init && module_has_import_inits ();
- if (is_module_init && (has_import_inits || has_body))
+ if (is_module_init && has_body)
{
// If the function is going to be empty, don't emit idempotency.
// 'static bool __in_chrg = false;
@@ -3930,9 +3929,6 @@ start_objects (bool initp, unsigned priority, bool has_body)
finish_expr_stmt (assign);
}
- if (has_import_inits)
-module_add_import_initializers ();
-
return body;
}
@@ -5195,6 +5191,12 @@ c_parse_final_cleanups (void)
maybe_warn_sized_delete ();
+ // Place the init fns in the right order. We need to do this now,
+ // so that any module init will go at the start.
+ if (static_init_fini_fns[true])
+for (auto iter : *static_init_fini_fns[true])
+ iter.second = nreverse (iter.second);
+
/* Then, do the Objective-C stuff. This is where all the
Objective-C module stuff gets generated (symtab,
class/protocol/selector lists etc). This must be done after C++
@@ -5203,11 +5205,18 @@ c_parse_final_cleanups (void)
if (c_dialect_objc ())
objc_write_global_declarations ();
- /* We give C linkage to static constructors and destructors. */
- push_lang_context (lang_name_c);
+ if (module_determine_import_inits ())
+{
+ input_location = locus_at_end_of_parsing;
+ tree body = start_partial_init_fini_fn (true, DEFAULT_INIT_PRIORITY,
+ ssdf_count++);
+ module_add_import_initializers ();
+ input_location = locus_at_end_of_parsing;
+ finish_partial_init_fini_fn (body);
+}
if ((c_dialect_objc () && objc_static_init_needed_p ())
- || module_global_init_needed () || module_has_import_inits ())
+ || module_global_init_needed ())
{
// Make sure there's a default priority entry.
if (!static_init_fini_fns[true])
@@ -5216,32 +5225,24 @@ c_parse_final_cleanups (void)
}
/* Generate initialization and destruction functions for all
- priorities for which they are required. */
+ priorities for which they are required. They have C-language
+ linkage. */
+ push_lang_context (lang_name_c);
for (unsigned in
c++: Add a late-writing step for modules
To add a module initializer optimization, we need to defer finishing
writing out the module file until the end of determining the dynamic
initializers. This is achieved by passing some saved-state from the main
module writing to a new function that completes it.
This patch merely adds the skeleton of that state and move things
around, allowing the finalization of the ELF file to be postponed. None
of the contents writing is moved, or the init optimization added.
nathan
--
Nathan SidwellFrom e6d369bbdb4eb5f03eec233ef9905013a735fd71 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Thu, 9 Jun 2022 08:14:31 -0700
Subject: [PATCH] c++: Add a late-writing step for modules
To add a module initializer optimization, we need to defer finishing writing
out the module file until the end of determining the dynamic initializers.
This is achieved by passing some saved-state from the main module writing
to a new function that completes it.
This patch merely adds the skeleton of that state and move things around,
allowing the finalization of the ELF file to be postponed. None of the
contents writing is moved, or the init optimization added.
gcc/cp/
* cp-tree.h (fini_modules): Add some parameters.
(finish_module_processing): Return an opaque pointer.
* decl2.cc (c_parse_final_cleanups): Propagate a cookie from
finish_module_processing to fini_modules.
* module.cc (struct module_processing_cookie): New.
(finish_module_processing): Return a heap-allocated cookie.
(late_finish_module): New. Finish out the module writing.
(fini_modules): Adjust.
---
gcc/cp/cp-tree.h | 4 +-
gcc/cp/decl2.cc | 4 +-
gcc/cp/module.cc | 145 ++-
3 files changed, 98 insertions(+), 55 deletions(-)
diff --git a/gcc/cp/cp-tree.h b/gcc/cp/cp-tree.h
index f1294dac7d5..60d7b201595 100644
--- a/gcc/cp/cp-tree.h
+++ b/gcc/cp/cp-tree.h
@@ -7209,9 +7209,9 @@ extern void import_module (module_state *, location_t, bool export_p,
extern void declare_module (module_state *, location_t, bool export_p,
tree attr, cpp_reader *);
extern void init_modules (cpp_reader *);
-extern void fini_modules ();
+extern void fini_modules (cpp_reader *, void *cookie);
extern void maybe_check_all_macros (cpp_reader *);
-extern void finish_module_processing (cpp_reader *);
+extern void *finish_module_processing (cpp_reader *);
extern char const *module_name (unsigned, bool header_ok);
extern bitmap get_import_bitmap ();
extern bitmap visible_instantiation_path (bitmap *);
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index ff1c36745cf..cc0b41324b3 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -5154,7 +5154,7 @@ c_parse_final_cleanups (void)
reconsider = true;
}
- finish_module_processing (parse_in);
+ void *module_cookie = finish_module_processing (parse_in);
lower_var_init ();
@@ -5238,7 +5238,7 @@ c_parse_final_cleanups (void)
}
pop_lang_context ();
- fini_modules ();
+ fini_modules (parse_in, module_cookie);
/* Generate any missing aliases. */
maybe_apply_pending_pragma_weaks ();
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 2b1877ea82e..51d774ae608 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -19854,11 +19854,32 @@ maybe_check_all_macros (cpp_reader *reader)
dump.pop (n);
}
+// State propagated from finish_module_processing to fini_modules
+struct module_processing_cookie
+{
+ elf_out out;
+ char *cmi_name;
+ char *tmp_name;
+ bool began;
+
+ module_processing_cookie (char *cmi, char *tmp, int fd, int e)
+: out (fd, e), cmi_name (cmi), tmp_name (tmp), began (false)
+ {
+ }
+ ~module_processing_cookie ()
+ {
+XDELETEVEC (tmp_name);
+XDELETEVEC (cmi_name);
+ }
+};
+
/* Write the CMI, if we're a module interface. */
-void
+void *
finish_module_processing (cpp_reader *reader)
{
+ module_processing_cookie *cookie = nullptr;
+
if (header_module_p ())
module_kind &= ~MK_EXPORTING;
@@ -19870,7 +19891,7 @@ finish_module_processing (cpp_reader *reader)
else if (!flag_syntax_only)
{
int fd = -1;
- int e = ENOENT;
+ int e = -1;
timevar_start (TV_MODULE_EXPORT);
@@ -19879,7 +19900,7 @@ finish_module_processing (cpp_reader *reader)
linemap_add (line_table, LC_ENTER, false, "", 0);
/* We write to a tmpname, and then atomically rename. */
- const char *path = NULL;
+ char *cmi_name = NULL;
char *tmp_name = NULL;
module_state *state = (*modules)[0];
@@ -19888,9 +19909,9 @@ finish_module_processing (cpp_reader *reader)
if (state->filename)
{
size_t len = 0;
- path = maybe_add_cmi_prefix (state->filename, &len);
+ cmi_name = xstrdup (maybe_add_cmi_prefix (state->filename, &len));
tmp_name = XNEWVEC (char, len + 3);
- memcpy (tmp_name, path, len);
+ memcpy (tmp_name, cmi_name, len);
strcpy (&tmp_name[len], "~");
if (!errorcount)
@@ -19905,57 +19926,23 @@ finish_module_processing
c++: Separate late stage module writing
This moves some module writing into a newly added write_end function,
which is called after writing initializers.
Thus paving the way to eliminate calls to NOP initializer fns.
nathan
--
Nathan SidwellFrom 6303eee4b92e8509409503a3abebde8bd50f0f05 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Thu, 9 Jun 2022 08:48:25 -0700
Subject: [PATCH] c++: Separate late stage module writing
This moves some module writing into a newly added write_end function,
which is called after writing initializers.
gcc/cp/
* module.cc (module_state::write): Separate to ...
(module_state::write_begin, module_state::write_end): ...
these.
(module_state::write_readme): Drop extensions parameter.
(struct module_processing_cookie): Add more fields.
(finish_module_processing): Adjust state writing call.
(late_finish_module): Call write_end.
---
gcc/cp/module.cc | 47 ++-
1 file changed, 30 insertions(+), 17 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 51d774ae608..e7ce40ef464 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3523,7 +3523,10 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state {
public:
/* Read and write module. */
- void write (elf_out *to, cpp_reader *);
+ void write_begin (elf_out *to, cpp_reader *,
+ module_state_config &, unsigned &crc);
+ void write_end (elf_out *to, cpp_reader *,
+ module_state_config &, unsigned &crc);
bool read_initial (cpp_reader *);
bool read_preprocessor (bool);
bool read_language (bool);
@@ -3545,8 +3548,7 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state {
private:
/* The README, for human consumption. */
- void write_readme (elf_out *to, cpp_reader *,
- const char *dialect, unsigned extensions);
+ void write_readme (elf_out *to, cpp_reader *, const char *dialect);
void write_env (elf_out *to);
private:
@@ -13954,8 +13956,7 @@ module_state::announce (const char *what) const
readelf -pgnu.c++.README $(module).gcm */
void
-module_state::write_readme (elf_out *to, cpp_reader *reader,
- const char *dialect, unsigned extensions)
+module_state::write_readme (elf_out *to, cpp_reader *reader, const char *dialect)
{
bytes_out readme (to);
@@ -17560,7 +17561,8 @@ ool_cmp (const void *a_, const void *b_)
*/
void
-module_state::write (elf_out *to, cpp_reader *reader)
+module_state::write_begin (elf_out *to, cpp_reader *reader,
+ module_state_config &config, unsigned &crc)
{
/* Figure out remapped module numbers, which might elide
partitions. */
@@ -17656,8 +17658,6 @@ module_state::write (elf_out *to, cpp_reader *reader)
}
ool->qsort (ool_cmp);
- unsigned crc = 0;
- module_state_config config;
location_map_info map_info = write_prepare_maps (&config);
unsigned counts[MSC_HWM];
@@ -17811,28 +17811,35 @@ module_state::write (elf_out *to, cpp_reader *reader)
unsigned clusters = counts[MSC_sec_hwm] - counts[MSC_sec_lwm];
dump () && dump ("Wrote %u clusters, average %u bytes/cluster",
clusters, (bytes + clusters / 2) / (clusters + !clusters));
+ trees_out::instrument ();
write_counts (to, counts, &crc);
- /* And finish up. */
- write_config (to, config, crc);
-
spaces.release ();
sccs.release ();
vec_free (ool);
- /* Human-readable info. */
- write_readme (to, reader, config.dialect_str, extensions);
-
// FIXME:QOI: Have a command line switch to control more detailed
// information (which might leak data you do not want to leak).
// Perhaps (some of) the write_readme contents should also be
// so-controlled.
if (false)
write_env (to);
+}
+
+// Finish module writing after we've emitted all dynamic initializers.
+
+void
+module_state::write_end (elf_out *to, cpp_reader *reader,
+ module_state_config &config, unsigned &crc)
+{
+ /* And finish up. */
+ write_config (to, config, crc);
+
+ /* Human-readable info. */
+ write_readme (to, reader, config.dialect_str);
- trees_out::instrument ();
dump () && dump ("Wrote %u sections", to->get_section_limit ());
}
@@ -19855,15 +19862,18 @@ maybe_check_all_macros (cpp_reader *reader)
}
// State propagated from finish_module_processing to fini_modules
+
struct module_processing_cookie
{
elf_out out;
+ module_state_config config;
char *cmi_name;
char *tmp_name;
+ unsigned crc;
bool began;
module_processing_cookie (char *cmi, char *tmp, int fd, int e)
-: out (fd, e), cmi_name (cmi), tmp_name (tmp), began (false)
+: out (fd, e), cmi_name (cmi), tmp_name (tmp), crc (0), began (false)
{
}
~module_processing_cookie ()
@@ -19941,7 +19951,7 @@ finish_module_processing (cpp_reader *reader)
auto loc = input_location;
/* So crashes finger-point the module decl. */
input_location = state->loc;
- state->write (&cookie->out, reader);
+ state->write_begin (&cookie->out, reader, cookie->config, cookie->crc);
input_loca
c++: Elide calls to NOP module initializers
This implements NOP module initializer elision. Each CMI gains a new flag informing importers whether its initializer actually does something (initializers its own globals, and/or calls initializers of its imports). This allows an importer to determine whether to call it. nathan -- Nathan Sidwell
c++: Use better module partition naming
It turns out that 'implementation partition' is not a term used in the
std, and is confusing to users. Let's use the better term 'internal
partition'. While there, adjust header unit naming.
nathan
--
Nathan SidwellFrom 052d89537a4c09e1e1437042e2d1ea215656325f Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 10 Jun 2022 11:57:38 -0700
Subject: [PATCH] c++: Use better module partition naming
It turns out that 'implementation partition' is not a term used in the
std, and is confusing to users. Let's use the better term 'internal
partition'. While there, adjust header unit naming.
gcc/cp/
* module.cc (module_state::write_readme): Use less confusing
importable unit names.
---
gcc/cp/module.cc | 10 +-
1 file changed, 5 insertions(+), 5 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 5566c49490f..b3fbd467ecb 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13962,11 +13962,11 @@ module_state::write_readme (elf_out *to, cpp_reader *reader, const char *dialect
readme.begin (false);
- readme.printf ("GNU C++ %smodule%s%s",
- is_header () ? "header " : is_partition () ? "" : "primary ",
- is_header () ? ""
- : is_interface () ? " interface" : " implementation",
- is_partition () ? " partition" : "");
+ readme.printf ("GNU C++ %s",
+ is_header () ? "header unit"
+ : !is_partition () ? "primary interface"
+ : is_interface () ? "interface partition"
+ : "internal partition");
/* Compiler's version. */
readme.printf ("compiler: %s", version_string);
--
2.30.2
c++: Elide inactive initializer fns from init array
There's no point adding no-op initializer fns (that a module might
have) to the static initializer list. Also, we can add any objc
initializer call to a partial initializer function and simplify some
control flow.
nathan
--
Nathan SidwellFrom c970d0072e3f962afa278e28f918fdcd1b3e755c Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Thu, 16 Jun 2022 10:14:56 -0700
Subject: [PATCH] c++: Elide inactive initializer fns from init array
There's no point adding no-op initializer fns (that a module might
have) to the static initializer list. Also, we can add any objc
initializer call to a partial initializer function and simplify some
control flow.
gcc/cp/
* decl2.cc (finish_objects): Add startp parameter, adjust.
(generate_ctor_or_dtor_function): Detect empty fn, and don't
generate unnecessary code. Remove objc startup here ...
(c_parse_final_cleanyps): ... do it here.
gcc/testsuite/
* g++.dg/modules/init-2_b.C: Add init check.
* g++.dg/modules/init-2_c.C: Add init check.
---
gcc/cp/decl2.cc | 97 +
gcc/testsuite/g++.dg/modules/init-2_b.C | 1 +
gcc/testsuite/g++.dg/modules/init-2_c.C | 1 +
3 files changed, 53 insertions(+), 46 deletions(-)
diff --git a/gcc/cp/decl2.cc b/gcc/cp/decl2.cc
index 0c4492f7354..3737e5f010c 100644
--- a/gcc/cp/decl2.cc
+++ b/gcc/cp/decl2.cc
@@ -56,7 +56,7 @@ int raw_dump_id;
extern cpp_reader *parse_in;
static tree start_objects (bool, unsigned, bool);
-static tree finish_objects (bool, unsigned, tree);
+static tree finish_objects (bool, unsigned, tree, bool = true);
static tree start_partial_init_fini_fn (bool, unsigned, unsigned);
static void finish_partial_init_fini_fn (tree);
static void emit_partial_init_fini_fn (bool, unsigned, tree,
@@ -3932,16 +3932,19 @@ start_objects (bool initp, unsigned priority, bool has_body)
return body;
}
-/* Finish a global constructor or destructor. */
+/* Finish a global constructor or destructor. Add it to the global
+ ctors or dtors, if STARTP is true. */
static tree
-finish_objects (bool initp, unsigned priority, tree body)
+finish_objects (bool initp, unsigned priority, tree body, bool startp)
{
/* Finish up. */
finish_compound_stmt (body);
tree fn = finish_function (/*inline_p=*/false);
- if (initp)
+ if (!startp)
+; // Neither ctor nor dtor I be.
+ else if (initp)
{
DECL_STATIC_CONSTRUCTOR (fn) = 1;
decl_init_priority_insert (fn, priority);
@@ -4307,58 +4310,54 @@ write_out_vars (tree vars)
}
}
-/* Generate a static constructor (if CONSTRUCTOR_P) or destructor
- (otherwise) that will initialize all global objects with static
- storage duration having the indicated PRIORITY. */
+/* Generate a static constructor or destructor that calls the given
+ init/fini fns at the indicated priority. */
static void
generate_ctor_or_dtor_function (bool initp, unsigned priority,
tree fns, location_t locus)
{
input_location = locus;
-
tree body = start_objects (initp, priority, bool (fns));
- /* To make sure dynamic construction doesn't access globals from other
- compilation units where they might not be yet constructed, for
- -fsanitize=address insert __asan_before_dynamic_init call that
- prevents access to either all global variables that need construction
- in other compilation units, or at least those that haven't been
- initialized yet. Variables that need dynamic construction in
- the current compilation unit are kept accessible. */
- if (initp && (flag_sanitize & SANITIZE_ADDRESS))
-finish_expr_stmt (asan_dynamic_init_call (/*after_p=*/false));
-
- if (initp && priority == DEFAULT_INIT_PRIORITY
- && c_dialect_objc () && objc_static_init_needed_p ())
-/* For Objective-C++, we may need to initialize metadata found in
- this module. This must be done _before_ any other static
- initializations. */
-objc_generate_static_init_call (NULL_TREE);
-
- /* Call the static init/fini functions. */
- for (tree node = fns; node; node = TREE_CHAIN (node))
+ if (fns)
{
- tree fn = TREE_PURPOSE (node);
+ /* To make sure dynamic construction doesn't access globals from
+ other compilation units where they might not be yet
+ constructed, for -fsanitize=address insert
+ __asan_before_dynamic_init call that prevents access to
+ either all global variables that need construction in other
+ compilation units, or at least those that haven't been
+ initialized yet. Variables that need dynamic construction in
+ the current compilation unit are kept accessible. */
+ if (initp && (flag_sanitize & SANITIZE_ADDRESS))
+ finish_expr_stmt (asan_dynamic_init_call (/*after_p=*/false));
- // We should never find a pure or constant cdtor.
- gcc_checking_assert (!(flags_from_decl_or_type (fn)
- & (ECF_CONST | ECF_PURE)));
+ /* Call the static init/fini functions. */
+ for (tree node = fns; node; node = TRE
doc: Document module language-linkage supported
I missed we documented this as unimplemented, when I implemented it.
--
Nathan SidwellFrom f1fcd6e3ad911945bc3c24a3a5c7ea99b910121e Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Tue, 21 Jun 2022 06:23:11 -0700
Subject: [PATCH] doc: Document module language-linkage supported
I missed we documented this as unimplemented, when I implemented it.
gcc/
* doc/invoke.texi (C++ Modules): Remove language-linkage
as missing feature.
---
gcc/doc/invoke.texi | 7 ---
1 file changed, 7 deletions(-)
diff --git a/gcc/doc/invoke.texi b/gcc/doc/invoke.texi
index 50f57877477..81d13f4e78e 100644
--- a/gcc/doc/invoke.texi
+++ b/gcc/doc/invoke.texi
@@ -34639,13 +34639,6 @@ Papers p1815 (@uref{https://wg21.link/p1815}) and p2003
exported region may reference (for instance, the entities an exported
template definition may reference). These are not fully implemented.
-@item Language-linkage module attachment
-Declarations with explicit language linkage (@code{extern "C"} or
-@code{extern "C++"}) are attached to the global module, even when in
-the purview of a named module. This is not implemented. Such
-declarations will be attached to the module, if any, in which they are
-declared.
-
@item Standard Library Header Units
The Standard Library is not provided as importable header units. If
you want to import such units, you must explicitly build them first.
--
2.30.2
c++: Remove ifdefed code
The only reason I chose to use DECL_UID on this hash table was to make
it stable against ASLR and perturbations due to other allocations.
It's not required for correctness, as the comment mentions the
equality fn uses pointer identity.
nathan
--
Nathan SidwellFrom d844478ab47a16c8ae65f253fd1cdc685c7951fc Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 22 Jun 2022 07:51:44 -0700
Subject: [PATCH] c++: Remove ifdefed code
The only reason I chose to use DECL_UID on this hash table was to make
it stable against ASLR and perturbations due to other allocations.
It's not required for correctness, as the comment mentions the
equality fn uses pointer identity.
gcc/cp/
* module.cc (struct duplicate_hash): Remove.
(duplicate_hash_map): Adjust.
---
gcc/cp/module.cc | 16 +---
1 file changed, 1 insertion(+), 15 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index b3fbd467ecb..d735d7e8b30 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2829,24 +2829,10 @@ struct merge_key {
}
};
-struct duplicate_hash : nodel_ptr_hash
-{
-#if 0
- /* This breaks variadic bases in the xtreme_header tests. Since ::equal is
- the default pointer_hash::equal, let's use the default hash as well. */
- inline static hashval_t hash (value_type decl)
- {
-if (TREE_CODE (decl) == TREE_BINFO)
- decl = TYPE_NAME (BINFO_TYPE (decl));
-return hashval_t (DECL_UID (decl));
- }
-#endif
-};
-
/* Hashmap of merged duplicates. Usually decls, but can contain
BINFOs. */
typedef hash_map >
+ simple_hashmap_traits,uintptr_t> >
duplicate_hash_map;
/* Tree stream reader. Note that reading a stream doesn't mark the
--
2.30.2
c++: Prune unneeded macro locations
This implements garbage collection on locations within macro
expansions, when streaming out a CMI. When doing the reachability
walks, we now note which macro locations we need and then only write
those locations. The complication here is that every macro expansion
location has an independently calculated offset. This complicates
writing, but reading remains the same -- the macro locations of a CMI
continue to form a contiguous block.
For std headers this reduced the number of macro maps by 40% and the
number of locations by 16%. For a GMF including iostream, it reduced
it by 80% and 60% respectively.
Ordinary locations are still transformed en-mass. They are somewhat
more complicated to apply a similar optimization to.
nathan
--
Nathan SidwellFrom 445cc1266ff8b9b8f96eceafa3c1116da54de967 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Wed, 22 Jun 2022 05:54:30 -0700
Subject: [PATCH] c++: Prune unneeded macro locations
This implements garbage collection on locations within macro
expansions, when streaming out a CMI. When doing the reachability
walks, we now note which macro locations we need and then only write
those locations. The complication here is that every macro expansion
location has an independently calculated offset. This complicates
writing, but reading remains the same -- the macro locations of a CMI
continue to form a contiguous block.
For std headers this reduced the number of macro maps by 40% and the
number of locations by 16%. For a GMF including iostream, it reduced
it by 80% and 60% respectively.
Ordinary locations are still transformed en-mass. They are somewhat
more complicated to apply a similar optimization to.
gcc/cp/
* module.cc (struct macro_info): New.
(struct macro_traits): New.
(macro_remap, macro_table): New globals.
(depset::hash::find_dependencies): Note namespace location.
(module_for_macro_loc): Adjust.
(module_state::note_location): New.
(module_state::Write_location): Note location when not
streaming. Adjust macro location streaming.
(module_state::read_location): Adjust macro location
streaming.
(module_state::write_init_maps): New.
(module_state::write_prepare_maps): Reimplement macro map
preparation.
(module_state::write_macro_maps): Reimplement.
(module_state::read_macro_maps): Likewise.
(module_state::write_begin): Adjust.
gcc/testsuite/
* g++.dg/modules/loc-prune-1.C: New.
* g++.dg/modules/loc-prune-2.C: New.
* g++.dg/modules/loc-prune-3.C: New.
* g++.dg/modules/pr98718_a.C: Adjust.
* g++.dg/modules/pr98718_b.C: Adjust.
---
gcc/cp/module.cc | 349 ++---
gcc/testsuite/g++.dg/modules/loc-prune-1.C | 19 ++
gcc/testsuite/g++.dg/modules/loc-prune-2.C | 14 +
gcc/testsuite/g++.dg/modules/loc-prune-3.C | 16 +
gcc/testsuite/g++.dg/modules/pr98718_a.C | 4 +-
gcc/testsuite/g++.dg/modules/pr98718_b.C | 6 +-
6 files changed, 290 insertions(+), 118 deletions(-)
create mode 100644 gcc/testsuite/g++.dg/modules/loc-prune-1.C
create mode 100644 gcc/testsuite/g++.dg/modules/loc-prune-2.C
create mode 100644 gcc/testsuite/g++.dg/modules/loc-prune-3.C
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index d735d7e8b30..7ee779d06b9 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3238,6 +3238,65 @@ public:
};
static loc_spans spans;
+
+/* Information about macro locations we stream out. */
+struct macro_info
+{
+ const line_map_macro *src;// original expansion
+ unsigned remap; // serialization
+
+ static int compare (const void *a_, const void *b_)
+ {
+auto *a = static_cast (a_);
+auto *b = static_cast (b_);
+
+gcc_checking_assert (MAP_START_LOCATION (a->src)
+ != MAP_START_LOCATION (b->src));
+if (MAP_START_LOCATION (a->src) < MAP_START_LOCATION (b->src))
+ return -1;
+else
+ return +1;
+ }
+};
+struct macro_traits
+{
+ typedef macro_info value_type;
+ typedef const line_map_macro *compare_type;
+
+ static const bool empty_zero_p = false;
+
+ static hashval_t hash (compare_type p)
+ {
+return pointer_hash::hash (p);
+ }
+ static hashval_t hash (const value_type &v)
+ {
+return hash (v.src);
+ }
+ static bool equal (const value_type &v, const compare_type p)
+ {
+return v.src == p;
+ }
+
+ static void mark_empty (value_type &v)
+ {
+v.src = nullptr;
+ }
+ static bool is_empty (value_type &v)
+ {
+return !v.src;
+ }
+
+ static bool is_deleted (value_type &) { return false; }
+ static void mark_deleted (value_type &) { gcc_unreachable (); }
+
+ static void remove (value_type &) {}
+};
+/* Table keyed by line_map_macro, used for noting. */
+static hash_table *macro_table;
+/* Sorted vector, used for writing. */
+static vec *macro_remap;
+
/* Indirection to allow bsearching imports by ordinary location. */
static vec *ool;
@@ -3398,7 +3457,7 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state {
/* Location ranges for this module. adhoc-locs are decomposed, so
c++: Rename macro location structs
The macro location tables should really mention they are about
locations. So rename them. Also, add a missing free of the remapping
table, and remove some now-unneeded macro checking.
nathan
--
Nathan SidwellFrom b0f25e1fdc6199725e69023a3dc49021f311ba66 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Fri, 24 Jun 2022 05:17:24 -0700
Subject: [PATCH] c++: Rename macro location structs
The macro location tables should really mention they are about
locations. So rename them. Also, add a missing free of the remapping
table, and remove some now-unneeded macro checking.
gcc/cp/
* module.cc (macro_info, macro_traits, macro_table,
macro_remap): Rename to ...
(macro_loc_info, macro_loc_traits, macro_loc_table,
macro_loc_remap): ... these. Update all uses.
(module_state::write_prepare_maps): Remove unneeded macro checking.
(module_state::write_begin): Free macro_loc_remap.
---
gcc/cp/module.cc | 73 +---
1 file changed, 25 insertions(+), 48 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 8bb22c2b305..68a7ce53ee4 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -3241,15 +3241,15 @@ public:
static loc_spans spans;
/* Information about macro locations we stream out. */
-struct macro_info
+struct macro_loc_info
{
const line_map_macro *src;// original expansion
unsigned remap; // serialization
static int compare (const void *a_, const void *b_)
{
-auto *a = static_cast (a_);
-auto *b = static_cast (b_);
+auto *a = static_cast (a_);
+auto *b = static_cast (b_);
gcc_checking_assert (MAP_START_LOCATION (a->src)
!= MAP_START_LOCATION (b->src));
@@ -3259,9 +3259,9 @@ struct macro_info
return +1;
}
};
-struct macro_traits
+struct macro_loc_traits
{
- typedef macro_info value_type;
+ typedef macro_loc_info value_type;
typedef const line_map_macro *compare_type;
static const bool empty_zero_p = false;
@@ -3294,9 +3294,9 @@ struct macro_traits
static void remove (value_type &) {}
};
/* Table keyed by line_map_macro, used for noting. */
-static hash_table *macro_table;
+static hash_table *macro_loc_table;
/* Sorted vector, used for writing. */
-static vec *macro_remap;
+static vec *macro_loc_remap;
/* Indirection to allow bsearching imports by ordinary location. */
static vec *ool;
@@ -15616,7 +15616,7 @@ module_state::imported_from () const
void
module_state::note_location (location_t loc)
{
- if (!macro_table)
+ if (!macro_loc_table)
;
else if (loc < RESERVED_LOCATION_COUNT)
;
@@ -15635,9 +15635,9 @@ module_state::note_location (location_t loc)
{
const line_map *map = linemap_lookup (line_table, loc);
const line_map_macro *mac_map = linemap_check_macro (map);
- hashval_t hv = macro_traits::hash (mac_map);
- macro_info *slot
- = macro_table->find_slot_with_hash (mac_map, hv, INSERT);
+ hashval_t hv = macro_loc_traits::hash (mac_map);
+ macro_loc_info *slot
+ = macro_loc_table->find_slot_with_hash (mac_map, hv, INSERT);
if (!slot->src)
{
slot->src = mac_map;
@@ -15698,11 +15698,11 @@ module_state::write_location (bytes_out &sec, location_t loc)
}
else if (loc >= LINEMAPS_MACRO_LOWEST_LOCATION (line_table))
{
- const macro_info *info = nullptr;
+ const macro_loc_info *info = nullptr;
unsigned offset = 0;
- if (unsigned hwm = macro_remap->length ())
+ if (unsigned hwm = macro_loc_remap->length ())
{
- info = macro_remap->begin ();
+ info = macro_loc_remap->begin ();
while (hwm != 1)
{
unsigned mid = hwm / 2;
@@ -15909,7 +15909,7 @@ module_state::read_location (bytes_in &sec) const
void
module_state::write_init_maps ()
{
- macro_table = new hash_table (EXPERIMENT (1, 400));
+ macro_loc_table = new hash_table (EXPERIMENT (1, 400));
}
location_map_info
@@ -15955,30 +15955,6 @@ module_state::write_prepare_maps (module_state_config *cfg)
info.num_maps.first += omap - fmap;
}
-
- if (span.macro.first != span.macro.second)
- {
- /* Iterate over the span's macros, to elide the empty
- expansions. */
- unsigned count = 0;
- for (unsigned macro
- = linemap_lookup_macro_index (line_table,
- span.macro.second - 1);
- macro < LINEMAPS_MACRO_USED (line_table);
- macro++)
- {
- line_map_macro const *mmap
- = LINEMAPS_MACRO_MAP_AT (line_table, macro);
- if (MAP_START_LOCATION (mmap) < span.macro.first)
- /* Fallen out of the span. */
- break;
-
- if (mmap->n_tokens)
- count++;
- }
- dump (dumper::LOCATION) && dump ("Span:%u %u macro maps", ix, count);
- info.num_maps.second += count;
- }
}
/* Adjust the maps. Ordinary ones ascend, and we must maintain
@@ -16024,23 +16000,23 @@ module_state::write_prepare_maps (module_state_config *cfg)
ord_off = span.ordinary.second + span.ordinary_delta;
}
- vec_alloc (macro_remap, macro_
c++: Note macro locations
In order to prune ordinary locations, we need to note the locations of
macros we'll be writing out. This reaaranges the macro processing to
achieve that. Also drop an unneeded parameter from macro reading & writing.
Fix some it's/its errors.
nathan
--
Nathan SidwellFrom 47e36785cd2ba35a577b0678a2ac185288eb9e52 Mon Sep 17 00:00:00 2001
From: Nathan Sidwell
Date: Mon, 27 Jun 2022 07:51:12 -0700
Subject: [PATCH] c++: Note macro locations
In order to prune ordinary locations, we need to note the locations of
macros we'll be writing out. This rearanges the macro processing to achieve
that. Also drop an unneeded parameter from macro reading & writing.
Fix some it's/its errors.
gcc/cp/
* module.cc (module_state::write_define): Drop located param.
(module_state::read_define): Likewise.
(module_state::prepare_macros): New, broken out of ...
(module_state::write_macros): ... here. Adjust.
(module_state::write_begin): Adjust.
gcc/testsuite/
* g++.dg/modules/inext-1.H: Check include-next happened.
---
gcc/cp/module.cc | 98 +-
gcc/testsuite/g++.dg/modules/inext-1.H | 1 +
2 files changed, 67 insertions(+), 32 deletions(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 68a7ce53ee4..238a5eb74d2 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -2281,7 +2281,7 @@ public:
EK_EXPLICIT_HWM,
EK_BINDING = EK_EXPLICIT_HWM, /* Implicitly encoded. */
EK_FOR_BINDING, /* A decl being inserted for a binding. */
-EK_INNER_DECL, /* A decl defined outside of it's imported
+EK_INNER_DECL, /* A decl defined outside of its imported
context. */
EK_DIRECT_HWM = EK_PARTIAL + 1,
@@ -3663,9 +3663,10 @@ class GTY((chain_next ("%h.parent"), for_user)) module_state {
bool read_macro_maps (unsigned);
private:
- void write_define (bytes_out &, const cpp_macro *, bool located = true);
- cpp_macro *read_define (bytes_in &, cpp_reader *, bool located = true) const;
- unsigned write_macros (elf_out *to, cpp_reader *, unsigned *crc_ptr);
+ void write_define (bytes_out &, const cpp_macro *);
+ cpp_macro *read_define (bytes_in &, cpp_reader *) const;
+ vec *prepare_macros (cpp_reader *);
+ unsigned write_macros (elf_out *to, vec *, unsigned *crc_ptr);
bool read_macros ();
void install_macros ();
@@ -7136,7 +7137,7 @@ trees_in::tree_node_vals (tree t)
}
-/* If T is a back reference, fixed reference or NULL, write out it's
+/* If T is a back reference, fixed reference or NULL, write out its
code and return WK_none. Otherwise return WK_value if we must write
by value, or WK_normal otherwise. */
@@ -10605,7 +10606,7 @@ trees_out::key_mergeable (int tag, merge_kind mk, tree decl, tree inner,
/* DECL is a new declaration that may be duplicated in OVL. Use RET &
ARGS to find its clone, or NULL. If DECL's DECL_NAME is NULL, this
- has been found by a proxy. It will be an enum type located by it's
+ has been found by a proxy. It will be an enum type located by its
first member.
We're conservative with matches, so ambiguous decls will be
@@ -11615,7 +11616,7 @@ trees_in::read_var_def (tree decl, tree maybe_template)
}
/* If MEMBER doesn't have an independent life outside the class,
- return it (or it's TEMPLATE_DECL). Otherwise NULL. */
+ return it (or its TEMPLATE_DECL). Otherwise NULL. */
static tree
member_owned_by_class (tree member)
@@ -15405,7 +15406,7 @@ module_state::read_entities (unsigned count, unsigned lwm, unsigned hwm)
sure the specified entities are loaded.
An optimization might be to have a flag in each key-entity saying
- that it's top key might be in the entity table. It's not clear to
+ that its top key might be in the entity table. It's not clear to
me how to set that flag cheaply -- cheaper than just looking.
FIXME: It'd be nice to have a bit in decls to tell us whether to
@@ -16444,7 +16445,7 @@ module_state::read_macro_maps (unsigned num_macro_locs)
/* Serialize the definition of MACRO. */
void
-module_state::write_define (bytes_out &sec, const cpp_macro *macro, bool located)
+module_state::write_define (bytes_out &sec, const cpp_macro *macro)
{
sec.u (macro->count);
@@ -16453,8 +16454,7 @@ module_state::write_define (bytes_out &sec, const cpp_macro *macro, bool located
sec.b (macro->syshdr);
sec.bflush ();
- if (located)
-write_location (sec, macro->line);
+ write_location (sec, macro->line);
if (macro->fun_like)
{
sec.u (macro->paramc);
@@ -16467,8 +16467,7 @@ module_state::write_define (bytes_out &sec, const cpp_macro *macro, bool located
for (unsigned ix = 0; ix != macro->count; ix++)
{
const cpp_token *token = ¯o->exp.tokens[ix];
- if (located)
- write_location (sec, token->src_loc);
+ write_location (sec, token->src_loc);
sec.u (token->type);
sec.u (token->flags);
switch (cpp_token_val_index (token))
@@ -16533,11 +16532
Re: PING [PATCH v3] c++: Allow module name to be a single letter on Windows
On 11/25/22 14:03, Torbjorn SVENSSON wrote:
Hi,
Ping, https://gcc.gnu.org/pipermail/gcc-patches/2022-November/606528.html
ok, thanks!
Kind regards,
Torbjörn
On 2022-11-17 14:20, Torbjörn SVENSSON wrote:
v1 -> v2:
Paths without "C:" part can still be absolute if they start with / or
\ on Windows.
v2 -> v3:
Use alternative approach by having platform specific code in module.cc.
Truth table for the new expression:
c:\foo -> true
c:/foo -> true
/foo -> true
\foo -> true
c:foo -> false
foo -> false
./foo -> true
.\foo -> true
Ok for trunk?
---
On Windows, the ':' character is special and when the module name is
a single character, like 'A', then the flatname would be (for
example) 'A:Foo'. On Windows, 'A:Foo' is treated as an absolute
path by the module loader and is likely not found.
Without this patch, the test case pr98944_c.C fails with:
In module imported at /src/gcc/testsuite/g++.dg/modules/pr98944_b.C:7:1,
of module A:Foo, imported at /src/gcc/testsuite/g++.dg/modules/pr98944_c.C:7:
A:Internals: error: header module expected, module 'A:Internals' found
A:Internals: error: failed to read compiled module: Bad file data
A:Internals: note: compiled module file is 'gcm.cache/A-Internals.gcm'
In module imported at /src/gcc/testsuite/g++.dg/modules/pr98944_c.C:7:8:
A:Foo: error: failed to read compiled module: Bad import dependency
A:Foo: note: compiled module file is 'gcm.cache/A-Foo.gcm'
A:Foo: fatal error: returning to the gate for a mechanical issue
compilation terminated.
gcc/cp/ChangeLog:
* module.cc: On Windows, 'A:Foo' is supposed to be a module
and not a path.
Tested on Windows with arm-none-eabi for Cortex-M3 in gcc-11 tree.
Co-Authored-By: Yvan ROUX
Signed-off-by: Torbjörn SVENSSON
---
gcc/cp/module.cc | 10 +-
1 file changed, 9 insertions(+), 1 deletion(-)
diff --git a/gcc/cp/module.cc b/gcc/cp/module.cc
index 0e9af318ba4..fa41a86213f 100644
--- a/gcc/cp/module.cc
+++ b/gcc/cp/module.cc
@@ -13960,7 +13960,15 @@ get_module (tree name, module_state *parent, bool
partition)
static module_state *
get_module (const char *ptr)
{
- if (ptr[0] == '.' ? IS_DIR_SEPARATOR (ptr[1]) : IS_ABSOLUTE_PATH (ptr))
+ /* On DOS based file systems, there is an ambiguity with A:B which can be
+ interpreted as a module Module:Partition or Drive:PATH. Interpret strings
+ which clearly starts as pathnames as header-names and everything else is
+ treated as a (possibly malformed) named moduled. */
+ if (IS_DIR_SEPARATOR (ptr[ptr[0] == '.']) // ./FOO or /FOO
+#if HAVE_DOS_BASED_FILE_SYSTEM
+ || (HAS_DRIVE_SPEC (ptr) && IS_DIR_SEPARATOR (ptr[2])) // A:/FOO
+#endif
+ || false)
/* A header name. */
return get_module (build_string (strlen (ptr), ptr));
--
Nathan Sidwell
Re: [PATCH] c++: modules and std::source_location::current() def arg [PR100881]
On 12/7/22 16:50, Patrick Palka wrote: We currently declare __builtin_source_location with a const void* return type instead of the true type (const std::source_location::__impl*), and later when folding this builtin we just obtain the true type via name lookup. But the below testcase demonstrates this name lookup approach seems to interact poorly with modules, since we may import an entity that uses std::source_location::current() in a default argument (or DMI) without also importing , and thus the name lookup will fail when folding the builtin at the call site unless we also import . This patch fixes by instead initially declaring __builtin_source_location with an auto return type and updating it appropriately upon its first use. Thus when folding calls to this builtin we can fish out the true return type through the type of the CALL_EXPR and avoid needing to do name lookup. That's a clever approach! LGTM nathan -- Nathan Sidwell
Re: [PATCH] c++: Fix Solaris bootstraps across midnight
On 4/11/23 04:12, Jakub Jelinek wrote: Hi! When working on the PR109040 fix, I wanted to test it on some WORD_REGISTER_OPERATIONS target and tried sparc-solaris on GCC Farm. My bootstrap failed in comparison failure on cp/module.o, because Solaris date doesn't support the -r option and one stage's cp/module.o was built before midnight and next stage's cp/module.o after midnight, so they had different -DMODULE_VERSION= value. Now, I think the advice (don't bootstrap at midnight) is something we shouldn't have, so the following patch stores the module version (still generated through the same way, date -r cp/module.cc if it works, otherwise just date) into a temporary file, makes sure that temporary file is updated when cp/module.cc source is updated and when date -r doesn't work copies file from previous stage if it is newer than cp/module.cc. looks good. one could tweak it slightly to avoid the MODULE_VERSION variable (but then I was the original lazy one!) Something like: s-cp-module-version: $(srcdir)/cp/module.cc if date -r $(srcdir)/cp/module.cc '+%y%m%d%H%MU' \ 2>/dev/null >$@; then :; \ elif test ../prev-gcc/$@ -nt \ $(srcdir)/cp/module.cc; then \ cp ../prev-gcc/$@ $@; \ else \ date '+%y%m%dU' 2>/dev/null >$@; \ fi`; \ nathan -- Nathan Sidwell
Re: [PATCH] c++: Fix Solaris bootstraps across midnight
Jakub, for avoidance of doubt, your version is fine. nathan On 4/11/23 18:06, Nathan Sidwell wrote: On 4/11/23 04:12, Jakub Jelinek wrote: Hi! When working on the PR109040 fix, I wanted to test it on some WORD_REGISTER_OPERATIONS target and tried sparc-solaris on GCC Farm. My bootstrap failed in comparison failure on cp/module.o, because Solaris date doesn't support the -r option and one stage's cp/module.o was built before midnight and next stage's cp/module.o after midnight, so they had different -DMODULE_VERSION= value. Now, I think the advice (don't bootstrap at midnight) is something we shouldn't have, so the following patch stores the module version (still generated through the same way, date -r cp/module.cc if it works, otherwise just date) into a temporary file, makes sure that temporary file is updated when cp/module.cc source is updated and when date -r doesn't work copies file from previous stage if it is newer than cp/module.cc. looks good. one could tweak it slightly to avoid the MODULE_VERSION variable (but then I was the original lazy one!) Something like: s-cp-module-version: $(srcdir)/cp/module.cc if date -r $(srcdir)/cp/module.cc '+%y%m%d%H%MU' \ 2>/dev/null >$@; then :; \ elif test ../prev-gcc/$@ -nt \ $(srcdir)/cp/module.cc; then \ cp ../prev-gcc/$@ $@; \ else \ date '+%y%m%dU' 2>/dev/null >$@; \ fi`; \ nathan -- Nathan Sidwell
