Fix for handling CapturedDecl, which was causing the failure of certain #979
Conversation
OpenMP test case after upgrade.
clang/lib/AST/CanonBounds.cpp
Outdated
| Result | ||
| Lexicographic::CompareLoc(const SourceLocation *SL1, | ||
| const SourceLocation *SL2) const { | ||
| if (SL1 && SL2) { |
There was a problem hiding this comment.
If SL1 or SL2 is null, you can early exit by moving the llvm_unreachable from line 146 to here.
There was a problem hiding this comment.
Yes, I can do that.
clang/lib/AST/CanonBounds.cpp
Outdated
| @@ -137,6 +179,20 @@ Lexicographic::CompareScope(const DeclContext *DC1, const DeclContext *DC2) cons | |||
| } | |||
| } | |||
|
|
|||
| Result | |||
| Lexicographic::CompareDecl(const CapturedDecl *CD1, | |||
There was a problem hiding this comment.
Should this function be called CompareCapturedDecl? Because it won't accept any other Decl.
There was a problem hiding this comment.
There is a CompareDecl function on line 197 that takes two NamedDecls as arguments. So, I named this new function in the same way to indicate its "overloaded" nature.
clang/lib/AST/CanonBounds.cpp
Outdated
| Stmt *SList1 = CD1->getBody(); | ||
| Stmt *SList2 = CD2->getBody(); | ||
| if (SList1 && SList2) { | ||
| const SourceLocation SL1 = SList1->getSourceRange().getBegin(); |
There was a problem hiding this comment.
As far as I have investigated, it doesn't return null.
| const CapturedDecl *CD2Arg) const { | ||
| const CapturedDecl *CD1 = dyn_cast<CapturedDecl>(CD1Arg->getCanonicalDecl()); | ||
| const CapturedDecl *CD2 = dyn_cast<CapturedDecl>(CD2Arg->getCanonicalDecl()); | ||
| if (CD1 == CD2) |
There was a problem hiding this comment.
Do we want to return Result::Equal even if both CD1 and CD2 are null? If not, then the null checks from line 190 should move here.
There was a problem hiding this comment.
This is a fair question. For the sake of uniformity, I have written the code to execute in the same way that the other overloaded function CompareDecl on line 206 (the function that compares two NamedDecls) executes.
before calling CompareLoc.
clang/lib/AST/CanonBounds.cpp
Outdated
|
|
||
| if (!SL1.isInvalid() && !SL2.isInvalid()) | ||
| return CompareLoc(&SL1, &SL2); | ||
| else { |
clang/lib/AST/CanonBounds.cpp
Outdated
| assert(false && "unexpected invalid source location(s)"); | ||
| if (!SL2.isInvalid()) | ||
| return Result::LessThan; | ||
| else if (!SL1.isInvalid()) |
clang/lib/AST/CanonBounds.cpp
Outdated
| return Result::LessThan; | ||
| else if (!SL1.isInvalid()) | ||
| return Result::GreaterThan; | ||
| else { |
clang/lib/AST/CanonBounds.cpp
Outdated
| // to the statement lists. | ||
| if (SList1 == SList2) | ||
| return Result::Equal; | ||
| else if (SList1 < SList2) |
…wo CapturedDecls. An interim solution using pointer comparison is present, with a TODO for an improved ordering.
OpenMP test cases after upgrade.