[PATCH] D146418: Support for OpenMP 5.0 sec 2.12.7 - Declare Target initializer expressions

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.


================
Comment at: clang/include/clang/Sema/Sema.h:11329
+  /// directive.
+  void ActOnOpenMPImplicitDeclareTarget(Decl *D);
+
----------------
Do I understand correctly, that the variable itself is marked explicitly, but the initializer shall be marked implicitly? If so, please fix the function name


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23097-23098
+public:
+  void SetTargetDecl(Decl *TargetDecl) { this->TargetDecl = TargetDecl; }
+  bool CheckDeclVector() { return this->DeclVector.empty(); }
+  void PushDeclVector(VarDecl *TargetVarDecl) {
----------------
Remove `this->`, where not required.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23099
+  bool CheckDeclVector() { return this->DeclVector.empty(); }
+  void PushDeclVector(VarDecl *TargetVarDecl) {
+    this->DeclVector.push_back(TargetVarDecl);
----------------
1. Wrong function name format.
2. Do you really need to make it public? Hide as much as possible.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23102
+  }
+  VarDecl *PopDeclVector() { return (this->DeclVector.pop_back_val()); }
+  void VisitDeclRefExpr(const DeclRefExpr *Node) {
----------------
Same, as for Push


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:23126-23140
+  while (!Checker.CheckDeclVector()) {
+    VarDecl *TargetVarDecl = Checker.PopDeclVector();
+    if (TargetVarDecl->hasAttr<OMPDeclareTargetDeclAttr>()) {
+      if (TargetVarDecl->hasInit() && TargetVarDecl->hasGlobalStorage()) {
+        Expr *Ex = TargetVarDecl->getInit();
+        if (Ex) {
+          if (auto *DeclRef = dyn_cast_or_null<DeclRefExpr>(Ex)) {
----------------
Better to hide this in GlobalDeclRefChecker


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

https://reviews.llvm.org/D146418