Revert "cmDefinitions: Don't store parent lookups"

This reverts commit 5abfde6cb8.

The behaviors associated with implicit pulldown on variable lookup
seriously conflict with the optimizations made in these commits.
Basically, since values were copied upon variable lookup, not just on
PARENT_SCOPE, coupled with PARENT_SCOPE's behavior based on whether the
variable is in the current scope or not causes serious problems with not
storing a value for every variable at every scope.

The commit changed behavior of the following example, among other cases:

  function(test_set)
    set(blah "value2")
    message("before PARENT_SCOPE blah=${blah}")
    set(blah ${blah} PARENT_SCOPE)
    message("after PARENT_SCOPE blah=${blah}")
  endfunction()
  set(blah value1)
  test_set()
  message("in parent scope, blah=${blah}")

Reported-by: Alex Merry <alex.merry@kde.org>
Reported-by: Ben Cooksley <bcooksley@kde.org>
This commit is contained in:
Ben Boeckel 2014-10-24 12:58:12 -04:00 committed by Brad King
parent 5abfde6cb8
commit 5f414cefb6
3 changed files with 10 additions and 24 deletions

View File

@ -28,7 +28,7 @@ void cmDefinitions::Reset(cmDefinitions* parent)
//----------------------------------------------------------------------------
cmDefinitions::Def const&
cmDefinitions::GetInternal(const std::string& key) const
cmDefinitions::GetInternal(const std::string& key)
{
MapType::const_iterator i = this->Map.find(key);
if(i != this->Map.end())
@ -37,8 +37,9 @@ cmDefinitions::GetInternal(const std::string& key) const
}
else if(cmDefinitions* up = this->Up)
{
// Query the parent scope.
return up->GetInternal(key);
// Query the parent scope and store the result locally.
Def def = up->GetInternal(key);
return this->Map.insert(MapType::value_type(key, def)).first->second;
}
return this->NoDef;
}
@ -70,25 +71,12 @@ cmDefinitions::SetInternal(const std::string& key, Def const& def)
}
//----------------------------------------------------------------------------
const char* cmDefinitions::Get(const std::string& key) const
const char* cmDefinitions::Get(const std::string& key)
{
Def const& def = this->GetInternal(key);
return def.Exists? def.c_str() : 0;
}
//----------------------------------------------------------------------------
void cmDefinitions::Pull(const std::string& key)
{
if (this->Up)
{
Def const& def = this->Up->GetInternal(key);
if (def.Exists)
{
this->SetInternal(key, def);
}
}
}
//----------------------------------------------------------------------------
const char* cmDefinitions::Set(const std::string& key, const char* value)
{

View File

@ -33,11 +33,9 @@ public:
/** Returns the parent scope, if any. */
cmDefinitions* GetParent() const { return this->Up; }
/** Get the value associated with a key; null if none. */
const char* Get(const std::string& key) const;
/** Pull a variable from the parent. */
void Pull(const std::string& key);
/** Get the value associated with a key; null if none.
Store the result locally if it came from a parent. */
const char* Get(const std::string& key);
/** Set (or unset if null) a value associated with a key. */
const char* Set(const std::string& key, const char* value);
@ -75,7 +73,7 @@ private:
MapType Map;
// Internal query and update methods.
Def const& GetInternal(const std::string& key) const;
Def const& GetInternal(const std::string& key);
Def const& SetInternal(const std::string& key, Def const& def);
// Implementation of Closure() method.

View File

@ -4422,7 +4422,7 @@ void cmMakefile::RaiseScope(const std::string& var, const char *varDef)
if(cmDefinitions* up = cur.GetParent())
{
// First localize the definition in the current scope.
cur.Pull(var);
cur.Get(var);
// Now update the definition in the parent scope.
up->Set(var, varDef);