Closed Bug 1018995 Opened 6 years ago Closed 6 years ago

Make forward class declarations match their definitions

Categories

(Core :: JavaScript Engine, defect)

defect
Not set

Tracking

()

RESOLVED FIXED
mozilla32

People

(Reporter: jonco, Assigned: jonco)

Details

Attachments

(1 file)

I found another couple of places in NamespaceImports.h where the class declarations don't match their definitions.  This doesn't seem to be causing a problem right now, but should be fixed anyway.
Attachment #8432534 - Flags: review?(jdemooij)
Comment on attachment 8432534 [details] [diff] [review]
fix-forward-declarations

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

Thanks for checking these.

::: js/src/NamespaceImports.h
@@ +36,5 @@
>  template <typename T> class AutoVectorRooter;
>  template<typename K, typename V> class AutoHashMapRooter;
>  template<typename T> class AutoHashSetRooter;
>  
> +class MOZ_STACK_CLASS SourceBufferHolder;

I don't think this one matters, but can't hurt probably if it compiles..
Attachment #8432534 - Flags: review?(jdemooij) → review+
https://hg.mozilla.org/mozilla-central/rev/6ea33c3c5d66
Status: NEW → RESOLVED
Closed: 6 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
Jon, this part

>-class AutoGCRooter;
>+class JS_PUBLIC_API(AutoGCRooter);

causes gcc to emit this warning:

>In file included from ../../dist/include/js/Utility.h:26:0,
>                 from /home/jorendorff/dev/mozilla-inbound/js/src/jsalloc.h:18,
>                 from /home/jorendorff/dev/mozilla-inbound/js/src/jsapi.h:21,
>                 from /home/jorendorff/dev/mozilla-inbound/js/src/builtin/SIMD.h:10,
>                 from /home/jorendorff/dev/mozilla-inbound/js/src/builtin/SIMD.cpp:14:
>/home/jorendorff/dev/mozilla-inbound/js/src/NamespaceImports.h:35:21: warning: type attributes ignored after type is already defined [-Wattributes]
> class JS_PUBLIC_API(AutoGCRooter);
>                     ^
>/home/jorendorff/dev/mozilla-inbound/js/src/jstypes.h:73:41: note: in definition of macro ‘JS_PUBLIC_API’
> #  define JS_PUBLIC_API(t)   MOZ_EXPORT t
>                                         ^

Please undo that part of the change, r=me.
(In reply to Jason Orendorff [:jorendorff] from comment #4)

Hmmm.  Not having the same annotation for AutoCheckCannotGC caused a build break for the fuzzers - see bug 1013531 comment 12.

I guess for now we can just get rid of this one, but in future we may have to implement some compiler-dependent macro for forward declaring public API classes :-/

BTW which compiler/architecture is this on?  I don't see the warning myself.
Flags: needinfo?(jorendorff)
(In reply to Jon Coppeard (:jonco) from comment #5)
> I guess for now we can just get rid of this one, but in future we may have
> to implement some compiler-dependent macro for forward declaring public API
> classes :-/

Yeah.

> BTW which compiler/architecture is this on?  I don't see the warning myself.

Fedora, x86-64, gcc version 4.8.2 20131212 (Red Hat 4.8.2-7) (GCC)

Gone in tip, of course.
Flags: needinfo?(jorendorff)
You need to log in before you can comment on or make changes to this bug.