[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