Discussion:
Review Request 127027: Add support for CXType_FunctionNoProto
(too old to reply)
Pedro Ferreira
2016-02-10 11:19:59 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127027/
-----------------------------------------------------------

Review request for KDevelop.


Repository: kdevelop


Description
-------

Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto.
Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them.


Diffs
-----

languages/clang/duchain/builder.cpp 22d00f0

Diff: https://git.reviewboard.kde.org/r/127027/diff/


Testing
-------

I found this by looking at the output of a running kdev session.

"kdevelop.plugins.clang: Unhandled type: 110"

Once I added this change, that output disappeared.


Thanks,

Pedro Ferreira
Kevin Funk
2016-02-10 11:58:00 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127027/#review92209
-----------------------------------------------------------



Thanks!

Could you extend our unit tests a bit?

Extending `languages/clang/tests/files/functions.cpp` would be a good idea.

If I understood http://clang.llvm.org/doxygen/classclang_1_1FunctionNoProtoType.html correctly, then the FunctionNoProto type represents an argumentless function declaration.

Please make sure the new unit test fails before your patch, and succeeds afterwards.

- Kevin Funk
Post by Pedro Ferreira
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127027/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 11:19 a.m.)
Review request for KDevelop.
Repository: kdevelop
Description
-------
Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto.
Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them.
Diffs
-----
languages/clang/duchain/builder.cpp 22d00f0
Diff: https://git.reviewboard.kde.org/r/127027/diff/
Testing
-------
I found this by looking at the output of a running kdev session.
"kdevelop.plugins.clang: Unhandled type: 110"
Once I added this change, that output disappeared.
Thanks,
Pedro Ferreira
Kevin Funk
2016-02-10 12:18:01 UTC
Permalink
Post by Kevin Funk
Thanks!
Could you extend our unit tests a bit?
Extending `languages/clang/tests/files/functions.cpp` would be a good idea.
If I understood http://clang.llvm.org/doxygen/classclang_1_1FunctionNoProtoType.html correctly, then the FunctionNoProto type represents an argumentless function declaration.
Please make sure the new unit test fails before your patch, and succeeds afterwards.
Hm, sure, I'll contrive a test for it. Thank you for pointing out where I should be adding it!
I intended to include my real-world source that triggered this, but the source is unfortunately confidential.
Will get back to you soon.
Just noticed: The K&R-style function decl syntax is a C-only feature.

In that case we have `tests/files/purec.c`, which unfortunately is useless atm (unit tests assume it's a C++ file). If you want to fix that, sure, give it a try.

Feel free to push your patch without a unit test for now.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127027/#review92209
-----------------------------------------------------------
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127027/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 11:19 a.m.)
Review request for KDevelop.
Repository: kdevelop
Description
-------
Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto.
Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them.
Diffs
-----
languages/clang/duchain/builder.cpp 22d00f0
Diff: https://git.reviewboard.kde.org/r/127027/diff/
Testing
-------
I found this by looking at the output of a running kdev session.
"kdevelop.plugins.clang: Unhandled type: 110"
Once I added this change, that output disappeared.
Thanks,
Pedro Ferreira
Aleix Pol Gonzalez
2016-02-10 12:05:04 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127027/#review92211
-----------------------------------------------------------



It would make sense to include a unit test.


languages/clang/duchain/builder.cpp (line 1259)
<https://git.reviewboard.kde.org/r/127027/#comment62901>

I wouldn't add this in this case, I understand your premise but we're not doing this anywhere else and wouldn't be consistent.


- Aleix Pol Gonzalez
Post by Pedro Ferreira
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127027/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 12:19 p.m.)
Review request for KDevelop.
Repository: kdevelop
Description
-------
Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto.
Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them.
Diffs
-----
languages/clang/duchain/builder.cpp 22d00f0
Diff: https://git.reviewboard.kde.org/r/127027/diff/
Testing
-------
I found this by looking at the output of a running kdev session.
"kdevelop.plugins.clang: Unhandled type: 110"
Once I added this change, that output disappeared.
Thanks,
Pedro Ferreira
Kevin Funk
2016-02-10 12:31:45 UTC
Permalink
Post by Pedro Ferreira
languages/clang/duchain/builder.cpp, line 1259
<https://git.reviewboard.kde.org/r/127027/diff/1/?file=443886#file443886line1259>
I wouldn't add this in this case, I understand your premise but we're not doing this anywhere else and wouldn't be consistent.
It's a "function local" define. Undefining it that case is fine. So +1 from my side.


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127027/#review92211
-----------------------------------------------------------
Post by Pedro Ferreira
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127027/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 11:19 a.m.)
Review request for KDevelop.
Repository: kdevelop
Description
-------
Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto.
Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them.
Diffs
-----
languages/clang/duchain/builder.cpp 22d00f0
Diff: https://git.reviewboard.kde.org/r/127027/diff/
Testing
-------
I found this by looking at the output of a running kdev session.
"kdevelop.plugins.clang: Unhandled type: 110"
Once I added this change, that output disappeared.
Thanks,
Pedro Ferreira
Kevin Funk
2016-02-10 12:31:49 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127027/#review92215
-----------------------------------------------------------


Ship it!




Ship It!

- Kevin Funk
Post by Pedro Ferreira
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127027/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 11:19 a.m.)
Review request for KDevelop.
Repository: kdevelop
Description
-------
Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto.
Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them.
Diffs
-----
languages/clang/duchain/builder.cpp 22d00f0
Diff: https://git.reviewboard.kde.org/r/127027/diff/
Testing
-------
I found this by looking at the output of a running kdev session.
"kdevelop.plugins.clang: Unhandled type: 110"
Once I added this change, that output disappeared.
Thanks,
Pedro Ferreira
Milian Wolff
2016-02-10 14:11:47 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127027/#review92222
-----------------------------------------------------------


Ship it!




please add a reference to this review request to the bug, that way we can add a proper unit test later on.

- Milian Wolff
Post by Pedro Ferreira
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127027/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 11:19 a.m.)
Review request for KDevelop.
Repository: kdevelop
Description
-------
Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto.
Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them.
Diffs
-----
languages/clang/duchain/builder.cpp 22d00f0
Diff: https://git.reviewboard.kde.org/r/127027/diff/
Testing
-------
I found this by looking at the output of a running kdev session.
"kdevelop.plugins.clang: Unhandled type: 110"
Once I added this change, that output disappeared.
Thanks,
Pedro Ferreira
Kevin Funk
2016-02-10 14:38:27 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127027/#review92224
-----------------------------------------------------------


Ship it!




@Pedro: Got commit access?

- Kevin Funk
Post by Pedro Ferreira
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127027/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 11:19 a.m.)
Review request for KDevelop.
Bugs: https://bugs.kde.org/show_bug.cgi?id=357615
http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=357615
Repository: kdevelop
Description
-------
Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto.
Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them.
Diffs
-----
languages/clang/duchain/builder.cpp 22d00f0
Diff: https://git.reviewboard.kde.org/r/127027/diff/
Testing
-------
I found this by looking at the output of a running kdev session.
"kdevelop.plugins.clang: Unhandled type: 110"
Once I added this change, that output disappeared.
Thanks,
Pedro Ferreira
Kevin Funk
2016-02-10 16:22:29 UTC
Permalink
Post by Kevin Funk
@Pedro: Got commit access?
I do not.
I don't think I'd want it either - that's a recipe for disaster! :P
You're too kind :)

Thanks for your patch. Pushed!


- Kevin


-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127027/#review92224
-----------------------------------------------------------
-----------------------------------------------------------
https://git.reviewboard.kde.org/r/127027/
-----------------------------------------------------------
(Updated Feb. 10, 2016, 4:22 p.m.)
Review request for KDevelop.
Bugs: https://bugs.kde.org/show_bug.cgi?id=357615
http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=357615
Repository: kdevelop
Description
-------
Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto.
Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them.
Diffs
-----
languages/clang/duchain/builder.cpp 22d00f0
Diff: https://git.reviewboard.kde.org/r/127027/diff/
Testing
-------
I found this by looking at the output of a running kdev session.
"kdevelop.plugins.clang: Unhandled type: 110"
Once I added this change, that output disappeared.
Thanks,
Pedro Ferreira
Pedro Ferreira
2016-02-10 16:22:07 UTC
Permalink
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/127027/
-----------------------------------------------------------

(Updated Feb. 10, 2016, 4:22 p.m.)


Status
------

This change has been marked as submitted.


Review request for KDevelop.


Changes
-------

Submitted with commit b930278bb44b37742bcbc19b12efeeb8026e1a3a by Kevin Funk on behalf of Pedro Ferreira to branch 5.0.


Bugs: https://bugs.kde.org/show_bug.cgi?id=357615
http://bugs.kde.org/show_bug.cgi?id=https://bugs.kde.org/show_bug.cgi?id=357615


Repository: kdevelop


Description
-------

Add support for CXType_FunctionNoProto as having the same behaviour as CXType_FunctionProto.
Also, undefine a local macro. This doesn't fix any problems, but I believe its a Good Thing to clean up local macros when we're done with them.


Diffs
-----

languages/clang/duchain/builder.cpp 22d00f0

Diff: https://git.reviewboard.kde.org/r/127027/diff/


Testing
-------

I found this by looking at the output of a running kdev session.

"kdevelop.plugins.clang: Unhandled type: 110"

Once I added this change, that output disappeared.


Thanks,

Pedro Ferreira
Loading...