On Mon, Jul 20, 2015 at 3:00 PM, Bob Wilson <bob.wil...@apple.com> wrote: > > On Jul 17, 2015, at 5:00 PM, Richard Smith <rich...@metafoo.co.uk> wrote: > > On Fri, Jul 17, 2015 at 3:43 PM, David Majnemer <david.majne...@gmail.com> > wrote: >> >> >> >> On Fri, Jul 17, 2015 at 3:13 PM, Richard Smith <rich...@metafoo.co.uk> >> wrote: >>> >>> It seems to me that we should be basing the check on the TargetCXXABI >>> rather than whether the target is Windows. >> >> >> That's why I suggested to use llvm::Triple::isKnownWindowsMSVCEnvironment, >> it's what we use to set the CXX ABI: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/TargetInfo.cpp?revision=242198&view=markup#l87 > > > That's only the default; targets are permitted to override it, and many of > them do so. ItaniumWindowsARMleTargetInfo sets a non-MS C++ ABI, for > instance. > > > Here’s a new patch that checks TargetCXXABI.
Generally LGTM, with a nit: > diff --git utils/TableGen/ClangAttrEmitter.cpp > utils/TableGen/ClangAttrEmitter.cpp > index 5dc33a0..e990d8a 100644 > --- utils/TableGen/ClangAttrEmitter.cpp > +++ utils/TableGen/ClangAttrEmitter.cpp > @@ -1885,7 +1885,8 @@ static void GenerateHasAttrSpellingStringSwitch( > } > } > > - // 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 GenerateHasAttrSpellingStringSwitch( > } > Test += ")"; > > + // If the attribute is specific to particular OSes, check those. > std::vector<std::string> OSes; > if (!R->isValueUnset("OSes")) { > Test += " && ("; > @@ -1916,6 +1918,22 @@ static void GenerateHasAttrSpellingStringSwitch( > Test += ")"; > } > > + // If one or more CXX ABIs are specified, check those as well. > + std::vector<std::string> CXXABIs; > + 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 +1980,7 @@ void EmitClangAttrHasAttrImpl(RecordKeeper &Records, > raw_ostream &OS) { > } > } > > + 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 +2483,7 @@ static std::string GenerateLangOptRequirements(const > Record &R, > } > > 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 +2552,20 @@ static std::string GenerateTargetRequirements(const > Record &Attr, > 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 += ")"; > + } This code appears to be duplicated from above (mostly), can the implementations be shared? > + > // If this code has already been generated, simply return the previous > // instance of it. > static std::set<std::string> CustomTargetSet; > @@ -2540,7 +2573,8 @@ static std::string GenerateTargetRequirements(const > Record &Attr, > 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"; > ~Aaron _______________________________________________ cfe-commits mailing list cfe-commits@cs.uiuc.edu http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits