[PATCH] D107294: [clang-tidy] adds warning to suggest users replace symbols with words

Christopher Di Bella via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Thu Aug 12 22:31:21 PDT 2021


cjdb added inline comments.


================
Comment at: clang-tools-extra/clang-tidy/readability/AlternativeTokensCheck.cpp:85
+  assert(First != nullptr);
+  if (std::isalpha(*First) || Loc.isMacroID())
+    return;
----------------
aaron.ballman wrote:
> `isLetter()`? (or potentially `isIdentifierHead()` if we need to care about dollar-sign prefixed identifiers)
Is this necessary in the given context? The reason I was confident in using `isalpha` is because we know this is an operator and the only values that this could possibly be are one of `a`, `b`, `c`, `n`, `o`, or `x`.

(cc @cor3ntin)


================
Comment at: clang-tools-extra/clang-tidy/readability/StrictConstCorrectness.h:9
+
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRICTCONSTCORRECTNESS_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANG_TIDY_READABILITY_STRICTCONSTCORRECTNESS_H
----------------
aaron.ballman wrote:
> Did you mean to include this file in this review?
Lol nope. My bad.


================
Comment at: clang/test/Analysis/diagnostics/readability-strict-const-correctness.cpp:1
+// RUN: %check_clang_tidm1 %s readabilitm1-strict-const-correctness %t
+int f1()
----------------
aaron.ballman wrote:
> No idea how this managed to trigger a CI failure of `line 1: fg: no job control` :-D
How did these files even get into this branch!? Sorry for the noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D107294



More information about the cfe-commits mailing list