http://d.puremagic.com/issues/show_bug.cgi?id=10862

           Summary: Assignment inside if condition still sometimes
                    accepted
           Product: D
           Version: D2
          Platform: All
        OS/Version: All
            Status: NEW
          Severity: normal
          Priority: P2
         Component: DMD
        AssignedTo: nobody@puremagic.com
        ReportedBy: andrei@erdani.com


--- Comment #0 from Andrei Alexandrescu <andrei@erdani.com> 2013-08-20 14:40:37 PDT ---
Consider:

void main() {
    int a, b;
    if (a = b) {}
}

./test.d(4): Error: assignment cannot be used as a condition, perhaps == was
meant?

Nice! Now consider:

void main() {
    int a, b;
    if ((a = b) = 0) {}
}

This compiles diagnostic-free. The shape if (expr1 = expr2) should be disallowed at a grammatical level, i.e. during parsing.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10862


Jonathan M Davis <jmdavisProg@gmx.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |jmdavisProg@gmx.com


--- Comment #1 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-08-20 14:52:21 PDT ---
Are you saying that

if((a = b) == 0)

should be disallowed or just

if((a = b) = 0)

The first syntax is what is used to shut up gcc's warnings about using the assignment operator in a condition, whereas the second clearly is using the assignment operator in the same way that

if(a = b)

is, and that's clearly illegal.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10862


Andrej Mitrovic <andrej.mitrovich@gmail.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |andrej.mitrovich@gmail.com


--- Comment #2 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-08-20 14:54:29 PDT ---
Whoever fixes this: Just be careful not to disallow:

if (auto a = b) {}

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10862



--- Comment #3 from Andrei Alexandrescu <andrei@erdani.com> 2013-08-20 14:57:08 PDT ---
(In reply to comment #1)
> Are you saying that
> 
> if((a = b) == 0)
> 
> should be disallowed or just
> 
> if((a = b) = 0)

Only the latter. It fits the pattern "if (expr1 = expr2)".

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10862



--- Comment #4 from Andrei Alexandrescu <andrei@erdani.com> 2013-08-20 14:58:03 PDT ---
(In reply to comment #3)
> (In reply to comment #1)
> > Are you saying that
> > 
> > if((a = b) == 0)
> > 
> > should be disallowed or just
> > 
> > if((a = b) = 0)
> 
> Only the latter. It fits the pattern "if (expr1 = expr2)".

(...and there should be a space after "if". In fact, the compiler should
enforce it :o).)

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10862



--- Comment #5 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-08-20 15:43:58 PDT ---
> (...and there should be a space after "if". In fact, the compiler should
> enforce it :o).)

LOL. Yeah, well, we're in trouble if the compiler is enforcing formatting. And to me, putting a space after the if is just wasted space, much as I know you like it.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10862



--- Comment #6 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-08-20 16:10:02 PDT ---
(In reply to comment #5)
> > (...and there should be a space after "if". In fact, the compiler should
> > enforce it :o).)
> 
> LOL. Yeah, well, we're in trouble if the compiler is enforcing formatting. And to me, putting a space after the if is just wasted space, much as I know you like it.

Rightit'snotlikespacescontributetoreadabilityoranything.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10862



--- Comment #7 from Jonathan M Davis <jmdavisProg@gmx.com> 2013-08-20 16:35:35 PDT ---
> Rightit'snotlikespacescontributetoreadabilityoranything.

There's a huge difference between not inserting a space between elements which are already separated by other grammar elements and not putting spaces between words, and even if you prefer having the space after the if for whatever reason, I don't see how you could think that if(expr) over if (expr) has an impact on legibility like not putting spaces between words would.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10862



--- Comment #8 from Andrej Mitrovic <andrej.mitrovich@gmail.com> 2013-08-20 17:21:26 PDT ---
(In reply to comment #7)
> > Rightit'snotlikespacescontributetoreadabilityoranything.
> 
> There's a huge difference between not inserting a space between elements which are already separated by other grammar elements and not putting spaces between words.

W.r.t. "if(" vs. "if (", it's not a big deal. However I have to argue about
your statement.

Someone was talking about the Fox toolkit recently, how it might be a good idea to port it to D. But then I had a look at it's codebase. Here's a glimpse of the code:

----- 
if(XGetSelectionOwner((Display*)getApp()->getDisplay(),getApp()->xdndSelection)!=xid){
      fxwarning("%s::beginDrag: failed to acquire DND
selection.\n",getClassName());
      return false;
      }
    resizeElms(getApp()->xdndTypeList,numtypes);
    memcpy(getApp()->xdndTypeList,types,sizeof(FXDragType)*numtypes);
    getApp()->xdndNumTypes=numtypes;

XChangeProperty((Display*)getApp()->getDisplay(),xid,getApp()->xdndTypes,XA_ATOM,32,PropModeReplace,(unsigned
char*)getApp()->xdndTypeList,getApp()->xdndNumTypes);

XChangeProperty((Display*)getApp()->getDisplay(),xid,getApp()->xdndActions,XA_ATOM,32,PropModeReplace,(unsigned
char*)getApp()->xdndActionList,6);
-----

Some of those lines span over 160 characters. I can barely see which argument becomes which parameter in these function calls, and everything is mushed together. There's a maximum effort of saving whitespace, as if having all characters on the screen is somehow going to enable someone to automatically process more information.

We're not machines that parse grammars. Grammar rules or not, we need some breaks and some whitespace to be able to read and comprehend.

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------

http://d.puremagic.com/issues/show_bug.cgi?id=10862


Nick Sabalausky <cbkbbejeap@mailinator.com> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
                 CC|                            |cbkbbejeap@mailinator.com


--- Comment #9 from Nick Sabalausky <cbkbbejeap@mailinator.com> 2013-08-20 19:38:39 PDT ---
(In reply to comment #8)
> if(XGetSelectionOwner((Display*)getApp()->getDisplay(),getApp()->xdndSelection)!=xid){

A: if
(XGetSelectionOwner((Display*)getApp()->getDisplay(),getApp()->xdndSelection)!=xid){

B: if (XGetSelectionOwner ((Display*)getApp ()->getDisplay (),getApp
()->xdndSelection)!=xid){

C: if( XGetSelectionOwner( (Display*)getApp()->getDisplay(),
getApp()->xdndSelection ) != xid ){


I'll take C, please!

-- 
Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email
------- You are receiving this mail because: -------