Bug 59165 - gcc looks up begin(), end() for for-range loops for ints in namespace std
Summary: gcc looks up begin(), end() for for-range loops for ints in namespace std
Status: RESOLVED FIXED
Alias: None
Product: gcc
Classification: Unclassified
Component: c++ (show other bugs)
Version: unknown
: P3 normal
Target Milestone: 4.9.0
Assignee: Paolo Carlini
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2013-11-18 04:13 UTC by thakis
Modified: 2014-01-03 11:13 UTC (History)
2 users (show)

See Also:
Host:
Target:
Build:
Known to work:
Known to fail:
Last reconfirmed: 2013-12-10 00:00:00


Attachments

Note You need to log in before you can comment on or make changes to this bug.
Description thakis 2013-11-18 04:13:36 UTC
This compiles, but shouldn't:

$ cat gcc4.8bug.cc 
// builds with gcc4.8, but shouldn't
namespace std {
int* begin(int i) { return (int*)0; }
int* end(int i) { return (int*)0; }
}

int main() {
  for (int a : 10) { }
}
$ gcc-4.8.1 -c gcc4.8bug.cc  -std=c++11
# works


The standard says that begin() and end() for foreach loops should be looked up in the associated namespace of the type of the expression (6.5.4p1)

"""otherwise, begin-expr and end-expr are begin(__range) and end(__range), respectively, where begin and end are looked up in the associated namespaces (3.4.2). [ Note: Ordinary unqualified lookup (3.4.1) is not performed. — end note ]"""

10 has type int, which is a fundamental type, and hence doesn't have an associated namespace. So this shouldn't compile. (It doesn't compile in clang.)



$ gcc-4.8.1 --version
gcc-4.8.1 (GCC) 4.8.1
Copyright (C) 2013 Free Software Foundation, Inc.
This is free software; see the source for copying conditions.  There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
Comment 1 Paolo Carlini 2013-11-18 10:28:54 UTC
Well, this changed only post-C++11 in http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_defects.html#1442 which is simply unimplemented. Frankly, I'm not even sure we want to implement the proposed resolution right now (for comparison, ICC doesn't). In case, changing things like (in cp_parser_perform_range_for_lookup):

	  member_begin = perform_koenig_lookup (id_begin, vec,
						/*include_std=*/true,
						tf_warning_or_error);

would be easy, I guess.
Comment 2 Jonathan Wakely 2013-11-18 10:46:19 UTC
(In reply to thakis from comment #0)
> This compiles, but shouldn't:

You add declarations to namespace std which is undefined behaviour, so any result is valid.

The DR looks wrong to me, the point of treating std as an associated namespace is that std::begin and std::end will be found for user-defined containers, not just the standard containers.  I don't know why core decided to change that.
Comment 3 Jonathan Wakely 2013-11-18 10:56:16 UTC
I think I remember the rationale now: std::begin and std::end only work if c.begin() and c.end() xist, in which case range-based for will use those members directly anyway.
Comment 4 Daniel Krügler 2013-11-20 09:48:06 UTC
(In reply to Jonathan Wakely from comment #3)
> I think I remember the rationale now: std::begin and std::end only work if
> c.begin() and c.end() xist, in which case range-based for will use those
> members directly anyway.

This is not really the *reason* for the language change. The actual reason becomes a bit clearer when looking at the dup of CWG 1442:

http://www.open-std.org/jtc1/sc22/wg21/docs/cwg_closed.html#1445

I remember that this dup was actually inspired by the observation that BOOST_FOR_EACH behaved subtly different to the language for-each lookup rules, albeit both constructs are intended to perform the same thing. CWG 1442 just ensured that the lookup rule in for each corresponds to existing lookup rules in other places and is thus easier to understand.
Comment 5 Paolo Carlini 2013-12-10 10:36:47 UTC
Mine.
Comment 6 paolo@gcc.gnu.org 2014-01-03 11:11:33 UTC
Author: paolo
Date: Fri Jan  3 11:11:31 2014
New Revision: 206313

URL: http://gcc.gnu.org/viewcvs?rev=206313&root=gcc&view=rev
Log:
/cp
2014-01-03  Paolo Carlini  <paolo.carlini@oracle.com>

	Core DR 1442
	PR c++/59165
	* parser.c (cp_parser_perform_range_for_lookup): Don't pass true
	as include_std to perform_koenig_lookup.
	(cp_parser_postfix_expression): Adjust.
	* pt.c (tsubst_copy_and_build): Likewise.
	* semantics.c (perform_koenig_lookup): Remove bool parameter.
	(omp_reduction_lookup): Adjust.
	* name-lookup.c (lookup_arg_dependent_1): Remove bool parameter.
	(lookup_arg_dependent): Likewise.
	(lookup_function_nonclass): Adjust.
	* name-lookup.h: Adjust declaration.
	* cp-tree.h: Likewise.

/testsuite
2014-01-03  Paolo Carlini  <paolo.carlini@oracle.com>

	Core DR 1442
	PR c++/59165
	* g++.dg/cpp0x/range-for28.C: New.
	* g++.dg/cpp0x/range-for3.C: Update.

Added:
    trunk/gcc/testsuite/g++.dg/cpp0x/range-for28.C
Modified:
    trunk/gcc/cp/ChangeLog
    trunk/gcc/cp/cp-tree.h
    trunk/gcc/cp/name-lookup.c
    trunk/gcc/cp/name-lookup.h
    trunk/gcc/cp/parser.c
    trunk/gcc/cp/pt.c
    trunk/gcc/cp/semantics.c
    trunk/gcc/testsuite/ChangeLog
    trunk/gcc/testsuite/g++.dg/cpp0x/range-for3.C
Comment 7 Paolo Carlini 2014-01-03 11:13:27 UTC
Done.