[libcxx-commits] [PATCH] D99789: [libc++] Add fallback standard flags and normalize corresponding feature.

[Marek Kurdej via Phabricator via libcxx-commits]

curdeius marked 2 inline comments as done.
curdeius added inline comments.


================
Comment at: libcxx/utils/libcxx/test/params.py:11-12
 
-_allStandards = ['c++03', 'c++11', 'c++14', 'c++17', 'c++2a', 'c++2b']
+_allStandards = {'c++03': None, 'c++11': None, 'c++14': None,
+                 'c++17': 'c++1z', 'c++20': 'c++2a', 'c++2b': None}
 _warningFlags = [
----------------
Quuxplusone wrote:
> I like this. Is it worth adding `c++0x`, `c++1y` as fallbacks for those two? I assume the logic of leaving them out is simply that we don't officially support any compiler old enough to lack a `c++14` mode. But it might be useful-to-someone and/or encyclopedic to keep them in the script, I dunno.
I can add these if Louis is OK. Louis removed them in his suggested edit.


================
Comment at: libcxx/utils/libcxx/test/params.py:58-62
+  Parameter(name='std', choices=_allStandardsAndFallbacks, type=str,
             help="The version of the standard to compile the test suite with.",
-            default=lambda cfg: next(s for s in reversed(_allStandards) if hasCompileFlag(cfg, '-std='+s)),
+            default=lambda cfg: getLatestStdFlag(cfg),
             actions=lambda std: [
+              AddFeature(_undoStandardFallback[std] if std in _undoStandardFallback else std),
----------------
Quuxplusone wrote:
> ldionne wrote:
> > curdeius wrote:
> > > curdeius wrote:
> > > > curdeius wrote:
> > > > > ldionne wrote:
> > > > > > 
> > > > > Unless I'm mistaken, we still need to call `AddFeature`, no? @ldionne 
> > > > Also, in actions, I don't have `cfg`, only `std`...
> > > WDYT about:
> > > ```
> > > AddFeature(next((s for s, f in reversed(_allStandards) if f == std), std)),
> > > ```
> > > ?
> > Yeah, it seems I really botched my code change suggestion :-).
> > 
> > Basically anything that works is fine by me, but I found the logic with multiple variables at the beginning of the file to be hard to follow.
> I think all we're actually doing here is saying, "If the user gave us `c++2a`, turn it into `c++20`." Right? So that would be, like,
> 
>     def what_were_doing(flag):
>       if flag in _allStandards.values():
>         AddFeature(next(s for s, f in _allStandards.items() if f == flag))
>       else:
>         AddFeature(flag)
> 
> which is like... exactly what you wrote, except without the `reversed`, right? The `reversed` would make a difference only if `_allStandards.items()` were in some relevant order to begin with.
> 
> I'm not thrilled by the readability of this expression, but I have no concrete improvements to suggest.
> 
> So I think I'm just asking to remove the `reversed(`...`)`.
Ok, I'll remove `reversed`.


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D99789/new/

https://reviews.llvm.org/D99789