riccibruno added inline comments.

Comment at: lib/AST/StmtOpenMP.cpp:40
+  if (auto *LD = dyn_cast<OMPLoopDirective>(this))
+    return LD->getBody();
+  return getInnermostCapturedStmt()->getCapturedStmt();
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;

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.

  rC Clang



cfe-commits mailing list

Reply via email to