[PATCH] Driver: enhance MSC version compatibility

[Aaron Ballman]

Minor thoughts, but overall, I like this approach.

On Fri, Jun 20, 2014 at 1:36 PM, Saleem Abdulrasool
<compnerd at compnerd.org> wrote:
> Hi rnk,
>
> The version information for Visual Studio is spread over multiple variables.
> The newer Windows SDK has started making use of some of the extended versioning
> variables that were previously undefined.  Enhance our compatibility definitions
> for these cases.
>
> _MSC_VER is defined to be the Major * 100 + Minor.  _MSC_FULL_VER is defined to
> be Major * 10000000 + Minor * 100000 + Build.  And _MSC_BUILD is the build
> revision of the compiler.
>
> Extend the -fmsc-version option in a compatible manner.  If the value is the
> previous form of MMmm, then we assume that the build number is 0.  Otherwise, a
> specific build number may be passed by using the form MMmmbbbbb.  Due to
> bitwidth limitations of the option, it is currently not possible to define a
> revision value.
>
> http://reviews.llvm.org/D4233
>
> Files:
>   lib/Basic/Targets.cpp
>   test/Driver/msc-version.c
>
> Index: lib/Basic/Targets.cpp
> ===================================================================
> --- lib/Basic/Targets.cpp
> +++ lib/Basic/Targets.cpp
> @@ -569,6 +569,39 @@
>                      MacroBuilder &Builder) const override {
>      Builder.defineMacro("_WIN32");
>    }
> +  void getMSCVersionDefines(uint32_t MSCVersion, MacroBuilder &Builder) const {
> +    if (MSCVersion == 0)
> +      return;

I prefer the old way where the check happens outside of the API --
basically, callers who don't have an MSCVersion should not expect good
things to happen in here. Hiding that detail away when it could be
asserted seems like a tiny step backwards.

> +
> +    // The MSC versioning scheme involves four versioning components:
> +    //  - Major
> +    //  - Minor
> +    //  - Build
> +    //  - Patch
> +    // Ideally, the -fmsc-version would take a period delimited component of the
> +    // form MM.mm.bbbbb.pp (e.g. 17.00.60610.1).  Unfortunately, the current
> +    // limitation of 32-bits prevents the encoding of Patch.  Assume that patch
> +    // is 0, and permit encodings of MMmmbbbbb or MMmm.
> +
> +    unsigned Major, Minor, Build, Patch;
> +
> +    if (MSCVersion <= 9999) {
> +      Major = MSCVersion / 100;
> +      Minor = MSCVersion % 100;
> +      Build = 0;
> +      Patch = 1;
> +    } else {
> +      Major = (MSCVersion / 100000) / 100;
> +      Minor = (MSCVersion / 100000) % 100;
> +      Build = (MSCVersion % 100000);
> +      Patch = 1;
> +    }

Perhaps:
unsigned ScalingFactor = MSCVersion < 100000 ? 1 : 100000;
Major = (MSCVersion / ScalingFactor) / 100;
Minor = (MSCVersion / ScalingFactor) % 100;
Build = ScalingFactor != 1 ? MSCVersion % ScalingFactor : 0;
Patch = 1; // FIXME: Note about Patch being unencodable

> +
> +    Builder.defineMacro("_MSC_VER", Twine(Major * 100 + Minor));
> +    Builder.defineMacro("_MSC_FULL_VER",
> +                        Twine(Major * 10000000 + Minor * 100000 + Build));
> +    Builder.defineMacro("_MSC_BUILD", Twine(Patch));
> +  }
>    void getVisualStudioDefines(const LangOptions &Opts,
>                                MacroBuilder &Builder) const {
>      if (Opts.CPlusPlus) {
> @@ -587,8 +620,7 @@
>      if (Opts.POSIXThreads)
>        Builder.defineMacro("_MT");
>
> -    if (Opts.MSCVersion != 0)
> -      Builder.defineMacro("_MSC_VER", Twine(Opts.MSCVersion));
> +    getMSCVersionDefines(Opts.MSCVersion, Builder);
>
>      if (Opts.MicrosoftExt) {
>        Builder.defineMacro("_MSC_EXTENSIONS");
> Index: test/Driver/msc-version.c
> ===================================================================
> --- /dev/null
> +++ test/Driver/msc-version.c
> @@ -0,0 +1,18 @@
> +// RUN: %clang -target i686-windows -fms-compatibility -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-NO-MSC-VERSION
> +
> +// CHECK-NO-MSC-VERSION: _MSC_BUILD 1
> +// CHECK-NO-MSC-VERSION: _MSC_FULL_VER 170000000
> +// CHECK-NO-MSC-VERSION: _MSC_VER 1700
> +
> +// RUN: %clang -target i686-windows -fms-compatibility -fmsc-version=1600 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION
> +
> +// CHECK-MSC-VERSION: _MSC_BUILD 1
> +// CHECK-MSC-VERSION: _MSC_FULL_VER 160000000
> +// CHECK-MSC-VERSION: _MSC_VER 1600
> +
> +// RUN: %clang -target i686-windows -fms-compatibility -fmsc-version=160030319 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION-EXT
> +
> +// CHECK-MSC-VERSION-EXT: _MSC_BUILD 1
> +// CHECK-MSC-VERSION-EXT: _MSC_FULL_VER 160030319
> +// CHECK-MSC-VERSION-EXT: _MSC_VER 1600
> +
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>

~Aaron