[PATCH] Driver: bifurcate extended and basic MSC versioning

[Aaron Ballman]

On Tue, Jul 1, 2014 at 2:50 PM, Reid Kleckner <rnk at google.com> wrote:
> On Tue, Jul 1, 2014 at 8:58 AM, Aaron Ballman <aaron.ballman at gmail.com>
> wrote:
>>
>> On Tue, Jul 1, 2014 at 1:15 AM, Saleem Abdulrasool
>> <compnerd at compnerd.org> wrote:
>> > Hi rnk, aaron.ballman,
>> >
>> > This restores the original behaviour of `-fmsc-version`.  The older
>> > option
>> > remains as a mechanism for specifying the basic version information.  A
>> > secondary option, `-fmsc-version-ex` permits the user to specify an
>> > extended
>> > version to the driver.
>> >
>> > The new version takes the value as a dot-separated value rather than the
>> > major * 100 + minor format that `-fmsc-version` format.  This makes it
>> > easier to
>> > specify the value as well as a more flexible manner for specifying the
>> > value.
>> >
>> > Specifying both values is considered an error (TODO: add test case).
>> > The driver
>> > gives preference to the extended format, defaulting to that format
>> > unless the
>> > old option is specified.
>>
>> > Index: include/clang/Basic/LangOptions.def
>> > ===================================================================
>> > --- include/clang/Basic/LangOptions.def
>> > +++ include/clang/Basic/LangOptions.def
>> > @@ -181,6 +181,7 @@
>> >          "if non-zero, warn about parameter or return Warn if
>> > parameter/return value is larger in bytes than this setting. 0 is no
>> > check.")
>> >  VALUE_LANGOPT(MSCVersion, 32, 0,
>> >                "version of Microsoft Visual C/C++")
>> > +VALUE_LANGOPT(MSCVersionEx, 32, 0, "Microsoft Visual C/C++ Extended
>> > Version")
>>
>> Is there a way we could get away with just one langopt member
>> variable? I'm not keen on seeing code that has to check both variables
>> to get the answer.
>
>
> IMO we should tablegen-ify this, so that we can sort the options by size to
> achieve better packing.  When we do that, we can use uint64_t as the
> bitfield type for options larger than 32 bits.  Until then, two 32-bit
> options is the best we can do.
>
> Besides, do you think anyone will care about the build number in Clang?
> Most language changes are introduced on new releases captured in the major
> version.
>
>>
>> If we do have to keep both variables, I would prefer a better name
>> than "Ex" (though I do enjoy the joke). Also, if both are needed, can
>> we normalize the text to be similar between both options?
>>
>> >  VALUE_LANGOPT(VtorDispMode, 2, 1, "How many vtordisps to insert")
>> >
>> >  LANGOPT(ApplePragmaPack, 1, 0, "Apple gcc-compatible #pragma pack
>> > handling")
>> > Index: include/clang/Driver/Options.td
>> > ===================================================================
>> > --- include/clang/Driver/Options.td
>> > +++ include/clang/Driver/Options.td
>> > @@ -577,6 +577,8 @@
>> >    HelpText<"Enable full Microsoft Visual C++ compatibility">;
>> >  def fmsc_version : Joined<["-"], "fmsc-version=">, Group<f_Group>,
>> > Flags<[CC1Option, CoreOption]>,
>
>
> You can remove this as a -cc1 option probably, now that the driver
> canonicalizes as -fmsc-full-version.
>
>>
>> >    HelpText<"Microsoft compiler version number to report in _MSC_VER (0
>> > = don't define it (default))">;
>> > +def fmsc_version_ex : Joined<["-"], "fmsc-version-ex=">,
>> > Group<f_Group>,
>> > +    Flags<[CC1Option, CoreOption]>, HelpText<"Extended Microsoft
>> > compiler version">;
>>
>> I think the public-facing name needs to change to something more
>> descriptive. How about -fmsc-version-full? Also, I'd like the help
>> text normalized between the two options.
>
>
> +1 for -fmsc-full-version.

What about -fmsc-extended-version?

I ask because Saleem pointed out in IRC that this really isn't the
full version (it doesn't include the build number).

~Aaron