Closed
Bug 1158430
Opened 9 years ago
Closed 9 years ago
"Only show this message once"
Categories
(Core :: Graphics, defect)
Tracking
()
RESOLVED
FIXED
mozilla40
Tracking | Status | |
---|---|---|
firefox40 | --- | fixed |
People
(Reporter: milan, Assigned: milan)
References
Details
Attachments
(1 file, 1 obsolete file)
5.18 KB,
patch
|
botond
:
review+
|
Details | Diff | Splinter Review |
Not convinced it's worth it, but kept running into the pattern like this in the last few days: static bool firstTime = true; if (firstTime) { firstTime = false; gfxCriticalError() << "message ..."; } and thought maybe make it a bit shorter. The closest I got to is: gfxCriticalOnce("message ..."); I'll attach a patch.
Assignee | ||
Comment 1•9 years ago
|
||
Attachment #8597553 -
Flags: feedback?(botond)
Attachment #8597553 -
Flags: feedback?(bas)
Comment 2•9 years ago
|
||
Comment on attachment 8597553 [details] [diff] [review] Once only critical error - code outside Logging.h are just examples. Review of attachment 8597553 [details] [diff] [review]: ----------------------------------------------------------------- I think we can do this without changing the invocation syntax to varargs: ------------------ struct Dummy { // Be implicitly constructible from a gfxCriticalError, but do nothing with it. MOZ_IMPLICIT Dummy(const gfxCriticalError&) {} }; #define gfxCriticalOnce static Dummy sOnlyOnceAtLine##__LINE__ = gfxCriticalError ------------------ Now consider a call site, with unchanged invocation syntax: ------------------ gfxCriticalOnce(CriticalLog::DefaultOptions(false)) << "message ..."; ------------------ The macro will expand to: ------------------ static Dummy sOnlyOnceAtLine##__LINE__ = gfxCriticalError(CriticalLog::DefaultOptions(false)) << "message ..."; ------------------ The static variable's initializer will be evaluated only once, when the function is first called. The temporary gfxCriticalError object will then be destroyed at the end of the statement (again, only when the function is first called). The only purpose of the Dummy class is to allow the gfxCriticalError object to be the initializer of some static variable, without the variable keeping around the gfxCriticalError, or doing anything in *its* destructor.
Attachment #8597553 -
Flags: feedback?(botond)
Assignee | ||
Comment 3•9 years ago
|
||
Comment on attachment 8597553 [details] [diff] [review] Once only critical error - code outside Logging.h are just examples. I want to try Botond's magic first.
Attachment #8597553 -
Flags: feedback?(bas)
Assignee | ||
Comment 4•9 years ago
|
||
Using Botond's suggestion and fixing the macro expansion.
Attachment #8597553 -
Attachment is obsolete: true
Attachment #8598150 -
Flags: review?(botond)
Comment 5•9 years ago
|
||
Comment on attachment 8598150 [details] [diff] [review] Once only gfx errors and warnings. r=botond Review of attachment 8598150 [details] [diff] [review]: ----------------------------------------------------------------- This is mostly an aside, but C++11 has a feature called "delegating constructors", which we can use as of Gecko 38 [1], which would allow us to write the constructors like so: // This constructor is unmodified. explicit Log(int aOptions = Log::DefaultOptions(L == LOG_CRITICAL)) : mOptions(aOptions) , mLogIt(BasicLogger::ShouldOutputMessage(L)) { if (mLogIt && AutoPrefix()) { ... } } // This constructor delegates to the other one. MOZ_IMPLICIT Log(const Log& log) : Log(log.mOptions, false) {} Up to you if you'd like to use this. [1] https://developer.mozilla.org/en-US/docs/Using_CXX_in_Mozilla_code
Attachment #8598150 -
Flags: review?(botond) → review+
Assignee | ||
Comment 6•9 years ago
|
||
Didn't realize 38 was the boundary for constructor calling constructors on all platform. I'll leave it as is for now. https://treeherder.mozilla.org/#/jobs?repo=try&revision=2be675803085
Assignee | ||
Updated•9 years ago
|
Assignee: nobody → milan
Keywords: checkin-needed
Comment 9•9 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/7516ef90ea60
Status: NEW → RESOLVED
Closed: 9 years ago
status-firefox40:
--- → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in
before you can comment on or make changes to this bug.
Description
•