Issue 24159 - BetterC: appending to dynamic arrays no longer errors at compile time
Summary: BetterC: appending to dynamic arrays no longer errors at compile time
Status: RESOLVED FIXED
Alias: None
Product: D
Classification: Unclassified
Component: dmd (show other issues)
Version: D2
Hardware: All All
: P1 regression
Assignee: No Owner
URL:
Keywords: betterC, pull
Depends on:
Blocks:
 
Reported: 2023-09-25 23:38 UTC by dave287091
Modified: 2023-11-20 11:13 UTC (History)
1 user (show)

See Also:


Attachments

Note You need to log in before you can comment on or make changes to this issue.
Description dave287091 2023-09-25 23:38:19 UTC
The following program compiled with betterC used to error at compile time, now it only errors at runtime:

extern(C)
int main(){
    int[] x = null;
    x ~= 3; // this should be diagnosed at compile time
    return x[0];
}

You now get a runtime assertion like:

Assertion failed: (Cannot append to array if compiling without support for runtime type information!), function core.internal.array.appending._d_arrayappendcTXImpl!(int[], int)._d_arrayappendcTX, file <snip>/import/core/internal/array/appending.d, line 52.

Which is this file: https://github.com/dlang/dmd/blob/396fc8d477ca66e20f6b4fec4d532f2aee053ba3/druntime/src/core/internal/array/appending.d#L52

Getting an error at runtime is much worse than compile time as some code might be rarely exercised.

Also note that the failed assertion doesn’t even point to your line of code where you used a feature banned in betterC!
Comment 1 Dlang Bot 2023-11-10 12:54:31 UTC
@teodutu updated dlang/dmd pull request #15791 "Fix Issue 21459: Store lowering of `CatAssignExp` in a separate field" fixing this issue:

- Fix Issue 24159: Store lowering of `CatAssignExp` in a separate field
  
  This preserves the `CatAssignExp` in the AST until the glue layer where
  an error is printed in case this expression is used with `-betterC`.
  This is required to happen in the glue layer as the semantic analysis
  doesn't correctly distinguish when code needs to be generated.
  
  Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

https://github.com/dlang/dmd/pull/15791
Comment 2 Dlang Bot 2023-11-13 11:06:10 UTC
dlang/dmd pull request #15791 "Fix Issue 21459: Store lowering of `CatAssignExp` in a separate field" was merged into stable:

- 71518bee8b2779a9cd6064537470582dae4c29c1 by Teodor Dutu:
  Fix Issue 24159: Store lowering of `CatAssignExp` in a separate field
  
  This preserves the `CatAssignExp` in the AST until the glue layer where
  an error is printed in case this expression is used with `-betterC`.
  This is required to happen in the glue layer as the semantic analysis
  doesn't correctly distinguish when code needs to be generated.
  
  Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

https://github.com/dlang/dmd/pull/15791
Comment 3 Dlang Bot 2023-11-20 11:13:25 UTC
dlang/dmd pull request #15829 "merge stable" was merged into master:

- b9f8e7cf24273f2283a39f703b2367c9cb09a0dc by Teodor Dutu:
  Fix Issue 24159: Store lowering of `CatAssignExp` in a separate field (#15791)
  
  This preserves the `CatAssignExp` in the AST until the glue layer where
  an error is printed in case this expression is used with `-betterC`.
  This is required to happen in the glue layer as the semantic analysis
  doesn't correctly distinguish when code needs to be generated.
  
  Signed-off-by: Teodor Dutu <teodor.dutu@gmail.com>

https://github.com/dlang/dmd/pull/15829