Page MenuHomePhabricator

[mlir] Change to requiring location for infer*
AcceptedPublic

Authored by jpienaar on Feb 27 2021, 7:18 PM.

Details

Summary

Previously it was optional to cater for potential future cases where we'd not
want errors reported while still verifying types. That lead to some
inconvenient usage patterns inside inference function. One can get the same
behavior via the diagnostic handler and given the vast majority of uses has a
location, flip it so that the common case is the easy case.

Diff Detail

Unit TestsFailed

TimeTest
40 msx64 debian > Flang.Semantics::resolve102.f90
Script: -- : 'RUN: at line 1'; /mnt/disks/ssd0/agent/llvm-project/flang/test/Semantics/test_errors.sh /mnt/disks/ssd0/agent/llvm-project/flang/test/Semantics/resolve102.f90 /mnt/disks/ssd0/agent/llvm-project/build/tools/flang/test/Semantics/Output/resolve102.f90.tmp /mnt/disks/ssd0/agent/llvm-project/build/bin/f18 -intrinsic-module-directory /mnt/disks/ssd0/agent/llvm-project/build/tools/flang/include/flang

Event Timeline

jpienaar created this revision.Feb 27 2021, 7:18 PM
jpienaar requested review of this revision.Feb 27 2021, 7:18 PM
rriddle accepted this revision.Mar 1 2021, 10:27 AM

LGTM in general, though one comment about the diagnostic handler.

mlir/test/lib/Dialect/Test/TestPatterns.cpp
103

I don't think we have a general utility for this. We would need to add something like a NullDiagnosticHandler or DiscardingDiagnosticHandler, that only ignores diagnostics from the current thread(or child threads perhaps?).

This revision is now accepted and ready to land.Mar 1 2021, 10:27 AM