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

Fangrui Song via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Aug 11 11:18:48 PDT 2022


MaskRay added a comment.

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}}

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

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.



================
Comment at: llvm/utils/lit/lit/llvm/config.py:579
+            l = h - clang_std_group % (h-l+1)
+            self.config.substitutions.append((s, '-std=c++' + clang_std_values[l]))
+
----------------
aaron.ballman wrote:
> One thing we should consider is whether we want to run in *all* the specified language modes instead of just the newest mode. This will make running tests slower because we'll run significantly more of them, and it might get awkward if a lot of tests change behavior in the different language modes, so I don't suggest it as part of this patch.
This is difficult in lit. Will answer in my main comment.


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