tra added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:93
+ StringRef Name = Features[I];
+ assert(Name[0] == '-' || Name[0] == '+');
+ LastOpt[Name.drop_front(1)] = I;
----------------
I don't think assert should be used for something that may be specified to the
user on command line.
================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:97
+
+ for (unsigned I = 0, N = Features.size(); I < N; ++I) {
+ // If this feature was overridden, ignore it.
----------------
Would it be simpler to just iterate over features backwards and skip the
features we've already seen?
================
Comment at: clang/lib/Driver/ToolChains/CommonArgs.h:120
+/// If there are multiple +xxx or -xxx features, keep the last one.
+void unifyTargetFeatures(std::vector<StringRef> &Features);
+
----------------
Returning a new `std::vector<StringRef>` would make it more convenient to use
and there's less complexity -- no need for the caller to create a temporary
vector and the function itself can just return the temporary vector it collects
the items in.
```
for (auto Feature : unifyTargetFeatures(Features)) {
...
}
```
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D82579/new/
https://reviews.llvm.org/D82579
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
