keepyixiao wrote: Alternative approaches Option 1: Change BuiltinConstantArgRange Change the early return to:
```c++ if (isConstantEvaluatedContext() && !RangeIsError) return false; ``` This allows range errors to be diagnosed directly by Sema in constant-evaluated contexts. Advantages: - Produces the same direct Sema diagnostic in all evaluation contexts. - Diagnoses invalid arguments even when constant evaluation does not visit the call, for example due to short-circuiting. Disadvantages: - Changes the behavior of a shared helper used by many unrelated builtins. - Can alter diagnostic ordering before builtin-specific conversions and checks have completed. - In particular, it caused __builtin_isfpclass to emit premature “must be a constant integer” errors, masking the expected incompatible-type diagnostics. - Therefore, it has a larger and less predictable impact. Option 2: Diagnose the failure in the constant evaluators Keep the original Sema behavior and replace the assertions in both the legacy and ByteCode constant evaluators with a constant-evaluation failure and a range diagnostic. Advantages: - Directly addresses the assertion failure at the point where it occurs. - Limits the behavior change to __builtin_object_size and __builtin_dynamic_object_size. - Does not affect other users of BuiltinConstantArgRange. - Preserves existing builtin-specific conversions and diagnostic ordering. Disadvantages: - Both constant evaluators need equivalent checks. - The diagnostic consists of the general constant-expression error followed by a specific note. - An invalid call that is not actually evaluated, such as one skipped by short-circuiting, is not diagnosed through this path. Rationale Option 2 was chosen because changing BuiltinConstantArgRange caused observable regressions in unrelated builtins, particularly __builtin_isfpclass. The evaluator-local approach fixes the crash with a much narrower behavioral impact while preserving the existing Sema diagnostic behavior for other builtins. https://github.com/llvm/llvm-project/pull/230979 _______________________________________________ cfe-commits mailing list [email protected] https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
