1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
185
186
187
188
189
190
191
192
193
194
195
196
197
198
199
200
201
202
203
204
205
206
207
208
209
210
211
212
213
214
215
216
217
218
219
220
221
222
223
224
225
226
227
228
229
230
231
232
233
234
235
236
237
238
239
240
241
242
243
244
245
246
247
248
249
250
251
252
253
254
255
256
257
258
259
260
261
262
263
264
265
266
267
268
269
270
271
272
273
274
275
276
277
278
279
280
281
282
283
284
285
286
287
288
289
290
291
292
293
294
295
296
297
298
299
300
301
302
303
304
305
306
307
308
309
310
311
312
313
314
315
316
317
318
319
320
321
322
323
324
325
326
327
328
329
330
331
332
333
334
335
336
337
338
339
340
341
342
343
344
345
346
347
348
349
350
351
352
353
354
355
356
357
358
359
360
361
362
363
364
365
366
367
368
369
370
371
372
373
374
375
376
377
378
379
380
381
382
383
384
385
386
387
388
389
390
391
392
393
394
395
396
397
398
399
400
401
402
403
404
405
406
407
408
409
410
411
412
413
414
415
416
417
418
419
420
421
422
423
424
425
426
427
428
429
430
431
432
433
434
435
436
437
438
439
440
441
442
443
444
445
446
447
448
449
450
451
452
453
454
455
456
457
458
459
460
461
462
463
464
465
466
467
468
469
470
471
472
473
474
475
476
477
478
479
480
481
482
483
484
485
486
487
488
489
490
491
492
493
494
495
496
497
498
499
500
501
502
503
504
505
506
507
508
509
510
511
512
513
514
515
516
517
518
519
520
521
522
523
524
525
526
527
528
529
530
531
532
533
534
535
536
537
538
539
540
541
542
543
544
545
546
547
548
549
550
551
552
553
554
555
556
557
558
559
560
561
562
563
564
565
566
567
568
569
570
571
572
573
574
575
576
577
578
579
580
581
582
583
584
585
586
587
588
589
590
591
592
593
594
595
596
597
598
599
600
601
602
603
604
605
606
607
608
609
610
611
612
613
614
615
616
617
618
619
620
621
622
623
624
625
|
<!DOCTYPE html>
<html>
<head>
<meta charset="utf-8" />
<meta name="viewport" content="width=device-width, initial-scale=1.0" />
<title>MyFirstTypoFix — LLVM 13 documentation</title>
<link rel="stylesheet" href="_static/pygments.css" type="text/css" />
<link rel="stylesheet" href="_static/llvm-theme.css" type="text/css" />
<script id="documentation_options" data-url_root="./" src="_static/documentation_options.js"></script>
<script src="_static/jquery.js"></script>
<script src="_static/underscore.js"></script>
<script src="_static/doctools.js"></script>
<link rel="index" title="Index" href="genindex.html" />
<link rel="search" title="Search" href="search.html" />
<link rel="next" title="Reference" href="Reference.html" />
<link rel="prev" title="4. Building a JIT: Extreme Laziness - Using LazyReexports to JIT from ASTs" href="tutorial/BuildingAJIT4.html" />
<style type="text/css">
table.right { float: right; margin-left: 20px; }
table.right td { border: 1px solid #ccc; }
</style>
</head><body>
<div class="logo">
<a href="index.html">
<img src="_static/logo.png"
alt="LLVM Logo" width="250" height="88"/></a>
</div>
<div class="related" role="navigation" aria-label="related navigation">
<h3>Navigation</h3>
<ul>
<li class="right" style="margin-right: 10px">
<a href="genindex.html" title="General Index"
accesskey="I">index</a></li>
<li class="right" >
<a href="Reference.html" title="Reference"
accesskey="N">next</a> |</li>
<li class="right" >
<a href="tutorial/BuildingAJIT4.html" title="4. Building a JIT: Extreme Laziness - Using LazyReexports to JIT from ASTs"
accesskey="P">previous</a> |</li>
<li><a href="https://llvm.org/">LLVM Home</a> | </li>
<li><a href="index.html">Documentation</a>»</li>
<li class="nav-item nav-item-1"><a href="GettingStartedTutorials.html" accesskey="U">Getting Started/Tutorials</a> »</li>
<li class="nav-item nav-item-this"><a href="">MyFirstTypoFix</a></li>
</ul>
</div>
<div class="sphinxsidebar" role="navigation" aria-label="main navigation">
<div class="sphinxsidebarwrapper">
<h3>Documentation</h3>
<ul class="want-points">
<li><a href="https://llvm.org/docs/GettingStartedTutorials.html">Getting Started/Tutorials</a></li>
<li><a href="https://llvm.org/docs/UserGuides.html">User Guides</a></li>
<li><a href="https://llvm.org/docs/Reference.html">Reference</a></li>
</ul>
<h3>Getting Involved</h3>
<ul class="want-points">
<li><a href="https://llvm.org/docs/Contributing.html">Contributing to LLVM</a></li>
<li><a href="https://llvm.org/docs/HowToSubmitABug.html">Submitting Bug Reports</a></li>
<li><a href="https://llvm.org/docs/GettingInvolved.html#mailing-lists">Mailing Lists</a></li>
<li><a href="https://llvm.org/docs/GettingInvolved.html#irc">IRC</a></li>
<li><a href="https://llvm.org/docs/GettingInvolved.html#meetups-and-social-events">Meetups and Social Events</a></li>
</ul>
<h3>Additional Links</h3>
<ul class="want-points">
<li><a href="https://llvm.org/docs/FAQ.html">FAQ</a></li>
<li><a href="https://llvm.org/docs/Lexicon.html">Glossary</a></li>
<li><a href="https://llvm.org/pubs">Publications</a></li>
<li><a href="https://github.com/llvm/llvm-project//">Github Repository</a></li>
</ul>
<div role="note" aria-label="source link">
<h3>This Page</h3>
<ul class="this-page-menu">
<li><a href="_sources/MyFirstTypoFix.rst.txt"
rel="nofollow">Show Source</a></li>
</ul>
</div>
<div id="searchbox" style="display: none" role="search">
<h3 id="searchlabel">Quick search</h3>
<div class="searchformwrapper">
<form class="search" action="search.html" method="get">
<input type="text" name="q" aria-labelledby="searchlabel" />
<input type="submit" value="Go" />
</form>
</div>
</div>
<script>$('#searchbox').show(0);</script>
</div>
</div>
<div class="document">
<div class="documentwrapper">
<div class="bodywrapper">
<div class="body" role="main">
<div class="section" id="myfirsttypofix">
<h1>MyFirstTypoFix<a class="headerlink" href="#myfirsttypofix" title="Permalink to this headline">¶</a></h1>
<div class="contents local topic" id="contents">
<ul class="simple">
<li><p><a class="reference internal" href="#introduction" id="id1">Introduction</a></p>
<ul>
<li><p><a class="reference internal" href="#the-change-we-re-making" id="id2">The change we’re making</a></p></li>
<li><p><a class="reference internal" href="#dependencies" id="id3">Dependencies</a></p></li>
</ul>
</li>
<li><p><a class="reference internal" href="#building-llvm" id="id4">Building LLVM</a></p>
<ul>
<li><p><a class="reference internal" href="#checkout" id="id5">Checkout</a></p></li>
<li><p><a class="reference internal" href="#configure-your-workspace" id="id6">Configure your workspace</a></p></li>
<li><p><a class="reference internal" href="#build-and-test" id="id7">Build and test</a></p></li>
</ul>
</li>
<li><p><a class="reference internal" href="#making-changes" id="id8">Making changes</a></p>
<ul>
<li><p><a class="reference internal" href="#edit" id="id9">Edit</a></p></li>
<li><p><a class="reference internal" href="#test-again" id="id10">Test again</a></p></li>
<li><p><a class="reference internal" href="#commit-locally" id="id11">Commit locally</a></p></li>
</ul>
</li>
<li><p><a class="reference internal" href="#code-review" id="id12">Code review</a></p>
<ul>
<li><p><a class="reference internal" href="#finding-a-reviewer" id="id13">Finding a reviewer</a></p></li>
<li><p><a class="reference internal" href="#uploading-a-change-for-review" id="id14">Uploading a change for review</a></p></li>
<li><p><a class="reference internal" href="#review-process" id="id15">Review process</a></p>
<ul>
<li><p><a class="reference internal" href="#comments" id="id16">Comments</a></p></li>
<li><p><a class="reference internal" href="#updating-your-change" id="id17">Updating your change</a></p></li>
<li><p><a class="reference internal" href="#accepting-a-revision" id="id18">Accepting a revision</a></p></li>
</ul>
</li>
<li><p><a class="reference internal" href="#commit-by-proxy" id="id19">Commit by proxy</a></p></li>
<li><p><a class="reference internal" href="#review-expectations" id="id20">Review expectations</a></p></li>
</ul>
</li>
<li><p><a class="reference internal" href="#commit-access" id="id21">Commit access</a></p>
<ul>
<li><p><a class="reference internal" href="#getting-commit-access" id="id22">Getting commit access</a></p></li>
<li><p><a class="reference internal" href="#with-great-power" id="id23">With great power</a></p></li>
<li><p><a class="reference internal" href="#commit" id="id24">Commit</a></p></li>
<li><p><a class="reference internal" href="#post-commit-errors" id="id25">Post-commit errors</a></p></li>
<li><p><a class="reference internal" href="#reverts" id="id26">Reverts</a></p></li>
</ul>
</li>
<li><p><a class="reference internal" href="#conclusion" id="id27">Conclusion</a></p></li>
</ul>
</div>
<div class="section" id="introduction">
<h2><a class="toc-backref" href="#id1">Introduction</a><a class="headerlink" href="#introduction" title="Permalink to this headline">¶</a></h2>
<p>This tutorial will guide you through the process of making a change to
LLVM, and contributing it back to the LLVM project. We’ll be making a
change to Clang, but the steps for other parts of LLVM are the same.
Even though the change we’ll be making is simple, we’re going to cover
steps like building LLVM, running the tests, and code review. This is
good practice, and you’ll be prepared for making larger changes.</p>
<p>We’ll assume you:</p>
<ul class="simple">
<li><p>know how to use an editor,</p></li>
<li><p>have basic C++ knowledge,</p></li>
<li><p>know how to install software on your system,</p></li>
<li><p>are comfortable with the command line,</p></li>
<li><p>have basic knowledge of git.</p></li>
</ul>
<div class="section" id="the-change-we-re-making">
<h3><a class="toc-backref" href="#id2">The change we’re making</a><a class="headerlink" href="#the-change-we-re-making" title="Permalink to this headline">¶</a></h3>
<p>Clang has a warning for infinite recursion:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> <span class="nb">echo</span> <span class="s2">"void foo() { foo(); }"</span> > ~/test.cc
<span class="gp">$</span> clang -c -Wall ~/test.cc
<span class="go">input.cc:1:14: warning: all paths through this function will call</span>
<span class="go">itself [-Winfinite-recursion]</span>
</pre></div>
</div>
<p>This is clear enough, but not exactly catchy. Let’s improve the wording
a little:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="go">input.cc:1:14: warning: to understand recursion, you must first</span>
<span class="go">understand recursion [-Winfinite-recursion]</span>
</pre></div>
</div>
</div>
<div class="section" id="dependencies">
<h3><a class="toc-backref" href="#id3">Dependencies</a><a class="headerlink" href="#dependencies" title="Permalink to this headline">¶</a></h3>
<p>We’re going to need some tools:</p>
<ul class="simple">
<li><p>git: to check out the LLVM source code,</p></li>
<li><p>a C++ compiler: to compile LLVM source code. You’ll want <a class="reference external" href="https://llvm.org/docs/GettingStarted.html#host-c-toolchain-both-compiler-and-standard-library">a recent
version</a>
of Clang, GCC, or Visual Studio.</p></li>
<li><p>CMake: used to configure how LLVM should be built on your system,</p></li>
<li><p>ninja: runs the C++ compiler to (re)build specific parts of LLVM,</p></li>
<li><p>python: to run the LLVM tests,</p></li>
<li><p>arcanist: for uploading changes for review,</p></li>
</ul>
<p>As an example, on Ubuntu:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> sudo apt-get install git clang cmake ninja-build python arcanist subversion
</pre></div>
</div>
</div>
</div>
<div class="section" id="building-llvm">
<h2><a class="toc-backref" href="#id4">Building LLVM</a><a class="headerlink" href="#building-llvm" title="Permalink to this headline">¶</a></h2>
<div class="section" id="checkout">
<h3><a class="toc-backref" href="#id5">Checkout</a><a class="headerlink" href="#checkout" title="Permalink to this headline">¶</a></h3>
<p>The source code is stored <a class="reference external" href="https://github.com/llvm/llvm-project">on
Github</a> in one large repository
(“the monorepo”).</p>
<p>It may take a while to download!</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> git clone https://github.com/llvm/llvm-project.git
</pre></div>
</div>
<p>This will create a directory “llvm-project” with all of the source
code.(Checking out anonymously is OK - pushing commits uses a different
mechanism, as we’ll see later)</p>
</div>
<div class="section" id="configure-your-workspace">
<h3><a class="toc-backref" href="#id6">Configure your workspace</a><a class="headerlink" href="#configure-your-workspace" title="Permalink to this headline">¶</a></h3>
<p>Before we can build the code, we must configure exactly how to build it
by running CMake. CMake combines information from three sources:</p>
<ul class="simple">
<li><p>explicit choices you make (is this a debug build?)</p></li>
<li><p>settings detected from your system (where are libraries installed?)</p></li>
<li><p>project structure (which files are part of ‘clang’?)</p></li>
</ul>
<p>First, create a directory to build in. Usually, this is
llvm-project/build.</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> mkdir llvm-project/build
<span class="gp">$</span> <span class="nb">cd</span> llvm-project/build
</pre></div>
</div>
<p>Now, run CMake:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> cmake -G Ninja ../llvm -DCMAKE_BUILD_TYPE<span class="o">=</span>Release -DLLVM_ENABLE_PROJECTS<span class="o">=</span>clang
</pre></div>
</div>
<p>If all goes well, you’ll see a lot of “performing test” lines, and
finally:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="go">Configuring done</span>
<span class="go">Generating done</span>
<span class="go">Build files have been written to: /path/llvm-project/build</span>
</pre></div>
</div>
<p>And you should see a build.ninja file.</p>
<p>Let’s break down that last command a little:</p>
<ul>
<li><p><strong>-G Ninja</strong>: we’re going to use ninja to build; please create
build.ninja</p></li>
<li><p><strong>../llvm</strong>: this is the path to the source of the “main” LLVM
project</p></li>
<li><p>The two <strong>-D</strong> flags set CMake variables, which override
CMake/project defaults:</p></li>
<li><p><strong>CMAKEBUILDTYPE=Release</strong>: build in optimized mode, which is
(surprisingly) the fastest option.</p>
<p>If you want to run under a debugger, you should use the default Debug
(which is totally unoptimized, and will lead to >10x slower test
runs) or RelWithDebInfo which is a halfway point.
<strong>CMAKEBUILDTYPE</strong> affects code generation only, assertions are
on by default regardless! <strong>LLVMENABLEASSERTIONS=Off</strong> disables
them.</p>
</li>
<li><p><strong>LLVMENABLEPROJECTS=clang</strong> : this lists the LLVM subprojects
you are interested in building, in addition to LLVM itself. Multiple
projects can be listed, separated by semicolons, such as “clang;
lldb”.In this example, we’ll be making a change to Clang, so we
should build it.</p></li>
</ul>
<p>Finally, create a symlink (or a copy) of
llvm-project/build/compile-commands.json into llvm-project/:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> ln -s build/compile_commands.json ../
</pre></div>
</div>
<p>(This isn’t strictly necessary for building and testing, but allows
tools like clang-tidy, clang-query, and clangd to work in your source
tree).</p>
</div>
<div class="section" id="build-and-test">
<h3><a class="toc-backref" href="#id7">Build and test</a><a class="headerlink" href="#build-and-test" title="Permalink to this headline">¶</a></h3>
<p>Finally, we can build the code! It’s important to do this first, to
ensure we’re in a good state before making changes. But what to build?
In ninja, you specify a <strong>target</strong>. If we just want to build the clang
binary, our target name is “clang” and we run:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> ninja clang
</pre></div>
</div>
<p>The first time we build will be very slow - Clang + LLVM is a lot of
code. But incremental builds are fast: ninja will only rebuild the parts
that have changed. When it finally finishes you should have a working
clang binary. Try running:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> bin/clang --version
</pre></div>
</div>
<p>There’s also a target for building and running all the clang tests:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> ninja check-clang
</pre></div>
</div>
<p>This is a common pattern in LLVM: check-llvm is all the checks for core,
other projects have targets like check-lldb.</p>
</div>
</div>
<div class="section" id="making-changes">
<h2><a class="toc-backref" href="#id8">Making changes</a><a class="headerlink" href="#making-changes" title="Permalink to this headline">¶</a></h2>
<div class="section" id="edit">
<h3><a class="toc-backref" href="#id9">Edit</a><a class="headerlink" href="#edit" title="Permalink to this headline">¶</a></h3>
<p>We need to find the file containing the error message.</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> git grep <span class="s2">"all paths through this function"</span> ..
<span class="go">../clang/include/clang/Basic/DiagnosticSemaKinds.td: "all paths through this function will call itself">,</span>
</pre></div>
</div>
<p>The string that appears in DiagnosticSemaKinds.td is the one that is
printed by Clang. *.td files define tables - in this case it’s a list
of warnings and errors clang can emit and their messages. Let’s update
the message in your favorite editor:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> vi ../clang/include/clang/Basic/DiagnosticSemaKinds.td
</pre></div>
</div>
<p>Find the message (it should be under
warn<em>infinite</em>recursive_function)Change the message to “in order to
understand recursion, you must first understand recursion”.</p>
</div>
<div class="section" id="test-again">
<h3><a class="toc-backref" href="#id10">Test again</a><a class="headerlink" href="#test-again" title="Permalink to this headline">¶</a></h3>
<p>To verify our change, we can build clang and manually check that it
works.</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> ninja clang
<span class="gp">$</span> bin/clang -Wall ~/test.cc
<span class="go">**/path/test.cc:1:124:** **warning****: in order to understand recursion, you must</span>
<span class="go">first understand recursion [-Winfinite-recursion]**</span>
</pre></div>
</div>
<p>We should also run the tests to make sure we didn’t break something.</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> ninja check-clang
</pre></div>
</div>
<p>Notice that it is much faster to build this time, but the tests take
just as long to run. Ninja doesn’t know which tests might be affected,
so it runs them all.</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="go">********************</span>
<span class="go">Testing Time: 408.84s</span>
<span class="go">********************</span>
<span class="go">Failing Tests (1):</span>
<span class="go"> Clang :: SemaCXX/warn-infinite-recursion.cpp</span>
</pre></div>
</div>
<p>Well, that makes sense… and the test output suggests it’s looking for
the old string “call itself” and finding our new message instead.</p>
<p>Let’s fix it by updating the expectation in the test.</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> vi ../clang/test/SemaCXX/warn-infinite-recursion.cpp
</pre></div>
</div>
<p>Everywhere we see // expected-warning{{call itself}}, let’s replace it
with // expected-warning{{to understand recursion}}.</p>
<p>Now we could run <strong>all</strong> the tests again, but this is a slow way to
iterate on a change! Instead, let’s find a way to re-run just the
specific test. There are two main types of tests in LLVM:</p>
<ul class="simple">
<li><p><strong>lit tests</strong> (e.g. SemaCXX/warn-infinite-recursion.cpp).</p></li>
</ul>
<p>These are fancy shell scripts that run command-line tools and verify the
output. They live in files like
clang/<strong>test</strong>/FixIt/dereference-addressof.c. Re-run like this:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> bin/llvm-lit -v ../clang/test/SemaCXX/warn-infinite-recursion.cpp
</pre></div>
</div>
<ul class="simple">
<li><p><strong>unit tests</strong> (e.g. ToolingTests/ReplacementText.CanDeleteAllText)</p></li>
</ul>
<p>These are C++ programs that call LLVM functions and verify the results.
They live in suites like ToolingTests. Re-run like this:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> ninja ToolingTests <span class="o">&&</span> tools/clang/unittests/Tooling/ToolingTests
<span class="go">--gtest_filter=ReplacementText.CanDeleteAllText</span>
</pre></div>
</div>
</div>
<div class="section" id="commit-locally">
<h3><a class="toc-backref" href="#id11">Commit locally</a><a class="headerlink" href="#commit-locally" title="Permalink to this headline">¶</a></h3>
<p>We’ll save the change to a local git branch. This lets us work on other
things while the change is being reviewed. Changes should have a
description, to explain to reviewers and future readers of the code why
the change was made.</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> git checkout -b myfirstpatch
<span class="gp">$</span> git commit -am <span class="s2">"[Diagnostic] Clarify -Winfinite-recursion message"</span>
</pre></div>
</div>
<p>Now we’re ready to send this change out into the world! By the way,
There is a unwritten convention of using tag for your commit. Tags
usually represent modules that you intend to modify. If you don’t know
the tags for your modules, you can look at the commit history :
<a class="reference external" href="https://github.com/llvm/llvm-project/commits/main">https://github.com/llvm/llvm-project/commits/main</a>.</p>
</div>
</div>
<div class="section" id="code-review">
<h2><a class="toc-backref" href="#id12">Code review</a><a class="headerlink" href="#code-review" title="Permalink to this headline">¶</a></h2>
<div class="section" id="finding-a-reviewer">
<h3><a class="toc-backref" href="#id13">Finding a reviewer</a><a class="headerlink" href="#finding-a-reviewer" title="Permalink to this headline">¶</a></h3>
<p>Changes can be reviewed by anyone in the LLVM community who has commit
access.For larger and more complicated changes, it’s important that the
reviewer has experience with the area of LLVM and knows the design goals
well. The author of a change will often assign a specific reviewer (git
blame and git log can be useful to find one).</p>
<p>As our change is fairly simple, we’ll add the cfe-commits mailing list
as a subscriber; anyone who works on clang can likely pick up the
review. (For changes outside clang, llvm-commits is the usual list. See
<a class="reference external" href="http://lists.llvm.org/mailman/listinfo">http://lists.llvm.org/</a> for
all the *-commits mailing lists).</p>
</div>
<div class="section" id="uploading-a-change-for-review">
<h3><a class="toc-backref" href="#id14">Uploading a change for review</a><a class="headerlink" href="#uploading-a-change-for-review" title="Permalink to this headline">¶</a></h3>
<p>LLVM code reviews happen at <a class="reference external" href="https://reviews.llvm.org">https://reviews.llvm.org</a>. The web interface
is called Phabricator, and the code review part is Differential. You
should create a user account there for reviews (click “Log In” and then
“Register new account”).</p>
<p>Now you can upload your change for review:</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> arc diff HEAD^
</pre></div>
</div>
<p>This creates a review for your change, comparing your current commit
with the previous commit. You will be prompted to fill in the review
details. Your commit message is already there, so just add cfe-commits
under the “subscribers” section. It should print a code review URL:
<a class="reference external" href="https://reviews.llvm.org/D58291">https://reviews.llvm.org/D58291</a> You can always find your active reviews
on Phabricator under “My activity”.</p>
</div>
<div class="section" id="review-process">
<h3><a class="toc-backref" href="#id15">Review process</a><a class="headerlink" href="#review-process" title="Permalink to this headline">¶</a></h3>
<p>When you upload a change for review, an email is sent to you, the
cfe-commits list, and anyone else subscribed to these kinds of changes.
Within a few days, someone should start the review. They may add
themselves as a reviewer, or simply start leaving comments. You’ll get
another email any time the review is updated. The details are in the
<a class="reference external" href="https://llvm.org/docs/CodeReview.html">https://llvm.org/docs/CodeReview/</a>.</p>
<div class="section" id="comments">
<h4><a class="toc-backref" href="#id16">Comments</a><a class="headerlink" href="#comments" title="Permalink to this headline">¶</a></h4>
<p>The reviewer can leave comments on the change, and you can reply. Some
comments are attached to specific lines, and appear interleaved with the
code. You can either reply to these, or address them and mark them as
“done”. Note that in-line replies are <strong>not</strong> sent straight away! They
become “draft” comments and you must click “Submit” at the bottom of the
page.</p>
</div>
<div class="section" id="updating-your-change">
<h4><a class="toc-backref" href="#id17">Updating your change</a><a class="headerlink" href="#updating-your-change" title="Permalink to this headline">¶</a></h4>
<p>If you make changes in response to a reviewer’s comments, simply run</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> arc diff
</pre></div>
</div>
<p>again to update the change and notify the reviewer. Typically this is a
good time to send any draft comments as well.</p>
</div>
<div class="section" id="accepting-a-revision">
<h4><a class="toc-backref" href="#id18">Accepting a revision</a><a class="headerlink" href="#accepting-a-revision" title="Permalink to this headline">¶</a></h4>
<p>When the reviewer is happy with the change, they will <strong>Accept</strong> the
revision. They may leave some more minor comments that you should
address, but at this point the review is complete. It’s time to get it
committed!</p>
</div>
</div>
<div class="section" id="commit-by-proxy">
<h3><a class="toc-backref" href="#id19">Commit by proxy</a><a class="headerlink" href="#commit-by-proxy" title="Permalink to this headline">¶</a></h3>
<p>As this is your first change, you won’t have access to commit it
yourself yet. The reviewer <strong>doesn’t know this</strong>, so you need to tell
them! Leave a message on the review like:</p>
<blockquote>
<div><p>Thanks @somellvmdev. I don’t have commit access, can you land this
patch for me? Please use “My Name <a class="reference external" href="mailto:my%40email">my<span>@</span>email</a>” to commit the change.</p>
</div></blockquote>
<p>The review will be updated when the change is committed.</p>
</div>
<div class="section" id="review-expectations">
<h3><a class="toc-backref" href="#id20">Review expectations</a><a class="headerlink" href="#review-expectations" title="Permalink to this headline">¶</a></h3>
<p>In order to make LLVM a long-term sustainable effort, code needs to be
maintainable and well tested. Code reviews help to achieve that goal.
Especially for new contributors, that often means many rounds of reviews
and push-back on design decisions that do not fit well within the
overall architecture of the project.</p>
<p>For your first patches, this means:</p>
<ul class="simple">
<li><p>be kind, and expect reviewers to be kind in return - LLVM has a <a class="reference external" href="https://llvm.org/docs/CodeOfConduct.html">Code
of Conduct</a>;</p></li>
<li><p>be patient - understanding how a new feature fits into the
architecture of the project is often a time consuming effort, and
people have to juggle this with other responsibilities in their
lives; <strong>ping the review once a week</strong> when there is no response;</p></li>
<li><p>if you can’t agree, generally the best way is to do what the reviewer
asks; we optimize for readability of the code, which the reviewer is
in a better position to judge; if this feels like it’s not the right
option, you can contact the cfe-dev mailing list to get more feedback
on the direction;</p></li>
</ul>
</div>
</div>
<div class="section" id="commit-access">
<h2><a class="toc-backref" href="#id21">Commit access</a><a class="headerlink" href="#commit-access" title="Permalink to this headline">¶</a></h2>
<p>Once you’ve contributed a handful of patches to LLVM, start to think
about getting commit access yourself. It’s probably a good idea if:</p>
<ul class="simple">
<li><p>you’ve landed 3-5 patches of larger scope than “fix a typo”</p></li>
<li><p>you’d be willing to review changes that are closely related to yours</p></li>
<li><p>you’d like to keep contributing to LLVM.</p></li>
</ul>
<div class="section" id="getting-commit-access">
<h3><a class="toc-backref" href="#id22">Getting commit access</a><a class="headerlink" href="#getting-commit-access" title="Permalink to this headline">¶</a></h3>
<p>LLVM uses Git for committing changes. The details are in the <a class="reference external" href="https://llvm.org/docs/DeveloperPolicy.html#obtaining-commit-access">developer
policy
document</a>.</p>
</div>
<div class="section" id="with-great-power">
<h3><a class="toc-backref" href="#id23">With great power</a><a class="headerlink" href="#with-great-power" title="Permalink to this headline">¶</a></h3>
<p>Actually, this would be a great time to read the rest of the <a class="reference external" href="https://llvm.org/docs/DeveloperPolicy.html">developer
policy</a>, too. At minimum,
you need to be subscribed to the relevant commits list before landing
changes (e.g. <a class="reference external" href="mailto:llvm-commits%40lists.llvm.org">llvm-commits<span>@</span>lists<span>.</span>llvm<span>.</span>org</a>), as discussion often happens
there if a new patch causes problems.</p>
</div>
<div class="section" id="commit">
<h3><a class="toc-backref" href="#id24">Commit</a><a class="headerlink" href="#commit" title="Permalink to this headline">¶</a></h3>
<p>Let’s say you have a change on a local git branch, reviewed and ready to
commit. Things to do first:</p>
<ul class="simple">
<li><p>if you used multiple fine-grained commits locally, squash them into a
single commit. LLVM prefers commits to match the code that was
reviewed. (If you created one commit and then used “arc diff”, you’re
fine)</p></li>
<li><p>rebase your patch against the latest LLVM code. LLVM uses a linear
history, so everything should be based on an up-to-date origin/main.</p></li>
</ul>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> git pull --rebase https://github.com/llvm/llvm-project.git main
</pre></div>
</div>
<ul class="simple">
<li><p>ensure the patch looks correct.</p></li>
</ul>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> git show
</pre></div>
</div>
<ul class="simple">
<li><p>run the tests one last time, for good luck</p></li>
</ul>
<p>At this point git show should show a single commit on top of
origin/main.</p>
<p>Now you can push your commit with</p>
<div class="highlight-console notranslate"><div class="highlight"><pre><span></span><span class="gp">$</span> git push https://github.com/llvm/llvm-project.git HEAD:main
</pre></div>
</div>
<p>You should see your change <a class="reference external" href="https://github.com/llvm/llvm-project/commits/main">on
GitHub</a> within
minutes.</p>
</div>
<div class="section" id="post-commit-errors">
<h3><a class="toc-backref" href="#id25">Post-commit errors</a><a class="headerlink" href="#post-commit-errors" title="Permalink to this headline">¶</a></h3>
<p>Once your change is submitted it will be picked up by automated build
bots that will build and test your patch in a variety of configurations.</p>
<p>You can see all configurations and their current state in a waterfall
view at <a class="reference external" href="http://lab.llvm.org:8011/waterfall">http://lab.llvm.org:8011/waterfall</a>. The waterfall view is good
to get a general overview over the tested configurations and to see
which configuration have been broken for a while.</p>
<p>The console view at <a class="reference external" href="http://lab.llvm.org:8011/console">http://lab.llvm.org:8011/console</a> helps to get a
better understanding of the build results of a specific patch. If you
want to follow along how your change is affecting the build bots, <strong>this
should be the first place to look at</strong> - the colored bubbles correspond
to projects in the waterfall.</p>
<p>If you see a broken build, do not despair - some build bots are
continuously broken; if your change broke the build, you will see a red
bubble in the console view, while an already broken build will show an
orange bubble. Of course, even when the build was already broken, a new
change might introduce a hidden new failure.</p>
<div class="line-block">
<div class="line">When you want to see more details how a specific build is broken,
click the red bubble.</div>
<div class="line">If post-commit error logs confuse you, do not worry too much -
everybody on the project is aware that this is a bit unwieldy, so
expect people to jump in and help you understand what’s going on!</div>
</div>
<p>buildbots, overview of bots, getting error logs.</p>
</div>
<div class="section" id="reverts">
<h3><a class="toc-backref" href="#id26">Reverts</a><a class="headerlink" href="#reverts" title="Permalink to this headline">¶</a></h3>
<p>if in doubt, revert and re-land.</p>
</div>
</div>
<div class="section" id="conclusion">
<h2><a class="toc-backref" href="#id27">Conclusion</a><a class="headerlink" href="#conclusion" title="Permalink to this headline">¶</a></h2>
<p>llvm is a land of contrasts.</p>
</div>
</div>
<div class="clearer"></div>
</div>
</div>
</div>
<div class="clearer"></div>
</div>
<div class="related" role="navigation" aria-label="related navigation">
<h3>Navigation</h3>
<ul>
<li class="right" style="margin-right: 10px">
<a href="genindex.html" title="General Index"
>index</a></li>
<li class="right" >
<a href="Reference.html" title="Reference"
>next</a> |</li>
<li class="right" >
<a href="tutorial/BuildingAJIT4.html" title="4. Building a JIT: Extreme Laziness - Using LazyReexports to JIT from ASTs"
>previous</a> |</li>
<li><a href="https://llvm.org/">LLVM Home</a> | </li>
<li><a href="index.html">Documentation</a>»</li>
<li class="nav-item nav-item-1"><a href="GettingStartedTutorials.html" >Getting Started/Tutorials</a> »</li>
<li class="nav-item nav-item-this"><a href="">MyFirstTypoFix</a></li>
</ul>
</div>
<div class="footer" role="contentinfo">
© Copyright 2003-2021, LLVM Project.
Last updated on 2021-09-18.
Created using <a href="https://www.sphinx-doc.org/">Sphinx</a> 3.5.4.
</div>
</body>
</html>
|
