[C++0x] Range-based for statements and ADL
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
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
> 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
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
> 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
>> 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
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
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
> 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
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
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
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
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
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
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.
> 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
> 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
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.
> 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
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
>> 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
> 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.
> 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.
> 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
> 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
> 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
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/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
> 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
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