[clang] disable implicit moves when not in CPlusPLus

[mizvekov]

Summary

See PR51842.

This fixes an assert firing in the static analyzer, triggered by implicit moves
in blocks in C mode:

This also simplifies the AST a little bit when compiling non C++ code,
as the xvalue implicit casts are not inserted.

We keep and test that the nrvo flag is still being set on the VarDecls,
as that is still a bit beneficial while not really making anything
more complicated.

Signed-off-by: Matheus Izvekov <[e-mail]>

[mizvekov]

.

[NoQ]

Thanks a lot!! This seems to take care of my issues.

Can you also include the following test case somewhere into, say, test/Analysis/blocks-nrvo.c? It crashes without this patch.

// RUN: %clang_analyze_cc1 -w -analyzer-checker=core -fblocks -verify %s

// expected-no-diagnostics

typedef struct {
  int x;
} S;

void foo() {
  ^{
    S s;
    return s; // no-crash
  };
}

[mizvekov]

include repro for analyzer crash.

[thakis]

Is it possible that this disabled this for Objective-C++ too?

[mizvekov]

In D109654#3002632, @thakis wrote:

Is it possible that this disabled this for Objective-C++ too?

As far as I understand, CPlusPLus will be set on ObjectiveC++ as well, and I think we do have test coverage there, but I haven't personally manually tested this.
Do you see any indications this is a problem?

[thakis]

Yes, https://bugs.chromium.org/p/chromium/issues/detail?id=1250052 I haven't yet verified it's definitely due to this change, but it's the most related-looking change in the regression range.

[mizvekov]

In D109654#3002862, @thakis wrote:

Yes, https://bugs.chromium.org/p/chromium/issues/detail?id=1250052 I haven't yet verified it's definitely due to this change, but it's the most related-looking change in the regression range.

Okay, I assumed any regressions here would be covered by at least clang\test\SemaObjCXX\block-capture.mm, but it's possible the test invocation is wrong.
We definitely need to improve the test coverage for ObjC++ if we missed this otherwise :-(

[thakis]

I bet my issue is due to D108243 and not this change. Sorry for the noise!

clang/lib/Sema/SemaStmt.cpp

 /// is the result of return value.
 ///
 /// This routine implements C++20 [class.copy.elision]p3, which attempts to
 /// treat returned lvalues as rvalues in certain cases (to prefer move
 /// construction), then falls back to treating them as lvalues if that failed.
 ExprResult Sema::PerformMoveOrCopyInitialization(
     const InitializedEntity &Entity, const NamedReturnInfo &NRInfo, Expr *Value,
     bool SupressSimplerImplicitMoves) {
-  if ((!getLangOpts().CPlusPlus2b || SupressSimplerImplicitMoves) &&
+  if (getLangOpts().CPlusPlus &&
+      (!getLangOpts().CPlusPlus2b || SupressSimplerImplicitMoves) &&
       NRInfo.isMoveEligible()) {
     ImplicitCastExpr AsRvalue(ImplicitCastExpr::OnStack, Value->getType(),
                               CK_NoOp, Value, VK_XValue, FPOptionsOverride());
     Expr *InitExpr = &AsRvalue;
     auto Kind = InitializationKind::CreateCopy(Value->getBeginLoc(),
                                                Value->getBeginLoc());
     InitializationSequence Seq(*this, Entity, Kind, InitExpr);
     auto Res = Seq.getFailedOverloadResult();

clang/test/AST/nrvo.c

+// RUN: %clang_cc1 -ast-dump -fblocks %s | FileCheck -strict-whitespace %s
+
+struct A {};
+
+struct A f1() {
+  // CHECK:      FunctionDecl 0x{{[^ ]*}} <line:[[@LINE-1]]:1, line:{{[^:]*}}:1> line:[[@LINE-1]]:10 f1 'struct A ()'
+  // CHECK-NEXT: CompoundStmt 0x{{[^ ]*}} <col:15, line:{{[^:]*}}:1>
+  struct A a;
+  // CHECK-NEXT: DeclStmt 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:13>
+  // CHECK-NEXT: VarDecl 0x{{[^ ]*}} <col:3, col:12> col:12 used a 'struct A':'struct A' nrvo
+  return a;
+  // CHECK-NEXT: ReturnStmt 0x{{[^ ]*}} <line:[[@LINE-1]]:3, col:10>
+  // CHECK-NEXT: ImplicitCastExpr 0x{{[^ ]*}} <col:10> 'struct A':'struct A' <LValueToRValue>
+  // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:10> 'struct A':'struct A' lvalue Var 0x{{[^ ]*}} 'a' 'struct A':'struct A'
+}
+
+void f2() {
+  (void)^{
+    // CHECK:      BlockDecl 0x{{[^ ]*}} <line:[[@LINE-1]]:9, line:{{[^:]*}}:3> line:[[@LINE-1]]:9
+    // CHECK-NEXT: CompoundStmt 0x{{[^ ]*}} <col:10, line:{{[^:]*}}:3>
+    struct A a;
+    // CHECK-NEXT: DeclStmt 0x{{[^ ]*}} <line:[[@LINE-1]]:5, col:15>
+    // CHECK-NEXT: VarDecl 0x{{[^ ]*}} <col:5, col:14> col:14 used a 'struct A':'struct A' nrvo
+    return a;
+    // CHECK-NEXT: ReturnStmt 0x{{[^ ]*}} <line:[[@LINE-1]]:5, col:12>
+    // CHECK-NEXT: ImplicitCastExpr 0x{{[^ ]*}} <col:12> 'struct A':'struct A' <LValueToRValue>
+    // CHECK-NEXT: DeclRefExpr 0x{{[^ ]*}} <col:12> 'struct A':'struct A' lvalue Var 0x{{[^ ]*}} 'a' 'struct A':'struct A'
+  }();
+}

clang/test/Analysis/blocks-nrvo.c

+// RUN: %clang_analyze_cc1 -w -analyzer-checker=core -fblocks -verify %s
+
+// expected-no-diagnostics
+
+typedef struct {
+  int x;
+} S;
+
+void foo() {
+  ^{
+    S s;
+    return s; // no-crash
+  };
+}