Closed Bug 909514 Opened 7 years ago Closed 7 years ago
Include <new> before mozilla::Maybe (and move Maybe into mfbt/Maybe
mozilla::Maybe uses placement new, so it should include <new>. In order to fix this, I need to move Maybe into its own file, because some random C++ test includes Util.h, and the compiler complains about infallible new when I use <new> in that file. I figure we want to do this anyway.
Whoops, forgot to include the removal of Maybe from Util.h in the previous patch.
Comment on attachment 795670 [details] [diff] [review] Patch, v2 Review of attachment 795670 [details] [diff] [review]: ----------------------------------------------------------------- ::: js/src/jsproxy.h @@ +8,5 @@ > #define jsproxy_h > > #include "jsapi.h" > #include "jsfriendapi.h" > +#include "mozilla/Maybe.h" This should move up above the jsapi.h include, into its own include-section. ::: mfbt/Maybe.h @@ +3,5 @@ > +/* This Source Code Form is subject to the terms of the Mozilla Public > + * License, v. 2.0. If a copy of the MPL was not distributed with this > + * file, You can obtain one at http://mozilla.org/MPL/2.0/. */ > + > +/* XXX */ /* A class for lazily constructing an object with stack-based lifetime. */ ? (although I'm not 100% certain Maybe is only used for stacked stuff, in practice -- probably this is something we should clarify with MOZ_STACK_CLASS if so) @@ +7,5 @@ > +/* XXX */ > + > +#ifndef mozilla_Maybe_h > +#define mozilla_Maybe_h > +#ifdef __cplusplus Get rid of this, this header's only useful in C++. (The same in Util.h dates back a really long time, to when JSAPI was C.) @@ +160,5 @@ > +}; > + > +} // namespace mozilla > +#endif /* __cplusplus */ > +#endif /* mozilla_Maybe_h */ A little breathing space: } // namespace mozilla #endif /* mozilla_Maybe_h */
Attachment #795670 - Flags: review?(jwalden+bmo) → review+
> ? (although I'm not 100% certain Maybe is only used for stacked stuff, in practice -- > probably this is something we should clarify with MOZ_STACK_CLASS if so) It's used inside a bunch of apparently non-stack classes. Without any deleterious effects, afaict. > +/* XXX */ Sorry I missed that!
I went with > /* A class for lazily constructing an object without sticking it on the heap. */
Let's make sure this builds everywhere: https://tbpl.mozilla.org/?tree=Try&rev=39c76321b6f1
Assignee: nobody → justin.lebar+bug
Status: NEW → RESOLVED
Closed: 7 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla26
Fun fact: adding jsapi.h to BindingDeclarations increased the number of built files that depend on jsapi.h from 1563 to 2157. It appears to be entirely unnecessary. I'll land a follow-up to remove it once I confirm that it builds everywhere.
> Fun fact: adding jsapi.h to BindingDeclarations increased the number of built files > that depend on jsapi.h from 1563 to 2157. Okay, but I didn't do that in this patch!
> Okay, but I didn't do that in this patch! You sure about that? https://hg.mozilla.org/integration/mozilla-inbound/annotate/535e7c112fc6/dom/bindings/BindingDeclarations.h
Huh. It was in my original patch as context, but you're right, I added it back. I must have screwed up the merge. Sorry about that!
You need to log in before you can comment on or make changes to this bug.