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

Add solutions for advent of code 2024 day 1 to examples/. #4673

Open
wants to merge 16 commits into
base: trunk
Choose a base branch
from

Conversation

zygoloid
Copy link
Contributor

@zygoloid zygoloid commented Dec 12, 2024

In order to support these examples, this adds two new builtins to the toolchain: print.char and read.char, which map to the libc functions putchar and getchar.

- Remove repeated definitions of `i32` operations that were causing
  duplicate name errors.
- Build object files for the prelude and link against them when building
  `carbon_binary` rules.
Set their parent as the current package and mark them as being closed so
that we can later recognize them as packages.

Fixes pretty-printing of an indirectly imported `Core.Int`.
This adds two new builtins to the toolchain: `print.char` and
`read.char`, which map to the libc functions `putchar` and `getchar`.

Also fixes a bug where global variables would create LLVM variable
declarations, not definitions, resulting in link errors.
@zygoloid zygoloid added the dependent Depends on another issue/PR label Dec 12, 2024
@zygoloid
Copy link
Contributor Author

Depends on #4644.

@github-actions github-actions bot added documentation An issue or proposed change to our documentation infrastructure toolchain labels Dec 12, 2024
@github-actions github-actions bot requested a review from jonmeow December 12, 2024 20:51
var EOF: i32 = -(1 as i32);
var push_back: i32 = EOF;

fn ReadCharRaw() -> i32 = "read.char";
Copy link
Contributor

Choose a reason for hiding this comment

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

Can these be moved out of examples/ and into a library?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

toolchain/check/eval.cpp Outdated Show resolved Hide resolved
toolchain/sem_ir/builtin_function_kind.def Outdated Show resolved Hide resolved
return from - 1;
}

fn Quicksort(p: [i32; 1000]*, from: i32, to: i32) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Had you considered moving Quicksort out to a library?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I had, but import_ref can't import array types yet, so this is blocked on that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

#4675 for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

And now moved out into a library.

toolchain/lower/handle_call.cpp Outdated Show resolved Hide resolved
@@ -0,0 +1,142 @@
// Part of the Carbon Language project, under the Apache License v2.0 with LLVM
Copy link
Contributor

Choose a reason for hiding this comment

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

These files don't really have much in terms of documentation. Would it be worth at least adding a README.md explaining what you're doing here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done.

Copy link
Contributor

@jonmeow jonmeow left a comment

Choose a reason for hiding this comment

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

Regarding the return type question you just asked -- consider me now neutral, the narrow delta of this change doesn't seem that bad to review. But, maybe fixing global variables should be in the PR title?

@zygoloid
Copy link
Contributor Author

Split the bugfix out to #4674.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependent Depends on another issue/PR documentation An issue or proposed change to our documentation infrastructure toolchain
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants