Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tweak Clang 5/6 gcc toolchain versions #905

Merged
merged 2 commits into from Oct 1, 2018
Merged

Tweak Clang 5/6 gcc toolchain versions #905

merged 2 commits into from Oct 1, 2018

Conversation

RubenRBS
Copy link
Member

Not meant to be merged yet, we need to make sure this time we get it right.
Any tests welcome

It fixes #903 if merged

@codecov
Copy link

codecov bot commented Apr 18, 2018

Codecov Report

Merging #905 into master will decrease coverage by 5.2%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #905      +/-   ##
==========================================
- Coverage   62.04%   56.83%   -5.21%     
==========================================
  Files          64       42      -22     
  Lines        2914     2187     -727     
  Branches      548      421     -127     
==========================================
- Hits         1808     1243     -565     
+ Misses       1106      944     -162
Impacted Files Coverage Δ
lib/compilers/golang.js 0% <0%> (-74.36%) ⬇️
lib/compiler-finder.js 0% <0%> (-42.03%) ⬇️
lib/base-compiler.js 5.14% <0%> (-5.26%) ⬇️
lib/compilers/pascal.js 50% <0%> (-1.58%) ⬇️
lib/languages.js 88.88% <0%> (-1.12%) ⬇️
lib/properties.js 98.24% <0%> (-0.48%) ⬇️
lib/aws.js 94.87% <0%> (-0.26%) ⬇️
lib/handlers/api.js 100% <0%> (ø) ⬆️
lib/asm-raw.js 0% <0%> (ø) ⬆️
lib/symbol-store.js 100% <0%> (ø) ⬆️
... and 37 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 90e6794...cbd68a8. Read the comment docs.

@partouf
Copy link
Contributor

partouf commented Apr 24, 2018

The problem I have with this kind of solution -- I mean you should probably merge this if it works and solves their issues, but -- is that it's not a proper solution.

I think 1. There should be some kind of proof (I can't find it on google) that this particular version of ClangC++ is tested and should work with libstdc++ version X.

And 2. It should probably be turned into a user option so people can use other compatible implementations and versions of libc++ and/or libc.

@mattgodbolt
Copy link
Member

While I agree completely with @partouf 's comments, this might be an improvement already. It does have the potential to break existing URLs, admittedly. My gut instinct is to go with this though, and use the latest gcc major revision available at each clang publish. According to Wikipedia:

  • clang 5 released on 7 Sep 2017
  • clang 6 released on 8 March 2018

According to GCC's release page, GCC 7.3 was the latest build at both these releases. So I'm cool to merge this.

@RubenRBS RubenRBS merged commit 8beee8c into master Oct 1, 2018
@RubenRBS RubenRBS deleted the clangs branch October 1, 2018 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Does clang compilation try to use GCC's libstdc++?
3 participants