[PATCH] D56295: [TSan] Use switches when dealing with enums
Julian Lettner via Phabricator via llvm-commits
llvm-commits at lists.llvm.org
Fri Jan 4 09:36:57 PST 2019
yln marked 2 inline comments as done.
yln added inline comments.
================
Comment at: lib/tsan/rtl/tsan_debugging.cc:41
+ }
}
----------------
delcypher wrote:
> @yln I'd feel slightly happier by having `UNREACHABLE("Unhandled case");` after the switch block (i.e. don't add a default case) so that if we do fall through (because someone missed a compiler warning, or perhaps they have warnings turned off) we stop execution rather than having undefined behaviour (reach end of function but no return statement).
>
> This applies to all the switch blocks you've added.
>
> I won't insist on this but if you think this a reasonable change that doesn't clutter things too much then I think you should do it.
Good point! Thanks for the review, Dan.
================
Comment at: lib/tsan/rtl/tsan_suppressions.cc:96
- else if (typ == ReportTypeErrnoInSignal)
- return kSuppressionNone;
- else if (typ == ReportTypeDeadlock)
----------------
yln wrote:
> Should this be `kSuppressionSignal`?
@dvyukov @kubamracek: can you please confirm that this is intentional and not an oversight?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56295/new/
https://reviews.llvm.org/D56295
More information about the llvm-commits
mailing list