[OpenMP] Fix false error report of array subscription for templated indexes.

[h-joo]

Summary

This is a fix for Bug 45383.

This revision fixes the error reported for array subscription for the cases where the array index is a template parameter.
Note that the ArraySubscriptExpr::getBase() might return something which might not be a base in the cases where either LHS or RHS of the ArraySubscriptExpr is a template declaration - I think the fundemental fix would include more changes with ArraySubscriptExpr::getBase() in the presence of template dependent types, but I thought it would involve a bigger scope of refactoring since many of the code is touching it.

Please also keep in mind this is my first patch, so I would be very glad for any kind of comment if I did something wrong.

[jdoerfert]

Can we create a test case that shows even if it is a dependent type we will eventuall issue an error if it is not an addressable lvalue or array item?
If this is the part that needs refactoring to work, we should add the test with a TODO.

[h-joo]

In D78643#1997103, @jdoerfert wrote:

Can we create a test case that shows even if it is a dependent type we will eventuall issue an error if it is not an addressable lvalue or array item?
If this is the part that needs refactoring to work, we should add the test with a TODO.

Added a test to check the test triggers in presence of lvalue expression. Although the test does not trigger line 15912, but rather line 15905. I tried some examples and it seems like after during the template instantiation, while an ArraySubscriptExpr is being constructed, it already checks whether it's a pointer type or an array type, thus, I am thinking this check in line 15912 might actually be redundant?

[jdoerfert]

This looks reasonable to me. @ABataev WDYT?

[ABataev]

In D78643#1997344, @jdoerfert wrote:

This looks reasonable to me. @ABataev WDYT?

I would add a positive test with -ast-print

clang/lib/Sema/SemaOpenMP.cpp

15913

Just QualType BaseType = ASE->getBase()->getType().getNonReferenceType(); and drop the call for getNonReferenceType() in later checks.

[h-joo]

In D78643#1997405, @ABataev wrote:

In D78643#1997344, @jdoerfert wrote:

This looks reasonable to me. @ABataev WDYT?

I would add a positive test with -ast-print

1. Changed into a positive test with -ast-print
2. Just QualType BaseType = ASE->getBase()->getType().getNonReferenceType(); and dropped all the call for getNonReferenceType() in later checks.

Thank you for your time for the review! I do have one more question to ask. I don't understand the log of the build failure, may I ask for some help to understand the failures?

[h-joo]

Sorry, I feel like I'm holding back the fix for this bug since I'm stuck on how to proceed with the build failure, @ABataev would you like to proceed with the other revision that you were working on? If you would, I will drop this one then.

[jdoerfert]

In D78643#2012212, @h-joo wrote:

Sorry, I feel like I'm holding back the fix for this bug since I'm stuck on how to proceed with the build failure, @ABataev would you like to proceed with the other revision that you were working on? If you would, I will drop this one then.

Don't worry. What build failure exactly? The lldb libc++ failures shown here in phab or something else?

[h-joo]

In D78643#2012511, @jdoerfert wrote:

In D78643#2012212, @h-joo wrote:

Sorry, I feel like I'm holding back the fix for this bug since I'm stuck on how to proceed with the build failure, @ABataev would you like to proceed with the other revision that you were working on? If you would, I will drop this one then.

Don't worry. What build failure exactly? The lldb libc++ failures shown here in phab or something else?

Exactly, I was a bit lost by the lldb and libc++ failures

clang/lib/Sema/SemaOpenMP.cpp

if (DepKind == OMPC_DEPEND_sink) {
             (OMPDependTFound &&
              DSAStack->getOMPDependT().getTypePtr() == ExprTy.getTypePtr())) {
           Diag(ELoc, diag::err_omp_expected_addressable_lvalue_or_array_item)
               << (LangOpts.OpenMP >= 50 ? 1 : 0) << 1
               << RefExpr->getSourceRange();
           continue;
         }
 
-        auto *ASE = dyn_cast<ArraySubscriptExpr>(SimpleExpr);
-        if (!RefExpr->IgnoreParenImpCasts()->isLValue() ||
-            (ASE &&
-             !ASE->getBase()
-                  ->getType()
-                  .getNonReferenceType()
-                  ->isPointerType() &&
-             !ASE->getBase()->getType().getNonReferenceType()->isArrayType())) {
+        if (!RefExpr->IgnoreParenImpCasts()->isLValue()) {
           Diag(ELoc, diag::err_omp_expected_addressable_lvalue_or_array_item)
               << (LangOpts.OpenMP >= 50 ? 1 : 0)
               << (LangOpts.OpenMP >= 50 ? 1 : 0) << RefExpr->getSourceRange();
           continue;
         }
 
+        if (auto *ASE = dyn_cast<ArraySubscriptExpr>(SimpleExpr)) {
+          QualType BaseType = ASE->getBase()->getType().getNonReferenceType();
+          if (!BaseType->isDependentType() && !BaseType->isPointerType() &&
+              !BaseType->isArrayType()) {
+            Diag(ELoc, diag::err_omp_expected_addressable_lvalue_or_array_item)
+                << (LangOpts.OpenMP >= 50 ? 1 : 0)
+                << (LangOpts.OpenMP >= 50 ? 1 : 0) << RefExpr->getSourceRange();
+            continue;
+          }
+        }
+
         ExprResult Res;
         {
           Sema::TentativeAnalysisScope Trap(*this);
           Res = CreateBuiltinUnaryOp(ELoc, UO_AddrOf,
                                      RefExpr->IgnoreParenImpCasts());
         }
         if (!Res.isUsable() && !isa<OMPArraySectionExpr>(SimpleExpr) &&
             !isa<OMPArrayShapingExpr>(SimpleExpr)) {

clang/test/OpenMP/depend_template_subscription.cpp

+// RUN: %clang_cc1 -verify -fopenmp -ast-print %s | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+
+template<typename IndexT>
+void test(double *A, IndexT k)
+{
+  #pragma omp task depend(out: A[k]) 
+  {
+    ;
+  }
+}
+// CHECK: template <typename IndexT> void test(double *A, IndexT k) {
+// CHECK:     #pragma omp task depend(out : A[k])
+// CHECK:         {
+// CHECK:             ;
+// CHECK:         }
+// CHECK: }
+// CHECK: template<> void test<int>(double *A, int k) {
+// CHECK:     #pragma omp task depend(out : A[k])
+// CHECK:         {
+// CHECK:             ;
+// CHECK:         }
+// CHECK: }
+
+
+struct lValueVector {
+  int operator [] (int index) {
+    return index + 42;
+  }
+};
+template<typename BaseTypeT, typename IndexT>
+void test2(BaseTypeT A, IndexT k)
+{
+  #pragma omp task depend(out: A[k]) // expected-error {{expected addressable lvalue expression, array element or array section}}
+  {
+    ;
+  }
+}
+int driver(double *A)
+{
+  int k = 42;
+  test(A, k);
+  test2(lValueVector(), k); // expected-note {{in instantiation of function template specialization 'test2<lValueVector, int>' requested here}} 
+  return 0;
+}
+
+#endif
 No newline at end of file