TaggedProto is a tagged pointer wrapper. It should be treated just like our other tagged pointer wrappers -- Value and jsid -- and not as a special snowflake.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=5ce999f72bc9 This appears to have involved two things, although the try run may still disagree. First, Rooted<TaggedProto> was storing this in the list of untagged JSObject* and then letting TraceNullableRoot's use of InternalGCMethods::isMarkable filter out tagged pointers. Of course this method doesn't actually do that -- it only does a null check. So. Second, we newly need to handle HeapPtr<TaggedProto>. This requires a bit of new boilerplate for GCMethods, InternalGCMethods, and BarrieredBaseMixins to pass through in the obvious way. Then it needed us to handle TraceEdge and friends. This required adding a new overload for all the internal methods we were overloading already for Value and jsid. This is a ton of copy-paste and we now have 3 instances at every location -- I plan to consolidate all of this next.

It looks like making InternalGCMethods<TaggedPointer>::preBarrier depend on InternalGCMethods<JSObject*>::preBarrier led to a requirement for JSObject to be instantiated before it was defined. At least on every platform other than clang on linux. Looks like we also need to take the preBarrier for Value and jsid out of line, so I probably should have expected this.

https://treeherder.mozilla.org/#/jobs?repo=try&revision=60d95053eb63 Split out vm/TaggedProto.h/cpp as its own file to resolve the circular requirements. Might have been able to get away with just moving the preBarrier def to cpp, but this seems clearer.

Comment on attachment 8662031 [details] [diff] [review] remove_isNullLike-v1.diff Review of attachment 8662031 [details] [diff] [review]: ----------------------------------------------------------------- > Bug _ - Unify per and post barriers; NOT_REVIEWED Should say "pre" if it's correct. (Also, I'd probably ask to split the code moving into a separate patch if I was reviewing.)

(In reply to :Ms2ger from comment #5) > Comment on attachment 8662031 [details] [diff] [review] > remove_isNullLike-v1.diff > > Review of attachment 8662031 [details] [diff] [review]: > ----------------------------------------------------------------- > > > Bug _ - Unify per and post barriers; NOT_REVIEWED > > Should say "pre" if it's correct. (Also, I'd probably ask to split the code > moving into a separate patch if I was reviewing.) As per the title that I forgot to update: this /is/ the split patch. :-P

Comment on attachment 8662031 [details] [diff] [review] remove_isNullLike-v1.diff Review of attachment 8662031 [details] [diff] [review]: ----------------------------------------------------------------- Makes sense to me, which probably means I'm fooling myself.

This broke the non-unified build (and the ARM64 simulator build). TaggedProto.cpp doesn't have any #includes.