[PATCH] D110714: [Flang][openmp] Added semantic checks for atomic construct

[Nimish Mishra via Phabricator via llvm-commits]

NimishMishra added inline comments.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1226
+      context_.Say(variable.GetSource(),
+          "Variable name mismatch on lvalue and rvalue"_err_en_US);
+    }
----------------
kiranchandramohan wrote:
> I think lvalue, rvalue are not commonly used terms in Fortran. Also, is this applicable for update atomic only? Is that enforced?
> 
> A possible error message could be something like the following,
> Atomic update variable <var-name> not found in the RHS of the assignment statement in an ATOMIC UPDATE construct.
Yes, this particular variable name mismatch in RHS and LHS is applicable to atomic update statements only.

I will change the error message.


================
Comment at: flang/lib/Semantics/check-omp-structure.cpp:1238
+template <typename T>
+void OmpStructureChecker::CheckAtomicConstructStructure(T &construct) {
+  const auto &dir{std::get<parser::Verbatim>(construct.t)};
----------------
kiranchandramohan wrote:
> Is the template required here? Can you pass the AssignmentStatement instead here?
It's possible. However, it would require extracting //Verbatim// and //AssignmentStmt// in //void OmpStructureChecker::Enter(const parser::OpenMPAtomicConstruct &x)// at two places: //parser::OmpAtomicUpdate// and //parser::OmpAtomic//

I put a template in this so that for both //OmpAtomicUpdate// and //OmpAtomic//, we have refactored code where the directive and assignment statement extractions are present in source code at one place. 

If there is no problem with duplication of these two extractions, I will remove this template and receive assignment statements instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D110714