Changeset View
Standalone View
clang/include/clang/Basic/DiagnosticLexKinds.td
| Show First 20 Lines • Show All 294 Lines • ▼ Show 20 Lines | |||||
| def pp_nonportable_system_path : NonportablePath, DefaultIgnore, | def pp_nonportable_system_path : NonportablePath, DefaultIgnore, | ||||
| InGroup<DiagGroup<"nonportable-system-include-path">>; | InGroup<DiagGroup<"nonportable-system-include-path">>; | ||||
| def pp_pragma_once_in_main_file : Warning<"#pragma once in main file">, | def pp_pragma_once_in_main_file : Warning<"#pragma once in main file">, | ||||
| InGroup<DiagGroup<"pragma-once-outside-header">>; | InGroup<DiagGroup<"pragma-once-outside-header">>; | ||||
| def pp_pragma_sysheader_in_main_file : Warning< | def pp_pragma_sysheader_in_main_file : Warning< | ||||
| "#pragma system_header ignored in main file">, | "#pragma system_header ignored in main file">, | ||||
| InGroup<DiagGroup<"pragma-system-header-outside-header">>; | InGroup<DiagGroup<"pragma-system-header-outside-header">>; | ||||
aaron.ballman: Design-wise, do we want this one to be an error instead of a warning? I don't have strong… | |||||
I'd prefer it to be an error, but I wasn't confident that would be accepted (same applies to all your comments re warn vs err). If you're game, I'll make the change. cjdb: I'd prefer it to be an error, but I wasn't confident that would be accepted (same applies to… | |||||
I would prefer errors here -- the goal of this feature is to prevent programmers from doing things we know are going to be a maintenance burden, so I think we should be restrictive here. If users find the restrictions too onerous and have a good reason for us to relax them, we can definitely consider it then. aaron.ballman: I would prefer errors here -- the goal of this feature is to prevent programmers from doing… | |||||
| def pp_pragma_include_instead_not_sysheader : Error< | |||||
I think these diagnostics should say '#pragma clang include_instead' (we're not super consistent about this, but the surrounding single quotes are intended to convey syntactic constructs and so it should probably be valid syntax). aaron.ballman: I think these diagnostics should say `'#pragma clang include_instead'` (we're not super… | |||||
aaron.ballmanUnsubmitted Can you prefix the diagnostic identifiers with err_? We're not super consistent in this file, but it's helpful when reading the use of the diagnostic to know whether it's an error or not. aaron.ballman: Can you prefix the diagnostic identifiers with `err_`? We're not super consistent in this file… | |||||
cjdbAuthorUnsubmitted I've replaced pp with err since pragma implies "preprocessor" and don't really think it adds much more value. err on the other hand is useful? cjdb: I've replaced `pp` with `err` since `pragma` implies "preprocessor" and don't really think it… | |||||
aaron.ballmanUnsubmitted SGTM, thanks! aaron.ballman: SGTM, thanks! | |||||
| "'#pragma clang include_instead' cannot be used outside of system headers">, | |||||
| ShowInSystemHeader; | |||||
O.o... why do we need to show this one in system headers if the diagnostic can only trigger outside of a system header? aaron.ballman: O.o... why do we need to show this one in system headers if the diagnostic can only trigger… | |||||
Mostly because I'm scared that there's some toggle that would accidentally make the diagnostic observable in a system header and wanted to test the behaviour. cjdb: Mostly because I'm scared that there's some toggle that would accidentally make the diagnostic… | |||||
You can remove the ShowInSystemHeader bits now because these are all errors (at least, I'm 99% sure you can remove it). aaron.ballman: You can remove the `ShowInSystemHeader` bits now because these are all errors (at least, I'm… | |||||
| def pp_pragma_include_instead_unexpected_token : Error< | |||||
| "'#pragma clang include_instead' expects '%0' as its next token; got '%1' instead">, | |||||
| ShowInSystemHeader; | |||||
| def pp_pragma_include_instead_system_reserved : Error< | |||||
No need for a new warning -- I think we can use existing diagnostics for this (like err_pp_expects_filename). Also, I think syntax issues should be diagnosed as an error instead of a warning -- we expect a particular syntactic form and it's reasonable for us to enforce that users get it right. aaron.ballman: No need for a new warning -- I think we can use existing diagnostics for this (like… | |||||
Hmm... the syntax is #pragma clang include_instead(header). This is diagnosing the lack of ( or ), not a missing filename. Perhaps there's already a diagnostic for that? cjdb: Hmm... the syntax is `#pragma clang include_instead(header)`. This is diagnosing the lack of `… | |||||
There is: diag::err_expected (and some related ones in DiagnosticCommonKinds.td). aaron.ballman: There is: `diag::err_expected` (and some related ones in DiagnosticCommonKinds.td). | |||||
| "header '%0' is an implementation detail; #include %select{'%2'|either '%2' or '%3'|one of %2}1 instead">, | |||||
| ShowInSystemHeader; | |||||
| def pp_poisoning_existing_macro : Warning<"poisoning existing macro">; | def pp_poisoning_existing_macro : Warning<"poisoning existing macro">; | ||||
Do we need this one or can we use err_pp_file_not_found? aaron.ballman: Do we need this one or can we use `err_pp_file_not_found`? | |||||
| def pp_out_of_date_dependency : Warning< | def pp_out_of_date_dependency : Warning< | ||||
Do we want to give this pragma teeth by making this an error? Right now, an intrepid user can simply ignore the diagnostic, form the reliance on the header, and we've not really solved the problem we set out to solve. Then again, perhaps you'd like that as an escape hatch for some reason? aaron.ballman: Do we want to give this pragma teeth by making this an error? Right now, an intrepid user can… | |||||
| "current file is older than dependency %0">; | "current file is older than dependency %0">; | ||||
| def ext_pp_undef_builtin_macro : ExtWarn<"undefining builtin macro">, | def ext_pp_undef_builtin_macro : ExtWarn<"undefining builtin macro">, | ||||
| InGroup<BuiltinMacroRedefined>; | InGroup<BuiltinMacroRedefined>; | ||||
| def ext_pp_redef_builtin_macro : ExtWarn<"redefining builtin macro">, | def ext_pp_redef_builtin_macro : ExtWarn<"redefining builtin macro">, | ||||
| InGroup<BuiltinMacroRedefined>; | InGroup<BuiltinMacroRedefined>; | ||||
| def pp_disabled_macro_expansion : Warning< | def pp_disabled_macro_expansion : Warning< | ||||
| "disabled expansion of recursive macro">, DefaultIgnore, | "disabled expansion of recursive macro">, DefaultIgnore, | ||||
| InGroup<DiagGroup<"disabled-macro-expansion">>; | InGroup<DiagGroup<"disabled-macro-expansion">>; | ||||
| ▲ Show 20 Lines • Show All 529 Lines • Show Last 20 Lines | |||||
Design-wise, do we want this one to be an error instead of a warning? I don't have strong feelings one way or the other, but making it an error ensures no one finds creative ways to use it outside of a system header by accident (and we can relax the restriction later if there are reasons to do so).