@swift-ci create

Should be a simple matter of printing a TypeLoc instead of just a Type. Tagging this as a Starter Bug. If anybody wants to fix this, reply here for more details.

Comment by Nikhil (JIRA)

I would like to.

Great! So, you'll need to go update the two uses of `diag::codable_non_conforming_property_here` in the compiler, and its diagnostic argument string in DiagnosticsSema.def. In order to fix this bug, we need to make those diagnostics print exactly what the user wrote, so instead of using a Type, we're going to use an syntactic equivalent called a TypeRepr. These are created by the parser from raw source text. But, there's still the possibility we could be dealing with types that are not from a parsed representation - for example, what if we loaded S<T> from a swiftmodule? In that case, we ought to fall back to printing the semantic Type again. If that sounds complex, luckily we have an abstraction in the compiler for dealing with exactly this scenario called a TypeLoc. You give it a (possibly null) TypeRepr and a (usually not null) Type and when it's handed as an argument to a diagnostic, it'll print the parsed representation if it can get it, otherwise it'll print the type. This'll also handle the general case of doing stuff like trying to print a variable whose type is not given because it's inferred from its initializer.

Anyways, look in DiagnosticsSema.def for codable_non_conforming_property_here, and change its argument types to `(Type, TypeLoc)`, then in DerivedConformanceCodable.cpp, you'll need to change both usages of it to take TypeLocs. So, for the second usage on line 356, we want to change this

      varDecl->diagnose(diag::codable_non_conforming_property_here,
                          derived.getProtocolType(), varDecl->getType());

into

        TypeLoc typeLoc = {
          varDecl->getTypeReprOrParentPatternTypeRepr(),
          varDecl->getType(),
        };
        varDecl->diagnose(diag::codable_non_conforming_property_here,
                          derived.getProtocolType(), typeLoc);

Once you've got both callsites updated, ping me here and we can talk about writing tests.

Comment by Nikhil (JIRA)

@CodaFi

I have fixed it like you suggested.

1. I have changed def and cpp files,

2. For tests, I ran grep on tests folder and it gave me some results.

   grep -rn "cannot automatically synthesize" test | grep "does not conform"

   Thanks.

Comment by Nikhil (JIRA)

I have added this PR #32611 and I can now successfully debug. I also get the below error now.

cannot automatically synthesize 'Decodable' because 'T!' does not conform to 'Decodable' 
      var s: T! 
             ^
cannot automatically synthesize 'Encodable' because 'T!' does not conform to 'Encodable'
   var s: T!
          ^

That looks correct. Please mention "Fixes [rdar://problem/64953106]. Fixes SR-13119." (for cross-referencing) in the PR description when you open your PR with the fix. 🙂

Comment by Nikhil (JIRA)

theindigamer (JIRA User)

I am trying to run tests locally. I get below error for any test I run using lit.

~/Code/swift-source/build/Xcode-RelWithDebInfoAssert/swift-macosx-x86_64/test-macosx-x86_64/Parse/Output/weakLinked.swift.script: line 1: swift-frontend: command not found

srinikhil07 (JIRA User), I suspect the issue might be that you haven't updated your compiler recently. You might need to rebase on top of recent changes. The swift-frontend target was newly added IIUC. I don't think this issue is related to this particular bug report; so it would be generally better to start a Swift forums thread in the Development:Compiler category to discuss issues encountered when building/running tests, so that (a) other people can see the post and try to help out and (b) it can more easily be discovered by someone else in case they hit the same issue.

Comment by Nikhil (JIRA)

theindigamer (JIRA User)

Thanks for the direction, I did a search in forums and I came across this discussion https://forums.swift.org/t/running-swift-standard-library-tests/17230/9

Seems with Xcode build alone lit doesn't work and we need ninja. If so, I think we can state this explicitly in docs.

Comment by Nikhil (JIRA)

theindigamer (JIRA User)

I ran the tests for 'codable' with lit using below command:

swift-source/llvm-project/llvm/utils/lit/lit.py -s -vv swift-source/build/Ninja-ReleaseAssert/swift-macosx-x86_64/test-macosx-x86_64 --filter "codable"

I got 5 failed tests:

Two tests are due to 'AnyHashable?' in tests we we now print as 'AnyHashable!'. I corrected them,

Two tests are for nested class/struct(e.g.,test/decl/protocol/special/coding/struct_codable_failure_diagnostics.swift), where the original vs new error is as below. Please suggest if the new diagnostic is okay or should we change anything?

cannot automatically synthesize 'Encodable' because 'C1.Nested' does not conform to 'Encodable'

cannot automatically synthesize 'Encodable' because 'Nested' does not conform to 'Encodable'

One in test/decl/protocol/special/coding/struct_codable_simple.swift, the difference is as below. Please suggest if this is also okay or any changes needed.
cannot automatically synthesize 'Encodable' because '<<error type>>' does not conform to 'Encodable'
cannot automatically synthesize 'Encodable' because 'Int' does not conform to 'Encodable'

  // rdar://problem/59655704  
  struct SR_12248_1: Codable { // expected-error `type 'SR_12248_1' does not conform to protocol 'Encodable'`  
      var x:Int// expected-note {{'x' previously declared here}}  
      var x:Int// expected-error {{invalid redeclaration of 'x'}}  
     // expected-note@-1 {{cannot automatically synthesize 'Encodable' because '\<\<error type\>\>' does not conform to 'Encodable'}}  
     // expected-note@-2 {{cannot automatically synthesize 'Encodable' because '\<\<error type\>\>' does not conform to 'Encodable'}}  
  }  

Also, since our case is covered already should I write a new test case?

 \>  One in test/decl/protocol/special/coding/struct_codable_simple.swift, the difference is as below. Please suggest if this is also okay or any changes needed.

Hey srinikhil07 (JIRA User) for the last <<error type>> diagnostics we have a PR open for that #32371

So I think you don't need to worry too much about that one 🙂

cc @CodaFi

> Two tests are for nested class/struct(e.g.,test/decl/protocol/special/coding/struct_codable_failure_diagnostics.swift), where the original vs new error is as below. Please suggest if the new diagnostic is okay or should we change anything?

One possible change here might be to check if there is an IUO. Based on that, call Type::getString with PrintOptionalAsImplicitlyUnwrapped field set to true or false (cc @hborla, she suggested using this field). Unlike TypeRepr, Type will print out the fully qualified thing, so it will avoid this regression.

Comment by Nikhil (JIRA)

theindigamer (JIRA User)

Changes done. Will raise the pull request.

Comment by Nikhil (JIRA)

theindigamer (JIRA User)

Please review the pull request #32987

theindigamer (JIRA User) Now that the PR has been merged, could you please verify the fix and mark this ticket as Resolved? Thanks!

Thank you for the reminder, done.

Btw, Open -> Resolved is generally done by the person who fixed the issue (or at least, that's the intention), and Resolved -> Closed is intended for the person who verifies the fix (likely the person who reported the issue, in this case me).