> On Jul 21, 2015, at 5:22 AM, Aaron Ballman <aa...@aaronballman.com> wrote: > > On Mon, Jul 20, 2015 at 6:57 PM, Bob Wilson <bob.wil...@apple.com> wrote: >> Author: bwilson >> Date: Mon Jul 20 17:57:31 2015 >> New Revision: 242730 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=242730&view=rev >> Log: >> Ignore the "novtable" declspec when not using the Microsoft C++ ABI. >> >> Clang used to silently ignore __declspec(novtable). It is implemented >> now, but leaving the vtable uninitialized does not work when using the >> Itanium ABI, where the class layout for complex class hierarchies is >> stored in the vtable. It might be possible to honor the novtable >> attribute in some simple cases and either report an error or ignore >> it in more complex situations, but it’s not clear if that would be >> worthwhile. There is also value in having a simple and predictable >> behavior, so this changes clang to simply ignore novtable when not using >> the Microsoft C++ ABI. >> >> Added: >> cfe/trunk/test/SemaCXX/ms-unsupported.cpp >> Modified: >> cfe/trunk/include/clang/Basic/Attr.td >> cfe/trunk/include/clang/Basic/Attributes.h >> cfe/trunk/include/clang/Sema/AttributeList.h >> cfe/trunk/lib/Basic/Attributes.cpp >> cfe/trunk/lib/Lex/PPMacroExpansion.cpp >> cfe/trunk/lib/Parse/ParseDecl.cpp >> cfe/trunk/lib/Parse/ParseDeclCXX.cpp >> cfe/trunk/lib/Sema/AttributeList.cpp >> cfe/trunk/lib/Sema/SemaDeclAttr.cpp >> cfe/trunk/test/Parser/MicrosoftExtensions.cpp >> cfe/trunk/test/SemaCXX/ms-novtable.cpp >> cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp >> >> Modified: cfe/trunk/include/clang/Basic/Attr.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attr.td?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/Attr.td (original) >> +++ cfe/trunk/include/clang/Basic/Attr.td Mon Jul 20 17:57:31 2015 >> @@ -241,14 +241,18 @@ def COnly : LangOpt<"CPlusPlus", 1>; >> class TargetArch<list<string> arches> { >> list<string> Arches = arches; >> list<string> OSes; >> + list<string> CXXABIs; >> } >> def TargetARM : TargetArch<["arm", "thumb"]>; >> +def TargetMips : TargetArch<["mips", "mipsel"]>; >> def TargetMSP430 : TargetArch<["msp430"]>; >> def TargetX86 : TargetArch<["x86"]>; >> def TargetWindows : TargetArch<["x86", "x86_64", "arm", "thumb"]> { >> let OSes = ["Win32"]; >> } >> -def TargetMips : TargetArch<["mips", "mipsel"]>; >> +def TargetMicrosoftCXXABI : TargetArch<["x86", "x86_64", "arm", "thumb"]> { >> + let CXXABIs = ["Microsoft"]; >> +} >> >> class Attr { >> // The various ways in which an attribute can be spelled in source >> @@ -1820,7 +1824,7 @@ def TypeTagForDatatype : InheritableAttr >> >> // Microsoft-related attributes >> >> -def MSNoVTable : InheritableAttr { >> +def MSNoVTable : InheritableAttr, TargetSpecificAttr<TargetMicrosoftCXXABI> >> { >> let Spellings = [Declspec<"novtable">]; >> let Subjects = SubjectList<[CXXRecord]>; >> let Documentation = [MSNoVTableDocs]; > > Can you also update MSNoVTableDocs to mention this new behavior? > > Thanks! > > ~Aaron
Sure. r242800 > >> >> Modified: cfe/trunk/include/clang/Basic/Attributes.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Attributes.h?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/Attributes.h (original) >> +++ cfe/trunk/include/clang/Basic/Attributes.h Mon Jul 20 17:57:31 2015 >> @@ -11,7 +11,7 @@ >> #define LLVM_CLANG_BASIC_ATTRIBUTES_H >> >> #include "clang/Basic/LangOptions.h" >> -#include "llvm/ADT/Triple.h" >> +#include "clang/Basic/TargetInfo.h" >> >> namespace clang { >> >> @@ -31,7 +31,7 @@ enum class AttrSyntax { >> /// \brief Return the version number associated with the attribute if we >> /// recognize and implement the attribute specified by the given information. >> int hasAttribute(AttrSyntax Syntax, const IdentifierInfo *Scope, >> - const IdentifierInfo *Attr, const llvm::Triple &T, >> + const IdentifierInfo *Attr, const TargetInfo &Target, >> const LangOptions &LangOpts); >> >> } // end namespace clang >> >> Modified: cfe/trunk/include/clang/Sema/AttributeList.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Sema/AttributeList.h (original) >> +++ cfe/trunk/include/clang/Sema/AttributeList.h Mon Jul 20 17:57:31 2015 >> @@ -16,11 +16,11 @@ >> #define LLVM_CLANG_SEMA_ATTRIBUTELIST_H >> >> #include "clang/Basic/SourceLocation.h" >> +#include "clang/Basic/TargetInfo.h" >> #include "clang/Basic/VersionTuple.h" >> #include "clang/Sema/Ownership.h" >> #include "llvm/ADT/PointerUnion.h" >> #include "llvm/ADT/SmallVector.h" >> -#include "llvm/ADT/Triple.h" >> #include "llvm/Support/Allocator.h" >> #include <cassert> >> >> @@ -464,7 +464,7 @@ public: >> bool hasVariadicArg() const; >> bool diagnoseAppertainsTo(class Sema &S, const Decl *D) const; >> bool diagnoseLangOpts(class Sema &S) const; >> - bool existsInTarget(const llvm::Triple &T) const; >> + bool existsInTarget(const TargetInfo &Target) const; >> bool isKnownToGCC() const; >> >> /// \brief If the parsed attribute has a semantic equivalent, and it would >> >> Modified: cfe/trunk/lib/Basic/Attributes.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Attributes.cpp?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Basic/Attributes.cpp (original) >> +++ cfe/trunk/lib/Basic/Attributes.cpp Mon Jul 20 17:57:31 2015 >> @@ -4,8 +4,8 @@ >> using namespace clang; >> >> int clang::hasAttribute(AttrSyntax Syntax, const IdentifierInfo *Scope, >> - const IdentifierInfo *Attr, const llvm::Triple &T, >> - const LangOptions &LangOpts) { >> + const IdentifierInfo *Attr, const TargetInfo >> &Target, >> + const LangOptions &LangOpts) { >> StringRef Name = Attr->getName(); >> // Normalize the attribute name, __foo__ becomes foo. >> if (Name.size() >= 4 && Name.startswith("__") && Name.endswith("__")) >> >> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original) >> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Mon Jul 20 17:57:31 2015 >> @@ -1633,13 +1633,13 @@ void Preprocessor::ExpandBuiltinMacro(To >> Value = FeatureII->getBuiltinID() != 0; >> } else if (II == Ident__has_attribute) >> Value = hasAttribute(AttrSyntax::GNU, nullptr, FeatureII, >> - getTargetInfo().getTriple(), getLangOpts()); >> + getTargetInfo(), getLangOpts()); >> else if (II == Ident__has_cpp_attribute) >> Value = hasAttribute(AttrSyntax::CXX, ScopeII, FeatureII, >> - getTargetInfo().getTriple(), getLangOpts()); >> + getTargetInfo(), getLangOpts()); >> else if (II == Ident__has_declspec) >> Value = hasAttribute(AttrSyntax::Declspec, nullptr, FeatureII, >> - getTargetInfo().getTriple(), getLangOpts()); >> + getTargetInfo(), getLangOpts()); >> else if (II == Ident__has_extension) >> Value = HasExtension(*this, FeatureII); >> else { >> >> Modified: cfe/trunk/lib/Parse/ParseDecl.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDecl.cpp?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Parse/ParseDecl.cpp (original) >> +++ cfe/trunk/lib/Parse/ParseDecl.cpp Mon Jul 20 17:57:31 2015 >> @@ -387,7 +387,7 @@ bool Parser::ParseMicrosoftDeclSpecArgs( >> // If the attribute isn't known, we will not attempt to parse any >> // arguments. >> if (!hasAttribute(AttrSyntax::Declspec, nullptr, AttrName, >> - getTargetInfo().getTriple(), getLangOpts())) { >> + getTargetInfo(), getLangOpts())) { >> // Eat the left paren, then skip to the ending right paren. >> ConsumeParen(); >> SkipUntil(tok::r_paren); >> >> Modified: cfe/trunk/lib/Parse/ParseDeclCXX.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/ParseDeclCXX.cpp?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Parse/ParseDeclCXX.cpp (original) >> +++ cfe/trunk/lib/Parse/ParseDeclCXX.cpp Mon Jul 20 17:57:31 2015 >> @@ -3612,7 +3612,7 @@ bool Parser::ParseCXX11AttributeArgs(Ide >> // If the attribute isn't known, we will not attempt to parse any >> // arguments. >> if (!hasAttribute(AttrSyntax::CXX, ScopeName, AttrName, >> - getTargetInfo().getTriple(), getLangOpts())) { >> + getTargetInfo(), getLangOpts())) { >> // Eat the left paren, then skip to the ending right paren. >> ConsumeParen(); >> SkipUntil(tok::r_paren); >> >> Modified: cfe/trunk/lib/Sema/AttributeList.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/AttributeList.cpp?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/AttributeList.cpp (original) >> +++ cfe/trunk/lib/Sema/AttributeList.cpp Mon Jul 20 17:57:31 2015 >> @@ -17,6 +17,7 @@ >> #include "clang/AST/DeclTemplate.h" >> #include "clang/AST/Expr.h" >> #include "clang/Basic/IdentifierTable.h" >> +#include "clang/Basic/TargetInfo.h" >> #include "clang/Sema/SemaInternal.h" >> #include "llvm/ADT/SmallString.h" >> #include "llvm/ADT/StringSwitch.h" >> @@ -155,7 +156,7 @@ struct ParsedAttrInfo { >> bool (*DiagAppertainsToDecl)(Sema &S, const AttributeList &Attr, >> const Decl *); >> bool (*DiagLangOpts)(Sema &S, const AttributeList &Attr); >> - bool (*ExistsInTarget)(const llvm::Triple &T); >> + bool (*ExistsInTarget)(const TargetInfo &Target); >> unsigned (*SpellingIndexToSemanticSpelling)(const AttributeList &Attr); >> }; >> >> @@ -195,8 +196,8 @@ bool AttributeList::isTypeAttr() const { >> return getInfo(*this).IsType; >> } >> >> -bool AttributeList::existsInTarget(const llvm::Triple &T) const { >> - return getInfo(*this).ExistsInTarget(T); >> +bool AttributeList::existsInTarget(const TargetInfo &Target) const { >> + return getInfo(*this).ExistsInTarget(Target); >> } >> >> bool AttributeList::isKnownToGCC() const { >> >> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Mon Jul 20 17:57:31 2015 >> @@ -4583,7 +4583,7 @@ static void ProcessDeclAttribute(Sema &S >> // which do not apply to the current target architecture are treated as >> // though they were unknown attributes. >> if (Attr.getKind() == AttributeList::UnknownAttribute || >> - !Attr.existsInTarget(S.Context.getTargetInfo().getTriple())) { >> + !Attr.existsInTarget(S.Context.getTargetInfo())) { >> S.Diag(Attr.getLoc(), Attr.isDeclspecAttribute() >> ? diag::warn_unhandled_ms_attribute_ignored >> : diag::warn_unknown_attribute_ignored) >> >> Modified: cfe/trunk/test/Parser/MicrosoftExtensions.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Parser/MicrosoftExtensions.cpp?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/test/Parser/MicrosoftExtensions.cpp (original) >> +++ cfe/trunk/test/Parser/MicrosoftExtensions.cpp Mon Jul 20 17:57:31 2015 >> @@ -1,4 +1,4 @@ >> -// RUN: %clang_cc1 %s -triple i386-mingw32 -std=c++14 -fsyntax-only >> -Wno-unused-getter-return-value -Wno-unused-value -Wmicrosoft -verify >> -fms-extensions -fms-compatibility -fdelayed-template-parsing >> +// RUN: %clang_cc1 %s -triple i386-pc-win32 -std=c++14 -fsyntax-only >> -Wno-unused-getter-return-value -Wno-unused-value -Wmicrosoft -verify >> -fms-extensions -fms-compatibility -fdelayed-template-parsing >> >> /* Microsoft attribute tests */ >> [repeatable][source_annotation_attribute( Parameter|ReturnValue )] >> >> Modified: cfe/trunk/test/SemaCXX/ms-novtable.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ms-novtable.cpp?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/test/SemaCXX/ms-novtable.cpp (original) >> +++ cfe/trunk/test/SemaCXX/ms-novtable.cpp Mon Jul 20 17:57:31 2015 >> @@ -1,4 +1,4 @@ >> -// RUN: %clang_cc1 %s -fsyntax-only -verify -fms-extensions -Wno-microsoft >> -std=c++11 >> +// RUN: %clang_cc1 -triple i386-pc-win32 %s -fsyntax-only -verify >> -fms-extensions -Wno-microsoft -std=c++11 >> >> struct __declspec(novtable) S {}; >> enum __declspec(novtable) E {}; // expected-warning{{'novtable' attribute >> only applies to classes}} >> >> Added: cfe/trunk/test/SemaCXX/ms-unsupported.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/ms-unsupported.cpp?rev=242730&view=auto >> ============================================================================== >> --- cfe/trunk/test/SemaCXX/ms-unsupported.cpp (added) >> +++ cfe/trunk/test/SemaCXX/ms-unsupported.cpp Mon Jul 20 17:57:31 2015 >> @@ -0,0 +1,5 @@ >> +// RUN: %clang_cc1 -triple x86_64-pc-windows-gnu %s -fsyntax-only -verify >> -fms-extensions -Wno-microsoft -std=c++11 >> + >> +// "novtable" is ignored except with the Microsoft C++ ABI. >> +// MinGW uses the Itanium C++ ABI so check that it is ignored there. >> +struct __declspec(novtable) S {}; // expected-warning{{__declspec attribute >> 'novtable' is not supported}} >> >> Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=242730&r1=242729&r2=242730&view=diff >> ============================================================================== >> --- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original) >> +++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Mon Jul 20 17:57:31 2015 >> @@ -1885,7 +1885,8 @@ static void GenerateHasAttrSpellingStrin >> } >> } >> >> - // It is assumed that there will be an llvm::Triple object named T >> within >> + // It is assumed that there will be an llvm::Triple object >> + // named "T" and a TargetInfo object named "Target" within >> // scope that can be used to determine whether the attribute exists in >> // a given target. >> std::string Test; >> @@ -1902,6 +1903,7 @@ static void GenerateHasAttrSpellingStrin >> } >> Test += ")"; >> >> + // If the attribute is specific to particular OSes, check those. >> std::vector<std::string> OSes; >> if (!R->isValueUnset("OSes")) { >> Test += " && ("; >> @@ -1916,6 +1918,21 @@ static void GenerateHasAttrSpellingStrin >> Test += ")"; >> } >> >> + // If one or more CXX ABIs are specified, check those as well. >> + if (!R->isValueUnset("CXXABIs")) { >> + Test += " && ("; >> + std::vector<std::string> CXXABIs = >> + R->getValueAsListOfStrings("CXXABIs"); >> + for (auto AI = CXXABIs.begin(), AE = CXXABIs.end(); AI != AE; ++AI) >> { >> + std::string Part = *AI; >> + >> + Test += "Target.getCXXABI().getKind() == TargetCXXABI::" + Part; >> + if (AI + 1 != AE) >> + Test += " || "; >> + } >> + Test += ")"; >> + } >> + >> // If this is the C++11 variety, also add in the LangOpts test. >> if (Variety == "CXX11") >> Test += " && LangOpts.CPlusPlus11"; >> @@ -1962,6 +1979,7 @@ void EmitClangAttrHasAttrImpl(RecordKeep >> } >> } >> >> + OS << "const llvm::Triple &T = Target.getTriple();\n"; >> OS << "switch (Syntax) {\n"; >> OS << "case AttrSyntax::GNU:\n"; >> OS << " return llvm::StringSwitch<int>(Name)\n"; >> @@ -2464,7 +2482,7 @@ static std::string GenerateLangOptRequir >> } >> >> static void GenerateDefaultTargetRequirements(raw_ostream &OS) { >> - OS << "static bool defaultTargetRequirements(const llvm::Triple &) {\n"; >> + OS << "static bool defaultTargetRequirements(const TargetInfo &) {\n"; >> OS << " return true;\n"; >> OS << "}\n\n"; >> } >> @@ -2533,6 +2551,20 @@ static std::string GenerateTargetRequire >> Test += ")"; >> } >> >> + // Test for the C++ ABI, if specified. >> + if (!R->isValueUnset("CXXABIs")) { >> + Test += " && ("; >> + std::vector<std::string> CXXABIs = >> R->getValueAsListOfStrings("CXXABIs"); >> + for (auto I = CXXABIs.begin(), E = CXXABIs.end(); I != E; ++I) { >> + std::string Part = *I; >> + Test += "Target.getCXXABI().getKind() == TargetCXXABI::" + Part; >> + if (I + 1 != E) >> + Test += " || "; >> + FnName += Part; >> + } >> + Test += ")"; >> + } >> + >> // If this code has already been generated, simply return the previous >> // instance of it. >> static std::set<std::string> CustomTargetSet; >> @@ -2540,7 +2572,8 @@ static std::string GenerateTargetRequire >> if (I != CustomTargetSet.end()) >> return *I; >> >> - OS << "static bool " << FnName << "(const llvm::Triple &T) {\n"; >> + OS << "static bool " << FnName << "(const TargetInfo &Target) {\n"; >> + OS << " const llvm::Triple &T = Target.getTriple();\n"; >> OS << " llvm::Triple::ArchType Arch = T.getArch();\n"; >> if (UsesOS) >> OS << " llvm::Triple::OSType OS = T.getOS();\n"; >> >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
