Is V8_DEPRECATED used consistently?

[Paweł Hajdan, Jr.]

According to https://code.google.com/p/v8/wiki/Source and confirmed by e.g. https://groups.google.com/d/msg/v8-users/aaSY-02HWzA/MP4gRTyB6Z4J all removals from v8 headers are to be tagged with V8_DEPRECATED.

Some examples (non-exhaustive for brevity) from a diff between 3.19 and 3.20 branches where V8_DEPRECATED isn't used but looks like it should be:

   /**

    * Returns the script id value.

+   * DEPRECATED: Please use GetId().

    */

   Local<Value> Id();

+  /**

+   * Returns scriptId object.

+   * DEPRECATED: use ScriptId() instead.

+   */

   Handle<Value> GetScriptId() const;

+    /**

+     * Deprecated. Never called directly by V8.

+     * For compatibility with legacy version of this interface.

+     */

+    virtual void Free(void* data);

+  // Deprecated, use Date::ValueOf() instead.

+  // TODO(svenpanne) Actually deprecate when Chrome is adapted.

+  double NumberValue() const { return ValueOf(); }

Actually I like the introduction of V8_DEPRECATED and its documentation on July 2013 (according to my reading of v8 wiki history). This is moving in a good direction, and will certainly be helpful for embedders and packagers. I appreciate v8 team's effort doing this.

Would it actually be possible to adapt the Chromium's WATCHLISTS mechanism (http://www.chromium.org/developers/contributing-code/watchlists) for v8? Most changes in v8 don't seem to touch the headers, and at least I'd find it useful to look at changes to the headers as they're being made.

Paweł

[Daniel Clifford]

I think you answered your own question: V8_DEPRECATED isn't always used consistently but should be. The APIs that are currently deprecated through comments are actually the ones that really should be V8_DEPRECATED, and the current V8_DEPRECATED APIs should already be purged from the source altogether, since by the time they get marked this way currently their tests have been removed. In any case, Dan Carney has graciously offered to clean up the usage of the macro so that it is consistent with the documentation on the wiki.

Adding WATCHLISTS support seems reasonable. I'll add it to the pending feature list of infrastructure projects.

Danno

--
--
v8-users mailing list
v8-u...@googlegroups.com
http://groups.google.com/group/v8-users
---
You received this message because you are subscribed to the Google Groups "v8-users" group.
To unsubscribe from this group and stop receiving emails from it, send an email to v8-users+u...@googlegroups.com.
For more options, visit https://groups.google.com/groups/opt_out.

[Dan Carney]

On Monday, September 30, 2013 9:36:04 AM UTC+2, danno wrote:

I think you answered your own question: V8_DEPRECATED isn't always used consistently but should be. The APIs that are currently deprecated through comments are actually the ones that really should be V8_DEPRECATED, and the current V8_DEPRECATED APIs should already be purged from the source altogether, since by the time they get marked this way currently their tests have been removed. In any case, Dan Carney has graciously offered to clean up the usage of the macro so that it is consistent with the documentation on the wiki.

The cleaned up api will be in the trunk revision.

V8_DEPRECATED now does nothing unless V8_DEPRECATION_WARNINGS is defined.

In the future, we will try to mark every function with V8_DEPRECATED that we intend to remove.  Please note that some such functions may be scheduled for removal very quickly, while some will by necessity hang around for months.

[Paweł Hajdan, Jr.]

On Mon, Sep 30, 2013 at 12:36 AM, Daniel Clifford <da...@chromium.org> wrote:

I think you answered your own question: V8_DEPRECATED isn't always used consistently but should be. The APIs that are currently deprecated through comments are actually the ones that really should be V8_DEPRECATED, and the current V8_DEPRECATED APIs should already be purged from the source altogether, since by the time they get marked this way currently their tests have been removed. In any case, Dan Carney has graciously offered to clean up the usage of the macro so that it is consistent with the documentation on the wiki.

Adding WATCHLISTS support seems reasonable. I'll add it to the pending feature list of infrastructure projects.

It's actually easy. I uploaded https://codereview.chromium.org/26574009/ .

Paweł