diff mbox

C++ PATCH for c++/43831 (missing diagnostics for redundant captures)

Message ID 4DFF59A7.9060801@redhat.com
State New
Headers show

Commit Message

Jason Merrill June 20, 2011, 2:31 p.m. UTC
Apparently it's ill-formed to explicitly specify a capture for a 
variable that would be captured the same way by default, so I've added 
this as a pedwarn.

Tested x86_64-pc-linux-gnu, applying to trunk.
diff mbox

Patch

commit ec4717b67c17d8970828f4f9c81ac8794f65a44f
Author: Jason Merrill <jason@redhat.com>
Date:   Sat Jun 18 15:03:12 2011 -0400

    	PR c++/43831
    	* parser.c (cp_parser_lambda_introducer): Complain about redundant
    	captures.
    	* semantics.c (add_capture): Likewise.
    	(register_capture_members): Clear IDENTIFIER_MARKED.

diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 49aa35e..75dac6a 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -7485,6 +7485,10 @@  cp_parser_lambda_introducer (cp_parser* parser, tree lambda_expr)
       /* Possibly capture `this'.  */
       if (cp_lexer_next_token_is_keyword (parser->lexer, RID_THIS))
 	{
+	  location_t loc = cp_lexer_peek_token (parser->lexer)->location;
+	  if (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda_expr) == CPLD_COPY)
+	    pedwarn (loc, 0, "explicit by-copy capture of %<this%> redundant "
+		     "with by-copy capture default");
 	  cp_lexer_consume_token (parser->lexer);
 	  add_capture (lambda_expr,
 		       /*id=*/this_identifier,
@@ -7568,6 +7572,21 @@  cp_parser_lambda_introducer (cp_parser* parser, tree lambda_expr)
 	capture_init_expr
 	  = unqualified_name_lookup_error (capture_init_expr);
 
+      if (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda_expr) != CPLD_NONE
+	  && !explicit_init_p)
+	{
+	  if (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda_expr) == CPLD_COPY
+	      && capture_kind == BY_COPY)
+	    pedwarn (capture_token->location, 0, "explicit by-copy capture "
+		     "of %qD redundant with by-copy capture default",
+		     capture_id);
+	  if (LAMBDA_EXPR_DEFAULT_CAPTURE_MODE (lambda_expr) == CPLD_REFERENCE
+	      && capture_kind == BY_REFERENCE)
+	    pedwarn (capture_token->location, 0, "explicit by-reference "
+		     "capture of %qD redundant with by-reference capture "
+		     "default", capture_id);
+	}
+
       add_capture (lambda_expr,
 		   capture_id,
 		   capture_init_expr,
diff --git a/gcc/cp/semantics.c b/gcc/cp/semantics.c
index 76c1862..1f683c7 100644
--- a/gcc/cp/semantics.c
+++ b/gcc/cp/semantics.c
@@ -8516,8 +8516,8 @@  tree
 add_capture (tree lambda, tree id, tree initializer, bool by_reference_p,
 	     bool explicit_init_p)
 {
-  tree type;
-  tree member;
+  char *buf;
+  tree type, member, name;
 
   type = lambda_capture_field_type (initializer);
   if (by_reference_p)
@@ -8527,18 +8527,31 @@  add_capture (tree lambda, tree id, tree initializer, bool by_reference_p,
 	error ("cannot capture %qE by reference", initializer);
     }
 
+  /* Add __ to the beginning of the field name so that user code
+     won't find the field with name lookup.  We can't just leave the name
+     unset because template instantiation uses the name to find
+     instantiated fields.  */
+  buf = (char *) alloca (IDENTIFIER_LENGTH (id) + 3);
+  buf[1] = buf[0] = '_';
+  memcpy (buf + 2, IDENTIFIER_POINTER (id),
+	  IDENTIFIER_LENGTH (id) + 1);
+  name = get_identifier (buf);
+
+  /* If TREE_TYPE isn't set, we're still in the introducer, so check
+     for duplicates.  */
+  if (!TREE_TYPE (lambda))
+    {
+      if (IDENTIFIER_MARKED (name))
+	{
+	  pedwarn (input_location, 0,
+		   "already captured %qD in lambda expression", id);
+	  return NULL_TREE;
+	}
+      IDENTIFIER_MARKED (name) = true;
+    }
+
   /* Make member variable.  */
-  {
-    /* Add __ to the beginning of the field name so that user code
-       won't find the field with name lookup.  We can't just leave the name
-       unset because template instantiation uses the name to find
-       instantiated fields.  */
-    char *buf = (char *) alloca (IDENTIFIER_LENGTH (id) + 3);
-    buf[1] = buf[0] = '_';
-    memcpy (buf + 2, IDENTIFIER_POINTER (id),
-	    IDENTIFIER_LENGTH (id) + 1);
-    member = build_lang_decl (FIELD_DECL, get_identifier (buf), type);
-  }
+  member = build_lang_decl (FIELD_DECL, name, type);
 
   if (!explicit_init_p)
     /* Normal captures are invisible to name lookup but uses are replaced
@@ -8548,6 +8561,9 @@  add_capture (tree lambda, tree id, tree initializer, bool by_reference_p,
        always visible.  */
     DECL_NORMAL_CAPTURE_P (member) = true;
 
+  if (id == this_identifier)
+    LAMBDA_EXPR_THIS_CAPTURE (lambda) = member;
+
   /* Add it to the appropriate closure class if we've started it.  */
   if (current_class_type && current_class_type == TREE_TYPE (lambda))
     finish_member_declaration (member);
@@ -8555,13 +8571,6 @@  add_capture (tree lambda, tree id, tree initializer, bool by_reference_p,
   LAMBDA_EXPR_CAPTURE_LIST (lambda)
     = tree_cons (member, initializer, LAMBDA_EXPR_CAPTURE_LIST (lambda));
 
-  if (id == this_identifier)
-    {
-      if (LAMBDA_EXPR_CAPTURES_THIS_P (lambda))
-        error ("already captured %<this%> in lambda expression");
-      LAMBDA_EXPR_THIS_CAPTURE (lambda) = member;
-    }
-
   if (TREE_TYPE (lambda))
     return build_capture_proxy (member);
   /* For explicit captures we haven't started the function yet, so we wait
@@ -8572,13 +8581,16 @@  add_capture (tree lambda, tree id, tree initializer, bool by_reference_p,
 /* Register all the capture members on the list CAPTURES, which is the
    LAMBDA_EXPR_CAPTURE_LIST for the lambda after the introducer.  */
 
-void register_capture_members (tree captures)
+void
+register_capture_members (tree captures)
 {
-  if (captures)
-    {
-      register_capture_members (TREE_CHAIN (captures));
-      finish_member_declaration (TREE_PURPOSE (captures));
-    }
+  if (captures == NULL_TREE)
+    return;
+
+  register_capture_members (TREE_CHAIN (captures));
+  /* We set this in add_capture to avoid duplicates.  */
+  IDENTIFIER_MARKED (DECL_NAME (TREE_PURPOSE (captures))) = false;
+  finish_member_declaration (TREE_PURPOSE (captures));
 }
 
 /* Similar to add_capture, except this works on a stack of nested lambdas.
diff --git a/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-capture-redundancy.C b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-capture-redundancy.C
new file mode 100644
index 0000000..51e55a7
--- /dev/null
+++ b/gcc/testsuite/g++.dg/cpp0x/lambda/lambda-capture-redundancy.C
@@ -0,0 +1,12 @@ 
+// FDIS 5.1.2/8
+// { dg-options "-pedantic-errors -std=c++0x" }
+
+struct S2 { void f(int i); };
+void S2::f(int i) {
+  [&, i]{ };       // OK
+  [&, &i]{ };	   // { dg-error "" } i preceded by & when & is the default
+  [=, i]{ };       // { dg-error "" } i not preceded by & when = is the default
+  [=, this]{ };	   // { dg-error "" } this when = is the default
+  [i, i]{ };	   // { dg-error "" } i repeated
+  [this, this]{ }; // { dg-error "" } i repeated
+}