Check type for forward reference definition

[mjp41]

Summary

The types of forward references are checked that they match with other
uses, but they do not check they match with the definition.

func @forward_reference_type_check() -> (i8) {
  br ^bb2

^bb1:
  return %1 : i8

^bb2:
  %1 = "bar"() : () -> (f32)
  br ^bb1
}

Would be parsed and the use site of '%1' would be silently changed to
'f32'.

This commit adds a test for this case, and a check during parsing for
the types to match.

[rriddle]

mlir/lib/Parser/Parser.cpp

3630

nit: previously used here with type

[mjp41]

Updated error message inline with feedback.

[rengolin]

@rriddle, do you know what's failing in the test build?

The page isn't very clear on what the problem is and the Jenkins URL requests user/passwd.

I've checked clang-format and ninja check-mlir and everything is fine. I was just about to commit...

Are we treating Harbourmaster's failures as blockers, or is that experimental?

cheers,
--renato

[mehdi_amini]

I can't tell what is failing from the log, I filed: https://github.com/google/llvm-premerge-checks/issues/176

[rengolin]

Ok, turns out the failure was a flaky test due to parallelism (CMake deps not right?), but the build finishes and the tests all pass.

I've pushed the change (5010b5b7e6c), as it has been approved and passes all test.

We'll discuss about the issues with the pre-merge tests in separate.

Thanks everyone!

[mehdi_amini]

Thanks for landing it @rengolin ; FYI we document now to not use "Patch by" but to use git commit --amend --author="John Doe <[e-mail]> to have the proper commit author recorded, see https://llvm.org/docs/DeveloperPolicy.html#commit-messages

[rengolin]

In D79317#2023267, @mehdi_amini wrote:

FYI we document now to not use "Patch by" but to use git commit --amend --author="John Doe <[e-mail]> to have the proper commit author recorded, see https://llvm.org/docs/DeveloperPolicy.html#commit-messages

Sorry, old habit. Will do that onward. :)

mlir/lib/Parser/Parser.cpp

           .append("previously defined here");
     }
 
+    if (existing.getType() != value.getType())
+    {
+      return emitError(useInfo.loc)
+        .append("definition of SSA value '", useInfo.name, "#", useInfo.number,
+                "' has type ", value.getType())
+        .attachNote(getEncodedSourceLocation(entries[useInfo.number].second))
+        .append("previously specified type ", existing.getType());
+    }
+
     // If it was a forward reference, update everything that used it to use
     // the actual definition instead, delete the forward ref, and remove it
     // from our set of forward references we track.

mlir/test/IR/invalid.mlir

   // expected-error @+1 {{duplicate key in dictionary attribute}}
   "foo.op"() {a, a} : () -> ()
 }
+
+// -----
+
+func @forward_reference_type_check() -> (i8) {
+  br ^bb2
+
+^bb1:
+  // expected-note @+1 {{previously specified type 'i8'}}
+  return %1 : i8
+
+^bb2:
+  // expected-error @+1 {{definition of SSA value '%1#0' has type 'f32'}}
+  %1 = "bar"() : () -> (f32)
+  br ^bb1
+}