[clang] [Clang][Sema]: Diagnose lambda to bool implicit casts (PR #83152)

[Aaron Ballman via cfe-commits]

================
@@ -16538,6 +16538,24 @@ void Sema::DiagnoseAlwaysNonNullPointer(Expr *E,
     }
   }
 
+  // Complain if we are converting a lambda expression to a boolean value
+  if (const auto *MCallExpr = dyn_cast<CXXMemberCallExpr>(E)) {
+    if (MCallExpr->getObjectType()->isRecordType()) {
+      if (const auto *MRecordDecl = MCallExpr->getRecordDecl()) {
+        if (MRecordDecl->isLambda()) {
+          std::string Str;
+          llvm::raw_string_ostream S(Str);
+
+          E->printPretty(S, nullptr, getPrintingPolicy());
+          Diag(E->getExprLoc(), diag::warn_impcast_pointer_to_bool)
+              << /*FunctionPointerType*/ 1 << S.str() << E->getSourceRange()
+              << Range << IsEqual;
+          return;
+        }
+      }
+    }
+  }
----------------
AaronBallman wrote:

Actually, I'm not convinced we *should* print the entire expression in this case. Lambdas usually aren't nearly as trivial as the ones we have in our test cases, and can be arbitrarily long. e.g.,
```
C:\Users\aballman\OneDrive - Intel Corporation\Desktop\test.cpp:2:7: warning: address of function '[]() {
    int foo = 0;
    return foo;
}' will always evaluate to 'true' [-Wpointer-bool-conversion]
    2 |   if ([](){
      |   ~~  ^~~~~
    3 |     // This is a comment
      |     ~~~~~~~~~~~~~~~~~~~~
    4 |     // intended to make the lambda
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    5 |     // longer so that it shows
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~
    6 |     // what the diagnostic behavior is
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    7 |     int foo = 0;
      |     ~~~~~~~~~~~~
    8 |     // of really large lambdas, and whether
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
    9 |     // it is worth it to print out the entire
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
   10 |     // expression in this case.
      |     ~~~~~~~~~~~~~~~~~~~~~~~~~~~
   11 |     return foo;
      |     ~~~~~~~~~~~
   12 |   }) {
      |   ~
1 warning generated.
```
This has two problems: 1) it repeats the user's code in both the diagnostic and in the highlighting, 2) the amount of context is huge and distracts from the diagnostic.

I think we should modify `warn_impcast_pointer_to_bool` to something along these lines:
```
def warn_impcast_pointer_to_bool : Warning<
    "address of%select{| function| array| lambda function pointer conversion operator}0 '%1' will always evaluate to 'true'">, InGroup<PointerBoolConversion>;
```
(Will need to be reformatted for the usual 80-col limits.) And when passing the source range for the lambda expression, I think we should pass only the lambda introducer as the source range so that we highlight far less code.

WDYT?

https://github.com/llvm/llvm-project/pull/83152