rogfer01 added inline comments.
================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:50
+static bool isExperimentalExtension(StringRef Ext) {
+ // Currently 'b' is the only supported experimental extension
----------------
Suggestion: I think we can avoid these two functions (`isExperimentalExtension`
and `getExperimentalExtensionVersion`) going out of sync if we have only one of
them and make it return an `llvm::Optional` of the pair of versions.
Thent it can be used like this
```lang=cpp
if (auto ExperimentalExtension = isExperimentalExtension(Ext)) {
std::pair<StringRef, StringRef> SupportedVers = *ExperimentalVersion;
...
}
```
I'd also add a comment that the pair's `first` is the major version and
`second` is the minor version (or alternative use a struct with two public
fields `Major` and `Minor`)
================
Comment at: clang/lib/Driver/ToolChains/Arch/RISCV.cpp:396
+ I += Minor.size() + 1 /*'p'*/;
+ if (*I == '_')
+ ++I;
----------------
There is no test for that case but I'm afraid we can't test it yet, can we?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D73891/new/
https://reviews.llvm.org/D73891
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
