Closed Bug 867426 Opened 7 years ago Closed 7 years ago

GC: remove the Raw* typedefs

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla23

People

(Reporter: terrence, Assigned: terrence)

References

Details

Attachments

(14 files)

15.03 KB, patch
jonco
: review+
Details | Diff | Splinter Review
4.94 KB, patch
jonco
: review+
Details | Diff | Splinter Review
3.02 KB, patch
jonco
: review+
Details | Diff | Splinter Review
14.17 KB, patch
jonco
: review+
Details | Diff | Splinter Review
15.28 KB, patch
jonco
: review+
Details | Diff | Splinter Review
126.60 KB, patch
jonco
: review+
Details | Diff | Splinter Review
1.49 KB, patch
jonco
: review+
Details | Diff | Splinter Review
22.13 KB, patch
jonco
: review+
Details | Diff | Splinter Review
170.45 KB, patch
jonco
: review+
Details | Diff | Splinter Review
19.40 KB, patch
jonco
: review+
Details | Diff | Splinter Review
3.30 KB, patch
jonco
: review+
Details | Diff | Splinter Review
26.30 KB, patch
jonco
: review+
Details | Diff | Splinter Review
149.82 KB, patch
jonco
: review+
Details | Diff | Splinter Review
1.37 KB, patch
jonco
: review+
Details | Diff | Splinter Review
With the static analysis in-place and working, the Raw typedefs are looking rather silly and vestigial. Removing them will reduce our type burden further and allow us to kill the ForwardDeclare macros with fire.

The following series of patches was constructed largely with find, grep, sed and a bunch of manual sanity checking. I think I caught all the places that would have thrown off spacing, but some thoughtful spot-checking would still probably be worthwhile.
Attached patch v0: RawAtomSplinter Review
Attachment #743898 - Flags: review?(jcoppeard)
Attachment #743899 - Flags: review?(jcoppeard)
Attachment #743900 - Flags: review?(jcoppeard)
Attached patch v0: RawBaseShapeSplinter Review
Attachment #743901 - Flags: review?(jcoppeard)
Attachment #743902 - Flags: review?(jcoppeard)
Attached patch v0: RawShapeSplinter Review
Attachment #743903 - Flags: review?(jcoppeard)
Attachment #743904 - Flags: review?(jcoppeard)
Attached patch v0: RawFunctionSplinter Review
Attachment #743905 - Flags: review?(jcoppeard)
Attached patch v0: RawScriptSplinter Review
Attachment #743906 - Flags: review?(jcoppeard)
Attached patch v0: RawStringSplinter Review
Attachment #743907 - Flags: review?(jcoppeard)
Attached patch v0: RawValueSplinter Review
Attachment #743908 - Flags: review?(jcoppeard)
Attached patch v0: RawIdSplinter Review
Attachment #743909 - Flags: review?(jcoppeard)
Attached patch v0: RawObjectSplinter Review
Attachment #743910 - Flags: review?(jcoppeard)
Attachment #743911 - Flags: review?(jcoppeard)
In case some of these leaked into the browser in such a way that ack can't find them, try run is up at:
https://tbpl.mozilla.org/?tree=Try&rev=a100e01c2b6c
Attachment #743898 - Flags: review?(jcoppeard) → review+
Attachment #743899 - Flags: review?(jcoppeard) → review+
Attachment #743900 - Flags: review?(jcoppeard) → review+
Attachment #743901 - Flags: review?(jcoppeard) → review+
Attachment #743902 - Flags: review?(jcoppeard) → review+
Attachment #743903 - Flags: review?(jcoppeard) → review+
Attachment #743904 - Flags: review?(jcoppeard) → review+
Attachment #743905 - Flags: review?(jcoppeard) → review+
Attachment #743906 - Flags: review?(jcoppeard) → review+
Attachment #743907 - Flags: review?(jcoppeard) → review+
Attachment #743908 - Flags: review?(jcoppeard) → review+
Attachment #743909 - Flags: review?(jcoppeard) → review+
Comment on attachment 743910 [details] [diff] [review]
v0: RawObject

Review of attachment 743910 [details] [diff] [review]:
-----------------------------------------------------------------

No problems with the patch, just a couple of incidental things I noticed:

::: js/src/gc/Marking.cpp
@@ +701,1 @@
>                                        const char *name)

We could fix the alignment here.

::: js/src/gc/RootMarking.cpp
@@ +488,5 @@
>        case OBJOBJHASHMAP: {
>          AutoObjectObjectHashMap::HashMapImpl &map = static_cast<AutoObjectObjectHashMap *>(this)->map;
>          for (AutoObjectObjectHashMap::Enum e(map); !e.empty(); e.popFront()) {
> +            mozilla::DebugOnly<JSObject *> key = e.front().key;
> +            MarkObjectRoot(trc, (JSObject **) &e.front().key, "AutoObjectObjectHashMap key");

Should we be using c++ casts here (and the following cases)?
Attachment #743910 - Flags: review?(jcoppeard) → review+
Attachment #743911 - Flags: review?(jcoppeard) → review+
You need to log in before you can comment on or make changes to this bug.