digitalmars.D.bugs - [Issue 8757] New: Require parenthesization of ternary operator when compounded
- d-bugmail puremagic.com (33/33) Oct 04 2012 http://d.puremagic.com/issues/show_bug.cgi?id=8757
- d-bugmail puremagic.com (150/150) Oct 21 2012 http://d.puremagic.com/issues/show_bug.cgi?id=8757
- d-bugmail puremagic.com (21/21) Jan 18 2013 http://d.puremagic.com/issues/show_bug.cgi?id=8757
- d-bugmail puremagic.com (9/12) Jan 19 2013 http://d.puremagic.com/issues/show_bug.cgi?id=8757
- d-bugmail puremagic.com (8/8) Jan 19 2013 http://d.puremagic.com/issues/show_bug.cgi?id=8757
- d-bugmail puremagic.com (21/25) Jan 19 2013 http://d.puremagic.com/issues/show_bug.cgi?id=8757
- d-bugmail puremagic.com (13/13) Jan 19 2013 http://d.puremagic.com/issues/show_bug.cgi?id=8757
- d-bugmail puremagic.com (7/7) Jun 26 2013 http://d.puremagic.com/issues/show_bug.cgi?id=8757
- d-bugmail puremagic.com (17/17) Jun 26 2013 http://d.puremagic.com/issues/show_bug.cgi?id=8757
- d-bugmail puremagic.com (20/28) Jun 26 2013 http://d.puremagic.com/issues/show_bug.cgi?id=8757
- d-bugmail puremagic.com (30/31) Jun 26 2013 My experience with the incredibly stupid stuff that Visual Studio likes ...
http://d.puremagic.com/issues/show_bug.cgi?id=8757 Summary: Require parenthesization of ternary operator when compounded Product: D Version: D2 Platform: All OS/Version: All Status: NEW Severity: enhancement Priority: P2 Component: DMD AssignedTo: nobody puremagic.com ReportedBy: bearophile_hugs eml.cc In past we have discussed in the D newsgroups about the bug-prone precedence of the ?: operator. Analysis of shared code repositories (and articles about static code analysis) shows that this is a common source of bugs. So I suggest to look for wasy to avoid/reduce such bugs in D code. One of the possible ideas is (this is a small breaking change): when the ?: is included in a larger expression, require parentheses around it. auto x1 = y1 ? z1 : w1; // OK auto x2 = x0 + (y1 ? z1 : w1); // OK auto x3 = (x0 + y1) ? z1 : w1; // OK auto x4 = x0 + y1 ? z1 : w1; // Compilation error auto x5 = y1 ? z1 : (y2 ? z2 : w2); // OK auto x6 = y1 ? z1 : y2 ? z2 : w2; // Compilation error In theory this increases the number of parentheses a little, but in practice in many similar situations I already put those parentheses, for readability and to avoid some of my mistakes. Ideas for other solutions are welcome. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Oct 04 2012
http://d.puremagic.com/issues/show_bug.cgi?id=8757 From: http://www.viva64.com/en/examples-V502/ Some bugs caused by ternary operator usage in already tested and used code of professionally-managed projects. ----------------- Grid Control Re-dux V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. void CGridCtrlDemoDlg::UpdateMenuUI() { ... GetMenu()->CheckMenuItem(IDC_HORZ_LINES, MF_BYCOMMAND | (bHorzLines)? MF_CHECKED: MF_UNCHECKED); GetMenu()->CheckMenuItem(IDC_LISTMODE, MF_BYCOMMAND | (m_Grid.GetListMode())? MF_CHECKED: MF_UNCHECKED); GetMenu()->CheckMenuItem(IDC_VERT_LINES, MF_BYCOMMAND | (bVertLines)? MF_CHECKED: MF_UNCHECKED); GetMenu()->EnableMenuItem(IDC_SINGLESELMODE, MF_BYCOMMAND | (m_Grid.GetListMode())? MF_ENABLED: MF_DISABLED|MF_GRAYED); ..... GetMenu()->CheckMenuItem(ID_HIDE2NDROWCOLUMN, MF_BYCOMMAND | (m_bRow2Col2Hidden)? MF_CHECKED: MF_UNCHECKED); ... } This code is incorrect as the priority of '?:' operator is lower than of '|'. The program works correctly because of MF_BYCOMMAND == 0. Nonetheless this code is potentially dangerous. ----------------- FCEUX V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. fceux memwatch.cpp 711 static BOOL CALLBACK MemWatchCallB(....) { ... EnableMenuItem(memwmenu, MEMW_FILE_SAVE, MF_BYCOMMAND | fileChanged ? MF_ENABLED:MF_GRAYED); ... } It works because of sheer luck, since #define MF_BYCOMMAND 0x00000000L. ----------------- IPP Samples V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '|' operator. vm vm_file_win.c 393 vm_file* vm_file_fopen(....) { ... mds[3] = FILE_ATTRIBUTE_NORMAL | (islog == 0) ? 0 : FILE_FLAG_NO_BUFFERING; ... } 0 is always written into mds[3]. Parentheses should be used: mds[3] = FILE_ATTRIBUTE_NORMAL | ((islog == 0) ? 0 : FILE_FLAG_NO_BUFFERING). ----------------- Newton Game Dynamics V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '*' operator. physics dgminkowskiconv.cpp 1061 dgInt32 CalculateConvexShapeIntersection (....) { ... den = dgFloat32 (1.0e-24f) * (den > dgFloat32 (0.0f)) ? dgFloat32 (1.0f) : dgFloat32 (-1.0f); ... } Identical errors can be found in some other places: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '*' operator. physics dgminkowskiconv.cpp 1081 ----------------- Chromium V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '-' operator. views custom_frame_view.cc 400 static const int kClientEdgeThickness; int height() const; bool ShouldShowClientEdge() const; void CustomFrameView::PaintMaximizedFrameBorder( gfx::Canvas* canvas) { ... int edge_height = titlebar_bottom->height() - ShouldShowClientEdge() ? kClientEdgeThickness : 0; ... } ----------------- OTS V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '+' operator. ots gdef.cc 278 bool version_2; bool ots_gdef_parse(....) { ... const unsigned gdef_header_end = static_cast<unsigned>(8) + gdef->version_2 ? static_cast<unsigned>(2) : static_cast<unsigned>(0); ... } ----------------- ReactOS V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '+' operator. uniata id_dma.cpp 1610 VOID NTAPI AtapiDmaInit(....) { ... ULONG treg = 0x54 + (dev < 3) ? (dev << 1) : 7; ... } The "0x54 + (dev < 3)" condition is always true. This is the correct code: ULONG treg = 0x54 + ((dev < 3) ? (dev << 1) : 7). ----------------- Chromium V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '+' operator. rtp_rtcp rtp_receiver_video.cc 480 WebRtc_Word32 RTPReceiverVideo::ReceiveH263Codec(....) { ... if (IP_PACKET_SIZE < parsedPacket.info.H263.dataLength + parsedPacket.info.H263.insert2byteStartCode ? 2:0) ... } Parentheses should be used. Identical errors can be found in some other places: V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '+' operator. rtp_rtcp rtp_receiver_video.cc 504 ----------------- MongoDB V502 Perhaps the '?:' operator works in a different way than it was expected. The '?:' operator has a lower priority than the '<<' operator. version.cpp 107 string sysInfo() { .... stringstream ss; .... ss << (sizeof(char *) == 8) ? " 64bit" : " 32bit"; .... } A very nice sample. 0 or 1 will be printed instead of "32bit"/"64bit". ----------------- -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Oct 21 2012
http://d.puremagic.com/issues/show_bug.cgi?id=8757 Jonathan M Davis <jmdavisProg gmx.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jmdavisProg gmx.com PST --- Any and all operators are bug-prone if you don't understand or remember their precedence rules. If you want the extra protection against precedence screw-ups, then use parens. But I don't see any reason to _require_ them. And honestly, I would be ticked if code like auto x4 = x0 + y1 ? z1 : w1; became illegal. I would never use parens here, because I find the precedence rules in this case to be very clear. This enhancement request is trying to enforce a particular coding/formatting style, and I'm very much opposed to that. The compiler shouldn't care how I format my code. Feel free to use parens to guarantee the correct order of operations if you don't feel confortable with the precedence rules in a particular expression, but I don't want that forced on everyone. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jan 18 2013
http://d.puremagic.com/issues/show_bug.cgi?id=8757Any and all operators are bug-prone if you don't understand or remember their precedence rules. If you want the extra protection against precedence screw-ups, then use parens. But I don't see any reason to _require_ them.I've shown that it's a common enough bug even for expert C/C++ programmers. So saying "I don't see any reason" is too much weak. To invalidate this enhancement request you have to show statistics that confirm your point. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jan 19 2013
http://d.puremagic.com/issues/show_bug.cgi?id=8757 PST --- I don't care if it solves half the bugs involving ternary operators that ever happen. You're suggesting that we force programmers to format their code in a particular way, and I object to that. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jan 19 2013
http://d.puremagic.com/issues/show_bug.cgi?id=8757I don't care if it solves half the bugs involving ternary operators that ever happen.So you are saying that data in language design should be ignored?You're suggesting that we force programmers to format their code in a particular way, and I object to that.C language has some design mistakes, like in its precedence rules. A well designed language, and one of the design principles of D, has to help the programmer avoid the most common bugs. D fixes some of the mistakes of C design. This is a "formatting forced by D", that has saved me few times: void main() { int x, y; auto z = x & 1 == y; } temp.d(3): Error: 1 == y must be parenthesized when next to operator & See also issue 5409 for something similar. You don't want to many parentheses in expressions, but few strategically placed ones are a small price to pay to save you code from common mistakes. Your words don't hold water, unless you show that adding a ( ) there harms readability or some other real coding quality. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jan 19 2013
http://d.puremagic.com/issues/show_bug.cgi?id=8757 Dicebot <m.strashun gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.strashun gmail.com It is hard to add something like that because of backward compatibility issues but I vote for this approaches. Personal formatting preferences mean nothing compared with possibility to remove common source of bugs. And this one is really common. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jan 19 2013
http://d.puremagic.com/issues/show_bug.cgi?id=8757 The Visual Studio 2012 warning: http://msdn.microsoft.com/en-us/library/ms182085.aspx -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jun 26 2013
http://d.puremagic.com/issues/show_bug.cgi?id=8757 PDT --- Visual Studio is one of the worst compilers I've ever seen with regards to warnings. It has tons of annoying, useless warnings which don't help one whit, forcing you to shut them off. So, I would consider Visual Studio to be a horrible example of what you should or shouldn't warn against. And I would put any warning about operator precedence on the list of warnings that should be removed. It subverts the language when you're forced to add parens rather than use operator precedence. It's one thing to force parens with the language (i.e. make it an error) in order to prevent bugs (which I'm still generally against), but it's far worse to warn about it, because the compiler is basically claiming the language is wrong, making it so that you have to do what the compiler says rather than what the language considers perfectly legal. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jun 26 2013
http://d.puremagic.com/issues/show_bug.cgi?id=8757Visual Studio is one of the worst compilers I've ever seen with regards to warnings. It has tons of annoying, useless warnings which don't help one whit, forcing you to shut them off. So, I would consider Visual Studio to be a horrible example of what you should or shouldn't warn against.What evidence do you have to back your claim? This article shows a significant amount of people enjoying Visual Studio static analysis to detect bugs in a large amount of C++ code: http://randomascii.wordpress.com/2013/06/24/two-years-and-thousands-of-bugs-of-/ They enable only a certain subset of the warnings and they use a Python script that shows new warnings only the first time they appear in the code base. This is a simple but very useful memory, to solve one of the most important downsides of warnings.And I would put any warning about operator precedence on the list of warnings that should be removed. It subverts the language when you're forced to add parens rather than use operator precedence.Experience has shown again and again that humans become unreliable at keeping in mind operator precedence when it goes beyond a small number of rules and levels. From the evidence on the frequency of bugs related to operator precedence in C and C++ code, they go past such limit. This enhancement request is asking for an warning, then for it to become a deprecation, and later an error. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jun 26 2013
http://d.puremagic.com/issues/show_bug.cgi?id=8757 PDT ---What evidence do you have to back your claim?My experience with the incredibly stupid stuff that Visual Studio likes to complain about. My favorite example would be converting to bool. C++ allows implicit conversion to bool for built-in types, so it's perfectly legal to do stuff like bool foo(int* bar) { return bar; } But the VS insists on complaining about a lot of conversions to bool - particularly when returning. It doesn't even like casts (complaining about potentially inefficient code, which makes no sense, since it would be trivial to adjust the code to be more efficient without changing the semantics). If you don't turn that warning off, you're forced to do stuff like return bar != 0; or return bar ? true : false; It would be one thing if the language had more restrictive conversion rules (like D), but it doesn't. So, VS ends up complaining about stuff which is perfectly valid C++ and not buggy in the least. It's _really_, _really_ annoying, and that's just _one _example. The compiler shouldn't be complaining about stuff that isn't actually broken. And it _definitely_ shouldn't be complaining about stuff that is 100% valid in the language and is 100% correct code just because some compiler writer decided that they thought that people should or shouldn't write their code in a particular way. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jun 26 2013