[Openmp-commits] [PATCH] D57615: [AST][OpenMP] OpenMP master Construct contains Structured block

[Alexey Bataev via Phabricator via Openmp-commits]

ABataev added a comment.

In D57615#1381467 <https://reviews.llvm.org/D57615#1381467>, @lebedev.ri wrote:

> In D57615#1381447 <https://reviews.llvm.org/D57615#1381447>, @ABataev wrote:
>
> > > Okay, enough is enough :)
> > >  I think a very important phrase was repeated a number of times, and it highlights the **actual** problem here:
> > > 
> > >> is not the representation of the structured block
> >
> > It is not a problem! It is the internal implementation design! What do you want to get? Why do you need all these flags?
>
>
> Uhm, i did state that in the commit message of rL352882 <https://reviews.llvm.org/rL352882> :
>
> >    Summary:
> >   I'm working on a clang-tidy check, much like existing [[ http://clang.llvm.org/extra/clang-tidy/checks/bugprone-exception-escape.html | bugprone-exception-escape ]],
> >   to detect when an exception might escape out of an OpenMP construct it isn't supposed to escape from.
>
> and in https://bugs.llvm.org/show_bug.cgi?id=40563#c4 :
>
> > (In reply to Roman Lebedev from comment #4)
> >  > (In reply to Alexey Bataev from comment #3)
> >  > > (In reply to Roman Lebedev from comment #2)
> >  > > > (In reply to Alexey Bataev from comment #1)
> >  > > > CapturedDecl is not the representation of the structured block. It used just
> >  > > > to simplify the parsing/sema/codegen. The outlined function is not generated
> >  > > > for the loop, so there is no problem with the standard compatibility.
> >  > > 
> >  > > Exactly. Perhaps i have poorly titled the bug.
> >  > > 
> >  > > The real question was formulated in the last phrase:
> >  > > 
> >  > > > There needs to be some way to represent the fact that
> >  > > > only the body is nothrow.
> >  > 
> >  > What for? It does not help with anything. Standard does not require to
> >  > perform a thorough analysis of the structured blocks for the single
> >  > entry/single exit criteria. It just states that such OpenMP directives are
> >  > not compatible with the standard. What do you want to get with this?
> > 
> > Such syntactic sugar in AST potentially helps in the same way the very
> > existence
> >  of well-defined AST helps - it is much easier to further operate on an
> > well-defined
> >  AST, rather than on something less specified for that. (IR, asm, ...)
> > 
> > In this case, i'm going through the OP spec, through the every
> >  > A throw executed inside a ??? region must cause execution to resume within the
> >  > same iteration of the ??? region, and the same thread that threw the exception must
> >  > catch it.
> >  note, and trying to verify that it is what clang does, either via
> >  CapturedDecl's 'nothrow' tag,
> >  or, like in this case, by filing an issue.
> > 
> > I want this info, so i can use this finely-structured info to do at least
> > partial
> >  validation that these notes "no exception may escape" in the standard are
> >  not violated.
> > 
> > In my case, it will be a clang-tidy check, because there is existing infra
> >  for that.
> >  A clang static analyzer check may appear later, or it may not.
> >  (Though it would be especially interesting in the presence of cross-TU
> >  analysis.)
> > 
> > Therefore i *insist* that it would be //nice// to have this info in the AST
> >  :)
> > 
> > This is not a bug in a form "omg your implementation is so broken, fix it
> >  immediately",
> >  i'm simply using it to track the remaining issues. When i'm done with the
> >  easy cases,
> >  i'll try to cycle back here, and look how this could be solved.
>
> Does that answer the question?
>
> >> Then could you please revoke LG from D57594 <https://reviews.llvm.org/D57594>.
> > 
> > Done!
>
> Thanks.


AST already has all the required information. You just need to teach your analyzer how to get it. The structured blocks are not represented by CapturedStmt/CapturedDecl, in many cases they are hidden inside of them.  You just need to get it. But it does not mean that this a compiler problem.
For some constructs, you may have 2,3 or 4 captured statements, but the structured block is just 1 and it is hidden somewhere inside for better codegen support.
And you need to teach the analyzer how to get this data from the existing AST nodes.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57615