Move PR_STATIC_ASSERT from prlog.h to prtypes.h

RESOLVED FIXED in 4.8.6

Status

NSPR
NSPR
RESOLVED FIXED
8 years ago
7 years ago

People

(Reporter: zwol, Assigned: zwol)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment)

(Assignee)

Description

8 years ago
Created attachment 431958 [details] [diff] [review]
patch

PR_STATIC_ASSERT is wanted in code (e.g. headers) that doesn't want the side-effects of including prlog.h, so we move it to prtypes.h.
Attachment #431958 - Flags: superreview?(bzbarsky)
Attachment #431958 - Flags: review?(ted.mielczarek)
Attachment #431958 - Flags: superreview?(bzbarsky) → superreview+
Attachment #431958 - Flags: review?(ted.mielczarek) → review+
I landed this in CVS for you:
Checking in pr/include/prlog.h;
/cvsroot/mozilla/nsprpub/pr/include/prlog.h,v  <--  prlog.h
new revision: 3.18; previous revision: 3.17
done
Checking in pr/include/prtypes.h;
/cvsroot/mozilla/nsprpub/pr/include/prtypes.h,v  <--  prtypes.h
new revision: 3.42; previous revision: 3.41
done
Status: ASSIGNED → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
(Assignee)

Comment 2

8 years ago
Thanks for landing the patch.  I presume we need to wait for the next NSPR code drop before we can start assuming PR_STATIC_ASSERT is in prtypes.h?
Correct.
I marked that with TM 4.8.6 as this is the first RTM with that change AFAIK. Please correct if I'm wrong.
Target Milestone: --- → 4.8.6

Comment 5

7 years ago
Zack: What's the side effects of including prlog.h
that you don't want?

The reason PR_STATIC_ASSERT was defined in prlog.h
was that the similar PR_ASSERT macro (run-time
assertions) is defined in prlog.h.
(Assignee)

Comment 6

7 years ago
(In reply to comment #5)
> Zack: What's the side effects of including prlog.h
> that you don't want?

It decides whether PR_LOG is compiled in.  In some cases, we want to make that decision on a per-.cpp-file basis, *after* including many other headers.

Comment 7

7 years ago
Zack: thanks for the explanation.  Let me confirm
my understanding with a concret example.

This seems to be what you described:

----- Begin Foo.cpp -----

#include "prtypes.h"

#include "header1.h"
#include "header2.h"

#if defined(CONDITION1)
#define FORCE_PR_LOG 1
#endif

#include "prlog.h"

#include "header3.h"
#include "header4.h"

// Actual C++ code

----- End Foo.cpp -----

If I understand you correctly, the CONDITION1
macro cannot be tested until you have included
header1.h and header2.h, but you want to use
PR_STATIC_ASSERT in header1.h and header2.h.

Is this correct?
(Assignee)

Comment 8

7 years ago
Yeah, that's my understanding of it.  I can't point you to any concrete cases in mozilla, though.
You need to log in before you can comment on or make changes to this bug.