Closed
Bug 541220
Opened 15 years ago
Closed 15 years ago
Integrate sixgill into build system
Categories
(Developer Infrastructure :: Source Code Analysis, defect)
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: taras.mozilla, Assigned: bhackett1024)
References
()
Details
Attachments
(1 file, 2 obsolete files)
(deleted),
patch
|
dwitte
:
review+
|
Details | Diff | Splinter Review |
See sixgill.org for more info.
Assignee | ||
Comment 1•15 years ago
|
||
Comment on attachment 422913 [details] [diff] [review] This patch adds macros for sixgill annotation attributes to nsDebug.h Benjamin, what do you think of these macros? I wonder if we want the macro names to be more like NS_XYZ.
Attachment #422913 -
Flags: review?(benjamin)
Comment 3•15 years ago
|
||
Comment on attachment 422913 [details] [diff] [review] This patch adds macros for sixgill annotation attributes to nsDebug.h Please don't introduce identifiers that start with two underscores... those are reserved for compilers. I wonder how generic we want to make this: if this is going to be specific to mozilla, I guess we should use NS_assert_static. If we want this to be sixgill-specific, then maybe have a sixgill header and use sg_assert_static? I don't have a strong preference except that I don't want naming conflicts with other stuff, so prefixes are probably necessary.
Attachment #422913 -
Flags: review?(benjamin) → review-
Assignee | ||
Comment 4•15 years ago
|
||
(In reply to comment #3) > I wonder how generic we want to make this: if this is going to be specific to > mozilla, I guess we should use NS_assert_static. If we want this to be > sixgill-specific, then maybe have a sixgill header and use sg_assert_static? Another alternative is to use macros that are not specific either to mozilla or sixgill. While sixgill is the only tool using these attributes, other tools should be able to use them in the future (currently the annotation parsing is done by the sixgill frontend plugin; I'd like to push this into gcc itself). STC_ASSERT // STATIC => STC_ STC_PRECONDITION STC_PRECONDITION_ASSUME ...
Comment 5•15 years ago
|
||
STATIC_ beats STC_ or other cybercrud, at least in Mozilla code. I hope gcc folks feel the same way (I would expect so, but my last gcc patch was in the '90s). /be
Assignee | ||
Comment 6•15 years ago
|
||
This patch uses STATIC_UPPERCASE macros for all the annotations, and does not make any '__' identifiers. I've tested this on my x86_64 machine and it works both with and without the sixgill plugin.
Assignee: nobody → bhackett1024
Attachment #422913 -
Attachment is obsolete: true
Status: NEW → ASSIGNED
Assignee | ||
Comment 7•15 years ago
|
||
This patch allows sixgill to check NS_ASSERTION macros when doing a release build (preferable (?) so we don't analyze debug code). Assertions referring to variables/functions that only exist in debug code will be ignored.
Attachment #423487 -
Attachment is obsolete: true
Assignee | ||
Updated•15 years ago
|
Attachment #438884 -
Flags: review?(dwitte)
Updated•15 years ago
|
Attachment #438884 -
Flags: review?(dwitte) → review+
Comment 8•15 years ago
|
||
Comment on attachment 438884 [details] [diff] [review] Updated patch that checks NS_ASSERTION in release and debug builds >diff -r 9614b3d45ff4 xpcom/glue/nsDebug.h >+/* Used to make identifiers for assert/assume annotations in a function. */ >+#define STATIC_PASTE2(X,Y) X ## Y >+#define STATIC_PASTE1(X,Y) STATIC_PASTE2(X,Y) What's the purpose of having two? Otherwise looks great. In general, debug code should be a superset of release code, but there's nothing forcing that -- so I agree, we definitely want to be able to analyze release code with the assistance of static assertions. r=dwitte
Comment 9•15 years ago
|
||
You need two levels of indirection if one of the tokens being pasted might be __LINE__.
Comment 10•15 years ago
|
||
> You need two levels of indirection if one of the tokens being pasted might be
> __LINE__.
Hilarious. :)
If this is ready to land, I'll stick it in my queue.
Comment 11•15 years ago
|
||
Comment on attachment 438884 [details] [diff] [review] Updated patch that checks NS_ASSERTION in release and debug builds http://hg.mozilla.org/mozilla-central/rev/ff91905fba1b
URL: http://sixgill.org/
Assignee | ||
Comment 12•15 years ago
|
||
Argh, it looks like this checkin has carriage returns at the end of every line (I think my Windows machine inserted them when I posted the patch here). Is there an easy way to get rid of these?
Assignee | ||
Comment 15•15 years ago
|
||
Looks like things are working good now. Thanks!
Status: ASSIGNED → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Updated•7 years ago
|
Product: Core → Firefox Build System
Updated•2 years ago
|
Product: Firefox Build System → Developer Infrastructure
You need to log in
before you can comment on or make changes to this bug.
Description
•