[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

Sam McCall via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Mar 9 05:09:02 PST 2022


sammccall added a comment.

In D121233#3369657 <https://reviews.llvm.org/D121233#3369657>, @aaron.ballman wrote:

> Thank you for working on this! I have a few thoughts on the renaming, but otherwise strongly support the direction here.
>
>> clang/lib/Tooling/Syntax/Pseudo/*           => clang-tools-extra/pseudo/lib/*
>
> The usual naming conventions in clang-tools-extra is to use the tool name as the folder it goes in. Based on that, should the folders be `clang-tools-extra/clang-pseudo/` instead of `clang-tools-extra/pseudo/`?

I don't think it's a good naming convention, and am not convinced consistency here is important enough to propagate a bad idea further.
We're in a directory called "clang-tools-extra", so the clang- prefix is redundant in the path (not in the binary name).
It seems like a small thing, but it adds up: the `clang-tools-extra` directory is ugly, so are `lib` and `include/clang-pseudo`, so is having both `llvm` and `llvm-project` because of the monorepo.
We have a file named `llvm/llvm-project/llvm/include/llvm/Support/InitLLVM.h`! People working with this code type these paths often.

(If it helps, I'd be happy to prepare changes to drop the `clang-` prefixes from any of the directories where they seem obviously redundant - everything except `clang-doc` and `clangd`. But I think the much larger burden of establishing consensus needs to fall on people who think that consistency is worth enforcing here).

>> clang/include/clang/Tooling/Syntax/Pseudo/* => clang-tools-extra/pseudo/include/clang-pseudo/*
>> clang/tools/clang/pseudo/*                  => clang-tools-extra/pseudo/tool/*
>> clang/test/Syntax/*                         => clang-tools-extra/pseudo/test/*
>
> The convention are for clang-tools-extra tests to live in `clang-tools-extra/test/<tool>` (clangd is using the style you propose here when it probably should have followed the existing conventions).

We used this layout for several years, and it was painful to navigate and maintain, so it was deliberately changed in b804eef09052cf40e79aa2ed8a23f2f39e2dda1b at which point all the pain went away.

Again, I'd be willing to work on improving this, but I don't want to spend weeks arguing with people about it. My experience is that these conversations are exhausting, negative because people are grumpy about dealing with churn, and in the end there's nobody empowered to say "OK, this is a good idea". See also the idea of using a separate bugtracker, website infra etc for clangd, where all the feedback was extremely negative, it was very difficult to decide to proceed anyway, and these turned out to be large improvements and none of the warnings came true.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121233



More information about the cfe-commits mailing list