442968 - Format outparams messages in the standard way

Status: RESOLVED FIXED

(Developer Infrastructure :: Source Code Analysis, defect)

Description

[Benjamin Smedberg]

Attachment: better warnings from outparams, rev. 1

The current outparams warnings are multiline and those lines are indented. This makes it easier to read in some cases, but tools such as compile-mode don't linkify the message correctly. This patch uses the standard GCC formatting so that locations are parsed correctly. It also gets rid of the bogus location reported at the beginning of the warning message.

Comment 1

[Benjamin Smedberg]

Oh, the resulting output looks like this:

../../../../src/xpcom/proxy/tests/proxy-create-threadsafety.cpp: In member function ‘virtual nsresult ProxyTest::Test(PRInt32, PRInt32, PRInt32*)’:
../../../../src/xpcom/proxy/tests/proxy-create-threadsafety.cpp:147: warning: outparam '_retval' not written on NS_SUCCEEDED(return value)
../../../../src/xpcom/proxy/tests/proxy-create-threadsafety.cpp:121:   outparam declared here

Comment 2

[Benjamin Smedberg]

And the commented-out code was an attempt to get the location of the un-annotated function call. It isn't working because the ss.getBlame(v) isn't a call expression, it's a GIMPLE_MODIFY_STMT. I'm guessing the CALL_EXPR was nested in the gimple statement, but I didn't follow up.

Comment 3

[David Mandelin [:dmandelin]]

(In reply to comment #2)
> And the commented-out code was an attempt to get the location of the
> un-annotated function call. It isn't working because the ss.getBlame(v) isn't a
> call expression, it's a GIMPLE_MODIFY_STMT. I'm guessing the CALL_EXPR was
> nested in the gimple statement, but I didn't follow up.

If 'isn' is a GIMPLE_MODIFY_STMT that contains a call, then isn.operands()[1] is a CALL_EXPR. I assume you're trying to get the location of the decl from that then?

Comment 4

[Benjamin Smedberg]

Attachment: better warnings from outparams, rev. 1.1

This adds maybewritten declaration goodness.

Comment 5

[Benjamin Smedberg]

Pushed, revision ad4228bbd961

Comment 6

[David Mandelin [:dmandelin]]

Attachment: Small additional fix

The original patch broke the test case e13.cpp, because that method returns void, so this.retvar is void, so ss.getBlame(this.retvar) fails. Here is a fix. The error message is still a little funny because it returns to NS_SUCCEEDED(return value) instead of noting that the return value is void, so the method is assumed to always succeed.

Comment 7

[David Mandelin [:dmandelin]]

Pushed.