-
Notifications
You must be signed in to change notification settings - Fork 123
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 LLVM17 support. #675
Add LLVM17 support. #675
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #675 +/- ##
==========================================
+ Coverage 94.37% 94.39% +0.01%
==========================================
Files 48 48
Lines 7062 7087 +25
==========================================
+ Hits 6665 6690 +25
Misses 397 397
|
936dcd2
to
8be26fd
Compare
test/FirstDerivative/Loops.C
Outdated
@@ -378,7 +378,7 @@ double fn10_darg0(double x, size_t n); | |||
// CHECK-NEXT: { | |||
// CHECK-NEXT: size_t _d_count = 0; | |||
// CHECK-NEXT: size_t _d_max_count = _d_n; | |||
// CHECK-NEXT: for (size_t count = 0; max_count; ++count) { | |||
// CHECK-NEXT: for (size_t count = 0; size_t max_count = n; ++count) { |
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.
@PetroZarytskyi, can you take a look at this change - I believe it was broken before that patch...
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.
@vgvassilev Yes. I think this was a bug in clang with dumping for-loop condition inits. It had to do with the fact that technically in such cases the condition is max_count
and size_t max_count = n
is the condition init.
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.
Ok, I've fixed it by making it conditional in the test...
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.
clang-tidy made some suggestions
8be26fd
to
25e2f3f
Compare
This patch replaces the artificial handling of the scopes and accesses the private CurScope member of Sema. This allows us to build precise lexical scope setup for when building AST nodes. That's essential for lambda support in clang17 because it tightly couples both Scopes and DeclContexts. The change in test/FirstDerivative/Loops.C is because the pretty printer did not print correctly the variables defined in the condition of the loop until clang17. Fixes #669
25e2f3f
to
7faee43
Compare
clang-tidy review says "All clean, LGTM! 👍" |
clang-tidy review says "All clean, LGTM! 👍" |
Fixes #669