@swift-ci create

Comment by Nikhil (JIRA)

theindigamer (JIRA User)

Is this for fixing and shall I give it a try.

Interesting. Wow, these diagnostics are pretty old. We also have

error: variable 'test' used before being initialized

which we emit in certain cases. I'm not really sure why the "pass by reference" thing is being mentioned at all. Arguably that's not relevant. Anything that counts as a use is not permitted, which includes passing by reference. Also, we have a note

note: variable defined here 

Maybe that note should be changed to note: 'test' declared here but not initialized.

As for assigning this to yourself, sure feel free to do that. For this one in particular, I don't see what the benefit is for a separate "pass by reference" diagnostic, so I'm in favor of removing it. Moreover, in certain cases, it leads to a duplicate diagnostic, which is not great (from the test suite):

func testOptionalChainingWithGenerics<T: DIOptionalTestProtocol>(p: T) -> T? {
  let x: T? // expected-note {{constant defined here}}
            // expected-note@-1 {{constant defined here}}
            // expected-note@-2 {{constant defined here}}


  // note that here assignment to 'f' is a call to the setter.
  x?.f = 0  // expected-error {{constant 'x' used before being initialized}}
            // expected-error@-1 {{constant 'x' passed by reference before being initialized}}
  return x  // expected-error {{constant 'x' used before being initialized}}
} 

—

Also, this diagnostic is not quite accurate.

func f() -> Int {
  var x: Int
  if (true) {x = 0}
  return x // error: variable 'x' used before being initialized
}

Maybe as a follow up, we should revisit all the definite initialization diagnostics. Rust uses the term "possibly-uninitialized", maybe that's better?

I think the “passed by reference” diagnostic is emitted for inout uses. It would be nice to point out where the inout access is happening (as notes) but I think it would be a little tricky.

Could you explain how the additional "passed by reference" text (or pointing out where the inout access is happening) help someone understand what the issue is, or how to fix it? If I put the two diagnostics side-by-side (even including a hypothetical extension pointing out where the inout access is happening), I think "used before being initialized" conveys the same information ("hey! you need to initialize this before using it here") in fewer words.

Hmm, my understanding was that the "passed by reference" bit was confusing here (not that it's not clear enough that the variable is uninitialized), at least in scenarios where it's happening behind-the-scenes like dictionary access. Normally, you'd see this diagnostic in scenarios like:

func takesInout(_: inout Int) {}
var foo: Int
takesInout(&foo) // variable 'foo' passed by reference before being initialized

There's another one for pointers:

func takesPointer<T>(_: UnsafePointer<T>) {}
var foo: Int
takesPointer(&foo) // address of variable 'foo' taken before it is initialized

Although for something like:

var dict: [String: Int]
dict["a"] = 0 // variable 'dict' passed by reference before being initialized

It's not immediately clear where the "pass by reference" bit is happening (I think it's happening via the synthesised _modify accessor), so I thought a note explaining the inout access would provide some additional information (or it could be in the form of educational notes or pretty-printed rust style diagnostics).

But okay, perhaps all of this extra information is simply unnecessary (as you hinted earlier) and in that case I agree that changing the diagnostic wording would be better!

@theblixguy theindigamer (JIRA User) I'm not really sure but to me this "pass by reference" bit might be happening because for mutating function/subscripts self is implicit passed as an inout parameter, so in the case of dict["a"] = 0, that would be something like subscript(&dict, "a", 0). So at SIL stage where I guess this diagnostic is being provided what is seeing its just an implicit inout access... That would be my guess, but I'm not really sure if that is the case 🙂

Seems like that is indeed the case and a simple change like

// For a use of self argument implicit passed as inout parameter e.g.
// let's just diagnosed as "used before initialized" because "passed by
// reference" might be a confusing diagnostic in this context.
auto diagID = (Use.Kind == DIUseKind::InOutSelfArgument)
                   ? diag::variable_used_before_initialized
                   : diag::variable_inout_before_initialized;

here