Rietveld Code Review Tool
Help | Bug tracker | Discussion group | Source code | Sign in
(19)

Issue 19330044: Fix build on QNX. (Closed)

Can't Edit
Can't Publish+Mail
Start Review
Created:
5 years, 9 months ago by efidler1
Modified:
5 years, 9 months ago
CC:
angleproject-review_googlegroups.com
Base URL:
https://chromium.googlesource.com/external/angle@master
Visibility:
Public.

Description

Fix build on QNX. InfoSink.h needs stdlib.h for abs(int) and free() in the global namespace. ExpressionParser needs malloc.h, because bison needs malloc and free in the global namespace, but "#include <cassert>" will put it only in the std:: namespace on QNX. BUG=500

Patch Set 1 #

Total comments: 1

Patch Set 2 : Fix ANGLE build on Mac. #

Total comments: 2

Patch Set 3 : use _MSC_VER instead of COMPILER_MSVC #

Unified diffs Side-by-side diffs Delta from patch set Stats (+10 lines, -0 lines) Patch
M src/compiler/preprocessor/ExpressionParser.cpp View 1 2 1 chunk +5 lines, -0 lines 0 comments Download
M src/compiler/preprocessor/ExpressionParser.y View 1 2 1 chunk +5 lines, -0 lines 0 comments Download

Messages

Total messages: 16
Geoff Lang
LGTM
5 years, 9 months ago (2013-10-29 20:26:59 UTC) #1
Geoff Lang
Also, can you add shannonwoods@chromium.org as a reviewer (be sure to add a message such ...
5 years, 9 months ago (2013-10-29 20:29:04 UTC) #2
efidler1
PTAL
5 years, 9 months ago (2013-10-29 20:30:46 UTC) #3
Shannon Woods
On 2013/10/29 20:30:46, efidler1 wrote: > PTAL LGTM! Thank you. One of us will land ...
5 years, 9 months ago (2013-10-29 21:00:35 UTC) #4
Jamie Madill
On 2013/10/29 21:00:35, Shannon Woods wrote: > On 2013/10/29 20:30:46, efidler1 wrote: > > PTAL ...
5 years, 9 months ago (2013-10-30 21:58:42 UTC) #5
Zhenyao Mo
https://codereview.appspot.com/19330044/diff/1/src/compiler/preprocessor/ExpressionParser.cpp File src/compiler/preprocessor/ExpressionParser.cpp (right): https://codereview.appspot.com/19330044/diff/1/src/compiler/preprocessor/ExpressionParser.cpp#newcode95 src/compiler/preprocessor/ExpressionParser.cpp:95: #include <malloc.h> Include this file causes Mac builds to ...
5 years, 9 months ago (2013-11-01 00:08:57 UTC) #6
Zhenyao Mo
See examples in chromium: https://code.google.com/p/chromium/codesearch#chromium/src/base/memory/aligned_memory.h&l=41
5 years, 9 months ago (2013-11-01 00:10:54 UTC) #7
efidler1
My apologies. stdlib.h should be fine for QNX instead of malloc.h, so that block should ...
5 years, 9 months ago (2013-11-01 01:27:19 UTC) #8
efidler1
confirmed, using the version from base/memory/aligned_memory.h works for QNX. I'm not set up to test ...
5 years, 9 months ago (2013-11-01 03:17:03 UTC) #9
Zhenyao Mo
On 2013/11/01 03:17:03, efidler1 wrote: > confirmed, using the version from base/memory/aligned_memory.h works for QNX. ...
5 years, 9 months ago (2013-11-01 03:22:10 UTC) #10
efidler1
uploaded
5 years, 9 months ago (2013-11-01 14:54:39 UTC) #11
Jamie Madill
On 2013/11/01 14:54:39, efidler1 wrote: > uploaded LGTM... going to land unless there's any objections ...
5 years, 9 months ago (2013-11-01 15:05:54 UTC) #12
Zhenyao Mo
https://codereview.appspot.com/19330044/diff/20001/src/compiler/preprocessor/ExpressionParser.cpp File src/compiler/preprocessor/ExpressionParser.cpp (right): https://codereview.appspot.com/19330044/diff/20001/src/compiler/preprocessor/ExpressionParser.cpp#newcode95 src/compiler/preprocessor/ExpressionParser.cpp:95: #if defined(COMPILER_MSVC) Actually it should be _MSC_VER COMPILER_MSVC is ...
5 years, 9 months ago (2013-11-01 17:09:48 UTC) #13
efidler1
updated
5 years, 9 months ago (2013-11-01 17:46:37 UTC) #14
Zhenyao Mo
On 2013/11/01 17:46:37, efidler1 wrote: > updated LGTM thanks
5 years, 9 months ago (2013-11-01 20:11:09 UTC) #15
Jamie Madill
5 years, 9 months ago (2013-11-04 15:19:31 UTC) #16
On 2013/11/01 20:11:09, Zhenyao Mo wrote:
> On 2013/11/01 17:46:37, efidler1 wrote:
> > updated
> 
> LGTM
> thanks

landed... efidler1, can you close this review? (under edit issue) I don't have
permission.
Sign in to reply to this message.

Powered by Google App Engine
RSS Feeds Recent Issues | This issue
This is Rietveld f62528b