This is an archive of the discontinued LLVM Phabricator instance.

[libc++] Remove the __libcpp_version file
ClosedPublic

Authored by ldionne on Mar 23 2022, 10:19 AM.

Details

Reviewers
EricWF
rsmith
ldionne
Group Reviewers
Restricted Project
Commits
rG19246b0779a2: [libc++] Remove the __libcpp_version file
Summary

It seems to have been added back in 761e42fa3dd72 for Clang to use it,
however it seems to have never been used for that purpose, so it is
probably fine to remove it.

Diff Detail

Event Timeline

ldionne created this revision.Mar 23 2022, 10:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2022, 10:19 AM
ldionne requested review of this revision.Mar 23 2022, 10:19 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 23 2022, 10:19 AM
Herald added a reviewer: Restricted Project. · View Herald Transcript

@EricWF I'm curious since you added this -- am I correct when I claim that this ended up not being used by Clang, or did I miss something? Do you think it's fine to remove it?

EricWF added a subscriber: rsmith.

@rsmith had plans for this header. Richard, are we good to remove this?

ldionne accepted this revision as: Restricted Project.Mar 30 2022, 5:41 AM

Gentle ping. I'll ship this tomorrow unless @rsmith provides more information. Richard, I'll also ping you on Discord just in case.

There was an idea that we could use this to enable / disable version-specific workarounds for libc++/clang incompatibilities (eg, if recent clang sees it's using a sufficiently new version of libc++ then it can avoid enabling a workaround that's only necessary for older versions of libc++). We never implemented the clang side of this, though, so I don't think it's used by anything, and it seems like something we can add back later if there's a motivating use case.

ldionne accepted this revision.Mar 31 2022, 6:33 AM

There was an idea that we could use this to enable / disable version-specific workarounds for libc++/clang incompatibilities (eg, if recent clang sees it's using a sufficiently new version of libc++ then it can avoid enabling a workaround that's only necessary for older versions of libc++). We never implemented the clang side of this, though, so I don't think it's used by anything, and it seems like something we can add back later if there's a motivating use case.

Got it. Also, things have evolved to a point where libc++ and Clang are generally in rough lockstep with each other, so hopefully this shouldn't be necessary. I agree this can be added later if needed, but thanks for the additional context.

This revision is now accepted and ready to land.Mar 31 2022, 6:33 AM

(I'm going to ship this since you didn't seem opposed to it)

This revision was automatically updated to reflect the committed changes.
libcxx/include/CMakeLists.txt