lebedev.ri added inline comments.
================
Comment at: lib/AST/StmtOpenMP.cpp:40
+ if (auto *LD = dyn_cast<OMPLoopDirective>(this))
+ return LD->getBody();
+ return getInnermostCapturedStmt()->getCapturedStmt();
----------------
riccibruno wrote:
> lebedev.ri wrote:
> > @riccibruno
> > `getBody()` exists in `const`-only variant
> > `getInnermostCapturedStmt()` does exist in both the `const` and non-`const`
> > variants,
> > but the `const` variant does `const_cast` and defers to non-`const` variant.
> > `getCapturedStmt()` is good though.
> >
> > So at best i can add
> > ```
> > Stmt *getStructuredBlock() {
> > return const_cast<Stmt *>(const_cast<OMPExecutableDirective
> > *>(this)->getStructuredBlock());
> > }
> > ```
> > I'm not sure it's worth it?
> Or perhaps something like the following to avoid one `const_cast`:
>
> ```
> Stmt *getStructuredBlock() {
> const OMPExecutableDirective *ConstThis = this;
> return const_cast<Stmt *>(ConstThis->getStructuredBlock());
> }
> ```
>
> My point is that in itself this is a minor thing, but not defining
> consistently both versions (when appropriate) inevitably leads to annoying
> errors. For example suppose that you only define the const version of
> `getStructuredBlock()`, and then there is some random function `bar` with the
> signature `void bar(Stmt *S)`.
>
> You then try:
> ```
> // Intentionally not `const Stmt *` since we will set some random flag in S.
> void bar(Stmt *S);
>
> // Non-const since we are modifying it.
> OMPExecutableDirective *OED = something;
> bar(OED->getStructuredBlock())
> ```
>
> which fails because `OED->getStructuredBlock()` has type `const Stmt*`, even
> though every other pointee type is non-const qualified.
>
> So unless it is intentional to forbid modifying the statement returned by
> `getStructuredBlock()`, defining both versions of `getStructuredBlock()`
> avoid this problem. Yes it is annoying to have to define the two versions but
> as far as I know this is inherent to C++ unfortunately.
Hmm, okay, will add during next update / before landing, whichever happens next.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D59214/new/
https://reviews.llvm.org/D59214
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
