remove redundant LLVM version from version string when setting CLANG_VENDOR

[nickdesaulniers]

Summary

When downstream LLVM distributions (like AOSP) set the CLANG_VENDOR
cmake variable, the version string printed by the clang driver looks
like:

$ clang --version
[CLANG_VENDOR] clang version X.X.X ([CLANG_REPOSITORY_STRING] sha) (based on LLVM X.X.X)

Rather than the more standard:
$ clang --version
clang version X.X.X ([CLANG_REPOSITORY_STRING] sha)

Based on feedback the the version string is a little long, the trailing
"(based on LLVM X.X.X)" is redundant and makes less sense after moving
LLVM to the monorepo. And it is only added should vendors set the cmake
variable CLANG_VENDOR. Let's remove it.

[nickdesaulniers]

clang/lib/Basic/Version.cpp

132

Looks like this is the sole use of BACKEND_PACKAGE_STRING, might as well remove that, too.

[nickdesaulniers]

clang/lib/Basic/Version.cpp

132

er, nvm, was running grep in my build dir.

[efriedma]

I think the reason it's written this way is that CLANG_VERSION_STRING might not correspond to an LLVM version? For example, Apple has its own versioning scheme.

[srhines]

In D69925#1736434, @efriedma wrote:

I think the reason it's written this way is that CLANG_VERSION_STRING might not correspond to an LLVM version? For example, Apple has its own versioning scheme.

Line 128 is already printing that info though, isn't it?

[arphaman]

+1, Apple's clang has an #ifdef 0 around this code.

clang/lib/Basic/Version.cpp

143

This use of CLANG_VENDOR should be removed as well presumably.

[arphaman]

clang/lib/Basic/Version.cpp

143

Please ignore my comment. I missed the fact that this output is still needed to match the CLANG_VENDOR use above that's not removed.

[nickdesaulniers]

In D69925#1737361, @arphaman wrote:

+1, Apple's clang has an #ifdef 0 around this code.

Is that +1 in favor of the patch as is, or +1 to @eli.friedman 's comment?

[nickdesaulniers]

Just grabbed a version string off a MBP:

$ clang --version
Apple LLVM version 10.0.1 (clang-1001.0.46.4)
Target: x86_64-apple-darwin18.7.0
Thread model: posix
InstalledDir: /Library/Developer/CommandLineTools/usr/bin

So it looks like the Apple distribution has a quite different implementation of this method.

[efriedma]

If we expect that CLANG_VERSION_STRING and BACKEND_PACKAGE_STRING are going to be the same, then the extra note isn't really buying us anything. LGTM

[nickdesaulniers]

Thanks for the review, leaving open for final comments, will merge tomorrow.

FWIW, I think we can now purge "svn" from BACKEND_PACKAGE_STRING and other places in the tree.

[emaste]

It looks like this conflicts with OpenSSL's compiler version check

• https://svnweb.freebsd.org/changeset/base/364822
• https://lists.freebsd.org/pipermail/svn-src-all/2020-August/201804.html

We've fixed it in FreeBSD and the change "should" happen upstream, but something to watch out for with other software.

[nickdesaulniers]

Sorry! Thanks for fixing up OpenSSL!

clang/lib/Basic/Version.cpp

std::string getClangToolFullVersion(StringRef ToolName) {
   std::string buf;
   llvm::raw_string_ostream OS(buf);
 #ifdef CLANG_VENDOR
   OS << CLANG_VENDOR;
 #endif
   OS << ToolName << " version " CLANG_VERSION_STRING " "
      << getClangFullRepositoryVersion();
 
-  // If vendor supplied, include the base LLVM version as well.
-#ifdef CLANG_VENDOR
-  OS << " (based on " << BACKEND_PACKAGE_STRING << ")";
-#endif
-
   return OS.str();
 }
 
 std::string getClangFullCPPVersion() {
   // The version string we report in __VERSION__ is just a compacted version of
   // the one we report on the command line.
   std::string buf;
   llvm::raw_string_ostream OS(buf);
 #ifdef CLANG_VENDOR
   OS << CLANG_VENDOR;
 #endif
   OS << "Clang " CLANG_VERSION_STRING " " << getClangFullRepositoryVersion();
   return OS.str();
 }
 
 } // end namespace clang