[PATCH] D93051: [Flang][openmp] Add semantic checks for OpenMP critical construct.

[sameeran joshi via Phabricator via llvm-commits]

sameeranjoshi added inline comments.


================
Comment at: flang/lib/Semantics/resolve-directives.cpp:369
   void ResolveOmpName(const parser::Name &, Symbol::Flag);
-  Symbol *ResolveName(const parser::Name *);
+  Symbol *ResolveName(const parser::Name *, Symbol::Flag);
   Symbol *DeclareOrMarkOtherAccessEntity(const parser::Name &, Symbol::Flag);
----------------
clementval wrote:
> What's the big difference with `ResolveOmpName`. Merging the two functions would be better. 
That's a good catch.
If you see `ResolveName` was implemented before this patch.
This patch highlighted the need to extend the name resolution logic, which was just extended inside `ResolveName` and no other code was affected(except that we need a one time signature change with this patch).
As discussed, there is no way I could find to reuse code from `resolve-names.cpp` inside this file, hence we had a short/quick way to implement `ResolveName` inside this file.
Being said that I still see with some new construct the `ResolveName` inside this file might need some change.
Hence I extracted into another function.
Which makes only changes to the above function and not much changes in other code.

If there's still something you see of no use to have separate functions I can refactor them and collapse them into 1.
Thank you.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D93051