[PATCH] D41732: [Option] Add 'findNearest' method to catch typos

Jonathan Roelofs via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Jan 4 12:19:46 PST 2018


jroelofs added inline comments.


================
Comment at: lib/Option/OptTable.cpp:258
+  unsigned BestDistance = UINT_MAX;
+  for (size_t I = FirstSearchableIndex, E = OptionInfos.size(); I < E; I++) {
+    const Info &CandidateInfo = OptionInfos[I];
----------------
I think you can use range-for via:

```
for (const Info &CandidateInfo : ArrayRef(OptionInfos).drop_front(FirstSearchableIndex))
```


================
Comment at: lib/Option/OptTable.cpp:260
+    const Info &CandidateInfo = OptionInfos[I];
+    StringRef CandidateName = StringRef(CandidateInfo.Name);
+
----------------
The ctor call here isn't needed, it's implicit in the assign if you drop it.


================
Comment at: lib/Option/OptTable.cpp:293
+    if (Last == '=' || Last == ':')
+      Delimiter = StringRef(std::string(1, Last));
+    else
----------------
I think this is a dangling reference to a dead temporary. Make `Delimiter` a `std::string`?


================
Comment at: lib/Option/OptTable.cpp:301
+    else {
+      std::pair<StringRef, StringRef> SplitArgs = Option.split(Last);
+      LHS = SplitArgs.first;
----------------
`std::tie(LHS, RHS) = Option.split(Last)`


================
Comment at: tools/llvm-mt/llvm-mt.cpp:29
 
+#include <sstream>
 #include <system_error>
----------------
Include raw_ostream.h instead, and use llvm::raw_string_ostream instead of ostringstream.


https://reviews.llvm.org/D41732





More information about the llvm-commits mailing list