[PATCH] Driver: bifurcate extended and basic MSC versioning
Saleem Abdulrasool
abdulras at fb.com
Tue Jul 1 12:00:48 PDT 2014
On 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.
Didn’t see this message before updating the patch (silly email).
Yes, in fact, I did exactly that :-).
> 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?
Yes, updated it to ExtendedVersion.
>> 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]>,
>> 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.
Sure, what would you like for the help message? I don’t really mind what the text is, as long as it is useful to the *user*.
>> def fdelayed_template_parsing : Flag<["-"], "fdelayed-template-parsing">, Group<f_Group>,
>> HelpText<"Parse templated function definitions at the end of the "
>> "translation unit">, Flags<[CC1Option]>;
>> Index: lib/Basic/Targets.cpp
>> ===================================================================
>> --- lib/Basic/Targets.cpp
>> +++ lib/Basic/Targets.cpp
>> @@ -587,11 +587,13 @@
>> if (Opts.POSIXThreads)
>> Builder.defineMacro("_MT");
>>
>> - if (Opts.MSCVersion != 0) {
>> - Builder.defineMacro("_MSC_VER", Twine(Opts.MSCVersion / 100000));
>> - Builder.defineMacro("_MSC_FULL_VER", Twine(Opts.MSCVersion));
>> + if (Opts.MSCVersionEx) {
>> + Builder.defineMacro("_MSC_VER", Twine(Opts.MSCVersionEx / 100000));
>> + Builder.defineMacro("_MSC_FULL_VER", Twine(Opts.MSCVersionEx));
>> // FIXME We cannot encode the revision information into 32-bits
>> Builder.defineMacro("_MSC_BUILD", Twine(1));
>> + } else if (Opts.MSCVersion != 0) {
>
> Can drop the != 0 like you do for the Ex version.
>
>> + Builder.defineMacro("_MSC_VER", Twine(Opts.MSCVersion));
>
> Can we not set the full version as well (just missing the information
> we do not have). Same for _MSC_BUILD.
>
> I think it would be surprising for one of these to exist, but not all three.
Since we normalize now, this would be pretty difficult.
>> }
>>
>> if (Opts.MicrosoftExt) {
>> Index: lib/Driver/Tools.cpp
>> ===================================================================
>> --- lib/Driver/Tools.cpp
>> +++ lib/Driver/Tools.cpp
>> @@ -3723,14 +3723,29 @@
>> true))))
>> CmdArgs.push_back("-fms-compatibility");
>>
>> - // -fmsc-version=1700 is default.
>> + // -fmsc-version-ex=17.00 is default.
>> if (Args.hasFlag(options::OPT_fms_extensions, options::OPT_fno_ms_extensions,
>> - IsWindowsMSVC) || Args.hasArg(options::OPT_fmsc_version)) {
>> - StringRef msc_ver = Args.getLastArgValue(options::OPT_fmsc_version);
>> - if (msc_ver.empty())
>> - CmdArgs.push_back("-fmsc-version=1700");
>> - else
>> - CmdArgs.push_back(Args.MakeArgString("-fmsc-version=" + msc_ver));
>> + IsWindowsMSVC) || Args.hasArg(options::OPT_fmsc_version) ||
>> + Args.hasArg(options::OPT_fmsc_version_ex)) {
>> + if (Args.hasArg(options::OPT_fmsc_version) &&
>> + Args.hasArg(options::OPT_fmsc_version_ex))
>> + D.Diag(diag::err_drv_argument_not_allowed_with)
>> + << Args.getArgString(options::OPT_fmsc_version)
>> + << Args.getArgString(options::OPT_fmsc_version_ex);
>> +
>> + if (Args.hasArg(options::OPT_fmsc_version_ex)) {
>> + StringRef MSCVerEx = Args.getLastArgValue(options::OPT_fmsc_version_ex);
>> + if (MSCVerEx.empty())
>> + CmdArgs.push_back("-fmsc-version-ex=17.00");
>> + else
>> + CmdArgs.push_back(Args.MakeArgString("-fmsc-version-ex=" + MSCVerEx));
>> + } else {
>> + StringRef MSCVer = Args.getLastArgValue(options::OPT_fmsc_version);
>> + if (MSCVer.empty())
>> + CmdArgs.push_back("-fmsc-version-ex=17.00");
>> + else
>> + CmdArgs.push_back(Args.MakeArgString("-fmsc-version=" + MSCVer));
>> + }
>> }
>>
>>
>> Index: lib/Frontend/CompilerInvocation.cpp
>> ===================================================================
>> --- lib/Frontend/CompilerInvocation.cpp
>> +++ lib/Frontend/CompilerInvocation.cpp
>> @@ -1208,7 +1208,7 @@
>> }
>>
>> static unsigned parseMSCVersion(ArgList &Args, DiagnosticsEngine &Diags) {
>> - auto Arg = Args.getLastArg(OPT_fmsc_version);
>> + auto Arg = Args.getLastArg(OPT_fmsc_version_ex);
>> if (!Arg)
>> return 0;
>>
>> @@ -1225,25 +1225,8 @@
>> // Unfortunately, due to the bit-width limitations, we cannot currently encode
>> // the value for the patch level.
>>
>> - StringRef Value = Arg->getValue();
>> -
>> - // parse the compatible old form of _MSC_VER or the newer _MSC_FULL_VER
>> - if (Value.find('.') == StringRef::npos) {
>> - unsigned Version = 0;
>> - if (Value.getAsInteger(10, Version)) {
>> - Diags.Report(diag::err_drv_invalid_value)
>> - << Arg->getAsString(Args) << Value;
>> - return 0;
>> - }
>> - if (Version < 100)
>> - Version = Version * 100; // major -> major.minor
>> - if (Version < 100000)
>> - Version = Version * 100000; // major.minor -> major.minor.build
>> - return Version;
>> - }
>> -
>> - // parse the dot-delimited component version
>> unsigned VC[4] = {0};
>> + StringRef Value = Arg->getValue();
>> SmallVector<StringRef, 4> Components;
>>
>> Value.split(Components, ".", llvm::array_lengthof(VC));
>> @@ -1430,7 +1413,8 @@
>> Opts.MSVCCompat = Args.hasArg(OPT_fms_compatibility);
>> Opts.MicrosoftExt = Opts.MSVCCompat || Args.hasArg(OPT_fms_extensions);
>> Opts.AsmBlocks = Args.hasArg(OPT_fasm_blocks) || Opts.MicrosoftExt;
>> - Opts.MSCVersion = parseMSCVersion(Args, Diags);
>> + Opts.MSCVersion = getLastArgIntValue(Args, OPT_fmsc_version, 0, Diags);
>> + Opts.MSCVersionEx = parseMSCVersion(Args, Diags);
>> Opts.VtorDispMode = getLastArgIntValue(Args, OPT_vtordisp_mode_EQ, 1, Diags);
>> Opts.Borland = Args.hasArg(OPT_fborland_extensions);
>> Opts.WritableStrings = Args.hasArg(OPT_fwritable_strings);
>> Index: lib/Frontend/TextDiagnostic.cpp
>> ===================================================================
>> --- lib/Frontend/TextDiagnostic.cpp
>> +++ lib/Frontend/TextDiagnostic.cpp
>> @@ -808,7 +808,8 @@
>> if (DiagOpts->getFormat() == DiagnosticOptions::Msvc) {
>> OS << ',';
>> // Visual Studio 2010 or earlier expects column number to be off by one
>> - if (LangOpts.MSCVersion && LangOpts.MSCVersion < 170000000)
>> + if ((LangOpts.MSCVersion && LangOpts.MSCVersion < 1700) ||
>> + (LangOpts.MSCVersionEx && LangOpts.MSCVersionEx < 170000000))
>> ColNo--;
>> } else
>> OS << ':';
>> Index: test/Driver/msc-version.c
>> ===================================================================
>> --- test/Driver/msc-version.c
>> +++ test/Driver/msc-version.c
>> @@ -4,37 +4,25 @@
>> // 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
>> -
>> -// RUN: %clang -target i686-windows -fms-compatibility -fmsc-version=14 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION-MAJOR
>> +// RUN: %clang -target i686-windows -fms-compatibility -fmsc-version-ex=14 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION-MAJOR
>>
>> // CHECK-MSC-VERSION-MAJOR: _MSC_BUILD 1
>> // CHECK-MSC-VERSION-MAJOR: _MSC_FULL_VER 140000000
>> // CHECK-MSC-VERSION-MAJOR: _MSC_VER 1400
>>
>> -// RUN: %clang -target i686-windows -fms-compatibility -fmsc-version=17.00 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION-MAJOR-MINOR
>> +// RUN: %clang -target i686-windows -fms-compatibility -fmsc-version-ex=15.00 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION-MAJOR-MINOR
>>
>> // CHECK-MSC-VERSION-MAJOR-MINOR: _MSC_BUILD 1
>> -// CHECK-MSC-VERSION-MAJOR-MINOR: _MSC_FULL_VER 170000000
>> -// CHECK-MSC-VERSION-MAJOR-MINOR: _MSC_VER 1700
>> +// CHECK-MSC-VERSION-MAJOR-MINOR: _MSC_FULL_VER 150000000
>> +// CHECK-MSC-VERSION-MAJOR-MINOR: _MSC_VER 1500
>>
>> -// RUN: %clang -target i686-windows -fms-compatibility -fmsc-version=15.00.20706 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION-MAJOR-MINOR-BUILD
>> +// RUN: %clang -target i686-windows -fms-compatibility -fmsc-version-ex=15.00.20706 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION-MAJOR-MINOR-BUILD
>>
>> // CHECK-MSC-VERSION-MAJOR-MINOR-BUILD: _MSC_BUILD 1
>> // CHECK-MSC-VERSION-MAJOR-MINOR-BUILD: _MSC_FULL_VER 150020706
>> // CHECK-MSC-VERSION-MAJOR-MINOR-BUILD: _MSC_VER 1500
>>
>> -// RUN: %clang -target i686-windows -fms-compatibility -fmsc-version=15.00.20706.01 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION-MAJOR-MINOR-BUILD-PATCH
>> +// RUN: %clang -target i686-windows -fms-compatibility -fmsc-version-ex=15.00.20706.01 -dM -E - </dev/null -o - | FileCheck %s -check-prefix CHECK-MSC-VERSION-MAJOR-MINOR-BUILD-PATCH
>>
>> // CHECK-MSC-VERSION-MAJOR-MINOR-BUILD-PATCH: _MSC_BUILD 1
>> // CHECK-MSC-VERSION-MAJOR-MINOR-BUILD-PATCH: _MSC_FULL_VER 150020706
>> Index: test/Misc/diag-format.c
>> ===================================================================
>> --- test/Misc/diag-format.c
>> +++ test/Misc/diag-format.c
>> @@ -3,10 +3,13 @@
>> // RUN: %clang -fsyntax-only -fdiagnostics-format=clang -target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=DEFAULT
>> //
>> // RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
>> +// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version-ex=13.00 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
>> // RUN: %clang -fsyntax-only -fdiagnostics-format=msvc %s 2>&1 | FileCheck %s -check-prefix=MSVC
>> // RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 -target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
>> +// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version-ex=13.00 -target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
>> // RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 %s 2>&1 | FileCheck %s -check-prefix=MSVC
>> // RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version=1300 -target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
>> +// RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -fmsc-version-ex=13.00 -target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s -check-prefix=MSVC2010
>> // RUN: %clang -fsyntax-only -fdiagnostics-format=msvc -target x86_64-pc-win32 -fshow-column %s 2>&1 | FileCheck %s -check-prefix=MSVC
>> //
>> // RUN: %clang -fsyntax-only -fdiagnostics-format=vi %s 2>&1 | FileCheck %s -check-prefix=VI
>> @@ -16,6 +19,7 @@
>> // RUN: %clang -fsyntax-only -fno-show-column %s 2>&1 | FileCheck %s -check-prefix=NO_COLUMN
>> //
>> // RUN: not %clang -fsyntax-only -Werror -fdiagnostics-format=msvc-fallback -fmsc-version=1300 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010-FALLBACK
>> +// RUN: not %clang -fsyntax-only -Werror -fdiagnostics-format=msvc-fallback -fmsc-version-ex=13.00 %s 2>&1 | FileCheck %s -check-prefix=MSVC2010-FALLBACK
>> // RUN: not %clang -fsyntax-only -Werror -fdiagnostics-format=msvc-fallback %s 2>&1 | FileCheck %s -check-prefix=MSVC-FALLBACK
>>
>>
>> @@ -30,12 +34,12 @@
>>
>> #ifdef foo
>> #endif bad // extension!
>> -// DEFAULT: {{.*}}:32:8: warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> -// MSVC2010: {{.*}}(32,7) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> -// MSVC: {{.*}}(32,8) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> -// VI: {{.*}} +32:8: warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> -// MSVC_ORIG: {{.*}}(32) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> -// NO_COLUMN: {{.*}}:32: warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> -// MSVC2010-FALLBACK: {{.*}}(32,7) : error(clang): extra tokens at end of #endif directive
>> -// MSVC-FALLBACK: {{.*}}(32,8) : error(clang): extra tokens at end of #endif directive
>> +// DEFAULT: {{.*}}:36:8: warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> +// MSVC2010: {{.*}}(36,7) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> +// MSVC: {{.*}}(36,8) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> +// VI: {{.*}} +36:8: warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> +// MSVC_ORIG: {{.*}}(36) : warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> +// NO_COLUMN: {{.*}}:36: warning: extra tokens at end of #endif directive [-Wextra-tokens]
>> +// MSVC2010-FALLBACK: {{.*}}(36,7) : error(clang): extra tokens at end of #endif directive
>> +// MSVC-FALLBACK: {{.*}}(36,8) : error(clang): extra tokens at end of #endif directive
>> int x;
>>
>
> ~Aaron
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> https://urldefense.proofpoint.com/v1/url?u=http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits&k=ZVNjlDMF0FElm4dQtryO4A%3D%3D%0A&r=CchYc4lrV44%2BZqxZADw0BQ%3D%3D%0A&m=MogsdnyfVQPpWg7ij3owcO%2BwL%2FvaJiVrYBKlSvFOeWM%3D%0A&s=708a8c12c91d433880e4684e181e83565c2723dfb0e28018750d6b8333b53e6f
--
Saleem Abdulrasool
abdulras (at) fb (dot) com
More information about the cfe-commits
mailing list