mirror of
https://github.com/Z3Prover/z3
synced 2026-06-07 17:40:54 +00:00
[WIP] Update code conventions analyzer for initializer_list usage (#8307)
* Initial plan * Add initializer_list pattern detection to code conventions analyzer Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com> * Compile updated code conventions analyzer workflow Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com> * Fix regex patterns in initializer_list search examples Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com> * Fix regex pattern to match both single and double pointers Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: NikolajBjorner <3085284+NikolajBjorner@users.noreply.github.com>
This commit is contained in:
parent
736d5924e5
commit
e3777d86d6
2 changed files with 472 additions and 14 deletions
248
.github/workflows/code-conventions-analyzer.lock.yml
generated
vendored
248
.github/workflows/code-conventions-analyzer.lock.yml
generated
vendored
|
|
@ -653,6 +653,44 @@ jobs:
|
||||||
return g(a, b);
|
return g(a, b);
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**TERTIARY FOCUS: Create Pull Requests for initializer_list Refactoring**
|
||||||
|
|
||||||
|
Your tertiary task is to identify and implement refactorings that use `std::initializer_list<T>` instead of array pointer + size parameters:
|
||||||
|
|
||||||
|
1. **Find array + size parameter patterns** - Functions taking `(unsigned sz, T* args)` or similar
|
||||||
|
2. **Implement the refactoring** - Replace with `std::initializer_list<T>` for cleaner APIs
|
||||||
|
3. **Create pull requests** - Automatically create a PR with your changes for initializer_list improvements
|
||||||
|
|
||||||
|
**Focus Areas for initializer_list Refactoring:**
|
||||||
|
- Functions with `unsigned sz/size/num, T* const* args` parameter pairs
|
||||||
|
- Call sites that create temporary arrays of constant length just to pass to these functions
|
||||||
|
- Internal APIs where changing the signature is safe and beneficial
|
||||||
|
- Functions where the size is always small and known at compile time
|
||||||
|
|
||||||
|
**Example refactoring:**
|
||||||
|
```cpp
|
||||||
|
// Before: Array + size parameters
|
||||||
|
R foo(unsigned sz, T const* args) {
|
||||||
|
for (unsigned i = 0; i < sz; ++i) {
|
||||||
|
use(args[i]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Call site before:
|
||||||
|
T args1[2] = {1, 2};
|
||||||
|
foo(2, args1);
|
||||||
|
|
||||||
|
// After: Using initializer_list
|
||||||
|
R foo(std::initializer_list<T> const& args) {
|
||||||
|
for (auto const& arg : args) {
|
||||||
|
use(arg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Call site after:
|
||||||
|
foo({1, 2});
|
||||||
|
```
|
||||||
|
|
||||||
**Additional Task:**
|
**Additional Task:**
|
||||||
Additionally, conduct analysis of other coding conventions and modern C++ opportunities for discussion (not immediate implementation)
|
Additionally, conduct analysis of other coding conventions and modern C++ opportunities for discussion (not immediate implementation)
|
||||||
|
|
||||||
|
|
@ -777,6 +815,129 @@ jobs:
|
||||||
|
|
||||||
If you identify other code quality issues (naming, formatting, other C++ features), create a **discussion** (not a PR) with those findings using the existing discussion format from the workflow.
|
If you identify other code quality issues (naming, formatting, other C++ features), create a **discussion** (not a PR) with those findings using the existing discussion format from the workflow.
|
||||||
|
|
||||||
|
## Workflow for initializer_list Refactoring (TERTIARY)
|
||||||
|
|
||||||
|
### Step A: Find initializer_list Refactoring Opportunities
|
||||||
|
|
||||||
|
1. **Search for common patterns** that should use `std::initializer_list`:
|
||||||
|
```bash
|
||||||
|
# Functions with unsigned + pointer parameter pairs (generic pattern)
|
||||||
|
grep pattern: "unsigned.*(sz|size|num|n).*\*" glob: "src/**/*.h"
|
||||||
|
|
||||||
|
# Specific patterns like mk_ functions with sz + args
|
||||||
|
grep pattern: "mk_[a-z_]+\(unsigned.*\*" glob: "src/**/*.h"
|
||||||
|
|
||||||
|
# Function declarations with sz/size/num + pointer (more specific, matches both single and double pointers)
|
||||||
|
grep pattern: "\\(unsigned (sz|size|num|n)[^,)]*,\\s*\\w+\\s*\\*(\\s*const)?\\s*\\*?" glob: "src/**/*.h"
|
||||||
|
```
|
||||||
|
|
||||||
|
2. **Analyze candidates** for refactoring:
|
||||||
|
- Use `view` to examine the function implementation
|
||||||
|
- Check call sites to see if they use temporary arrays
|
||||||
|
- Verify that the function is internal (not part of public C API)
|
||||||
|
- Ensure the array size is typically small and known at compile time
|
||||||
|
- Confirm that changing to initializer_list would simplify call sites
|
||||||
|
|
||||||
|
3. **Select 1-2 high-value targets** per run:
|
||||||
|
- Prefer internal helper functions over widely-used APIs
|
||||||
|
- Choose functions where call sites create temporary arrays
|
||||||
|
- Focus on functions that would benefit from simpler call syntax
|
||||||
|
|
||||||
|
### Step B: Implement the Refactoring
|
||||||
|
|
||||||
|
For each selected function:
|
||||||
|
|
||||||
|
1. **Update the function signature** in header file:
|
||||||
|
```cpp
|
||||||
|
// Before:
|
||||||
|
R foo(unsigned sz, T const* args);
|
||||||
|
// or
|
||||||
|
R foo(unsigned sz, T* const* args);
|
||||||
|
|
||||||
|
// After:
|
||||||
|
R foo(std::initializer_list<T> const& args);
|
||||||
|
```
|
||||||
|
|
||||||
|
2. **Update the function implementation**:
|
||||||
|
```cpp
|
||||||
|
// Before:
|
||||||
|
R foo(unsigned sz, T const* args) {
|
||||||
|
for (unsigned i = 0; i < sz; ++i) {
|
||||||
|
process(args[i]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// After:
|
||||||
|
R foo(std::initializer_list<T> const& args) {
|
||||||
|
for (auto const& arg : args) {
|
||||||
|
process(arg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Or access size with args.size() if needed
|
||||||
|
```
|
||||||
|
|
||||||
|
3. **Update all call sites** to use the new API:
|
||||||
|
```cpp
|
||||||
|
// Before:
|
||||||
|
T args1[2] = {1, 2};
|
||||||
|
foo(2, args1);
|
||||||
|
|
||||||
|
// After:
|
||||||
|
foo({1, 2});
|
||||||
|
```
|
||||||
|
|
||||||
|
4. **Add necessary includes**:
|
||||||
|
- Add `#include <initializer_list>` to header file if not already present
|
||||||
|
|
||||||
|
5. **Verify the changes**:
|
||||||
|
- Use `grep` to find any remaining call sites with the old pattern
|
||||||
|
- Check that the refactoring is complete
|
||||||
|
- Ensure no compilation errors would occur
|
||||||
|
|
||||||
|
### Step C: Create the Pull Request
|
||||||
|
|
||||||
|
Use the `output.create-pull-request` tool to create a PR with:
|
||||||
|
- **Title**: "Refactor [function_name] to use std::initializer_list"
|
||||||
|
- **Description**:
|
||||||
|
- Explain what was changed
|
||||||
|
- Why initializer_list is better (cleaner call sites, type safety)
|
||||||
|
- List all modified files
|
||||||
|
- Note any caveats or considerations
|
||||||
|
|
||||||
|
**Example PR description:**
|
||||||
|
```markdown
|
||||||
|
# Refactor to use std::initializer_list
|
||||||
|
|
||||||
|
This PR refactors the following functions to use `std::initializer_list<T>` instead of array pointer + size parameters:
|
||||||
|
|
||||||
|
- `mk_and(unsigned sz, expr* const* args)` in `src/ast/some_file.cpp`
|
||||||
|
- `mk_or(unsigned sz, expr* const* args)` in `src/ast/another_file.cpp`
|
||||||
|
|
||||||
|
## Benefits:
|
||||||
|
- Cleaner call sites: `mk_and({a, b, c})` instead of creating temporary arrays
|
||||||
|
- Type safety: Size is implicit, no mismatch possible
|
||||||
|
- Modern C++ idiom (std::initializer_list is C++11)
|
||||||
|
- Compile-time size verification
|
||||||
|
|
||||||
|
## Changes:
|
||||||
|
- Updated function signatures to take `std::initializer_list<T> const&`
|
||||||
|
- Modified implementations to use range-based for loops or `.size()`
|
||||||
|
- Updated all call sites to use brace-initialization
|
||||||
|
- Added `#include <initializer_list>` where needed
|
||||||
|
|
||||||
|
## Testing:
|
||||||
|
- No functional changes to logic
|
||||||
|
- All existing call sites updated
|
||||||
|
|
||||||
|
## Considerations:
|
||||||
|
- Only applied to internal functions where call sites typically use small, fixed-size arrays
|
||||||
|
- Public C API functions were not modified to maintain compatibility
|
||||||
|
```
|
||||||
|
|
||||||
|
### Step D: Create Discussion for Other Findings
|
||||||
|
|
||||||
|
If you identify other code quality issues, create a **discussion** with those findings.
|
||||||
|
|
||||||
## Step 1: Initialize or Resume Progress (Cache Memory)
|
## Step 1: Initialize or Resume Progress (Cache Memory)
|
||||||
|
|
||||||
**Check your cache memory for:**
|
**Check your cache memory for:**
|
||||||
|
|
@ -892,6 +1053,8 @@ jobs:
|
||||||
- Classes with virtual functions but non-virtual destructors (potential issue)
|
- Classes with virtual functions but non-virtual destructors (potential issue)
|
||||||
- Base classes without virtual destructors (check if inheritance is intended)
|
- Base classes without virtual destructors (check if inheritance is intended)
|
||||||
- Non-virtual destructors missing `noexcept` (should be added)
|
- Non-virtual destructors missing `noexcept` (should be added)
|
||||||
|
PROMPT_EOF
|
||||||
|
cat << 'PROMPT_EOF' >> "$GH_AW_PROMPT"
|
||||||
- Leaf classes with unnecessary virtual destructors (minor overhead)
|
- Leaf classes with unnecessary virtual destructors (minor overhead)
|
||||||
- Missing `noexcept` on non-default constructors and destructors
|
- Missing `noexcept` on non-default constructors and destructors
|
||||||
- Opportunities to use compiler-generated special members (`= default`, `= delete`)
|
- Opportunities to use compiler-generated special members (`= default`, `= delete`)
|
||||||
|
|
@ -1025,8 +1188,6 @@ jobs:
|
||||||
- If compile_commands.json can be generated with clang, and clang-tidy
|
- If compile_commands.json can be generated with clang, and clang-tidy
|
||||||
is available, run a targeted checkset on the selected files:
|
is available, run a targeted checkset on the selected files:
|
||||||
- modernize-use-nullptr
|
- modernize-use-nullptr
|
||||||
PROMPT_EOF
|
|
||||||
cat << 'PROMPT_EOF' >> "$GH_AW_PROMPT"
|
|
||||||
- modernize-use-override
|
- modernize-use-override
|
||||||
- modernize-loop-convert (review carefully)
|
- modernize-loop-convert (review carefully)
|
||||||
- bugprone-* (selected high-signal checks)
|
- bugprone-* (selected high-signal checks)
|
||||||
|
|
@ -1290,6 +1451,8 @@ jobs:
|
||||||
1. Find candidate functions
|
1. Find candidate functions
|
||||||
2. Refactor using the `edit` tool
|
2. Refactor using the `edit` tool
|
||||||
3. Create a pull request with your changes
|
3. Create a pull request with your changes
|
||||||
|
PROMPT_EOF
|
||||||
|
cat << 'PROMPT_EOF' >> "$GH_AW_PROMPT"
|
||||||
- **API Improvements**: Specific function signatures to update
|
- **API Improvements**: Specific function signatures to update
|
||||||
- **Examples**: File:line references with before/after code
|
- **Examples**: File:line references with before/after code
|
||||||
- **Output**: Pull request (not just discussion)
|
- **Output**: Pull request (not just discussion)
|
||||||
|
|
@ -1359,24 +1522,76 @@ jobs:
|
||||||
- **String Copies**: [Unnecessary copies when raising exceptions]
|
- **String Copies**: [Unnecessary copies when raising exceptions]
|
||||||
- **Examples**: [Specific exception construction sites]
|
- **Examples**: [Specific exception construction sites]
|
||||||
|
|
||||||
### 4.11 Array Parameter Modernization
|
### 4.11 Array Parameter Modernization (std::span)
|
||||||
- **Current**: [Pointer + size parameter pairs]
|
- **Current**: [Pointer + size parameter pairs for runtime-sized arrays]
|
||||||
- **Modern**: [std::span usage opportunities]
|
- **Modern**: [std::span usage opportunities]
|
||||||
- **Type Safety**: [How span improves API safety]
|
- **Type Safety**: [How span improves API safety]
|
||||||
- **Examples**: [Function signatures to update]
|
- **Examples**: [Function signatures to update]
|
||||||
|
|
||||||
### 4.12 Increment Operator Patterns
|
### 4.12 Array Parameter Modernization (std::initializer_list) - **IMPLEMENT AS PULL REQUEST**
|
||||||
|
|
||||||
|
**This is a TERTIARY focus area - implement these changes directly:**
|
||||||
|
|
||||||
|
- **Current Pattern**: Functions with `unsigned sz, T* args` or `unsigned sz, T* const* args` parameters
|
||||||
|
- **Modern Pattern**: Use `std::initializer_list<T>` for functions called with compile-time constant arrays
|
||||||
|
- **Benefits**:
|
||||||
|
- Cleaner call sites: `foo({1, 2, 3})` instead of creating temporary arrays
|
||||||
|
- No size/pointer mismatch possible
|
||||||
|
- Type safety with implicit size
|
||||||
|
- More readable and concise
|
||||||
|
- **Action**: Find and refactor array + size parameter patterns:
|
||||||
|
1. Search for functions with `unsigned sz/size/num` + pointer parameters
|
||||||
|
2. Identify functions where call sites use temporary arrays of constant size
|
||||||
|
3. Refactor to use `std::initializer_list<T> const&`
|
||||||
|
4. Create a pull request with changes
|
||||||
|
- **Example Pattern**:
|
||||||
|
```cpp
|
||||||
|
// Before: Array + size parameters
|
||||||
|
R foo(unsigned sz, T const* args) {
|
||||||
|
for (unsigned i = 0; i < sz; ++i) {
|
||||||
|
process(args[i]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Call site before:
|
||||||
|
T args1[2] = {1, 2};
|
||||||
|
foo(2, args1);
|
||||||
|
|
||||||
|
// After: Using initializer_list
|
||||||
|
R foo(std::initializer_list<T> const& args) {
|
||||||
|
for (auto const& arg : args) {
|
||||||
|
process(arg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Call site after:
|
||||||
|
foo({1, 2});
|
||||||
|
```
|
||||||
|
- **Search Patterns**: Look for:
|
||||||
|
- Function signatures with `unsigned sz/size/num/n` followed by pointer parameter
|
||||||
|
- Common Z3 patterns like `mk_and(unsigned sz, expr* const* args)`
|
||||||
|
- Internal helper functions (not public C API)
|
||||||
|
- Functions where typical call sites use small, fixed arrays
|
||||||
|
- **Candidates**: Functions that:
|
||||||
|
- Are called with temporary arrays created at call site
|
||||||
|
- Have small, compile-time known array sizes
|
||||||
|
- Are internal APIs (not part of public C interface)
|
||||||
|
- Would benefit from simpler call syntax
|
||||||
|
- **Output**: Pull request with refactored code
|
||||||
|
- **Note**: Only apply to internal C++ APIs, not to public C API functions that need C compatibility
|
||||||
|
|
||||||
|
### 4.13 Increment Operator Patterns
|
||||||
- **Postfix Usage**: [Count of i++ where result is unused]
|
- **Postfix Usage**: [Count of i++ where result is unused]
|
||||||
- **Prefix Preference**: [Places to use ++i instead]
|
- **Prefix Preference**: [Places to use ++i instead]
|
||||||
- **Iterator Loops**: [Heavy iterator usage areas]
|
- **Iterator Loops**: [Heavy iterator usage areas]
|
||||||
|
|
||||||
### 4.13 Exception Control Flow
|
### 4.14 Exception Control Flow
|
||||||
- **Current Usage**: [Exceptions used for normal control flow]
|
- **Current Usage**: [Exceptions used for normal control flow]
|
||||||
- **Modern Alternatives**: [std::expected, std::optional, error codes]
|
- **Modern Alternatives**: [std::expected, std::optional, error codes]
|
||||||
- **Performance**: [Impact of exception-based control flow]
|
- **Performance**: [Impact of exception-based control flow]
|
||||||
- **Refactoring Opportunities**: [Specific patterns to replace]
|
- **Refactoring Opportunities**: [Specific patterns to replace]
|
||||||
|
|
||||||
### 4.14 Inefficient Stream Output
|
### 4.15 Inefficient Stream Output
|
||||||
- **Current Usage**: [string stream output operator used for single characters]
|
- **Current Usage**: [string stream output operator used for single characters]
|
||||||
- **Modern Alternatives**: [use char output operator]
|
- **Modern Alternatives**: [use char output operator]
|
||||||
- **Performance**: [Reduce code size and improve performance]
|
- **Performance**: [Reduce code size and improve performance]
|
||||||
|
|
@ -1450,8 +1665,6 @@ jobs:
|
||||||
- Date last verified
|
- Date last verified
|
||||||
|
|
||||||
3. **Track analysis progress**:
|
3. **Track analysis progress**:
|
||||||
PROMPT_EOF
|
|
||||||
cat << 'PROMPT_EOF' >> "$GH_AW_PROMPT"
|
|
||||||
- Which directories/areas have been analyzed
|
- Which directories/areas have been analyzed
|
||||||
- Which analysis categories have been covered
|
- Which analysis categories have been covered
|
||||||
- Percentage of codebase examined
|
- Percentage of codebase examined
|
||||||
|
|
@ -1641,6 +1854,21 @@ jobs:
|
||||||
grep pattern: "\([^,]+\*[^,]*,\s*size_t|, unsigned.*size\)" glob: "src/**/*.h"
|
grep pattern: "\([^,]+\*[^,]*,\s*size_t|, unsigned.*size\)" glob: "src/**/*.h"
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Find array + size parameters (initializer_list opportunities):**
|
||||||
|
```
|
||||||
|
# Functions with unsigned sz/size/num + pointer parameter pairs (matches both single and double pointers)
|
||||||
|
grep pattern: "\\(unsigned (sz|size|num|n)[^,)]*,\\s*\\w+\\s*\\*(\\s*const)?\\s*\\*?" glob: "src/**/*.h"
|
||||||
|
|
||||||
|
# Common Z3 patterns like mk_ functions
|
||||||
|
grep pattern: "mk_[a-z_]+\(unsigned.*\*" glob: "src/**/*.h"
|
||||||
|
|
||||||
|
# Function declarations with size + pointer combinations (broader pattern)
|
||||||
|
grep pattern: "unsigned.*(sz|size|num|n).*\*" glob: "src/**/*.h"
|
||||||
|
|
||||||
|
# Call sites creating temporary arrays
|
||||||
|
grep pattern: "\w+\s+\w+\[[0-9]+\]\s*=\s*\{.*\};" glob: "src/**/*.cpp"
|
||||||
|
```
|
||||||
|
|
||||||
**Find postfix increment:**
|
**Find postfix increment:**
|
||||||
```
|
```
|
||||||
grep pattern: "[a-z_]+\+\+\s*[;\)]" glob: "src/**/*.cpp"
|
grep pattern: "[a-z_]+\+\+\s*[;\)]" glob: "src/**/*.cpp"
|
||||||
|
|
@ -1687,6 +1915,8 @@ jobs:
|
||||||
- Use the structured format above
|
- Use the structured format above
|
||||||
- Include specific file references for all examples
|
- Include specific file references for all examples
|
||||||
- Provide actionable recommendations
|
- Provide actionable recommendations
|
||||||
|
PROMPT_EOF
|
||||||
|
cat << 'PROMPT_EOF' >> "$GH_AW_PROMPT"
|
||||||
- Previous discussions created by this workflow will be automatically closed (using `close-older-discussions: true`)
|
- Previous discussions created by this workflow will be automatically closed (using `close-older-discussions: true`)
|
||||||
|
|
||||||
PROMPT_EOF
|
PROMPT_EOF
|
||||||
|
|
|
||||||
238
.github/workflows/code-conventions-analyzer.md
vendored
238
.github/workflows/code-conventions-analyzer.md
vendored
|
|
@ -78,6 +78,44 @@ auto [a, b] = f(y);
|
||||||
return g(a, b);
|
return g(a, b);
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**TERTIARY FOCUS: Create Pull Requests for initializer_list Refactoring**
|
||||||
|
|
||||||
|
Your tertiary task is to identify and implement refactorings that use `std::initializer_list<T>` instead of array pointer + size parameters:
|
||||||
|
|
||||||
|
1. **Find array + size parameter patterns** - Functions taking `(unsigned sz, T* args)` or similar
|
||||||
|
2. **Implement the refactoring** - Replace with `std::initializer_list<T>` for cleaner APIs
|
||||||
|
3. **Create pull requests** - Automatically create a PR with your changes for initializer_list improvements
|
||||||
|
|
||||||
|
**Focus Areas for initializer_list Refactoring:**
|
||||||
|
- Functions with `unsigned sz/size/num, T* const* args` parameter pairs
|
||||||
|
- Call sites that create temporary arrays of constant length just to pass to these functions
|
||||||
|
- Internal APIs where changing the signature is safe and beneficial
|
||||||
|
- Functions where the size is always small and known at compile time
|
||||||
|
|
||||||
|
**Example refactoring:**
|
||||||
|
```cpp
|
||||||
|
// Before: Array + size parameters
|
||||||
|
R foo(unsigned sz, T const* args) {
|
||||||
|
for (unsigned i = 0; i < sz; ++i) {
|
||||||
|
use(args[i]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Call site before:
|
||||||
|
T args1[2] = {1, 2};
|
||||||
|
foo(2, args1);
|
||||||
|
|
||||||
|
// After: Using initializer_list
|
||||||
|
R foo(std::initializer_list<T> const& args) {
|
||||||
|
for (auto const& arg : args) {
|
||||||
|
use(arg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Call site after:
|
||||||
|
foo({1, 2});
|
||||||
|
```
|
||||||
|
|
||||||
**Additional Task:**
|
**Additional Task:**
|
||||||
Additionally, conduct analysis of other coding conventions and modern C++ opportunities for discussion (not immediate implementation)
|
Additionally, conduct analysis of other coding conventions and modern C++ opportunities for discussion (not immediate implementation)
|
||||||
|
|
||||||
|
|
@ -202,6 +240,129 @@ This PR refactors the following functions to use `std::optional<T>` instead of p
|
||||||
|
|
||||||
If you identify other code quality issues (naming, formatting, other C++ features), create a **discussion** (not a PR) with those findings using the existing discussion format from the workflow.
|
If you identify other code quality issues (naming, formatting, other C++ features), create a **discussion** (not a PR) with those findings using the existing discussion format from the workflow.
|
||||||
|
|
||||||
|
## Workflow for initializer_list Refactoring (TERTIARY)
|
||||||
|
|
||||||
|
### Step A: Find initializer_list Refactoring Opportunities
|
||||||
|
|
||||||
|
1. **Search for common patterns** that should use `std::initializer_list`:
|
||||||
|
```bash
|
||||||
|
# Functions with unsigned + pointer parameter pairs (generic pattern)
|
||||||
|
grep pattern: "unsigned.*(sz|size|num|n).*\*" glob: "src/**/*.h"
|
||||||
|
|
||||||
|
# Specific patterns like mk_ functions with sz + args
|
||||||
|
grep pattern: "mk_[a-z_]+\(unsigned.*\*" glob: "src/**/*.h"
|
||||||
|
|
||||||
|
# Function declarations with sz/size/num + pointer (more specific, matches both single and double pointers)
|
||||||
|
grep pattern: "\\(unsigned (sz|size|num|n)[^,)]*,\\s*\\w+\\s*\\*(\\s*const)?\\s*\\*?" glob: "src/**/*.h"
|
||||||
|
```
|
||||||
|
|
||||||
|
2. **Analyze candidates** for refactoring:
|
||||||
|
- Use `view` to examine the function implementation
|
||||||
|
- Check call sites to see if they use temporary arrays
|
||||||
|
- Verify that the function is internal (not part of public C API)
|
||||||
|
- Ensure the array size is typically small and known at compile time
|
||||||
|
- Confirm that changing to initializer_list would simplify call sites
|
||||||
|
|
||||||
|
3. **Select 1-2 high-value targets** per run:
|
||||||
|
- Prefer internal helper functions over widely-used APIs
|
||||||
|
- Choose functions where call sites create temporary arrays
|
||||||
|
- Focus on functions that would benefit from simpler call syntax
|
||||||
|
|
||||||
|
### Step B: Implement the Refactoring
|
||||||
|
|
||||||
|
For each selected function:
|
||||||
|
|
||||||
|
1. **Update the function signature** in header file:
|
||||||
|
```cpp
|
||||||
|
// Before:
|
||||||
|
R foo(unsigned sz, T const* args);
|
||||||
|
// or
|
||||||
|
R foo(unsigned sz, T* const* args);
|
||||||
|
|
||||||
|
// After:
|
||||||
|
R foo(std::initializer_list<T> const& args);
|
||||||
|
```
|
||||||
|
|
||||||
|
2. **Update the function implementation**:
|
||||||
|
```cpp
|
||||||
|
// Before:
|
||||||
|
R foo(unsigned sz, T const* args) {
|
||||||
|
for (unsigned i = 0; i < sz; ++i) {
|
||||||
|
process(args[i]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// After:
|
||||||
|
R foo(std::initializer_list<T> const& args) {
|
||||||
|
for (auto const& arg : args) {
|
||||||
|
process(arg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
// Or access size with args.size() if needed
|
||||||
|
```
|
||||||
|
|
||||||
|
3. **Update all call sites** to use the new API:
|
||||||
|
```cpp
|
||||||
|
// Before:
|
||||||
|
T args1[2] = {1, 2};
|
||||||
|
foo(2, args1);
|
||||||
|
|
||||||
|
// After:
|
||||||
|
foo({1, 2});
|
||||||
|
```
|
||||||
|
|
||||||
|
4. **Add necessary includes**:
|
||||||
|
- Add `#include <initializer_list>` to header file if not already present
|
||||||
|
|
||||||
|
5. **Verify the changes**:
|
||||||
|
- Use `grep` to find any remaining call sites with the old pattern
|
||||||
|
- Check that the refactoring is complete
|
||||||
|
- Ensure no compilation errors would occur
|
||||||
|
|
||||||
|
### Step C: Create the Pull Request
|
||||||
|
|
||||||
|
Use the `output.create-pull-request` tool to create a PR with:
|
||||||
|
- **Title**: "Refactor [function_name] to use std::initializer_list"
|
||||||
|
- **Description**:
|
||||||
|
- Explain what was changed
|
||||||
|
- Why initializer_list is better (cleaner call sites, type safety)
|
||||||
|
- List all modified files
|
||||||
|
- Note any caveats or considerations
|
||||||
|
|
||||||
|
**Example PR description:**
|
||||||
|
```markdown
|
||||||
|
# Refactor to use std::initializer_list
|
||||||
|
|
||||||
|
This PR refactors the following functions to use `std::initializer_list<T>` instead of array pointer + size parameters:
|
||||||
|
|
||||||
|
- `mk_and(unsigned sz, expr* const* args)` in `src/ast/some_file.cpp`
|
||||||
|
- `mk_or(unsigned sz, expr* const* args)` in `src/ast/another_file.cpp`
|
||||||
|
|
||||||
|
## Benefits:
|
||||||
|
- Cleaner call sites: `mk_and({a, b, c})` instead of creating temporary arrays
|
||||||
|
- Type safety: Size is implicit, no mismatch possible
|
||||||
|
- Modern C++ idiom (std::initializer_list is C++11)
|
||||||
|
- Compile-time size verification
|
||||||
|
|
||||||
|
## Changes:
|
||||||
|
- Updated function signatures to take `std::initializer_list<T> const&`
|
||||||
|
- Modified implementations to use range-based for loops or `.size()`
|
||||||
|
- Updated all call sites to use brace-initialization
|
||||||
|
- Added `#include <initializer_list>` where needed
|
||||||
|
|
||||||
|
## Testing:
|
||||||
|
- No functional changes to logic
|
||||||
|
- All existing call sites updated
|
||||||
|
|
||||||
|
## Considerations:
|
||||||
|
- Only applied to internal functions where call sites typically use small, fixed-size arrays
|
||||||
|
- Public C API functions were not modified to maintain compatibility
|
||||||
|
```
|
||||||
|
|
||||||
|
### Step D: Create Discussion for Other Findings
|
||||||
|
|
||||||
|
If you identify other code quality issues, create a **discussion** with those findings.
|
||||||
|
|
||||||
## Step 1: Initialize or Resume Progress (Cache Memory)
|
## Step 1: Initialize or Resume Progress (Cache Memory)
|
||||||
|
|
||||||
**Check your cache memory for:**
|
**Check your cache memory for:**
|
||||||
|
|
@ -782,24 +943,76 @@ For each opportunity, provide:
|
||||||
- **String Copies**: [Unnecessary copies when raising exceptions]
|
- **String Copies**: [Unnecessary copies when raising exceptions]
|
||||||
- **Examples**: [Specific exception construction sites]
|
- **Examples**: [Specific exception construction sites]
|
||||||
|
|
||||||
### 4.11 Array Parameter Modernization
|
### 4.11 Array Parameter Modernization (std::span)
|
||||||
- **Current**: [Pointer + size parameter pairs]
|
- **Current**: [Pointer + size parameter pairs for runtime-sized arrays]
|
||||||
- **Modern**: [std::span usage opportunities]
|
- **Modern**: [std::span usage opportunities]
|
||||||
- **Type Safety**: [How span improves API safety]
|
- **Type Safety**: [How span improves API safety]
|
||||||
- **Examples**: [Function signatures to update]
|
- **Examples**: [Function signatures to update]
|
||||||
|
|
||||||
### 4.12 Increment Operator Patterns
|
### 4.12 Array Parameter Modernization (std::initializer_list) - **IMPLEMENT AS PULL REQUEST**
|
||||||
|
|
||||||
|
**This is a TERTIARY focus area - implement these changes directly:**
|
||||||
|
|
||||||
|
- **Current Pattern**: Functions with `unsigned sz, T* args` or `unsigned sz, T* const* args` parameters
|
||||||
|
- **Modern Pattern**: Use `std::initializer_list<T>` for functions called with compile-time constant arrays
|
||||||
|
- **Benefits**:
|
||||||
|
- Cleaner call sites: `foo({1, 2, 3})` instead of creating temporary arrays
|
||||||
|
- No size/pointer mismatch possible
|
||||||
|
- Type safety with implicit size
|
||||||
|
- More readable and concise
|
||||||
|
- **Action**: Find and refactor array + size parameter patterns:
|
||||||
|
1. Search for functions with `unsigned sz/size/num` + pointer parameters
|
||||||
|
2. Identify functions where call sites use temporary arrays of constant size
|
||||||
|
3. Refactor to use `std::initializer_list<T> const&`
|
||||||
|
4. Create a pull request with changes
|
||||||
|
- **Example Pattern**:
|
||||||
|
```cpp
|
||||||
|
// Before: Array + size parameters
|
||||||
|
R foo(unsigned sz, T const* args) {
|
||||||
|
for (unsigned i = 0; i < sz; ++i) {
|
||||||
|
process(args[i]);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Call site before:
|
||||||
|
T args1[2] = {1, 2};
|
||||||
|
foo(2, args1);
|
||||||
|
|
||||||
|
// After: Using initializer_list
|
||||||
|
R foo(std::initializer_list<T> const& args) {
|
||||||
|
for (auto const& arg : args) {
|
||||||
|
process(arg);
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// Call site after:
|
||||||
|
foo({1, 2});
|
||||||
|
```
|
||||||
|
- **Search Patterns**: Look for:
|
||||||
|
- Function signatures with `unsigned sz/size/num/n` followed by pointer parameter
|
||||||
|
- Common Z3 patterns like `mk_and(unsigned sz, expr* const* args)`
|
||||||
|
- Internal helper functions (not public C API)
|
||||||
|
- Functions where typical call sites use small, fixed arrays
|
||||||
|
- **Candidates**: Functions that:
|
||||||
|
- Are called with temporary arrays created at call site
|
||||||
|
- Have small, compile-time known array sizes
|
||||||
|
- Are internal APIs (not part of public C interface)
|
||||||
|
- Would benefit from simpler call syntax
|
||||||
|
- **Output**: Pull request with refactored code
|
||||||
|
- **Note**: Only apply to internal C++ APIs, not to public C API functions that need C compatibility
|
||||||
|
|
||||||
|
### 4.13 Increment Operator Patterns
|
||||||
- **Postfix Usage**: [Count of i++ where result is unused]
|
- **Postfix Usage**: [Count of i++ where result is unused]
|
||||||
- **Prefix Preference**: [Places to use ++i instead]
|
- **Prefix Preference**: [Places to use ++i instead]
|
||||||
- **Iterator Loops**: [Heavy iterator usage areas]
|
- **Iterator Loops**: [Heavy iterator usage areas]
|
||||||
|
|
||||||
### 4.13 Exception Control Flow
|
### 4.14 Exception Control Flow
|
||||||
- **Current Usage**: [Exceptions used for normal control flow]
|
- **Current Usage**: [Exceptions used for normal control flow]
|
||||||
- **Modern Alternatives**: [std::expected, std::optional, error codes]
|
- **Modern Alternatives**: [std::expected, std::optional, error codes]
|
||||||
- **Performance**: [Impact of exception-based control flow]
|
- **Performance**: [Impact of exception-based control flow]
|
||||||
- **Refactoring Opportunities**: [Specific patterns to replace]
|
- **Refactoring Opportunities**: [Specific patterns to replace]
|
||||||
|
|
||||||
### 4.14 Inefficient Stream Output
|
### 4.15 Inefficient Stream Output
|
||||||
- **Current Usage**: [string stream output operator used for single characters]
|
- **Current Usage**: [string stream output operator used for single characters]
|
||||||
- **Modern Alternatives**: [use char output operator]
|
- **Modern Alternatives**: [use char output operator]
|
||||||
- **Performance**: [Reduce code size and improve performance]
|
- **Performance**: [Reduce code size and improve performance]
|
||||||
|
|
@ -1062,6 +1275,21 @@ grep pattern: "\([^)]*\.first[^)]*\.second[^)]*\)" glob: "src/**/*.cpp"
|
||||||
grep pattern: "\([^,]+\*[^,]*,\s*size_t|, unsigned.*size\)" glob: "src/**/*.h"
|
grep pattern: "\([^,]+\*[^,]*,\s*size_t|, unsigned.*size\)" glob: "src/**/*.h"
|
||||||
```
|
```
|
||||||
|
|
||||||
|
**Find array + size parameters (initializer_list opportunities):**
|
||||||
|
```
|
||||||
|
# Functions with unsigned sz/size/num + pointer parameter pairs (matches both single and double pointers)
|
||||||
|
grep pattern: "\\(unsigned (sz|size|num|n)[^,)]*,\\s*\\w+\\s*\\*(\\s*const)?\\s*\\*?" glob: "src/**/*.h"
|
||||||
|
|
||||||
|
# Common Z3 patterns like mk_ functions
|
||||||
|
grep pattern: "mk_[a-z_]+\(unsigned.*\*" glob: "src/**/*.h"
|
||||||
|
|
||||||
|
# Function declarations with size + pointer combinations (broader pattern)
|
||||||
|
grep pattern: "unsigned.*(sz|size|num|n).*\*" glob: "src/**/*.h"
|
||||||
|
|
||||||
|
# Call sites creating temporary arrays
|
||||||
|
grep pattern: "\w+\s+\w+\[[0-9]+\]\s*=\s*\{.*\};" glob: "src/**/*.cpp"
|
||||||
|
```
|
||||||
|
|
||||||
**Find postfix increment:**
|
**Find postfix increment:**
|
||||||
```
|
```
|
||||||
grep pattern: "[a-z_]+\+\+\s*[;\)]" glob: "src/**/*.cpp"
|
grep pattern: "[a-z_]+\+\+\s*[;\)]" glob: "src/**/*.cpp"
|
||||||
|
|
|
||||||
Loading…
Add table
Add a link
Reference in a new issue