0x8000-0000 marked an inline comment as done.
0x8000-0000 added a comment.
Added regex for exception names and rebased onto current 'main' branch.
================
Comment at:
clang-tools-extra/clang-tidy/readability/ReadabilityTidyModule.cpp:136
+ CheckFactories.registerCheck<VariableLengthCheck>(
+ "readability-variable-length");
}
----------------
aaron.ballman wrote:
> Is there a reason this should be restricted to variables? For example,
> wouldn't the same functionality be useful for type names, or dare I say it,
> even macro names? I'm wondering if this should be
> `readability-identifier-length` to be more generic.
I consider those to be in separate namespaces. I suppose we could have a single
checker with multiple rules, but on the other hand I don't want to combine too
many things into one, just because they share the "compare length" dimension.
================
Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:20
+
+const unsigned DefaultMinimumVariableNameLength = 3;
+const unsigned DefaultMinimumLoopCounterNameLength = 2;
----------------
aaron.ballman wrote:
> Should it be possible to enforce parameters differently than local variables?
> (It seems a bit odd that we'd allow you to specify exception variable length
> separate from loops but not function parameters separate from locals.)
Exceptions and Loops historically are "throw-away" / "don't care" / "one-off"
variables. Parameter names are names, and they are the interface between the
outside of the routine and the processing inside; other than historical, I
don't see good arguments (sic) to allow single-letter parameter names.
Note that this check will be quite noisy on a legacy code base, and people will
find little reason to invest the work to remove the warnings. But if somebody
starts something new and want to enforce some quality attributes, it is the
right tool at the right time. There will be new projects starting in 2021 and
2022 that could benefit from this, I hope.
================
Comment at: clang-tools-extra/clang-tidy/readability/VariableLengthCheck.cpp:26
+
+const char ErrorMessage[] = "%select{|exception |loop }0 variable name %1 is "
+ "too short, expected at least %2 characters";
----------------
aaron.ballman wrote:
> It looks like there will be whitespace issues with this. If the variable is a
> loop or exception, it'll have an extra space (`loop variable name`) and if
> it's not, it will start with a leading space (` variable name`).
This was recommended by a previous reviewer. Any alternative suggestion here?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D97753/new/
https://reviews.llvm.org/D97753
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
