Page MenuHomePhabricator

Move LangStandard*, InputKind::Language to Basic
ClosedPublic

Authored by ro on Aug 1 2019, 2:03 AM.

Details

Summary

This patch is a prerequisite for using LangStandard from Driver in https://reviews.llvm.org/D64793.

It moves LangStandard* and InputKind::Language to Basic. It is mostly
mechanical, with only a few changes of note:

  • I've renamed OpenCL to LF_OpenCL in enum LangFeatures to avoid a clash with OpenCL in enum Language.
  • Now that getLangStandardForName, which is currently unused, also checks both canonical and alias names, I've introduced a helper getLangKind which factors out a code pattern already used 3 times.

The patch has been tested on x86_64-pc-solaris2.11, sparcv9-sun-solaris2.11,
and x86_64-pc-linux-gnu.

However, it will need companion patches for lldb and polly which both use either
or both of LangStandard.h and InputKind::*. To avoid everyone's time, I'll only
write and test them once it becomes clear that the current approach is acceptable.

Diff Detail

Repository
rL LLVM

Event Timeline

ro created this revision.Aug 1 2019, 2:03 AM
Herald added a project: Restricted Project. · View Herald Transcript
rnk added a comment.Aug 1 2019, 11:22 AM

Thanks!

include/clang/Basic/LangStandard.h
19 ↗(On Diff #212753)

Is it feasible to make this an enum class? I'm worried about namespace clashes on these otherwise very short names, like C, CXX, HIP, etc. It should be straightforward and mechanical to replace most existing instances of InputKind:: with Language::. It would also remove the need to make an exception for LF_OpenCL.

ro marked 2 inline comments as done.Aug 2 2019, 1:55 AM
ro added inline comments.
include/clang/Basic/LangStandard.h
19 ↗(On Diff #212753)

That works perfectly indeed, and is way clearer than my hack
with LF_OpenCL.

There's only one downside: I had to change InputKind.Lang
from a 4-bit bitfield to Language, otherwise neither assignment
nor comparison would work. No idea if that's really a problem.

ro updated this revision to Diff 212992.Aug 2 2019, 1:56 AM
ro marked an inline comment as done.

Change Language to enum class.

Tested as before.

rnk accepted this revision.Aug 2 2019, 2:28 PM

Looks good either way.

include/clang/Basic/LangStandard.h
19 ↗(On Diff #212753)

I don't think it matters, but I would like to keep InputKind 32-bits or less. The easy way would be to use enum class Language : uint8_t here. The other way would be to keep the bitfield and add a static cast in the constructor. I see getLanguage() already performs one, so most of the uses shouldn't need a change.

This revision is now accepted and ready to land.Aug 2 2019, 2:28 PM
ro updated this revision to Diff 213243.Aug 4 2019, 6:29 AM
ro marked 2 inline comments as done.
  • Restrict enum class Languge to uint8_t to save space.
  • Filter patch through clang-format-diff.py
include/clang/Basic/LangStandard.h
19 ↗(On Diff #212753)

I decided to be lazy this time and went for the first alternative.

ro added a comment.Aug 4 2019, 6:37 AM

I've now submitted the lldb patch to reflect the LangStandard.h move.

It turns out the part of isl (included in polly) that uses InputKind::C isn't even compiled inside the llvm tree. Nonetheless, I've submitted a patch upstream to allow for Language::C instead. However, that part of the isl code didn't even compile before my patch, so acceptance of my patch won't have to block integrating this one.

This revision was automatically updated to reflect the committed changes.
Herald added a project: Restricted Project. · View Herald TranscriptAug 5 2019, 6:58 AM