This is an archive of the discontinued LLVM Phabricator instance.

Differential D127496

[NFC] Suggest Release mode in clang GettingStarted.html
ClosedPublic

Authored by xgupta on Jun 10 2022, 6:20 AM.

Details

Reviewers
thieta
aaron.ballman
Summary

This fix https://github.com/llvm/llvm-project/issues/23841.
Lots of beginners are not of aware of this option do suggesting it here
would be helpful.

Diff Detail

Unit TestsFailed

TimeTest
60 msx64 debian > Clang.Misc::warning-flags.c
760 msx64 debian > SanitizerCommon-tsan-x86_64-Linux.Linux::decorate_proc_maps.cpp

Event Timeline

xgupta created this revision.Jun 10 2022, 6:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2022, 6:20 AM
xgupta requested review of this revision.Jun 10 2022, 6:20 AM
Herald added a project: Restricted Project. · View Herald TranscriptJun 10 2022, 6:20 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
Harbormaster completed remote builds in B169058: Diff 435892.Jun 10 2022, 7:15 AM
thieta requested changes to this revision.Jun 10 2022, 7:21 AM

Thanks for fixing - but it's not completely accurate right now.

clang/www/get_started.html
72

This isn't correct - invoking cmake without a CMAKE_BUILD_TYPE argument will just print an error. I would fix the command line below to include -DCMAKE_BUILD_TYPE=Release and have a comment saying switch Release to Debug if you need a debug build.

This revision now requires changes to proceed.Jun 10 2022, 7:21 AM
aaron.ballman added inline comments.Jun 10 2022, 7:29 AM
clang/www/get_started.html
72

+1 to most of this, but I'd have the comment say something along the lines of "See https://llvm.org/docs/CMake.html#frequently-used-cmake-variables for more options".

xgupta updated this revision to Diff 435947.Jun 10 2022, 9:25 AM

Address comments

xgupta marked 2 inline comments as done.Jun 10 2022, 9:25 AM
xgupta added inline comments.Jun 10 2022, 9:28 AM
clang/www/get_started.html
72

I hope 'most of this' also includes making changes to the default suggestion of -DCMAKE_BUILD_TYPE :)

aaron.ballman added a comment.Jun 10 2022, 9:36 AM

Looks pretty close, just some minor nits from me.

clang/www/get_started.html
72

I hope 'most of this' also includes making changes to the default suggestion of -DCMAKE_BUILD_TYPE :)

Heh, it did! :-)

73–74
xgupta updated this revision to Diff 435957.Jun 10 2022, 9:47 AM
aaron.ballman accepted this revision.Jun 10 2022, 9:50 AM

LGTM aside from a missing comma (that can be fixed when landing). But you should give @thieta a chance to chime in before landing.

clang/www/get_started.html
73

Looks like this was missed with the other changes.

thieta accepted this revision.Jun 10 2022, 10:19 AM

Looks fine to me. Thanks for fixing!

This revision is now accepted and ready to land.Jun 10 2022, 10:19 AM
xgupta added a comment.Jun 10 2022, 10:35 AM

Thank you for review! Committed in https://reviews.llvm.org/rGa01579ad0a1bde4d90f4fb656f07586c3097428a.

xgupta closed this revision.Jun 10 2022, 10:35 AM
Harbormaster completed remote builds in B169104: Diff 435957.Jun 10 2022, 11:13 AM

Revision Contents

PathSize
clang/
www/
2 lines

Diff 435892

clang/www/get_started.html

Show First 20 Lines • Show All 63 Lines • ▼ Show 20 Lines <ul>
</li> </li>
</ul> </ul>
</li> </li>
<li>Build LLVM and Clang: <li>Build LLVM and Clang:
<ul> <ul>
<li><tt>cd llvm-project</tt></li> <li><tt>cd llvm-project</tt></li>
<li><tt>mkdir build</tt> (in-tree build is not supported)</li> <li><tt>mkdir build</tt> (in-tree build is not supported)</li>
<li><tt>cd build</tt></li> <li><tt>cd build</tt></li>
<li>This builds both LLVM and Clang for debug mode and takes a lot of time and space. Alternatively -DCMAKE_BUILD_TYPE=Release" can be use to make a release build.</li>
thietaUnsubmitted
Done
ReplyInline Actions

This isn't correct - invoking cmake without a CMAKE_BUILD_TYPE argument will just print an error. I would fix the command line below to include -DCMAKE_BUILD_TYPE=Release and have a comment saying switch Release to Debug if you need a debug build.

thieta: This isn't correct - invoking cmake without a `CMAKE_BUILD_TYPE` argument will just print an…
aaron.ballmanUnsubmitted
Done
ReplyInline Actions

+1 to most of this, but I'd have the comment say something along the lines of "See https://llvm.org/docs/CMake.html#frequently-used-cmake-variables for more options".

aaron.ballman: +1 to most of this, but I'd have the comment say something along the lines of "See https://llvm.
xguptaAuthorUnsubmitted
Done
ReplyInline Actions

I hope 'most of this' also includes making changes to the default suggestion of -DCMAKE_BUILD_TYPE :)

xgupta: I hope 'most of this' also includes making changes to the default suggestion of…
aaron.ballmanUnsubmitted
Done
ReplyInline Actions

I hope 'most of this' also includes making changes to the default suggestion of -DCMAKE_BUILD_TYPE :)

Heh, it did! :-)

aaron.ballman: > I hope 'most of this' also includes making changes to the default suggestion of…
<li><tt>cmake -DLLVM_ENABLE_PROJECTS=clang -G "Unix Makefiles" ../llvm</tt></li> <li><tt>cmake -DLLVM_ENABLE_PROJECTS=clang -G "Unix Makefiles" ../llvm</tt></li>
aaron.ballmanUnsubmitted
Not Done
ReplyInline Actions
<li><tt>cd build</tt></li>
- <li>This builds both LLVM and Clang in release mode. Alternatively if
+ <li>This builds both LLVM and Clang in release mode. Alternatively, if
you need a debug build, switch Release to Debug. See

Looks like this was missed with the other changes.

aaron.ballman: Looks like this was missed with the other changes.
<li><tt>make</tt></li> <li><tt>make</tt></li>
aaron.ballmanUnsubmitted
Not Done
ReplyInline Actions
<li><tt>cd build</tt></li>
- <li>This builds both LLVM and Clang for release mode. Alternatively if
+ <li>This builds both LLVM and Clang in release mode. Alternatively, if
you need a debug build, switch Release to Debug. See
- <a href="ttps://llvm.org/docs/CMake.html#frequently-used-cmake-variables">frequently used cmake variables</a>
+ <a href="https://llvm.org/docs/CMake.html#frequently-used-cmake-variables">frequently used cmake variables</a>
for more options.
</li>
aaron.ballman:
<li>This builds both LLVM and Clang for debug mode.</li>
<li>Note: For subsequent Clang development, you can just run <li>Note: For subsequent Clang development, you can just run
<tt>make clang</tt>.</li> <tt>make clang</tt>.</li>
<li>CMake allows you to generate project files for several IDEs: Xcode, <li>CMake allows you to generate project files for several IDEs: Xcode,
Eclipse CDT4, CodeBlocks, Qt-Creator (use the CodeBlocks generator), Eclipse CDT4, CodeBlocks, Qt-Creator (use the CodeBlocks generator),
KDevelop3. For more details see KDevelop3. For more details see
<a href="https://llvm.org/docs/CMake.html">Building LLVM with CMake</a> <a href="https://llvm.org/docs/CMake.html">Building LLVM with CMake</a>
page.</li> page.</li>
</ul> </ul>
▲ Show 20 Lines • Show All 236 Lines • Show Last 20 Lines