Fork me on GitHub
#cljs-dev
<
2020-11-08
>
mhuebert13:11:04

I’ve updated these tests with 4 cases - https://github.com/clojure/clojurescript/compare/master...mhuebert:cljc-macros-require-test observed behaviour: • failure (expected): a cljc macro namespace :require’s another namespace containing macros, which does not “self-require”. when the macro is used, there is no resolve warning & the macro function is called without arguments being correctly passed to it (see wrap-1). Seems reasonable that this does not work - the compiler has no way to know that the ns has macros, unless :require-macros is called somewhere, so a $macros ns is never created. though perhaps should warn? • failure (bug?): a cljc macro namespace :require-macro’s another namespace containing macros. consuming the macro always fails, does not matter if the other ns self-requires. (see wrap-2). error occurs when trying to resolve the alias.. does namespace resolution in syntax-quote follow :require-macros aliases? this may be a bug? • success: a cljc macro namespace :require’s another namespace, which self-requires. (see wrap-3, and wrap-4 for a variant with both cljs + cljc files)

mhuebert13:11:01

additionally it looks like alias resolution fails in more cases in shadow’s bootstrap build (cc @U05224H0W) - I added a couple failing examples here, including the issue with reagent macros: https://github.com/mhuebert/shadow-bootstrap-example/blob/1d5cadbb013660b4e0bea80a8ee9ea4a544ec2ab/src/shadow_eval/core.cljs#L32

thheller16:11:28

@mhuebert the tests you are doing all start at the same time so they are all racing against each other. you need to ensure that one eval completes before the next one runs. at least that would eliminate some of the randomness?

mhuebert16:11:44

I think I’ve eliminated the randomness by using a different namespace for each test

mhuebert16:11:34

The cause is how cljs ns forms work, different from clj I believe

thheller16:11:51

no I mean that all the eval-str calls are kicked off in the same cycle

thheller16:11:14

so depending on when they go async or even if they go async the order they complete in is more or less random

thheller16:11:45

so each eval-str should only the done once the previous completes

thheller16:11:22

at least thats what looks suspicious to me about the tests. whether or not that actually affects anything I do not know but the async behaviour is "bad"

mhuebert16:11:30

Right, but the order shouldn’t matter if operating on different namespaces, or otherwise not interacting? I just based it on how the cljs self host tests are set up

mhuebert16:11:14

I agree it would be cleaner to have them run sequentially

thheller16:11:54

well at least from a theory perspective I can see how one eval sees that a namespace is not yet loaded and kicks off the async load. the next eval does the same ending up in loading the namespace twice and so on

thheller16:11:59

its just one thread but that doesn't mean things can't get weird at times 😉

mhuebert16:11:12

Yeah. I’ll rejig them to be sequential

mhuebert16:11:07

I don’t think any of these tests are anymore tho

thheller16:11:06

probably better to have one deftest for each instead of the latch thing?

mhuebert22:11:32

i also updated the mhuebert/shadow-bootstrap-example project to use a queue