[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–10926

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

clang/docs/ReleaseNotes.rst

 - Fix issue that the standard C++ modules importer will call global
   constructor/destructor for the global varaibles in the importing modules.
   This fixes `Issue 59765 <https://github.com/llvm/llvm-project/issues/59765>`_
 - Reject in-class defaulting of previosly declared comparison operators. Fixes
   `Issue 51227 <https://github.com/llvm/llvm-project/issues/51227>`_.
 
 Improvements to Clang's diagnostics
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+- Disabled FIT-IT suggested for a case of bad conversion in system headers.
 - Clang will now check compile-time determinable string literals as format strings.
   Fixes `Issue 55805: <https://github.com/llvm/llvm-project/issues/55805>`_.
 - ``-Wformat`` now recognizes ``%b`` for the ``printf``/``scanf`` family of
   functions and ``%B`` for the ``printf`` family of functions. Fixes
   `Issue 56885: <https://github.com/llvm/llvm-project/issues/56885>`_.
 - Introduced ``-Wsingle-bit-bitfield-constant-conversion``, grouped under
   ``-Wbitfield-constant-conversion``, which diagnoses implicit truncation when
   ``1`` is assigned to a 1-bit signed integer bitfield. This fixes

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.
+  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)
-    FDiag << *HI;
+    for (const FixItHint &HI : Cand->Fix.Hints)
+        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.
 static bool CheckArityMismatch(Sema &S, OverloadCandidate *Cand,

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;
+}