[clang] [Sema, Lex, Parse] Preprocessor embed in C and C++ (and Obj-C and Obj-C++ by-proxy) (PR #68620)

via cfe-commits cfe-commits at lists.llvm.org
Sat Oct 14 02:28:55 PDT 2023


h-vetinari wrote:

OK, I've got a first rebase going. However, because iterating on a diff with 1000s of lines is extremely hard to review and bound to lead to mistakes and iterations (making it harder still to follow what's going on), I've tried to follow a more structured approach.

Basically, I've pushed tags for intermediate milestones during the rebase to my fork, and am documenting below what I've done. That should make it easier to:
* understand how the diff evolved
* make comparisons between the various different stages (e.g. one can add `/compare/<tag>..<tag>` on Github)
* roll back to intermediate iterations if something goes wrong, without having to start from scratch
* validate what I did (for those so inclined) or branch off in a different direction

I've used a simple tag scheme `embed_vX[.Y]`, where X increases if exisiting commits were changed, and Y increases if changes happened without rebasing. I did some basic compilation tests to ensure I didn't accidentally drop/introduce changes that break the build, but no more than that.

The table is probably busted visually on mobile (sorry...), but I hope it reads alright on a desktop. Where used, equality (`==`) refers to the content of the tree at the commits being compared, not the diffs in those commits.

| tag | description | steps to reproduce | compile-tested[^1] | diff to</br>prev. version | 
|-|-|-|-|-|
| [`embed_v0`](https://github.com/h-vetinari/llvm-project/commits/embed_v0) | the state of the PR<br>as opened originally<br/>(base 08b20d838549e8ff3939c6793b5067b0d9b36337) | check out [this](https://github.com/ThePhD/llvm-project/tree/thephd/embed-speed) | 32c8097f983a65526516353082268f50a017e27d ✅ (presumably)<br/>357bda5d13594bbb47f8a76c935fd6aacbf2f67a ❌ (needs adc973776a526f97b6da2435edf143a10e7e0ad5)<br/>dfd638311acb61bab8113c41ddf138199b360dfa ❌<br/>adc973776a526f97b6da2435edf143a10e7e0ad5 ✅ |
| [`embed_v1`](https://github.com/h-vetinari/llvm-project/commits/embed_v1) | separated superfluous<br/>formatting changes<br/>from first 2 commits<br/>(base 08b20d838549e8ff3939c6793b5067b0d9b36337) | Interactive rebase galore<br/>(the most painful part) | 0b4b6a654edf6fa2781e1c80983c124a0d0088c5 ✅<br/>a05117d39e372e0360655b5da2101f8296bfe5fb ✅ (==v0:[`32c8097`](https://github.com/llvm/llvm-project/commit/32c8097f983a65526516353082268f50a017e27d))<br/>8231b72fbe322bef74bba8177a6bc64754b5cede ❌ (needs 36c1a5f04a61dff60d8326906a42c4bf8fdde2f9)<br/>0a6f48bbb00552b92401476871386f53cac83512 ❌ (==v0:[`357bda5`](https://github.com/llvm/llvm-project/commit/357bda5d13594bbb47f8a76c935fd6aacbf2f67a))<br/>b356860919bd186bf2c340a545e6924dbd9decf0 ❌ (==v0:[`dfd6383`](https://github.com/llvm/llvm-project/commit/dfd638311acb61bab8113c41ddf138199b360dfa))<br/>36c1a5f04a61dff60d8326906a42c4bf8fdde2f9 ✅ (==v0:[`adc9737`](https://github.com/llvm/llvm-project/commit/adc973776a526f97b6da2435edf143a10e7e0ad5)) | [empty](https://github.com/h-vetinari/llvm-project/compare/embed_v0..embed_v1) |
| [`embed_v1.1`](https://github.com/h-vetinari/llvm-project/commits/embed_v1.1) | added [suggestions](https://github.com/llvm/llvm-project/pull/68620#issuecomment-1753515605) from<br/>`git-clang-format`<br/>(base 08b20d838549e8ff3939c6793b5067b0d9b36337) | C&P + `git apply` | v1 + 26c6fcc8d9a85519a4b1d98aa8a0d11fd725855b ✅ | 1 commit;<br/>no rebase | 
| [`embed_v2`](https://github.com/h-vetinari/llvm-project/commits/embed_v2) | undo superfluous</br>formatting changes<br/>(base 08b20d838549e8ff3939c6793b5067b0d9b36337) | `git rebase -i ...`; drop<br/>a05117d39e372e0360655b5da2101f8296bfe5fb & 0a6f48bbb00552b92401476871386f53cac83512, fix<br/>minor conflicts in 26c6fcc8d9a85519a4b1d98aa8a0d11fd725855b | 0b4b6a654edf6fa2781e1c80983c124a0d0088c5 ✅ (same as v1)<br/>07795f280fe6727f76fac5028bd5cbe3bc82a662 ❌ (needs 748c3b54e2e3b00d8c7c1a6813886e3f338ae0d4)<br/>1179116dac7ccc37a38cb8ae2b37a15f994ebdf5 ❌<br/>748c3b54e2e3b00d8c7c1a6813886e3f338ae0d4 ✅ (likely; untested) <br/>b6d053e62a8ea9c88c2bd7ffe0e02e75c7abcc0a ✅ | [uninteresting](https://github.com/h-vetinari/llvm-project/compare/embed_v1.1..embed_v2) | 
| [`embed_v3`](https://github.com/h-vetinari/llvm-project/commits/embed_v3) | squash small commits<br/>(base 08b20d838549e8ff3939c6793b5067b0d9b36337) | `git rebase -i ...`; squash | 0b4b6a654edf6fa2781e1c80983c124a0d0088c5 ✅ (same as v1)<br/>a6f134d30f0007efa339fd434ac109749c30bb11 ✅ | [empty](https://github.com/h-vetinari/llvm-project/compare/embed_v2..embed_v3) | 
| [`embed_v4`](https://github.com/h-vetinari/llvm-project/commits/embed_v4) | rebase (cleanly) on main<br/>(base 7060422265902f11a13f785a1a0ba246eff96114) | `git rebase -i ...` | 7050c932f63f9cb9e94636b287887f8241083117 ❔ (untested) <br/>5956fc215c905fa8520ded8368ceb6b1fab1f996 ❔ (untested) | [uninteresting](https://github.com/h-vetinari/llvm-project/compare/embed_v3..embed_v4) | 

Since the rebase from v3 to v4 went cleanly, I haven't run the compilation tests. I will push v4 now, and hope that this is a good enough jumping off point for further review and iteration.

[^1]: `cmake -G Ninja -DLLVM_ENABLE_PROJECTS="clang" -DLLVM_BUILD_TOOLS=OFF -DLLVM_CCACHE_BUILD=ON ..\llvm`

https://github.com/llvm/llvm-project/pull/68620


More information about the cfe-commits mailing list