348191 – Quick fix for catch by value should propose to replace by a const reference

Some Eclipse Foundation services are deprecated, or will be soon. Please ensure you've read this important communication.

Bug 348191 - Quick fix for catch by value should propose to replace by a const reference

Summary: Quick fix for catch by value should propose to replace by a const reference

Status:	RESOLVED FIXED

Alias:	None

Product:	CDT
Classification:	Tools
Component:	cdt-codan (show other bugs)
Version:	8.0
Hardware:	All All

Importance:	P3 enhancement (vote)
Target Milestone:	8.0.1
Assignee:	Marc-André Laperle
QA Contact:	Elena Laskavaia

URL:
Whiteboard:
Keywords:

Depends on:
Blocks:

Reported:	2011-06-03 06:40 EDT by Corentin Jabot
Modified:	2011-07-21 13:19 EDT (History)
CC List:	3 users (show)

See Also:

Attachments
added catch by const ref (6.19 KB, text/plain) 2011-06-16 09:50 EDT, Tomasz Wesolowski	no flags	Details
quick fix + test (9.32 KB, patch) 2011-07-19 15:59 EDT, Tomasz Wesolowski	no flags	Details \| Diff
quick fix + test (9.96 KB, patch) 2011-07-19 16:05 EDT, Tomasz Wesolowski	malaperle: iplog+	Details \| Diff
quick fix + test, modified (13.53 KB, patch) 2011-07-21 02:16 EDT, Marc-André Laperle	malaperle: iplog-	Details \| Diff
Show Obsolete (3) View All Add an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.

Description Corentin Jabot

2011-06-03 06:40:22 EDT

Build Identifier: 20110526-1053

When catching an exception by value instead of by reference, the quick fixed actually proposed by Codan, is to replace with a non-const reference.

But, in most cases an exception should be catch by const-reference.
So, it would be great to propose this fix instead, or even better, to have both choices.


Reproducible: Always

Steps to Reproduce:
1.Run code analysis on a snippet involving catching an exception by value
2.
3.

Comment 1 Tomasz Wesolowski

2011-06-16 09:50:11 EDT

Created attachment 198105 [details]
added catch by const ref

So I assume we want to keep both options as possibilities?

Comment 2 Elena Laskavaia

2011-07-15 21:21:03 EDT

can you provide a junit test please?

Comment 3 Marc-André Laperle

2011-07-15 21:55:08 EDT

I see that this appends 'const &' instead of pre-pending const and appending &. I think most people do const Exception &, not Exception const &, what do you think ? And it would be cool to have the const & quick fix listed first in the list of quick fixes but I'm not sure how easy it is to do that.

Comment 4 Tomasz Wesolowski

2011-07-19 15:59:33 EDT

Created attachment 199945 [details]
quick fix + test

Added test cases.

Also followed the suggestion to catch (const Exception & name).

Alena, does Codan already have a facility for prioritizing quick fixes? I agree that it sounds useful.

Comment 5 Tomasz Wesolowski

2011-07-19 16:05:06 EDT

Created attachment 199946 [details]
quick fix + test

(Fixed some comments)

Comment 6 Marc-André Laperle

2011-07-21 02:16:32 EDT

Created attachment 200053 [details]
quick fix + test, modified

Hi Tomasz. I modified a bit your patch:
- Changed CatchByReference and CatchByConstReference to use pretty much the same logic (it's just a static method...)
- Handle the case where there is no declName (bug 352263) and add related test cases
- Add test to suite
- Minor copyright updates

What do you think of this patch?

Comment 7 Tomasz Wesolowski

2011-07-21 11:50:30 EDT

I've diffed the patches (gits gets helpful :)) and reviewed your changes. I appreciate this version, thank you for the valuable input!

Comment 8 Marc-André Laperle

2011-07-21 13:05:27 EDT

Thanks! Fixed in cdt_8_0 and master > 20110721.

Comment 9 CDT Genie

2011-07-21 13:19:26 EDT

*** cdt git genie on behalf of 348191 ***

    Bug 348191 - Quick fix for catch by value should propose to replace by a
    const reference
    Bug 352263 - [qf] Catch by reference quick fix places & at a wrong place
    when no space after the type

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=db669ba9633e67facd61a36cc34361d8363e1e56

Comment 10 CDT Genie

2011-07-21 13:19:28 EDT

*** cdt git genie on behalf of 348191 ***

    Bug 348191 - Quick fix for catch by value should propose to replace by a
    const reference
    Bug 352263 - [qf] Catch by reference quick fix places & at a wrong place
    when no space after the type

[*] http://git.eclipse.org/c/cdt/org.eclipse.cdt.git/commit/?id=f434c8c5564989c2c0a578e74bde0ff7b70678de