This is going to break CI until relevant libraries are updated. What's the protocol on that?

While you're at it, would you also take a look at Language.PureScript.Ide.Prim? If it's not clear what's happening don't worry about it and I'll fix it.

I think having them explicitly imported is perhaps best; I don't think these classes are quite ubiquitous enough to justify being imported by default. Having to import them explicitly was what Phil suggested originally too. What do others think?

Actually following from my other comment (#2903 (comment)), having thought about it a little more, I think the best approach would be to move all of Fail, Warn, TypeConcat and TypeString to Prim.TypeErrors.

I think the idea is that we decide that pretty much everything apart from the most ubiquitous stuff belongs in submodules of Prim, and only Prim is imported automatically. Also, Prim itself should change very rarely. This gives us the freedom to be able to add more kinds/types/classes into submodules of Prim in the future without worrying about breaking people's code (e.g. if someone has already created a class called Warn, we want them to be able to upgrade the compiler to a new version including a compiler-magic class of the same name without having to change their imports).

I think what we've done in the past with CI is to change tests/support/bower.json to point at a branch of the libraries which need to change during development. Then, just before we make the compiler release, we publish new versions of all the core libraries, update tests/support/bower.json here, and check it all still works.

We might have also used release candidates for libraries, i.e. publishing versions of the form vN.0.0-rc.1, so that we can publish versions which work with upcoming compiler releases, but when people run bower install purescript-whatever they still get the version which works with the current compiler release series.

I can't remember which of these two approaches tends to work better though; @garyb?

@kritzcreek I'll give that a peek, thanks for the suggestion 😄

@hdgarrood Cool, I'll make the requisite changes in the downstream libraries and remove the non-Prim classes from the default environment.

That leads to another question: How do I make a module available without it being explicitly defined?

I think putting it in the Environment like we already have done for Prim should do that; IIRC there's some separate code during the desugaring phase which injects a Prim import for modules which don't already have an explicit one.

Right now the tests all pass locally for me, because Prim.Row.Cons is in scope thanks to it being in the primTypes in the environment. I may be mistaken though

I know one instance of Prim being added is inside Language.PureScript.Make.make:

buildModule barriers (importPrim m) (deps `inOrderOf` map getModuleName sorted)

Ah right, okay; in that case, I'm probably wrong, and I'm not quite sure how to do it.

I figured out how to make "virtual modules" but I'm not sure how to get the imports actually working yet.

I've updated the tests, and they're green (on my machine) 😄

well, the tests are green on my computer :\

Failing test screenshot because it's annoying to dig through the output in CI:

This fails locally for me in the same way.

It doesn't fail for me locally, either on my Mac or Windows machine. I tried stack clean just in case, also cleared out the test support node/bower files, still works (did that on both machines too). 🤔

Travis cache maybe? Not sure why it would fail for Liam too though.

I'm doing: rm -rf tests/support/bower_components .psci_modules .test_modules; stack clean; stack test --fast. Checked both: this work by itself; and also pulled into master. All the same results.

Do you see this too?

Please note that,
    purescript-test-suite-support, purescript-typelevel-prelude#d029d9041a depends on purescript-symbols#compiler/0.12 which resolved to purescript-symbols#3b9236e8b0
    purescript-generics-rep#5.1.0 depends on purescript-symbols#^3.0.0 which resolved to purescript-symbols#3.0.0
Resort to using purescript-symbols#compiler/0.12 which resolved to purescript-symbols#3b9236e8b0
Code incompatibilities may occur.

Please note that,
    purescript-test-suite-support depends on purescript-lists#compiler/0.12 which resolved to purescript-lists#f2f3ec15c6
    purescript-generics#4.0.0 depends on purescript-lists#^4.0.0 which resolved to purescript-lists#4.12.0
Resort to using purescript-lists#compiler/0.12 which resolved to purescript-lists#f2f3ec15c6
Code incompatibilities may occur.

I'm wondering if there is a difference in behaviour across bower versions? (I'm clutching at straws here).

bower: 1.8.0
node: 8.9.4
npm: 5.6.0
stack: 1.6.3 (hpack-0.20.0)

I get those messages, yeah. And:

bower: 1.8.2
node: 8.8.1
npm: 5.4.2
stack: 1.6.3 (hpack-0.20.0)

Running the command you posted gives me the test failure also. Huh. 🤷‍♂️

Versions:

λ bower --version
1.8.2
λ node --version
v6.11.4
λ npm --version
3.5.2
λ stack --version
Version 1.6.5, Git revision 24ab0d6ff07f28276e082c3ce74dfdeb1a2ca9e9 (5514 commits) x86_64 hpack-0.20.0

Ubuntu 17.10 if that makes any difference.

I forgot about test_modules, clearing that out breaks it for me too 😄

hmm, this is weird, as I have not touched any of the Entailment code where this is getting errors :\

We're getting "no instance found" errors. This appears to be specific to the recursion in the instance chain.

I changed the order of instances in the instance context for the second case. This puts 'Balanced sym2' before the others.

instance balanced1 :: Balanced ""
else
instance balanced2
  :: ( Balanced sym2
     , AppendSymbol "(" sym1 sym
     , AppendSymbol sym2 ")" sym1
     ) => Balanced sym

The error then becomes this:

Tests
  compiler
    Passing examples
      'AppendInReverse.purs' should compile and run without error: FAIL (0.09s)
        Error found:
        in module Main
        at /home/matt/Projects/purescript/examples/passing/AppendInReverse.purs line 32, column 6 - line 32, column 42
        
          No type class instance was found for
                            
            Main.Balanced t1
                            
          The instance head contains unknown type variables. Consider adding a type annotation.
        
        while applying a function balanced
          of type Balanced t0 => SProxy t0 -> String
          to argument SProxy
        while checking that expression balanced SProxy
          has type String
        in value declaration b3
        
        where t0 is an unknown type
              t1 is an unknown type
        
        See https://github.com/purescript/documentation/blob/master/errors/NoInstanceFound.md for more information,
        or to contribute content related to this error.

1 out of 1 tests failed (0.09s)

It looks like it is having a hard time doing instance lookup recursively in the presence of an instance chain.

It looks like it is having a hard time doing instance lookup recursively in the presence of an instance chain.

FWIW I have run into similar-seeming problems in different contexts, where switching instance order, or even adding redundant entailments, makes the problem go away. I imagine someone must have filed a bug at some point.

Ah! The previous version of the test suite used this branch:

https://github.com/purescript/purescript-typelevel-prelude/compare/phil/append-symbol

Looks like this should be merged into compiler/0.12 branch on purescript-typelevel-prelude. @LiamGoodacre I think that should fix it 😄

now that the PR in purescript-typelevel-prelude was merged, I've pointed the tests to that commit. they pass locally. let's hope the CI agrees 😄

I opened a PR on your PR that removes the State stuff again, and the tests still pass - the change that enables this is to always populate Environment with allPrimTypes and allPrimClasses - which is fine as accessing those things in Environment would be guarded by the fact code runs through name desugaring first.

Hopefully my next contribution will be more straightforward 😄

Yeah, sorry, I should have gotten involved with reviewing this , etc. a long time ago too.