[PATCH] D108567: Implement #pragma clang final extension

[Chris Bieneman via Phabricator via cfe-commits]

beanz marked 3 inline comments as done and an inline comment as not done.
beanz added a comment.

Will push updates in a second.



================
Comment at: clang/test/Lexer/final-macro.c:14
+// expected-warning at +2{{macro 'Foo' has been marked as final and should not be redefined}}
+// expected-note at +1{{previous definition is here}}
+#define Foo 1
----------------
aaron.ballman wrote:
> This previous definition marker looks wrong to me -- it should be pointing to line 4, right?
Since the macro is redefined here, when the later warning gets hit the note pops up here, which makes sense. This itself being a redefinition is amusing, but I think this is probably the way we want it to work. I don't have strong opinions though...


================
Comment at: clang/test/Lexer/final-macro.c:18
+// expected-warning at +2{{macro 'Foo' has been marked as final and should not be redefined}}
+// expected-warning at +1{{'Foo' macro redefined}}
+#define Foo 2
----------------
aaron.ballman wrote:
> Should we suppress this diagnostic when we know we're already issuing the previous one? I get why they both are issued, but it does seem a bit unclean to have two warnings that are  basically both "you are redefining this macro and maybe you should reconsider that" diagnostics. (I don't feel strongly, mostly wondering out loud.)
I can see this going both ways. I tend to err toward listing _all_ the warnings in case someone wants to try and suppress them for a specific use case. Otherwise they have to build multiple times in a cycle to suppress everything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567