[clang] 873e279 - [SemaObjC] Add a warning for dictionary literals with duplicate keys
Erik Pilkington via cfe-commits
cfe-commits at lists.llvm.org
Tue May 5 12:30:55 PDT 2020
Author: Erik Pilkington
Date: 2020-05-05T15:30:39-04:00
New Revision: 873e279095391df347b58ba4bab26dbfecab1262
URL: https://github.com/llvm/llvm-project/commit/873e279095391df347b58ba4bab26dbfecab1262
DIFF: https://github.com/llvm/llvm-project/commit/873e279095391df347b58ba4bab26dbfecab1262.diff
LOG: [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
Added:
clang/test/SemaObjC/dictionary-literal-duplicates.m
Modified:
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExprObjC.cpp
Removed:
################################################################################
diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index ac53ad6ab344..8beb47812d40 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<ObjCLiteralConversion>;
+def warn_nsdictionary_duplicate_key : Warning<
+ "duplicate key in dictionary literal">,
+ InGroup<DiagGroup<"objc-dictionary-duplicate-keys">>;
+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 11ab4db1e485..474aba6a0d44 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<StringRef, SourceLocation> StringKeys;
+ std::map<llvm::APSInt, SourceLocation, APSIntCompare> 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<ObjCStringLiteral>(Key)) {
+ StringRef Bytes = StrLit->getString()->getBytes();
+ SourceLocation Loc = StrLit->getExprLoc();
+ checkOneKey(StringKeys, Bytes, Loc);
+ }
+
+ if (auto *BE = dyn_cast<ObjCBoxedExpr>(Key)) {
+ Expr *Boxed = BE->getSubExpr();
+ SourceLocation Loc = BE->getExprLoc();
+
+ // Check for @("foo").
+ if (auto *Str = dyn_cast<StringLiteral>(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<ObjCDictionaryElement> 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 000000000000..5bfe66d280fe
--- /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
+
+ at 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;
+ at end
+
+ at interface NSString
+ at end
+
+ at interface NSDictionary
++ (instancetype)dictionaryWithObjects:(const id[])objects
+ forKeys:(const id[])keys
+ count:(unsigned long)cnt;
+ at 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 <class... Ts> 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
More information about the cfe-commits
mailing list