llvm dev - Aug 2021 - Top-level .clang-tidy options and VariableName suggestion on CodingStandards

Hi all,

I created a .clang-tidy patch https://reviews.llvm.org/D108265 to push
the variable naming related options from
top-level .clang-tidy down to the projects actually using the style
(llvm/, clang-tools-extra/).

The rationale is: uppercase VariableName doesn't really reflect the
reality for many projects and (in my view) is a bad
suggestion for new projects. My summary about the reality:

* Use VariableName and have readability-identifier-naming check: llvm,
clang-tools-extra, parallel-libs, polly
* Use VariableName but disable readability-identifier-naming: clang
* Use lowercase variable names and disable
readability-identifier-naming: (mostly lowercase) compiler-rt, flang,
lldb
* Use lowercase variable names and enable
readability-identifier-naming: libc, libclc, libcxx, libcxxabi,
libunwind, lld, mlir, openmp, pstl

(I noticed the issue because some Phabricator bot wanted me to use
VariableName for a libunwind patch of mine.
The request looked unreasonable to me. I could add
libunwind/.clang-tidy, but I figured out the top-level .clang-tidy
problem.)
>From my vague memory about previous discussion on variable names, most
people agree that VariableName was a mistake and new projects do not
necessarily need to follow.

However, one feedback is that
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
says
> Variable names should be nouns (as they represent state). The name should
be camel case, and start with an uppercase letter (e.g. Leader or Boats).
so the top-level .clang-tidy should keep the `VariableName` suggestion
and should apply to new top-level projects.

Request
======
I would like to get feedback on the following two points:

1. Should we proceed with the .clang-tidy change
https://reviews.llvm.org/D108265 ?
2. Should the VariableName wording on CodingStandards.html be
clarified/relaxed? llvm, clang, and clang-tools-extra can keep using
it but lowercase names should also be allowed.

On Wed, Aug 18, 2021 at 4:50 PM Fāng-ruì Sòng via llvm-dev <
llvm-dev at lists.llvm.org> wrote:
> Hi all,
>
> I created a .clang-tidy patch https://reviews.llvm.org/D108265 to push
> the variable naming related options from
> top-level .clang-tidy down to the projects actually using the style
> (llvm/, clang-tools-extra/).
>
> The rationale is: uppercase VariableName doesn't really reflect the
> reality for many projects and (in my view) is a bad
> suggestion for new projects. My summary about the reality:
>
> * Use VariableName and have readability-identifier-naming check: llvm,
> clang-tools-extra, parallel-libs, polly
> * Use VariableName but disable readability-identifier-naming: clang
> * Use lowercase variable names and disable
> readability-identifier-naming: (mostly lowercase) compiler-rt, flang,
> lldb
> * Use lowercase variable names and enable
> readability-identifier-naming: libc, libclc, libcxx, libcxxabi,
> libunwind, lld, mlir, openmp, pstl
>
> (I noticed the issue because some Phabricator bot wanted me to use
> VariableName for a libunwind patch of mine.
> The request looked unreasonable to me. I could add
> libunwind/.clang-tidy, but I figured out the top-level .clang-tidy
> problem.)
>
> From my vague memory about previous discussion on variable names, most
> people agree that VariableName was a mistake and new projects do not
> necessarily need to follow.
>
Might be worth going back over those discussions and summarizing/linking to
supporting archives, etc, so we don't rehash the whole conversation again
here? (& cc'ing interested parties so they don't have to stumble
across
this thread or miss it by accident)

>
> However, one feedback is that
>
>
https://llvm.org/docs/CodingStandards.html#name-types-functions-variables-and-enumerators-properly
> says
>
> > Variable names should be nouns (as they represent state). The name
> should be camel case, and start with an uppercase letter (e.g. Leader or
> Boats).
>
> so the top-level .clang-tidy should keep the `VariableName` suggestion
> and should apply to new top-level projects.
>
> Request
> ======>
> I would like to get feedback on the following two points:
>
> 1. Should we proceed with the .clang-tidy change
> https://reviews.llvm.org/D108265 ?
> 2. Should the VariableName wording on CodingStandards.html be
> clarified/relaxed? llvm, clang, and clang-tools-extra can keep using
> it but lowercase names should also be allowed.
>
Further thought: I don't think (1) should proceed as-is no matter the
decision on (2). We should have an LLVM umbrella naming convention (I don't
think the number of projects that use one or the other should be a vote in
favor or against - LLVM Core is still the core of the umbrella project and
has more weight here than other subprojects (not the only thing that
matters, but part of it), removing any naming convention I think would be
unhelpful.

& I don't think (2) should be "use either of these naming
conventions" it
should be "this is the naming convention" perhaps with a note that
LLVM
Core (& some other places, such as clang-tools-extra) uses a different one
for historical reasons, for instance.
-------------- next part --------------
An HTML attachment was scrubbed...
URL:
<http://lists.llvm.org/pipermail/llvm-dev/attachments/20210819/f081b759/attachment.html>