Closed Bug 1158430 Opened 9 years ago Closed 9 years ago

"Only show this message once"

Categories

(Core :: Graphics, defect)

36 Branch
defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla40
Tracking Status
firefox40 --- fixed

People

(Reporter: milan, Assigned: milan)

References

Details

Attachments

(1 file, 1 obsolete file)

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.
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)
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)
Using Botond's suggestion and fixing the macro expansion.
Attachment #8597553 - Attachment is obsolete: true
Attachment #8598150 - Flags: review?(botond)
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+
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: nobody → milan
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/7516ef90ea60
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla40
You need to log in before you can comment on or make changes to this bug.