Thanks! Let me know if you want me to edit the 3 lines you pointed out and
the 1
I'm pointing out, or if you'll do that before landing. (I figured I'd miss
some;
http://build.chromium.org/p/chromium.fyi/builders/Cr%20Win%20Clang/builds/1226/steps/compile/logs/stdio
shows all these warnings and whatever I missed will be easier to see once
this
set of fixes is rolled into chromium.)
svenpanne:
If I understand the discussion on chromium-dev
(https://groups.google.com/a/chromium.org/forum/#!topic/chromium-dev/VTNZzizN0zo)
correctly, there are 2 steps after this:
* Replace "OVERRIDE" with "override" (and nuke our feature detection
for
this).
* Remove all "virtual"s when there is an "override", because the latter
implies the former (it's required by the Google3 style guide).
Correct. If v8 wants to require c++11 language features, then v8 can do this
too.
But we can't do
that right now because cpplint/presubmit doesn't like it yet.
Correct?
In chromium-land, the problem is that the plugin currently
requires "virtual" on
virtual method. dcheng is fixing this. v8 doesn't use the chromium clang
plugin
as far as I know, so this might not be a problem for you.
https://codereview.chromium.org/688533002/diff/1/src/compiler/ast-graph-builder.h
File src/compiler/ast-graph-builder.h (right):
https://codereview.chromium.org/688533002/diff/1/src/compiler/ast-graph-builder.h#newcode119
src/compiler/ast-graph-builder.h:119: virtual void
VisitDeclarations(ZoneList<Declaration*>* declarations);
On 2014/10/29 10:17:45, Jakob wrote:
Why don't we need OVERRIDE here? Oversight?
Yes, oversight.
https://code.google.com/p/v8/issues/attachmentText?id=3658&aid=36580003000&name=inconsistent-v8.txt&token=ABZ6GAcI-WqtssI8Gf3vBzcEUnI2u5meAQ%3A1414597070982#164
mentions it. Do you want to add it before landing, or do you want me to
upload another patch set?
https://codereview.chromium.org/688533002/diff/1/src/full-codegen.h
File src/full-codegen.h (right):
https://codereview.chromium.org/688533002/diff/1/src/full-codegen.h#newcode395
src/full-codegen.h:395: void VisitDeclarations(ZoneList<Declaration*>*
declarations);
On 2014/10/29 10:17:45, Jakob wrote:
Another |VisitDeclarations| that should be both virtual and OVERRIDE.
Yes (mentioned here:
https://code.google.com/p/v8/issues/attachmentText?id=3658&aid=36580003000&name=inconsistent-v8.txt&token=ABZ6GAcI-WqtssI8Gf3vBzcEUnI2u5meAQ%3A1414597070982#166).
Same question as on the other.
https://codereview.chromium.org/688533002/diff/1/src/typing.h
File src/typing.h (right):
https://codereview.chromium.org/688533002/diff/1/src/typing.h#newcode75
src/typing.h:75: void VisitDeclarations(ZoneList<Declaration*>*
declarations);
On 2014/10/29 10:17:45, Jakob wrote:
...and one more VisitDeclarations
Yes:
https://code.google.com/p/v8/issues/attachmentText?id=3658&aid=36580003000&name=inconsistent-v8.txt&token=ABZ6GAcI-WqtssI8Gf3vBzcEUnI2u5meAQ%3A1414597070982#357
https://codereview.chromium.org/688533002/diff/1/src/typing.h#newcode76
src/typing.h:76: void VisitStatements(ZoneList<Statement*>* statements);
This one too:
https://code.google.com/p/v8/issues/attachmentText?id=3658&aid=36580003000&name=inconsistent-v8.txt&token=ABZ6GAcI-WqtssI8Gf3vBzcEUnI2u5meAQ%3A1414597070982#358
https://codereview.chromium.org/688533002/
--
--
v8-dev mailing list
v8-dev@googlegroups.com
http://groups.google.com/group/v8-dev
---
You received this message because you are subscribed to the Google Groups "v8-dev" group.
To unsubscribe from this group and stop receiving emails from it, send an email
to v8-dev+unsubscr...@googlegroups.com.
For more options, visit https://groups.google.com/d/optout.
