* [c++-concepts] Check function concept definitions
@ 2014-09-29 14:55 Andrew Sutton
2014-09-29 15:07 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Sutton @ 2014-09-29 14:55 UTC (permalink / raw)
To: gcc-patches, Jason Merrill, Roland Bock
[-- Attachment #1: Type: text/plain, Size: 799 bytes --]
This fixes an ICE trying to normalize a function concept with multiple
statements. That error will now be diagnosed at the point of
definition.
Jason, do you want to review this before I commit? This is a pretty small patch.
2014-09-01 Andrew Sutton <andrew.n.sutton@gmail.com>
Check requirements on function concept definitions.
* gcc/cp/decl.c (finish_function): Check properties of a function
concept definition.
* gcc/cp/constraint.cc (check_function_concept): New. Check
for deduced return type and multiple statements.
(normalize_misc): Don't normalize multiple statements.
(normalize_stmt_list): Removed.
* gcc/cp/cp-tree.h (check_function_concept): New.
* gcc/testsuite/g++.dg/concepts/fn-concept1.C: New.
Andrew
[-- Attachment #2: fn-concept.patch --]
[-- Type: text/x-patch, Size: 4140 bytes --]
Index: cp/cp-tree.h
===================================================================
--- cp/cp-tree.h (revision 214991)
+++ cp/cp-tree.h (working copy)
@@ -6444,6 +6444,7 @@ extern tree build_concept_check
extern tree build_constrained_parameter (tree, tree, tree = NULL_TREE);
extern bool deduce_constrained_parameter (tree, tree&, tree&);
extern tree resolve_constraint_check (tree);
+extern tree check_function_concept (tree);
extern tree finish_concept_introduction (tree, tree);
extern tree finish_template_constraints (tree);
Index: cp/decl.c
===================================================================
--- cp/decl.c (revision 214268)
+++ cp/decl.c (working copy)
@@ -14360,6 +14360,10 @@ finish_function (int flags)
fntype = TREE_TYPE (fndecl);
}
+ // If this is a concept, check that the definition is reasonable.
+ if (DECL_DECLARED_CONCEPT_P (fndecl))
+ check_function_concept (fndecl);
+
/* Save constexpr function body before it gets munged by
the NRV transformation. */
maybe_save_function_definition (fndecl);
Index: cp/constraint.cc
===================================================================
--- cp/constraint.cc (revision 214991)
+++ cp/constraint.cc (working copy)
@@ -269,6 +269,35 @@ deduce_concept_introduction (tree expr)
gcc_unreachable ();
}
+
+// -------------------------------------------------------------------------- //
+// Declarations
+
+// Check that FN satisfies the structural requirements of a
+// function concept definition.
+tree
+check_function_concept (tree fn)
+{
+ location_t loc = DECL_SOURCE_LOCATION (fn);
+
+ // If fn was declared with auto, make sure the result type is bool.
+ if (FNDECL_USED_AUTO (fn) && TREE_TYPE (fn) != boolean_type_node)
+ error_at (loc, "deduced type of concept definition %qD is not %qT",
+ fn, boolean_type_node);
+
+ // Check that the function is comprised of only a single
+ // return statements.
+ tree body = DECL_SAVED_TREE (fn);
+ if (TREE_CODE (body) == BIND_EXPR)
+ body = BIND_EXPR_BODY (body);
+ if (TREE_CODE (body) != RETURN_EXPR)
+ error_at (loc, "function concept definition %qD has multiple statements",
+ fn);
+
+ return NULL_TREE;
+}
+
+
// -------------------------------------------------------------------------- //
// Normalization
//
@@ -425,9 +454,10 @@ normalize_misc (tree t)
case CONSTRUCTOR:
return t;
- case STATEMENT_LIST:
- return normalize_stmt_list (t);
-
+ // This should have been caught as an error.
+ case STATEMENT_LIST:
+ return NULL_TREE;
+
default:
gcc_unreachable ();
}
@@ -630,28 +660,6 @@ normalize_requires (tree t)
return t;
}
-// Reduction rules for the statement list STMTS.
-//
-// Recursively reduce each statement in the list, concatenating each
-// reduced result into a conjunction of requirements.
-//
-// A constexpr function may include statements other than a return
-// statement. The primary purpose of these rules is to filter those
-// non-return statements from the constraints language.
-tree
-normalize_stmt_list (tree stmts)
-{
- tree lhs = NULL_TREE;
- tree_stmt_iterator i = tsi_start (stmts);
- while (!tsi_end_p (i))
- {
- if (tree rhs = normalize_node (tsi_stmt (i)))
- lhs = conjoin_constraints (lhs, rhs);
- tsi_next (&i);
- }
- return lhs;
-}
-
// Normalize a cast expression.
tree
normalize_cast (tree t)
@@ -686,6 +694,7 @@ normalize_constraints (tree reqs)
++processing_template_decl;
tree expr = normalize_node (reqs);
--processing_template_decl;
+
return expr;
}
Index: testsuite/g++.dg/concepts/fn-concept1.C
===================================================================
--- testsuite/g++.dg/concepts/fn-concept1.C (revision 0)
+++ testsuite/g++.dg/concepts/fn-concept1.C (revision 0)
@@ -0,0 +1,9 @@
+// { dg-options "-std=c++1z" }
+
+template<typename T>
+ concept bool Tuple() { // { dg-error "multiple statements" }
+ static_assert(T::value, "");
+ return true;
+ }
+
+ void f(Tuple&);
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [c++-concepts] Check function concept definitions
2014-09-29 14:55 [c++-concepts] Check function concept definitions Andrew Sutton
@ 2014-09-29 15:07 ` Jason Merrill
2014-09-29 15:46 ` Andrew Sutton
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2014-09-29 15:07 UTC (permalink / raw)
To: Andrew Sutton, gcc-patches, Roland Bock
On 09/29/2014 10:55 AM, Andrew Sutton wrote:
> Jason, do you want to review this before I commit? This is a pretty small patch.
No need to wait for review before committing to the branch.
> + // If fn was declared with auto, make sure the result type is bool.
> + if (FNDECL_USED_AUTO (fn) && TREE_TYPE (fn) != boolean_type_node)
> + error_at (loc, "deduced type of concept definition %qD is not %qT",
> + fn, boolean_type_node);
This should say what the deduced type is.
> + // Check that the function is comprised of only a single
> + // return statements.
*statement
> +template<typename T>
> + concept bool Tuple() { // { dg-error "multiple statements" }
> + static_assert(T::value, "");
> + return true;
> + }
Hmm, have we actually discussed this in core review? I'm not seeing it
on the wiki. Constexpr started out this way too, and allowing
static_assert was added pretty fast. C++11 said
its function-body shall be = delete, = default, or a compound-statement
that contains only
— null statements,
— static_assert-declarations
— typedef declarations and alias-declarations that do not define classes
or enumerations,
— using-declarations,
— using-directives,
— and exactly one return statement;
Is there a reason we want to be more strict than this for concept functions?
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [c++-concepts] Check function concept definitions
2014-09-29 15:07 ` Jason Merrill
@ 2014-09-29 15:46 ` Andrew Sutton
2014-09-29 16:32 ` Jason Merrill
0 siblings, 1 reply; 5+ messages in thread
From: Andrew Sutton @ 2014-09-29 15:46 UTC (permalink / raw)
To: Jason Merrill; +Cc: gcc-patches, Roland Bock
> Hmm, have we actually discussed this in core review? I'm not seeing it on
> the wiki. Constexpr started out this way too, and allowing static_assert
> was added pretty fast. C++11 said
>
> its function-body shall be = delete, = default, or a compound-statement that
> contains only
> — null statements,
> — static_assert-declarations
> — typedef declarations and alias-declarations that do not define classes or
> enumerations,
> — using-declarations,
> — using-directives,
> — and exactly one return statement;
>
> Is there a reason we want to be more strict than this for concept functions?
I don't remember much controversy on that particular limitation in
either Rapperswil or the previous telecon review.
The main reason for the restriction is that concept definitions are
normalized into a single constraint-expression. And it's not obvious
how things like using declarations and static-assertions should be
interpreted within the constraint language.
That said, having a static_assert inside a concept kind of defeats the
purpose since it triggers a diagnostic when its condition isn't
satisfied. That's not very SFINAE friendly :)
Maybe the restriction can relaxed when we consider the TS for adoption in 17.
Andrew
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [c++-concepts] Check function concept definitions
2014-09-29 15:46 ` Andrew Sutton
@ 2014-09-29 16:32 ` Jason Merrill
2014-09-29 17:50 ` Roland Bock
0 siblings, 1 reply; 5+ messages in thread
From: Jason Merrill @ 2014-09-29 16:32 UTC (permalink / raw)
To: Andrew Sutton; +Cc: gcc-patches, Roland Bock
On 09/29/2014 11:46 AM, Andrew Sutton wrote:
> The main reason for the restriction is that concept definitions are
> normalized into a single constraint-expression. And it's not obvious
> how things like using declarations and static-assertions should be
> interpreted within the constraint language.
A using-declaration just affects name lookup. They and typedefs/aliases
can help to make the return statement easier to write.
> That said, having a static_assert inside a concept kind of defeats the
> purpose since it triggers a diagnostic when its condition isn't
> satisfied. That's not very SFINAE friendly :)
True. It might still be useful if for some reason testing a concept for
a certain class of types indicates an error somewhere else. And people
are likely to try it, as indicated by the bug report. :)
> Maybe the restriction can relaxed when we consider the TS for adoption in 17.
I suppose, but I'd prefer not to wait that long. I guess we can talk
about it on the call today.
Jason
^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [c++-concepts] Check function concept definitions
2014-09-29 16:32 ` Jason Merrill
@ 2014-09-29 17:50 ` Roland Bock
0 siblings, 0 replies; 5+ messages in thread
From: Roland Bock @ 2014-09-29 17:50 UTC (permalink / raw)
To: Jason Merrill, Andrew Sutton; +Cc: gcc-patches
On 2014-09-29 18:32, Jason Merrill wrote:
> On 09/29/2014 11:46 AM, Andrew Sutton wrote:
>> The main reason for the restriction is that concept definitions are
>> normalized into a single constraint-expression. And it's not obvious
>> how things like using declarations and static-assertions should be
>> interpreted within the constraint language.
>
> A using-declaration just affects name lookup. They and
> typedefs/aliases can help to make the return statement easier to write.
>
>> That said, having a static_assert inside a concept kind of defeats the
>> purpose since it triggers a diagnostic when its condition isn't
>> satisfied. That's not very SFINAE friendly :)
>
> True. It might still be useful if for some reason testing a concept
> for a certain class of types indicates an error somewhere else. And
> people are likely to try it, as indicated by the bug report. :)
>
>> Maybe the restriction can relaxed when we consider the TS for
>> adoption in 17.
>
> I suppose, but I'd prefer not to wait that long. I guess we can talk
> about it on the call today.
>
> Jason
>
Since I sent that sample code with the static_assert inside the concept,
let me add that I wanted to test something completely different and
stumbled over that internal error by accident :-)
That being said, I had expected the same rules for concepts as for
constexpr functions indeed. It seemed quite natural (and would require
less RAM in my brain).
Best,
Roland
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2014-09-29 17:50 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2014-09-29 14:55 [c++-concepts] Check function concept definitions Andrew Sutton
2014-09-29 15:07 ` Jason Merrill
2014-09-29 15:46 ` Andrew Sutton
2014-09-29 16:32 ` Jason Merrill
2014-09-29 17:50 ` Roland Bock
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).