dilim/CMake
1
0
Fork 0
mirror of https://github.com/Kitware/CMake.git synced 2026-06-24 08:47:59 +00:00
Code Issues Packages Projects Releases Wiki Activity

cmFindPackageStack: Restore pure value semantics

Browse Source 
A stack entry's storage may be shared by other copies, so mutation is
incompatible with value semantics.  We've migrated the motivating use
case to another approach.

Revert commit b3873b8272 (cmFindPackageStack: Allow controlled mutation,
2025-07-29, v4.2.0-rc1~438^2) and commit f2bdc2176f (cmStack: New,
mutable stack class, 2025-07-29, v4.2.0-rc1~438^2~1).  Record their
parent as a second parent of this commit so `git blame` can see the
original history of the restored content.
This commit is contained in:
Brad King
2026-04-01 09:21:04 -04:00
parent e935ed22fb a69131cd88
commit 561ece2407
12 changed files with 107 additions and 182 deletions
Download Patch File Download Diff File Expand all files Collapse all files

4
Source/CMakeLists.txt
View File

@@ -148,6 +148,8 @@ add_library(
cmComputeTargetDepends.cxx
cmConfigureLog.h
cmConfigureLog.cxx
cmConstStack.h
cmConstStack.tcc
cmCPackPropertiesGenerator.h
cmCPackPropertiesGenerator.cxx
cmCryptoHash.cxx
@@ -460,8 +462,6 @@ add_library(
cmSourceFileLocationKind.h
cmSourceGroup.cxx
cmSourceGroup.h
cmStack.h
cmStack.tcc
cmStandardLevel.h
cmStandardLevelResolver.cxx
cmStandardLevelResolver.h

40
Source/cmStack.h → Source/cmConstStack.h
View File

@@ -5,31 +5,19 @@
#include "cmConfigure.h" // IWYU pragma: keep
#include <memory>
#include <type_traits>
enum class cmStackType
/** Base class template for CRTP to represent a stack of constant values.
Provide value semantics, but use efficient reference-counting underneath
to avoid copies. */
template <typename T, typename Stack>
class cmConstStack
{
Const,
Mutable,
};
template <typename T, cmStackType Mutable>
struct cmStackEntry;
/** Base class template for CRTP to represent a stack of values.
Copies of the stack <i>share data</i>; mutating data on one copy will
change the data on <i>all</i> copies. */
template <typename T, typename Stack,
cmStackType Mutable = cmStackType::Mutable>
class cmStack
{
using Entry = cmStackEntry<T, Mutable>;
struct Entry;
std::shared_ptr<Entry const> TopEntry;
public:
/** Default-construct an empty stack. */
cmStack();
cmConstStack();
/** Get a stack with the given call context added to the top. */
Stack Push(T value) const;
@@ -41,21 +29,11 @@ public:
/** Get the value at the top of the stack.
This may be called only if Empty() would return false. */
T const& Top() const;
template <bool E = (Mutable == cmStackType::Mutable)>
typename std::enable_if<E, T>::type& Top();
/** Return true if this stack is empty. */
bool Empty() const;
protected:
using Base = cmStack<T, Stack, Mutable>;
cmStack(std::shared_ptr<Entry const> parent, T value);
cmStack(std::shared_ptr<Entry const> top);
cmConstStack(std::shared_ptr<Entry const> parent, T value);
cmConstStack(std::shared_ptr<Entry const> top);
};
/** Specialization of cmStack for CRTP to represent a stack of constant values.
Provide value semantics, but use efficient reference-counting underneath
to avoid copies. */
template <typename T, typename Stack>
using cmConstStack = cmStack<T const, Stack, cmStackType::Const>;

62
Source/cmConstStack.tcc Normal file
View File

@@ -0,0 +1,62 @@
/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying
file LICENSE.rst or https://cmake.org/licensing for details. */
#include <cassert>
#include <memory>
#include <utility>
template <typename T, typename Stack>
struct cmConstStack<T, Stack>::Entry
{
Entry(std::shared_ptr<Entry const> parent, T value)
: Value(std::move(value))
, Parent(std::move(parent))
{
}
T Value;
std::shared_ptr<Entry const> Parent;
};
template <typename T, typename Stack>
cmConstStack<T, Stack>::cmConstStack() = default;
template <typename T, typename Stack>
Stack cmConstStack<T, Stack>::Push(T value) const
{
return Stack(this->TopEntry, std::move(value));
}
template <typename T, typename Stack>
Stack cmConstStack<T, Stack>::Pop() const
{
assert(this->TopEntry);
return Stack(this->TopEntry->Parent);
}
template <typename T, typename Stack>
T const& cmConstStack<T, Stack>::Top() const
{
assert(this->TopEntry);
return this->TopEntry->Value;
}
template <typename T, typename Stack>
bool cmConstStack<T, Stack>::Empty() const
{
return !this->TopEntry;
}
template <typename T, typename Stack>
cmConstStack<T, Stack>::cmConstStack(std::shared_ptr<Entry const> parent,
T value)
: TopEntry(
std::make_shared<Entry const>(std::move(parent), std::move(value)))
{
}
template <typename T, typename Stack>
cmConstStack<T, Stack>::cmConstStack(std::shared_ptr<Entry const> top)
: TopEntry(std::move(top))
{
}

4
Source/cmFindPackageCommand.cxx
View File

@@ -1225,8 +1225,8 @@ bool cmFindPackageCommand::FindPackage(
FlushDebugBufferOnExit flushDebugBufferOnExit(*this);
PushPopRootPathStack pushPopRootPathStack(*this);
SetRestoreFindDefinitions setRestoreFindDefinitions(*this);
cmFindPackageStackRAII findPackageStackRAII(this->Makefile, this->Name,
this->PackageInfo);
cmMakefile::FindPackageStackRAII findPackageStackRAII(
this->Makefile, this->Name, this->PackageInfo);
// See if we have been told to delegate to FetchContent or some other
// redirected config package first. We have to check all names that

12
Source/cmFindPackageStack.cxx
View File

@@ -3,13 +3,5 @@
#define cmFindPackageStack_cxx
#include "cmFindPackageStack.h"
#include "cmStack.tcc" // IWYU pragma: keep
template class cmStack<cmFindPackageCall, cmFindPackageStack>;
template cmFindPackageCall&
cmStack<cmFindPackageCall, cmFindPackageStack>::Top<true>();
cmFindPackageCall const& cmFindPackageStack::Top() const
{
return this->cmStack::Top();
}
#include "cmConstStack.tcc" // IWYU pragma: keep
template class cmConstStack<cmFindPackageCall, cmFindPackageStack>;

40
Source/cmFindPackageStack.h
View File

@@ -10,9 +10,7 @@
#include <cm/optional>
#include "cmStack.h"
class cmMakefile;
#include "cmConstStack.h"
/**
* This data represents the actual contents of find_package
@@ -43,43 +41,15 @@ public:
unsigned int Index;
};
/**
* RAII type to manage the find_package call stack.
*/
// Note: implemented in cmMakefile.cxx
class cmFindPackageStackRAII
{
cmMakefile* Makefile;
public:
cmFindPackageStackRAII(cmMakefile* mf, std::string const& pkg,
std::shared_ptr<cmPackageInformation const> pkgInfo);
~cmFindPackageStackRAII();
cmFindPackageStackRAII(cmFindPackageStackRAII const&) = delete;
cmFindPackageStackRAII& operator=(cmFindPackageStackRAII const&) = delete;
};
/**
* Represents a stack of find_package calls with efficient value semantics.
*/
class cmFindPackageStack
: protected cmStack<cmFindPackageCall, cmFindPackageStack>
: public cmConstStack<cmFindPackageCall, cmFindPackageStack>
{
using cmStack::cmStack;
friend cmFindPackageStack::Base;
friend class cmFindPackageStackRAII;
public:
using cmStack::Push;
using cmStack::Pop;
using cmStack::Empty;
cmFindPackageCall const& Top() const;
using cmConstStack::cmConstStack;
friend class cmConstStack<cmFindPackageCall, cmFindPackageStack>;
};
#ifndef cmFindPackageStack_cxx
extern template class cmStack<cmFindPackageCall, cmFindPackageStack>;
extern template cmFindPackageCall&
cmStack<cmFindPackageCall, cmFindPackageStack>::Top<true>();
extern template class cmConstStack<cmFindPackageCall, cmFindPackageStack>;
#endif

5
Source/cmListFileCache.cxx
View File

@@ -454,9 +454,8 @@ bool cmListFile::ParseString(cm::string_view str, char const* virtual_filename,
return !parseError;
}
#include "cmStack.tcc"
template class cmStack<cmListFileContext const, cmListFileBacktrace,
cmStackType::Const>;
#include "cmConstStack.tcc"
template class cmConstStack<cmListFileContext, cmListFileBacktrace>;
std::ostream& operator<<(std::ostream& os, cmListFileContext const& lfc)
{

9
Source/cmListFileCache.h
View File

@@ -13,8 +13,8 @@
#include <cm/optional>
#include <cm/string_view>
#include "cmConstStack.h"
#include "cmList.h"
#include "cmStack.h"
#include "cmSystemTools.h"
/** \class cmListFileCache
@@ -170,12 +170,11 @@ bool operator!=(cmListFileContext const& lhs, cmListFileContext const& rhs);
class cmListFileBacktrace
: public cmConstStack<cmListFileContext, cmListFileBacktrace>
{
using cmStack::cmStack;
friend cmListFileBacktrace::Base;
using cmConstStack::cmConstStack;
friend class cmConstStack<cmListFileContext, cmListFileBacktrace>;
};
#ifndef cmListFileCache_cxx
extern template class cmStack<cmListFileContext const, cmListFileBacktrace,
cmStackType::Const>;
extern template class cmConstStack<cmListFileContext, cmListFileBacktrace>;
#endif
// Wrap type T as a value with a backtrace. For purposes of

4
Source/cmMakefile.cxx
View File

@@ -4241,7 +4241,7 @@ cmMakefile::MacroPushPop::~MacroPushPop()
this->Makefile->PopMacroScope(this->ReportError);
}
cmFindPackageStackRAII::cmFindPackageStackRAII(
cmMakefile::FindPackageStackRAII::FindPackageStackRAII(
cmMakefile* mf, std::string const& name,
std::shared_ptr<cmPackageInformation const> pkgInfo)
: Makefile(mf)
@@ -4255,7 +4255,7 @@ cmFindPackageStackRAII::cmFindPackageStackRAII(
this->Makefile->FindPackageStackNextIndex++;
}
cmFindPackageStackRAII::~cmFindPackageStackRAII()
cmMakefile::FindPackageStackRAII::~FindPackageStackRAII()
{
this->Makefile->FindPackageStackNextIndex =
this->Makefile->FindPackageStack.Top().Index + 1;

16
Source/cmMakefile.h
View File

@@ -1034,7 +1034,21 @@ public:
// searches
std::deque<std::vector<std::string>> FindPackageRootPathStack;
friend class cmFindPackageStackRAII;
/**
* RAII type to manage the find_package call stack.
*/
class FindPackageStackRAII
{
cmMakefile* Makefile;
public:
FindPackageStackRAII(cmMakefile* mf, std::string const& pkg,
std::shared_ptr<cmPackageInformation const> pkgInfo);
~FindPackageStackRAII();
FindPackageStackRAII(FindPackageStackRAII const&) = delete;
FindPackageStackRAII& operator=(FindPackageStackRAII const&) = delete;
};
class DebugFindPkgRAII
{

85
Source/cmStack.tcc
View File

@@ -1,85 +0,0 @@
/* Distributed under the OSI-approved BSD 3-Clause License. See accompanying
file LICENSE.rst or https://cmake.org/licensing for details. */
#include <cassert>
#include <memory>
#include <utility>
template <typename T>
struct cmStackEntry<T, cmStackType::Mutable>
{
cmStackEntry(std::shared_ptr<cmStackEntry const> parent, T value)
: Value(std::move(value))
, Parent(std::move(parent))
{
}
T mutable Value;
std::shared_ptr<cmStackEntry const> Parent;
};
template <typename T>
struct cmStackEntry<T, cmStackType::Const>
{
cmStackEntry(std::shared_ptr<cmStackEntry const> parent, T value)
: Value(std::move(value))
, Parent(std::move(parent))
{
}
T Value;
std::shared_ptr<cmStackEntry const> Parent;
};
template <typename T, typename Stack, cmStackType Mutable>
cmStack<T, Stack, Mutable>::cmStack() = default;
template <typename T, typename Stack, cmStackType Mutable>
Stack cmStack<T, Stack, Mutable>::Push(T value) const
{
return Stack(this->TopEntry, std::move(value));
}
template <typename T, typename Stack, cmStackType Mutable>
Stack cmStack<T, Stack, Mutable>::Pop() const
{
assert(this->TopEntry);
return Stack(this->TopEntry->Parent);
}
template <typename T, typename Stack, cmStackType Mutable>
T const& cmStack<T, Stack, Mutable>::Top() const
{
assert(this->TopEntry);
return this->TopEntry->Value;
}
template <typename T, typename Stack, cmStackType Mutable>
template <bool E>
typename std::enable_if<E, T>::type& cmStack<T, Stack, Mutable>::Top()
{
static_assert(Mutable == cmStackType::Mutable,
"T& cmStack::Top should only exist for mutable cmStack");
assert(this->TopEntry);
return this->TopEntry->Value;
}
template <typename T, typename Stack, cmStackType Mutable>
bool cmStack<T, Stack, Mutable>::Empty() const
{
return !this->TopEntry;
}
template <typename T, typename Stack, cmStackType Mutable>
cmStack<T, Stack, Mutable>::cmStack(std::shared_ptr<Entry const> parent,
T value)
: TopEntry(
std::make_shared<Entry const>(std::move(parent), std::move(value)))
{
}
template <typename T, typename Stack, cmStackType Mutable>
cmStack<T, Stack, Mutable>::cmStack(std::shared_ptr<Entry const> top)
: TopEntry(std::move(top))
{
}

8
Utilities/IWYU/mapping.imp
View File

@@ -66,12 +66,8 @@
{ include: [ "<ncurses.h>", private, "\"cmCursesStandardIncludes.h\"", public ] },
{ include: [ "\"form.h\"", private, "\"cmCursesStandardIncludes.h\"", public ] },
# Help IWYU understand our explicit instantiation for cmStack.
{ symbol: [ "cmStack::cmStack<T, Stack, Mutable>", private, "\"cmStack.h\"", public ] },
{ symbol: [ "cmStack<cmFindPackageCall, cmFindPackageStack>::Empty", private, "\"cmStack.h\"", public ] },
{ symbol: [ "cmStack<cmFindPackageCall, cmFindPackageStack>::Top", private, "\"cmStack.h\"", public ] },
{ symbol: [ "cmStack<cmFindPackageCall, cmFindPackageStack>::Pop", private, "\"cmStack.h\"", public ] },
{ symbol: [ "cmStack<cmFindPackageCall, cmFindPackageStack>::Push", private, "\"cmStack.h\"", public ] },
# Help IWYU understand our explicit instantiation for cmConstStack.
{ symbol: [ "cmConstStack::cmConstStack<T, Stack>", private, "\"cmConstStack.h\"", public ] },
]
# vim: set ft=toml: