[PATCH] D73891: [RISCV] Support experimental/unratified extensions

[Roger Ferrer Ibanez via Phabricator via cfe-commits]

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