[C++0x] Range-based for statements and ADL

2011-03-17 Thread Rodrigo Rivas
Hello all.

I've just read the paper titled "Range-based for statements and ADL"
by Jonathan Wakely and Bjarne Stroustrup at:
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2011/n3257.pdf

The point of this article is that the argument dependent lookup in the
range-for specification will make some trouble in the real world. E.g:
#include 
namespace n
{
struct X { void begin(); };
struct Y { void begin(); };
template void begin(T& t) { t.begin(); }
}
int main()
{
std::vector v;
for (auto i : v)  // error: call to begin is ambiguous: std::begin
or n::begin?
{
//...
}
}

I've implemented Option 5 from the paper (which is the coolest, IMHO):
 - [...], if _rangeT has a member begin or a member end, begin-expr
and end-expr are __range.begin() and __range.end(), respectively
 - otherwise, begin-expr and end-expr are begin(__range) and
end(__range), respectively, where begin and end are looked up with
argument-dependent lookup [...]

I'm not sure about what should happen if _rangeT has a member begin
but not a member end. I think that there are just two sensible
options:
 * Use them only if both members, begin and end, exist.
 * Look for them independently. This is what my patch does.

Also, if the member begin/end is not accessible or not callable, a
compiler error will follow immediately (this is as expected).

I'll appreciate any comments.

Best regards
--
Rodrigo
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index a9fd201..8fc77a0 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -1871,6 +1871,8 @@ static tree cp_parser_c_for
   (cp_parser *, tree, tree);
 static tree cp_parser_range_for
   (cp_parser *, tree, tree, tree);
+static tree cp_parser_perform_range_for_lookup
+  (const char *, tree);
 static tree cp_parser_jump_statement
   (cp_parser *);
 static void cp_parser_declaration_statement
@@ -8870,25 +8872,8 @@ cp_convert_range_for (tree statement, tree range_decl, 
tree range_expr)
   else
{
  /* If it is not an array, we must call begin(__range)/end__range() */
- VEC(tree,gc) *vec;
-
- begin_expr = get_identifier ("begin");
- vec = make_tree_vector ();
- VEC_safe_push (tree, gc, vec, range_temp);
- begin_expr = perform_koenig_lookup (begin_expr, vec,
- /*include_std=*/true);
- begin_expr = finish_call_expr (begin_expr, &vec, false, true,
-tf_warning_or_error);
- release_tree_vector (vec);
-
- end_expr = get_identifier ("end");
- vec = make_tree_vector ();
- VEC_safe_push (tree, gc, vec, range_temp);
- end_expr = perform_koenig_lookup (end_expr, vec,
-   /*include_std=*/true);
- end_expr = finish_call_expr (end_expr, &vec, false, true,
-  tf_warning_or_error);
- release_tree_vector (vec);
+ begin_expr = cp_parser_perform_range_for_lookup("begin", range_temp);
+ end_expr = cp_parser_perform_range_for_lookup("end", range_temp);
 
  /* The unqualified type of the __begin and __end temporaries should
   * be the same as required by the multiple auto declaration */
@@ -8940,6 +8925,42 @@ cp_convert_range_for (tree statement, tree range_decl, 
tree range_expr)
   return statement;
 }
 
+static tree
+cp_parser_perform_range_for_lookup (const char *name, tree range)
+{
+  tree ident, expr;
+
+  ident = get_identifier (name);
+  expr = build_qualified_name (/*type=*/NULL_TREE,
+  TREE_TYPE (range),
+  ident,
+  /*template_p=*/false);
+  expr = finish_class_member_access_expr(range, expr, false, tf_none);
+
+  if (expr != error_mark_node)
+{
+  tree instance, fn;
+  instance = TREE_OPERAND (expr, 0);
+  fn = TREE_OPERAND (expr, 1);
+
+  expr = build_new_method_call (instance, fn, NULL, NULL, LOOKUP_NORMAL,
+   NULL, tf_warning_or_error);
+}
+  else
+{
+  VEC(tree,gc) *vec;
+  vec = make_tree_vector ();
+  VEC_safe_push (tree, gc, vec, range);
+  expr = get_identifier (name);
+  expr = perform_koenig_lookup (expr, vec,
+   /*include_std=*/true);
+  expr = finish_call_expr (expr, &vec, false, true,
+  tf_warning_or_error);
+  release_tree_vector (vec);
+}
+  return expr;
+}
+
 
 /* Parse an iteration-statement.
 


Re: [C++0x] Range-based for statements and ADL

2011-03-17 Thread Rodrigo Rivas
On Thu, Mar 17, 2011 at 7:18 PM, Jonathan Wakely  wrote:
> do you mind if I forward your mail to the committee reflector?  The
> fact that there's an implementation is useful.
I'd do that gladly... if only I'd know how...
The [1] reference in the cited paper requires authorization :-(


Re: [C++0x] Range-based for statements and ADL

2011-03-17 Thread Rodrigo Rivas
> Actually, re-reading that and the following messages on the thread,
> I'm wrong about the lookup being independent - if it only finds a
> begin() member and no end() member it should still try to use r.end(),
> and therefore give an error. It should not be possible to use
> r.begin() and end(r)

I think that assuring that both are read from the same place makes more sense.
Using r.begin() and end(r) is mixing two partial, possibly
incompatible interfaces to the same data, and that should not be done
silently.

My patch does this mainly because it was easier to implement ;-).


Re: [C++0x] Range-based for statements and ADL

2011-03-17 Thread Rodrigo Rivas
Exactly what I had in mind!

You could also inhibit the member/non-member error message if any of
the lookups resulted in an error; and likewise inhibit the type
mismatch if this other one happened. But just now, it doesn't really
matter,
--
Rodrigo


Re: [C++0x] Range-based for statements and ADL

2011-03-17 Thread Rodrigo Rivas
> Yes, I was just trying exactly that improvement.
How about this?

> I'm going to let the committee know that option 5 has been
> implemented.  Thanks very much!

Cool! Thank you.
--
Rodrigo
diff --git a/gcc/cp/parser.c b/gcc/cp/parser.c
index 9523fdc..aab9294 100644
--- a/gcc/cp/parser.c
+++ b/gcc/cp/parser.c
@@ -1871,6 +1871,8 @@ static tree cp_parser_c_for
   (cp_parser *, tree, tree);
 static tree cp_parser_range_for
   (cp_parser *, tree, tree, tree);
+static tree cp_parser_perform_range_for_lookup
+  (const char *, tree, int*);
 static tree cp_parser_jump_statement
   (cp_parser *);
 static void cp_parser_declaration_statement
@@ -8870,32 +8872,34 @@ cp_convert_range_for (tree statement, tree range_decl, 
tree range_expr)
   else
{
  /* If it is not an array, we must call begin(__range)/end__range() */
- VEC(tree,gc) *vec;
-
- begin_expr = get_identifier ("begin");
- vec = make_tree_vector ();
- VEC_safe_push (tree, gc, vec, range_temp);
- begin_expr = perform_koenig_lookup (begin_expr, vec,
- /*include_std=*/true);
- begin_expr = finish_call_expr (begin_expr, &vec, false, true,
-tf_warning_or_error);
- release_tree_vector (vec);
-
- end_expr = get_identifier ("end");
- vec = make_tree_vector ();
- VEC_safe_push (tree, gc, vec, range_temp);
- end_expr = perform_koenig_lookup (end_expr, vec,
-   /*include_std=*/true);
- end_expr = finish_call_expr (end_expr, &vec, false, true,
-  tf_warning_or_error);
- release_tree_vector (vec);
-
- /* The unqualified type of the __begin and __end temporaries should
-  * be the same as required by the multiple auto declaration */
- iter_type = cv_unqualified (TREE_TYPE (begin_expr));
- if (!same_type_p (iter_type, cv_unqualified (TREE_TYPE (end_expr
-   error ("inconsistent begin/end types in range-based for: %qT and 
%qT",
-  TREE_TYPE (begin_expr), TREE_TYPE (end_expr));
+ int begin_is_member = 0;
+ begin_expr = cp_parser_perform_range_for_lookup("begin", range_temp, 
&begin_is_member);
+ int end_is_member = 0;
+ end_expr = cp_parser_perform_range_for_lookup("end", range_temp, 
&end_is_member);
+
+ if (begin_expr == error_mark_node || end_expr == error_mark_node)
+   /* If one of the expressions is an error do no more checks.  */
+   begin_expr = end_expr = iter_type = error_mark_node;
+ else
+   {
+ iter_type = cv_unqualified (TREE_TYPE (begin_expr));
+ /* Check that the __begin and __end expressions are consistent.  
*/
+ if (begin_is_member != end_is_member)
+   {
+ error ("range-based for found %s % and %s %",
+(begin_is_member ? "member" : "non-member"),
+(end_is_member ? "member" : "non-member"));
+   }
+ else
+   {
+ /* The unqualified type of the __begin and __end temporaries 
should
+  * be the same as required by the multiple auto declaration */
+ if (!same_type_p (iter_type, cv_unqualified (TREE_TYPE 
(end_expr
+   error ("inconsistent begin/end types in range-based for: "
+  "%qT and %qT",
+  TREE_TYPE (begin_expr), TREE_TYPE (end_expr));
+   }
+   }
}
 }
 
@@ -8940,6 +8944,44 @@ cp_convert_range_for (tree statement, tree range_decl, 
tree range_expr)
   return statement;
 }
 
+static tree
+cp_parser_perform_range_for_lookup (const char *name, tree range, int* 
is_member)
+{
+  tree ident, expr;
+
+  ident = get_identifier (name);
+  expr = build_qualified_name (/*type=*/NULL_TREE,
+  TREE_TYPE (range),
+  ident,
+  /*template_p=*/false);
+  expr = finish_class_member_access_expr(range, expr, false, tf_none);
+
+  if (expr != error_mark_node)
+{
+  tree instance, fn;
+  instance = TREE_OPERAND (expr, 0);
+  fn = TREE_OPERAND (expr, 1);
+
+  expr = build_new_method_call (instance, fn, NULL, NULL, LOOKUP_NORMAL,
+   NULL, tf_warning_or_error);
+  *is_member = 1;
+}
+  else
+{
+  VEC(tree,gc) *vec;
+  vec = make_tree_vector ();
+  VEC_safe_push (tree, gc, vec, range);
+  expr = get_identifier (name);
+  expr = perform_koenig_lookup (expr, vec,
+   /*include_std=*/true);
+  expr = finish_call_expr (expr, &vec, false, true,
+  tf_warning_or_error);
+  release_tree_vector (vec);
+  *is_member = 0;
+}
+  return expr;
+}
+

Re: [C++0x] Range-based for statements and ADL

2011-03-17 Thread Rodrigo Rivas
>> Nice.  The comments should be changed too:

Sure. Also, should this ever be accepted, the std::begin() and
std::end() declarations should be removed from the header files.


Re: [C++0x] Range-based for statements and ADL

2011-03-25 Thread Rodrigo Rivas
On Wed, Mar 23, 2011 at 5:46 PM, Jason Merrill  wrote:
> Yep.  Here is the wording that we approved today:
[snip]

Nice. Thank you.
But now I'm a bit concerned about the complexity of this issue. Mind
me, I think that this solution is nice, but maybe it is a burden for
library implementors.

Let's say that I want to imitate 'exactly' the behavior of the
range-for, but with a plain old for (there are plenty of reasons for
that):
template void foo(C &c)
{
auto _b = begin(c);
auto _e = end(c);
for (auto _i = _b; _i != _e; ++_i)
{
auto _e = *_i;
//do stuff
}
}

But this will fail sometimes for the reasons stated in N3257. If I
change the first line to:
   auto _b = c.begin();
it will fail in a lot of different cases.
So I'm forced to write a pair of functions, let's name them
'range_begin/range_end' with a lot of SFINAE tricks to mimic the
range-for behavior. And there are a lot of subtleties and magic in the
range-for specification that makes that a hard work, if at all
possible.

IMHO, it would be much easier if the standard library provided these
functions for me. Before, I considered that was what
std::begin/std::end did, but now...

--
Rodrigo


Re: [C++0x] Range-based for statements and ADL

2011-03-25 Thread Rodrigo Rivas
On Fri, Mar 25, 2011 at 11:52 AM, James Dennett  wrote:
> I'd be interested to know those reasons.
Well, maybe not *plenty*, but *some*... Let's say that I have a function:
template  void Dump(S &s, const T &t)
{
for (auto i : t)
s.dump(i);
}
Cool!

Now I want to do the following:
template  void DumpExtra(S &s, const T &t)
{
   auto _b = begin(c);
   auto _e = end(c);
   for (auto _it = _b; _it != _e; ++_it)
   {
   auto i = *_it;
   if (s.condition(i))
   s.dump(i);
   else
   {
  if (++_it == _e)
  break;
  s.dump2(i, *_it);
   }
}
I cannot use the range-for because I increment the iterator. But now
I'm not sure if this may not compile, or worse, may use a different
range definition than the other one!

Disclaimer: the fact that I lack the imagination for a better example
doesn't mean that it will not happen in the real world.
--
Rodrigo


Re: [C++0x] Range-based for statements and ADL

2011-03-25 Thread Rodrigo Rivas
> On 03/25/2011 11:13 AM, Rodrigo Rivas wrote:
> Note that there was already a special case for arrays.

Yes, but std::begin/std::end are overloaded for arrays also, so the
library implementor would get the right behavior for free.
They are still vulnerable to the ADL issue, though.
--
Rodrigo


Re: [C++0x] Range-based for statements and ADL

2011-03-25 Thread Rodrigo Rivas
On Fri, Mar 25, 2011 at 11:27 AM, Jonathan Wakely  wrote:
> Or just don't use range-for, it's not essential (given the trouble
> it's caused I'd quite happily have lived without it in C++0x!)

IMO, this goes beyond the syntactic sugar that the range-for provides.
It is about specifying a unified 'range' concept. The range-for is
just the first, highly visible, user of this implicit specification.
--
Rodrigo


Re: [C++0x] Range-based for statements and ADL

2011-03-25 Thread Rodrigo Rivas
On Fri, Mar 25, 2011 at 1:34 PM, Gabriel Dos Reis
 wrote:
> But what is that `unified range concept'?  And why do we need it?
See Boost.Range for the concept and possibly uses. There has been some
discussion to accept it in the standard, IIRC.

> Exactly.  Which for me means, it must be simple.  Simple to learn,
> simple to use, simple to teach.
The range-for as it is specified in this thread *is* simple to learn
use and teach. Not so easy to implement, but not so hard either.
I am merely pointing out that strictly emulating the range-for
behavior is far from trivial.

> BTW, if you are trying to change the specification is gcc-patches
> the appropriate place to discuss that?
I have no intention to specify anything, I'm just suggesting that it
would be nice to have a library function that does this.
And this is not gcc-patches@ but gcc@: "Anything relevant to the
development or testing of GCC and not covered by other mailing lists
is suitable for discussion here."
--
Rodrigo


Re: [C++0x] Range-based for statements and ADL

2011-03-25 Thread Rodrigo Rivas
On Fri, Mar 25, 2011 at 1:33 PM, Jonathan Wakely  wrote:
> Yes but it's too late to specify it in C++0x.
>
> Boost.Range is the best place to work on that idea at present.
> If/when it's fully baked I hope we'll see something like that in a
> future TR or standard.

Agreed.
But just now, how would you explain if the following to loops behave
differently?

template void foo(T &t)
{
for (auto i : t)
;
for (auto i : boost::any_range(t))
;
}

Because the boost::any_range constructor is unable to replicate the
logic from the range-for?


Re: [C++0x] Range-based for statements and ADL

2011-03-25 Thread Rodrigo Rivas
On Fri, Mar 25, 2011 at 7:50 PM, Gabriel Dos Reis
 wrote:
> Boost.Range is a library component.
True, but should it ever make its way to the standard library, it
would be good if it is consistent with the 'range' used by the
range-for. If not, we will have two subtly different definitions of
'range' in the language.

> If "as it is specified in this thread" you mean option 5,
Yes. Exactly that.

> If you are going to do then you are going to specify that function
> before you implement it.  Are you suggesting that as an ISO C++
> library function or a GNU extension?

Well, I am suggesting a ISO C++ library function with the exact
semantics from option 5:
namespace std {
pair range(T &t);
}
Being IT the same type deduced by the range-for iterator. The return
value would be a pair, being the first value the begin iterator and
the second value the end.
I think that one function returning a pair instead of two functions is
better, as the 'special semantics' of the range-for, as it is in
'Option 5', imply both begin and end together.

>> And this is not gcc-patches@ but gcc@: "Anything relevant to the
>> development or testing of GCC and not covered by other mailing lists
>> is suitable for discussion here."
> you are right this is "gcc@".  I am not sure you what you imply by the rest.
I'm merely implying that this list is suitable for this discussion. It
looked like you disagree.

Regards.
--
Rodrigo


Re: [C++0x] Range-based for statements and ADL

2011-03-25 Thread Rodrigo Rivas
On Fri, Mar 25, 2011 at 9:35 PM, Gabriel Dos Reis
 wrote:
> You were earlier talking about some "unified concept"; weren't you?
> Now, it is shifting to library component.

With "unified" I meant that the same concept (range) is present both
in the core language and the standard library. And that it would be
wise that both definitions are the same.

> Option 5 does not say that we have to have a library
> that exactly emulates what is in in the core language.
No it does not. That is why I am proposing, If it said that, I would
add nothing.
BTW, if that function existed, the natural consequence would be to
define the range-for in terms of that function, not to copy-paste the
specification.

>> I'm merely implying that this list is suitable for this discussion. It
>> looked like you disagree.
> yes, I do.  Because what you are suggesting is a change to the
> the ISO C++ definition.  This isn't the proper place for that.
> if is an ISO C++ library, then the proper place is a ISO C++ committee forum.

Yes, but sadly I'm not part of the committee, and since there are
people here that are, I find it useful to post my suggestion here.

Are you saying that in the GCC mailing lists we should discuss only
the use and implementation of the languages, but not their
specifications? Even if those specifications are in a draft status? I
find that hard quite radical.

Regards.


Re: [C++0x] Range-based for statements and ADL

2011-03-26 Thread Rodrigo Rivas
On Sat, Mar 26, 2011 at 2:48 AM, Gabriel Dos Reis
 wrote:
> I see a core language specification, I see a third party library 
> implementation.
> I don't see a unified concept.

I don't see it either. It's just a wish.

> if it would add nothing, then we are in violent agreement that we should
> not be having the discussion here :-)

 Not so violent, I've had worse...

> A better forum is news:comp.std.c++

Granted.

> Yes -- even more so.  In fact by the time were having the
> discussion today, it was past the status of draft that could be modified
> without major disruption: the document went into FDIS status.
> Being in draft status does not magically confer appropriateness for gcc@.

Ok, I admit that my point went *slightly* off-topic... so I'll leave it here.

Regards
--
Rodrigo.


Re: [C++-0x] User-defined literals.

2010-09-19 Thread Rodrigo Rivas
> Maybe Rodrigo would be interested in collaborating on this work?
Sure I am! Please, let me a couple of days to re-read the C++ draft,
and check this patch.
Also, take in account that I'm in no way a GCC expert... but I'll do my best.

Also I have a little patch on my own that might use some help... But
this is for another post.

Regards.
--
Rodrigo


RE: Range-based for in c++98

2010-09-20 Thread Rodrigo Rivas
> This is quite unreadable and not very informative.
Totally agree.

> Here there are two problems...
> snipped

I think that you are taking the wrong approach: you call
"cp_parser_range_for" with C++98 and then if such a loop is parsed
(the ':') you issue an error.
Maybe you should try to add the check to the "cp_parser_c_for" function.
Note that a range-based for always have a declaration in the first
sub-statement, so next the "cp_parser_c_for" will expect a ';' or a
initialization ('(', '=' or '{', IIRC).
If instead of that, a ':' is found there is your chance to diagnose!

Regards.
Rodrigo


[C++0x] implementing forward declarations for enums

2010-09-20 Thread Rodrigo Rivas
Hello all.

This patch tries to implement the C++0x featue "Forward declarations
for enums" aka "opaque enum declarations":
http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2008/n2764.pdf

Please note that this is a WIP, and as such lacks formatting,
comments, testcases, etc.

Except for the things noted below, I think it works pretty well.

TODOs
1
A enum declaration should fail if adds a list of constants and it
already have one. I check it with "TYPE_VALUES (type)", but this is
incorrect because an empty list will not be seen.
So, the current patch will accept incorrect code like:
enum class Foo { };
enum class Foo { A, B, C};
I think that a new flag is needed for ENUMERAL_TYPE, but I don't know which one.

2
I am calling "finish_enum" several types, for each of the opaque
declarations until it gets a list of constants. It doesn't seem to
cause problems except with the debug information. With default flags
and everything, gdb sees only the first declaration:
enum class Foo;
enum class Foo {A, B, C}
Foo f;

(gdb) ptype f;
enum Foo {}

I don't see an easy way to solve it...

3
I don't like very much the added parameter to the "start_enum"
function, but I don't see how to do it without breaking existing code.

Comments are most welcomed.

Regards.
Rodrigo
Index: gcc/cp/decl.c
===
--- gcc/cp/decl.c   (revision 164424)
+++ gcc/cp/decl.c   (working copy)
@@ -11321,27 +11321,48 @@ xref_basetypes (tree ref, tree base_list)
may be used to declare the individual values as they are read.  */
 
 tree
-start_enum (tree name, tree underlying_type, bool scoped_enum_p)
+start_enum (tree name, tree enumtype, tree underlying_type, bool scoped_enum_p)
 {
-  tree enumtype;
-
   gcc_assert (TREE_CODE (name) == IDENTIFIER_NODE);
 
+  /* [C++0x dcl.enum]p5: 
+
+If not explicitly specified, the underlying type of a scoped
+enumeration type is int.  */
+  if (!underlying_type && scoped_enum_p)
+underlying_type = integer_type_node;
+
   /* If this is the real definition for a previous forward reference,
  fill in the contents in the same object that used to be the
  forward reference.  */
-
-  enumtype = lookup_and_check_tag (enum_type, name,
-  /*tag_scope=*/ts_current,
-  /*template_header_p=*/false);
-
+  if (!enumtype)
+enumtype = lookup_and_check_tag (enum_type, name,
+/*tag_scope=*/ts_current,
+/*template_header_p=*/false);
   if (enumtype != NULL_TREE && TREE_CODE (enumtype) == ENUMERAL_TYPE)
 {
-  error_at (input_location, "multiple definition of %q#T", enumtype);
-  error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)),
-   "previous definition here");
-  /* Clear out TYPE_VALUES, and start again.  */
-  TYPE_VALUES (enumtype) = NULL_TREE;
+  if (scoped_enum_p != SCOPED_ENUM_P (enumtype))
+   {
+ error_at (input_location, "scoped/unscoped mismatch in enum %q#T", 
enumtype);
+ error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)),
+   "previous definition here");
+   }
+  if (!underlying_type
+ || !same_type_p (underlying_type, ENUM_UNDERLYING_TYPE (enumtype)))
+   {
+ error_at (input_location, "different underlying type in enum %q#T", 
enumtype);
+ error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)),
+   "previous definition here");
+   }
+
+  if (cxx_dialect == cxx98)
+   {
+ error_at (input_location, "multiple definition of %q#T", enumtype);
+ error_at (DECL_SOURCE_LOCATION (TYPE_MAIN_DECL (enumtype)),
+   "previous definition here");
+ /* Clear out TYPE_VALUES, and start again.  */
+ TYPE_VALUES (enumtype) = NULL_TREE;
+   }
 }
   else
 {
@@ -11357,19 +11378,10 @@ tree
   if (enumtype == error_mark_node)
 return enumtype;
 
+  SET_SCOPED_ENUM_P (enumtype, scoped_enum_p);
   if (scoped_enum_p)
-{
-  SET_SCOPED_ENUM_P (enumtype, 1);
-  begin_scope (sk_scoped_enum, enumtype);
+begin_scope (sk_scoped_enum, enumtype);
 
-  /* [C++0x dcl.enum]p5: 
-
-  If not explicitly specified, the underlying type of a scoped
-  enumeration type is int.  */
-  if (!underlying_type)
-underlying_type = integer_type_node;
-}
-
   if (underlying_type)
 {
   if (CP_INTEGRAL_TYPE_P (underlying_type))
Index: gcc/cp/pt.c
===
--- gcc/cp/pt.c (revision 164424)
+++ gcc/cp/pt.c (working copy)
@@ -6654,7 +6654,7 @@ lookup_template_class (tree d1,
  if (!is_dependent_type)
{
  set_current_access_from_decl (TYPE_NAME (template_type));
- t = start_enum (TYPE_IDENTIFIER (template_type),
+ t = start_enum (TYPE_IDENTIFIER (template_

Re: [C++-0x] User-defined literals.

2010-09-20 Thread Rodrigo Rivas
> I'm looking at (besides input on what I've got currently):
So far I see it fine... except:
int len = TREE_STRING_LENGTH (strl);
should be:
int len = TREE_STRING_LENGTH (strl) - 1;
since the draft says "its length excluding the terminating null character".

Also, I had to change:
identifier = perform_koenig_lookup (identifier, vec,
/*include_std=*/false);
into:
identifier = lookup_function_nonclass (identifier, vec,
/*block_p*/0);
to make it work. The issue here is that "perform_koenig_lookup" does
not work with built-in types, since they don't have associated
namespaces, I guess. Not sure about the block_p parameter, however.

> 1. A warning or error if a user chooses a suffix that gcc uses.  I was
> surprised that 'w', 'q', 'i', 'j' and several others were used by gcc for
> floats.  I won't be the only one.  The standard is clear that
> implementations get first crack at these but it shouldn't be a mystery or a
> surprise when things don't work as expected.
Agreed.

> 2. Should we at least pedwarn about user not starting a suffix with an
> underscore?  Probably.  Non-underscore suffixen are reserved to the
> implementation but I think a user should be able to use one if it's not used
> by gcc though the user risks non-portability and potential but unlikely
> future breakage.
Hmmm, I don't think so. Reserved but unused identifiers are not warned
about anywhere, AFAIK.
For example, I type:
   int __x, _Y;
and no warning at all.
Sure it would be pedanticly nice to have such a warning, but it would
be another topic.

> 3. The big one: Getting the integer(long long) and float(long double)
> suffixes that are not used by gcc out of the preprocessor.  Then we can
> build the calls.
Oh, I see the problem...

> 4.  If, for long long and long double the usual signature is not found,
> first look for a: _suffix(char*) and failing that: template
> _suffix().  So we'll need to make the lookup of these two signatures more
> complex.
Yes, the problem here is that we don't have to use the usual overload
resolution, but check the results of the lookup by hand.
I guess that it won't be so easy...

Regards.
Rodrigo


Re: Range-based for in c++98

2010-09-20 Thread Rodrigo Rivas
On second reading of the C++0x draft in cannot find any reason of what
the new type in range-fors should not be allowed.

Maybe someone can read it differently?

Regard.
Rodrigo


Re: Range-based for in c++98

2010-09-20 Thread Rodrigo Rivas
>> The easy solution would be to remove the assignment to
>> type_definition_forbidden_message and then check for this case
>> particulary.
>
> cp_parser_type_specifier_seq could return some indication of why the
> parsing has failed or whether it has parsed a declaration. This is
> much more useful than just throwing an error. In general this is true
> of the whole C++ parser.
You are right. Being a C++ parser it would be appropriate to implement
some kind of exception-like error management.
But this is way out of my scope (pun intended).

>> Or we can change the cp_parser_check_type_definition() function to call
>> cp_parser_error() instead of error().
>
> No please, cp_parser_error() is almost always annoying and
> uninformative saying things like:
>
> expected __asm__ before ';'
>
> because it lacks context.
I said that just because of c_parser_error respect the tentative flag.
I think that you can put whatever message you like, can't you?

>
>> Or we can just accept the type definition in the range-for and hope that
>> nobody will notice :P
>
> You will have to diagnose it with -pedantic anyway.
Are you sure? As I said in other post, I am no longer sure that the
C++0x draft forbids the type definition in this context.
But I'm no expert in standarese, so I'm still undecided.


Re: Range-based for in c++98

2010-09-21 Thread Rodrigo Rivas
> You do not need exceptions to implement what I said, just to return a
> value. A boolean would suffice to detect whether you parsed a
> definition.
Sure, I was talking generally.
> The "before whatever" is hardcoded. And tentative parsing is
> thoroughly abused in the parser.
Well, I tried to parse it normally, but the function called from the
regular for loop expected after the declaration one of the following
';', ',', '(', '=', '{' if I remember right. When it found a ':' it
choked and emitted an error. Probably it would be better to let the
unknown char to be handled by the calling function. In this way if I
see a ':' I know that it is a range-based for and no tentative is
needed.

But this is too big a change for me at the moment. For now, in order
to solve Magnus's problem I suggest to just remove the check for new
type in cp_parser_range_for().

Regards.
Rodrigo


Re: [C++-0x] User-defined literals.

2010-09-21 Thread Rodrigo Rivas
> 3. The big one: Getting the integer(long long) and float(long double)
> suffixes that are not used by gcc out of the preprocessor.  Then we can
> build the calls.

Just my two cents:
Add an output parameter to the function "cpp_classify_number()"
(libcpp/expr.c) to get the user-defined suffix.
It should be optional, so that it is ignored when not needed.

unsigned int
-cpp_classify_number (cpp_reader *pfile, const cpp_token *token)
+cpp_classify_number (cpp_reader *pfile, const cpp_token *token, char
**suffix) //exposition only!

Then in the function "c_lex_with_flags()" (c-family/c-lex.c) retrieve
the suffix and pass it on to the function "interpret_integer()" /
"interpret_float()", that should add an input parameter accordingly.

The function "interpret_{integer,float}()" calls
"build_int_cst_wide()" / "build_real()" to build the constant. Then
you can build a tree for the identifier (with "get_identifier()",
perhaps) and attach it somehow to the constant.
You may consider creating a new TREE type for that...

I'm afraid that you have quite some job here...

Best regards.
Rodrigo


Re: Re: [C++-0x] User-defined literals.

2010-09-21 Thread Rodrigo Rivas
> I'm holding out for rolling back the lexer in some way that won't break 
> everything and emitting the (unrecognized by cpp ) suffix as a separate 
> identifier token.  I'm thinking the cp_lexer_* routines or maybe a new one in 
> parser.c would be worth trying.  Then the code I have now would just work 
> (except I would still need enhanced lookup rules as discussed earlier).  It 
> would be nice to have all types on the same page too.

Hmmm, you indend to break the user-defined integer/float into two
tokens: the number and the suffix. I don't know... it may cause quite
some unexpected problems. Note that the draft defines a
user-defined-literal as *one* token but you want to read it as *two*
tokens.
That would make ill-formed code parseable, for example:
int x = 10 /* whatever */ _foo;

On second thought, this already happens with strings and chars:
"xxx" /**/ _bar;
is parsed fine with your current patch, but I believe it should not.

Rodrigo.


Re: [C++0x] implementing forward declarations for enums

2010-09-21 Thread Rodrigo Rivas
> I had to initialize the variable nested_being_defined to get it to compile
> (possible uninitialized warning).  I initialized it to false.
Ok, actually it is never used uninitialized, but let's get rid of the warning.

> It looks like the first two are related.  What does an enum look like in
> tree?  Maybe the tree for enum could have a has_definition flag.
As far as I know it is a "tree_type". I was thinking of using
lang_flag_0 or similar, but I'm unsure if they are used for any other
purpose, or if there is a protocol to assign them...

Rodrigo


Re: [C++0x] implementing forward declarations for enums

2010-09-22 Thread Rodrigo Rivas
> I saw the flag situation after I sent my message.  There must be a "Keeper of 
> the Sacred Tree Flag Set"
There are a few comments en tree.h about that, I if remember correctly...

> I get the idea the real estate is at a premium and consuming flags could be
> a problem.
> Perhaps there is a context flag saying "You're inside braces" or something
> so we wouldn't need the second flag.
I think one flag should be enough. It is set just when the constant
list is added. Then, in another declaration, if you see a constant
list and the flag is set, it is an error.

> I'll try to figure out what is done with classes, unions, etc. in this
> regard.  Basically this is the same situation.
Actually it is not (I've already tried that). With classes, a forward
declaration creates an incomplete type, and they use a predicate
TYPE_COMPLETED_P() or something like that. But a forward enum always
defines a complete type, so this test is not useful.

Rodrigo.


Re: [C++0x] implementing forward declarations for enums

2010-09-22 Thread Rodrigo Rivas
 I had to initialize the variable nested_being_defined to get it to compile
 (possible uninitialized warning).  I initialized it to false.

>>> Ok, actually it is never used uninitialized, but let's get rid of the 
>>> warning.
>>>
>> I saw that it was never used uninitialized and was surprised gcc wasn't
>> able to diagnose that.
>
> Can you file a bug for this warning false positive?
Actually, I don't think this is an error. This situation can be
simplified to the following code:

void fun()
{
   bool flag; /* uninitalized */

   if (some_condition())
  flag = xxx;

   actions();

   if (some_condition())
  use(flag);
}

Now, I know that "some_condition()" returns the same value in the two
calls, so "flag" is initialized and used, or not used at all. But the
compiler cannot know that. That's why it emits the *possible*
uninitialized warning.
Curiously, it does so with -O2 but not with -O0, probably due to the
extra analysis of the life of variables.

Regards.
Rodrigo


Re: Range-based for in c++98

2010-10-02 Thread Rodrigo Rivas
2010/10/1 Jason Merrill :
> It took me some searching, but yes, it does:
>
> "A type-specifier-seq shall not define a class or enumeration unless it
> appears in the type-id of an alias-declaration (7.1.3)."
>
> Normal declarations don't have a type-specifier-seq, they have a
> decl-specifier-seq.
No wonder I couldn't find it!

> I would change cp_parser_range_for to use cp_parser_decl_specifier_seq
> instead of cp_parser_type_specifier_seq and then wait to complain about
> defining a type until after we've seen the ':'.
I already tried that, but it didn't work. It seemed to me that it was
because it called cp_parser_commit_to_tentative_parse(), and if then I
wanted to roll-back the parsing of the range-for, I went badly. I had
to agree with Manuel that the tentative parsing is a bit messy...

But, I managed to solve this particual issue with the following patch
(no improvement in the error messages, however):

Index: gcc/cp/parser.c
===
--- gcc/cp/parser.c (revision 164871)
+++ gcc/cp/parser.c (working copy)
@@ -8644,21 +8644,13 @@ cp_parser_range_for (cp_parser *parser)
   tree stmt, range_decl, range_expr;
   cp_decl_specifier_seq type_specifiers;
   cp_declarator *declarator;
-  const char *saved_message;
   tree attributes, pushed_scope;

   cp_parser_parse_tentatively (parser);
-  /* New types are not allowed in the type-specifier-seq for a
- range-based for loop.  */
-  saved_message = parser->type_definition_forbidden_message;
-  parser->type_definition_forbidden_message
-= G_("types may not be defined in range-based for loops");
   /* Parse the type-specifier-seq.  */
   cp_parser_type_specifier_seq (parser, /*is_declaration==*/true,
-   /*is_trailing_return=*/false,
+   /*is_trailing_return=*/true,
&type_specifiers);
-  /* Restore the saved message.  */
-  parser->type_definition_forbidden_message = saved_message;
   /* If all is well, we might be looking at a declaration.  */
   if (cp_parser_error_occurred (parser))
 {

Admittedly, this is not a "trailing_return_type", but AFAICT it has
exactly the same restrictions. Maybe renaming the parameter would be a
good idea.

Regards.
Rodrigo


Re: Range-based for in c++98

2010-10-04 Thread Rodrigo Rivas
> Ah, yes.  So we should share the parsing of the decl-specifier-seq with the
> C-style for loop, which allows us to avoid the tentative parsing.
That was my original idea, but the C-style loop calls
cp_parser_simple_declaration(), that shouts at the ':'. So we should
either modify it to accept the ':' or split it in two. Both options
are well beyond my intentions.
Anyway the C-style for loop does use tentative parsing, so you don't
avoid it completely.

>> Admittedly, this is not a "trailing_return_type", but AFAICT it has
>> exactly the same restrictions.
>
> The restrictions are slightly different; in the case of a trailing return
> type a class-specifier is not one of the expansions, so we don't treat a {
> as beginning a class body.  In the case of a type-specifier-seq, we do treat
> it as beginning a class body, but we give an error about it.

Well, the net effect is the same: the tentative parsing is aborted.
Then the C-style loop for parses the code, and it is this that prints
the error message if needed.

Regards.
Rodrigo


Re: Range-based for in c++98

2010-10-04 Thread Rodrigo Rivas
On Mon, Oct 4, 2010 at 7:16 PM, Nicola Pero
 wrote:
> I just implemented "fast enumeration" (ie, "for (object in array) { ... }")
> for Objective-C, and I was now planning on doing it for Objective-C++ too. ;-)
>
> If you're doing range-based for-loops for C++, we may as well share ideas ;-)
Actually, they are already done. Just some minor improvements left :-)

The issue here is that I didn't feel comfortable modifying the
existing parser code of generic types just for the range-loop sake.

> Anyway, let me know if this makes any sense or if I missed everything.  Am I 
> right that the beginning
> of a C++ range-based for-loop is identical to a standard C for loop up until 
> the ':' ?  If not, obviously
> this technique won't work. ;-)
I don't know much about Obj-C, but I can tell that in C++ there are a
few subtle differences besides the obvious ':' vs. ';'.
C-loops may have an "expression" or a "simple-declaration" while
range-loops have a "type-specifier-seq" followed by a "declarator".
The case of an "expression" is not relevant to this discussion.

Putting it plainly, the differences are basically:
1. C-loops may declare several variables, while range-loops just one.
2. C-loops may specify a storage class (extern, static), while
range-loops may not.
3. C-loops may define new types, while range-loops may not.
4. C-loops may initialize the variables, while range-loops may not.

If these restrictions apply to the Obj-C fast-loops then the code
could be shared easily. If not, well, maybe with a little more work.

Regards.
Rodrigo