Fix incomplete-type errors when building GoogleSQL with C++23
- Dominant language
- C++
- Stars
- 2.6k
- Forks
- 260
- PR merge metrics
- No merged PRs in 30d
Description
Please incorporate the incomplete-type ownership fixes proposed in [PR #175](https://github.com/google/googlesql/pull/175) so GoogleSQL can be built by C++23 consumers without carrying a downstream source patch. This requests compatibility with a consumer-selected C++23 toolchain, not a change to GoogleSQL's C++20 default.
## Why this is needed
The Cloud Spanner emulator encountered C++23 compilation failures where `std::unique_ptr` destruction or assignment is instantiated before the owned type is complete. [cloud-spanner-emulator#375](https://github.com/GoogleCloudPlatform/cloud-spanner-emulator/pull/375) carries the GoogleSQL fixes as a downstream patch. Incorporating the fixes would let the emulator drop its downstream patch after updating to a GoogleSQL release containing them, and let other consumers use the same fixes when adopting C++23.
The affected definitions remain in GoogleSQL master `289c4bc6cf6e134129a032ccb2e4f849e000279c`:
- Generated AST constructors, node-owning mutators, and interleaved factory functions can require node types defined later in the generated header.
- `DotStarSourceExprInfo::lateral_reference_state` has a redundant `= nullptr` initializer while `LateralReferenceState` is forward-declared.
- `RewriterVisitor`'s constructor precedes `WithEntryRewriteState`, and `SQLBuilder`'s constructor/destructor precede `SQLBuilder::AnalyticFunctionInfo`.
- Reference-implementation argument classes own `ValueExpr` and `RelationalOp` before those classes are defined.
## Requested change
Move the operations that require complete types after the corresponding type definitions, and generate AST constructors and node-owning mutators in `resolved_ast.cc`. Preserve ownership, initialization, public signatures, inline builder methods, and the C++20 default. The linked PR applies the existing emulator patch directly to the seven GoogleSQL source/template files.
## Validation
Regenerated all 12 C++ AST outputs before and after the change. The builder and visitor/node-kind outputs are byte-identical. Checked matching declarations and unchanged bodies for all moved generated constructors and mutators, and verified that factories follow all node definitions. C++ compilation and runtime tests were not run.
I understand GoogleSQL does not accept external code contributions; the PR is a reference implementation for maintainers to incorporate through the internal development process.
Contributor guide
Research direction
Start by reviewing PR #175 and the affected generated AST definitions, forward-declared ownership sites, RewriterVisitor, SQLBuilder, and reference-implementation argument classes. Regenerate all 12 C++ AST outputs and compare the results against the stated unchanged builder and visitor/node-kind outputs. Done means the seven source/template files incorporate the fixes while preserving ownership, initialization, public signatures, inline builder methods, and the C++20 default.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- cpp
- Domain
- build-system, compilers
- Issue type
- Bug
- Difficulty
- 5/5
- Estimated time
- Over a week
- Activity status
- Active
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100