Copilot commented on code in PR #12889:
URL: https://github.com/apache/trafficserver/pull/12889#discussion_r2814085778
##########
.github/copilot-instructions.md:
##########
@@ -0,0 +1,422 @@
+# Apache Traffic Server - GitHub Copilot Instructions
+
+## Repository Overview
+
+Apache Traffic Server (ATS) is a high-performance HTTP/HTTPS caching proxy
server written in C++20. It processes large-scale web traffic using an
event-driven, multi-threaded architecture with a sophisticated plugin system.
+
+**Key Facts:**
+- ~500K lines of C++20 code (strict - no C++23)
+- Event-driven architecture using Continuation callbacks
+- Handles HTTP/1.1, HTTP/2, HTTP/3 (QUIC)
+- Supports TLS termination and caching
+- Extensible via C/C++ plugins
+
+## Code Style Requirements
+
+### C++ Style (Enforced by CI)
+
+**Formatting Rules:**
+- **Line length:** 132 characters maximum
+- **Indentation:** 2 spaces (never tabs)
+- **Braces:** Linux kernel style (opening brace on same line)
+```cpp
+if (condition) {
+ // code here
+}
+```
+- **Pointer/reference alignment:** Right side
+```cpp
+Type *ptr; // Correct
+Type &ref; // Correct
+Type* ptr; // Wrong
+```
+- **Empty line after declarations:**
+```cpp
+void function() {
+ int x = 5;
+ std::string name = "test";
+
+ // Empty line before first code statement
+ process_data(x, name);
+}
+```
+- **Always use braces** for if/while/for (no naked conditions):
+```cpp
+if (x > 0) { // Correct
+ foo();
+}
+
+if (x > 0) // Wrong - missing braces
+ foo();
+```
+- **Keep variable declarations together:**
+```cpp
+// Correct - declarations grouped together
+void function() {
+ int count = 0;
+ std::string name = "test";
+ auto *buffer = new_buffer();
+
+ // Empty line before first code statement
+ process_data(count, name, buffer);
+}
+
+// Wrong - declarations scattered
+void function() {
+ int count = 0;
+ process_count(count);
+ std::string name = "test"; // Don't scatter declarations
+}
+```
+
+**Naming Conventions:**
+- Classes: `CamelCase` → `HttpSM`, `NetVConnection`, `CacheProcessor`
+- Functions/variables: `snake_case` → `handle_request()`, `server_port`,
`cache_key`
+- Constants/macros: `UPPER_CASE` → `HTTP_STATUS_OK`, `MAX_BUFFER_SIZE`
+- Member variables: `snake_case` with no prefix → `connection_count`,
`buffer_size`
+
+**C++20 Patterns (Use These):**
+```cpp
+// GOOD - Modern C++20
+auto buffer = std::make_unique<MIOBuffer>(size);
+for (const auto &entry : container) {
+ if (auto *ptr = entry.get(); ptr != nullptr) {
+ process(ptr);
+ }
+}
+
+// AVOID - Legacy C-style
+MIOBuffer *buffer = (MIOBuffer*)malloc(sizeof(MIOBuffer));
+for (int i = 0; i < container.size(); i++) {
+ process(container[i]);
+}
+```
+
+**Memory Management:**
+- Use RAII and smart pointers (`std::unique_ptr`, `std::shared_ptr`)
+- Use `ats_malloc()`/`ats_free()` for large allocations (not `malloc`)
+- Use `IOBuffer` for network data (zero-copy design)
+- Never call `delete` manually - use RAII
Review Comment:
“Never call `delete` manually” is inconsistent with established ATS patterns
(there are many `delete` / `delete this` sites for Continuation lifetimes and
other ownership models). Suggest rewording this as “prefer RAII/smart pointers
where practical” and noting that some subsystems legitimately use explicit
deletes/self-deletion patterns.
```suggestion
- Prefer RAII/smart pointers over manual `delete` where practical; some
subsystems legitimately use explicit deletes / `delete this`
```
##########
.github/instructions/HRW.instructions.md:
##########
@@ -0,0 +1,218 @@
+---
+applyTo:
+ - "plugins/header_rewrite/**/*"
+ - "tools/hrw4u/**/*"
+---
+
+# Header Rewrite Plugin and HRW4U Transpiler
+
+## Overview
+
+Two closely related components that must be kept in sync:
+
+1. **header_rewrite plugin** (`plugins/header_rewrite/`) - ATS plugin for
modifying HTTP headers
+2. **hrw4u transpiler** (`tools/hrw4u/`) - DSL compiler for generating
header_rewrite configurations
+
+## Critical Requirement: Feature Synchronization
+
+**Features added to either component may require corresponding changes in the
other.**
+
+### When to Update Both
+
+- **New operator in header_rewrite** → Add syntax and code generation in hrw4u
+- **New condition in header_rewrite** → Add parsing and symbols in hrw4u
+- **New variable/resource in header_rewrite** → Update hrw4u symbol tables and
types
+- **New hook in header_rewrite** → Add hook syntax in hrw4u
+- **New hrw4u syntax** → Ensure correct header_rewrite config generation
+
+### Bidirectional Compilation
+
+Both directions must work:
+- **hrw4u** (forward): HRW4U source → header_rewrite config
+- **u4wrh** (reverse): header_rewrite config → HRW4U source
+
+Round-trip test: `hrw4u example.hrw4u | u4wrh` should produce equivalent
output.
+
+## Header Rewrite Plugin
+
+### Architecture
+
+**Core files:**
+- `parser.cc/h` - Configuration syntax parser
+- `factory.cc/h` - Factory for operators and conditions
+- `operators.cc/h` - Header manipulation operations
+- `conditions.cc/h` - Conditional logic
+- `resources.cc/h` - Available variables (headers, IPs, etc.)
+- `statement.cc/h` - Rule statement abstraction
+- `ruleset.cc/h` - Rule collection and execution
+- `matcher.cc/h` - Pattern matching
+- `value.cc/h` - Value extraction and manipulation
+
+### Adding Features
+
+**New operator:**
+1. Define class in `operators.h`, implement in `operators.cc`
+2. Register in `factory.cc`
+3. Update hrw4u: `generators.py` for code gen, `hrw_symbols.py` for symbols
+
+**New condition:**
+1. Define class in `conditions.h`, implement in `conditions.cc`
+2. Register in `factory.cc`
+3. Update hrw4u: `hrw_visitor.py` for parsing, `hrw_symbols.py` for symbols
Review Comment:
This “New operator” checklist points to `generators.py` for code generation
and `hrw_symbols.py` for symbols, but in the current hrw4u codebase the forward
mapping tables live in `tools/hrw4u/src/tables.py`, and the forward compiler
logic is in `tools/hrw4u/src/visitor.py`. `generators.py` is used for
derived/reverse mapping generation. Updating these file pointers would make the
guidance actionable.
```suggestion
3. Update hrw4u: `tables.py` (forward mapping tables), `visitor.py` (forward
compiler logic), and `generators.py` if reverse mappings are affected
**New condition:**
1. Define class in `conditions.h`, implement in `conditions.cc`
2. Register in `factory.cc`
3. Update hrw4u: `visitor.py` for parsing/forward logic and `tables.py` for
mapping/symbol tables
```
##########
.github/instructions/HRW.instructions.md:
##########
@@ -0,0 +1,218 @@
+---
+applyTo:
+ - "plugins/header_rewrite/**/*"
+ - "tools/hrw4u/**/*"
+---
+
+# Header Rewrite Plugin and HRW4U Transpiler
+
+## Overview
+
+Two closely related components that must be kept in sync:
+
+1. **header_rewrite plugin** (`plugins/header_rewrite/`) - ATS plugin for
modifying HTTP headers
+2. **hrw4u transpiler** (`tools/hrw4u/`) - DSL compiler for generating
header_rewrite configurations
+
+## Critical Requirement: Feature Synchronization
+
+**Features added to either component may require corresponding changes in the
other.**
+
+### When to Update Both
+
+- **New operator in header_rewrite** → Add syntax and code generation in hrw4u
+- **New condition in header_rewrite** → Add parsing and symbols in hrw4u
+- **New variable/resource in header_rewrite** → Update hrw4u symbol tables and
types
+- **New hook in header_rewrite** → Add hook syntax in hrw4u
+- **New hrw4u syntax** → Ensure correct header_rewrite config generation
+
+### Bidirectional Compilation
+
+Both directions must work:
+- **hrw4u** (forward): HRW4U source → header_rewrite config
+- **u4wrh** (reverse): header_rewrite config → HRW4U source
+
+Round-trip test: `hrw4u example.hrw4u | u4wrh` should produce equivalent
output.
+
+## Header Rewrite Plugin
+
+### Architecture
+
+**Core files:**
+- `parser.cc/h` - Configuration syntax parser
+- `factory.cc/h` - Factory for operators and conditions
+- `operators.cc/h` - Header manipulation operations
+- `conditions.cc/h` - Conditional logic
+- `resources.cc/h` - Available variables (headers, IPs, etc.)
+- `statement.cc/h` - Rule statement abstraction
+- `ruleset.cc/h` - Rule collection and execution
+- `matcher.cc/h` - Pattern matching
+- `value.cc/h` - Value extraction and manipulation
+
+### Adding Features
+
+**New operator:**
+1. Define class in `operators.h`, implement in `operators.cc`
+2. Register in `factory.cc`
+3. Update hrw4u: `generators.py` for code gen, `hrw_symbols.py` for symbols
+
+**New condition:**
+1. Define class in `conditions.h`, implement in `conditions.cc`
+2. Register in `factory.cc`
+3. Update hrw4u: `hrw_visitor.py` for parsing, `hrw_symbols.py` for symbols
Review Comment:
This “New condition” checklist says to update `hrw_visitor.py` for parsing,
but `tools/hrw4u/src/hrw_visitor.py` is the inverse visitor used by `u4wrh`.
For forward parsing/compilation the relevant module is
`tools/hrw4u/src/visitor.py` (plus `tables.py` for the symbol maps). Please
adjust the referenced files so contributors modify the correct side of the
pipeline.
```suggestion
3. Update hrw4u: `visitor.py` for parsing, `tables.py` for symbol maps
```
##########
.github/copilot-instructions.md:
##########
@@ -0,0 +1,422 @@
+# Apache Traffic Server - GitHub Copilot Instructions
+
+## Repository Overview
+
+Apache Traffic Server (ATS) is a high-performance HTTP/HTTPS caching proxy
server written in C++20. It processes large-scale web traffic using an
event-driven, multi-threaded architecture with a sophisticated plugin system.
+
+**Key Facts:**
+- ~500K lines of C++20 code (strict - no C++23)
+- Event-driven architecture using Continuation callbacks
+- Handles HTTP/1.1, HTTP/2, HTTP/3 (QUIC)
+- Supports TLS termination and caching
+- Extensible via C/C++ plugins
+
+## Code Style Requirements
+
+### C++ Style (Enforced by CI)
+
Review Comment:
The heading “C++ Style (Enforced by CI)” is broader than what the repo’s
automated checks can enforce (e.g., clang-format enforces formatting like
ColumnLimit/brace style and clang-tidy checks braces-around-statements, but
guidelines like declaration grouping/blank lines aren’t mechanically enforced).
Consider splitting this into “enforced” vs “recommended” to avoid implying CI
will fail on stylistic guidance that isn’t checked.
```suggestion
### C++ Style Guidelines
Some of these rules are enforced automatically by CI (via clang-format and
clang-tidy); others are recommended conventions that will not themselves cause
CI failures but should still be followed in code reviews.
```
##########
.github/copilot-instructions.md:
##########
@@ -0,0 +1,422 @@
+# Apache Traffic Server - GitHub Copilot Instructions
+
+## Repository Overview
+
+Apache Traffic Server (ATS) is a high-performance HTTP/HTTPS caching proxy
server written in C++20. It processes large-scale web traffic using an
event-driven, multi-threaded architecture with a sophisticated plugin system.
Review Comment:
This doc largely duplicates guidance already present in `.claude/CLAUDE.md`
(style, architecture, testing, etc.). Having two near-copies increases the risk
of them drifting and giving conflicting guidance. Consider extracting shared
content to a single canonical doc (or linking to the existing one) and keeping
this file Copilot-specific.
##########
.github/copilot-instructions.md:
##########
@@ -0,0 +1,422 @@
+# Apache Traffic Server - GitHub Copilot Instructions
+
+## Repository Overview
+
+Apache Traffic Server (ATS) is a high-performance HTTP/HTTPS caching proxy
server written in C++20. It processes large-scale web traffic using an
event-driven, multi-threaded architecture with a sophisticated plugin system.
+
+**Key Facts:**
+- ~500K lines of C++20 code (strict - no C++23)
+- Event-driven architecture using Continuation callbacks
+- Handles HTTP/1.1, HTTP/2, HTTP/3 (QUIC)
+- Supports TLS termination and caching
+- Extensible via C/C++ plugins
+
+## Code Style Requirements
+
+### C++ Style (Enforced by CI)
+
+**Formatting Rules:**
+- **Line length:** 132 characters maximum
+- **Indentation:** 2 spaces (never tabs)
+- **Braces:** Linux kernel style (opening brace on same line)
+```cpp
+if (condition) {
+ // code here
+}
+```
+- **Pointer/reference alignment:** Right side
+```cpp
+Type *ptr; // Correct
+Type &ref; // Correct
+Type* ptr; // Wrong
+```
+- **Empty line after declarations:**
+```cpp
+void function() {
+ int x = 5;
+ std::string name = "test";
+
+ // Empty line before first code statement
+ process_data(x, name);
+}
+```
+- **Always use braces** for if/while/for (no naked conditions):
+```cpp
+if (x > 0) { // Correct
+ foo();
+}
+
+if (x > 0) // Wrong - missing braces
+ foo();
+```
+- **Keep variable declarations together:**
+```cpp
+// Correct - declarations grouped together
+void function() {
+ int count = 0;
+ std::string name = "test";
+ auto *buffer = new_buffer();
+
+ // Empty line before first code statement
+ process_data(count, name, buffer);
+}
+
+// Wrong - declarations scattered
+void function() {
+ int count = 0;
+ process_count(count);
+ std::string name = "test"; // Don't scatter declarations
+}
+```
+
+**Naming Conventions:**
+- Classes: `CamelCase` → `HttpSM`, `NetVConnection`, `CacheProcessor`
+- Functions/variables: `snake_case` → `handle_request()`, `server_port`,
`cache_key`
+- Constants/macros: `UPPER_CASE` → `HTTP_STATUS_OK`, `MAX_BUFFER_SIZE`
+- Member variables: `snake_case` with no prefix → `connection_count`,
`buffer_size`
+
+**C++20 Patterns (Use These):**
+```cpp
+// GOOD - Modern C++20
+auto buffer = std::make_unique<MIOBuffer>(size);
+for (const auto &entry : container) {
+ if (auto *ptr = entry.get(); ptr != nullptr) {
+ process(ptr);
+ }
+}
+
+// AVOID - Legacy C-style
+MIOBuffer *buffer = (MIOBuffer*)malloc(sizeof(MIOBuffer));
+for (int i = 0; i < container.size(); i++) {
+ process(container[i]);
+}
+```
+
+**Memory Management:**
+- Use RAII and smart pointers (`std::unique_ptr`, `std::shared_ptr`)
+- Use `ats_malloc()`/`ats_free()` for large allocations (not `malloc`)
+- Use `IOBuffer` for network data (zero-copy design)
+- Never call `delete` manually - use RAII
+
+**What NOT to Use:**
+- ❌ C++23 features (code must compile with C++20)
+- ❌ Raw `new`/`delete` (use smart pointers)
+- ❌ `malloc`/`free` for large allocations (use `ats_malloc`)
+- ❌ Blocking operations in event threads
+- ❌ Creating threads manually (use async event system)
+
+### Comments and Documentation
+
+**Minimal comments philosophy:**
+- Only add comments where code isn't self-explanatory
+- **Don't** describe what the code does (the code already shows that)
+- **Do** explain *why* something is done if not obvious
+- Avoid stating the obvious
+
+```cpp
+// BAD - stating the obvious
+// Increment the counter
+counter++;
+
+// GOOD - explaining why
+// Skip the first element since it's always the sentinel value
+counter++;
+
+// BAD - describing what
+// Loop through all connections and close them
+for (auto &conn : connections) {
+ conn.close();
+}
+
+// GOOD - explaining why (if not obvious)
+// Must close connections before destroying the acceptor to avoid
use-after-free
+for (auto &conn : connections) {
+ conn.close();
+}
+```
+
+**When to add comments:**
+- Non-obvious algorithms or math
+- Workarounds for bugs in dependencies
+- Performance optimizations that reduce clarity
+- Security-critical sections
+- Complex state machine transitions
+
+**When NOT to add comments:**
+- Self-documenting code
+- Obvious operations
+- Function/variable names that explain themselves
+
+### Python Style
+
+- Python 3.11+ with type hints
+- 4-space indentation (never tabs)
+- Type annotations on all function signatures
+
+### License Headers
+
+**ALL new files** must start with Apache License 2.0 header:
+```cpp
+/** @file
Review Comment:
“ALL new files must start with Apache License 2.0 header” is too broad for
this repository (many non-source files like README/docs don’t start with the
full header, and these new `.md` instruction files don’t either). Consider
narrowing this to “new source/test code files” (e.g., `.cc/.h/.py`) or
describing the project’s actual documentation-file licensing convention.
##########
.github/instructions/HRW.instructions.md:
##########
@@ -0,0 +1,218 @@
+---
+applyTo:
+ - "plugins/header_rewrite/**/*"
+ - "tools/hrw4u/**/*"
+---
+
+# Header Rewrite Plugin and HRW4U Transpiler
+
+## Overview
+
+Two closely related components that must be kept in sync:
+
+1. **header_rewrite plugin** (`plugins/header_rewrite/`) - ATS plugin for
modifying HTTP headers
+2. **hrw4u transpiler** (`tools/hrw4u/`) - DSL compiler for generating
header_rewrite configurations
+
+## Critical Requirement: Feature Synchronization
+
+**Features added to either component may require corresponding changes in the
other.**
+
+### When to Update Both
+
+- **New operator in header_rewrite** → Add syntax and code generation in hrw4u
+- **New condition in header_rewrite** → Add parsing and symbols in hrw4u
+- **New variable/resource in header_rewrite** → Update hrw4u symbol tables and
types
+- **New hook in header_rewrite** → Add hook syntax in hrw4u
+- **New hrw4u syntax** → Ensure correct header_rewrite config generation
+
+### Bidirectional Compilation
+
+Both directions must work:
+- **hrw4u** (forward): HRW4U source → header_rewrite config
+- **u4wrh** (reverse): header_rewrite config → HRW4U source
+
+Round-trip test: `hrw4u example.hrw4u | u4wrh` should produce equivalent
output.
+
+## Header Rewrite Plugin
+
+### Architecture
+
+**Core files:**
+- `parser.cc/h` - Configuration syntax parser
+- `factory.cc/h` - Factory for operators and conditions
+- `operators.cc/h` - Header manipulation operations
+- `conditions.cc/h` - Conditional logic
+- `resources.cc/h` - Available variables (headers, IPs, etc.)
+- `statement.cc/h` - Rule statement abstraction
+- `ruleset.cc/h` - Rule collection and execution
+- `matcher.cc/h` - Pattern matching
+- `value.cc/h` - Value extraction and manipulation
+
+### Adding Features
+
+**New operator:**
+1. Define class in `operators.h`, implement in `operators.cc`
+2. Register in `factory.cc`
+3. Update hrw4u: `generators.py` for code gen, `hrw_symbols.py` for symbols
+
+**New condition:**
+1. Define class in `conditions.h`, implement in `conditions.cc`
+2. Register in `factory.cc`
+3. Update hrw4u: `hrw_visitor.py` for parsing, `hrw_symbols.py` for symbols
+
+**New resource/variable:**
+1. Define in `resources.h`, implement in `resources.cc`
+2. Update hrw4u: `types.py` for type system, `symbols.py` for symbol tables
+
+## HRW4U Transpiler
+
+### Purpose
+
+Provides readable DSL syntax that compiles to header_rewrite configuration.
+
+**Requirements:** Python 3.11+, ANTLR4
+
+### Project Structure
+
+```
+tools/hrw4u/
+├── src/ # Python source
+│ ├── common.py # Shared utilities
+│ ├── types.py # Type system
+│ ├── symbols.py # Symbol resolution
+│ ├── hrw_symbols.py # Header rewrite symbols
+│ ├── tables.py # Symbol/type tables
+│ ├── hrw_visitor.py # Forward compiler
+│ ├── visitor.py # Reverse compiler
+│ ├── generators.py # Code generation
+│ ├── validation.py # Semantic validation
Review Comment:
The hrw4u module roles are mislabeled here: `tools/hrw4u/src/visitor.py`
contains `HRW4UVisitor` (used by the `hrw4u` script for forward compilation),
while `tools/hrw4u/src/hrw_visitor.py` defines `HRWInverseVisitor` (used by
`u4wrh` for reverse compilation). Also, `tools/hrw4u/src/generators.py`
currently generates reverse-resolution tables, not HRW config output. Please
update this section to match the actual file responsibilities to avoid sending
contributors to the wrong modules.
##########
.github/instructions/HRW.instructions.md:
##########
@@ -0,0 +1,218 @@
+---
+applyTo:
+ - "plugins/header_rewrite/**/*"
+ - "tools/hrw4u/**/*"
+---
+
+# Header Rewrite Plugin and HRW4U Transpiler
+
+## Overview
+
+Two closely related components that must be kept in sync:
+
+1. **header_rewrite plugin** (`plugins/header_rewrite/`) - ATS plugin for
modifying HTTP headers
+2. **hrw4u transpiler** (`tools/hrw4u/`) - DSL compiler for generating
header_rewrite configurations
+
+## Critical Requirement: Feature Synchronization
+
+**Features added to either component may require corresponding changes in the
other.**
+
+### When to Update Both
+
+- **New operator in header_rewrite** → Add syntax and code generation in hrw4u
+- **New condition in header_rewrite** → Add parsing and symbols in hrw4u
+- **New variable/resource in header_rewrite** → Update hrw4u symbol tables and
types
+- **New hook in header_rewrite** → Add hook syntax in hrw4u
+- **New hrw4u syntax** → Ensure correct header_rewrite config generation
+
+### Bidirectional Compilation
+
+Both directions must work:
+- **hrw4u** (forward): HRW4U source → header_rewrite config
+- **u4wrh** (reverse): header_rewrite config → HRW4U source
+
+Round-trip test: `hrw4u example.hrw4u | u4wrh` should produce equivalent
output.
+
+## Header Rewrite Plugin
+
+### Architecture
+
+**Core files:**
+- `parser.cc/h` - Configuration syntax parser
+- `factory.cc/h` - Factory for operators and conditions
+- `operators.cc/h` - Header manipulation operations
+- `conditions.cc/h` - Conditional logic
+- `resources.cc/h` - Available variables (headers, IPs, etc.)
+- `statement.cc/h` - Rule statement abstraction
+- `ruleset.cc/h` - Rule collection and execution
+- `matcher.cc/h` - Pattern matching
+- `value.cc/h` - Value extraction and manipulation
+
+### Adding Features
+
+**New operator:**
+1. Define class in `operators.h`, implement in `operators.cc`
+2. Register in `factory.cc`
+3. Update hrw4u: `generators.py` for code gen, `hrw_symbols.py` for symbols
+
+**New condition:**
+1. Define class in `conditions.h`, implement in `conditions.cc`
+2. Register in `factory.cc`
+3. Update hrw4u: `hrw_visitor.py` for parsing, `hrw_symbols.py` for symbols
+
+**New resource/variable:**
+1. Define in `resources.h`, implement in `resources.cc`
+2. Update hrw4u: `types.py` for type system, `symbols.py` for symbol tables
Review Comment:
For new header_rewrite resources/variables, the hrw4u “symbol tables” are
primarily defined in `tools/hrw4u/src/tables.py`
(OPERATOR_MAP/CONDITION_MAP/etc.). `tools/hrw4u/src/symbols.py` is the resolver
implementation rather than the table definition. Consider updating this pointer
so readers know where to add the actual mappings.
```suggestion
2. Update hrw4u: `types.py` for type system, `tables.py`
(OPERATOR_MAP/CONDITION_MAP/etc.) for symbol tables, and `symbols.py` for
resolver wiring
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
