Syntax: correct implementation for removing
#19972
Conversation
|
CC: @bitjammer |
|
@swift-ci please test |
|
@rintaro - bleh, yes, I see your point now that I look at it again. The model that I had in mind was that of an erase from iterator as in the STL. Actually, how is this supposed to work? |
|
Yeah, this doesn't work right now. I think the easiest way is to make a temporary |
compnerd
force-pushed
the
removing-syntax
branch
2 times, most recently
from
8430271 to
1c97ab2
Compare
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@rintaro - there, this should work I believe. Not sure about the default size, let me know if that needs to be adjusted. |
| NewLayout.erase(NewLayout.begin() + i); | ||
| llvm::SmallVector<RC<RawSyntax>, 16> NewLayout = getRaw()->getLayout(); | ||
| auto iterator = NewLayout.begin(); | ||
| std::advance(iterator, i - 1); |
There was a problem hiding this comment.
I don't think i - 1 is correct. What if i == 0?
Also, could you add test cases in unittests/Syntax/SyntaxCollectionTests.cpp?
compnerd
force-pushed
the
removing-syntax
branch
from
1c97ab2 to
3b4919d
Compare
|
@swift-ci please test |
|
Build failed |
|
Build failed |
|
@swift-ci please test |
|
Build failed |
|
Build failed |
| ASSERT_EQ(List.size(), static_cast<size_t>(2)); | ||
|
|
||
| SmallString<48> Scratch; | ||
| llvm::raw_svector_stream OS(Scratch); |
There was a problem hiding this comment.
/home/buildnode/jenkins/workspace/swift-PR-Linux/branch-master/swift/unittests/Syntax/SyntaxCollectionTests.cpp:272:27: error: expected ';' after expression
23:37:50 llvm::raw_svector_stream OS(Scratch);
23:37:50 ^
23:37:50 ;
I guess:
| llvm::raw_svector_stream OS(Scratch); | |
| llvm::raw_svector_ostream OS(Scratch); |
`llvm::ArrayRef<T>` does not define `erase`. However, since the intent here is to remove the last n elements, we can use `drop_back` instead. Furthermore, replace the direct use of `Layout` with `getLayout()`. This was identified by gcc 8.2.
compnerd
force-pushed
the
removing-syntax
branch
from
3b4919d to
e07c00c
Compare
|
@swift-ci please test |
|
Build failed |
|
Build failed |
llvm::ArrayRef<T>does not defineerase. However, since the intenthere is to remove the last n elements, we can use
drop_backinstead.Furthermore, replace the direct use of
LayoutwithgetLayout().This was identified by gcc 8.2.
Replace this paragraph with a description of your changes and rationale. Provide links to external references/discussions if appropriate.
Resolves SR-NNNN.