r303712 - Enhance the 'diagnose_if' attribute so that we can apply it for ObjC methods and properties as well
Argyrios Kyrtzidis via cfe-commits
cfe-commits at lists.llvm.org
Wed May 24 10:05:35 PDT 2017
> On May 24, 2017, at 8:59 AM, Aaron Ballman <aaron at aaronballman.com> wrote:
>
> On Tue, May 23, 2017 at 8:46 PM, Argyrios Kyrtzidis via cfe-commits
> <cfe-commits at 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__)))
>> +
>> + at interface I
>> +-(void)meth _diagnose_if(1, "don't use this", "warning"); // expected-note 1{{from 'diagnose_if'}}
>> + at property (assign) id prop _diagnose_if(1, "don't use this", "warning"); // expected-note 2{{from 'diagnose_if'}}
>> + at 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 at lists.llvm.org
>> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
More information about the cfe-commits
mailing list