[PATCH] D110270: [flang][OpenMP] Check for occurrence of multiple list items in nontemporal clause for simd directive

[Kiran Chandramohan via Phabricator via llvm-commits]

kiranchandramohan added inline comments.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:184-192
+    for (auto const &var : alignedNameList) {
+      if (listVars.count(*(var.symbol)) == 1) {
+        context_.Say(itr->second->source,
+            "List item '%s' present at multiple ALIGNED clauses"_err_en_US,
+            var.ToString());
+        break;
+      }
----------------
arnamoy10 wrote:
> kiranchandramohan wrote:
> > Can you make this a lambda function that takes in a list of variables, with listvars declared inside the lambda and an argument (pass ALIGNED or NONTEMPORAL to it)?
> Thanks for the comment.  I was creating a lambda as per your suggestion and passing `alignedClauses`/ `nonTemporalClauses` as the argument and another argument to denote whether we are processing ALIGNED or NONTEMPORAL.
> 
> However, in that lambda, how do I declare a generic variable, which will be defined as the following pseudocode based on the passed argument?  It cannot be `auto` e.g. the following will not work.
> 
> ```
> auto &Clause;
> if (arg == ALIGNED) 
>   Clause = std::get<parser::OmpClause::Aligned>(itr->second->u)};
> else
>   Clause = std::get<parser::OmpClause::Aligned>(itr->second->u)};
> ```
> What will be the type of `Clause` in this case?
I think the lambda at that level might not work since templates are not properly supported for lambdas in C++17. I was thinking something like the following (templated functions) will work. It can be called as `checkMultiple<parser::OmpClause::Aligned>(*itr, "ALIGNED")`. But I see that there is some difference in the way the namelist is constructed. So there are  additional checks or conditions required.
```
const auto &alignedNameList{std::get<std::list<parser::Name>>(alignedClause.v.t)};
const auto &nontempNameList{nontempClause.v};
```

```
  template<typename C, typename V>
void checkMultiple(const V &v, const std::string &clauseName) {

    semantics::UnorderedSymbolSet listVars;
  auto checkMultipleOcurrence = [&](const std::list<parser::Name>& nameList, const parser::CharBlock& item) {
    for (auto const &var : nameList) {
      if (listVars.count(*(var.symbol)) == 1) {
        context_.Say(item,
            "List item '%s' present at multiple %s clauses"_err_en_US,
            var.ToString(), clauseName);
        break;
      }
      listVars.insert(*(var.symbol));
    }
  };
    const auto &alignedClause{
        std::get<C>(v.second->u)};
    const auto &alignedNameList{
        std::get<std::list<parser::Name>>(alignedClause.v.t)};
    checkMultipleOcurrence(alignedNameList, v.second->source);

}
```

Instead, you can try the lambda `checkMultipleOcurrence` that is there in the function above. I think this is what I suggested.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D110270/new/

https://reviews.llvm.org/D110270