[IDE] Disable constraint system diagnostics during code completion #24842
Conversation
|
@swift-ci Please smoke test |
lib/Sema/TypeCheckConstraints.cpp
Outdated
| @@ -2175,7 +2176,8 @@ Type TypeChecker::typeCheckExpressionImpl(Expr *&expr, DeclContext *dc, | |||
| // Construct a constraint system from this expression. | |||
| ConstraintSystemOptions csOptions = ConstraintSystemFlags::AllowFixes; | |||
|
|
|||
| if (options.contains(TypeCheckExprFlags::SuppressDiagnostics)) | |||
| if (options.contains(TypeCheckExprFlags::SuppressDiagnostics) || | |||
| DiagnosticSuppression::isEnabled(Diags)) | |||
There was a problem hiding this comment.
@xedin Do we really need both mechanisms?
There was a problem hiding this comment.
@rintaro Shouldn't enabled diagnostic suppression mean SuppressDiagnostics flag while forming constraint system?
There was a problem hiding this comment.
@xedin
Currently, DiagnosticSuppression does nothing related to TypeCheckExprFlags::SuppressDiagnostics.
@slavapestov
We can drop TypeCheckExprFlags::SuppressDiagnostics and consolidate it to DiagnosticSuppression.
Currently, we have only 2 usage of TypeCheckExprFlags::SuppressDiagnostics, and one of them is already under DiagnosticSuppression.
There was a problem hiding this comment.
@rintaro I think DiagnosticSuppression::isEnabled returning true should correspond to ConstraintSystemFlags::SuppressDiagnostics flag set in ConstraintSystem so we only have one source of input about this in the solver.
There was a problem hiding this comment.
I added another commit to consolidate TypeCheckExprFlags::SuppressDiagnostics with DiagnosticSuppression. Is this what you are suggesting?
There was a problem hiding this comment.
Yes, I think the way you changed it is okay, I just how that we wouldn't need to decouple it again later...
There was a problem hiding this comment.
I'm all in favor of consolidating them. I don't have an opinion on which one is 'better', I just think it's confusing when there are two ways of doing the same thing :)
|
@swift-ci Please smoke test |
| @@ -233,9 +233,6 @@ enum class TypeCheckExprFlags { | |||
| /// that we force for style or other reasons. | |||
| DisableStructuralChecks = 0x02, | |||
|
|
|||
| /// Set if the client wants diagnostics suppressed. | |||
| SuppressDiagnostics = 0x04, | |||
|
@swift-ci Please Build Toolchain macOS Platform |
|
macOS Toolchain Install command |
|
@swift-ci Please Sourcekit Stress test |
It seems disabling diagnstics doesn't introduce any regression. rdar://problem/50679428
TypeCheckExprFlags::SuppressDiagnostics is now done by DiagnosticSuppression.
…ies" This reverts commit c6eade1.
rintaro
force-pushed
the
ide-completion-disableexprdiag-rdar50679428
branch
from
581519e
to
1813202
Compare
|
Added revert commit for #25922. |
|
@swift-ci Please Sourcekit Stress test |
|
@swift-ci Please smoke test |
|
Superseded by #26888 |
It seems disabling diagnostics doesn't introduce any regression.
rdar://problem/50679428