[RISCV] Regenerate autogen test to remove spurious diff
[llvm-project.git] / llvm / docs / MyFirstTypoFix.rst
blobb8d34733122c114718d8621026aca95b160d6b69
1 ==============
2 MyFirstTypoFix
3 ==============
5 .. contents::
6    :local:
8 Introduction
9 ============
11 This tutorial will guide you through the process of making a change to
12 LLVM, and contributing it back to the LLVM project. We'll be making a
13 change to Clang, but the steps for other parts of LLVM are the same.
14 Even though the change we'll be making is simple, we're going to cover
15 steps like building LLVM, running the tests, and code review. This is
16 good practice, and you'll be prepared for making larger changes.
18 We'll assume you:
20 -  know how to use an editor,
22 -  have basic C++ knowledge,
24 -  know how to install software on your system,
26 -  are comfortable with the command line,
28 -  have basic knowledge of git.
31 The change we're making
32 -----------------------
34 Clang has a warning for infinite recursion:
36 .. code:: console
38    $ echo "void foo() { foo(); }" > ~/test.cc
39    $ clang -c -Wall ~/test.cc
40    input.cc:1:14: warning: all paths through this function will call
41    itself [-Winfinite-recursion]
43 This is clear enough, but not exactly catchy. Let's improve the wording
44 a little:
46 .. code:: console
48    input.cc:1:14: warning: to understand recursion, you must first
49    understand recursion [-Winfinite-recursion]
52 Dependencies
53 ------------
55 We're going to need some tools:
57 -  git: to check out the LLVM source code,
59 -  a C++ compiler: to compile LLVM source code. You'll want `a recent
60    version <https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library>`__
61    of Clang, GCC, or Visual Studio.
63 -  CMake: used to configure how LLVM should be built on your system,
65 -  ninja: runs the C++ compiler to (re)build specific parts of LLVM,
67 -  python: to run the LLVM tests,
69 As an example, on Ubuntu:
71 .. code:: console
73    $ sudo apt-get install git clang cmake ninja-build python arcanist
76 Building LLVM
77 =============
80 Checkout
81 --------
83 The source code is stored `on
84 Github <https://github.com/llvm/llvm-project>`__ in one large repository
85 ("the monorepo").
87 It may take a while to download!
89 .. code:: console
91    $ git clone https://github.com/llvm/llvm-project.git
93 This will create a directory "llvm-project" with all of the source
94 code. (Checking out anonymously is OK - pushing commits uses a different
95 mechanism, as we'll see later.)
97 Configure your workspace
98 ------------------------
100 Before we can build the code, we must configure exactly how to build it
101 by running CMake. CMake combines information from three sources:
103 -  explicit choices you make (is this a debug build?)
105 -  settings detected from your system (where are libraries installed?)
107 -  project structure (which files are part of 'clang'?)
109 First, create a directory to build in. Usually, this is
110 llvm-project/build.
112 .. code:: console
114    $ mkdir llvm-project/build
115    $ cd llvm-project/build
117 Now, run CMake:
119 .. code:: console
121    $ cmake -G Ninja ../llvm -DCMAKE_BUILD_TYPE=Release -DLLVM_ENABLE_PROJECTS=clang
123 If all goes well, you'll see a lot of "performing test" lines, and
124 finally:
126 .. code:: console
128    Configuring done
129    Generating done
130    Build files have been written to: /path/llvm-project/build
132 And you should see a build.ninja file.
134 Let's break down that last command a little:
136 -  **-G Ninja**: we're going to use ninja to build; please create
137    build.ninja
139 -  **../llvm**: this is the path to the source of the "main" LLVM
140    project
142 -  The two **-D** flags set CMake variables, which override
143    CMake/project defaults:
145     -  **CMAKE_BUILD_TYPE=Release**: build in optimized mode, which is
146        (surprisingly) the fastest option.
148        If you want to run under a debugger, you should use the default Debug
149        (which is totally unoptimized, and will lead to >10x slower test
150        runs) or RelWithDebInfo which is a halfway point.
151        **CMAKE_BUILD_TYPE** affects code generation only, assertions are
152        on by default regardless! **LLVM_ENABLE_ASSERTIONS=Off** disables
153        them.
155     -  **LLVM_ENABLE_PROJECTS=clang**: this lists the LLVM subprojects
156        you are interested in building, in addition to LLVM itself. Multiple
157        projects can be listed, separated by semicolons, such as "clang;
158        lldb".In this example, we'll be making a change to Clang, so we
159        should build it.
161 Finally, create a symlink (or a copy) of
162 llvm-project/build/compile-commands.json into llvm-project/:
164 .. code:: console
166    $ ln -s build/compile_commands.json ../
168 (This isn't strictly necessary for building and testing, but allows
169 tools like clang-tidy, clang-query, and clangd to work in your source
170 tree).
173 Build and test
174 --------------
176 Finally, we can build the code! It's important to do this first, to
177 ensure we're in a good state before making changes. But what to build?
178 In ninja, you specify a **target**. If we just want to build the clang
179 binary, our target name is "clang" and we run:
181 .. code:: console
183    $ ninja clang
185 The first time we build will be very slow - Clang + LLVM is a lot of
186 code. But incremental builds are fast: ninja will only rebuild the parts
187 that have changed. When it finally finishes you should have a working
188 clang binary. Try running:
190 .. code:: console
192    $ bin/clang --version
194 There's also a target for building and running all the clang tests:
196 .. code:: console
198    $ ninja check-clang
200 This is a common pattern in LLVM: check-llvm is all the checks for core,
201 other projects have targets like check-lldb.
204 Making changes
205 ==============
208 Edit
209 ----
211 We need to find the file containing the error message.
213 .. code:: console
215    $ git grep "all paths through this function" ..
216    ../clang/include/clang/Basic/DiagnosticSemaKinds.td:  "all paths through this function will call itself">,
218 The string that appears in DiagnosticSemaKinds.td is the one that is
219 printed by Clang. \*.td files define tables - in this case it's a list
220 of warnings and errors clang can emit and their messages. Let's update
221 the message in your favorite editor:
223 .. code:: console
225    $ vi ../clang/include/clang/Basic/DiagnosticSemaKinds.td
227 Find the message (it should be under
228 ``warn_infinite_recursive_function``). Change the message to "in order to
229 understand recursion, you must first understand recursion".
232 Test again
233 ----------
235 To verify our change, we can build clang and manually check that it
236 works.
238 .. code:: console
240    $ ninja clang
241    $ bin/clang -Wall ~/test.cc
242    /path/test.cc:1:124: warning: in order to understand recursion, you must
243    first understand recursion [-Winfinite-recursion]
245 We should also run the tests to make sure we didn't break something.
247 .. code:: console
249    $ ninja check-clang
251 Notice that it is much faster to build this time, but the tests take
252 just as long to run. Ninja doesn't know which tests might be affected,
253 so it runs them all.
255 .. code:: console
257    ********************
258    Testing Time: 408.84s
259    ********************
260    Failing Tests (1):
261        Clang :: SemaCXX/warn-infinite-recursion.cpp
263 Well, that makes senseā€¦ and the test output suggests it's looking for
264 the old string "call itself" and finding our new message instead.
265 Note that more tests may fail in a similar way as new tests are
266 added time to time.
268 Let's fix it by updating the expectation in the test.
270 .. code:: console
272    $ vi ../clang/test/SemaCXX/warn-infinite-recursion.cpp
274 Everywhere we see `// expected-warning{{call itself}}` (or something similar
275 from the original warning text), let's replace it with
276 `// expected-warning{{to understand recursion}}`.
278 Now we could run **all** the tests again, but this is a slow way to
279 iterate on a change! Instead, let's find a way to re-run just the
280 specific test. There are two main types of tests in LLVM:
282 -  **lit tests** (e.g. SemaCXX/warn-infinite-recursion.cpp).
284 These are fancy shell scripts that run command-line tools and verify the
285 output. They live in files like
286 clang/**test**/FixIt/dereference-addressof.c. Re-run like this:
288 .. code:: console
290    $ bin/llvm-lit -v ../clang/test/SemaCXX/warn-infinite-recursion.cpp
292 -  **unit tests** (e.g. ToolingTests/ReplacementTest.CanDeleteAllText)
294 These are C++ programs that call LLVM functions and verify the results.
295 They live in suites like ToolingTests. Re-run like this:
297 .. code:: console
299    $ ninja ToolingTests && tools/clang/unittests/Tooling/ToolingTests
300    --gtest_filter=ReplacementTest.CanDeleteAllText
303 Commit locally
304 --------------
306 We'll save the change to a local git branch. This lets us work on other
307 things while the change is being reviewed. Changes should have a
308 description, to explain to reviewers and future readers of the code why
309 the change was made.
311 .. code:: console
313    $ git checkout -b myfirstpatch
314    $ git commit -am "[Diagnostic] Clarify -Winfinite-recursion message"
316 Now we're ready to send this change out into the world! By the way,
317 There is an unwritten convention of using tag for your commit. Tags
318 usually represent modules that you intend to modify. If you don't know
319 the tags for your modules, you can look at the commit history :
320 https://github.com/llvm/llvm-project/commits/main.
323 Code review
324 ===========
327 Finding a reviewer
328 ------------------
330 Changes can be reviewed by anyone in the LLVM community who has commit
331 access.For larger and more complicated changes, it's important that the
332 reviewer has experience with the area of LLVM and knows the design goals
333 well. The author of a change will often assign a specific reviewer (git
334 blame and git log can be useful to find one).
336 As our change is fairly simple, we'll add the cfe-commits mailing list
337 as a subscriber; anyone who works on clang can likely pick up the
338 review. (For changes outside clang, llvm-commits is the usual list. See
339 `http://lists.llvm.org/ <http://lists.llvm.org/mailman/listinfo>`__ for
340 all the \*-commits mailing lists).
343 Uploading a change for review
344 -----------------------------
346 LLVM code reviews happen through pull-request on GitHub, see
347 :ref:`GitHub <github-reviews>` documentation for how to open
348 a pull-request on GitHub.
350 Review process
351 --------------
353 When you open a pull-request, some automation will add a comment and
354 notify different member of the projects depending on the component you
355 changed.
356 Within a few days, someone should start the review. They may add
357 themselves as a reviewer, or simply start leaving comments. You'll get
358 another email any time the review is updated. The details are in the
359 `https://llvm.org/docs/CodeReview/ <https://llvm.org/docs/CodeReview.html>`__.
361 Comments
362 ~~~~~~~~
364 The reviewer can leave comments on the change, and you can reply. Some
365 comments are attached to specific lines, and appear interleaved with the
366 code. You can either reply to these, or address them and mark them as
367 "done". Note that in-line replies are **not** sent straight away! They
368 become "draft" comments and you must click "Submit" at the bottom of the
369 page.
372 Updating your change
373 ~~~~~~~~~~~~~~~~~~~~
375 If you make changes in response to a reviewer's comments, simply update
376 your branch with more commits and push to your fork. It may be a good
377 idea to answer the comments from the reviewer explicitly.
379 Accepting a revision
380 ~~~~~~~~~~~~~~~~~~~~
382 When the reviewer is happy with the change, they will **Accept** the
383 revision. They may leave some more minor comments that you should
384 address, but at this point the review is complete. It's time to get it
385 merged!
388 Commit by proxy
389 ---------------
391 As this is your first change, you won't have access to merge it
392 yourself yet. The reviewer **doesn't know this**, so you need to tell
393 them! Leave a message on the review like:
395    Thanks @somellvmdev. I don't have commit access, can you land this
396    patch for me?
398 The pull-request will be closed and you will be notified by GitHub.
400 Review expectations
401 -------------------
403 In order to make LLVM a long-term sustainable effort, code needs to be
404 maintainable and well tested. Code reviews help to achieve that goal.
405 Especially for new contributors, that often means many rounds of reviews
406 and push-back on design decisions that do not fit well within the
407 overall architecture of the project.
409 For your first patches, this means:
411 -  be kind, and expect reviewers to be kind in return - LLVM has a `Code
412    of Conduct <https://llvm.org/docs/CodeOfConduct.html>`__;
414 -  be patient - understanding how a new feature fits into the
415    architecture of the project is often a time consuming effort, and
416    people have to juggle this with other responsibilities in their
417    lives; **ping the review once a week** when there is no response;
419 -  if you can't agree, generally the best way is to do what the reviewer
420    asks; we optimize for readability of the code, which the reviewer is
421    in a better position to judge; if this feels like it's not the right
422    option, you can contact the cfe-dev mailing list to get more feedback
423    on the direction;
426 Commit access
427 =============
429 Once you've contributed a handful of patches to LLVM, start to think
430 about getting commit access yourself. It's probably a good idea if:
432 -  you've landed 3-5 patches of larger scope than "fix a typo"
434 -  you'd be willing to review changes that are closely related to yours
436 -  you'd like to keep contributing to LLVM.
439 Getting commit access
440 ---------------------
442 LLVM uses Git for committing changes. The details are in the `developer
443 policy
444 document <https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access>`__.
447 With great power
448 ----------------
450 Actually, this would be a great time to read the rest of the `developer
451 policy <https://llvm.org/docs/DeveloperPolicy.html>`__, too. At minimum,
452 you need to be subscribed to the relevant commits list before landing
453 changes (e.g. llvm-commits@lists.llvm.org), as discussion often happens
454 there if a new patch causes problems.
457 Post-commit errors
458 ------------------
460 Once your change is submitted it will be picked up by automated build
461 bots that will build and test your patch in a variety of configurations.
463 You can see all configurations and their current state in a waterfall
464 view at http://lab.llvm.org/buildbot/#/waterfall. The waterfall view is good
465 to get a general overview over the tested configurations and to see
466 which configuration have been broken for a while.
468 The console view at http://lab.llvm.org/buildbot/#/console helps to get a
469 better understanding of the build results of a specific patch. If you
470 want to follow along how your change is affecting the build bots, **this
471 should be the first place to look at** - the colored bubbles correspond
472 to projects in the waterfall.
474 If you see a broken build, do not despair - some build bots are
475 continuously broken; if your change broke the build, you will see a red
476 bubble in the console view, while an already broken build will show an
477 orange bubble. Of course, even when the build was already broken, a new
478 change might introduce a hidden new failure.
480 | When you want to see more details how a specific build is broken,
481   click the red bubble.
482 | If post-commit error logs confuse you, do not worry too much -
483   everybody on the project is aware that this is a bit unwieldy, so
484   expect people to jump in and help you understand what's going on!
486 buildbots, overview of bots, getting error logs.
489 Reverts
490 -------
492 If in doubt, revert immediately, and re-land later after investigation
493 and fix.
496 Conclusion
497 ==========
499 llvm is a land of contrasts.