Page MenuHomePhabricator

[constexpr][c++2a] Try-catch blocks in constexpr functions
ClosedPublic

Authored by bruno on Nov 29 2018, 5:24 PM.

Details

Summary

Implement support for try-catch blocks in constexpr functions, as
proposed in http://wg21.link/P1002 and voted in San Diego for c++20.

The idea is that we can still never throw inside constexpr, so the catch
block is never entered. A try-catch block like this:

try { f(); } catch (...) { }

is then morally equivalent to just

{ f(); }

Same idea should apply for function/constructor try blocks.

rdar://problem/45530773

Diff Detail

Repository
rL LLVM

Event Timeline

bruno created this revision.Nov 29 2018, 5:24 PM
rsmith added inline comments.Nov 29 2018, 5:42 PM
lib/AST/ExprConstant.cpp
4278–4287 ↗(On Diff #176001)

The children of a try statement are the try body followed by the catch statements, in order. We only want to evaluate the normal body, so you should just recurse to evaluating cast<CXXTryStmt>(S)->getTryBlock()...

4290–4294 ↗(On Diff #176001)

... and this should then be unreachable.

lib/Sema/SemaDeclCXX.cpp
1904–1905 ↗(On Diff #176001)

Is there a reason to not allow this as an extension in earlier language modes?

1906–1907 ↗(On Diff #176001)

Cxx1yLoc isn't right here; we should produce a -Wc++17-compat diagnostic, not a -Wc++14-compat diagnostic. We should probably prefer the Cxx1zLoc warning over the Cxx1yLoc warning if we have both.

1915–1922 ↗(On Diff #176001)

We should just unconditionally allow CXXCatchStmts (that is, don't bother checking the language mode, just walk the children), since whenever we encounter one we must also have a CXXTryStmt, which we'll already have checked.

1962–1983 ↗(On Diff #176001)

The repetition can be avoided here by walking the children of Body (regardless of what kind of statement it is), and checking them all. (We can't *quite* just pass Body directly to CheckConstexprFunctionStmt because (nested) compound statements aren't permitted until C++14.)

1973 ↗(On Diff #176001)

Don't return here: we still need to do the other checks below.

bruno updated this revision to Diff 176559.Dec 4 2018, 12:43 AM

Address @rsmith comments

Hi Bruno, thanks for working on this!

include/clang/Basic/DiagnosticSemaKinds.td
2370 ↗(On Diff #176559)

I guess this should technically be C++2a

2431 ↗(On Diff #176559)

(ditto)

lib/Sema/SemaDeclCXX.cpp
1913–1916 ↗(On Diff #176559)

I think we still need to check out the catch stmt (even if it'll never be evaluated) to make sure that it doesn't have any prohibited statements. i.e., we should error here:

constexpr int f() { 
  try { return 0; }
  catch (...) {
    merp: goto merp;
  }
}

But I believe that this patch will accept this.

bruno updated this revision to Diff 176733.Dec 4 2018, 4:43 PM

Update patch after @erik.pilkington review!

LGTM, but you should probably let @rsmith have the final word!

lib/Sema/SemaDeclCXX.cpp
1916–1919 ↗(On Diff #176733)

Might be clearer to just write if (!CheckConstexprFunctionStmt(SemaRef, Dcl, cast<CXXCatchStmt>(SubStmt)->getHandlerBlock())), rather than looping over 1 statement. Could you also add that example I posted as a testcase for this?

Some minor nits from the peanut gallery.

lib/AST/ExprConstant.cpp
4278 ↗(On Diff #176733)

Missing full stop.

lib/Sema/SemaDeclCXX.cpp
1906 ↗(On Diff #176733)

I'd appreciate curly braces here, even though they're not strictly required by the coding standard. ;-)

1971 ↗(On Diff #176733)

Likewise here.

bruno updated this revision to Diff 177367.Dec 7 2018, 5:52 PM
bruno marked an inline comment as done.

Address @aaron.ballman and @erik.pilkington reviews.

aaron.ballman accepted this revision.Dec 8 2018, 5:38 AM

LGTM aside from a few other small nits.

lib/Sema/SemaDeclCXX.cpp
1904 ↗(On Diff #177367)

Cxx2aLoc.isInvalid()

1956 ↗(On Diff #177367)

This restriction is lifted in C++2a, as long as inner statements also apply the general constexpr rules.

This revision is now accepted and ready to land.Dec 8 2018, 5:38 AM
This revision was automatically updated to reflect the committed changes.