Preparation for macro expansion.

Preparation for macro expansion.
Committed: http://code.google.com/p/angleproject/source/detail?r=741

[kbr1, 2011-08-23 02:06:09]

LGTM overall. One high-level comment echoing Daniel's concerns from another of
your recent patches. The actual macro evaluation is going to be interesting and
tricky. Is there any specification you can look to?

http://codereview.appspot.com/4919045/diff/1/src/compiler/preprocessor/new/Co...
File src/compiler/preprocessor/new/Context.cpp (right):

http://codereview.appspot.com/4919045/diff/1/src/compiler/preprocessor/new/Co...
src/compiler/preprocessor/new/Context.cpp:55: mMacros[*identifier] = new
Macro(type, identifier, parameters, replacements);
As Daniel mentioned we need to think about error handling. How would allocation
failures be handled and reported here and throughout? In Chrome there is likely
no issue because of the out-of-memory killer, but assuming this code sees
broader use we really need to do a good job with handling allocation failures
gracefully.

[Alok Priyadarshi, 2011-08-24 17:46:00]

On 2011/08/23 02:06:09, kbr1 wrote:
> LGTM overall. One high-level comment echoing Daniel's concerns from another of
> your recent patches. The actual macro evaluation is going to be interesting
and
> tricky. Is there any specification you can look to?

I am looking at GLSL spec and GCC manual -
http://gcc.gnu.org/onlinedocs/cpp/index.html. Please let me know if you know of
a better reference.

>
http://codereview.appspot.com/4919045/diff/1/src/compiler/preprocessor/new/Co...
> File src/compiler/preprocessor/new/Context.cpp (right):
> 
>
http://codereview.appspot.com/4919045/diff/1/src/compiler/preprocessor/new/Co...
> src/compiler/preprocessor/new/Context.cpp:55: mMacros[*identifier] = new
> Macro(type, identifier, parameters, replacements);
> As Daniel mentioned we need to think about error handling. How would
allocation
> failures be handled and reported here and throughout? In Chrome there is
likely
> no issue because of the out-of-memory killer, but assuming this code sees
> broader use we really need to do a good job with handling allocation failures
> gracefully.

Daniel: Could you point me to the out-of-memory bug?

[dgkoch, 2011-08-30 03:08:37]

overall LGTM as well.  A few comments.

http://codereview.appspot.com/4919045/diff/5001/src/compiler/preprocessor/new...
File src/compiler/preprocessor/new/Context.cpp (right):

http://codereview.appspot.com/4919045/diff/5001/src/compiler/preprocessor/new...
src/compiler/preprocessor/new/Context.cpp:42: delete mInput;
should null mInput here so we don't have dangling pointers

http://codereview.appspot.com/4919045/diff/5001/src/compiler/preprocessor/new...
File src/compiler/preprocessor/new/Context.h (right):

http://codereview.appspot.com/4919045/diff/5001/src/compiler/preprocessor/new...
src/compiler/preprocessor/new/Context.h:39: private:
missing DISALLOW_COPY_AND_ASSIGN?

[dgkoch, 2011-08-30 03:09:28]

On 2011/08/24 17:46:00, Alok Priyadarshi wrote:
> On 2011/08/23 02:06:09, kbr1 wrote:

> > As Daniel mentioned we need to think about error handling. How would
> allocation
> > failures be handled and reported here and throughout? In Chrome there is
> likely
> > no issue because of the out-of-memory killer, but assuming this code sees
> > broader use we really need to do a good job with handling allocation
failures
> > gracefully.
> 
> Daniel: Could you point me to the out-of-memory bug?

Will email separately.

[Alok Priyadarshi, 2011-08-30 04:35:02]

http://codereview.appspot.com/4919045/diff/5001/src/compiler/preprocessor/new...
File src/compiler/preprocessor/new/Context.cpp (right):

http://codereview.appspot.com/4919045/diff/5001/src/compiler/preprocessor/new...
src/compiler/preprocessor/new/Context.cpp:42: delete mInput;
On 2011/08/30 03:08:37, dgkoch wrote:
> should null mInput here so we don't have dangling pointers

Done.

http://codereview.appspot.com/4919045/diff/5001/src/compiler/preprocessor/new...
File src/compiler/preprocessor/new/Context.h (right):

http://codereview.appspot.com/4919045/diff/5001/src/compiler/preprocessor/new...
src/compiler/preprocessor/new/Context.h:39: private:
On 2011/08/30 03:08:37, dgkoch wrote:
> missing DISALLOW_COPY_AND_ASSIGN?

Done.