[PATCH] D159162: [llvm] Add assembly syntax highlighting

Tue Sep 5 10:55:50 PDT 2023

danilaml added inline comments.

================
Comment at: llvm/include/llvm/MC/MCInstPrinter.h:100
+  public:
+    WithMarkup(raw_ostream &OS, Markup M, bool EnableMarkup, bool EnableColor);
+    ~WithMarkup();
----------------
jroelofs wrote:
> DavidSpickett wrote:
> > danilaml wrote:
> > > danilaml wrote:
> > > > jroelofs wrote:
> > > > > maybe this should be `[[nodiscard]]`
> > > > `[[nodiscard]]` on constructors is a C++20 feature, even though it works on older compilers (like gcc 7.4 we claim to support), it still produces an annoying (especially if you compile with -Werror) warning. Just wanted to mention.
> > > Correction - it's a DR against C++17 so it's retroactively added there, but doesn't change the fact that older compilers may still produce warnings.
> > https://github.com/llvm/llvm-project/commit/48987fb8cc844667c6ee2a315140cb8f3d817fc1 removes it, just because it's blocking some downstream stuff we have.
> > 
> > I do see the logic in having it so we don't forget to colour things but not sure how to do that in a compatible way right now.
> We could do the macro back-compat thing:
> 
> ```
> // [[nodiscard]] on constructors is a C++20 feature...
> #if __cplusplus >= 202002L
> #  define LLVM_CTOR_NODISCARD [[nodiscard]]
> #else
> #  define LLVM_CTOR_NODISCARD
> #endif
> ```
Since this was a defect report against C++17, it should've been applied against it retroactively, so the proper check would be something like this:

```
#ifdef __has_cpp_attribute
#  if  __has_cpp_attribute(nodiscard) >= 201907L
...
```
see https://www.open-std.org/jtc1/sc22/wg21/docs/papers/2019/p1771r1.pdf and https://isocpp.org/std/standing-documents/sd-6-sg10-feature-test-recommendations/#examples

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D159162