MSVC tool: prevent overwriting the PCH object node builder/emitter #4444
Conversation
Suppress the assignment of the static object builder/emitter when the target is the PCH object file (e.g., PCH.obj) and the source is the PCH c++ file (e.g., PCH.cpp). When the object_emitter is called for target PCH.obj and source PCH.cpp, the target list and source list are invalidated (i.e., both are set to None).
jcbrill
changed the title
…is undefined or evaluates as false.
| @@ -121,8 +124,16 @@ def object_emitter(target, source, env, parent_emitter): | |||
| # https://github.com/SCons/scons/issues/2505 | |||
| pch=get_pch_node(env, target, source) | |||
| if pch: | |||
| if str(target[0]) != SCons.Util.splitext(str(pch))[0] + '.obj': | |||
| pch_basename = SCons.Util.splitext(str(pch))[0] | |||
| if str(target[0]) != pch_basename + '.obj': | |||
There was a problem hiding this comment.
This is okay given the pretty constrained context, but maybe ought to use $OBJSUFFIX (though then you have to subst is before using...)
| @@ -40,7 +40,7 @@ | |||
| env = Environment(tools=['msvc', 'mslink']) | |||
| env['PCH'] = env.PCH('Source1.cpp')[0] | |||
| env['PCHSTOP'] = 'Header1.hpp' | |||
| env.Program('foo', ['foo.cpp', 'Source2.cpp', 'Source1.cpp']) | |||
| env.Program('foo', ['Source2.cpp', 'Source1.cpp', 'foo.cpp']) | |||
There was a problem hiding this comment.
Maybe add a comment here that the order is chosen for a specific reason, so somebody doesn't resort it later.
| @@ -99,6 +99,9 @@ def pch_emitter(target, source, env): | |||
|
|
|||
| target = [pch, obj] # pch must be first, and obj second for the PCHCOM to work | |||
|
|
|||
| if 'PCH' not in env or not env['PCH']: | |||
| env['PCH'] = pch | |||
|
|
|||
There was a problem hiding this comment.
If this really needs both parts of the check I guess it's fine. The common idiom when you want to preserve any existing value (but not to also test what it is):
env.SetDefault(PCH=pch)There was a problem hiding this comment.
I haven't looked at this in quite awhile.
There was a comment in the issue #2249 (#2249 (comment)) where there appeared to be a reason evaluating False was important:
I wonder if the idea is to say not to set the PCH consvar manually, and instead have the PCH builder be the one that sets it.
The suggested solution was adjusted to set
env['PCH']=pchin the msvc pch_emitter when eitherenv['PCH']is undefined or evaluates to false.
I'll have to dig back in one of these days. Unfortunately, available time is limited in the immediate short term.
Work was suspended on this for awhile because I was convinced there was a situation/confluence of events that needed state to be saved and/or maintained which is problematic. Somewhere here I had some prototype code that looked promising.
Prevent overwriting the PCH object node builder/emitter when the PCH source file is included in the source list.
Suppress the assignment of the static object builder/emitter when the target is the PCH object file (e.g., PCH.obj) and the source is the PCH c++ file (e.g., PCH.cpp).
When the object_emitter is called for target PCH.obj and source PCH.cpp, the target list and source list are invalidated (i.e., both are set to None).
See #2249 (comment) for discussion.
Contributor Checklist:
CHANGES.txt(and read theREADME.rst)