[PATCH] D108567: Implement #pragma clang final extension

[Chris Bieneman via Phabricator via cfe-commits]

beanz added a comment.

In D108567#3110925 , @sberg wrote:

> does not produce any warnings/errors for me and indicates that NULL silently 
> gets redefined as a null pointer constant in stddef.h.  Is that intended?

Nope, that's a bug. Fixed in 3e7ad1f2b2c0a753749eaba88d369d6032a50764 
. Thank 
you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

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

[PATCH] D108567: Implement #pragma clang final extension

[Stephan Bergmann via Phabricator via cfe-commits]

sberg added a comment.

> Final macros will always warn on redefinition, including situations with 
> identical bodies and in system headers.

But

  #define NULL syntax error
  #pragma clang final(NULL)
  #include 
  void * p = NULL;

does not produce any warnings/errors for me and indicates that NULL silently 
gets redefined as a null pointer constant in stddef.h.  Is that intended?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

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

[PATCH] D108567: Implement #pragma clang final extension

[Chris Bieneman via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG1e48ef20358f: Implement #pragma clang final extension 
(authored by beanz).

Changed prior to commit:
  https://reviews.llvm.org/D108567?vs=374377=375357#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/test/Lexer/Inputs/final-macro.h
  clang/test/Lexer/Inputs/unsafe-macro.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Lexer/final-macro.c
  clang/test/Lexer/pedantic-macro-interplay.c

Index: clang/test/Lexer/pedantic-macro-interplay.c
===
--- clang/test/Lexer/pedantic-macro-interplay.c
+++ clang/test/Lexer/pedantic-macro-interplay.c
@@ -11,4 +11,17 @@
 // not-expected-warning@+1{{macro 'UNSAFE_MACRO_2' has been marked as deprecated: Don't use this!}}
 #pragma clang restrict_expansion(UNSAFE_MACRO_2, "Don't use this!")
 
-// expected-no-diagnostics
+
+#define Foo 1
+#pragma clang final(Foo)
+// expected-note@+2{{macro marked 'deprecated' here}}
+// expected-note@+1{{macro marked 'deprecated' here}}
+#pragma clang deprecated(Foo)
+// expected-note@+1{{macro marked 'restrict_expansion' here}}
+#pragma clang restrict_expansion(Foo)
+
+// Test that unsafe_header and deprecated markings stick around after the undef
+#include "Inputs/final-macro.h"
+
+// expected-warning@+1{{macro 'Foo' has been marked as deprecated}}
+const int X = Foo;
Index: clang/test/Lexer/final-macro.c
===
--- /dev/null
+++ clang/test/Lexer/final-macro.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -Wfinal-macro %s -fsyntax-only -verify
+
+// Test warning production
+#define Foo 1
+// expected-note@+1 4{{macro marked 'final' here}}
+#pragma clang final(Foo)
+
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be redefined}}
+// expected-note@+1{{previous definition is here}}
+#define Foo 1
+
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be redefined}}
+// expected-warning@+1{{'Foo' macro redefined}}
+#define Foo 2
+
+// expected-warning@+1{{redefining builtin macro}}
+#define __TIME__ 1
+
+// expected-warning@+1{{undefining builtin macro}}
+#undef __TIMESTAMP__
+
+// expected-warning@+1{{macro 'Foo' has been marked as final and should not be undefined}}
+#undef Foo
+// expected-warning@+1{{macro 'Foo' has been marked as final and should not be redefined}}
+#define Foo 3
+
+// Test parse errors
+// expected-error@+1{{expected (}}
+#pragma clang final
+
+// expected-error@+1{{expected )}}
+#pragma clang final(Foo
+
+// expected-error@+1{{no macro named 'Baz'}}
+#pragma clang final(Baz)
+
+// expected-error@+1{{expected identifier}}
+#pragma clang final(4)
+
+// expected-error@+1{{expected (}}
+#pragma clang final Baz
+
+// no diagnostics triggered by these pragmas.
+#pragma clang deprecated(Foo)
+#pragma clang restrict_expansion(Foo)
Index: clang/test/Lexer/deprecate-macro.c
===
--- clang/test/Lexer/deprecate-macro.c
+++ clang/test/Lexer/deprecate-macro.c
@@ -6,10 +6,11 @@
 // expected-error@+1{{expected identifier}}
 #pragma clang deprecated(4
 
-// expected-error@+1{{no macro named foo}}
+// expected-error@+1{{no macro named 'foo'}}
 #pragma clang deprecated(foo)
 
 #define bar 1
+// expected-note@+1{{macro marked 'deprecated' here}} 
 #pragma clang deprecated(bar, "bar is deprecated use 1")
 
 // expected-warning@+1{{macro 'bar' has been marked as deprecated: bar is deprecated use 1}}
@@ -17,6 +18,14 @@
 #endif
 
 #define foo 1
+// expected-note@+8{{macro marked 'deprecated' here}} 
+// expected-note@+7{{macro marked 'deprecated' here}} 
+// expected-note@+6{{macro marked 'deprecated' here}} 
+// expected-note@+5{{macro marked 'deprecated' here}} 
+// expected-note@+4{{macro marked 'deprecated' here}} 
+// expected-note@+3{{macro marked 'deprecated' here}} 
+// expected-note@+2{{macro marked 'deprecated' here}} 
+// expected-note@+1{{macro marked 'deprecated' here}} 
 #pragma clang deprecated(foo)
 
 // expected-error@+1{{expected )}}
@@ -39,7 +48,7 @@
 #endif
 
 int main(int argc, char** argv) {
-  // expected-error@+1{{no macro named main}}
+// expected-error@+1{{no macro named 'main'}}
 #pragma clang deprecated(main)
 
   // expected-warning@+1{{macro 'foo' has been marked as deprecated}}
Index: clang/test/Lexer/Inputs/unsafe-macro.h
===
--- 

[PATCH] D108567: Implement #pragma clang final extension

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

This LGTM, but please wait a bit before landing in case @rsmith has concerns.




Comment at: clang/test/Lexer/final-macro.c:14
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be 
redefined}}
+// expected-note@+1{{previous definition is here}}
+#define Foo 1

beanz wrote:
> 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...
O! I see it now. There were two diagnostics for `Foo` and this is the note 
for the most recent previous definition. Got it, thanks!



Comment at: clang/test/Lexer/final-macro.c:18
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be 
redefined}}
+// expected-warning@+1{{'Foo' macro redefined}}
+#define Foo 2

beanz wrote:
> 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.
I think the current behavior is defensible; if a user has a real world problem 
with it, we can address it at that point.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

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

[PATCH] D108567: Implement #pragma clang final extension

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 374377.
beanz added a comment.

Updates based on feedback from @aaron.ballman.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/test/Lexer/Inputs/final-macro.h
  clang/test/Lexer/Inputs/unsafe-macro.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Lexer/final-macro.c
  clang/test/Lexer/pedantic-macro-interplay.c

Index: clang/test/Lexer/pedantic-macro-interplay.c
===
--- clang/test/Lexer/pedantic-macro-interplay.c
+++ clang/test/Lexer/pedantic-macro-interplay.c
@@ -11,4 +11,17 @@
 // not-expected-warning@+1{{macro 'UNSAFE_MACRO_2' has been marked as deprecated: Don't use this!}}
 #pragma clang restrict_expansion(UNSAFE_MACRO_2, "Don't use this!")
 
-// expected-no-diagnostics
+
+#define Foo 1
+#pragma clang final(Foo)
+// expected-note@+2{{macro marked 'deprecated' here}}
+// expected-note@+1{{macro marked 'deprecated' here}}
+#pragma clang deprecated(Foo)
+// expected-note@+1{{macro marked 'restrict_expansion' here}}
+#pragma clang restrict_expansion(Foo)
+
+// Test that unsafe_header and deprecated markings stick around after the undef
+#include "Inputs/final-macro.h"
+
+// expected-warning@+1{{macro 'Foo' has been marked as deprecated}}
+const int X = Foo;
Index: clang/test/Lexer/final-macro.c
===
--- /dev/null
+++ clang/test/Lexer/final-macro.c
@@ -0,0 +1,45 @@
+// RUN: %clang_cc1 -Wfinal-macro %s -fsyntax-only -verify
+
+// Test warning production
+#define Foo 1
+// expected-note@+1 4{{macro marked 'final' here}}
+#pragma clang final(Foo)
+
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be redefined}}
+// expected-note@+1{{previous definition is here}}
+#define Foo 1
+
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be redefined}}
+// expected-warning@+1{{'Foo' macro redefined}}
+#define Foo 2
+
+// expected-warning@+1{{redefining builtin macro}}
+#define __TIME__ 1
+
+// expected-warning@+1{{undefining builtin macro}}
+#undef __TIMESTAMP__
+
+// expected-warning@+1{{macro 'Foo' has been marked as final and should not be undefined}}
+#undef Foo
+// expected-warning@+1{{macro 'Foo' has been marked as final and should not be redefined}}
+#define Foo 3
+
+// Test parse errors
+// expected-error@+1{{expected (}}
+#pragma clang final
+
+// expected-error@+1{{expected )}}
+#pragma clang final(Foo
+
+// expected-error@+1{{no macro named 'Baz'}}
+#pragma clang final(Baz)
+
+// expected-error@+1{{expected identifier}}
+#pragma clang final(4)
+
+// expected-error@+1{{expected (}}
+#pragma clang final Baz
+
+// no diagnostics triggered by these pragmas.
+#pragma clang deprecated(Foo)
+#pragma clang header_unsafe(Foo)
Index: clang/test/Lexer/deprecate-macro.c
===
--- clang/test/Lexer/deprecate-macro.c
+++ clang/test/Lexer/deprecate-macro.c
@@ -6,7 +6,7 @@
 // expected-error@+1{{expected identifier}}
 #pragma clang deprecated(4
 
-// expected-error@+1{{no macro named foo}}
+// expected-error@+1{{no macro named 'foo'}}
 #pragma clang deprecated(foo)
 
 #define bar 1
@@ -48,7 +48,7 @@
 #endif
 
 int main(int argc, char** argv) {
-// expected-error@+1{{no macro named main}}
+// expected-error@+1{{no macro named 'main'}}
 #pragma clang deprecated(main)
 
   // expected-warning@+1{{macro 'foo' has been marked as deprecated}}
Index: clang/test/Lexer/Inputs/unsafe-macro.h
===
--- clang/test/Lexer/Inputs/unsafe-macro.h
+++ clang/test/Lexer/Inputs/unsafe-macro.h
@@ -4,7 +4,7 @@
 // expected-error@+1{{expected identifier}}
 #pragma clang restrict_expansion(4
 
-// expected-error@+1{{no macro named foo}}
+// expected-error@+1{{no macro named 'foo'}}
 #pragma clang restrict_expansion(foo)
 
 
Index: clang/test/Lexer/Inputs/final-macro.h
===
--- /dev/null
+++ clang/test/Lexer/Inputs/final-macro.h
@@ -0,0 +1,4 @@
+// expected-warning@+2{{macro 'Foo' has been marked as deprecated}}
+// expected-warning@+1{{macro 'Foo' has been marked as unsafe for use in headers}}
+#if Foo
+#endif
Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -1413,26 +1413,46 @@
   return true;
 }
 
-void Preprocessor::emitMacroDeprecationWarning(const Token ) {
-  auto DepMsg = 

[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@+2{{macro 'Foo' has been marked as final and should not be 
redefined}}
+// expected-note@+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@+2{{macro 'Foo' has been marked as final and should not be 
redefined}}
+// expected-warning@+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

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

[PATCH] D108567: Implement #pragma clang final extension

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:3979
+   #undef FINAL_MACRO  // warning: FINAL_MACRO is marked final and should not 
be undefined
+   #define FINAL_MACRO // warning: FINAL_MACRO is marked final and should not 
be redefined
+

beanz wrote:
> aaron.ballman wrote:
> > What happens if the redefinition is to the same token sequence as the 
> > original definition? e.g.,
> > ```
> > #define FINAL_MACRO 1+1
> > #pragma clang final(FINAL_MACRO)
> > #define FINAL_MACRO 1+1 // ok?
> > #define FINAL_MACRO (1+1) // Whoa...slow your roll there, champ!
> > ```
> `-Wmacro-redefined` currently warns on redefinitions even if they are the 
> same as the existing definition.
> 
> The implementation in this patch only triggers on redefining macros that have 
> been undef'd and relies on `-Wmacro-redefined` to catch masking 
> redefinitions. Although I should probably change that so that final catches 
> on both.
> -Wmacro-redefined currently warns on redefinitions even if they are the same 
> as the existing definition.

Okay, SGTM.

> The implementation in this patch only triggers on redefining macros that have 
> been undef'd and relies on -Wmacro-redefined to catch masking redefinitions. 
> Although I should probably change that so that final catches on both.

+1



Comment at: clang/test/Lexer/final-macro.c:5-8
+// expected-note@+4{{macro marked 'final' here}}
+// expected-note@+3{{macro marked 'final' here}}
+// expected-note@+2{{macro marked 'final' here}}
+// expected-note@+1{{macro marked 'final' here}}

99% sure I got the syntax right, but you can specify a number to avoid 
duplicating the diagnostic multiple times and I'm pretty sure it works with the 
`@+N` syntax as well, but I don't recall trying in recent history.



Comment at: clang/test/Lexer/final-macro.c:10-11
+#pragma clang final(Foo)
+#pragma clang deprecated(Foo)
+#pragma clang header_unsafe(Foo)
+

If the goal is to test that mention of the macro here does not cause 
diagnostics, I'd recommend adding this to the end of the file instead of the 
beginning -- from there it's more obvious that none of the preceding 
diagnostics are triggered because of those pragmas.



Comment at: clang/test/Lexer/final-macro.c:14
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be 
redefined}}
+// expected-note@+1{{previous definition is here}}
+#define Foo 1

This previous definition marker looks wrong to me -- it should be pointing to 
line 4, right?



Comment at: clang/test/Lexer/final-macro.c:18
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be 
redefined}}
+// expected-warning@+1{{'Foo' macro redefined}}
+#define Foo 2

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.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

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

[PATCH] D108567: Implement #pragma clang final extension

[Chris Bieneman via Phabricator via cfe-commits]

beanz updated this revision to Diff 372862.
beanz added a comment.

Updates based on feedback from @aaron.ballman.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/test/Lexer/Inputs/final-macro.h
  clang/test/Lexer/Inputs/unsafe-macro.h
  clang/test/Lexer/deprecate-macro.c
  clang/test/Lexer/final-macro.c
  clang/test/Lexer/pedantic-macro-interplay.c

Index: clang/test/Lexer/pedantic-macro-interplay.c
===
--- clang/test/Lexer/pedantic-macro-interplay.c
+++ clang/test/Lexer/pedantic-macro-interplay.c
@@ -11,4 +11,17 @@
 // not-expected-warning@+1{{macro 'UNSAFE_MACRO_2' has been marked as deprecated: Don't use this!}}
 #pragma clang restrict_expansion(UNSAFE_MACRO_2, "Don't use this!")
 
-// expected-no-diagnostics
+
+#define Foo 1
+#pragma clang final(Foo)
+// expected-note@+2{{macro marked 'deprecated' here}}
+// expected-note@+1{{macro marked 'deprecated' here}}
+#pragma clang deprecated(Foo)
+// expected-note@+1{{macro marked 'restrict_expansion' here}}
+#pragma clang restrict_expansion(Foo)
+
+// Test that unsafe_header and deprecated markings stick around after the undef
+#include "Inputs/final-macro.h"
+
+// expected-warning@+1{{macro 'Foo' has been marked as deprecated}}
+const int X = Foo;
Index: clang/test/Lexer/final-macro.c
===
--- /dev/null
+++ clang/test/Lexer/final-macro.c
@@ -0,0 +1,46 @@
+// RUN: %clang_cc1 -Wfinal-macro %s -fsyntax-only -verify
+
+// Test warning production
+#define Foo 1
+// expected-note@+4{{macro marked 'final' here}}
+// expected-note@+3{{macro marked 'final' here}}
+// expected-note@+2{{macro marked 'final' here}}
+// expected-note@+1{{macro marked 'final' here}}
+#pragma clang final(Foo)
+#pragma clang deprecated(Foo)
+#pragma clang header_unsafe(Foo)
+
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be redefined}}
+// expected-note@+1{{previous definition is here}}
+#define Foo 1
+
+// expected-warning@+2{{macro 'Foo' has been marked as final and should not be redefined}}
+// expected-warning@+1{{'Foo' macro redefined}}
+#define Foo 2
+
+// expected-warning@+1{{redefining builtin macro}}
+#define __TIME__ 1
+
+// expected-warning@+1{{undefining builtin macro}}
+#undef __TIMESTAMP__
+
+// expected-warning@+1{{macro 'Foo' has been marked as final and should not be undefined}}
+#undef Foo
+// expected-warning@+1{{macro 'Foo' has been marked as final and should not be redefined}}
+#define Foo 3
+
+// Test parse errors
+// expected-error@+1{{expected (}}
+#pragma clang final
+
+// expected-error@+1{{expected )}}
+#pragma clang final(Foo
+
+// expected-error@+1{{no macro named 'Baz'}}
+#pragma clang final(Baz)
+
+// expected-error@+1{{expected identifier}}
+#pragma clang final(4)
+
+// expected-error@+1{{expected (}}
+#pragma clang final Baz
Index: clang/test/Lexer/deprecate-macro.c
===
--- clang/test/Lexer/deprecate-macro.c
+++ clang/test/Lexer/deprecate-macro.c
@@ -6,7 +6,7 @@
 // expected-error@+1{{expected identifier}}
 #pragma clang deprecated(4
 
-// expected-error@+1{{no macro named foo}}
+// expected-error@+1{{no macro named 'foo'}}
 #pragma clang deprecated(foo)
 
 #define bar 1
@@ -48,7 +48,7 @@
 #endif
 
 int main(int argc, char** argv) {
-// expected-error@+1{{no macro named main}}
+// expected-error@+1{{no macro named 'main'}}
 #pragma clang deprecated(main)
 
   // expected-warning@+1{{macro 'foo' has been marked as deprecated}}
Index: clang/test/Lexer/Inputs/unsafe-macro.h
===
--- clang/test/Lexer/Inputs/unsafe-macro.h
+++ clang/test/Lexer/Inputs/unsafe-macro.h
@@ -4,7 +4,7 @@
 // expected-error@+1{{expected identifier}}
 #pragma clang restrict_expansion(4
 
-// expected-error@+1{{no macro named foo}}
+// expected-error@+1{{no macro named 'foo'}}
 #pragma clang restrict_expansion(foo)
 
 
Index: clang/test/Lexer/Inputs/final-macro.h
===
--- /dev/null
+++ clang/test/Lexer/Inputs/final-macro.h
@@ -0,0 +1,4 @@
+// expected-warning@+2{{macro 'Foo' has been marked as deprecated}}
+// expected-warning@+1{{macro 'Foo' has been marked as unsafe for use in headers}}
+#if Foo
+#endif
Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -1413,26 +1413,46 @@
   return true;
 }
 
-void 

[PATCH] D108567: Implement #pragma clang final extension

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:3968-3984
+Final Macros
+
+
+Clang supports the pragma ``#pragma clang final``, which can be used to
+mark macros as final, meaning they cannot be undef'd or re-defined. For 
example:
+
+.. code-block:: c

aaron.ballman wrote:
> Design question: would it make sense to extend this slightly so that the 
> macro does not have to be defined in order to be finalized? e.g., this could 
> be used as a way for a library author to say "this identifier cannot be 
> defined as a macro"?
That's an interesting thought. I can look into that. Since the current 
implementation relies on the identifier info I'm not sure how much work it 
would be to support that.



Comment at: clang/docs/LanguageExtensions.rst:3979
+   #undef FINAL_MACRO  // warning: FINAL_MACRO is marked final and should not 
be undefined
+   #define FINAL_MACRO // warning: FINAL_MACRO is marked final and should not 
be redefined
+

aaron.ballman wrote:
> What happens if the redefinition is to the same token sequence as the 
> original definition? e.g.,
> ```
> #define FINAL_MACRO 1+1
> #pragma clang final(FINAL_MACRO)
> #define FINAL_MACRO 1+1 // ok?
> #define FINAL_MACRO (1+1) // Whoa...slow your roll there, champ!
> ```
`-Wmacro-redefined` currently warns on redefinitions even if they are the same 
as the existing definition.

The implementation in this patch only triggers on redefining macros that have 
been undef'd and relies on `-Wmacro-redefined` to catch masking redefinitions. 
Although I should probably change that so that final catches on both.



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:544
+  ExtWarn<"macro %0 has been marked as final and should not be "
+  "%select{un|re}1defined">,
+  InGroup;

aaron.ballman wrote:
> Heh, I like your approach, but a goal of %select is to ease translation of 
> our diagnostics to other languages (in theory, anyway).
Ha... Okay... Can do :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

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

[PATCH] D108567: Implement #pragma clang final extension

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/docs/LanguageExtensions.rst:3968-3984
+Final Macros
+
+
+Clang supports the pragma ``#pragma clang final``, which can be used to
+mark macros as final, meaning they cannot be undef'd or re-defined. For 
example:
+
+.. code-block:: c

Design question: would it make sense to extend this slightly so that the macro 
does not have to be defined in order to be finalized? e.g., this could be used 
as a way for a library author to say "this identifier cannot be defined as a 
macro"?



Comment at: clang/docs/LanguageExtensions.rst:3979
+   #undef FINAL_MACRO  // warning: FINAL_MACRO is marked final and should not 
be undefined
+   #define FINAL_MACRO // warning: FINAL_MACRO is marked final and should not 
be redefined
+

What happens if the redefinition is to the same token sequence as the original 
definition? e.g.,
```
#define FINAL_MACRO 1+1
#pragma clang final(FINAL_MACRO)
#define FINAL_MACRO 1+1 // ok?
#define FINAL_MACRO (1+1) // Whoa...slow your roll there, champ!
```



Comment at: clang/include/clang/Basic/DiagnosticLexKinds.td:544
+  ExtWarn<"macro %0 has been marked as final and should not be "
+  "%select{un|re}1defined">,
+  InGroup;

Heh, I like your approach, but a goal of %select is to ease translation of our 
diagnostics to other languages (in theory, anyway).



Comment at: clang/include/clang/Basic/IdentifierTable.h:196
+  // If this is a final macro, make the deprecation and header unsafe bits
+  // stick around after the undefinition so they apply to any redefinitions
+  if (!IsFinal) {





Comment at: clang/include/clang/Lex/Preprocessor.h:828
 
-  /// Usage warning for macros marked by #pragma clang restrict_expansion.
-  llvm::DenseMap
-  RestrictExpansionMacroMsgs;
+  /// Warning information for macro annotations
+  llvm::DenseMap AnnotationInfos;





Comment at: clang/lib/Lex/Pragma.cpp:2083
+if (!II->hasMacroDefinition()) {
+  PP.Diag(Tok, diag::err_pp_visibility_non_macro) << II->getName();
+  return;

This should cause the macro name to be properly quoted in the diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D108567

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

[PATCH] D108567: Implement #pragma clang final extension

[Chris Bieneman via Phabricator via cfe-commits]

beanz created this revision.
beanz added reviewers: aaron.ballman, rsmith, lgerbarg, pete, lebedev.ri, 
dexonsmith.
beanz requested review of this revision.
Herald added a project: clang.

This patch adds a new preprocessor extension ``#pragma clang final``
which enables warning on undefinition and re-definition of macros.

The intent of this warning is to extend beyond ``-Wmacro-redefined`` to
warn against any and all alterations to macros that are marked `final`.

This warning is part of the ``-Wpedantic-macros`` diagnostics group.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D108567

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Basic/IdentifierTable.h
  clang/include/clang/Lex/Preprocessor.h
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Lex/Pragma.cpp
  clang/lib/Lex/Preprocessor.cpp
  clang/test/Lexer/Inputs/final-macro.h
  clang/test/Lexer/final-macro.c
  clang/test/Lexer/pedantic-macro-interplay.c

Index: clang/test/Lexer/pedantic-macro-interplay.c
===
--- clang/test/Lexer/pedantic-macro-interplay.c
+++ clang/test/Lexer/pedantic-macro-interplay.c
@@ -11,4 +11,17 @@
 // not-expected-warning@+1{{macro 'UNSAFE_MACRO_2' has been marked as deprecated: Don't use this!}}
 #pragma clang restrict_expansion(UNSAFE_MACRO_2, "Don't use this!")
 
-// expected-no-diagnostics
+
+#define Foo 1
+#pragma clang final(Foo)
+// expected-note@+2{{macro marked 'deprecated' here}}
+// expected-note@+1{{macro marked 'deprecated' here}}
+#pragma clang deprecated(Foo)
+// expected-note@+1{{macro marked 'restrict_expansion' here}}
+#pragma clang restrict_expansion(Foo)
+
+// Test that unsafe_header and deprecated markings stick around after the undef
+#include "Inputs/final-macro.h"
+
+// expected-warning@+1{{macro 'Foo' has been marked as deprecated}}
+const int X = Foo;
Index: clang/test/Lexer/final-macro.c
===
--- /dev/null
+++ clang/test/Lexer/final-macro.c
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -Wfinal-macro %s -fsyntax-only -verify
+
+// Test warning production
+// expected-note@+1{{previous definition is here}}
+#define Foo 1
+// expected-note@+2{{macro marked 'final' here}}
+// expected-note@+1{{macro marked 'final' here}}
+#pragma clang final(Foo)
+#pragma clang deprecated(Foo)
+#pragma clang header_unsafe(Foo)
+
+// expected-warning@+1{{'Foo' macro redefined}}
+#define Foo 2
+
+// expected-warning@+1{{redefining builtin macro}}
+#define __TIME__ 1
+
+// expected-warning@+1{{undefining builtin macro}}
+#undef __TIMESTAMP__
+
+// expected-warning@+1{{macro 'Foo' has been marked as final and should not be undefined}}
+#undef Foo
+// expected-warning@+1{{macro 'Foo' has been marked as final and should not be redefined}}
+#define Foo 3
+
+// Test parse errors
+// expected-error@+1{{expected (}}
+#pragma clang final
+
+// expected-error@+1{{expected )}}
+#pragma clang final(Foo
+
+// expected-error@+1{{no macro named Baz}}
+#pragma clang final(Baz)
+
+// expected-error@+1{{expected identifier}}
+#pragma clang final(4)
+
+// expected-error@+1{{expected (}}
+#pragma clang final Baz
Index: clang/test/Lexer/Inputs/final-macro.h
===
--- /dev/null
+++ clang/test/Lexer/Inputs/final-macro.h
@@ -0,0 +1,4 @@
+// expected-warning@+2{{macro 'Foo' has been marked as deprecated}}
+// expected-warning@+1{{macro 'Foo' has been marked as unsafe for use in headers}}
+#if Foo
+#endif
Index: clang/lib/Lex/Preprocessor.cpp
===
--- clang/lib/Lex/Preprocessor.cpp
+++ clang/lib/Lex/Preprocessor.cpp
@@ -1413,26 +1413,46 @@
   return true;
 }
 
-void Preprocessor::emitMacroDeprecationWarning(const Token ) {
-  auto DepMsg = getMacroDeprecationMsg(Identifier.getIdentifierInfo());
-  if (DepMsg.first.empty())
+void Preprocessor::emitMacroDeprecationWarning(const Token ) const {
+  const MacroAnnotations  =
+  getMacroAnnotations(Identifier.getIdentifierInfo());
+  assert(A.DeprecationInfo &&
+ "Macro deprecation warning without recorded annotation!");
+  const MacroAnnotationInfo  = *A.DeprecationInfo;
+  if (Info.Message.empty())
 Diag(Identifier, diag::warn_pragma_deprecated_macro_use)
 << Identifier.getIdentifierInfo() << 0;
   else
 Diag(Identifier, diag::warn_pragma_deprecated_macro_use)
-<< Identifier.getIdentifierInfo() << 1 << DepMsg.first;
-  Diag(DepMsg.second, diag::note_pp_macro_annotation) << 0;
+<< Identifier.getIdentifierInfo() << 1 << Info.Message;
+  Diag(Info.Location, diag::note_pp_macro_annotation) << 0;
 }
 
-void Preprocessor::emitMacroUnsafeHeaderWarning(const Token ) {
-  auto DepMsg = getRestrictExpansionMsg(Identifier.getIdentifierInfo());
-  if (DepMsg.first.empty())
+void