Enable lazy ClangImporter diagnostics by default #40903
Conversation
NuriAmari
force-pushed
the
default-diagnostics
branch
5 times, most recently
from
2a6aa15 to
6726b13
Compare
|
@swift-ci please smoke test. |
|
As discussed offline, I just realized because these tests require objc-interop they are only getting run on macOS. Please add a test that uses C++ interop so they get run on all platforms. You can do something like this: |
NuriAmari
force-pushed
the
default-diagnostics
branch
from
6726b13 to
44fd9c8
Compare
|
@swift-ci please smoke test |
NuriAmari
force-pushed
the
default-diagnostics
branch
from
44fd9c8 to
c236160
Compare
|
@swift-ci please smoke test |
|
@swift-ci please test Windows |
| @@ -1,5 +1,5 @@ | |||
| // RUN: %empty-directory(%t.mcp) | |||
| // RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk) -I %S/Inputs/custom-modules -Xcc -w -typecheck %s -module-cache-path %t.mcp -disable-named-lazy-member-loading 2>&1 | %FileCheck %s | |||
| // RUN: not %target-swift-frontend(mock-sdk: %clang-importer-sdk) -disable-experimental-clang-importer-diagnostics -I %S/Inputs/custom-modules -Xcc -w -typecheck %s -module-cache-path %t.mcp -disable-named-lazy-member-loading 2>&1 | %FileCheck %s | |||
There was a problem hiding this comment.
Why did this test need to be updated?
There was a problem hiding this comment.
It doesn't need updating anymore, but at the time it was because the new diagnostics were tripping a CHECK-NOT. The diagnostic was accurate, and is still there, but now it's emmitted later in a position that does not trip a CHECK-NOT.
The new diagnostic informs about a circularFriends method that can't be imported because parameter type CircularName is unimportable. Not exactly sure of the details, but looks intentional, I can dig deeper if need be.
|
Don't merge this until we resolve the updated test. I also want to understand why these can't be errors anymore. |
Replace the existing `-enable-experimental-clang-importer-diagnostics` flag with an opt-out version entitled `-disable-experimentalc-clang-importer-diagnostics`. Enable the beviour previously hidden behind the old flag by default.
NuriAmari
force-pushed
the
default-diagnostics
branch
from
c236160 to
7ec861f
Compare
|
@swift-ci please smoke test |
| // CHECK-NOT: error | ||
| // CHECK-NOT: note | ||
|
|
||
| // CHECK-NOT: warning |
There was a problem hiding this comment.
TODO: Remove unnecessary CHECK-NOTs
|
@swift-ci please smoke test. |
|
MacOS failure looks unrelated to me. I will complete the TODO comment I have left over above, and will test with errors instead of notes here: #41162. If that goes well, should be ready for merge. |
|
@swift-ci please smoke test macOS |
Replace the existing
-enable-experimental-clang-importer-diagnosticsflag with an opt-out version entitled
-disable-experimentalc-clang-importer-diagnostics.Enable the beviour previously hidden behind the old flag by default.
Implements the changes described in this forum post.