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

[Roman Lebedev via Phabricator via Openmp-commits]

lebedev.ri added a comment.

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.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57615