Skip to content

Conversation

jonmeow
Copy link
Contributor

@jonmeow jonmeow commented Oct 8, 2025

This is in particular to pick up fixes in llvm/llvm-project#160804 (so that a more recent LLVM compiler can be used with Carbon), but also addresses a few deprecations.

@jonmeow jonmeow requested a review from a team as a code owner October 8, 2025 22:51
@jonmeow jonmeow requested review from danakj and removed request for a team October 8, 2025 22:51
@dwblaikie dwblaikie enabled auto-merge October 8, 2025 23:16
// discard the pointer without destroying or deallocating it.
auto clang_instance = std::make_unique<clang::CompilerInstance>(
std::move(invocation), std::move(pch_ops));
clang_instance->setVirtualFileSystem(fs_);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Can this go down on L329 (just before createDiagnostics()) to continue making this function match with the one in cc1_main that it's mirroring?

https://github.com/llvm/llvm-project/blob/0c2e900074a4a412afdbee5e56b1a513ea97b90f/clang/tools/driver/cc1_main.cpp#L274-L278

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmmmm, interesting point, I hadn't been aware this was a copy of LLVM code since there's no separate license notice.

Moving this PR to draft until we can discuss it with @chandlerc

Copy link
Contributor

@danakj danakj left a comment

Choose a reason for hiding this comment

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

LGTM

@danakj
Copy link
Contributor

danakj commented Oct 9, 2025

Some pretty interesting errors in clangd-tidy though...

@jonmeow
Copy link
Contributor Author

jonmeow commented Oct 9, 2025

Some pretty interesting errors in clangd-tidy though...

Yes, I think that's a bug in the action.

@jonmeow jonmeow disabled auto-merge October 9, 2025 20:02
@jonmeow jonmeow marked this pull request as draft October 9, 2025 20:02
github-merge-queue bot pushed a commit that referenced this pull request Oct 9, 2025
Tested locally in relation to
#6182, difficult to
test server-side because of the action change combined with the
edge-case issue.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants