aaron.ballman added inline comments.
================
Comment at: clang/lib/Sema/SemaExprCXX.cpp:8483
+ if ((SemaRef.getLangOpts().CPlusPlus && Trap.hasErrorOccurred()) ||
+ Res.isInvalid())
return ExprError();
----------------
efriedma wrote:
> I'd prefer not to introduce unnecessary differences in C vs. C++ handling
> like this...
>
> This seems weird for two reasons:
>
> - We don't use ext_typecheck_convert_pointer_int in C++, so there's not
> really any reason to mark it SFINAEFailure
> - Whatever code is calling this probably doesn't want an SFINAE context. Is
> this coming from TransformToPotentiallyEvaluated? We don't want SFINAE traps
> from there, in C or C++.
> We don't use ext_typecheck_convert_pointer_int in C++, so there's not really
> any reason to mark it SFINAEFailure
The original changes came from:
https://github.com/llvm/llvm-project/issues/39709 but I see now that we used to
treat this as a warning in C++ rather than an error. So adding SFINAEFailure
made sense at that point, but I think you're right that it should be removed
now. I'll try that locally and see if anything explodes as a result.
> Whatever code is calling this probably doesn't want an SFINAE context. Is
> this coming from TransformToPotentiallyEvaluated? We don't want SFINAE traps
> from there, in C or C++.
Typo correction went haywire. Specifically:
https://github.com/llvm/llvm-project/blob/main/clang/test/Sema/typo-correction-ambiguity.c#L13
We would typo correct `v_231` to be `v_2_0` here, and when deciding whether
that was valid or not, we'd hit the SFINAE trap in C code and that changed the
behavior of the test to add suggestions instead of silence them.
I think it's incredibly weird to ever use SFINAE in C, even by accident.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D129881/new/
https://reviews.llvm.org/D129881
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
