[OpenACC][Sema] Implement warning for non-effective 'private' (#149004)

A 'private' variable reference needs to have a default constructor and a
destructor, else we cannot properly emit them in codegen. This patch
adds a warning-as-default-error to diagnose this.

We'll have to do something similar for firstprivate/reduction, however
it isn't clear whether we could skip the check for default-constructor
for those two (they still need a destructor!). Depending on how we
intend to create them (and we probably have to figure this out?), we
could either require JUST a copy-constructor (then make the init section
    just the alloca, and the copy-ctor be the 'copy' section), OR they
require a default-constructor + copy-assignment.

Author: erichkeane

clang/include/clang/Basic/DiagnosticSemaKinds.td

@@ -13489,6 +13489,12 @@ def err_acc_invalid_default_type
 def err_acc_device_type_multiple_archs
     : Error<"OpenACC 'device_type' clause on a 'set' construct only permits "
             "one architecture">;
+def warn_acc_var_referenced_lacks_op
+    : Warning<"variable of type %0 referenced in OpenACC '%1' clause does not "
+              "have a %enum_select<AccVarReferencedReason>{%DefCtor{default "
+              "constructor}|%Dtor{destructor}}2; reference has no effect">,
+      InGroup<DiagGroup<"openacc-var-lacks-operation">>,
+      DefaultError;
 
 // AMDGCN builtins diagnostics
 def err_amdgcn_load_lds_size_invalid_value : Error<"invalid size value">;

clang/lib/Sema/SemaOpenACC.cpp

@@ -624,6 +624,66 @@ void SemaOpenACC::CheckDeclReference(SourceLocation Loc, Expr *E, Decl *D) {
   // loop (or we aren't in a loop!) so skip the diagnostic.
 }
 
+namespace {
+// Check whether the type of the thing we are referencing is OK for things like
+// private, firstprivate, and reduction, which require certain operators to be
+// available.
+ExprResult CheckVarType(SemaOpenACC &S, OpenACCClauseKind CK, Expr *VarExpr,
+                        Expr *InnerExpr) {
+  // There is nothing to do here, only these three have these sorts of
+  // restrictions.
+  if (CK != OpenACCClauseKind::Private &&
+      CK != OpenACCClauseKind::FirstPrivate &&
+      CK != OpenACCClauseKind::Reduction)
+    return VarExpr;
+
+  // We can't test this if it isn't here, or if the type isn't clear yet.
+  if (!InnerExpr || InnerExpr->isTypeDependent())
+    return VarExpr;
+
+  const auto *RD = InnerExpr->getType()->getAsCXXRecordDecl();
+
+  // if this isn't a C++ record decl, we can create/copy/destroy this thing at
+  // will without problem, so this is a success.
+  if (!RD)
+    return VarExpr;
+
+  // TODO: OpenACC:
+  // Private must have default ctor + dtor in InnerExpr
+  // FirstPrivate must have copyctor + dtor in InnerExpr
+  // Reduction must have copyctor + dtor + operation in InnerExpr
+
+  // TODO OpenACC: It isn't clear what the requirements are for default
+  // constructor/copy constructor are for First private and reduction, but
+  // private requires a default constructor.
+  if (CK == OpenACCClauseKind::Private) {
+    bool HasNonDeletedDefaultCtor =
+        llvm::find_if(RD->ctors(), [](const CXXConstructorDecl *CD) {
+          return CD->isDefaultConstructor() && !CD->isDeleted();
+        }) != RD->ctors().end();
+    if (!HasNonDeletedDefaultCtor && !RD->needsImplicitDefaultConstructor()) {
+      S.Diag(InnerExpr->getBeginLoc(),
+             clang::diag::warn_acc_var_referenced_lacks_op)
+          << InnerExpr->getType() << CK
+          << clang::diag::AccVarReferencedReason::DefCtor;
+      return ExprError();
+    }
+  }
+
+  // All 3 things need to make sure they have a dtor.
+  bool DestructorDeleted =
+      RD->getDestructor() && RD->getDestructor()->isDeleted();
+  if (DestructorDeleted && !RD->needsImplicitDestructor()) {
+    S.Diag(InnerExpr->getBeginLoc(),
+           clang::diag::warn_acc_var_referenced_lacks_op)
+        << InnerExpr->getType() << CK
+        << clang::diag::AccVarReferencedReason::Dtor;
+    return ExprError();
+  }
+  return VarExpr;
+}
+} // namespace
+
 ExprResult SemaOpenACC::ActOnVar(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
                                  Expr *VarExpr) {
   // This has unique enough restrictions that we should split it to a separate
@@ -660,7 +720,7 @@ ExprResult SemaOpenACC::ActOnVar(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
   if (const auto *DRE = dyn_cast<DeclRefExpr>(CurVarExpr)) {
     if (isa<VarDecl, NonTypeTemplateParmDecl>(
             DRE->getFoundDecl()->getCanonicalDecl()))
-      return VarExpr;
+      return CheckVarType(*this, CK, VarExpr, CurVarExpr);
   }
 
   // If CK is a Reduction, this special cases for OpenACC3.3 2.5.15: "A var in a
@@ -679,9 +739,9 @@ ExprResult SemaOpenACC::ActOnVar(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
         // declare, reduction, and use_device.
         const auto *This = dyn_cast<CXXThisExpr>(ME->getBase());
         if (This && This->isImplicit())
-          return VarExpr;
+          return CheckVarType(*this, CK, VarExpr, CurVarExpr);
       } else {
-        return VarExpr;
+        return CheckVarType(*this, CK, VarExpr, CurVarExpr);
       }
     }
   }
@@ -690,14 +750,14 @@ ExprResult SemaOpenACC::ActOnVar(OpenACCDirectiveKind DK, OpenACCClauseKind CK,
   // doesn't fall into 'variable or array name'
   if (CK != OpenACCClauseKind::UseDevice &&
       DK != OpenACCDirectiveKind::Declare && isa<CXXThisExpr>(CurVarExpr))
-    return VarExpr;
+    return CheckVarType(*this, CK, VarExpr, CurVarExpr);
 
   // Nothing really we can do here, as these are dependent.  So just return they
   // are valid.
   if (isa<DependentScopeDeclRefExpr>(CurVarExpr) ||
       (CK != OpenACCClauseKind::Reduction &&
        isa<CXXDependentScopeMemberExpr>(CurVarExpr)))
-    return VarExpr;
+    return CheckVarType(*this, CK, VarExpr, CurVarExpr);
 
   // There isn't really anything we can do in the case of a recovery expr, so
   // skip the diagnostic rather than produce a confusing diagnostic.

clang/test/SemaOpenACC/private_firstprivate_reduction_required_ops.cpp

@@ -0,0 +1,103 @@
+// RUN: %clang_cc1 %s -fopenacc -verify
+
+struct ImplicitCtorDtor{};
+
+struct ImplDeletedCtor{
+  ImplDeletedCtor(int i);
+};
+
+struct DefaultedCtor {
+  DefaultedCtor() = default;
+};
+
+struct ImpledCtor {
+  ImpledCtor() = default;
+};
+
+
+struct DeletedCtor {
+  DeletedCtor() = delete;
+};
+
+struct ImpledDtor {
+  ~ImpledDtor();
+};
+
+struct DefaultedDtor {
+  ~DefaultedDtor() = default;
+};
+
+struct DeletedDtor {
+  ~DeletedDtor() = delete;
+};
+
+struct ImplicitDelDtor {
+  DeletedDtor d;
+};
+
+void private_uses(ImplicitCtorDtor &CDT, ImplDeletedCtor &IDC,
+                  DefaultedCtor &DefC, ImpledCtor &IC, DeletedCtor &DelC,
+                  ImpledDtor &ID, DefaultedDtor &DefD, DeletedDtor &DelD,
+                  ImplicitDelDtor &IDD) {
+
+#pragma acc parallel private(CDT)
+  ;
+
+  // expected-error@+1{{variable of type 'ImplDeletedCtor' referenced in OpenACC 'private' clause does not have a default constructor; reference has no effect}}
+#pragma acc parallel private(IDC)
+  ;
+
+#pragma acc parallel private(DefC)
+  ;
+
+#pragma acc parallel private(IC)
+  ;
+
+  // expected-error@+1{{variable of type 'DeletedCtor' referenced in OpenACC 'private' clause does not have a default constructor; reference has no effect}}
+#pragma acc parallel private(DelC)
+  ;
+
+#pragma acc parallel private(ID)
+  ;
+
+#pragma acc parallel private(DefD)
+  ;
+
+  // expected-error@+1{{variable of type 'DeletedDtor' referenced in OpenACC 'private' clause does not have a destructor; reference has no effect}}
+#pragma acc parallel private(DelD)
+  ;
+
+  // expected-error@+1{{variable of type 'ImplicitDelDtor' referenced in OpenACC 'private' clause does not have a destructor; reference has no effect}}
+#pragma acc parallel private(IDD)
+  ;
+
+}
+
+template<typename T>
+void private_templ(T& t) {
+#pragma acc parallel private(t) // #PRIV
+  ;
+}
+
+void inst(ImplicitCtorDtor &CDT, ImplDeletedCtor &IDC,
+                  DefaultedCtor &DefC, ImpledCtor &IC, DeletedCtor &DelC,
+                  ImpledDtor &ID, DefaultedDtor &DefD, DeletedDtor &DelD,
+                  ImplicitDelDtor &IDD) {
+  private_templ(CDT);
+  // expected-error@#PRIV{{variable of type 'ImplDeletedCtor' referenced in OpenACC 'private' clause does not have a default constructor; reference has no effect}}
+  // expected-note@+1{{in instantiation}}
+  private_templ(IDC);
+  private_templ(DefC);
+  private_templ(IC);
+  // expected-error@#PRIV{{variable of type 'DeletedCtor' referenced in OpenACC 'private' clause does not have a default constructor; reference has no effect}}
+  // expected-note@+1{{in instantiation}}
+  private_templ(DelC);
+  private_templ(ID);
+  private_templ(DefD);
+  // expected-error@#PRIV{{variable of type 'DeletedDtor' referenced in OpenACC 'private' clause does not have a destructor; reference has no effect}}
+  // expected-note@+1{{in instantiation}}
+  private_templ(DelD);
+  // expected-error@#PRIV{{variable of type 'ImplicitDelDtor' referenced in OpenACC 'private' clause does not have a destructor; reference has no effect}}
+  // expected-note@+1{{in instantiation}}
+  private_templ(IDD);
+}