[PATCH] Driver: bifurcate extended and basic MSC versioning
Aaron Ballman
aaron.ballman at gmail.com
Tue Jul 1 08:58:26 PDT 2014
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.
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]>,
> 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.
> 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.
> }
>
> 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
More information about the cfe-commits
mailing list