* [PATCH] Fixes canonical types PRs c++/33977, c++/33886
@ 2007-11-05 15:53 Doug Gregor
2007-11-05 23:28 ` Mark Mitchell
0 siblings, 1 reply; 3+ messages in thread
From: Doug Gregor @ 2007-11-05 15:53 UTC (permalink / raw)
To: GCC Patches, Mark Mitchell
[-- Attachment #1: Type: text/plain, Size: 1776 bytes --]
(Mark, I'm directing this to you since you're the only maintainer who
has studied the canonical types system.)
This patches fixes PRs c++/33977 (P1) and c++/33886 (P3), both of
which stem from the same problem with canonical types and arrays. I'm
continually amazed by the sheer number of places where we copy,
create, and munge ARRAY_TYPE nodes. Here, I stumbled upon the
duplicate code for creating cv-qualified (array) types in C
(c_build_qualified_type) and C++ (cp_build_qualified_type). The latter
handles canonical types for cv-qualified arrays, because C++ uses
canonical types; the former does not. However, through
cp_complete_array_type and its call to complete_array_type, the C++
front end can actually end up calling c_build_qualified_type,
thwarting the canonical types system. Gaaa!
The fix, in this case, was to move c_build_qualified_type into
c-typeck.c, so it will only be available in the C front end. Then, the
C++ front end defines its own c_build_qualified_type, which merely
redirects to cp_build_qualified_type.
Tested i686-pc-linux-gnu. Okay for mainline?
- Doug
2007-11-05 Douglas Gregor <doug.gregor@gmail.com>
PR c++/33977
PR c++/33886
* tree.c (c_build_qualified_type): Define bridge to
cp_build_qualified_type.
2007-11-05 Douglas Gregor <doug.gregor@gmail.com>
PR c++/33977
PR c++/33886
* c-common.c (c_build_qualified_type): Moved to c-typeck.c.
(complete_array_type): Set canonical type appropriately.
* c-typeck.c (c_build_qualified_type): Moved from c-common.c. The
C and C++ front ends now have different versions of this function,
because the C++ version needs to maintain canonical types here.
2007-11-05 Douglas Gregor <doug.gregor@gmail.com>
PR c++/33977
PR c++/33886
* g++.dg/other/canon-array.C: New.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: canon-array.patch --]
[-- Type: text/x-patch; name=canon-array.patch, Size: 6704 bytes --]
Index: testsuite/g++.dg/other/canon-array.C
===================================================================
--- testsuite/g++.dg/other/canon-array.C (revision 0)
+++ testsuite/g++.dg/other/canon-array.C (revision 0)
@@ -0,0 +1,4 @@
+// PR c++/33977
+typedef char sal_Char;
+const sal_Char sHTML[] = "HTML";
+extern const char sHTML[];
Index: cp/tree.c
===================================================================
--- cp/tree.c (revision 129899)
+++ cp/tree.c (working copy)
@@ -649,6 +649,14 @@ cp_build_reference_type (tree to_type, b
}
+/* Used by the C++ front end to build qualified array types. However,
+ the C version of this function does not properly maintain canonical
+ types (which are not used in C). */
+tree
+c_build_qualified_type (tree type, int type_quals)
+{
+ return cp_build_qualified_type (type, type_quals);
+}
\f
/* Make a variant of TYPE, qualified with the TYPE_QUALS. Handles
Index: c-typeck.c
===================================================================
--- c-typeck.c (revision 129899)
+++ c-typeck.c (working copy)
@@ -8896,3 +8896,68 @@ c_finish_omp_clauses (tree clauses)
bitmap_obstack_release (NULL);
return clauses;
}
+
+/* Make a variant type in the proper way for C/C++, propagating qualifiers
+ down to the element type of an array. */
+
+tree
+c_build_qualified_type (tree type, int type_quals)
+{
+ if (type == error_mark_node)
+ return type;
+
+ if (TREE_CODE (type) == ARRAY_TYPE)
+ {
+ tree t;
+ tree element_type = c_build_qualified_type (TREE_TYPE (type),
+ type_quals);
+
+ /* See if we already have an identically qualified type. */
+ for (t = TYPE_MAIN_VARIANT (type); t; t = TYPE_NEXT_VARIANT (t))
+ {
+ if (TYPE_QUALS (strip_array_types (t)) == type_quals
+ && TYPE_NAME (t) == TYPE_NAME (type)
+ && TYPE_CONTEXT (t) == TYPE_CONTEXT (type)
+ && attribute_list_equal (TYPE_ATTRIBUTES (t),
+ TYPE_ATTRIBUTES (type)))
+ break;
+ }
+ if (!t)
+ {
+ tree domain = TYPE_DOMAIN (type);
+
+ t = build_variant_type_copy (type);
+ TREE_TYPE (t) = element_type;
+
+ if (TYPE_STRUCTURAL_EQUALITY_P (element_type)
+ || (domain && TYPE_STRUCTURAL_EQUALITY_P (domain)))
+ SET_TYPE_STRUCTURAL_EQUALITY (t);
+ else if (TYPE_CANONICAL (element_type) != element_type
+ || (domain && TYPE_CANONICAL (domain) != domain))
+ {
+ tree unqualified_canon
+ = build_array_type (TYPE_CANONICAL (element_type),
+ domain? TYPE_CANONICAL (domain)
+ : NULL_TREE);
+ TYPE_CANONICAL (t)
+ = c_build_qualified_type (unqualified_canon, type_quals);
+ }
+ else
+ TYPE_CANONICAL (t) = t;
+ }
+ return t;
+ }
+
+ /* A restrict-qualified pointer type must be a pointer to object or
+ incomplete type. Note that the use of POINTER_TYPE_P also allows
+ REFERENCE_TYPEs, which is appropriate for C++. */
+ if ((type_quals & TYPE_QUAL_RESTRICT)
+ && (!POINTER_TYPE_P (type)
+ || !C_TYPE_OBJECT_OR_INCOMPLETE_P (TREE_TYPE (type))))
+ {
+ error ("invalid use of %<restrict%>");
+ type_quals &= ~TYPE_QUAL_RESTRICT;
+ }
+
+ return build_qualified_type (type, type_quals);
+}
Index: c-common.c
===================================================================
--- c-common.c (revision 129899)
+++ c-common.c (working copy)
@@ -3086,70 +3086,6 @@ static void def_builtin_1 (enum built_i
bool both_p, bool fallback_p, bool nonansi_p,
tree fnattrs, bool implicit_p);
-/* Make a variant type in the proper way for C/C++, propagating qualifiers
- down to the element type of an array. */
-
-tree
-c_build_qualified_type (tree type, int type_quals)
-{
- if (type == error_mark_node)
- return type;
-
- if (TREE_CODE (type) == ARRAY_TYPE)
- {
- tree t;
- tree element_type = c_build_qualified_type (TREE_TYPE (type),
- type_quals);
-
- /* See if we already have an identically qualified type. */
- for (t = TYPE_MAIN_VARIANT (type); t; t = TYPE_NEXT_VARIANT (t))
- {
- if (TYPE_QUALS (strip_array_types (t)) == type_quals
- && TYPE_NAME (t) == TYPE_NAME (type)
- && TYPE_CONTEXT (t) == TYPE_CONTEXT (type)
- && attribute_list_equal (TYPE_ATTRIBUTES (t),
- TYPE_ATTRIBUTES (type)))
- break;
- }
- if (!t)
- {
- tree domain = TYPE_DOMAIN (type);
-
- t = build_variant_type_copy (type);
- TREE_TYPE (t) = element_type;
-
- if (TYPE_STRUCTURAL_EQUALITY_P (element_type)
- || (domain && TYPE_STRUCTURAL_EQUALITY_P (domain)))
- SET_TYPE_STRUCTURAL_EQUALITY (t);
- else if (TYPE_CANONICAL (element_type) != element_type
- || (domain && TYPE_CANONICAL (domain) != domain))
- {
- tree unqualified_canon
- = build_array_type (TYPE_CANONICAL (element_type),
- domain? TYPE_CANONICAL (domain)
- : NULL_TREE);
- TYPE_CANONICAL (t)
- = c_build_qualified_type (unqualified_canon, type_quals);
- }
- else
- TYPE_CANONICAL (t) = t;
- }
- return t;
- }
-
- /* A restrict-qualified pointer type must be a pointer to object or
- incomplete type. Note that the use of POINTER_TYPE_P also allows
- REFERENCE_TYPEs, which is appropriate for C++. */
- if ((type_quals & TYPE_QUAL_RESTRICT)
- && (!POINTER_TYPE_P (type)
- || !C_TYPE_OBJECT_OR_INCOMPLETE_P (TREE_TYPE (type))))
- {
- error ("invalid use of %<restrict%>");
- type_quals &= ~TYPE_QUAL_RESTRICT;
- }
-
- return build_qualified_type (type, type_quals);
-}
/* Apply the TYPE_QUALS to the new DECL. */
@@ -7044,6 +6980,19 @@ complete_array_type (tree *ptype, tree i
hashcode);
main_type = type_hash_canon (hashcode, main_type);
+ /* Fix the canonical type. */
+ if (TYPE_STRUCTURAL_EQUALITY_P (TREE_TYPE (main_type))
+ || TYPE_STRUCTURAL_EQUALITY_P (TYPE_DOMAIN (main_type)))
+ SET_TYPE_STRUCTURAL_EQUALITY (main_type);
+ else if (TYPE_CANONICAL (TREE_TYPE (main_type)) != TREE_TYPE (main_type)
+ || (TYPE_CANONICAL (TYPE_DOMAIN (main_type))
+ != TYPE_DOMAIN (main_type)))
+ TYPE_CANONICAL (main_type)
+ = build_array_type (TYPE_CANONICAL (TREE_TYPE (main_type)),
+ TYPE_CANONICAL (TYPE_DOMAIN (main_type)));
+ else
+ TYPE_CANONICAL (main_type) = main_type;
+
if (quals == 0)
type = main_type;
else
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fixes canonical types PRs c++/33977, c++/33886
2007-11-05 15:53 [PATCH] Fixes canonical types PRs c++/33977, c++/33886 Doug Gregor
@ 2007-11-05 23:28 ` Mark Mitchell
2007-11-06 14:48 ` Doug Gregor
0 siblings, 1 reply; 3+ messages in thread
From: Mark Mitchell @ 2007-11-05 23:28 UTC (permalink / raw)
To: Doug Gregor; +Cc: GCC Patches
Doug Gregor wrote:
> 2007-11-05 Douglas Gregor <doug.gregor@gmail.com>
>
> PR c++/33977
> PR c++/33886
> * tree.c (c_build_qualified_type): Define bridge to
> cp_build_qualified_type.
>
> 2007-11-05 Douglas Gregor <doug.gregor@gmail.com>
>
> PR c++/33977
> PR c++/33886
> * c-common.c (c_build_qualified_type): Moved to c-typeck.c.
> (complete_array_type): Set canonical type appropriately.
> * c-typeck.c (c_build_qualified_type): Moved from c-common.c. The
> C and C++ front ends now have different versions of this function,
> because the C++ version needs to maintain canonical types here.
>
> 2007-11-05 Douglas Gregor <doug.gregor@gmail.com>
>
> PR c++/33977
> PR c++/33886
> * g++.dg/other/canon-array.C: New.
OK.
(I suppose that what this is really telling us is that we should make
the C front end use canonical types too; after all, our goal is to share
as much code as possible between the C and C++ front ends. But, that's
certainly too big a thing to try to change now.)
Thanks,
--
Mark Mitchell
CodeSourcery
mark@codesourcery.com
(650) 331-3385 x713
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH] Fixes canonical types PRs c++/33977, c++/33886
2007-11-05 23:28 ` Mark Mitchell
@ 2007-11-06 14:48 ` Doug Gregor
0 siblings, 0 replies; 3+ messages in thread
From: Doug Gregor @ 2007-11-06 14:48 UTC (permalink / raw)
To: Mark Mitchell; +Cc: GCC Patches
On Nov 5, 2007 6:28 PM, Mark Mitchell <mark@codesourcery.com> wrote:
> > 2007-11-05 Douglas Gregor <doug.gregor@gmail.com>
> >
> > PR c++/33977
> > PR c++/33886
> > * c-common.c (c_build_qualified_type): Moved to c-typeck.c.
> > (complete_array_type): Set canonical type appropriately.
> > * c-typeck.c (c_build_qualified_type): Moved from c-common.c. The
> > C and C++ front ends now have different versions of this function,
> > because the C++ version needs to maintain canonical types here.
> OK.
Thanks!
> (I suppose that what this is really telling us is that we should make
> the C front end use canonical types too; after all, our goal is to share
> as much code as possible between the C and C++ front ends. But, that's
> certainly too big a thing to try to change now.)
Eventually, yes, but not in this release cycle. However, I don't
expect us to see a big win in compile time from canonical types in C,
since C uses many fewer same-type comparisons than C++. The biggest
wins in C++ come from, e.g., matching template specializations, where
it's type equality (not type conversions) that matter.
- Doug
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2007-11-06 14:48 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2007-11-05 15:53 [PATCH] Fixes canonical types PRs c++/33977, c++/33886 Doug Gregor
2007-11-05 23:28 ` Mark Mitchell
2007-11-06 14:48 ` Doug Gregor
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).