[PATCH] MS ABI: Add support for the -vm{b,g,s,m,v} flags
Aaron Ballman
aaron.ballman at gmail.com
Tue Feb 11 12:27:32 PST 2014
Tiny nits below.
> Index: include/clang/Basic/LangOptions.def
> ===================================================================
> --- include/clang/Basic/LangOptions.def
> +++ include/clang/Basic/LangOptions.def
> @@ -113,6 +113,7 @@
> BENIGN_LANGOPT(AccessControl , 1, 1, "C++ access control")
> LANGOPT(CharIsSigned , 1, 1, "signed char")
> LANGOPT(ShortWChar , 1, 0, "unsigned short wchar_t")
> +ENUM_LANGOPT(MSPointerToMemberRepresentationMethod, PragmaMSPointersToMembersKind, 2, PPTMK_BestCase, "member-pointer representation method")
>
> LANGOPT(ShortEnums , 1, 0, "short enum types")
>
> Index: include/clang/Basic/LangOptions.h
> ===================================================================
> --- include/clang/Basic/LangOptions.h
> +++ include/clang/Basic/LangOptions.h
> @@ -66,6 +66,13 @@
> SOB_Trapping // -ftrapv
> };
>
> + enum PragmaMSPointersToMembersKind {
> + PPTMK_BestCase,
> + PPTMK_FullGeneralitySingleInheritance,
> + PPTMK_FullGeneralityMultipleInheritance,
> + PPTMK_FullGeneralityVirtualInheritance,
> + };
> +
> enum AddrSpaceMapMangling { ASMM_Target, ASMM_On, ASMM_Off };
>
> public:
> Index: include/clang/Driver/CLCompatOptions.td
> ===================================================================
> --- include/clang/Driver/CLCompatOptions.td
> +++ include/clang/Driver/CLCompatOptions.td
> @@ -96,6 +96,16 @@
> Alias<show_includes>;
> def _SLASH_U : CLJoinedOrSeparate<"U">, HelpText<"Undefine macro">,
> MetaVarName<"<macro>">, Alias<U>;
> +def _SLASH_vmb : CLFlag<"vmb">,
> + HelpText<"Use a best case representation method for member pointers">;
It should be best-case instead of best case.
> +def _SLASH_vmg : CLFlag<"vmg">,
> + HelpText<"Use a most general representation for member pointers">;
most-general instead of most general. Same goes below.
> +def _SLASH_vms : CLFlag<"vms">,
> + HelpText<"Set the default most general representation to single inheritance">;
> +def _SLASH_vmm : CLFlag<"vmm">,
> + HelpText<"Set the default most general representation to multiple inheritance">;
> +def _SLASH_vmv : CLFlag<"vmv">,
> + HelpText<"Set the default most general representation to virtual inheritance">;
> def _SLASH_W0 : CLFlag<"W0">, HelpText<"Disable all warnings">, Alias<w>;
> def _SLASH_W1 : CLFlag<"W1">, HelpText<"Enable -Wall">, Alias<Wall>;
> def _SLASH_W2 : CLFlag<"W2">, HelpText<"Enable -Wall">, Alias<Wall>;
> @@ -168,7 +178,6 @@
> def _SLASH_RTC : CLIgnoredJoined<"RTC">;
> def _SLASH_sdl : CLIgnoredFlag<"sdl">;
> def _SLASH_sdl_ : CLIgnoredFlag<"sdl-">;
> -def _SLASH_vmg : CLIgnoredFlag<"vmg">;
> def _SLASH_w : CLIgnoredJoined<"w">;
> def _SLASH_Zc_forScope : CLIgnoredFlag<"Zc:forScope">;
> def _SLASH_Zc_wchar_t : CLIgnoredFlag<"Zc:wchar_t">;
> @@ -233,10 +242,6 @@
> def _SLASH_u : CLFlag<"u">;
> def _SLASH_V : CLFlag<"V">;
> def _SLASH_vd : CLJoined<"vd">;
> -def _SLASH_vmb : CLFlag<"vmb">;
> -def _SLASH_vmm : CLFlag<"vmm">;
> -def _SLASH_vms : CLFlag<"vms">;
> -def _SLASH_vmv : CLFlag<"vmv">;
> def _SLASH_volatile : CLFlag<"volatile">;
> def _SLASH_WL : CLFlag<"WL">;
> def _SLASH_Wp64 : CLFlag<"Wp64">;
> Index: include/clang/Driver/Options.td
> ===================================================================
> --- include/clang/Driver/Options.td
> +++ include/clang/Driver/Options.td
> @@ -571,6 +571,7 @@
> 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]>;
> +def fms_memptr_rep_EQ : Joined<["-"], "fms-memptr-rep=">, Group<f_Group>, Flags<[CC1Option]>;
> def fmodules_cache_path : Joined<["-"], "fmodules-cache-path=">, Group<i_Group>,
> Flags<[DriverOption, CC1Option]>, MetaVarName<"<directory>">,
> HelpText<"Specify the module cache path">;
> Index: include/clang/Sema/Sema.h
> ===================================================================
> --- include/clang/Sema/Sema.h
> +++ include/clang/Sema/Sema.h
> @@ -262,15 +262,9 @@
>
> bool MSStructPragmaOn; // True when \#pragma ms_struct on
>
> - enum PragmaMSPointersToMembersKind {
> - PPTMK_BestCase,
> - PPTMK_FullGeneralitySingleInheritance,
> - PPTMK_FullGeneralityMultipleInheritance,
> - PPTMK_FullGeneralityVirtualInheritance,
> - };
> -
> /// \brief Controls member pointer representation format under the MS ABI.
> - PragmaMSPointersToMembersKind MSPointerToMemberRepresentationMethod;
> + LangOptions::PragmaMSPointersToMembersKind
> + MSPointerToMemberRepresentationMethod;
This looks a bit odd; should MSPointerToMemberRepresentationMethod be indented?
>
> /// \brief Source location for newly created implicit MSInheritanceAttrs
> SourceLocation ImplicitMSInheritanceAttrLoc;
> @@ -6986,8 +6980,9 @@
> /// ActOnPragmaMSPointersToMembers - called on well formed \#pragma
> /// pointers_to_members(representation method[, general purpose
> /// representation]).
> - void ActOnPragmaMSPointersToMembers(PragmaMSPointersToMembersKind Kind,
> - SourceLocation PragmaLoc);
> + void ActOnPragmaMSPointersToMembers(
> + LangOptions::PragmaMSPointersToMembersKind Kind,
> + SourceLocation PragmaLoc);
>
> /// ActOnPragmaDetectMismatch - Call on well-formed \#pragma detect_mismatch
> void ActOnPragmaDetectMismatch(StringRef Name, StringRef Value);
> Index: lib/Driver/Tools.cpp
> ===================================================================
> --- lib/Driver/Tools.cpp
> +++ lib/Driver/Tools.cpp
> @@ -4017,6 +4017,36 @@
> if (Arg *A = Args.getLastArg(options::OPT_show_includes))
> A->render(Args, CmdArgs);
>
> + const Driver &D = getToolChain().getDriver();
> + bool HasMostGeneralArg = Args.getLastArg(options::OPT__SLASH_vmg);
> + bool HasBestCaseArg = Args.getLastArg(options::OPT__SLASH_vmb);
> + if (HasMostGeneralArg && HasBestCaseArg)
> + D.Diag(clang::diag::err_drv_argument_not_allowed_with)
> + << "-vmg" << "-vmb";
> +
> + if (HasMostGeneralArg) {
> + bool HasSingleArg = Args.hasArg(options::OPT__SLASH_vms);
> + bool HasMultipleArg = Args.hasArg(options::OPT__SLASH_vmm);
> + bool HasVirtualArg = Args.hasArg(options::OPT__SLASH_vmv);
> +
> + if (HasSingleArg && (HasMultipleArg || HasVirtualArg))
> + D.Diag(clang::diag::err_drv_argument_not_allowed_with)
> + << "-vms" << (HasMultipleArg ? "-vmm" : "-vmv");
> + else if (HasMultipleArg && (HasSingleArg || HasVirtualArg))
> + D.Diag(clang::diag::err_drv_argument_not_allowed_with)
> + << "-vmm" << (HasSingleArg ? "-vms" : "-vmv");
> + else if (HasVirtualArg && (HasSingleArg || HasMultipleArg))
> + D.Diag(clang::diag::err_drv_argument_not_allowed_with)
> + << "-vmv" << (HasSingleArg ? "-vms" : "-vmm");
> +
> + if (HasSingleArg)
> + CmdArgs.push_back("-fms-memptr-rep=single");
> + else if (HasMultipleArg)
> + CmdArgs.push_back("-fms-memptr-rep=multiple");
> + else
> + CmdArgs.push_back("-fms-memptr-rep=virtual");
> + }
> +
> if (!Args.hasArg(options::OPT_fdiagnostics_format_EQ)) {
> CmdArgs.push_back("-fdiagnostics-format");
> if (Args.hasArg(options::OPT__SLASH_fallback))
> Index: lib/Frontend/CompilerInvocation.cpp
> ===================================================================
> --- lib/Frontend/CompilerInvocation.cpp
> +++ lib/Frontend/CompilerInvocation.cpp
> @@ -1402,6 +1402,24 @@
> }
> }
>
> + if (Arg *A = Args.getLastArg(OPT_fms_memptr_rep_EQ)) {
> + LangOptions::PragmaMSPointersToMembersKind InheritanceModel =
> + llvm::StringSwitch<LangOptions::PragmaMSPointersToMembersKind>(
> + A->getValue())
> + .Case("single",
> + LangOptions::PPTMK_FullGeneralitySingleInheritance)
> + .Case("multiple",
> + LangOptions::PPTMK_FullGeneralityMultipleInheritance)
> + .Case("virtual",
> + LangOptions::PPTMK_FullGeneralityVirtualInheritance)
> + .Default(LangOptions::PPTMK_BestCase);
> + if (InheritanceModel == LangOptions::PPTMK_BestCase)
> + Diags.Report(diag::err_drv_invalid_value)
> + << "-fms-memptr-rep=" << A->getValue();
Do we have a test for this?
> +
> + Opts.setMSPointerToMemberRepresentationMethod(InheritanceModel);
> + }
> +
> // Check if -fopenmp is specified.
> Opts.OpenMP = Args.hasArg(OPT_fopenmp);
>
> Index: lib/Parse/ParsePragma.cpp
> ===================================================================
> --- lib/Parse/ParsePragma.cpp
> +++ lib/Parse/ParsePragma.cpp
> @@ -182,8 +182,8 @@
>
> void Parser::HandlePragmaMSPointersToMembers() {
> assert(Tok.is(tok::annot_pragma_ms_pointers_to_members));
> - Sema::PragmaMSPointersToMembersKind RepresentationMethod =
> - static_cast<Sema::PragmaMSPointersToMembersKind>(
> + LangOptions::PragmaMSPointersToMembersKind RepresentationMethod =
> + static_cast<LangOptions::PragmaMSPointersToMembersKind>(
> reinterpret_cast<uintptr_t>(Tok.getAnnotationValue()));
> SourceLocation PragmaLoc = ConsumeToken(); // The annotation token.
> Actions.ActOnPragmaMSPointersToMembers(RepresentationMethod, PragmaLoc);
> @@ -834,9 +834,9 @@
> }
> PP.Lex(Tok);
>
> - Sema::PragmaMSPointersToMembersKind RepresentationMethod;
> + LangOptions::PragmaMSPointersToMembersKind RepresentationMethod;
> if (Arg->isStr("best_case")) {
> - RepresentationMethod = Sema::PPTMK_BestCase;
> + RepresentationMethod = LangOptions::PPTMK_BestCase;
> } else {
> if (Arg->isStr("full_generality")) {
> if (Tok.is(tok::comma)) {
> @@ -854,7 +854,7 @@
> // #pragma pointers_to_members(full_generality) implicitly specifies
> // virtual_inheritance.
> Arg = 0;
> - RepresentationMethod = Sema::PPTMK_FullGeneralityVirtualInheritance;
> + RepresentationMethod = LangOptions::PPTMK_FullGeneralityVirtualInheritance;
> } else {
> PP.Diag(Tok.getLocation(), diag::err_expected_punc)
> << "full_generality";
> @@ -864,11 +864,14 @@
>
> if (Arg) {
> if (Arg->isStr("single_inheritance")) {
> - RepresentationMethod = Sema::PPTMK_FullGeneralitySingleInheritance;
> + RepresentationMethod =
> + LangOptions::PPTMK_FullGeneralitySingleInheritance;
> } else if (Arg->isStr("multiple_inheritance")) {
> - RepresentationMethod = Sema::PPTMK_FullGeneralityMultipleInheritance;
> + RepresentationMethod =
> + LangOptions::PPTMK_FullGeneralityMultipleInheritance;
> } else if (Arg->isStr("virtual_inheritance")) {
> - RepresentationMethod = Sema::PPTMK_FullGeneralityVirtualInheritance;
> + RepresentationMethod =
> + LangOptions::PPTMK_FullGeneralityVirtualInheritance;
> } else {
> PP.Diag(Tok.getLocation(),
> diag::err_pragma_pointers_to_members_unknown_kind)
> Index: lib/Sema/Sema.cpp
> ===================================================================
> --- lib/Sema/Sema.cpp
> +++ lib/Sema/Sema.cpp
> @@ -76,7 +76,9 @@
> CollectStats(false), CodeCompleter(CodeCompleter),
> CurContext(0), OriginalLexicalContext(0),
> PackContext(0), MSStructPragmaOn(false),
> - MSPointerToMemberRepresentationMethod(PPTMK_BestCase), VisContext(0),
> + MSPointerToMemberRepresentationMethod(
> + pp.getLangOpts().getMSPointerToMemberRepresentationMethod()),
> + VisContext(0),
> IsBuildingRecoveryCallExpr(false),
> ExprNeedsCleanups(false), LateTemplateParser(0), OpaqueParser(0),
> IdResolver(pp), StdInitializerList(0), CXXTypeInfoDecl(0), MSVCGuidDecl(0),
> Index: lib/Sema/SemaAttr.cpp
> ===================================================================
> --- lib/Sema/SemaAttr.cpp
> +++ lib/Sema/SemaAttr.cpp
> @@ -288,7 +288,7 @@
> }
>
> void Sema::ActOnPragmaMSPointersToMembers(
> - PragmaMSPointersToMembersKind RepresentationMethod,
> + LangOptions::PragmaMSPointersToMembersKind RepresentationMethod,
> SourceLocation PragmaLoc) {
> MSPointerToMemberRepresentationMethod = RepresentationMethod;
> ImplicitMSInheritanceAttrLoc = PragmaLoc;
> Index: lib/Sema/SemaType.cpp
> ===================================================================
> --- lib/Sema/SemaType.cpp
> +++ lib/Sema/SemaType.cpp
> @@ -5087,26 +5087,26 @@
> MSInheritanceAttr::Spelling InheritanceModel;
>
> switch (MSPointerToMemberRepresentationMethod) {
> - case PPTMK_BestCase:
> + case LangOptions::PPTMK_BestCase:
> InheritanceModel = RD->calculateInheritanceModel();
> break;
> - case PPTMK_FullGeneralitySingleInheritance:
> + case LangOptions::PPTMK_FullGeneralitySingleInheritance:
> InheritanceModel = MSInheritanceAttr::Keyword_single_inheritance;
> break;
> - case PPTMK_FullGeneralityMultipleInheritance:
> + case LangOptions::PPTMK_FullGeneralityMultipleInheritance:
> InheritanceModel =
> MSInheritanceAttr::Keyword_multiple_inheritance;
> break;
> - case PPTMK_FullGeneralityVirtualInheritance:
> + case LangOptions::PPTMK_FullGeneralityVirtualInheritance:
> InheritanceModel =
> MSInheritanceAttr::Keyword_unspecified_inheritance;
> break;
> }
>
> RD->addAttr(MSInheritanceAttr::CreateImplicit(
> getASTContext(), InheritanceModel,
> /*BestCase=*/MSPointerToMemberRepresentationMethod ==
> - PPTMK_BestCase,
> + LangOptions::PPTMK_BestCase,
> ImplicitMSInheritanceAttrLoc.isValid()
> ? ImplicitMSInheritanceAttrLoc
> : RD->getSourceRange()));
> Index: test/Driver/cl-options.c
> ===================================================================
> --- test/Driver/cl-options.c
> +++ test/Driver/cl-options.c
> @@ -67,6 +67,24 @@
> // RUN: %clang_cl /U mymacro -### -- %s 2>&1 | FileCheck -check-prefix=U %s
> // U: "-U" "mymacro"
>
> +// RUN: %clang_cl /vmg -### -- %s 2>&1 | FileCheck -check-prefix=VMG %s
> +// VMG: "-fms-memptr-rep=virtual"
> +
> +// RUN: %clang_cl /vmg /vms -### -- %s 2>&1 | FileCheck -check-prefix=VMS %s
> +// VMS: "-fms-memptr-rep=single"
> +
> +// RUN: %clang_cl /vmg /vmm -### -- %s 2>&1 | FileCheck -check-prefix=VMM %s
> +// VMM: "-fms-memptr-rep=multiple"
> +
> +// RUN: %clang_cl /vmg /vmv -### -- %s 2>&1 | FileCheck -check-prefix=VMV %s
> +// VMV: "-fms-memptr-rep=virtual"
> +
> +// RUN: %clang_cl /vmg /vmb -### -- %s 2>&1 | FileCheck -check-prefix=VMB %s
> +// VMB: '-vmg' not allowed with '-vmb'
> +
> +// RUN: %clang_cl /vmg /vmm /vms -### -- %s 2>&1 | FileCheck -check-prefix=VMX %s
> +// VMX: '-vms' not allowed with '-vmm'
> +
> // RUN: %clang_cl /W0 -### -- %s 2>&1 | FileCheck -check-prefix=W0 %s
> // W0: -w
>
> @@ -197,10 +215,6 @@
> // RUN: /u \
> // RUN: /V \
> // RUN: /vd2 \
> -// RUN: /vmb \
> -// RUN: /vmm \
> -// RUN: /vms \
> -// RUN: /vmv \
> // RUN: /volatile \
> // RUN: /wfoo \
> // RUN: /WL \
> Index: test/SemaCXX/member-pointer-ms.cpp
> ===================================================================
> --- test/SemaCXX/member-pointer-ms.cpp
> +++ test/SemaCXX/member-pointer-ms.cpp
> @@ -1,5 +1,6 @@
> -// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -triple=i386-pc-win32 -verify %s
> -// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -triple=x86_64-pc-win32 -verify %s
> +// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -triple=i386-pc-win32 -verify -DVMB %s
> +// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -triple=x86_64-pc-win32 -verify -DVMB %s
> +// RUN: %clang_cc1 -std=c++11 -fms-compatibility -fsyntax-only -triple=x86_64-pc-win32 -verify -DVMV -fms-memptr-rep=virtual %s
> //
> // This file should also give no diagnostics when run through cl.exe from MSVS
> // 2012, which supports C++11 and static_assert. It should pass for both 64-bit
> @@ -18,6 +19,7 @@
> int f;
> };
>
> +#ifdef VMB
> enum {
> kSingleDataAlign = 1 * sizeof(int),
> kSingleFunctionAlign = 1 * sizeof(void *),
> @@ -43,11 +45,47 @@
> kUnspecifiedDataSize = 3 * sizeof(int),
> kUnspecifiedFunctionSize = 2 * sizeof(int) + 2 * sizeof(void *),
> };
> +#elif VMV
> +enum {
> + // Everything with more than 1 field is 8 byte aligned, except virtual data
> + // member pointers on x64 (ugh).
> +#ifdef _M_X64
> + kVirtualDataAlign = 4,
> +#else
> + kVirtualDataAlign = 8,
> +#endif
> + kMultipleDataAlign = kVirtualDataAlign,
> + kSingleDataAlign = kVirtualDataAlign,
> +
> + kUnspecifiedFunctionAlign = 8,
> + kVirtualFunctionAlign = kUnspecifiedFunctionAlign,
> + kMultipleFunctionAlign = kUnspecifiedFunctionAlign,
> + kSingleFunctionAlign = kUnspecifiedFunctionAlign,
> +
> + kUnspecifiedDataSize = 3 * sizeof(int),
> + kVirtualDataSize = kUnspecifiedDataSize,
> + kMultipleDataSize = kUnspecifiedDataSize,
> + kSingleDataSize = kUnspecifiedDataSize,
> +
> + kUnspecifiedFunctionSize = 2 * sizeof(int) + 2 * sizeof(void *),
> + kVirtualFunctionSize = kUnspecifiedFunctionSize,
> + kMultipleFunctionSize = kUnspecifiedFunctionSize,
> + kSingleFunctionSize = kUnspecifiedFunctionSize,
> +};
> +#else
> +#error "test doesn't yet support this mode!"
> +#endif
>
> // incomplete types
> +#ifdef VMB
> class __single_inheritance IncSingle;
> class __multiple_inheritance IncMultiple;
> class __virtual_inheritance IncVirtual;
> +#else
> +class IncSingle;
> +class IncMultiple;
> +class IncVirtual;
> +#endif
> static_assert(sizeof(int IncSingle::*) == kSingleDataSize, "");
> static_assert(sizeof(int IncMultiple::*) == kMultipleDataSize, "");
> static_assert(sizeof(int IncVirtual::*) == kVirtualDataSize, "");
> @@ -83,9 +121,15 @@
>
> // Test both declared and defined templates.
> template <typename T> class X;
> +#ifdef VMB
> template <> class __single_inheritance X<IncSingle>;
> template <> class __multiple_inheritance X<IncMultiple>;
> template <> class __virtual_inheritance X<IncVirtual>;
> +#else
> +template <> class X<IncSingle>;
> +template <> class X<IncMultiple>;
> +template <> class X<IncVirtual>;
> +#endif
> // Don't declare X<IncUnspecified>.
> static_assert(sizeof(int X<IncSingle>::*) == kSingleDataSize, "");
> static_assert(sizeof(int X<IncMultiple>::*) == kMultipleDataSize, "");
> @@ -183,8 +227,10 @@
> void (T::*func_ptr)();
> };
>
> +#ifdef VMB
> int Virtual::*CastTest = reinterpret_cast<int Virtual::*>(&AA::x);
> // expected-error at -1 {{cannot reinterpret_cast from member pointer type}}
> +#endif
>
> namespace ErrorTest {
> template <typename T, typename U> struct __single_inheritance A;
> @@ -202,7 +248,7 @@
> struct __virtual_inheritance D;
> struct D : virtual B {};
> }
> -
> +#ifdef VMB
> #pragma pointers_to_members(full_generality, multiple_inheritance)
> struct TrulySingleInheritance;
> static_assert(sizeof(int TrulySingleInheritance::*) == kMultipleDataSize, "");
> @@ -225,3 +271,4 @@
> static_assert(sizeof(int SingleInheritanceAsVirtualBeforePragma::*) == 12, "");
>
> #pragma pointers_to_members(single) // expected-error{{unexpected 'single'}}
> +#endif
>
~Aaron
On Tue, Feb 11, 2014 at 3:20 PM, David Majnemer
<david.majnemer at gmail.com> wrote:
> - Address review comments
>
> Hi hansw, aaron.ballman, rnk, rsmith,
>
> http://llvm-reviews.chandlerc.com/D2741
>
> CHANGE SINCE LAST DIFF
> http://llvm-reviews.chandlerc.com/D2741?vs=6997&id=7001#toc
>
> Files:
> include/clang/Basic/LangOptions.def
> include/clang/Basic/LangOptions.h
> include/clang/Driver/CLCompatOptions.td
> include/clang/Driver/Options.td
> include/clang/Sema/Sema.h
> lib/Driver/Tools.cpp
> lib/Frontend/CompilerInvocation.cpp
> lib/Parse/ParsePragma.cpp
> lib/Sema/Sema.cpp
> lib/Sema/SemaAttr.cpp
> lib/Sema/SemaType.cpp
> test/Driver/cl-options.c
> test/SemaCXX/member-pointer-ms.cpp
More information about the cfe-commits
mailing list