Upgrade to Pro — share decks privately, control downloads, hide ads and more …

[COSCUP 2021] A trip about how I contribute to LLVM

[COSCUP 2021] A trip about how I contribute to LLVM

https://coscup.org/2021/zh-TW/session/YBFMNB

1. Motivation
CppNameLint and Clang-Tidy

2. Beginning of a trip
Phabricator, Arcanist, Build, Test, and their flows

3. Tips in a trip
Get out of trouble

4. Trip moments
Happened during the code review

5. The last

48a351a06897d7cc9721e2ddb7198e8a?s=128

dougpuob

July 31, 2021
Tweet

Transcript

  1. Douglas Chen [陳鍵源] <dougpuob@gmail.com> A trip about how I contribute

    to LLVM COSCUP 2021 Taiwan 2021/July/31
  2. 2 1. Motivation CppNameLint and Clang-Tidy 2. Beginning of a

    trip Phabricator, Arcanist, Build, Test, and their flows 3. Tips in a trip Get out of trouble 4. Trip moments Happened during the code review 5. The last Agenda
  3. Agenda 1. Motivation

  4. 4 CppNameLint Windows Linux macOS https://github.com/dougpuob/cppnamelint 1. UpperCamel, 2. lowerCamel,

    3. UPPER_SNAKE_CASE, 4. lower_snake_case, 5. szHungarianNotion
  5. 5 Implement into Clang-Tidy Clang-Tidy CppNameLint readability-identifier-naming 1. Libtooling (Clang

    based library) 2. Plenty checks (abseil-*, boost-*, cert-*, google-*, llvm-*, modernize-*, performance-*, readability-, …) 3. Auto fix
  6. Agenda 2. Beginning of a trip

  7. 7 1. Phabricator (phab) 2. Arcanist (arc) 3. Build 4.

    Test
  8. 8 Phabricator https://reviews.llvm.org/ arc web-based system cmd utility (Arcanist)

  9. 9 Phabricator LLVM Discussion Forums (https://llvm.discourse.group/) All recent commits Chat

    Invite Link (https://discord.gg/xS7Z362) GitHub Repository (https://github.com/llvm/llvm-project) BuildBots (https://lab.llvm.org/buildbot) Review works you participate to
  10. 10 Phabricator (differential page) 1⃣ 3⃣ 2⃣ 4⃣ 5⃣

  11. 11 Phabricator flow (a life of a differential) Create a

    differential and upload to Phab Make a change Discuss it Request changes Ready to land Land it Close LGTM 1⃣ 3⃣ 2⃣ 4⃣ 5⃣ 7⃣ 8⃣ 9⃣ 6⃣ Approved
  12. 12 Arcanist (command line interface for Phabricator) • arc patch

    (apply changes in a revision to the working copy) • arc lint (check your code for syntax and style errors) • arc diff (send your code to Differential for review) • arc land (push Git and Mercurial changes) • arc help (shows this help) • ... arc
  13. Local Git repo 13 Arcanist flow Remote Git repo Central

    repo with Phab Code review on Phab arc land arc diff arc patch git add git commit arc lint Unaccepted arc Accepted Ready to land 1⃣ 3⃣ 2⃣ 4⃣ 5⃣ 7⃣ LGTM 6⃣
  14. 14 Build (command with BuildKite) cmake ../llvm \ -G Ninja

    \ -D LLVM_ENABLE_PROJECTS="clang;clang-tools-extra;libc;llvm" \ -D CMAKE_BUILD_TYPE=Release \ -D LLVM_ENABLE_ASSERTIONS=ON \ -D LLVM_BUILD_EXAMPLES=ON \ -D LLVM_LIT_ARGS="-v --xunit-xml-output test-results.xml" \ -D LLVM_ENABLE_LLD=ON \ -D CMAKE_CXX_FLAGS=-gmlt https://buildkite.com/llvm-project/premerge-checks/builds/39326#578995ee-9588-4901-89eb-8deedf3caea0
  15. 15 Test 1. Unit Test (in llvm/unittests with Google Test)

    2. Regression Test (in llvm/test with llvm-lit - LLVM Integrated Tester) 3. Whole programs (test-suite, https://github.com/llvm/llvm-test-suite.git) • ninja check-clang • ninja check-llvm • ninja check-all Three categories Run via build system
  16. 16 Test (manual trigger unit test) $ build-rel/tools/clang/tools/extra/unittests/clang-tidy/ClangTidyTests Run it

    Result
  17. 17 Test (manual trigger regression test) $ llvm-lit readability-identifier-naming-hungarian-notation.cpp Result

    Run it
  18. 18 Test (buildbots & buildkite) https://buildkite.com/llvm-project/premerge-checks/builds/39 326#578995ee-9588-4901-89eb-8deedf3caea0 https://lab.llvm.org/buildbot/#/waterfall

  19. Agenda 3. Tips in the trip

  20. # Based on main branch to create diff then send

    it for review arc diff main --update D86671 # Based on the 99304d commit to create diff then send it for review arc diff 99304d --update D86671 # Based on the 99304d commit to create diff but don’t send it out arc diff 99304d --preview D86671 20 TIPS #1 Choose the right base
  21. 21 TIPS #2 Generate documents -DLLVM_ENABLE_SPHINX=ON • ninja docs-clang-html •

    ninja docs-clang-tools-html • ninja docs-llvm-html • ninja docs-dsymutil-man • ninja docs-clang-man • ninja docs-clang-tools-man • ninja docs-llvm-dwarfdump-man • ninja docs-llvm-man Sphinx is a tool that makes it easy to create intelligent and beautiful documentation, written by Georg Brandl and licensed under the BSD license. It was originally created for the Python documentation, and it has excellent facilities for the documentation of software projects in a range of languages. Of course, this site is also created from reStructuredText sources using Sphinx! CMake option Build via build system
  22. 22 TIPS #3 You need an more powerful computer Unit

    is mins https:/ /dougpuob.github.io/posts/20210530-build-speed-comparison-btw-lxc-qemu-and-baremetal-by-llvm/
  23. 23 TIPS #4 Linker is a memory monster Add -DLLVM_USE_LINKER=lld

    debug build CMake option
  24. Agenda 4. Trip moments

  25. 25 Moment #1 int iVal = 0; size_t nVal =

    0; uint8_t u8Val = 0; uint16_t u16Val = 0; uint32_t u32Val = 0; BOOL bEnabled = TRUE; ULONG ulVal = 0; WORD wVal = 0; DWORD dwVal = 0; int *piVal = NULL; char *szStr = “Str”; char szText[] = “Text”; Is there a standard for Hungarain notation ? int iVal = 0; int nVal = 0; BOOL* bEnabled = TRUE; BYTE* bValue = 0xA5; char* szText = “Text”; char* strText = “Text”; Built-in types Microsoft defined Composite types ❓ ❓ ❓
  26. 26 Moment #2 Original idea was changed, but better

  27. 27 Moment #3 Invalid identifier naming option $ clang-tidy ./main.cpp

    ... 1 warning generated. warning: invalid identifier naming option 'MacroDefinitionHungarianPrefix' [clang-tidy-config] Checks: '-*,readability-identifier-naming' CheckOptions: - { key: readability-identifier-naming.MacroDefinitionHungarianPrefix, value: On } - { key: readability-identifier-naming.AbstractClassHungarianPrefix , value: On } - { key: readability-identifier-naming.ClassHungarianPrefix , value: On } .clang-tidy console output
  28. 28 Moment #4 [Hungarian.WordList] "uint32_t" = "u32" "int" = "i"

    "unsigned int" = "ui" "DWORD" = "dw" "BOOL" = "b" limited configuration readability-identifier-naming.HungarianNotation.PrimitiveType.uint32_t , value: u32 readability-identifier-naming.HungarianNotation.PrimitiveType.int , value: i readability-identifier-naming.HungarianNotation.PrimitiveType.unsigned-int , value: i readability-identifier-naming.HungarianNotation.PrimitiveType.DWORD , value: dw readability-identifier-naming.HungarianNotation.PrimitiveType.BOOL , value: b cppnamelint.toml .clang-tidy
  29. 29 Moment #5 How ‘--fix’ works // readability-identifier-naming-hungarian-notation-cfgfile.cpp int LocalVariableCase

    = 0; warning: invalid case style for local variable 'LocalVariableCase' [readability-identifier-naming] int LocalVariableCase = 0; ^~~~~~~~~~~~~~~~~ iLocalVariableCase input source file console output clang-tidy source
  30. 30 Moment #6 How Regression Test works FileCheck check result

    matching pattern input from stdin or argv pattern regression test source
  31. Agenda 5. The last

  32. 32 If you was stuck ... 1. Read related documents

    on https://llvm.org/docs/ . 2. Search similar projects on GitHub. 3. Official community forum (https://llvm.discourse.group/) 4. Discuss with your reviewers.
  33. 33 Reference (hightly recommend) • https://llvm.org/docs/CodeReview.html • https://llvm.org/docs/CodingStandards.html • https://llvm.org/docs/DeveloperPolicy.html

    • https://llvm.org/docs/Phabricator.html • How to Contribute to LLVM (Bieneman & K. Barton)
  34. Agenda END