[PATCH] D67935: Add `#pragma clang deprecated`, used to deprecate macros
Aaron Ballman via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Thu Sep 3 06:12:55 PDT 2020
aaron.ballman added a comment.
In D67935#2251145 <https://reviews.llvm.org/D67935#2251145>, @erik.pilkington wrote:
> @aaron.ballman: Did you happen to get any feedback on macro attributes? There are a growing number of macros that we'd like to be able to deprecate, and having a workable solution would be useful for us.
Thank you for bringing this back up! I've worked on a patch to add preprocessor attributes to clang but have set it aside because it feels like it may be an awkward fit because of attribute arguments -- for instance, the preprocessor has no type system or AST, so we track values for things with `APValue` objects and there is no string `APValue` type, so it would be a fair amount of work to support `# [[deprecated("don't use baz")]] define BAZ`, let alone the more esoteric situations for arbitrary attributes. Another issue is that the preprocessor is sometimes shared between C/C++ frontends and, say, a FORTRAN frontend, which could suddenly introduce a new feature into a FORTRAN compiler without extra work.
Based on all of that, I think we should go with your approach of using a `#pragma` as it does solve a problem and isn't quite as novel. However, I am wondering about the design a bit -- why are you using a string literal to supply the macro name? It's my understanding that all preprocessing directives are executed at the same phase of translation (so you don't have to worry about `#define` changing the behavior of `#pragma` or `_Pragma`), so I would have expected a design more like:
#define FOOBAR(x) whatever(x)
#pragma clang deprecated(FOOBAR, "don't use FOOBAR, it will do bad things")
This feels like a more approachable way to expose the feature, to me, but I wonder if I'm missing something.
Other feedback is: should we diagnose if the pp-token for the macro identifier doesn't actually match the name of a macro? Or are we allowing constructs like:
#pragma clang deprecated(FOOBAR, "don't use FOOBAR, twelve is a terrible number")
#define FOOBAR 12
================
Comment at: clang/lib/Lex/Pragma.cpp:1544
+ PP.Lex(Tok);
+ if (!PP.FinishLexStringLiteral(Tok, EntityString,
+ "#pragma clang deprecated",
----------------
Do we want to allow arbitrary string literals, or only narrow character string literals? e.g., should we disallow something like: `#pragma clang deprecated(L"oops", U"hahahaha")` ?
================
Comment at: clang/test/Lexer/pragma-deprecated.c:17-20
+#pragma clang deprecated("\"flerp.h\"", "use \"flarp.h\" instead")
+
+// expected-warning at +1{{'flerp' is deprecated}}
+#pragma clang deprecated("flerp")
----------------
What is the utility of unconditionally emitting a diagnostic like this? We've already got `#warning`.
Repository:
rC Clang
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D67935/new/
https://reviews.llvm.org/D67935
More information about the cfe-commits
mailing list