[PATCH] D114025: [clang][NFC] Inclusive terms: replace some uses of sanity in clang

Aaron Ballman via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Fri Nov 19 05:10:33 PST 2021


aaron.ballman accepted this revision.
aaron.ballman added a comment.

In D114025#3141414 <https://reviews.llvm.org/D114025#3141414>, @keryell wrote:

> In D114025#3140192 <https://reviews.llvm.org/D114025#3140192>, @Quuxplusone wrote:
>
>> I think "sanity-check" could be reasonably replaced with "smoke-test," but (1) this PR doesn't do that, and (2) the phrase "smoke-test" is probably //harder// to understand,
>
> It seems difficult considering the potential atmospheric pollution, carbon footprint, health issues, lung cancer, drug abuse, etc. implied.

This is not a constructive comment either, please stop.

In D114025#3141358 <https://reviews.llvm.org/D114025#3141358>, @ZarkoCA wrote:

> @Quuxplusone Thanks for thorough review.

+1, you caught some stuff I was glossing over, but this is much improved. I made a few tiny suggestions (take them or leave them). Continues to LGTM



================
Comment at: clang/include/clang/AST/Redeclarable.h:261
       assert(Current && "Advancing while iterator has reached end");
-      // Sanity check to avoid infinite loop on invalid redecl chain.
+      // Validation check to avoid infinite loop on invalid redecl chain.
       if (Current->isFirstDecl()) {
----------------
Quuxplusone wrote:
> `// Make sure we don't infinite-loop on an invalid redecl chain. This should never happen.`
Alternatively, "Make sure we don't infinitely loop..."


================
Comment at: clang/lib/Sema/SemaChecking.cpp:5536
+    // GCC does not enforce these rules for GNU atomics, but we do, because if
+    // we didn't it would be very confusing [For whom? How so?]
     auto IsAllowedValueType = [&](QualType ValType) {
----------------



================
Comment at: clang/lib/Sema/SemaChecking.cpp:5578
+    // the GNU atomics specification, but we enforce it, because if we didn't it
+    // would be very confusing [For whom? How so?]
     Diag(ExprRange.getBegin(), diag::err_atomic_op_needs_trivial_copy)
----------------



Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D114025/new/

https://reviews.llvm.org/D114025



More information about the cfe-commits mailing list