This is an archive of the discontinued LLVM Phabricator instance.

Fix a few issues while skipping function bodies
ClosedPublic

Authored by ogoffart on May 31 2016, 10:03 AM.

Details

Summary

Fix a few issues while skipping function bodies

  • In functions with try { } catch { }, only the try block would be skipped, not the catch blocks
  • The template functions would still be parsed.
  • The initializers within a constructor would still be parsed.
  • The inline functions within class would still be stored, only to be discared later.
  • Invalid code with try would assert (as in "int foo() try assert_here")

This attempt to do even less while skipping function bodies.

Diff Detail

Repository
rL LLVM

Event Timeline

ogoffart updated this revision to Diff 59084.May 31 2016, 10:03 AM
ogoffart retitled this revision from to Fix a few issues while skipping function bodies.
ogoffart updated this object.
ogoffart added reviewers: cfe-commits, rsmith, akyrtzi.
rsmith added inline comments.May 31 2016, 10:41 AM
lib/Parse/ParseStmt.cpp
1989 ↗(On Diff #59084)

Comments should start with a capital letter and end with a period.

1990 ↗(On Diff #59084)

This is not correct. Skipping constructor initializer lists correctly is hard (which is probably why we used to parse the initializer list rather than skipping it). You can use Parser::ConsumeAndStoreFunctionPrologue to get this (approximately) correct.

ogoffart updated this revision to Diff 59197.Jun 1 2016, 4:29 AM

Right, i forgot about the C++11 initializer list syntax. I hope I got it right now.

The idea is that when we see a ") {" or "} {" in the ctor-initializers, (optionally with "..."), it is necessarily the start of the body.

Unless there might be lambda expressions within a template aregument, as in:

  
A::A() : Base<[](){return 42; }()>(){}

But this is not valid because lambda expression cannot appear in this context as far as i know since lambda expression cannot appear in template argument

rsmith edited edge metadata.Jun 9 2016, 1:46 PM

Please call Parser::ConsumeAndStoreFunctionPrologue rather than trying to reinvent it. You're still getting a number of cases wrong that it handles properly.

You also need to handle the case where the code completion token appears within the constructor *mem-initializer*s.

ogoffart updated this revision to Diff 60340.Jun 10 2016, 4:48 AM
ogoffart edited edge metadata.

Using Parser::ConsumeAndStoreFunctionPrologue this time

rsmith accepted this revision.Jun 16 2016, 12:22 PM
rsmith edited edge metadata.

LGTM

This revision is now accepted and ready to land.Jun 16 2016, 12:22 PM
This revision was automatically updated to reflect the committed changes.
chapuni added inline comments.
cfe/trunk/unittests/Tooling/ToolingTest.cpp
315

It didn't pass for targeting *-msvc. Tweaked in r272985.