[all-commits] [llvm/llvm-project] 5e140c: [Clang] [NFC] Clarify assume diagnostic (#93077)

[Sirraide via All-commits]

  Branch: refs/heads/main
  Home:   https://github.com/llvm/llvm-project
  Commit: 5e140c8a17889a4cc677edee9f5db618dca70535
      https://github.com/llvm/llvm-project/commit/5e140c8a17889a4cc677edee9f5db618dca70535
  Author: Sirraide <aeternalmail at gmail.com>
  Date:   2024-05-27 (Mon, 27 May 2024)

  Changed paths:
    M clang/include/clang/Basic/DiagnosticSemaKinds.td
    M clang/test/Parser/MicrosoftExtensions.cpp
    M clang/test/Sema/builtin-assume.c
    M clang/test/Sema/stmtexprs.c
    M clang/test/SemaCXX/cxx23-assume.cpp

  Log Message:
  -----------
  [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.



To unsubscribe from these emails, change your notification settings at https://github.com/llvm/llvm-project/settings/notifications