Wiki

Ticket #152 (assigned defect)

Opened 9 years ago

Last modified 5 years ago

Automatic library referencing from "use" does not always work

Reported by: Chuck Owned by: Chuck
Priority: major Milestone: Cobra 0.9
Component: Cobra Compiler Version: 0.8.0
Keywords: use, dll Cc:

Description

The following program gives an error if you comment out the "args" directive:

use System.Windows.Forms
use System.Drawing

%% args -ref:System.Drawing

class Program

    def main is shared
        print Color.red
  • error: The type or namespace name "Color" does not exist in the namespace "System.Drawing" (are you missing an assembly reference?)

I believe the problem is that

  • "use System.Windows.Forms" automatically refs System.Drawing
  • then "use System.Drawing" doesn't fault the System.Drawing.dll reference in
  • so Cobra fails to pass an /r: onto C#.

See also:  discussion

Attachments

ticket152.patch Download (4.0 KB) - added by eric.sellon 8 years ago.
implicit-dependency-refs.patch Download (6.8 KB) - added by hopscc 5 years ago.

Change History

Changed 8 years ago by eric.sellon

  • status changed from new to assigned
  • owner set to eric.sellon

Just started to look at this, but I found if you reverse the order of the uses statement, you don't need the args directive. Seems to give weight to theory put forth in the description.

Changed 8 years ago by Chuck

Yeah, but what's the fix? :-)

Changed 8 years ago by eric.sellon

Soon as I figure it out, I'll let you know (and post a patch file, of course) ;-)

Changed 8 years ago by eric.sellon

The attached patch file contains a fix for the issue and a test case that is the example provided in the description. This patch will cause refs to be generated for all (possible) dependent dlls. Also it can cause a dll to be refed multiple times on the command line for the C# back end. The size of the generated executable is the same before and after the patch (this was tested with both MS's and Mono's compiler).

Changed 8 years ago by Chuck

I'm currently unable to apply this patch because when I do, the WPF programs fail. They are in:

<workspace>\Tests\720-libraries\100-microsoft
<workspace>\HowTo

The new errors caused by the patch are:

    ...\720-libraries\100-microsoft\200-wpf.cobra
    Test #668
error: Metadata file "WindowsBase.dll" could not be found
error: Metadata file "UIAutomationTypes.dll" could not be found
error: Metadata file "UIAutomationProvider.dll" could not be found
error: Metadata file "PresentationCFFRasterizer.dll" could not be found
error: Metadata file "PresentationCore.dll" could not be found
error: Metadata file "ReachFramework.dll" could not be found
error: Metadata file "System.Printing.dll" could not be found
error: Metadata file "PresentationFramework.dll" could not be found
error: Metadata file "PresentationUI.dll" could not be found
Compilation failed - 9 errors, 0 warnings

Please be sure that all test cases pass (or at least no new ones fail) before submitting a patch.

Thanks.

Changed 8 years ago by eric.sellon

Uh-oh. I'll look into it.

Changed 8 years ago by eric.sellon

Changed 8 years ago by eric.sellon

Patch file updated. No new tests failing this time.

Changed 8 years ago by Chuck

  • owner changed from eric.sellon to Chuck

Looking at this now.

Changed 8 years ago by Chuck

  • status changed from assigned to closed
  • resolution set to fixed

Applied in changeset:2126

Thanks!

Changed 8 years ago by Chuck

  • status changed from closed to reopened
  • resolution fixed deleted

It turns out that the patch broke some test cases on the Mono side. My bad for not testing and detecting this before applying the patch.

Also, the approach of this patch, which includes (a) using absolute paths to assemblies in the GAC (I think that was in this patch) and (b) always trying to find a Foo.Bar.dll for a "use Foo.Bar", ... are not compatible with the fixes required for the -native-compiler option bug reported at  http://cobra-language.com/forums/viewtopic.php?f=4&t=501

The patch is largely still in place in the code, but I made some tweaks which fix the above two problems, but reopen this ticket. In Source/Phases/BindUsePhase?.cobra is a comment starting with "2009-08 CE:" with some further explanation.

I'm convinced we will not be able to go that route at all (always looking for an assembly for every "use"). My current thinking is that we need to record when a reference was brought in through dependencies or not. If a "use" binds to an assembly that was loaded indirectly, then the assembly reference needs to become explicit. Not that this is easy or we would have done it already.

For reference, the first tests to try when working on this are:

testify ..\Tests\110-basics-two\500-namespaces ..\Tests\720-libraries

Of course, the final solution must pass all tests on both .NET and Mono. If one can only test one of these, a patch can still be submitted; just make note of it.

If no one gets to this, I will eventually.

Changed 8 years ago by hopscc

This test is failing in latest pull on an otherwise clean tree

Tests/720-libraries/400-other/211-use-lib-with-no-ref-special-order.cobra

Is that intentional???

$ ./cobra  -ref:System.Windows.Forms ../Tests/720
-libraries/400-other/211-use-lib-with-no-ref-special-order.cobra
c:\home\hops\src\cobra\wkspace\Tests\720-libraries\tests\720-libraries\400-other\211-use-lib-with-no-ref-special-order.cobra(14): error: The type or namespace name "Color" does not exist in the namespace "System.Drawing" (are you missing an assembly reference?) (C#)
Compilation failed - 1 error, 0 warnings

Changed 5 years ago by hopscc

Changed 5 years ago by hopscc

  • status changed from reopened to assigned

Heres a patch that corrects the above failure - case above added as a new test.
It also fixes the failing test

\720-libraries\400-other\500-follow-dependencies\104-use-b.cobra

Thats been failing since the last patch for this was added....

Changed 5 years ago by Charles

  • owner changed from Chuck to hopscc

When I applied the new patch, implicit-dependency-refs.patch, the test suite had +2 failures. This was on Mono 3.0.6 on Mac OS X (and I would expect the same results on Linux since Mono's behavior doesn't vary much between them).

Both test failures are due to new unexpected warnings. The tests are:

Tests/720-libraries/100-microsoft/500-misc-designer.cobra
warning: Already have declaration "ResXResourceSet" in namespace "System.Resources".
warning: Already have declaration "TypedDataSetGenerator" in namespace "System.Data.Design".

Tests/720-libraries/400-other/600-nhibernate/100-use-iquery-over.cobra
warning: Already have declaration "ResXResourceSet" in namespace "System.Resources".
warning: Already have declaration "ChallengeResponse" in namespace "Mono.Security.Protocol.Ntlm".
warning: Already have declaration "MessageBase" in namespace "Mono.Security.Protocol.Ntlm".
warning: Already have declaration "NtlmFlags" in namespace "Mono.Security.Protocol.Ntlm".
warning: Already have declaration "Type1Message" in namespace "Mono.Security.Protocol.Ntlm".
warning: Already have declaration "Type2Message" in namespace "Mono.Security.Protocol.Ntlm".
warning: Already have declaration "Type3Message" in namespace "Mono.Security.Protocol.Ntlm".
warning: Already have declaration "CipherAlgorithmType" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "ExchangeAlgorithmType" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "HashAlgorithmType" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "SecurityCompressionType" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "SecurityProtocolType" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "CertificateValidationCallback" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "ValidationResult" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "CertificateValidationCallback2" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "CertificateSelectionCallback" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "PrivateKeySelectionCallback" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "SslClientStream" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "SslServerStream" in namespace "Mono.Security.Protocol.Tls".
warning: Already have declaration "SslStreamBase" in namespace "Mono.Security.Protocol.Tls".

Changed 5 years ago by hopscc

  • owner changed from hopscc to Chuck

Tests clean on my system - Windows(7)

Cobra svn:2955 (post 0.9.3) / 2013-04-27 on .NET CLR v2.0.50727 on Microsoft Win
dows NT 6.1.7601 Service Pack 1

Below
'patchp' - patch wrapper run on new clean source tree
'mkcobc' is shell script wrapper around cobra compile of the compiler sources
'mktests' is script wrapper around ./cobra -testify invocation

548 xx:...src/cobra/wkspace1> patchp ../patches-sent/implicit-dependency-refs.patch
patching file Source/Compiler.cobra
patching file Source/NameSpace.cobra
patching file Source/BackEndClr/SharpGenerator.cobra
patching file Source/BackEndClr/ScanClrType.cobra
patching file Tests/510-implicit-dependency.cobra
549 xx:...src/cobra/wkspace1>
549 xx:...src/cobra/wkspace1> cd Source/
550 xx:...wkspace1/Source> mkcobc
Compilation succeeded
timeit compile = 00:00:11.2897890
53427 lines compiled at 4732.3 lines/sec
157336 nodes compiled at 13936.1 nodes/sec
287818 tokens compiled at 25493.7 tokens/sec
551 xx:...wkspace1/Source>
551 xx:...wkspace1/Source>
551 xx:...wkspace1/Source> mktests ../Tests/720-libraries/100-microsoft/
#Note: for test subsets just specify dirname rather than dirname/*.cobra
..\Tests\720-libraries\100-microsoft\
    (0) 100-winforms.cobra
    (1) 110-winforms-native-compiler.cobra
    (2) 120-winforms.cobra
    (3) 200-wpf.cobra
    (4) 300-system-web.cobra
    (5) 310-system-io.cobra
    (6) 320-system-servicemodel.cobra
        Skipping test because of directive. "Need to be able to require CLR 4"
    (7) 400-xml.cobra
    (8) 410-xml-linq.cobra
    (9) 500-misc-designer.cobra
    (10) 510-dll-import.cobra

Finished at 3/05/2013 7:11:19 p.m.
10 Tests in 00:00:17.1549812.

Success.
timeit total   = 00:00:17.1596931
552 xx:...wkspace1/Source> mktests ../Tests/720-libraries/400-other/600-nhibernate/
#Note: for test subsets just specify dirname rather than dirname/*.cobra
..\Tests\720-libraries\400-other\600-nhibernate\
    (0) 100-use-iquery-over.cobra

Finished at 3/05/2013 7:12:45 p.m.
1 Tests in 00:00:04.2322421.

Success.
timeit total   = 00:00:04.2370992
553 xx:...wkspace1/Source>

Its something specific to mono....

Changed 5 years ago by hopscc

FWIW Didnt see this message at all for anything while generating/testing this patch.

Things to try (off the top of my head)
Turn on the debugging for library loading and see if any of those reffed libs are being doubly loaded
- there should be check gates against that but perhaps we're missing one

dump a stack trace at the warning - whats the trigger for that load, Where has it been loaded previously? (probably from an implicit reference - can you determine what that was? perhaps the following load needs a better check gate?)

dump the decls at the point of warning - are they complete?
If so theres an implicit ref doing the loading and the follow on call is superfluous
and should be suppressed (or ignored/escaped)

The warning is from a library load as an integrity check - perhaps rather than warning it just needs to silently return - unless verbose??? (:-)

Changed 5 years ago by Charles

Thanks for the follow up.

Note to self: workspace-s

Note: See TracTickets for help on using tickets.