> On Jan. 25, 2011, 11:30 a.m., Boroondas Gupte wrote:
> > indra/llcommon/llpreprocessor.h, lines 164-167
> > <http://codereview.secondlife.com/r/122/diff/1/?file=635#file635line164>
> >
> > Isn't the 'pop' sufficient for restoring the previous warning settings?
> > What purpose do the 'disable', 'push' and 'default' after that serve?
> >
> > See also OlafvdSpek's comment about the proposed workaround on the page
> > you linked.
>
> Nicky Perian wrote:
> I thought I followed the second comment. I could not tell from
> OlafvdSpek's comments if that was the complete solution or a added to
> solution.
>
> Nicky Perian wrote:
> I think the 'disable" 'push", 'default' are to correct later includes
> causing warnings. But, I can take them out and test it.
As I read it, OlafvdSpek's comment is neither a complete solution, nor does it
just add something to Maxime Chamley's workaround. It *replaces* a part of
Maxime's solution:
Instead of
push, disable, (do something for which the warning needs to be
disabled,) push, default
one should do
push, disable, (do something for which the warning needs to be
disabled,) pop
so the second 'push' and the 'default' are replaced by just 'pop'.
This makes sense under the assumption that (an educated guess) 'push' records
the current state of what warnings are enabled and disabled and places it on a
stack (a FIFO data structure) and 'pop' takes such a record from the stack
(i.e., reads it and removes it from the stack) and restores the respective
state. 'default' probably just restores the default value for the specified
warning type.
So the original workaround has two problems:
* It leaves two recorded states on the stack which might never be used. (Or
worse, they might not be the ones expected to be on the stack where they'll get
used.)
* If the enable/disable warning 4005 setting was at its default value before
this part of the code, everything is fine, as the default will be restored. But
if the value wasn't the default one, that previous value will be lost and the
default restored instead anyway.
- Boroondas
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://codereview.secondlife.com/r/122/#review251
-----------------------------------------------------------
On Jan. 25, 2011, 11:12 a.m., Nicky Perian wrote:
>
> -----------------------------------------------------------
> This is an automatically generated e-mail. To reply, visit:
> http://codereview.secondlife.com/r/122/
> -----------------------------------------------------------
>
> (Updated Jan. 25, 2011, 11:12 a.m.)
>
>
> Review request for Viewer.
>
>
> Summary
> -------
>
> Correct marco redefinition warnings introduced in Visual Studio 10. Patch
> applies workaround documented at:
> http://connect.microsoft.com/VisualStudio/feedback/details/621653/including-stdint-after-intsafe-generates-warnings
>
> Depends on vwr-24610 which should be applied first.
>
>
> This addresses bug vwr-24612.
> http://jira.secondlife.com/browse/vwr-24612
>
>
> Diffs
> -----
>
> indra/llcommon/llpreprocessor.h 26c09ad4293e
>
> Diff: http://codereview.secondlife.com/r/122/diff
>
>
> Testing
> -------
>
> Before (snip)
> 1>C:\Program Files (x86)\Microsoft Visual Studio
> 10.0\VC\include\stdint.h(81): warning C4005: 'UINT32_MAX' : macro redefinition
> 1> indra.l.cpp(85) : see previous definition of 'UINT32_MAX'
> ========== Build: 0 succeeded, 1 failed, 0 up-to-date, 0 skipped ==========
> After patch applied:
> 1>------ Build started: Project: lscript_compile, Configuration: Release
> Win32 ------
> 1> lscript_bytecode.cpp
> 1> lscript_error.cpp
> 1> lscript_resource.cpp
> 1> lscript_scope.cpp
> 1> lscript_tree.cpp
> 1> lscript_typecheck.cpp
> 1> indra.y.cpp
> 1> indra.l.cpp
> 1> lscript_compile.vcxproj ->
> C:\lindenhg\vcexpress2010build\indra\build-vc100\lscript\lscript_compile\Release\lscript_compile.lib
> ========== Build: 1 succeeded, 0 failed, 0 up-to-date, 0 skipped ==========
>
>
> Thanks,
>
> Nicky
>
>
_______________________________________________
Policies and (un)subscribe information available here:
http://wiki.secondlife.com/wiki/OpenSource-Dev
Please read the policies before posting to keep unmoderated posting privileges
