[PATCH] D141871: [Clang][OpenMP] Add parse and sema for iterator map modifier

[Gheorghe-Teodor Bercea via Phabricator via cfe-commits]

doru1004 added inline comments.


================
Comment at: clang/lib/Sema/SemaOpenMP.cpp:1166-1168
+    return llvm::any_of(Top->IteratorVarDecls, [VD](const VarDecl *IteratorVD) {
+      return IteratorVD == VD->getCanonicalDecl();
+    });
----------------
ABataev wrote:
> doru1004 wrote:
> > doru1004 wrote:
> > > ABataev wrote:
> > > > doru1004 wrote:
> > > > > ABataev wrote:
> > > > > > doru1004 wrote:
> > > > > > > ABataev wrote:
> > > > > > > > Do you really need to store the variable in the stack, is not it enough just to check that the type of this variable is BuiltinType::OMPIterator?
> > > > > > > I'm happy to replace this if you think it will work. Is there an example somewhere in the code where I can get from the VarDecl to the build in type you mention?
> > > > > > You have already a check IteratorModifier->getType()->isSpecificBuiltinType(BuiltinType::OMPIterator), you can you something similar for the variable
> > > > > This didn't work and I had to revert to using the stack!
> > > > Why?
> > > I checked the output of the check and it was false when it should have been true! If you check the latest test that I added it showcases the source code and in the case of OpenMP 5.2 you can see that the message "only variable 'vvec' is allowed in map clauses of this 'omp declare mapper' directive" doesn't appear when a legal iteration variable is used.
> > > If I used the check you suggested then the error message appears.
> > > In the example you pasted the check is performed on a `Expr *`. In the case here, we only have VD which is a VarDecl.
> > I am not sure how I can force it to have that type when it just doesn't. Do you have any suggestions?
> Did not get it. It still shall be of type builtintype::OMPIterator.
The VD that we are checking for this builtin is coming from somewhere else in the code, it is passed into the `Sema::DiagnoseUseOfDecl(` function. It's not a VarDecl that is under the control of anything added in this patch.


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

https://reviews.llvm.org/D141871