Bounds context [1/n] #807
Bounds context [1/n] #807
Conversation
…ing declared bounds in a statement
…ter checking statements
| OrderedDecls.push_back(Pair.first); | ||
| llvm::sort(OrderedDecls.begin(), OrderedDecls.end(), | ||
| [] (DeclaratorDecl *A, DeclaratorDecl *B) { | ||
| if (A->getNameAsString() == B->getNameAsString()) |
There was a problem hiding this comment.
You could write this comparator more succinctly as:
return std::tie(A->getNameAsString(), A->getLocation()) < std::tie(B->getNameAsString(), B->getLocation())
There was a problem hiding this comment.
After I made this change, there were build warnings related to A->getNameAsString(), etc. not being lvalues so I've reverted it at least for now.
There was a problem hiding this comment.
Overall, looks good. I had one minor code suggestion.
Could you add comments explaining the general approach that is being used? As we discussed off-line, the goal of attaching contexts and propagating them around is that we can determine what variables with bounds declarations are in scope? Also, what is the effect on compile time of this change?
clang/lib/Sema/SemaBounds.cpp
Outdated
| @@ -2867,7 +2958,10 @@ namespace { | |||
| BoundsExpr *B = nullptr; | |||
| InteropTypeExpr *IT = nullptr; | |||
| if (VD) { | |||
| B = VD->getBoundsExpr(); | |||
| if (State.UC[VD]) | |||
There was a problem hiding this comment.
Should use the find method instead of the subscript operator? This avoids two lookups in the DenseMap.
clang/lib/Sema/SemaBounds.cpp
Outdated
| @@ -2958,8 +2961,9 @@ namespace { | |||
| BoundsExpr *B = nullptr; | |||
| InteropTypeExpr *IT = nullptr; | |||
| if (VD) { | |||
| if (State.UC[VD]) | |||
| B = State.UC[VD]; | |||
| auto Declared = State.UC.find(VD); | |||
There was a problem hiding this comment.
I would do it this way:
if (State.UC.count(VD))
B = State.UC[VD];
This change resulted in the following compile time deltas compared to the current master:
|
This PR adds a bounds context member to the CheckingState class and determines the incoming (declared) bounds context for a statement. After #802 is merged, the bounds context can be updated during bounds checking.
Testing: