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

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Thanks, the movement looks roughly good.

We should be aware of that (when landing this patch):

- the upstream llvm gn build files need to be adjust (IIUC, there is no expectation from us to update the gn files)
- we'd better prepare a patch for the internal integration (make the integration life easier)



================
Comment at: clang-tools-extra/pseudo/CMakeLists.txt:1
+include_directories(include ${CMAKE_CURRENT_BINARY_DIR}/include)
+add_subdirectory(lib)
----------------
Why not use `CMAKE_CURRENT_SOURCE_DIR`? It seems a more natural fit.


================
Comment at: clang-tools-extra/pseudo/include/clang-pseudo/DirectiveMap.h:31
 
+#include "clang-pseudo/Token.h"
 #include "clang/Basic/TokenKinds.h"
----------------
`clang-pseudo` is somewhat nice, it is probably ok, but 
- using hyphen in the include path is not a canonical pattern in LLVM code-style;
- we have a command-line tool which calls `clang-pseudo`, clang-pseudo makes me think this is a header from the command-line tool rather from the *library*.

there are other options, but they don't seem significantly better than the current one :(

- `clang/Pseudo/Token.h`: this matches the canonical pattern of using headers from clang libraries, but it causes confusion (which we want to avoid in this patch);
- `clangPseudo/Token.h`: this feels slightly better, as we build a library called `clangPseudo`;


I know we want to explicitly express that this is intended to be a library, another aggressive option (which mostly follows the existing pattern in clang-tools-extra) would be

```
clang-tools-extra/clangPseudo/Token.{h, cpp}
clang-tools-extra/clangPseudo/internal/Forest.h
clang-tools-extra/clangPseudo/tool/
```

internal private headers are put into the sub `internal` directory, headers in non `internal` directories are treat as public headers implicitly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121233