[SR-13950] Replace uses of find(x) != end() idiom with contains(x) for Set-like types #56347
Comments
|
Feel free to ask questions here and/or ping me for review on GitHub (@ varungandhi-apple on GitHub). |
|
@swift-ci create |
|
Comment by Minhyuk Kim (JIRA) Hi theindigamer (JIRA User), I'm trying to take on this issue, and I have a question. 🙂 I was able to change |
|
For this bug report, I think it's fine if we address things for sets only, since that has already landed in LLVM and we have that available on LLVM's The "problem" with DenseMap is that LLVM has many types for hashtables (as it does for sets) and it would be nice to fix all of those at once. If you are going to submit that change to LLVM, and are not sure about the review process/landing changes, you can ask David Blaikie (dblaikie on Phabricator) for review and guidance. Once changes have landed in LLVM and are available on the swift/main branch, we can file a separate task to update Swift; I don't think it is worth waiting on that to land to not update usages for set-like types. Moreover, the larger the Swift-side PR becomes, the more likely you are to run into merge conflicts, which will get tedious quickly. |
|
Comment by Minhyuk Kim (JIRA) Thanks for the further explanation! 🙂 I went ahead and changed all other usages(changes to llvm could come later i guess) of `find(t) == .end()` to `contains(t)` with the changes from the c+20 to set data-types. But I found that swift's default c+ std for build is c+14, and the build doesn't run unless I change it to c20 in the cmake file. Is it okay for me to commit as-is, or are there any other configurations for using c+20 that I may be missing? |
|
We can't bump the version to C++20 right now. This bug report applies only to uses of LLVM's Set-like data types, not for the standard library's set/unordered_set. Also, note that the polarity is the other way: so |
|
PR merged. #35229 |
Additional Detail from JIRA
md5: 361ae0b10fe3e9e35324139c472df244
Issue Description:
A little while back, LLVM Set-like data types got
contains(t)methods which make code more readable. Many places in the compiler use thefind(a) != .end()idiom to check for membership (and in some placesfind(a) == .end()to check if the item is not present).It would be nice to update the code to use
contains(a)to improve the readability.It's probably easier to manually audit the code for places calling
find(...)andend(). That said, if you want to have extra fun, you could try creating a clang-tidy check which automates this. There are various YouTube tutorials and blog posts on clang-tidy, and it has a "REPL" clang-query which can be used to interactively explore the AST.Then you can run clang-tidy on the codebase. See
docs/DebuggingTheCompiler.mdon how to do that.This change shouldn't require any additional tests.
The text was updated successfully, but these errors were encountered: