[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

Aaron Ballman via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Tue Aug 16 11:15:05 PDT 2022


aaron.ballman added a comment.

In D131464#3716905 <https://reviews.llvm.org/D131464#3716905>, @MaskRay wrote:

> Sorry, my previous main comment had been written before I introduced `LIT_CLANG_STD_GROUP` in `llvm/utils/lit/lit/llvm/config.py`. The multiple `%clang_cc1` approach actually looks like the following.
> Note the use of `%stdcxx_17-` to make the test future-proof.
> (It is non-trivial to run one `RUN` line multiples times with different `LIT_CLANG_STD_GROUP`. For now I just test locally with different `LIT_CLANG_STD_GROUP`.)
>
>   // RUN: %clang_cc1 %s -fsyntax-only -verify=expected,precxx17 %stdcxx_11-14 -fdata-sections -fcolor-diagnostics
>   // RUN: %clang_cc1 %s -fsyntax-only -verify %stdcxx_17- -fdata-sections -fcolor-diagnostics
>   
>   ...
>     TypedefAligned4 TA8c = TA8a + TA8b;  // expected-warning {{passing 4-byte aligned argument to 8-byte aligned parameter 'this' of 'operator+' may result in an unaligned pointer access}} \
>                                          // expected-warning {{passing 4-byte aligned argument to 8-byte aligned parameter 1 of 'operator+' may result in an unaligned pointer access}} \
>                                          // precxx17-warning {{passing 4-byte aligned argument to 8-byte aligned parameter 'this' of 'StructAligned8' may result in an unaligned pointer access}}

Personally, I like this style. I tend to be a heavy user of `-verify` prefixes though, so I might be biased.

> If this is changed to use `#if __cplusplus >= 201703L`, there will be multiple lines with relative line numbers (e.g. `@-2` `@-4`)
>
>   register int ro; // expected-error {{illegal storage class on file-scoped variable}}
>   #if __cplusplus >= 201703L
>   // expected-error at -2 {{ISO C++17 does not allow 'register' storage class specifier}}
>   #elif __cplusplus >= 201103L
>   // expected-warning at -4 {{'register' storage class specifier is deprecated}}
>   #endif

FWIW, you don't have to use relative markers, but that uses an even less common idiom of bookmarks. e.g.,

  register int ro; // expected-error {{illegal storage class on file-scoped variable}} \
                      #bookmark
  #if __cplusplus >= 201703L
  // expected-error@#bookmark {{ISO C++17 does not allow 'register' storage class specifier}}
  #elif __cplusplus >= 201103L
  // expected-warning@#bookmark {{'register' storage class specifier is deprecated}}
  #endif



> Personally I prefer multiple `%clang_cc1` over `#if`. The first few lines give users a first impression. The dispatch makes it clear the test has different behaviors with the `%stdcxx_*` described dialects.

I tend to have the same opinion.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131464



More information about the llvm-commits mailing list