[PATCH] D63048: Update __VERSION__ to remove the hardcoded 4.2.1 version

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a subscriber: dexonsmith.
rnk added a comment.

Personally, I think it's safe to do this. I believe this "4.2.1" compat thing was something Apple added as part of their switch from GCC, and then llvm-gcc, over to Clang, and I think most of the code they care about probably doesn't rely on `__VERSION__` anymore. In any case, we should let @dexonsmith know.

As an alternative, maybe we should just stop defining the `__VERSION__` macro. It's ambiguous. What is it the version of? The compiler, or the standard library? What gives the compiler the right to take some identifier, even in the implementer's namespace, and use it without putting it in their namespace by adding `clang` or `GNUC` to it? We already define `__clang_version__`, which has the same information.



================
Comment at: lib/Frontend/InitPreprocessor.cpp:610
+  Builder.defineMacro("__VERSION__", "\"" CLANG_VERSION_STRING
+                      " Compatible " + Twine(getClangFullCPPVersion()) +
+                      "\"");
----------------
sylvestre.ledru wrote:
> lebedev.ri wrote:
> > So how does the entire `__VERSION__` macro looks like now?
> ```
> #define __VERSION__ "9.0.0 Compatible Clang 9.0.0 (trunk 362877)"
> 
> ```
> instead of
> 
> ```
> #define __VERSION__ "4.2.1 Compatible Clang 9.0.0 (trunk 362877)"
> ```
> 
> 
> 
> ```
> clang -dM -E -xc - < /dev/null 
> ```
> to see it
If we're going to make this change, I don't think we need to say `$VER Compatible Clang $VER`, we can just say `Clang $VER (trunk $REVISION)`, so this can be simplified to `getClangFullCPPVersion()`. For a vendor like Apple, it will become `Apple Clang N.M.Q`, which is probably fine. @dexonsmith 


Repository:
  rC Clang

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D63048/new/

https://reviews.llvm.org/D63048