beanz added inline comments.
================ Comment at: clang/test/Lexer/Inputs/unsafe-macro-2.h:23-26 +// not-expected-warning@+1{{macro 'UNSAFE_MACRO_2' has been marked as unsafe for use in headers}} +#undef UNSAFE_MACRO_2 +// not-expected-warning@+1{{macro 'UNSAFE_MACRO_2' has been marked as unsafe for use in headers}} +#define UNSAFE_MACRO_2 2 ---------------- aaron.ballman wrote: > beanz wrote: > > aaron.ballman wrote: > > > beanz wrote: > > > > aaron.ballman wrote: > > > > > Why do we not expect warnings for these cases? I would have expected > > > > > that undefining a macro is just as unsafe for ABI reasons as defining > > > > > a macro is. > > > > I kinda waffled on this myself. My thought was to treat this similarly > > > > to how we handle the macro redefinition warning. If you `undef`, you're > > > > kind of claiming the macro as your own and all bets are off... > > > > > > > > That said, my next clang extension closes that loop hole too: > > > > https://github.com/llvm-beanz/llvm-project/commit/f0a5216e18f5ee0883039095169bd380295b1de0 > > > So `header_unsafe` is "diagnose if someone expands this macro from > > > outside the main source file" and `final` is "diagnose if someone defines > > > or undefines this macro anywhere", correct? Would it make sense to have a > > > shorthand to combine these effects for a "fully reserved" macro > > > identifier (`#pragma clang reserve_macro(IDENT[, msg])` as a strawman)? > > My thought process for implementing them separately was that final would be > > useful independent of header_unsafe. I could, for example, see applying > > final to macros like MIN and MAX, where they can be safely used anywhere, > > but you really don’t want multiple definitions floating around. > FWIW, I agree that having separation is useful -- I think these serve > orthogonal (but related) purposes: macros which can only be used by "user > code", and macros which cannot be redefined or undefined. I was thinking that > would be an additional pragma instead of a replacement for the two proposed. > > I should probably tell you my use cases so we're both on the same page. One > of the most frustrating problems with trying to write a highly portable > library is the fact that I have to worry about users defining macros that may > conflict with my identifiers (like function names, structure names, template > names, etc), but I have no way to reserve those identifiers. I'm hopeful we > can find a way that I can "protect" those identifiers in a library with an > extension that basically says "you can't use these identifiers for macro > purposes without breaking me". I think that's a combination of > `header_unsafe` and `final` -- I don't want other libraries to start using > macros with the names of my library's functions if my header has been > included somewhere, and I don't want user code defining macros that may > conflict with my library. Ah! That makes perfect sense. I think I misunderstood your comment. Totally agree, having a shorthand that groups both together would be super useful. I suspect we'll have some uses where we will want both `final` and `header_unsafe` too! Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D107095/new/ https://reviews.llvm.org/D107095 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits