digitalmars.D.bugs - [Issue 3536] New: [patch] Make switch case error at unintentional fallthrough. (allow intentional fallthrough)
- d-bugmail puremagic.com (125/125) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (7/7) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (7/7) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (7/7) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (7/7) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (7/7) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (7/7) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (7/7) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (7/7) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (9/9) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (11/11) Nov 20 2009 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (10/10) Jun 19 2010 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (6/6) May 27 2012 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (11/11) May 27 2012 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (15/16) May 27 2012 http://d.puremagic.com/issues/show_bug.cgi?id=3536
- d-bugmail puremagic.com (7/16) May 29 2012 http://d.puremagic.com/issues/show_bug.cgi?id=3536
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Summary: [patch] Make switch case error at unintentional fallthrough. (allow intentional fallthrough) Product: D Version: 2.036 Platform: Other OS/Version: Linux Status: NEW Severity: enhancement Priority: P2 Component: DMD AssignedTo: nobody puremagic.com ReportedBy: chadjoan gmail.com Created an attachment (id=504) Contains patch and test cases. [patch] Make switch case error at unintentional fallthrough. (allow intentional fallthrough) This patch implements the following: <qoute> Case statements and default statements come in two flavors: 'fall through' and 'unconditional branch'. The 'unconditional branch' variety are denoted with a normal colon (:) like C and C++ case statements. However, unlike C and C++, the block contained by these must end in one of these unconditional branch statements: break, return, continue, goto, goto case, goto default, or throw. The reasons for this are to prevent the common mistake of forgetting to place break at the end of a case statement and to encourage the catching of such mistakes while porting C and C++ code to D. The 'fall through' variety are denoted with the !: token. These behave like the traditional C and C++ case statements and allow for case statements to 'fall through' to subsequent case values. </qoute> For example: switch (i) { case 1!: // Intent to use fall through behavior. x = 3; case 2!: // It's OK to decide to not actually fall through. x = 4; break; case 3,4,5: x = 5; break; case 6: // Error: You either forgot a break; or need to use !: instead of : case 7: // Fine, ends with goto case. goto case 1; case 8: break; x = 6; // Error: break; must be the last statement for case 8. } The complete patch is given as switchcase.patch. The patch is from dmd 2.036. There are two test cases in the attached file: pass.d and fail.d. pass.d should compile while fail.d is full of things that shouldn't compile. Patches of finer granularity are also given: switchcase-html.patch // Patches the entire html directory (mostly the language spec). switchcase-src.patch // Patches the entire src directory. switchcase-src-dmd.patch // Patches src/dmd (only the things needed to make dmd accept the changes). switchcase-src-phobos.patch // Patches src/phobos switchcase-src-druntime.patch // Patches src/druntime These should make it easier to play with the patch. For example you could patch dmd and not phobos, then see what kind of error messages it gives when run on phobos' pre-patch D code. As far as I could tell, phobos passed all unit tests after the changes. This was difficult due to http://d.puremagic.com/issues/show_bug.cgi?id=3140 If you don't care about combing your code for fallthrough bugs, you can use a sed expression to update most code to the new switch-case: sed -r 's/((case|default)\s*?[_a-zA-Z0-9]*?):/\1!:/' input.d > output.d I realize that I could have made swapped !: and : to make existing code survive unmodified. I decided against that since : is what people will natural use (due to experience from C-like languages) and they may not realize !: exists until it is too late. This makes the intuitive default option become the safe one, while making the harder to find/use feature be the unsafe option (fallthrough). You'll probably notice that I haven't added assert to the list of unconditional branches. That's because it's conditional. assert(1) won't divert execution, and it is trivial to write asserts that may or may not end the program depending on some runtime value. The common pattern is much more restricted: assert(0). If the compiler bothers to dig into the assert's parameter and finds a literal 0 or false there, then it can safely assume that execution is not supposed to reach that point. assert(0) also raises the question: what about assert(true ? 0 : 1); ? what about assert(zero!()); ? Should the compiler be expected to figure those out, or expected to figure out that the zero it's looking at isn't one of them? If you specify that constant-folded zeros are not allowed, then dmd's implementation is fairly straightforward: int ExpStatement::isUnconditionalBranch() { AssertExp *assertExp = dynamic_cast<AssertExp*>(this->exp); if ( assertExp ) { return assertExp->e1->isBool(FALSE); } return FALSE; } Other minor known issues: The case range syntax suggests the existence of these: case 50: .. case 54!: case 55!: .. case 59: however those don't really make sense. The patch is written such that those are treated as invalid D code and are not recognized by the spec or by dmd. There are some cases in which an erraneous case statement can cause the compiler to emit extra garbage error messages: T bar(T)(T baz) { return baz; } ... switch(i) { case '1': case '2': // Error: The last statement in a case or default statement must be... int j = .bar!(int)(i); // Error: function expected before (), not __error of type int break; ... } The first error is expected. The second is garbage. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Created an attachment (id=505) Complete patch. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Created an attachment (id=506) Patch for all source files only (excludes spec/doc changes). -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Created an attachment (id=507) Patch for dmd compiler source files only. diff of ${dmd}/src/dmd -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Created an attachment (id=508) Patch for phobos source files only. diff of ${dmd}/src/phobos -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Created an attachment (id=509) Patch for druntime source files only. diff of ${dmd}/src/druntime -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Created an attachment (id=510) Patch for dmd docs only. diff of ${dmd}/html -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Created an attachment (id=511) Test cases that should compile with a patched compiler. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Created an attachment (id=512) Things that should not compile with the patched compiler. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Chad Joan <chadjoan gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- patch| | -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Chad Joan <chadjoan gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- description|only (excludes spec/doc |only (excludes spec/doc |changes). |changes). diff of | |${dmd}/src -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Nov 20 2009
http://d.puremagic.com/issues/show_bug.cgi?id=3536 bearophile_hugs eml.cc changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bearophile_hugs eml.cc See also bug 4349 -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
Jun 19 2010
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Is it possible to close this bug now? -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
May 27 2012
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Chad Joan <chadjoan gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|NEW |RESOLVED Resolution| |WONTFIX Yes, it can be closed. Here ya go. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
May 27 2012
http://d.puremagic.com/issues/show_bug.cgi?id=3536 Walter Bright <bugzilla digitalmars.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |bugzilla digitalmars.com 13:47:13 PDT ---Yes, it can be closed. Here ya go.I do appreciate the effort you put into this (especially since you've gone the distance with test cases and documentation), and am sorry that it hasn't caught interest. One of the most difficult parts of my job with D is to say no, and sometimes I feel like Doctor No. I know how hard it is on contributors like you. -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------
May 27 2012
http://d.puremagic.com/issues/show_bug.cgi?id=3536Hey, thanks for the acknowledgment! -- Configure issuemail: http://d.puremagic.com/issues/userprefs.cgi?tab=email ------- You are receiving this mail because: -------Yes, it can be closed. Here ya go.I do appreciate the effort you put into this (especially since you've gone the distance with test cases and documentation), and am sorry that it hasn't caught interest. One of the most difficult parts of my job with D is to say no, and sometimes I feel like Doctor No. I know how hard it is on contributors like you.
May 29 2012