Eric Lemings wrote:
-----Original Message-----
From: Martin Sebor [mailto:[EMAIL PROTECTED] On Behalf Of Martin Sebor
Sent: Monday, June 02, 2008 5:41 PM
To: [email protected]
Subject: Re: [STDCXX-550] Please peer review this change.
Eric Lemings wrote:
-----Original Message-----
From: Martin Sebor [mailto:[EMAIL PROTECTED] On Behalf Of
Martin Sebor
Sent: Monday, June 02, 2008 4:21 PM
To: [email protected]
Subject: Re: [STDCXX-550] Please peer review this change.
Eric Lemings wrote:
-----Original Message-----
From: Martin Sebor [mailto:[EMAIL PROTECTED]
Sent: Monday, June 02, 2008 2:44 PM
To: [email protected]
Subject: Re: [STDCXX-550] Please peer review this change.
Eric Lemings wrote:
Would like to get another set of eyes on this before I submit.
$ svn diff include/deque
Index: include/deque
===================================================================
--- include/deque (revision 662487)
+++ include/deque (working copy)
@@ -749,7 +749,7 @@
void _C_insert (const iterator &__it,
_IntType __n, _IntType __x, int) {
// see 23.1.1, p9 and DR 438
- _C_insert_n (__it, __n, __x);
+ _C_insert_n (__it, __n, const_reference (__x));
I suspect this is not correct. Suppose the type of __x is
a 4 byte int and const_reference is an 8 byte long. The cast
will make _C_insert_n() to read 4 bytes past the end of __x.
A better fix might be to cast __x to value_type, as long as
doing so doesn't violate [sequence.reqmts].
Yep. Good call. Will change to value_type() cast.
Do read DR 438 before applying the cast. There are some obscure
corner cases here, e.g., IntType being a user-defined "integer-
like" type. I don't know if we support this case now, or if we
do, if it's being tested, but suffice it to say that there are
subtle differences between direct-initialization either via
a function-style cast or static_cast, or copy-initialization
(passing arguments to functions).
I did read the DR briefly. Perhaps I should add some conditional
compilation so that __x is converted using value_type() only when
using Sun C++? That would limit the scope of the change and the
issue only relates to this compiler anywho.
I don't think we want to do it differently for different compilers.
Or should I just skip it entirely and leave the warnings as they
are?
That depends on what a small test case looks like. How likely are
users to run into the same warning and what can they do to silence
it? A small test case reproducing the warning should help answer
these questions.
I don't think a test case is really needed. Users can easily encounter
the warnings anytime they use a deque instantiated with 64-bit integer
types as shown by lines 593-601 in file
tests/containers/23.deque.modifiers.cpp where the warnings originally
appeared.
I saw the test and the warnings. It's not obvious from the test what
exactly is being instantiated on what type, and there are 82 warnings
in the build logs. It would be a lot easier to talk about if it was
distilled to a few lines of code.
For example, does this produce a warning:
std::deque<int> d;
long x = LONG_MAX;
d.insert (d.begin (), &x, &x);
or does this:
std::deque<long> d;
int x = INT_MAX;
d.insert (d.begin (), &x, &x);
and/or something else...?
It is just a warning and only for Sun C++ but I see how the explicit
conversion could potentially cause problems. Not likely but possible.
I'll just put it on hold for now.
Well, the first case above would lose the high bits of x. Seems like
the warning at the POI would be justified. In the second case I don't
see a need for a warning.
Martin
