From 873e279095391df347b58ba4bab26dbfecab1262 Mon Sep 17 00:00:00 2001 From: Erik Pilkington Date: Tue, 5 May 2020 13:32:08 -0400 Subject: [PATCH] [SemaObjC] Add a warning for dictionary literals with duplicate keys Duplicate keys in a literal break NSDictionary's invariants. rdar://50454461A Differential revision: https://reviews.llvm.org/D78660 --- clang/include/clang/Basic/DiagnosticSemaKinds.td | 5 ++ clang/lib/Sema/SemaExprObjC.cpp | 70 ++++++++++++++++++++-- .../test/SemaObjC/dictionary-literal-duplicates.m | 62 +++++++++++++++++++ 3 files changed, 131 insertions(+), 6 deletions(-) create mode 100644 clang/test/SemaObjC/dictionary-literal-duplicates.m diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index ac53ad6..8beb478 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -2969,6 +2969,11 @@ def warn_objc_collection_literal_element : Warning< "object of type %0 is not compatible with " "%select{array element type|dictionary key type|dictionary value type}1 %2">, InGroup; +def warn_nsdictionary_duplicate_key : Warning< + "duplicate key in dictionary literal">, + InGroup>; +def note_nsdictionary_duplicate_key_here : Note< + "previous equal key is here">; def err_swift_param_attr_not_swiftcall : Error< "'%0' parameter can only be used with swiftcall calling convention">; def err_swift_indirect_result_not_first : Error< diff --git a/clang/lib/Sema/SemaExprObjC.cpp b/clang/lib/Sema/SemaExprObjC.cpp index 11ab4db..474aba6 100644 --- a/clang/lib/Sema/SemaExprObjC.cpp +++ b/clang/lib/Sema/SemaExprObjC.cpp @@ -894,6 +894,62 @@ ExprResult Sema::BuildObjCArrayLiteral(SourceRange SR, MultiExprArg Elements) { ArrayWithObjectsMethod, SR)); } +/// Check for duplicate keys in an ObjC dictionary literal. For instance: +/// NSDictionary *nd = @{ @"foo" : @"bar", @"foo" : @"baz" }; +static void +CheckObjCDictionaryLiteralDuplicateKeys(Sema &S, + ObjCDictionaryLiteral *Literal) { + if (Literal->isValueDependent() || Literal->isTypeDependent()) + return; + + // NSNumber has quite relaxed equality semantics (for instance, @YES is + // considered equal to @1.0). For now, ignore floating points and just do a + // bit-width and sign agnostic integer compare. + struct APSIntCompare { + bool operator()(const llvm::APSInt &LHS, const llvm::APSInt &RHS) const { + return llvm::APSInt::compareValues(LHS, RHS) < 0; + } + }; + + llvm::DenseMap StringKeys; + std::map IntegralKeys; + + auto checkOneKey = [&](auto &Map, const auto &Key, SourceLocation Loc) { + auto Pair = Map.insert({Key, Loc}); + if (!Pair.second) { + S.Diag(Loc, diag::warn_nsdictionary_duplicate_key); + S.Diag(Pair.first->second, diag::note_nsdictionary_duplicate_key_here); + } + }; + + for (unsigned Idx = 0, End = Literal->getNumElements(); Idx != End; ++Idx) { + Expr *Key = Literal->getKeyValueElement(Idx).Key->IgnoreParenImpCasts(); + + if (auto *StrLit = dyn_cast(Key)) { + StringRef Bytes = StrLit->getString()->getBytes(); + SourceLocation Loc = StrLit->getExprLoc(); + checkOneKey(StringKeys, Bytes, Loc); + } + + if (auto *BE = dyn_cast(Key)) { + Expr *Boxed = BE->getSubExpr(); + SourceLocation Loc = BE->getExprLoc(); + + // Check for @("foo"). + if (auto *Str = dyn_cast(Boxed->IgnoreParenImpCasts())) { + checkOneKey(StringKeys, Str->getBytes(), Loc); + continue; + } + + Expr::EvalResult Result; + if (Boxed->EvaluateAsInt(Result, S.getASTContext(), + Expr::SE_AllowSideEffects)) { + checkOneKey(IntegralKeys, Result.Val.getInt(), Loc); + } + } + } +} + ExprResult Sema::BuildObjCDictionaryLiteral(SourceRange SR, MutableArrayRef Elements) { SourceLocation Loc = SR.getBegin(); @@ -1061,12 +1117,14 @@ ExprResult Sema::BuildObjCDictionaryLiteral(SourceRange SR, HasPackExpansions = true; } - QualType Ty - = Context.getObjCObjectPointerType( - Context.getObjCInterfaceType(NSDictionaryDecl)); - return MaybeBindToTemporary(ObjCDictionaryLiteral::Create( - Context, Elements, HasPackExpansions, Ty, - DictionaryWithObjectsMethod, SR)); + QualType Ty = Context.getObjCObjectPointerType( + Context.getObjCInterfaceType(NSDictionaryDecl)); + + auto *Literal = + ObjCDictionaryLiteral::Create(Context, Elements, HasPackExpansions, Ty, + DictionaryWithObjectsMethod, SR); + CheckObjCDictionaryLiteralDuplicateKeys(*this, Literal); + return MaybeBindToTemporary(Literal); } ExprResult Sema::BuildObjCEncodeExpression(SourceLocation AtLoc, diff --git a/clang/test/SemaObjC/dictionary-literal-duplicates.m b/clang/test/SemaObjC/dictionary-literal-duplicates.m new file mode 100644 index 0000000..5bfe66d --- /dev/null +++ b/clang/test/SemaObjC/dictionary-literal-duplicates.m @@ -0,0 +1,62 @@ +// RUN: %clang_cc1 -Wno-objc-root-class %s -verify +// RUN: %clang_cc1 -xobjective-c++ -Wno-objc-root-class %s -verify + +#define YES __objc_yes +#define NO __objc_no + +@interface NSNumber ++(instancetype)numberWithChar:(char)value; ++(instancetype)numberWithInt:(int)value; ++(instancetype)numberWithDouble:(double)value; ++(instancetype)numberWithBool:(unsigned char)value; ++(instancetype)numberWithUnsignedLong:(unsigned long)value; ++(instancetype)numberWithLongLong:(long long) value; ++(instancetype)numberWithUnsignedInt:(unsigned)value; +@end + +@interface NSString +@end + +@interface NSDictionary ++ (instancetype)dictionaryWithObjects:(const id[])objects + forKeys:(const id[])keys + count:(unsigned long)cnt; +@end + +void test() { + NSDictionary *t1 = @{ + @"foo" : @0, // expected-note 2 {{previous equal key is here}} + @"foo" : @0, // expected-warning{{duplicate key in dictionary literal}} + @("foo") : @0, // expected-warning{{duplicate key in dictionary literal}} + @"foo\0" : @0, + + @1 : @0, // expected-note + {{previous equal key is here}} + @YES : @0, // expected-warning{{duplicate key in dictionary literal}} + @'\1' : @0, // expected-warning{{duplicate key in dictionary literal}} + @1 : @0, // expected-warning{{duplicate key in dictionary literal}} + @1ul : @0, // expected-warning{{duplicate key in dictionary literal}} + @1ll : @0, // expected-warning{{duplicate key in dictionary literal}} +#ifdef __cplusplus + @true : @0, // expected-warning{{duplicate key in dictionary literal}} +#endif + @1.0 : @0, // FIXME: should warn + + @-1 : @0, // expected-note + {{previous equal key is here}} + @4294967295u : @0, // no warning + @-1ll : @0, // expected-warning{{duplicate key in dictionary literal}} + @(NO-YES) : @0, // expected-warning{{duplicate key in dictionary literal}} + }; +} + +#ifdef __cplusplus +template void variadic(Ts... ts) { + NSDictionary *nd = @{ + ts : @0 ..., + @0 : ts ... // expected-warning 2 {{duplicate key in dictionary literal}} expected-note 2 {{previous equal key is here}} + }; +} + +void call_variadic() { + variadic(@0, @1, @2); // expected-note {{in instantiation}} +} +#endif -- 2.7.4