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();
+    });
----------------
doru1004 wrote:
> 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.
This implementation is in line with the current checks for the declaration of 
the mapper variable. You store the declaration onto the stack so that you can 
compare it with the incoming VarDecl passed to the diagnose function.


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

https://reviews.llvm.org/D141871

_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to