[PATCH] D64793: [Driver] Properly use values-X[ca].o, values-xpg[46].o on Solaris

Reid Kleckner via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Mon Jul 29 11:08:01 PDT 2019

rnk added a comment.

In D64793#1597765 <https://reviews.llvm.org/D64793#1597765>, @ro wrote:

> In D64793#1597550 <https://reviews.llvm.org/D64793#1597550>, @jyknight wrote:
> > How about instead just adding "values-xpg6.o" unconditionally, alongside the current unconditional "values-Xa.o", and just forget about the xpg4 and Xc modes?
> If all else fails, that would have to be the last fallback.  I'd rather do things correctly, though.

I think the cost of complexity in the solaris-specific part of the driver is low, so if the maintainer thinks it's worth having, we can certainly add it.

Comment at: lib/Driver/ToolChains/Solaris.cpp:16
 #include "clang/Driver/Options.h"
+#include "clang/Frontend/LangStandard.h"
 #include "llvm/Option/ArgList.h"
ro wrote:
> jyknight wrote:
> > I'm not sure that's an acceptable dependency -- I think Driver probably is not supposed to depend on Frontend. If so, I guess LangStandard should move to clang/Basic/. Which also means moving InputKind from clang/include/clang/Frontend/FrontendOptions.h.
> > 
> > (Maybe someone else can weigh in on this question?)
> I wondered about this myself, including frontend code in the
> driver might be considered a layering violation.  I certainly need
> guidance here what is and isn't acceptable here.
I see there are no other includes of Frontend from Driver, so I think LangStandards* does need to move to Basic. The only piece of InputKind that's needed is the language enum. I'm surprised there isn't already one somewhere else, but if there isn't, I think it would be reasonable to define the input kind languages in LangStandard.h and use them from FrontendOptions.

Comment at: lib/Frontend/LangStandards.cpp:31-37
   Kind K = llvm::StringSwitch<Kind>(Name)
 #define LANGSTANDARD(id, name, lang, desc, features) \
     .Case(name, lang_##id)
+#define LANGSTANDARD_ALIAS(id, alias) \
+    .Case(alias, lang_##id)
 #include "clang/Frontend/LangStandards.def"
I see that this code pattern is repeated in two other places, lib/Tooling/InterpolatingCompilationDatabase.cpp and lib/Frontend/CompilerInvocation.cpp. I think it would be good to factor out a string-to-kind helper and use it in the three places.

  rC Clang



More information about the cfe-commits mailing list