Format outparams messages in the standard way

RESOLVED FIXED in mozilla1.9.1a1

Status

RESOLVED FIXED
10 years ago
8 months ago

People

(Reporter: benjamin, Assigned: benjamin)

Tracking

unspecified
mozilla1.9.1a1
x86
Linux

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments, 1 obsolete attachment)

(Assignee)

Description

10 years ago
Created attachment 327622 [details] [diff] [review]
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.
Attachment #327622 - Flags: review?(dmandelin)
(Assignee)

Comment 1

10 years ago
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
(Assignee)

Comment 2

10 years ago
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.
Attachment #327622 - Flags: review?(dmandelin) → review+
(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?

(Assignee)

Comment 4

10 years ago
Created attachment 327632 [details] [diff] [review]
better warnings from outparams, rev. 1.1

This adds maybewritten declaration goodness.
Attachment #327622 - Attachment is obsolete: true
Attachment #327632 - Flags: review?(dmandelin)
Attachment #327632 - Flags: review?(dmandelin) → review+
(Assignee)

Comment 5

10 years ago
Pushed, revision ad4228bbd961
Status: NEW → RESOLVED
Last Resolved: 10 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla1.9.1a1
Created attachment 327706 [details] [diff] [review]
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.
Attachment #327706 - Flags: review?
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Attachment #327706 - Flags: review? → review?(benjamin)
(Assignee)

Updated

10 years ago
Attachment #327706 - Flags: review?(benjamin) → review+
Pushed.
Status: REOPENED → RESOLVED
Last Resolved: 10 years ago10 years ago
Resolution: --- → FIXED

Updated

8 months ago
Product: Core → Firefox Build System
You need to log in before you can comment on or make changes to this bug.