Jorijn van der Graaf
651720e494
Two rules the AST makes possible, plus the mutation analysis behind them.
const-local reports a local that is never written. It is restricted to SCALARS
— integers, bools, enums, floating types — and that restriction is what makes
the answer exact rather than a guess: a scalar has no member functions, so the
only ways to write one are assignment, ++/--, having its address taken, or
binding to a non-const reference. All four are now tracked in the walk:
- assignment and compound assignment visit their LEFT operand in a write
context, the right one normally;
- ++/-- and & write their operand;
- a call argument is checked against the callee's parameter type, so passing
to `const int&` or by value is a read while `int&` is a write;
- initialising a non-const reference writes what it binds to.
For a class type a non-const method call could mutate it, and deciding that is
the whole-program analysis clang-tidy does, so those are simply out of scope
rather than guessed at.
constexpr-constant promotes a const constant whose initialiser is made only of
literals and operators, so `const int A = 1 << 4;` qualifies and
`const int B = Compute();` does not.
On this repository const-local found 103 candidates, which was too many to be
useful, and the reason was informative: most were range-for bindings and
pointer locals. `for (T* const x : …)` and `T* const p` are not spellings
anybody writes, and the useful constness for a pointer is on the pointee, which
this rule cannot advise on. Excluding both leaves 36, all plain bool or enum
locals worth fixing — isWasm, isPe, exists, writes, isC and so on. Those 36 are
fixed in this commit; the compiler verified every one.
Both rules are report-only. The analysis is exact, but adding const is a
judgement about intent as much as mechanics, and a wrong suggestion should cost
a glance rather than a build. const-local also deliberately does not become a
transform: inserting `const` before a shared type would apply it to every
declarator in a multi-declarator statement, including any that IS written.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
79 lines
3.1 KiB
C++
79 lines
3.1 KiB
C++
// SPDX-License-Identifier: LGPL-3.0-only
|
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
|
|
|
import std;
|
|
import Crafter.Build;
|
|
namespace fs = std::filesystem;
|
|
using namespace Crafter;
|
|
|
|
// `crafter-build clean` exists so the fix for a stale build is discoverable
|
|
// rather than folklore. It must not need the project to load: the state you most
|
|
// want to clear is one where the build is already broken.
|
|
namespace {
|
|
std::int32_t Failures = 0;
|
|
|
|
void Check(bool cond, std::string_view msg) {
|
|
if (!cond) {
|
|
std::println(std::cerr, "FAIL: {}", msg);
|
|
++Failures;
|
|
}
|
|
}
|
|
|
|
fs::path StageProject(std::string_view leaf) {
|
|
fs::path root = fs::temp_directory_path() / std::format("crafter-build-clean-{}", leaf);
|
|
fs::remove_all(root);
|
|
fs::create_directories(root / "bin" / "app-x86_64-pc-linux-gnu-native-native-0badf00d");
|
|
fs::create_directories(root / "build" / "app-x86_64-pc-linux-gnu-native-native-0badf00d");
|
|
std::ofstream(root / "bin" / "app-x86_64-pc-linux-gnu-native-native-0badf00d" / "app") << "binary";
|
|
std::ofstream(root / "build" / "app-x86_64-pc-linux-gnu-native-native-0badf00d" / "main_impl.o") << "object";
|
|
std::ofstream(root / "keep-me.txt") << "untouched";
|
|
return root;
|
|
}
|
|
}
|
|
|
|
int main() {
|
|
{
|
|
// A project file that could never compile: clean has to work anyway.
|
|
fs::path root = StageProject("broken");
|
|
fs::path projectFile = root / "project.cpp";
|
|
std::ofstream(projectFile) << "this is not valid C++ at all\n";
|
|
|
|
std::vector<fs::path> removed = CleanProject(projectFile);
|
|
Check(removed.size() == 2, std::format("both trees reported removed, got {}", removed.size()));
|
|
Check(!fs::exists(root / "bin"), "bin/ is gone");
|
|
Check(!fs::exists(root / "build"), "build/ is gone");
|
|
Check(fs::exists(root / "keep-me.txt"), "unrelated files in the project root are left alone");
|
|
Check(fs::exists(projectFile), "the project file itself is left alone");
|
|
}
|
|
|
|
{
|
|
// Idempotent, and quiet about it — nothing to remove is not an error.
|
|
fs::path root = StageProject("twice");
|
|
fs::path projectFile = root / "project.cpp";
|
|
std::ofstream(projectFile) << "\n";
|
|
|
|
CleanProject(projectFile);
|
|
std::vector<fs::path> second = CleanProject(projectFile);
|
|
Check(second.empty(), "a second clean reports nothing removed");
|
|
}
|
|
|
|
{
|
|
// Resolved relative to the project file, not the cwd, so --project=
|
|
// cleans the project it names.
|
|
fs::path root = StageProject("elsewhere");
|
|
fs::path projectFile = root / "project.cpp";
|
|
std::ofstream(projectFile) << "\n";
|
|
fs::path cwdBin = fs::current_path() / "bin";
|
|
const bool cwdBinExisted = fs::exists(cwdBin);
|
|
|
|
CleanProject(projectFile);
|
|
Check(!fs::exists(root / "bin"), "the named project's bin/ is gone");
|
|
Check(fs::exists(cwdBin) == cwdBinExisted, "the cwd's bin/ is untouched");
|
|
}
|
|
|
|
if (Failures > 0) {
|
|
std::println(std::cerr, "{} assertions failed", Failures);
|
|
return 1;
|
|
}
|
|
return 0;
|
|
}
|