[Clang] [Sema] Removed a fix-it for system headers

[fahadnayyar]

Summary

Disabled an invalid fix-it which suggested fixes to be applied in system headers for some programs in IDEs like Xcode.

rdar://100890960

[NoQ]

Looks great! Sounds like you're looking for a more permanent fix, I guess ConversionFixItGenerator could try to avoid adding fixits to system header functions?

clang/lib/Sema/SemaOverload.cpp

10923–10927

Since you're reformatting anyway, maybe use a range-based for-loop?

[NoQ]

clang/lib/Sema/SemaOverload.cpp

10923–10927

(or for_each)

[aaron.ballman]

You should also add a release note for the changes.

clang/lib/Sema/SemaOverload.cpp

10920

Let's drop the rdar link as that's not helpful for anyone outside of Apple.

[fahadnayyar]

Added release notes and changed a normal for-loop to range-based for-loop.

[fahadnayyar]

In D141868#4060757, @NoQ wrote:

Looks great! Sounds like you're looking for a more permanent fix, I guess ConversionFixItGenerator could try to avoid adding fixits to system header functions?

It probably requires some larger rearchitecturing of the FixItGeneration implementation to avoid system headers for all kinds of FixitHints like ConversionFixIt. We can do that later as a separate patch after getting some more examples where fixits are suggested for system headers.

In D141868#4065287, @aaron.ballman wrote:

You should also add a release note for the changes.

Done!

[aaron.ballman]

LGTM!

clang/docs/ReleaseNotes.rst

356 ↗

(On Diff #490618)

clang/lib/Sema/SemaOverload.cpp

static void DiagnoseBadConversion(Sema &S, OverloadCandidate *Cand,
 
   // Emit the generic diagnostic and, optionally, add the hints to it.
   PartialDiagnostic FDiag = S.PDiag(diag::note_ovl_candidate_bad_conv);
   FDiag << (unsigned)FnKindPair.first << (unsigned)FnKindPair.second << FnDesc
         << (FromExpr ? FromExpr->getSourceRange() : SourceRange()) << FromTy
         << ToTy << (unsigned)isObjectArgument << I + 1
         << (unsigned)(Cand->Fix.Kind);
 
+  // Check that location of Fn is not in system header (rdar://100890960).
+  if (!S.SourceMgr.isInSystemHeader(Fn->getLocation())) {
     // If we can fix the conversion, suggest the FixIts.
     for (std::vector<FixItHint>::iterator HI = Cand->Fix.Hints.begin(),
-       HE = Cand->Fix.Hints.end(); HI != HE; ++HI)
+                                          HE = Cand->Fix.Hints.end();
+         HI != HE; ++HI)
         FDiag << *HI;
+  }
+
   S.Diag(Fn->getLocation(), FDiag);
 
   MaybeEmitInheritedConstructorNote(S, Cand->FoundDecl);
 }
 
 /// Additional arity mismatch diagnosis specific to a function overload
 /// candidates. This is not covered by the more general DiagnoseArityMismatch()
 /// over a candidate in any candidate set.

clang/test/Index/fixit-sys-header.h

+#pragma clang system_header
+
+int func_in_sys_header(char * s, unsigned long len)
+{
+    return 0;
+}

clang/test/Index/fixit-sysheader-test.cpp

+// RUN: c-index-test -test-load-source all %s 2>&1 | FileCheck %s
+
+#include "fixit-sys-header.h"
+#include "fixit-user-header.h"
+
+int main(int argc, char const *argv[])
+{
+    char* str;{};
+    
+    func_in_sys_header(str, str + 10);
+    // CHECK: Number FIX-ITs = 0
+    // CHECK-NEXT: candidate function not viable: no known conversion from 'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+    // CHECK-NEXT: Number FIX-ITs = 0
+    
+    func_in_user_header(str, str + 10);
+    // CHECK: Number FIX-ITs = 0
+    // CHECK-NEXT: candidate function not viable: no known conversion from 'char *' to 'unsigned long' for 2nd argument; dereference the argument with *
+    // CHECK-NEXT: Number FIX-ITs = 2
+
+    return 0;
+}

clang/test/Index/fixit-user-header.h

+int func_in_user_header(char * s, unsigned long len)
+{
+    return 0;
+}