-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Update LLVM #6182
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
base: trunk
Are you sure you want to change the base?
Update LLVM #6182
Conversation
// 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_); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
Some pretty interesting errors in clangd-tidy though... |
Yes, I think that's a bug in the action. |
Tested locally in relation to #6182, difficult to test server-side because of the action change combined with the edge-case issue.
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.