[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

Author: epilk

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<

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,

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 <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