Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use format strings directly in debug/error/log functions #218

Merged
merged 3 commits into from
Mar 8, 2022

Conversation

AltGr
Copy link
Contributor

@AltGr AltGr commented Mar 8, 2022

This avoids many intermediate calls to e.g. Format.asprintf; should result
in some cases in "more correct" use of Format¹, avoid the computation of
unused debug strings, and make the code more readable.

¹ for Format to work as expected, all intermediate calls need to go through
it. Some cases of formatting to an intermediate string then printing through
Format again are still present, but this makes the situation better.

This avoids many intermediate calls to e.g. `Format.asprintf`; should result in
some cases in "more correct" use of `Format`¹, avoid the computation of unused
debug strings, and make the code more readable.

¹ for `Format` to work as expected, all intermediate calls need to go through
it. Some cases of formatting to an intermediate string then printing through Format
again are still present, but this makes the situation better.
@AltGr
Copy link
Contributor Author

AltGr commented Mar 8, 2022

(note: this should be reviewed and merged if fit with high priority due to the high potential for merge conflicts :) )

@EmileRolley EmileRolley added ✨ enhancement New feature or request 🏗️ build system Build system or Makefile 🔧 compiler Issue concerns the compiler labels Mar 8, 2022
Copy link
Contributor

@denismerigoux denismerigoux left a comment

Choose a reason for hiding this comment

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

Thank you so much! it pained me indeed to have to use Format.asprintf all the time....

compiler/dcalc/interpreter.ml Outdated Show resolved Hide resolved
@denismerigoux denismerigoux merged commit 5a186f8 into CatalaLang:master Mar 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
🏗️ build system Build system or Makefile 🔧 compiler Issue concerns the compiler ✨ enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants