Skip to content

Compiler: move c2_assert to c2#306

Open
chqrlie wants to merge 1 commit intoc2lang:masterfrom
chqrlie:assert
Open

Compiler: move c2_assert to c2#306
chqrlie wants to merge 1 commit intoc2lang:masterfrom
chqrlie:assert

Conversation

@chqrlie
Copy link
Contributor

@chqrlie chqrlie commented Jun 27, 2025

  • compute the assert string and function call at parse time
  • add libc2 module in libs/libc/c2internals.c2i
  • add auto_func function parameter attribute
  • reject invalid combinations of auto_xxx attributes

@chqrlie chqrlie force-pushed the assert branch 2 times, most recently from ac7bab2 to f6f55a9 Compare June 27, 2025 20:11
@bvdberg
Copy link
Member

bvdberg commented Jun 28, 2025

It also has auto-arg changes.. please keep commits small so it's easier for me to review..

@chqrlie
Copy link
Contributor Author

chqrlie commented Jun 29, 2025

I just split this PR into 2 separate commits:

  • one for the new auto_func attribute and other attribute fixes
  • one for the c2_assert function migration to the library.


if (a.getCall()) {
qt = ma.analyseExpr(a.getCall2(), true, RHS);
if (qt.isInvalid()) return;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In which case will a.getCall() be nil?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the call expression is only constructed by the parser if asserts are enabled. The expression is always parsed and stored for the analyser.

@chqrlie chqrlie force-pushed the assert branch 6 times, most recently from e15d382 to 32c8780 Compare July 6, 2025 10:14
@chqrlie chqrlie force-pushed the assert branch 5 times, most recently from a483970 to 1fea052 Compare July 15, 2025 06:41
@chqrlie chqrlie force-pushed the assert branch 12 times, most recently from d7be4b3 to f5c7b25 Compare July 29, 2025 06:49
@chqrlie chqrlie force-pushed the assert branch 3 times, most recently from 55626d0 to e143bd3 Compare August 29, 2025 12:14
@chqrlie chqrlie force-pushed the assert branch 4 times, most recently from 83ea8af to 35d20f6 Compare September 3, 2025 08:54
@chqrlie chqrlie force-pushed the assert branch 3 times, most recently from 9be0d32 to 4010aa5 Compare September 14, 2025 20:28
@chqrlie chqrlie force-pushed the assert branch 2 times, most recently from 113f33a to 7481cb5 Compare September 28, 2025 15:44
@chqrlie chqrlie force-pushed the assert branch 6 times, most recently from 31a11a7 to 3c4cf82 Compare October 8, 2025 09:38
@chqrlie chqrlie force-pushed the assert branch 4 times, most recently from 23cd690 to 4b162e3 Compare October 18, 2025 20:11
@chqrlie chqrlie force-pushed the assert branch 3 times, most recently from a10471a to e9bc6a6 Compare October 20, 2025 20:54
@chqrlie chqrlie force-pushed the assert branch 2 times, most recently from 6a171b7 to 3fd9fc8 Compare November 2, 2025 14:44
@bvdberg
Copy link
Member

bvdberg commented Mar 14, 2026

I lookat the the PR again. It slows down parsing 16.1 -> 16.6 ms on avg on my system. Is there a reason we add the call during parsing? Otherwise it could follow the other AST changes and just store the basic info during parsing and only change the AST if asserts are enabled and the compiler is not in check-only mode. Then --check would be faster

@chqrlie
Copy link
Contributor Author

chqrlie commented Mar 14, 2026

Thanks for reviewing this again. Indeed I can delay constructing the call to the analyser and only if needed. I suspect the slowdown is due in large part to the conversion from SrcLoc to the location components, which was impacting the C source generation time only. Moving the construction to the analyser will impact the analysis time unless --check is selected. Improving the conversion method would accelerate the combined parsing + analysis + generation time.

* compute the assert string and function call at analysis time if !check_only
* add `c2_assert` module in C library interface `libs/libc/c2_assert.c2i`
* update tests
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants