From hpages at fhcrc.org  Wed Jun 25 00:38:07 2014
From: hpages at fhcrc.org (=?UTF-8?B?SGVydsOpIFBhZ8Oocw==?=)
Date: Tue, 24 Jun 2014 15:38:07 -0700
Subject: [Rd]
 =?utf-8?q?operation_on_=E2=80=98numsels=E2=80=99_may_be_unde?=
 =?utf-8?q?fined?=
In-Reply-To: <CAJXgQP0racSjePV_Aoexkcr6DGCyQ_VZv7GF+SuJfGm_da6rYA@mail.gmail.com>
References: <53A8298C.9010605@aon.at>	<244EC819-4336-4A49-BAEB-86B57E8A2970@r-enthusiasts.com>	<53A855C6.1000301@aon.at>	<BE6B83B4-2A60-4F84-8B5F-C0754997E5C5@r-enthusiasts.com>	<53A877EE.1080201@aon.at>	<53A8A79A.9070301@fhcrc.org>	<53A8AD3F.2030901@fhcrc.org>	<CAC2h7utL_vzyRCSOHB9xi+nqmkm3aqWwNBbiitvpsu4jevqr6g@mail.gmail.com>
	<CAJXgQP0racSjePV_Aoexkcr6DGCyQ_VZv7GF+SuJfGm_da6rYA@mail.gmail.com>
Message-ID: <53A9FDCF.2030501@fhcrc.org>

Hi Kevin,

On 06/23/2014 10:28 PM, Kevin Ushey wrote:
> I don't see what's so surprising here.
>
> That statement is identical to writing:
>
>      if (arrMask[i] == 1) {
>          numsels = ++numsels;
>      } else {
>          numsels = numsels;
>      }
>
> and
>
>      numsels = ++numsels;
>
> has two statements modifying the value of numsels (= and prefix-++) in
> a single sequence point. (Do we increment then assign, or assign then
> increment? The C / C++ standards leave this undefined.)
>
> Imagine writing the operations out as functions: we have the `=`
> function, and the `prefix-++` function -- both of these 'mo dify' (one
> of) their arguments. Do we evaluate it as `=`(a, `prefix-++`(a)) or
> `prefix-++`(`=`(a, a))?

As the former? Isn't it that `prefix-++` has precedence over `=`? Also
according to the standard, `prefix-++` must perform the increment
*before* returning the incremented value. So I'm still a little bit
confused about this warning. Although I would understand it if
`suffix-++` was used instead of `prefix-++`:

     numsels = numsels++;

In that case the incrementation happens *after* the right value
of the assignment is evaluated but it's undefined whether it will
happen before or after the assignment itself (on my machine it
happens after).

> The C standard leaves this undefined, so
> compilers are free to do what they wish (and the nice ones warn you
> when there is such an ambiguity). I guess the net result of the
> operation is the same in each case _here_, but this is of course not
> the case for other functions that modify the value of their
> operand(s). And, in truth, this is _undefined behaviour_ and so the
> compiler could still rightly make demons fly out of your nose if it
> wanted to upon program execution.
>
> I highly recommend reading the slides at
> http://www.slideshare.net/olvemaudal/deep-c, especially the bit on
> sequence points.

Nice slides. Pretty entertaining! The numsels = numsels++ case is
actually discussed at slide 142. Thanks for the link!

H.

>
> Cheers,
> Kevin
>
> On Mon, Jun 23, 2014 at 9:22 PM, Kasper Daniel Hansen
> <kasperdanielhansen at gmail.com> wrote:
>> I am not an expert on this, but I note that the section on -Wsequence-point
>> at
>>    http://gcc.gnu.org/onlinedocs/gcc/Warning-Options.html
>> specifically mentions ? and :.  Perhaps some more work on tracking down
>> their definitions and precedence might lead to insights.
>>
>> Best,
>> Kasper
>>
>>
>> On Mon, Jun 23, 2014 at 6:42 PM, Herv? Pag?s <hpages at fhcrc.org> wrote:
>>
>>> On 06/23/2014 03:18 PM, Herv? Pag?s wrote:
>>>
>>>> Hi Christian,
>>>>
>>>> On 06/23/2014 11:54 AM, cstrato wrote:
>>>>
>>>>> Dear Romain,
>>>>>
>>>>> I do not know enough about compilers, but as far as I remember, they
>>>>> 'work' from right to left,
>>>>>
>>>>
>>>> Not necessarily. So you should not rely on that. This is what the
>>>> (somewhat obscure) warning you see on zin1 is trying to tell you.
>>>>
>>>
>>> Actually, I don't see an ambiguity in your code:
>>>
>>>
>>>    numsels = (arrMask[i] == 1) ? ++numsels : numsels;
>>>
>>> Yes it's confusing and unnecessarily complicated but I don't see that
>>> it relies on some undefined behavior. It's not like in the thread on
>>> Bioc-devel where the expression was:
>>>
>>>    *p++ = tolower(*p);
>>>
>>> In that case the left-value of the assignment is itself an expression
>>> that needs to be evaluated and the outcome of the assignment depends
>>> on whether the left-value is evaluated before the right expression or
>>> not. But in your case the left-value is a variable name so there is
>>> nothing to evaluate.
>>>
>>> So I don't know. Looks like a false positive from the gcc compiler to
>>> me. Someone on the list might have a better insight.
>>>
>>> Cheers,
>>>
>>> H.
>>>
>>>
>>>   Personally I would just do:
>>>>
>>>>       if (arrMask[i] == 1) numsels++;
>>>>
>>>> which is the standard way to implement the "if (some condition)
>>>> then increment counter" idiom.
>>>>
>>>> As Kasper mentioned, a similar issue was recently discussed here:
>>>>
>>>>     https://stat.ethz.ch/pipermail/bioc-devel/2014-June/005858.html
>>>>
>>>> Cheers,
>>>> H.
>>>>
>>>>
>>>>   so numsels = ++numsels should not confuse the
>>>>> compiler. Anyhow I will change my code to your first suggestion since it
>>>>> is more elegant.
>>>>>
>>>>> Best regards,
>>>>> Christian
>>>>>
>>>>>
>>>>> On 6/23/14 7:13 PM, Romain Fran?ois wrote:
>>>>>
>>>>>>
>>>>>> Le 23 juin 2014 ? 18:28, cstrato <cstrato at aon.at> a ?crit :
>>>>>>
>>>>>>   Dear Romain,
>>>>>>>
>>>>>>> Thank you for your suggestions, I like especially the first one.
>>>>>>>
>>>>>>> However, you did not explain why I have never got this warning
>>>>>>> message on any compiler, and why only one of the two identical Ubuntu
>>>>>>> compilers did give this warning message?
>>>>>>>
>>>>>>> Best regards,
>>>>>>> Christian
>>>>>>>
>>>>>>
>>>>>> I don?t know, but this:
>>>>>>
>>>>>> numsels = ++numsels ;
>>>>>>
>>>>>> seems fishy to me, and so it keeps feeling weird with the addition of
>>>>>> the ternary operator.
>>>>>>
>>>>>> There is obviously a difference of setup between these two machines,
>>>>>> but I don?t have time to sherlock that for you. One of the compilers
>>>>>> is getting more careful than the other. Getting warnings you did not
>>>>>> get before is a good thing, as it helps you update the code with that
>>>>>> new insight.
>>>>>>
>>>>>> Welcome to my world, I?m sometimes thrown all kinds of new warnings
>>>>>> from esoteric compilers, all of them have value .
>>>>>>
>>>>>> Romain
>>>>>>
>>>>>>   On 6/23/14 3:45 PM, Romain Fran?ois wrote:
>>>>>>>
>>>>>>>>
>>>>>>>> Le 23 juin 2014 ? 15:20, cstrato <cstrato at aon.at> a ?crit :
>>>>>>>>
>>>>>>>>   Dear all,
>>>>>>>>>
>>>>>>>>> Since many years the following C++ code does compile on ALL
>>>>>>>>> Bioconductor servers (Linux, Windows, Mac) without any warnings:
>>>>>>>>>
>>>>>>>>>     Int_t numsels = 0;  //number of selected entries
>>>>>>>>>     ...
>>>>>>>>>     for (Int_t i=0; i<size; i++) {
>>>>>>>>>        numsels = (arrMask[i] == 1) ? ++numsels : numsels;
>>>>>>>>>     }//for_i
>>>>>>>>>
>>>>>>>>
>>>>>>>> This is confusing. I would write the loop body like this:
>>>>>>>>
>>>>>>>> numsels += (arrMask[i] == 1) ;
>>>>>>>>
>>>>>>>>
>>>>>>>> or preferably using the STL:
>>>>>>>>
>>>>>>>> Int_t numsels = std::count( begin(arrMask), end(arrMask), 1 ) ;
>>>>>>>>
>>>>>>>> or some other variation of this, i.e. perhaps you don?t have a C++11
>>>>>>>> compiler, so perhaps one of these depending on what is arrMask:
>>>>>>>>
>>>>>>>> Int_t numsels = std::count( arrMask.begin(), arrMask.end(), 1 ) ;
>>>>>>>> Int_t numsels = std::count( arrMask, arrMask + size, 1 ) ;
>>>>>>>>
>>>>>>>> Romain
>>>>>>>>
>>>>>>>>   Even on the recently added release server 'zin2' Linux (Ubuntu
>>>>>>>>> 12.04.4 LTS) the above code compiles w/o warnings.
>>>>>>>>>
>>>>>>>>> However, on the new development server 'zin1' Linux (Ubuntu 12.04.4
>>>>>>>>> LTS) I get suddenly the following warning message:
>>>>>>>>>
>>>>>>>>> Found the following significant warnings:
>>>>>>>>>    XPSPreProcessing.cxx:3026:56: warning: operation on ?numsels? may
>>>>>>>>> be undefined [-Wsequence-point]
>>>>>>>>>
>>>>>>>>> Interestingly, both servers do not only run the same version of
>>>>>>>>> Ubuntu, but also the same version of the C++ compiler, i.e. g++
>>>>>>>>> (Ubuntu/Linaro 4.6.3-1ubuntu5) 4.6.3, and use the same flags, see:
>>>>>>>>> http://bioconductor.org/checkResults/2.14/bioc-LATEST/
>>>>>>>>> zin2-NodeInfo.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> http://bioconductor.org/checkResults/devel/bioc-
>>>>>>>>> LATEST/zin1-NodeInfo.html
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> My question is now, why do I suddenly get the compiler warning?
>>>>>>>>>
>>>>>>>>> The reason why I ask at R-devel and not Bioc-devel is that it may
>>>>>>>>> not only be a Bioc question, since I found the following links:
>>>>>>>>> http://c-faq.com/expr/seqpoints.html
>>>>>>>>> http://stackoverflow.com/questions/16838884/why-i-got-
>>>>>>>>> operation-may-be-undefined-in-statement-expression-in-c
>>>>>>>>>
>>>>>>>>>
>>>>>>>>>
>>>>>>>>> I am not sure if I understand the meaning, but until now I have
>>>>>>>>> never got any warning from any compiler the I have used (including
>>>>>>>>> MS Visual C++).
>>>>>>>>>
>>>>>>>>> Do I really have to replace '++numsels' with 'numsels+1'?
>>>>>>>>>
>>>>>>>>> Best regards,
>>>>>>>>> Christian
>>>>>>>>> _._._._._._._._._._._._._._._._._._
>>>>>>>>> C.h.r.i.s.t.i.a.n   S.t.r.a.t.o.w.a
>>>>>>>>> V.i.e.n.n.a           A.u.s.t.r.i.a
>>>>>>>>> e.m.a.i.l:        cstrato at aon.at
>>>>>>>>> _._._._._._._._._._._._._._._._._._
>>>>>>>>>
>>>>>>>>> ______________________________________________
>>>>>>>>> R-devel at r-project.org mailing list
>>>>>>>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>>>
>>>>>>
>>>>>>
>>>>> ______________________________________________
>>>>> R-devel at r-project.org mailing list
>>>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>>>>
>>>>
>>>>
>>> --
>>> Herv? Pag?s
>>>
>>> Program in Computational Biology
>>> Division of Public Health Sciences
>>> Fred Hutchinson Cancer Research Center
>>> 1100 Fairview Ave. N, M1-B514
>>> P.O. Box 19024
>>> Seattle, WA 98109-1024
>>>
>>> E-mail: hpages at fhcrc.org
>>> Phone:  (206) 667-5791
>>> Fax:    (206) 667-1319
>>>
>>> ______________________________________________
>>> R-devel at r-project.org mailing list
>>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>>
>>
>>          [[alternative HTML version deleted]]
>>
>>
>> ______________________________________________
>> R-devel at r-project.org mailing list
>> https://stat.ethz.ch/mailman/listinfo/r-devel
>>

-- 
Herv? Pag?s

Program in Computational Biology
Division of Public Health Sciences
Fred Hutchinson Cancer Research Center
1100 Fairview Ave. N, M1-B514
P.O. Box 19024
Seattle, WA 98109-1024

E-mail: hpages at fhcrc.org
Phone:  (206) 667-5791
Fax:    (206) 667-1319



[Rd] =?utf-8?q?operation_on_=E2=80=98numsels=E2=80=99_may_be_unde?= =?utf-8?q?fined?=
=?UTF-8?B?SGVydsOpIFBhZ8Oocw==?=