<div dir="ltr">On MSVC 2015 I get this:<div><br></div><div><div>1>c:\users\zach\documents\visual studio 2015\projects\consoleapplication2\consoleapplication2\consoleapplication2.cpp(22): warning C5030: attribute 'clang::fallthrough' is not recognized</div><div>1>c:\users\zach\documents\visual studio 2015\projects\consoleapplication2\consoleapplication2\consoleapplication2.cpp(22): warning C4649: attributes are ignored in this context</div></div><div><br></div><div>I wouldn't mind disabling the warning, but the bigger problem is that the android folks are still using MSVC 2013.</div><div><br></div><div>I would suggest making a #define out of it.</div><div><br></div><div>#if defined(CLANG)</div><div><div>#define LLVM_CASE_FALLTHROUGH() [[clang::fallthrough]];</div></div><div>#else</div><div>#define <span style="line-height:1.5">LLVM_CASE_FALLTHROUGH()</span></div><div><span style="line-height:1.5">#endif</span></div><div><span style="line-height:1.5"><br></span></div><div><span style="line-height:1.5">I would also suggest trying to get this into LLVM, as it seems generally useful to the larger project and non-specific to LLDB.</span></div></div><br><div class="gmail_quote"><div dir="ltr">On Sat, Feb 6, 2016 at 2:25 AM Jason Molenda <<a href="mailto:jason@molenda.com">jason@molenda.com</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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<br>
<br>
case 0:<br>
case 1:<br>
case 2:<br>
... do something for values 0-2<br>
break;<br>
<br>
<br>
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<br>
<br>
case eConnectionStatusNoConnection: // No connection<br>
case eConnectionStatusLostConnection: // Lost connection while connected to a valid connection<br>
done = true;<br>
// Fall through...<br>
case eConnectionStatusTimedOut: // Request timed out<br>
if (log)<br>
error.LogIfError (log,<br>
<br>
<br>
etc. In this case, if we add [[clang::fallthrough]]; before the '// Fall through...' comment, clang will not emit a warning.<br>
<br>
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?<br>
<br>
<br>
(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.)<br>
<br>
<br>
> On Feb 5, 2016, at 4:46 PM, Zachary Turner via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a>> wrote:<br>
><br>
> 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.<br>
><br>
> On Fri, Feb 5, 2016 at 4:35 PM Jim Ingham via lldb-commits <<a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a>> wrote:<br>
> Author: jingham<br>
> Date: Fri Feb 5 18:31:23 2016<br>
> New Revision: 259962<br>
><br>
> URL: <a href="http://llvm.org/viewvc/llvm-project?rev=259962&view=rev" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project?rev=259962&view=rev</a><br>
> Log:<br>
> Fix "thread backtrace -s": option was misparsed because of a missing break.<br>
><br>
> <rdar://problem/24525106><br>
><br>
> Modified:<br>
> lldb/trunk/source/Commands/CommandObjectThread.cpp<br>
><br>
> Modified: lldb/trunk/source/Commands/CommandObjectThread.cpp<br>
> URL: <a href="http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectThread.cpp?rev=259962&r1=259961&r2=259962&view=diff" rel="noreferrer" target="_blank">http://llvm.org/viewvc/llvm-project/lldb/trunk/source/Commands/CommandObjectThread.cpp?rev=259962&r1=259961&r2=259962&view=diff</a><br>
> ==============================================================================<br>
> --- lldb/trunk/source/Commands/CommandObjectThread.cpp (original)<br>
> +++ lldb/trunk/source/Commands/CommandObjectThread.cpp Fri Feb 5 18:31:23 2016<br>
> @@ -194,6 +194,7 @@ public:<br>
> if (!success)<br>
> error.SetErrorStringWithFormat("invalid integer value for option '%c'", short_option);<br>
> }<br>
> + break;<br>
> case 'e':<br>
> {<br>
> bool success;<br>
><br>
><br>
> _______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br>
> _______________________________________________<br>
> lldb-commits mailing list<br>
> <a href="mailto:lldb-commits@lists.llvm.org" target="_blank">lldb-commits@lists.llvm.org</a><br>
> <a href="http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits" rel="noreferrer" target="_blank">http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits</a><br>
<br>
</blockquote></div>