typecheck: Load unfoldings lazily
Changes PlannedPublic
Actions

Authored by bgamari on Oct 17 2016, 10:00 PM.

Details

Reviewers

simonpj
austin

Trac Issues

#9370

Summary

This carries out the refactoring proposed in Trac #9370, comment 28.
Previously -fignore-interface-pragmas would disable typechecking of unfoldings
loaded from interface files during deserialization. This would mean that
changes in the state of this flag would not take effect after a module's
interface file has been loaded, leading to unpredictable behavior when
compiling programs which set this flag on a per-module basis.

Here we rework interface file loading such that unfoldings are always loaded,
but type-checked lazily. We then disable inlining in the simplifier when
-fignore-interface-pragmas is set.

Test Plan

Validate

Diff Detail

Repository

rGHC Glasgow Haskell Compiler

Branch

master

Lint

Lint Warnings

Excuse: bah

Severity	Location	Code	Message
Warning	compiler/basicTypes/IdInfo.hs:215	TXT3	Line Too Long

Unit

No Unit Test Coverage

Build Status

Buildable 11453
Build 14193: [GHC] Linux/amd64: Patch building
Build 14192: [GHC] Windows/amd64: Continuous Integration
Build 14191: arc lint + arc unit

bgamari updated this revision to Diff 9050.Oct 17 2016, 10:00 PM

bgamari retitled this revision from to typecheck: Load unfoldings lazily.

bgamari updated this object.

bgamari edited the test plan for this revision. (Show Details)

bgamari added a reviewer: simonpj.

bgamari updated the Trac tickets for this revision.

Herald added a reviewer: austin. · View Herald TranscriptOct 17 2016, 10:00 PM

Herald added a subscriber: thomie. · View Herald Transcript

Fix long lines

Herald edited edge metadata. · View Herald TranscriptOct 17 2016, 10:02 PM

I like that this patch mainly deletes code!

But the ticket has a long discussion, laziness is important, so it all needs a proper Note to explain.

Have you checked that without -O those unfoldings really are not typechecked? In practice.

Is there any perf impace in the no -O case?

Simon

compiler/simplCore/SimplCore.hs
147	This needs a major Note to explain!!! So small, so elegant (if that's really it!), but needs attention drawn to it.

This revision is now accepted and ready to land.Oct 18 2016, 6:35 AM

bgamari added inline comments.Oct 19 2016, 8:34 AM

compiler/simplCore/SimplCore.hs
147	Agreed; I really just wanted to see whether this validated on Harbormaster. There's quite a bit of documentation that needs to be done.

This needs more work.

austin resigned from this revision.Nov 9 2017, 9:45 AM

Revision Contents
Changeset List

			Path	Packages
M			compiler/basicTypes/IdInfo.hs (2 lines)
M			compiler/iface/LoadIface.hs (19 lines)
M			compiler/iface/TcIface.hs (136 lines)
M			compiler/iface/TcIface.hs-boot (4 lines)
M			compiler/simplCore/SimplCore.hs (3 lines)