[clang] clang: Relax LangOpts checks when lexing quoted numbers during preprocessing (PR #95798)

Jan Svoboda via cfe-commits cfe-commits at lists.llvm.org
Wed Jul 3 09:08:53 PDT 2024


================
@@ -2068,7 +2068,8 @@ bool Lexer::LexNumericConstant(Token &Result, const char *CurPtr) {
   }
 
   // If we have a digit separator, continue.
-  if (C == '\'' && (LangOpts.CPlusPlus14 || LangOpts.C23)) {
+  if (C == '\'' &&
+      (LangOpts.CPlusPlus14 || LangOpts.C23 || ParsingPreprocessorDirective)) {
----------------
jansvoboda11 wrote:

> But I think it's a stretch to argue that the standard mode tracked by `LangOptions` is not exposed via flags.

Not sure I understand. I'm suggesting that in the future, parts of the lexer behavior could be controlled in a way that's hidden from users. And I'm saying that we are already hiding existing `LangOptions` fields from users (e.g. `LangOptions::CompilingPCH`).

> I don't think per-feature `LangOptions` is viable. There's digit separators, hex float support, binary literal support, different literal suffixes, various keywords, etc and that doesn't seem likely to scale well.

This doesn't need to scale. We'd only do this for options that control lexing. FWIW, code that says "literal digit separators are enabled in C++14+, C23+, or when explicitly allowed" seems more logical to me than checking `ParsingPreprocessorDirective`, which doesn't really explain why we're suddenly allowing digit separators.

> Also, it makes it seem like we support a la carte language features when we don't -- we intentionally don't want to let users disable standard features (in general; exceptions exist) and so that approach gives the impression contributors need to check both language mode and feature availability which is a maintenance concern.

I think documenting the cases where we introduce this extra knob should pretty much clear up any confusion and prevent contributors cargo culting that approach to options that don't need it.

> I'm still wondering about the question I asked here: [#93753 (comment)](https://github.com/llvm/llvm-project/pull/93753#issuecomment-2173666602)

Are there any comments that weren't addressed by [my reply](https://github.com/llvm/llvm-project/pull/93753#issuecomment-2204578263)?

https://github.com/llvm/llvm-project/pull/95798


More information about the cfe-commits mailing list