Removed unused variable in clang

[nadav]

Summary

Cleanup the code a bit by removing an unused variable in clang. All of the clang-check tests pass. I discovered this with a Facebook-internal tool.

[modocache]

This LGTM! Looks like the last time this variable was touched was in 2010 as part of a mechanical renaming, https://github.com/llvm/llvm-project/commit/a771f46c82d7. At that point it was used, as a parameter to the static function MakeObjCStringLiteralFixItHint. That static function was later renamed on Dec 17 2013 https://github.com/llvm/llvm-project/commit/bd714e9bb12, to ConversionToObjCStringLiteralCheck, and on the following day Dec 18 2013 in https://github.com/llvm/llvm-project/commit/283bf895066 the signature was changed to no longer take a FixitHint parameter. The author must not have removed this variable at that point because technically it is still referenced, but as you point out, it's never mutated in a way that gives the hint a value.

[modocache]

By the way, I tried to accept this diff and leave the following inline comment on SemaExpr.cpp:15799:

I guess this assert was never capable of being hit previously, since the FixItHint::isNull would always return true? I wonder if we'll now see some genuine asserts for ConversionFixItGenerator::isNull thanks to this change.

However when I attempt to do either, Phabricator displays a dialog indicating I don't have permission to "modify" the diff. Not sure what's going on there.

[nadav]

@modocache Thank you for the review. I updated the permissions of the diff. Sorry, for the trouble.

[modocache]

No problem, thanks for this!

[hiraditya]

LGTM

[hiraditya]

Not sure why this didnt get closed after the patch is merged in: https://reviews.llvm.org/rG4fd91b0f946f49370a3ab33c480a807a3f48b247

[dblaikie]

In D83788#2183684, @hiraditya wrote:

Not sure why this didnt get closed after the patch is merged in: https://reviews.llvm.org/rG4fd91b0f946f49370a3ab33c480a807a3f48b247

The commit didn't contain the special line " Differential Revision: https://reviews.llvm.org/DXXXX" - it did link to the review, but without the special prefix, so Phab didn't pick it up and auto-close this review.

clang/lib/Sema/SemaExpr.cpp

   bool CheckInferredResultType = false;
   bool isInvalid = false;
   unsigned DiagKind = 0;
-  FixItHint Hint;
   ConversionFixItGenerator ConvHints;
   bool MayHaveConvFixit = false;
   bool MayHaveFunctionDiff = false;

     }
     CheckInferredResultType = DstType->isObjCObjectPointerType() &&
       SrcType->isObjCObjectPointerType();
-    if (Hint.isNull() && !CheckInferredResultType) {
+    if (!CheckInferredResultType) {
       ConvHints.tryToFixConversion(SrcExpr, SrcType, DstType, *this);
     }
     else if (CheckInferredResultType) {

     FDiag << FirstType << SecondType << Action << SrcExpr->getSourceRange();
 
   // If we can fix the conversion, suggest the FixIts.
-  assert(ConvHints.isNull() || Hint.isNull());
+  assert(ConvHints.isNull());
   if (!ConvHints.isNull()) {
     for (FixItHint &H : ConvHints.Hints)
       FDiag << H;
-  } else {
-    FDiag << Hint;
   }
   if (MayHaveConvFixit) { FDiag << (unsigned) (ConvHints.Kind); }