[Lldb-commits] [lldb] r259962 - Fix "thread backtrace -s": option was misparsed because of a missing break.

[Jason Molenda via lldb-commits]

Hi Zachary, related to this bug, I was looking into clang diagnostics and there is one that would have helped us catch this, -Wimplicit-fallthrough.  It would have issued a warning for CommandObjectThread.cpp (when built with clang, of course) but it won't generate a warning for

case 0:
case 1:
case 2: 
 ... do something for values 0-2
break;


There are some places in lldb where we have code AND we intentionally fall through; remarkably often there is a comment that it is intentional, like

        case eConnectionStatusNoConnection:     // No connection
        case eConnectionStatusLostConnection:   // Lost connection while connected to a valid connection
            done = true;
            // Fall through...
        case eConnectionStatusTimedOut:         // Request timed out
            if (log)
                error.LogIfError (log,


etc.  In this case, if we add [[clang::fallthrough]]; before the '// Fall through...' comment, clang will not emit a warning.

I don't know the [[clang::fallthrough]]; syntax, I have a feeling it might be some C++11 thing that other compilers will ignore.  Do you know if MSVC will ignore this markup if we add it to the sources in the places where we mean to fall through?


(a quick look over the warnings makes me think there are probably another dozen of these bugs in the lldb codebase.  I'll look at those more closely on Monday.)


> On Feb 5, 2016, at 4:46 PM, Zachary Turner via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> 
> Sounds like we don't have a test for thread backtrace -s.  I know I usually argue against tests of the CLI, but only when they're used instead of tests for the api.  Seems like we should have at least one test for every option of every command.
> 
> On Fri, Feb 5, 2016 at 4:35 PM Jim Ingham via lldb-commits <lldb-commits at lists.llvm.org> wrote:
> Author: jingham
> Date: Fri Feb  5 18:31:23 2016
> New Revision: 259962
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=259962&view=rev
> Log:
> Fix "thread backtrace -s": option was misparsed because of a missing break.
> 
> <rdar://problem/24525106>
> 
> Modified:
>     lldb/trunk/source/Commands/CommandObjectThread.cpp
> 
> Modified: lldb/trunk/source/Commands/CommandObjectThread.cpp
> URL: http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectThread.cpp?rev=259962&r1=259961&r2=259962&view=diff
> ==============================================================================
> --- lldb/trunk/source/Commands/CommandObjectThread.cpp (original)
> +++ lldb/trunk/source/Commands/CommandObjectThread.cpp Fri Feb  5 18:31:23 2016
> @@ -194,6 +194,7 @@ public:
>                      if (!success)
>                          error.SetErrorStringWithFormat("invalid integer value for option '%c'", short_option);
>                  }
> +                break;
>                  case 'e':
>                  {
>                      bool success;
> 
> 
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
> _______________________________________________
> lldb-commits mailing list
> lldb-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits