> On May 24, 2017, at 8:59 AM, Aaron Ballman <aa...@aaronballman.com> wrote: > > On Tue, May 23, 2017 at 8:46 PM, Argyrios Kyrtzidis via cfe-commits > <cfe-commits@lists.llvm.org> wrote: >> Author: akirtzidis >> Date: Tue May 23 19:46:27 2017 >> New Revision: 303712 >> >> URL: http://llvm.org/viewvc/llvm-project?rev=303712&view=rev >> Log: >> Enhance the 'diagnose_if' attribute so that we can apply it for ObjC methods >> and properties as well >> >> This is an initial commit to allow using it with constant expressions, a >> follow-up commit will enable full support for it in ObjC methods. >> >> Added: >> cfe/trunk/test/SemaObjC/diagnose_if.m >> Modified: >> cfe/trunk/include/clang/Basic/Attr.td >> cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> cfe/trunk/include/clang/Sema/AttributeList.h >> cfe/trunk/include/clang/Sema/Sema.h >> cfe/trunk/lib/Lex/PPMacroExpansion.cpp >> cfe/trunk/lib/Sema/SemaDeclAttr.cpp >> cfe/trunk/lib/Sema/SemaExpr.cpp >> cfe/trunk/lib/Sema/SemaOverload.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=303712&r1=303711&r2=303712&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/Attr.td (original) >> +++ cfe/trunk/include/clang/Basic/Attr.td Tue May 23 19:46:27 2017 >> @@ -149,6 +149,9 @@ class ExprArgument<string name, bit opt >> class FunctionArgument<string name, bit opt = 0, bit fake = 0> : >> Argument<name, >> >> opt, >> >> fake>; >> +class NamedArgument<string name, bit opt = 0, bit fake = 0> : Argument<name, >> + >> opt, >> + >> fake>; >> class TypeArgument<string name, bit opt = 0> : Argument<name, opt>; >> class UnsignedArgument<string name, bit opt = 0> : Argument<name, opt>; >> class VariadicUnsignedArgument<string name> : Argument<name, 1>; >> @@ -1819,14 +1822,14 @@ def Unavailable : InheritableAttr { >> >> def DiagnoseIf : InheritableAttr { >> let Spellings = [GNU<"diagnose_if">]; >> - let Subjects = SubjectList<[Function]>; >> + let Subjects = SubjectList<[Function, ObjCMethod, ObjCProperty]>; >> let Args = [ExprArgument<"Cond">, StringArgument<"Message">, >> EnumArgument<"DiagnosticType", >> "DiagnosticType", >> ["error", "warning"], >> ["DT_Error", "DT_Warning"]>, >> BoolArgument<"ArgDependent", 0, /*fake*/ 1>, >> - FunctionArgument<"Parent", 0, /*fake*/ 1>]; >> + NamedArgument<"Parent", 0, /*fake*/ 1>]; >> let DuplicatesAllowedWhileMerging = 1; >> let LateParsed = 1; >> let AdditionalMembers = [{ >> >> Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=303712&r1=303711&r2=303712&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original) >> +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Tue May 23 19:46:27 >> 2017 >> @@ -2771,6 +2771,7 @@ def warn_attribute_wrong_decl_type : War >> "|types and namespaces" >> "|Objective-C interfaces" >> "|methods and properties" >> + "|functions, methods and properties" > > functions, methods, and properties (inserting the Oxford comma).
Ok. > >> "|struct or union" >> "|struct, union or class" >> "|types" >> >> Modified: cfe/trunk/include/clang/Sema/AttributeList.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/AttributeList.h?rev=303712&r1=303711&r2=303712&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Sema/AttributeList.h (original) >> +++ cfe/trunk/include/clang/Sema/AttributeList.h Tue May 23 19:46:27 2017 >> @@ -915,6 +915,7 @@ enum AttributeDeclKind { >> ExpectedTypeOrNamespace, >> ExpectedObjectiveCInterface, >> ExpectedMethodOrProperty, >> + ExpectedFunctionOrMethodOrProperty, >> ExpectedStructOrUnion, >> ExpectedStructOrUnionOrClass, >> ExpectedType, >> >> Modified: cfe/trunk/include/clang/Sema/Sema.h >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=303712&r1=303711&r2=303712&view=diff >> ============================================================================== >> --- cfe/trunk/include/clang/Sema/Sema.h (original) >> +++ cfe/trunk/include/clang/Sema/Sema.h Tue May 23 19:46:27 2017 >> @@ -2727,7 +2727,7 @@ public: >> /// of a function. >> /// >> /// Returns true if any errors were emitted. >> - bool diagnoseArgIndependentDiagnoseIfAttrs(const FunctionDecl *Function, >> + bool diagnoseArgIndependentDiagnoseIfAttrs(const NamedDecl *ND, >> SourceLocation Loc); >> >> /// Returns whether the given function's address can be taken or not, >> >> Modified: cfe/trunk/lib/Lex/PPMacroExpansion.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/PPMacroExpansion.cpp?rev=303712&r1=303711&r2=303712&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Lex/PPMacroExpansion.cpp (original) >> +++ cfe/trunk/lib/Lex/PPMacroExpansion.cpp Tue May 23 19:46:27 2017 >> @@ -1166,6 +1166,7 @@ static bool HasFeature(const Preprocesso >> .Case("objc_generics", LangOpts.ObjC2) >> .Case("objc_generics_variance", LangOpts.ObjC2) >> .Case("objc_class_property", LangOpts.ObjC2) >> + .Case("objc_diagnose_if_attr", LangOpts.ObjC2) >> // C11 features >> .Case("c_alignas", LangOpts.C11) >> .Case("c_alignof", LangOpts.C11) >> >> Modified: cfe/trunk/lib/Sema/SemaDeclAttr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclAttr.cpp?rev=303712&r1=303711&r2=303712&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaDeclAttr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaDeclAttr.cpp Tue May 23 19:46:27 2017 >> @@ -949,7 +949,7 @@ static bool checkFunctionConditionAttr(S >> Msg = "<no message provided>"; >> >> SmallVector<PartialDiagnosticAt, 8> Diags; >> - if (!Cond->isValueDependent() && >> + if (isa<FunctionDecl>(D) && !Cond->isValueDependent() && >> !Expr::isPotentialConstantExprUnevaluated(Cond, cast<FunctionDecl>(D), >> Diags)) { >> S.Diag(Attr.getLoc(), diag::err_attr_cond_never_constant_expr) >> @@ -1037,10 +1037,11 @@ static void handleDiagnoseIfAttr(Sema &S >> return; >> } >> >> - auto *FD = cast<FunctionDecl>(D); >> - bool ArgDependent = ArgumentDependenceChecker(FD).referencesArgs(Cond); >> + bool ArgDependent = false; >> + if (auto *FD = dyn_cast<FunctionDecl>(D)) > > Please use const auto *. > >> + ArgDependent = ArgumentDependenceChecker(FD).referencesArgs(Cond); >> D->addAttr(::new (S.Context) DiagnoseIfAttr( >> - Attr.getRange(), S.Context, Cond, Msg, DiagType, ArgDependent, FD, >> + Attr.getRange(), S.Context, Cond, Msg, DiagType, ArgDependent, >> cast<NamedDecl>(D), >> Attr.getAttributeSpellingListIndex())); >> } >> >> >> Modified: cfe/trunk/lib/Sema/SemaExpr.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=303712&r1=303711&r2=303712&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaExpr.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue May 23 19:46:27 2017 >> @@ -366,8 +366,19 @@ bool Sema::DiagnoseUseOfDecl(NamedDecl * >> >> if (getLangOpts().CUDA && !CheckCUDACall(Loc, FD)) >> return true; >> + } >> >> - if (diagnoseArgIndependentDiagnoseIfAttrs(FD, Loc)) >> + auto getReferencedObjCProp = [](const NamedDecl *D) -> >> + const ObjCPropertyDecl * { >> + if (auto *MD = dyn_cast<ObjCMethodDecl>(D)) > > Please use const auto *. What is the rationale for this, the casted object is ‘const’ already so it’s unnecessary, and I don’t see a mention in coding conventions that we should be using ‘const’ explicitly along with auto. > >> + return MD->findPropertyDecl(); >> + return nullptr; >> + }; >> + if (auto *ObjCPDecl = getReferencedObjCProp(D)) { > > Please do not use auto here (the type is not spelled out in the > initialization). Also, please mark it explicitly as being a const > pointer. Ok. > >> + if (diagnoseArgIndependentDiagnoseIfAttrs(ObjCPDecl, Loc)) >> + return true; >> + } else { >> + if (diagnoseArgIndependentDiagnoseIfAttrs(D, Loc)) > > Please bump the if up to be part of the else clause. Ok. > > ~Aaron > >> return true; >> } >> >> >> Modified: cfe/trunk/lib/Sema/SemaOverload.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaOverload.cpp?rev=303712&r1=303711&r2=303712&view=diff >> ============================================================================== >> --- cfe/trunk/lib/Sema/SemaOverload.cpp (original) >> +++ cfe/trunk/lib/Sema/SemaOverload.cpp Tue May 23 19:46:27 2017 >> @@ -6242,11 +6242,11 @@ EnableIfAttr *Sema::CheckEnableIf(Functi >> } >> >> template <typename CheckFn> >> -static bool diagnoseDiagnoseIfAttrsWith(Sema &S, const FunctionDecl *FD, >> +static bool diagnoseDiagnoseIfAttrsWith(Sema &S, const NamedDecl *ND, >> bool ArgDependent, SourceLocation >> Loc, >> CheckFn &&IsSuccessful) { >> SmallVector<const DiagnoseIfAttr *, 8> Attrs; >> - for (const auto *DIA : FD->specific_attrs<DiagnoseIfAttr>()) { >> + for (const auto *DIA : ND->specific_attrs<DiagnoseIfAttr>()) { >> if (ArgDependent == DIA->getArgDependent()) >> Attrs.push_back(DIA); >> } >> @@ -6293,16 +6293,16 @@ bool Sema::diagnoseArgDependentDiagnoseI >> // EvaluateWithSubstitution only cares about the position of each >> // argument in the arg list, not the ParmVarDecl* it maps to. >> if (!DIA->getCond()->EvaluateWithSubstitution( >> - Result, Context, DIA->getParent(), Args, ThisArg)) >> + Result, Context, cast<FunctionDecl>(DIA->getParent()), >> Args, ThisArg)) >> return false; >> return Result.isInt() && Result.getInt().getBoolValue(); >> }); >> } >> >> -bool Sema::diagnoseArgIndependentDiagnoseIfAttrs(const FunctionDecl >> *Function, >> +bool Sema::diagnoseArgIndependentDiagnoseIfAttrs(const NamedDecl *ND, >> SourceLocation Loc) { >> return diagnoseDiagnoseIfAttrsWith( >> - *this, Function, /*ArgDependent=*/false, Loc, >> + *this, ND, /*ArgDependent=*/false, Loc, >> [&](const DiagnoseIfAttr *DIA) { >> bool Result; >> return DIA->getCond()->EvaluateAsBooleanCondition(Result, Context) && >> >> Added: cfe/trunk/test/SemaObjC/diagnose_if.m >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/diagnose_if.m?rev=303712&view=auto >> ============================================================================== >> --- cfe/trunk/test/SemaObjC/diagnose_if.m (added) >> +++ cfe/trunk/test/SemaObjC/diagnose_if.m Tue May 23 19:46:27 2017 >> @@ -0,0 +1,16 @@ >> +// RUN: %clang_cc1 %s -verify -fno-builtin >> + >> +_Static_assert(__has_feature(objc_diagnose_if_attr), "feature check >> failed?"); >> + >> +#define _diagnose_if(...) __attribute__((diagnose_if(__VA_ARGS__))) >> + >> +@interface I >> +-(void)meth _diagnose_if(1, "don't use this", "warning"); // expected-note >> 1{{from 'diagnose_if'}} >> +@property (assign) id prop _diagnose_if(1, "don't use this", "warning"); // >> expected-note 2{{from 'diagnose_if'}} >> +@end >> + >> +void test(I *i) { >> + [i meth]; // expected-warning {{don't use this}} >> + id o1 = i.prop; // expected-warning {{don't use this}} >> + id o2 = [i prop]; // expected-warning {{don't use this}} >> +} >> >> Modified: cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp >> URL: >> http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp?rev=303712&r1=303711&r2=303712&view=diff >> ============================================================================== >> --- cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp (original) >> +++ cfe/trunk/utils/TableGen/ClangAttrEmitter.cpp Tue May 23 19:46:27 2017 >> @@ -312,7 +312,7 @@ namespace { >> } >> >> void writeDump(raw_ostream &OS) const override { >> - if (type == "FunctionDecl *") { >> + if (type == "FunctionDecl *" || type == "NamedDecl *") { >> OS << " OS << \" \";\n"; >> OS << " dumpBareDeclRef(SA->get" << getUpperName() << "());\n"; >> } else if (type == "IdentifierInfo *") { >> @@ -1181,6 +1181,8 @@ createArgument(const Record &Arg, String >> Ptr = llvm::make_unique<ExprArgument>(Arg, Attr); >> else if (ArgName == "FunctionArgument") >> Ptr = llvm::make_unique<SimpleArgument>(Arg, Attr, "FunctionDecl *"); >> + else if (ArgName == "NamedArgument") >> + Ptr = llvm::make_unique<SimpleArgument>(Arg, Attr, "NamedDecl *"); >> else if (ArgName == "IdentifierArgument") >> Ptr = llvm::make_unique<SimpleArgument>(Arg, Attr, "IdentifierInfo *"); >> else if (ArgName == "DefaultBoolArgument") >> @@ -3103,6 +3105,8 @@ static std::string CalculateDiagnostic(c >> " : ExpectedVariableOrFunction))"; >> >> case ObjCMethod | ObjCProp: return "ExpectedMethodOrProperty"; >> + case Func | ObjCMethod | ObjCProp: >> + return "ExpectedFunctionOrMethodOrProperty"; >> case ObjCProtocol | ObjCInterface: >> return "ExpectedObjectiveCInterfaceOrProtocol"; >> case Field | Var: return "ExpectedFieldOrGlobalVar"; >> >> >> _______________________________________________ >> cfe-commits mailing list >> cfe-commits@lists.llvm.org >> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
