aaron.ballman added inline comments.

================
Comment at: clang/test/Parser/objc-static-assert.mm:1
+// RUN: %clang_cc1 -fsyntax-only -verify -Wno-objc-root-class %s
+
----------------
thakis wrote:
> aaron.ballman wrote:
> > Can you try explicitly specifying C++98 as the underlying language standard 
> > mode? I feel like `_Static_assert()` will continue to work there (because 
> > we made it a language extension in all modes) but `static_assert()` may 
> > fail (because that's gated on C++11 support). If that turns out to be the 
> > case, then I think `objc_static_assert` should be more complex than 
> > expanding to `true`, or we should talk about supporting `static_assert()` 
> > in all C++ language modes like we do for `_Static_assert()`.
> Correct, with -std=c++98 static_assert isn't available but _Static_assert 
> still is. If you want, I can add a test for this, but this is covered by 
> regular c++ tests already.
> 
> I think the has_feature() should stay as-is though: Else you have no way to 
> know if _Static_assert works in obj-c mode, and you can check if 
> static_assert works by checkout has_feature && __cplusplus >= 201103L if you 
> still care about c++98.
> 
> (And adding one feature each for static_assert and _Static_assert seems like 
> overkill.)
> Correct, with -std=c++98 static_assert isn't available but _Static_assert 
> still is. If you want, I can add a test for this, but this is covered by 
> regular c++ tests already.

Please do (if we don't change the availability of `static_assert()`), because 
the test currently relies on the unwritten -std option in order to pass.

> I think the has_feature() should stay as-is though: Else you have no way to 
> know if _Static_assert works in obj-c mode, and you can check if 
> static_assert works by checkout has_feature && __cplusplus >= 201103L if you 
> still care about c++98.

I don't think this is the correct approach. Testing for `static_assert()` 
support should not leave the user guessing at what the correct spelling is.

> (And adding one feature each for static_assert and _Static_assert seems like 
> overkill.)

Definitely agreed there.

I think the correct way forward is to support `static_assert()` in all language 
modes like we already do for `_Static_assert()`, then `objc_static_assert` 
becomes useful as-is. I cannot think of a reason why we would want to allow 
`_Static_assert()` in C++ but not allow `static_assert()`.


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

https://reviews.llvm.org/D59223



_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Reply via email to