Issue 96087 - basic: ambiguous && || sequence
Summary: basic: ambiguous && || sequence
Status: CLOSED FIXED
Alias: None
Product: App Dev
Classification: Unclassified
Component: scripting (show other issues)
Version: 3.3.0 or older (OOo)
Hardware: All Linux, all
: P3 Trivial
Target Milestone: ---
Assignee: tkr
QA Contact: Unknown
URL:
Keywords:
Depends on:
Blocks: 96084
  Show dependency tree
 
Reported: 2008-11-11 10:35 UTC by caolanm
Modified: 2013-02-24 21:01 UTC (History)
2 users (show)

See Also:
Issue Type: PATCH
Latest Confirmation in: ---
Developer Difficulty: ---

Attachments
dodgy code (1.31 KB, patch)
2008-11-11 10:38 UTC, caolanm 		no flags Details | Diff
Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this issue.
Description caolanm 2008-11-11 10:35:30 UTC 
X && Y || Z
binds operator precedence-wise as
(X && Y) || Z
not
X && (Y || Z)

so I am a little suspicious about...

in basic/source/sbx/sbxexec.cxx
while( *p && rCharClass.isAlphaNumeric( *p ) || *p == '_' )
which really means...
while( (*p && rCharClass.isAlphaNumeric( *p )) || *p == '_' )
while
while( *p && (rCharClass.isAlphaNumeric( *p ) || *p == '_' ))
seems the more likely desired meaning ?

similarly 
while( refVar.Is() && *p == '.' || *p == '!' )
means
while( (refVar.Is() && *p == '.') || *p == '!' )
while
while( refVar.Is() && (*p == '.' || *p == '!') )
seems more the likely intent ?

and finally in
basic/source/comp/scanner.cxx
while( *pLine && ( *pLine == ' ' ) || ( *pLine == '\t' ) || ( *pLine == '\f' ) )
means
while( (*pLine && ( *pLine == ' ' )) || ( *pLine == '\t' ) || ( *pLine == '\f' ) )
but I suspect
while( *pLine && (( *pLine == ' ' ) || ( *pLine == '\t' ) || ( *pLine == '\f' ) ))
is what we want.

Patch attached to change to what I *suspect* we might mean.
Comment 1 caolanm 2008-11-11 10:38:29 UTC 
Created attachment 57868 [details]
dodgy code
Comment 2 kay.ramme 2008-11-11 11:10:28 UTC 
Andreas, please have a look ...
Comment 3 ab 2008-11-12 09:12:47 UTC 
STARTED
Comment 4 ab 2008-11-13 10:22:54 UTC 
-> OOo 3.1
Comment 5 ab 2009-01-09 08:11:10 UTC 
This somehow confuses me. I wonder why this bug does not affect the functionality.
I will try to understand this, otherwise I'll move it to 3.2 to be on the save
side in
the sense of "never touch a running system".
Comment 6 ab 2009-01-13 13:49:47 UTC 
I had a closer look and interestingly this seems to make no difference,
at least for two of the cases.

The results of (X && Y) || Z and X && (Y || Z) only differ if
X==false/Y==true/Z==true or X==false/Y==false/Z==true

In the cases while( *p... and while( *pLine... this is not possible.
If *p / *pLine is false, the char is 0, so it cannot be anything else.

This is not so obvious for the while( refVar.Is() case, but the cri-
tical cases require z==true -> *p=='!'. I've never seen this, so
this probably is an exotic case.

At least this explains why this hasn't caused any problems so far.
Applied patch, FIXED
Comment 7 ab 2009-01-21 12:36:03 UTC 
ab->tkr: Please verify in source code
Comment 8 tkr 2009-01-22 10:49:31 UTC 
Verified in source code
Comment 9 dtardon 2009-04-08 14:53:54 UTC 
seen in DEV300_m45 and OOO310_m9