Author: Sirraide Date: 2024-05-27T15:58:58+02:00 New Revision: 5e140c8a17889a4cc677edee9f5db618dca70535
URL: https://github.com/llvm/llvm-project/commit/5e140c8a17889a4cc677edee9f5db618dca70535 DIFF: https://github.com/llvm/llvm-project/commit/5e140c8a17889a4cc677edee9f5db618dca70535.diff LOG: [Clang] [NFC] Clarify assume diagnostic (#93077) Currently, if the argument to `__builtin_assume` and friends contains side-effects, we issue the following diagnostic: ``` <source>:1:34: warning: the argument to '__builtin_assume' has side effects that will be discarded [-Wassume] 1 | void f(int x) { __builtin_assume(x++); } | ``` The issue here is that this diagnostic misrepresents what is actually happening: not only do we discard the side-effects of the expression, but we also don’t even emit any assumption information at all because the backend is not equipped to deal with eliminating side-effects in cases such as this. This has caused some confusion (see #91612) beacuse the current wording of the warning suggests that, sensibly, only the side-effects of the expression, and not the assumption itself, will be discarded. This pr updates the diagnostic to state what is actually happening: that the assumption has no effect at all because its argument contains side-effects: ``` <source>:1:34: warning: assumption is ignored because it contains (potential) side-effects [-Wassume] 1 | void f(int x) { __builtin_assume(x++); } | ``` I’ve deliberately included ‘(potential)’ here because even expressions that only contain potential side-effects (e.g. `true ? x : x++` or a call to a function that is pure, but we don’t know that it is) cause the assumption to be discarded. This, too, has caused some confusion because it was erroneously assumed that Clang would e.g. infer that a function call is pure and not discard the assumption as a result when that isn’t the case. This is intended to be temporary; we should revert back to the original diagnostic once we have proper support for assumptions with side-effects in the backend (in which case the side-effects will still be discarded, but the assumption won’t) This fixes #91612. Added: Modified: clang/include/clang/Basic/DiagnosticSemaKinds.td clang/test/Parser/MicrosoftExtensions.cpp clang/test/Sema/builtin-assume.c clang/test/Sema/stmtexprs.c clang/test/SemaCXX/cxx23-assume.cpp Removed: ################################################################################ diff --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td b/clang/include/clang/Basic/DiagnosticSemaKinds.td index a06a951c72df4..f15cba63624ea 100644 --- a/clang/include/clang/Basic/DiagnosticSemaKinds.td +++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td @@ -855,7 +855,7 @@ def note_strncat_wrong_size : Note< "the terminating null byte">; def warn_assume_side_effects : Warning< - "the argument to %0 has side effects that will be discarded">, + "assumption is ignored because it contains (potential) side-effects">, InGroup<DiagGroup<"assume">>; def warn_omp_assume_attribute_string_unknown : Warning< "unknown assumption string '%0'; attribute is potentially ignored">, diff --git a/clang/test/Parser/MicrosoftExtensions.cpp b/clang/test/Parser/MicrosoftExtensions.cpp index 6bf802a29ace3..9102bca8f6bb2 100644 --- a/clang/test/Parser/MicrosoftExtensions.cpp +++ b/clang/test/Parser/MicrosoftExtensions.cpp @@ -426,7 +426,7 @@ bool f(int); template <typename T> struct A { constexpr A(T t) { - __assume(f(t)); // expected-warning{{the argument to '__assume' has side effects that will be discarded}} + __assume(f(t)); // expected-warning{{assumption is ignored because it contains (potential) side-effects}} } constexpr bool g() { return false; } }; diff --git a/clang/test/Sema/builtin-assume.c b/clang/test/Sema/builtin-assume.c index 932fb5c973eb0..21d62d8fd06c1 100644 --- a/clang/test/Sema/builtin-assume.c +++ b/clang/test/Sema/builtin-assume.c @@ -8,20 +8,20 @@ int ispure(int) __attribute__((pure)); int foo(int *a, int i) { #ifdef _MSC_VER __assume(i != 4); - __assume(++i > 2); //expected-warning {{the argument to '__assume' has side effects that will be discarded}} - __assume(nonconst() > 2); //expected-warning {{the argument to '__assume' has side effects that will be discarded}} + __assume(++i > 2); //expected-warning {{assumption is ignored because it contains (potential) side-effects}} + __assume(nonconst() > 2); //expected-warning {{assumption is ignored because it contains (potential) side-effects}} __assume(isconst() > 2); __assume(ispure(i) > 2); - __assume(ispure(++i) > 2); //expected-warning {{the argument to '__assume' has side effects that will be discarded}} + __assume(ispure(++i) > 2); //expected-warning {{assumption is ignored because it contains (potential) side-effects}} int test = sizeof(struct{char qq[(__assume(i != 5), 7)];}); #else __builtin_assume(i != 4); - __builtin_assume(++i > 2); //expected-warning {{the argument to '__builtin_assume' has side effects that will be discarded}} - __builtin_assume(nonconst() > 2); //expected-warning {{the argument to '__builtin_assume' has side effects that will be discarded}} + __builtin_assume(++i > 2); //expected-warning {{assumption is ignored because it contains (potential) side-effects}} + __builtin_assume(nonconst() > 2); //expected-warning {{assumption is ignored because it contains (potential) side-effects}} __builtin_assume(isconst() > 2); __builtin_assume(ispure(i) > 2); - __builtin_assume(ispure(++i) > 2); //expected-warning {{the argument to '__builtin_assume' has side effects that will be discarded}} + __builtin_assume(ispure(++i) > 2); //expected-warning {{assumption is ignored because it contains (potential) side-effects}} int test = sizeof(struct{char qq[(__builtin_assume(i != 5), 7)];}); // expected-warning {{variable length array}} #endif diff --git a/clang/test/Sema/stmtexprs.c b/clang/test/Sema/stmtexprs.c index 708fc9abb75c0..7493bbcef363d 100644 --- a/clang/test/Sema/stmtexprs.c +++ b/clang/test/Sema/stmtexprs.c @@ -4,6 +4,6 @@ int stmtexpr_fn(void); void stmtexprs(int i) { __builtin_assume( ({ 1; }) ); // no warning about "side effects" __builtin_assume( ({ if (i) { (void)0; }; 42; }) ); // no warning about "side effects" - // expected-warning@+1 {{the argument to '__builtin_assume' has side effects that will be discarded}} + // expected-warning@+1 {{assumption is ignored because it contains (potential) side-effects}} __builtin_assume( ({ if (i) ({ stmtexpr_fn(); }); 1; }) ); } diff --git a/clang/test/SemaCXX/cxx23-assume.cpp b/clang/test/SemaCXX/cxx23-assume.cpp index ea71e7b251823..9138501d726dd 100644 --- a/clang/test/SemaCXX/cxx23-assume.cpp +++ b/clang/test/SemaCXX/cxx23-assume.cpp @@ -28,7 +28,7 @@ bool f2(); template <typename T> constexpr void f3() { - [[assume(T{})]]; // expected-error {{not contextually convertible to 'bool'}} expected-warning {{has side effects that will be discarded}} ext-warning {{C++23 extension}} + [[assume(T{})]]; // expected-error {{not contextually convertible to 'bool'}} expected-warning {{assumption is ignored because it contains (potential) side-effects}} ext-warning {{C++23 extension}} } void g(int x) { @@ -38,13 +38,13 @@ void g(int x) { S<false>{}.f(); S<true>{}.g<char>(); S<true>{}.g<int>(); - [[assume(f2())]]; // expected-warning {{side effects that will be discarded}} ext-warning {{C++23 extension}} + [[assume(f2())]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}} ext-warning {{C++23 extension}} - [[assume((x = 3))]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}} - [[assume(x++)]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}} - [[assume(++x)]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}} - [[assume([]{ return true; }())]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}} - [[assume(B{})]]; // expected-warning {{has side effects that will be discarded}} // ext-warning {{C++23 extension}} + [[assume((x = 3))]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}} // ext-warning {{C++23 extension}} + [[assume(x++)]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}} // ext-warning {{C++23 extension}} + [[assume(++x)]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}} // ext-warning {{C++23 extension}} + [[assume([]{ return true; }())]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}} // ext-warning {{C++23 extension}} + [[assume(B{})]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}} // ext-warning {{C++23 extension}} [[assume((1, 2))]]; // expected-warning {{has no effect}} // ext-warning {{C++23 extension}} f3<A>(); // expected-note {{in instantiation of}} @@ -91,7 +91,7 @@ static_assert(S<false>{}.g<A>()); // expected-error {{not an integral constant e template <typename T> constexpr bool f4() { - [[assume(!T{})]]; // expected-error {{invalid argument type 'D'}} // expected-warning 2 {{side effects}} ext-warning {{C++23 extension}} + [[assume(!T{})]]; // expected-error {{invalid argument type 'D'}} // expected-warning 2 {{assumption is ignored because it contains (potential) side-effects}} ext-warning {{C++23 extension}} return sizeof(T) == sizeof(int); } @@ -137,8 +137,8 @@ static_assert(f5<F>() == 2); // expected-note {{while checking constraint satisf // Do not validate assumptions whose evaluation would have side-effects. constexpr int foo() { int a = 0; - [[assume(a++)]] [[assume(++a)]]; // expected-warning 2 {{has side effects that will be discarded}} ext-warning 2 {{C++23 extension}} - [[assume((a+=1))]]; // expected-warning {{has side effects that will be discarded}} ext-warning {{C++23 extension}} + [[assume(a++)]] [[assume(++a)]]; // expected-warning 2 {{assumption is ignored because it contains (potential) side-effects}} ext-warning 2 {{C++23 extension}} + [[assume((a+=1))]]; // expected-warning {{assumption is ignored because it contains (potential) side-effects}} ext-warning {{C++23 extension}} return a; } @@ -154,7 +154,7 @@ int foo (int x, int y) { __attribute__((assume(x == 42))); - __attribute__((assume(++y == 43))); // expected-warning {{has side effects that will be discarded}} + __attribute__((assume(++y == 43))); // expected-warning {{assumption is ignored because it contains (potential) side-effects}} return x + y; } } _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
