Fix issue 20012 - clear diagnostic for CTFE array cast in UDA context - #23820
Fix issue 20012 - clear diagnostic for CTFE array cast in UDA context#23820usefahmed07 wants to merge 19 commits into
Conversation
When an array cast that changes element size (e.g. cast(dstring) from a string literal) appeared in a context requiring compile-time evaluation but going through the codegen-lowering path (such as a UDA), the compiler rewrote it to a call to object.__ArrayCast!(...). CTFE then had to interpret that call's body, including onArrayCastError()'s use of pureMalloc for building the error message - which cannot be evaluated at compile time. This surfaced a confusing, unrelated error (e.g. `fakePureErrno` cannot be interpreted at compile time) instead of a clear message, unlike the equivalent enum case which hits dinterpret.d's CTFE-aware array cast check directly and gives a clear message. Added a __ctfe branch in onArrayCastError() (druntime) that builds the same error message using plain string concatenation instead of pureMalloc, since that's evaluable during CTFE. The non-CTFE path is untouched. Added test/fail_compilation/fail20012.d covering the original UDA repro from the issue. Verified casting.d's existing unittests (covering the ordinary runtime path) still pass, and the full fail_compilation/compilable suites pass with no regressions.
5441c48 to
7402c95
Compare
|
no, |
dkorpel
left a comment
There was a problem hiding this comment.
This is still printing multiple redundant messages
The CTFE branch of onArrayCastError used ~ and .idup, which are heap-allocating operations. Since __ArrayCast is @nogc, the D compiler's attribute inference marked onArrayCastError as non-@nogc based on this branch, breaking the @nogc call from __ArrayCast and producing a confusing secondary 'CTFE failed because of previous errors' message alongside the real one. Build the CTFE error message on a stack-allocated char[2048] buffer instead, with no ~ or .idup, so attribute inference keeps the function @nogc. This also fixes the same issue under -betterC. Fixes https://issues.dlang.org/show_bug.cgi?id=22546
The expected error message hardcodes '4LU' for the toElemSize argument printed by the compiler's call-site diagnostics. This literal suffix reflects size_t being ulong on 64-bit platforms; on 32-bit platforms size_t is uint, so the compiler prints '4u' instead, causing a text mismatch. Disable the test on 32-bit linux/windows targets rather than attempt to match both formats, consistent with existing tests like chkformat_clong_smalllong.d that hit the same size_t size-dependent formatting issue.
Adding the DISABLED: linux32 win32 line shifted the source code down by one line, so the expected error line numbers (10, 11) no longer matched the actual output (11, 12). Update the expected TEST_OUTPUT line numbers accordingly.
Previous CI failure was an unrelated apt-get/Google Chrome mirror hash mismatch during runner setup, unrelated to this change.
Previous CI failure was an unrelated apt-get/Google Chrome mirror hash mismatch during runner setup, unrelated to this change.
Previous CI failure was an unrelated apt-get/Google Chrome mirror hash mismatch during runner setup, unrelated to this change.
Previous CI failure was an unrelated apt-get/Google Chrome mirror hash mismatch during runner setup, unrelated to this change.
Previous CI failure was an unrelated Google Chrome apt mirror hash mismatch during runner setup, unrelated to this change.
Previous CI failure was an unrelated Google Chrome apt mirror hash mismatch during runner setup, unrelated to this change.
|
@thewilsonator Looking good now — btw the buildkite/dmd CI (which is the required check) was failing on all previous PRs, but it's passing now. |
|
My comment has not been addressed yet |
|
This adds one branch There are two errors given, because each line of the test case is an error. What do you mean that "This is still printing multiple redundant messages"? |
|
The expected error message "Error: array cast from |
UserAttributeDeclaration.semantic evaluated attribute expressions with a normal (non-CTFE) scope, unlike enum initializers which use sc.startCTFE(). This caused array casts inside UDA context to be lowered to a runtime __ArrayCast call instead of being evaluated directly by the CTFE interpreter, producing a messy druntime error trace instead of the clean "array cast from ... is not supported at compile time" message. Wrap the UDA attribute semantic in startCTFE()/endCTFE(), matching the enum case, so CTFE handles the diagnostic directly.
The previous approach duplicated druntime code in onArrayCastError to print a CTFE-safe message, and tweaked the __ArrayCast lowering condition in expressionsem.d. Both are reverted here in favor of a minimal fix to UDA attribute semantic (see next commit).
The previous revert commit mistakenly used origin/master as the target, which also discarded three unrelated legitimate commits on this branch (issue 5010, issue 18586, and the AA array comparison fix) that touched the same files. This restores expressionsem.d and casting.d to the state right before the CTFE-array-cast duplication commits, keeping those legitimate fixes intact.
DMD perf checkNo differences above the noise thresholds. All measurements
2379886 vs merge-base 70145d6 · about these metrics |
|
@dkorpel @thewilsonator Pushed a new fix. Root cause: UDA attributes weren't evaluated in a CTFE scope like enum initializers are, so the cast got lowered to a runtime Fix: wrap UDA semantic in Now both enum and UDA cases print the same clean message, no druntime trace, no |
GitHub issue dlang#20012 (linked by dkorpel) confirms this was transferred from bugzilla bug 22546 by dlangBugzillaToGithub. The bugzilla page at issues.dlang.org/show_bug.cgi?id=20012 is an unrelated bug (extern(C) mangling in template mixins) using bugzilla's own numbering, which differs from GitHub's issue numbering after the migration. The original id=22546 link was correct all along.
Point directly to dlang#20012 (where this issue now lives) instead of the old bugzilla URL it was transferred from.
Fixes #20012