bob.wilson closed this revision.
bob.wilson marked an inline comment as done.
bob.wilson added a comment.

Committed in r260787.

I had previously missed a regression in one of the existing ovl-check.m tests. 
The testTakesCFTypeRef function was checking that the CFTypeRef overload was 
selected. However, that does not match the behavior prior to r249995, where 
neither overload candidate is chosen as the "best match" and clang falls back 
to using the first one. With my change here, the previous behavior is restored. 
I reordered the two overload candidates in the test to match that. While I was 
at it, I also merged my "Type2" typedef with the existing CFTypeRef in the test.


================
Comment at: lib/Sema/SemaExpr.cpp:7580
@@ +7579,3 @@
+      if (Diagnose) {
+        // If an error was diagnosed here, replace the expression with a
+        // corrected version and continue so we can find further errors.
----------------
george.burgess.iv wrote:
> Because we're doing a similar thing at line 7573 (not necessarily replacing 
> an expression, but still answering differently depending on the value of 
> `Diagnose`), can we either put a similar comment there, or make this comment 
> a bit more general and move it above `if (getLangOpts().ObjCAutoRefCount && 
> [...]`?
Good idea. I'm going to put a comment above the "if" but I'll also leave 
something to explain why the expression is replaced. (I'm also going to change 
those "result = Incompatible" lines to just return, since there is no point in 
continuing with further checks if we're not reporting the diagnostics anyway.)


http://reviews.llvm.org/D17166



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to