بنقرة واحدة
programming-cpp
C++ programming skill based on C++ Core Guidelines - use for implementing C++ code
التثبيت باستخدام Codex أو Claude انسخ هذا Prompt والصقه في Codex أو Claude أو مساعد آخر ليراجع صفحة Skill ويثبّتها لك.
القائمة
C++ programming skill based on C++ Core Guidelines - use for implementing C++ code
التثبيت باستخدام Codex أو Claude انسخ هذا Prompt والصقه في Codex أو Claude أو مساعد آخر ليراجع صفحة Skill ويثبّتها لك.
Given a TheRock nightly build (URL or run-id), return the rocm-systems pin_sha used in that build
Check whether a given rocm-systems commit is included in a specific TheRock nightly build
Find the first TheRock nightly build that includes a given rocm-systems commit
Reviews Pull Requests or local diffs with an 8-agent fan-out covering static analysis, dead code, code smells + quality (naming, complexity, single-responsibility, magic numbers), language rules (C++/Python/CMake), architecture, simplification, performance (hot-path classification, allocations, locks, I/O), and undefined behaviour (signed overflow, lifetime, strict aliasing, data races, sanitizer coverage; C/C++/unsafe-Rust only). Use when the user asks to "review this PR", "review the diff", "audit this branch", "/pr-review", or when staging changes before push.
Walk through a PR review interactively, one finding at a time. Generate review via pr-review, then for each issue present analysis + proposed inline comment, let user accept/edit/skip, accumulate into a PENDING GitHub review, submit at end.
Use when user wants to list, analyze, review, or summarize GitHub PR comments on a pull request number or URL
استنادا إلى تصنيف SOC المهني
| name | programming-cpp |
| description | C++ programming skill based on C++ Core Guidelines - use for implementing C++ code |
Use this skill when writing or modifying C++ code.
Follow the [C++ Core Guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) as the primary reference for best practices.C++17 Standard. This project uses C++17. Use only features available in C++17. Do NOT use C++20 features (concepts, ranges, std::span, etc.).
Compile-time execution is a PRIORITY. If code CAN be executed at compile time, it SHOULD be. Use constexpr, if constexpr, template metaprogramming. Move as much computation as possible from runtime to compile time.
Performance is critical. This is performance-sensitive software. Always consider performance implications. Prefer zero-cost abstractions, avoid unnecessary allocations, and be cache-friendly.
All code MUST be unit testable. Use Dependency Injection and/or Policy-based design to achieve testability.
Guidelines override existing code style. If you see existing code that violates the rules in this skill, apply these guidelines first and ignore the current project style. Do NOT propagate bad patterns just because they exist in the codebase.
T*) or reference (T&)constexprnoexceptT* or T& arguments rather than smart pointersconstconstunique_ptr<T> to transfer ownership where a pointer is neededshared_ptr<T> to share ownershipstructs or classes)class if the class has an invariant; use struct if the data members can vary independently=delete any copy, move, or destructor function, define or =delete them all (Rule of 0/5)=default if you have to be explicit about using the default semanticsvirtual, override, or finalunique_ptr or shared_ptr to avoid forgetting to delete objects created using newT*) is non-owningT&) is non-owningmalloc() and free()new and delete explicitlyunique_ptr or shared_ptr to represent ownershipunique_ptr over shared_ptr unless you need to share ownershipmake_shared() to make shared_ptrsmake_unique() to make unique_ptrsswap, and exception type copy/move construction must never failtry/catchThis is performance-critical software. Apply these rules from C++ Core Guidelines - Performance:
// BAD: Allocations in hot path
void process_items(const std::vector<item>& items) {
for (const auto& item : items) {
auto result = std::make_unique<result_t>(); // Allocation per iteration!
// ...
}
}
// GOOD: Pre-allocate or reuse
void process_items(const std::vector<item>& items) {
result_t result; // Stack allocation, reused
for (const auto& item : items) {
result.reset();
// ...
}
}
// GOOD: Reserve capacity
std::vector<int> results;
results.reserve(items.size()); // Avoid reallocations
// BAD: Unnecessary copy
void process(std::vector<int> data) { } // Copies entire vector
// GOOD: Pass by const reference
void process(const std::vector<int>& data) { }
// GOOD: Pass by value if you need to modify/own it (enables move)
void take_ownership(std::vector<int> data) {
m_data = std::move(data);
}
// BAD: Return by const reference from temporary
const std::string& get_name() { return m_name; } // OK
const std::string& bad() { return std::string("temp"); } // Dangling!
// GOOD: Return by value (RVO/NRVO applies)
std::string get_computed_name() {
std::string result = compute();
return result; // Move or RVO, no copy
}
// Use std::move when transferring ownership
std::vector<int> source = get_data();
process(std::move(source)); // source is now empty
// Implement move constructor/assignment for heavy classes
class heavy_resource {
public:
heavy_resource(heavy_resource&& other) noexcept
: m_data(std::exchange(other.m_data, nullptr)) {}
heavy_resource& operator=(heavy_resource&& other) noexcept {
if (this != &other) {
delete m_data;
m_data = std::exchange(other.m_data, nullptr);
}
return *this;
}
};
// BAD: Cache-unfriendly (pointer chasing)
struct node {
node* next;
int data;
};
// GOOD: Cache-friendly (contiguous memory)
std::vector<int> data; // Contiguous, prefetch-friendly
// BAD: Column-major access in row-major array
for (int col = 0; col < cols; ++col)
for (int row = 0; row < rows; ++row)
matrix[row][col] = 0; // Cache miss every access!
// GOOD: Row-major access
for (int row = 0; row < rows; ++row)
for (int col = 0; col < cols; ++col)
matrix[row][col] = 0; // Sequential access
// Prefer constexpr for compile-time computation
constexpr int factorial(int n) {
return n <= 1 ? 1 : n * factorial(n - 1);
}
constexpr int fact_10 = factorial(10); // Computed at compile time
// Use if constexpr for compile-time branching
template<typename T>
void process(T value) {
if constexpr (std::is_integral_v<T>) {
// Integer-specific code
} else {
// Other types
}
}
// Prefer templates over virtual for known types (no vtable lookup)
template<typename Handler>
void process(Handler& h) { h.handle(); } // Inlined, no virtual call
| Pitfall | Solution |
|---|---|
std::endl in loops | Use '\n' (no flush) |
std::map for small sets | Use std::vector + linear search for < 20 elements |
| String concatenation in loops | Use std::ostringstream or pre-reserve |
Unnecessary shared_ptr | Use unique_ptr (no atomic refcount) |
| Virtual calls in hot loops | Use CRTP or templates |
std::function in hot path | Use templates or function pointers |
| Exceptions for control flow | Use return values or std::optional |
| Copying in range-for | Use const auto& |
const&, small by valuestd::move when transferring ownershipreserve() when size is knownconstexpr for compile-time computationsAll C++ code must be designed for unit testing. Use these patterns:
Inject dependencies through constructor or setter instead of creating them internally:
// BAD: Hard to test - creates its own dependency
class order_processor {
database m_db; // Creates concrete database
public:
void process(const order& o) {
m_db.save(o); // Can't mock this
}
};
// GOOD: Testable - dependency injected via interface
class i_database {
public:
virtual ~i_database() = default;
virtual void save(const order& o) = 0;
};
class order_processor {
i_database& m_db; // Injected dependency
public:
explicit order_processor(i_database& db) : m_db(db) {}
void process(const order& o) {
m_db.save(o); // Can be mocked in tests
}
};
// Test with mock
class mock_database : public i_database {
public:
void save(const order& o) override { /* verify call */ }
};
Use templates with policy classes for compile-time dependency injection:
// Policy-based design - zero runtime overhead
template<typename DatabasePolicy>
class order_processor {
DatabasePolicy m_db;
public:
void process(const order& o) {
m_db.save(o);
}
};
// Production policy
struct production_database {
void save(const order& o) { /* real implementation */ }
};
// Test policy
struct mock_database {
void save(const order& o) { /* mock implementation */ }
};
// Usage
using prod_processor = order_processor<production_database>;
using test_processor = order_processor<mock_database>;
| Pattern | Use When... |
|---|---|
| Dependency Injection | Need runtime polymorphism, plugins, or when interface is natural |
| Policy-Based Design | Need maximum performance (no virtual calls), behavior known at compile time |
| Both | Complex systems may combine both approaches |
Use early return to reduce nesting and improve readability:
// BAD: Deep nesting
std::optional<result> process(const input& in) {
if (in.is_valid()) {
if (auto data = fetch_data(in)) {
if (data->size() > 0) {
return compute(*data);
}
}
}
return std::nullopt;
}
// GOOD: Early return (guard clauses)
std::optional<result> process(const input& in) {
if (!in.is_valid())
return std::nullopt;
auto data = fetch_data(in);
if (!data)
return std::nullopt;
if (data->empty())
return std::nullopt;
return compute(*data);
}
Mark everything const that doesn't need to change:
// Variables
const int max_size = 100;
const auto& item = container[0]; // const reference
// Parameters
void process(const std::string& input); // Won't modify input
// Member functions
class data_holder {
public:
int get_value() const { return m_value; } // Doesn't modify object
const std::string& get_name() const { return m_name; }
private:
int m_value;
std::string m_name;
};
// Pointers
const int* ptr_to_const; // Can't modify *ptr
int* const const_ptr; // Can't modify ptr itself
const int* const both_const; // Can't modify either
Use constexpr for compile-time evaluation:
// Constants
constexpr int max_buffer_size = 1024;
constexpr double pi = 3.14159265359;
// Functions that CAN be evaluated at compile time
constexpr int square(int x) { return x * x; }
constexpr int size = square(10); // Computed at compile time
// Classes with constexpr constructors
struct point {
constexpr point(int x, int y) : m_x(x), m_y(y) {}
constexpr int x() const { return m_x; }
constexpr int y() const { return m_y; }
private:
int m_x, m_y;
};
constexpr point origin{0, 0}; // Compile-time construction
Mark functions noexcept when they don't throw:
// Move operations SHOULD be noexcept (enables optimizations)
class resource {
public:
resource(resource&& other) noexcept;
resource& operator=(resource&& other) noexcept;
};
// Destructors are implicitly noexcept, but be explicit if needed
~resource() noexcept;
// Swap should be noexcept
void swap(resource& other) noexcept;
// Simple getters that can't fail
int get_size() const noexcept { return m_size; }
// Conditional noexcept
template<typename T>
void process(T& obj) noexcept(noexcept(obj.do_work())) {
obj.do_work();
}
Use [[nodiscard]] when ignoring return value is likely a bug:
// Factory functions - result must be used
[[nodiscard]] std::unique_ptr<widget> create_widget();
// Error codes - must check result
[[nodiscard]] error_code save_file(const std::string& path);
// Functions with important return values
[[nodiscard]] bool try_lock();
[[nodiscard]] iterator find(const key_type& key);
// Entire class (C++17) - all methods return important values
class [[nodiscard]] result {
// ...
};
// Note: [[nodiscard("message")]] requires C++20, use without message in C++17
Use [[maybe_unused]] to suppress warnings for intentionally unused variables:
// Parameters unused in some configurations
void log_message([[maybe_unused]] const char* file,
[[maybe_unused]] int line,
const std::string& message) {
#ifdef DEBUG
std::cerr << file << ":" << line << ": " << message << "\n";
#else
std::cerr << message << "\n";
#endif
}
// Variables used only in assertions
void process(const data& d) {
[[maybe_unused]] bool valid = validate(d);
assert(valid); // Only used in debug builds
// ...
}
// RAII guards where variable name isn't used
void critical_section() {
[[maybe_unused]] std::lock_guard lock(m_mutex);
// lock is "unused" but its lifetime matters
}
| Attribute | Use When |
|---|---|
const | Value/object won't be modified |
constexpr | Can be evaluated at compile time |
noexcept | Function guaranteed not to throw |
[[nodiscard]] | Ignoring return value is likely a bug |
[[maybe_unused]] | Intentionally unused (suppress warning) |
This project uses C++17. Use these features:
auto for type inference when type is obviousauto [key, value] = map.begin();std::pair p{1, 2.0};if constexpr for compile-time branching (PRIORITY!)if/switch with initializer: if (auto it = map.find(key); it != map.end())std::optional<T> for optional values (no more nullptr/sentinel)std::variant<Ts...> for type-safe unionsstd::string_view for read-only string parameters (zero-copy!)std::any for type-safe void* (use sparingly)std::invoke for uniform callable invocationstd::apply for tuple unpacking to function callsconstexpr if - compile-time branchingconstexpr lambdas - lambdas can be constexprinline variables - define variables in headers(args + ...) for variadic templates[[nodiscard]] - warn if return value ignored[[maybe_unused]] - suppress unused warnings[[fallthrough]] - intentional switch fallthroughstd::filesystem - portable path/file operationsstd::execution::par, std::execution::seq// C++17 examples
auto [iter, success] = map.insert({key, value}); // Structured binding
if constexpr (std::is_integral_v<T>) { // Compile-time if
// Only compiled for integral types
}
std::optional<int> find_value(int key); // Optional return
std::string_view get_name(); // Zero-copy string view
std::spanstatic_assertstd::formatfmt library or streams (if available in project)[[nodiscard("message")]][[nodiscard]] without messagegsl::spanFor span-like functionality:
span, array_view, buffer_view)// Instead of span
void process(const T* data, size_t size);
// Or use iterator pair
template<typename Iterator>
void process(Iterator begin, Iterator end);
Follow project conventions. If none exist, use:
snake_case for functions, variables, namespacesPascalCase for types (classes, structs, enums, type aliases)SCREAMING_SNAKE_CASE for macros and constantsm_ or _ prefix for member variablesUse Doxygen-style comments for all public APIs:
/**
* Calculates the sum of two integers.
* @param a The first integer.
* @param b The second integer.
* @return The sum of a and b.
*/
int add(int a, int b) {
return a + b;
}
/**
* Represents a network connection to a remote server.
*
* This class manages the lifecycle of a TCP connection,
* including automatic reconnection on failure.
*/
class connection {
public:
/**
* Establishes a connection to the specified host.
* @param host The hostname or IP address.
* @param port The port number.
* @throws connection_error If the connection cannot be established.
*/
void connect(std::string_view host, uint16_t port);
/**
* Sends data over the connection.
* @param data Pointer to the data buffer.
* @param size Number of bytes to send.
* @return Number of bytes actually sent.
*/
[[nodiscard]] size_t send(const void* data, size_t size);
};
| Tag | Usage |
|---|---|
@param name | Document a function parameter |
@return | Document return value |
@throws exception | Document exceptions thrown |
@note | Additional notes |
@warning | Important warnings |
@see | Cross-reference to related items |
@deprecated | Mark as deprecated |
@code / @endcode | Code example block |
Two distinct things:
| Kind | Purpose | Form | Lives |
|---|---|---|---|
| Documentation | Tells callers how to use this API | Doxygen block, javadoc style (/** @param @return @throws */) | Above public APIs |
| Inline comment | Explains a non-obvious line | // short why | Beside the line, same indentation |
Code that needs neither is the goal. Reach for them only when the code by itself does not answer a question a competent reader will have.
Add a /** */ block when ANY of these is true and the answer is not in the function name + signature:
@throws, "returns std::nullopt on FOO").Use exactly the javadoc-style tags already documented above (@param, @return, @throws, @note, @warning, @see, @deprecated).
Add a // ... line ONLY when ALL of these hold:
If any one fails, drop the comment.
i++; // skip header row
timeout *= 2; // exponential backoff
buffer.reserve(1024); // avoid reallocations in hot path
result |= 0x80; // protocol spec sets MSB on negative flag
{ // caller holds m_mutex
state.value = compute(...);
}
// workaround for FOO-1234; remove once bar.so >= 2.5 ships
auto handle = legacy_open(path, /*safe=*/true);
Each one carries information the code does not: a hidden invariant, a non-obvious algorithm choice, a workaround tagged to a ticket, a domain quirk.
i++; // increment i
int sum = a + b; // add a and b
std::vector<int> numbers; // vector of numbers
result.clear(); // clear the result
for (auto& item : items) { // loop through items
process(item); // process each item
}
m_count = 0; // initialize count to zero
Each one restates what the code already says. Delete.
These need neither documentation nor inline comments:
int size() const { return m_size; }bool is_empty() const, bool has_value()std::move(x), std::make_unique<T>()operator==, operator+)TEST(suite, name) already names the case// ❌ NOISE - getter is self-explanatory
/**
* Returns the size.
* @return The size.
*/
size_t size() const { return m_size; }
// ✅ GOOD - just the code
size_t size() const { return m_size; }
Multi-line // ... blocks at the top of files, above test cases, or between helpers tend to re-tell story already in the diff, the test name, the PR description, or the commit message. Strip them by default.
Delete:
file:line in another file. The line numbers rot; the PR description owns cross-file context.// === Helpers ===.Keep:
Rule of thumb: if removing the comment would not confuse a competent reader a year from now, it is noise.
Long if/else chains are a code smell. Replace with these patterns:
| Smell | Threshold |
|---|---|
| if/else chain | 3+ branches |
| switch statement | 5+ cases |
| Deep nesting | 3+ levels |
| Type-based branching | Any dynamic_cast chain |
Use when: Behavior varies by type, need extensibility.
// BAD: if/else chain
void process(int type, Data& data) {
if (type == 1) {
handleType1(data);
} else if (type == 2) {
handleType2(data);
} else if (type == 3) {
handleType3(data);
}
}
// GOOD: Polymorphism
class Handler {
public:
virtual ~Handler() = default;
virtual void process(Data& data) = 0;
};
class Type1Handler : public Handler {
void process(Data& data) override { /* handle type 1 */ }
};
// Usage - no if/else
void process(Handler& handler, Data& data) {
handler.process(data);
}
Use when: Mapping values to values, simple dispatch.
// BAD: if/else for value mapping
std::string getStatusText(int code) {
if (code == 200) return "OK";
else if (code == 404) return "Not Found";
else if (code == 500) return "Server Error";
return "Unknown";
}
// GOOD: Lookup table
const std::unordered_map<int, std::string> STATUS_TEXT = {
{200, "OK"},
{404, "Not Found"},
{500, "Server Error"}
};
std::string getStatusText(int code) {
auto it = STATUS_TEXT.find(code);
return it != STATUS_TEXT.end() ? it->second : "Unknown";
}
Use when: Different actions based on command/type.
// BAD: if/else dispatch
void execute(const std::string& cmd, Context& ctx) {
if (cmd == "start") start(ctx);
else if (cmd == "stop") stop(ctx);
else if (cmd == "pause") pause(ctx);
else if (cmd == "resume") resume(ctx);
}
// GOOD: Command map
using CommandFn = std::function<void(Context&)>;
const std::unordered_map<std::string, CommandFn> COMMANDS = {
{"start", [](Context& ctx) { start(ctx); }},
{"stop", [](Context& ctx) { stop(ctx); }},
{"pause", [](Context& ctx) { pause(ctx); }},
{"resume", [](Context& ctx) { resume(ctx); }}
};
void execute(const std::string& cmd, Context& ctx) {
if (auto it = COMMANDS.find(cmd); it != COMMANDS.end()) {
it->second(ctx);
}
}
Use when: Type-safe union, different handling per type.
// BAD: dynamic_cast chain
double getArea(Shape* shape) {
if (auto* c = dynamic_cast<Circle*>(shape)) {
return 3.14159 * c->radius * c->radius;
} else if (auto* r = dynamic_cast<Rectangle*>(shape)) {
return r->width * r->height;
} else if (auto* t = dynamic_cast<Triangle*>(shape)) {
return 0.5 * t->base * t->height;
}
return 0;
}
// GOOD: std::variant + std::visit
using Shape = std::variant<Circle, Rectangle, Triangle>;
double getArea(const Shape& shape) {
return std::visit([](const auto& s) -> double {
using T = std::decay_t<decltype(s)>;
if constexpr (std::is_same_v<T, Circle>) {
return 3.14159 * s.radius * s.radius;
} else if constexpr (std::is_same_v<T, Rectangle>) {
return s.width * s.height;
} else {
return 0.5 * s.base * s.height;
}
}, shape);
}
Use when: Validation or precondition checks cause deep nesting.
// BAD: Deep nesting
Result process(Request* req) {
if (req != nullptr) {
if (req->isValid()) {
if (req->hasPermission()) {
if (req->data.size() > 0) {
return doActualWork(req);
}
}
}
}
return Result::Error;
}
// GOOD: Early return (flat structure)
Result process(Request* req) {
if (!req) return Result::Error;
if (!req->isValid()) return Result::InvalidRequest;
if (!req->hasPermission()) return Result::Forbidden;
if (req->data.empty()) return Result::EmptyData;
return doActualWork(req);
}
Use when: Eliminating null checks throughout code.
// BAD: Null checks everywhere
void logMessage(Logger* logger, const std::string& msg) {
if (logger != nullptr) {
logger->log(msg);
}
}
// GOOD: Null Object pattern
class NullLogger : public Logger {
void log(const std::string&) override { /* intentionally empty */ }
};
// Always have valid logger - no null checks needed
Logger& getLogger() {
static NullLogger nullLogger;
return currentLogger ? *currentLogger : nullLogger;
}
void logMessage(Logger& logger, const std::string& msg) {
logger.log(msg); // No null check needed
}
Use when: Compile-time type-based branching.
// BAD: Runtime type checking
void serialize(const std::any& value, std::ostream& out) {
if (value.type() == typeid(int)) {
out << std::any_cast<int>(value);
} else if (value.type() == typeid(std::string)) {
out << std::any_cast<std::string>(value);
}
}
// GOOD: Compile-time branching (when types known)
template<typename T>
void serialize(const T& value, std::ostream& out) {
if constexpr (std::is_integral_v<T>) {
out << value;
} else if constexpr (std::is_same_v<T, std::string>) {
out << '"' << value << '"';
} else {
static_assert(always_false<T>, "Unsupported type");
}
}
| Pattern | Best For | Overhead |
|---|---|---|
| Polymorphism | Extensible behavior, OOP design | Virtual call |
| Lookup table | Value mapping | Map lookup |
| Command map | Action dispatch | Map + function call |
| std::variant | Type-safe unions, closed set | Visit overhead |
| Early return | Validation, preconditions | None |
| Null Object | Eliminating null checks | None |
| if constexpr | Compile-time branching | None (compile-time) |
Keep simple if/else when:
Before submitting C++ code:
new/delete - use smart pointersunique_ptr or shared_ptrconst correctness (mark const what doesn't change)constexpr for compile-time constants and functionsnoexcept on move ops, destructors, swap, simple getters[[nodiscard]] on factory functions and error codes[[maybe_unused]] for intentionally unused variablesconst&std::move used for ownership transferreserve() when possibleconstexpr for compile-time computations/** */)