fix: omit the synthesized initializer for default-constructed variables#1239
Conversation
|
| Scope | Lines Δ% | Lines Δ | Lines + | Lines - | Files Δ | Files + | Files ~ | Files ↔ | Files - |
|---|---|---|---|---|---|---|---|---|---|
| 🛠️ Source | 54% | 468 | 267 | 201 | 11 | 1 | 10 | - | - |
| 🥇 Golden Tests | 30% | 254 | 235 | 19 | 7 | 2 | 5 | - | - |
| 🔧 Toolchain Tests | 9% | 80 | 54 | 26 | 3 | - | 3 | - | - |
| 🔧 Toolchain | 4% | 33 | 23 | 10 | 2 | - | 2 | - | - |
| ⚙️ CI | 1% | 12 | 6 | 6 | 2 | - | 2 | - | - |
| 🧪 Unit Tests | 1% | 6 | 3 | 3 | 1 | - | 1 | - | - |
| 🤝 Third-party | 1% | 6 | 3 | 3 | 1 | - | 1 | - | - |
| Total | 100% | 859 | 591 | 268 | 27 | 3 | 24 | - | - |
Legend: Files + (added), Files ~ (modified), Files ↔ (renamed), Files - (removed)
🔝 Top Files
- src/lib/MrDocsCompilationDatabase.cpp (Source): 274 lines Δ (+137 / -137)
- test-files/golden-tests/symbols/variable/default-constructed-instance.xml (Golden Tests): 203 lines Δ (+203 / -0)
- src/lib/AST/ClangHelpers.cpp (Source): 102 lines Δ (+75 / -27)
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1239 +/- ##
========================================
Coverage 83.16% 83.16%
========================================
Files 35 35
Lines 3658 3660 +2
Branches 843 844 +1
========================================
+ Hits 3042 3044 +2
Misses 409 409
Partials 207 207
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
|
An automated preview of the documentation is available at https://1239.mrdocs.prtest2.cppalliance.org/index.html If more commits are pushed to the pull request, the docs will rebuild at the same URL. 2026-07-24 04:28:40 UTC |
c0780e7 to
9a18c3e
Compare
The new pin includes llvm/llvm-project#198452, which attaches documentation comments to macro definitions. That is the prerequisite for reading them from Clang directly instead of scanning the source, as we were initially doing in PR #1192. The pin was about six months behind, so this also adapts MrDocs to the Clang API changes across that range: - USRGeneration.h moved from clang/Index/ to clang/UnifiedSymbolResolution/; the `clang::index` namespace and signatures are unchanged. - `clang::DiagnosticConsumer::finish()` was removed, so the collecting diagnostic consumer relies on the forwarded `EndSourceFile()` instead. - The driver option table moved to clang/Options/Options.h, with its enumerators now in `clang::options` and `getDriverOptTable()` in `clang`. - `cl::getRegisteredOptions()` now returns a `DenseMap`, so its entries expose `.second` instead of `getValue()`. - `SubstituteConstraintExpressionWithoutSatisfaction` was re-synced with Clang's current implementation. Parsing the newer libc++ also needs a new vcruntime_new.h stub (its align_val_t.h now includes it under the vcruntime ABI) and C++ guards so stdbool.h, stdalign.h, and threads.h stop redefining some C++ keywords as macros. The regenerated goldens change only because Clang's output drifted over the six months: constrained templates and destructors get different symbol IDs from USR generation, though their structure is identical (requires-clause, dtor-overloads), and a few declaration locations changed.
The MSan and ASan jobs build MrDocs against a sanitizer-instrumented libc++. MSan requires that, since it reports false positives on any uninstrumented code; ASan doesn't require it, but is more effective with it, through libc++'s container-overflow annotations. The pinned libc++ contains a self-guard (added upstream in llvm/llvm-project#168955) that errors out when an ASan-instrumented libc++ is parsed without ASan, and MrDocs parses libc++ with normal flags for the libcxx.cpp golden. So an instrumented libc++ can no longer double as the one MrDocs reads. Thus, install both an instrumented and an uninstrumented libc++. Specifically, install the instrumented libc++ into a prefix separate from the main LLVM build's plain libc++; `needs_libcxx_runtimes` still selects it for Clang + ASan/MSan. MrDocs links the instrumented library through `libcxx_runtime_flags` but parses the plain headers the main build leaves at the LLVM install prefix. Also, move Clang ASan and MSan to the latest Clang, since building the pinned libc++ from source needs a recent one.
The coverage step globs build/ for *.profraw, which after the bump also holds the extracted LLVM source tree. That tree ships llvm-profdata test fixtures in an older raw-profile version that llvm-profdata rejects, failing the merge. Delete build/third-party profraw first; MrDocs' own profraw live elsewhere under build/.
9a18c3e to
4cc3c81
Compare
|
It looks good to me. The only thing I don't like is the comment in the AST visitor. The comment is huge, and it's obviously AI slop because it's huge and it contains very little information. No human would go into this level of detail because none of the details provided there are relevant in the context. It's very easy to describe what this function does in this AST context with less than one line of code. Still, AI always expands on things way more than we need to. If we keep doing that, the documentation becomes very incoherent because AI is only focused on this change, but the rest of the file doesn't follow this structure. Anyone reading the file without looking at this specific comment will stop here and think this change is way more important or tricky than it is. I mean, anyone, including another AI agent, could be confused by that. In a way, I'm kind of doing the same here, but the only reason I'm talking about this so much is that it's a recurring comment I'm making on all the PRs lately. |
Default-constructed variables (`T t;`) were rendered with a spurious initializer (`= t`). That's because Clang represents them with an implicit construction expression located at the variable name; so the extracted initializer text was the variable name itself. Therefore, skip the initializer when it is an implicit default construction. Closes #1238.
4cc3c81 to
c9b41aa
Compare
|
Oh, actually the comment was meant to explain what, for me, was indeed a tricky point: the fact that a variable with no written initializer still gets an implicit initializer in the AST which is located at the variable name. It probably isn't so tricky for someone more used to how Clang represents things. |
|
Oh. OK. Fair enough. Thanks. |
3 participants
Default-constructed variables (
T t;) were rendered with a spurious initializer (= t). That's because Clang represents them with an implicit construction expression located at the variable name; so the extracted initializer text was the variable name itself.Therefore, skip the initializer when it is an implicit default construction.
Closes #1238.
Changes
Testing
A new golden test, with XML output, tests a handful of variable declarations.
Documentation
No documentation needed. This is a bug fix.