Skip to content

Make clang use libc++ by default#5943

Merged
junghans merged 1 commit intospack:developfrom
certik:clang_libcxx_fix
Oct 25, 2017
Merged

Make clang use libc++ by default#5943
junghans merged 1 commit intospack:developfrom
certik:clang_libcxx_fix

Conversation

@certik
Copy link
Copy Markdown
Contributor

@certik certik commented Oct 25, 2017

Fixes #5942.

@certik
Copy link
Copy Markdown
Contributor Author

certik commented Oct 25, 2017

I tested this with [email protected] and it works for me, see #5942 (comment). But I think this cmake option is not available in older LLVM versions, so this PR needs to be improved to take this into account.

This was referenced Oct 25, 2017
Closed
Closed
davydden
davydden reviewed Oct 25, 2017
View reviewed changes
var/spack/repos/builtin/packages/llvm/package.py Outdated
if '+libcxx' not in spec:
cmake_args.append('-DLLVM_EXTERNAL_LIBCXX_BUILD:Bool=OFF')
cmake_args.append('-DLLVM_EXTERNAL_LIBCXXABI_BUILD:Bool=OFF')
else:
Copy link
Copy Markdown
Member

@davydden davydden Oct 25, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think it would be easier to read if you do

if '+libcxx' in spec:
   cmake_args.append('-DCLANG_DEFAULT_CXX_STDLIB=libc++')
else:
   // old code

@tgamblin
Copy link
Copy Markdown
Member

tgamblin commented Oct 25, 2017

@certik: can you add the version check to the conditional?

tgamblin
tgamblin previously requested changes Oct 25, 2017
View reviewed changes
Copy link
Copy Markdown
Member

@tgamblin tgamblin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see above

@certik
Copy link
Copy Markdown
Contributor Author

certik commented Oct 25, 2017 •
edited
Loading

It looks like this option has been introduced by this commit: llvm-mirror/clang@1b363a4, so we just need to figure out what version of LLVM that was by using:

+certik@redhawk:~/repos/clang(master)$ git branch -r
  origin/HEAD -> origin/master
  origin/google/stable
  origin/google/testing
  origin/master
  origin/release_26
  origin/release_27
  origin/release_28
  origin/release_29
  origin/release_30
  origin/release_31
  origin/release_32
  origin/release_33
  origin/release_34
  origin/release_35
  origin/release_35@215011
  origin/release_36
  origin/release_37
  origin/release_38
  origin/release_39
  origin/release_40
  origin/release_50
+certik@redhawk:~/repos/clang(master)$ git branch -r --contains 1b363a46f7845c67019f432f429f96cf64a99f4e
  origin/HEAD -> origin/master
  origin/google/stable
  origin/google/testing
  origin/master
  origin/release_39
  origin/release_40
  origin/release_50

and from this we can see that LLVM 3.9 and up support this.

@certik
Make clang use libc++ by default
4cdacf0 
Since LLVM 3.9 Clang can use the libc++ library by default using the
CLANG_DEFAULT_CXX_STDLIB cmake configuration variable, without having to
specify the -stdlib=libc++ option on the clang++ command line.

This commit makes clang++ use libc++ by default for LLVM 3.9 and later if the
libcxx variant is on.

Fixes spack#5942.
@certik
Copy link
Copy Markdown
Contributor Author

certik commented Oct 25, 2017 •
edited
Loading

@tgamblin tests pass. I tested this, it seems to work. I tried the earliest version 3.9.0, and this cmake flag does not seem to cause problems (the build actually fails, but due to an unrelated error #5954 it seems).

@certik
Copy link
Copy Markdown
Contributor Author

certik commented Oct 25, 2017

Could this please be merged? So that I have one less branch to worry about.

@junghans junghans merged commit 89b0a09 into spack:develop Oct 25, 2017
@certik certik deleted the clang_libcxx_fix branch October 25, 2017 22:20
@certik
Copy link
Copy Markdown
Contributor Author

certik commented Oct 25, 2017

Thanks @junghans.

@cdfh cdfh mentioned this pull request Jun 10, 2022
Merged
@cdfh
Copy link
Copy Markdown

cdfh commented Jun 10, 2022

For anybody landing here from Google, it should be noted that this behaviour has now been reverted by #19933.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@junghans junghans junghans approved these changes

@tgamblin tgamblin tgamblin left review comments

+1 more reviewer

@davydden davydden davydden approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

The llvm+clang c++ compiler cannot find -lstdc++

5 participants

@certik @tgamblin @cdfh @junghans @davydden