Page MenuHomePhabricator

clang -dumpversion returns 4.2.1 for legacy reason, update it
ClosedPublic

Authored by sylvestre.ledru on Jan 16 2019, 1:04 PM.

Diff Detail

Repository
rL LLVM

Event Timeline

Herald added a project: Restricted Project. · View Herald TranscriptMar 13 2019, 10:45 AM
Herald added a subscriber: jdoerfert. · View Herald Transcript
sylvestre.ledru edited the summary of this revision. (Show Details)Mar 23 2019, 10:59 AM
rnk accepted this revision.Mar 25 2019, 10:08 AM

Code lgtm, with a suggestion to tighten the test.

lib/Driver/Driver.cpp
1631 ↗(On Diff #182128)

We really ought to reconsider __VERSION__ as well, although I don't think anyone checks that. Today I get this for it:

"4.2.1 Compatible Clang 9.0.0 (git@github.com:llvm/llvm-project.git bd6354105b055af6193fb2b7561fe6f5ae0eb0d8)"
test/Driver/immediate-options.c
10 ↗(On Diff #182128)

Maybe check {{[0-9]+\.[0-9.]+}}, to insist that it's a sequence of numbers and dots.

This revision is now accepted and ready to land.Mar 25 2019, 10:08 AM
sylvestre.ledru marked 3 inline comments as done.
sylvestre.ledru edited the summary of this revision. (Show Details)

Improve the test (thanks rnk)

@rnk btw, do you think it should be added to the clang release notes?

lib/Driver/Driver.cpp
1631 ↗(On Diff #182128)

I can have a look to that if you want

Remove the empty directories

Harbormaster completed remote builds in B29587: Diff 192160.
This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptMar 25 2019, 11:04 AM
rnk added a comment.Mar 25 2019, 11:28 AM

@rnk btw, do you think it should be added to the clang release notes?

I think it is release-note worthy. If you change VERSION, you can mention that as well.

lib/Driver/Driver.cpp
1631 ↗(On Diff #182128)

If you're feeling motivated, I think it's worth doing to try to get us into a more consistent state.