On Jul 1, 2014, at 8:58 AM, Aaron Ballman <[email protected]> wrote:
> On Tue, Jul 1, 2014 at 1:15 AM, Saleem Abdulrasool > <[email protected]> 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 > [email protected] > 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 _______________________________________________ cfe-commits mailing list [email protected] http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
