This is an archive of the discontinued LLVM Phabricator instance.

[SyntaxTree] Ignore implicit non-leaf `CXXConstructExpr`
ClosedPublic

Authored by eduucaldas on Aug 27 2020, 4:44 AM.

Diff Detail

Event Timeline

eduucaldas created this revision.Aug 27 2020, 4:44 AM
Herald added a project: Restricted Project. · View Herald TranscriptAug 27 2020, 4:44 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
eduucaldas requested review of this revision.Aug 27 2020, 4:44 AM

Use IgnoreExpr.h infrastructure.

eduucaldas added inline comments.
clang/lib/Tooling/Syntax/BuildTree.cpp
48–58

Should this go into IgnoreExpr as well?

If yes, should we unify this with the lambda inside IgnoreUnlessSpelledInSource, thus removing the lambda and using this free function instead?

gribozavr2 accepted this revision.Aug 31 2020, 12:31 PM
gribozavr2 added inline comments.
clang/lib/Tooling/Syntax/BuildTree.cpp
48–58

That sounds like a good idea to me.

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
1749

Could you also add a test that invokes implicit conversions?

struct X {
  X(int);
};
void TakeX(const X&);

void test() {
  [[TakeX(1)]];
}
This revision is now accepted and ready to land.Aug 31 2020, 12:31 PM
eduucaldas updated this revision to Diff 289103.Sep 1 2020, 1:59 AM
eduucaldas marked 2 inline comments as done.

Add further tests and extract IgnoreImplicitConstructorSingleStep

gribozavr2 accepted this revision.Sep 1 2020, 2:18 AM
eduucaldas updated this revision to Diff 290223.Sep 7 2020, 2:36 AM

Add more tests, not extract IgnoreImplicitConstructorSingleStep

eduucaldas updated this revision to Diff 290259.Sep 7 2020, 5:39 AM

Add comments

eduucaldas added inline comments.Sep 7 2020, 5:45 AM
clang/lib/Tooling/Syntax/BuildTree.cpp
48–50

Please give feedback on this comments and should I comment the rest of the function?

gribozavr2 accepted this revision.Sep 7 2020, 10:30 AM
gribozavr2 added inline comments.
clang/lib/Tooling/Syntax/BuildTree.cpp
48
48–50

Seems straightforward to me.

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
4042

This is not a conversion, this is an explicit constructor call (CXXTemporaryObjectExpr) -- so please rename the test. Same for other tests below.

eduucaldas marked 3 inline comments as done.

answer comments

clang/unittests/Tooling/Syntax/BuildTreeTest.cpp
4042

I am trying to follow the syntax to name things. And in the grammar this is under "Explicit type conversion (functional notation)", which englobes as well things of the type float(3).

  • Rename tests
  • Add FIXMEs for init-declarators
gribozavr2 accepted this revision.Sep 8 2020, 1:38 AM