Fix bug 8220 - llvm-config: Only keep flags starting by -I, -D & -std for --cflags, --cxxflags & --ldflags

[sylvestre.ledru]

Summary

This will change the behavior of for --cflags, --cxxflags & --ldflags
Currently, a lot of projects are filtering out the results.
It has been a pain for linux distro using llvm

[sylvestre.ledru]

Other potential options:

• add options --cxxflags-short --cppflags-short --ldflags-short
• or an option -- strip-extra-flags

[tstellar]

Don't we need to leave -fno-rtti here if LLVM was complied without rtti? Also, rather than filtering out, would it be possible to explicitly add the flags that users need to successfully link/build against this install of LLVM?

[infinity0]

In D55174#1318718, @tstellar wrote:

Don't we need to leave -fno-rtti here if LLVM was complied without rtti? Also, rather than filtering out, would it be possible to explicitly add the flags that users need to successfully link/build against this install of LLVM?

Some projects are filtering out, some projects are filtering in - there is no standard approach currently.

This PR is suggesting indeed to filter-in, which I think is probably better. However neither me nor sylvestre knows both GCC and Clang well enough, to know if -I -D -std covers everything. If you know that -fno-rtti is necessary then yes of course it will have to be part of this PR as well.

Is this sort of logic already covered by pkg-config? It sounds like their developers would know the most about this topic. If so, perhaps in the long-run LLVM should move to that, rather than implementing its own?

[tstellar]

I only mentioned -fno-rtti specifically because this is something that llvm-config already knows about. I also don't have knowledge of all the possible flags that might need to be filtered.

After thinking about this more, I don't think we should pass through any user specific compiler flags at all. We should limit it to -I and -D flags added by the build system or compiler flags added by llvm specific options, like LLVM_ENABLE_RTTI. I think keeping upstream simple and having package maintainers filter-in options as needed to llvm-config makes more sense, because package maintainers know the flags that they are using and are in the best position to decide which flags users should be using too.

[sylvestre.ledru]

@tstellar will you accept this patch if I remove -std?
thanks

[sylvestre.ledru]

And should I update the release notes?

[tstellar]

In D55174#1321973, @sylvestre.ledru wrote:

@tstellar will you accept this patch if I remove -std?
thanks

I think stripping out -fno-rtti is going to be really disruptive. Here is an alternative solution I came up with: D55391.

tools/llvm-config/llvm-config.cpp

std::vector<std::string> GetAllDyLibComponents(const bool IsInDevelopmentTree,
 
   assert(!DyLibComponents.empty());
 
   return ComputeLibsForComponents(DyLibComponents,
                                   /*IncludeNonInstalled=*/IsInDevelopmentTree,
                                   GetComponentNames, nullptr, nullptr, DirSep);
 }
 
+/// Remove all arguments beside -I, -D & -std from the args flags
+/// This is mostly done to get rid of the warnings as they should
+/// be managed by the downstream projects
+///
+/// \param flag the raw list of flags
+static std::string ignoreArguments(const char *flag) {
+  StringRef FlagStr(flag);
+  SmallVector<StringRef, 4> Opts;
+  FlagStr.trim().split(Opts, ' ');
+  std::string args = "";
+  for (StringRef Opt : Opts) {
+      if (Opt.startswith("-I") || Opt.startswith("-D") || Opt.startswith("-std")) {
+          args += Opt;
+          args += " ";
+      }
+  }
+  return args;
+}
+
+
 int main(int argc, char **argv) {
   std::vector<StringRef> Components;
   bool PrintLibs = false, PrintLibNames = false, PrintLibFiles = false;
   bool PrintSystemLibs = false, PrintSharedMode = false;
   bool HasAnyOption = false;
 
   // llvm-config is designed to support being run both from a development tree
   // and from an installed path. We try and auto-detect which case we are in so
if (Arg.startswith("-")) {
         OS << ActiveBinDir << '\n';
       } else if (Arg == "--includedir") {
         OS << ActiveIncludeDir << '\n';
       } else if (Arg == "--libdir") {
         OS << ActiveLibDir << '\n';
       } else if (Arg == "--cmakedir") {
         OS << ActiveCMakeDir << '\n';
       } else if (Arg == "--cppflags") {
-        OS << ActiveIncludeOption << ' ' << LLVM_CPPFLAGS << '\n';
+          OS << ActiveIncludeOption << ' ' << ignoreArguments(LLVM_CPPFLAGS) << '\n';
       } else if (Arg == "--cflags") {
-        OS << ActiveIncludeOption << ' ' << LLVM_CFLAGS << '\n';
+        OS << ActiveIncludeOption << ' ' << ignoreArguments(LLVM_CFLAGS) << '\n';
       } else if (Arg == "--cxxflags") {
-        OS << ActiveIncludeOption << ' ' << LLVM_CXXFLAGS << '\n';
+          OS << ActiveIncludeOption << ' ' << ignoreArguments(LLVM_CXXFLAGS) << '\n';
       } else if (Arg == "--ldflags") {
         OS << ((HostTriple.isWindowsMSVCEnvironment()) ? "-LIBPATH:" : "-L")
            << ActiveLibDir << ' ' << LLVM_LDFLAGS << '\n';
       } else if (Arg == "--system-libs") {
         PrintSystemLibs = true;
       } else if (Arg == "--libs") {
         PrintLibs = true;
       } else if (Arg == "--libnames") {