Roundhouse Cleanup 02

[mgreter]

Rebased some cleanup refactorings I had laying around for over half a year now.
Should not change any functionality. Splitting AST into different compile units should
reduce RAM pressure when compiling (e.g. some people reported issues when compiling
libsass on memory restricted environments). @glebm it might be possible that I regressed
some clang warning you've fixed, I'll keep an eye on it once CI results come back.

Further PRs will follow shortly ...

[mgreter]

Note: we may should keep an eye out for performance regressions due to missing inline opportunities for the compiler. But I think we should not pre-optimize and have everything implemented in the header files. Only once we can prove that certain implementations really profit from being in the headers. This split should also slightly improve compile time since the compiler does not have to parse the heavy headers for every compile unit and we could still provide a "unified compile unit" (where all sources/headers are included in one big compile unit, IMO there was some discussion a few years ago about this...).

[mgreter]

OK, seems this does produce quite a few clang warnings ... time to fire up my linux VM 😔

[mgreter]

Hopefully clang and MSVC are happy now ... waiting for CI ...

[glebm]

Looks like this reverted 2a4935c

I wonder if there are others

[mgreter]

It might be possible that due to the rebase it will come in a later PR (I remember that I manually re-added this change to some commit). But thx for checking, I will check myself again

[mgreter]

Those previous changes should be part of this PR now too!

[glebm]

Does make test_probe return the same result as before?

[mgreter]

Does make test_probe return the same result as before?

Wasn't this spec test already merged/activated!?

[glebm]

Well, make test_probe returns a list of over a dozen specs that used to fail but now pass on master. We should make sure it's the same list.

[mgreter]

Well, make test_probe returns a list of over a dozen specs that used to fail but now pass on master

sass/sass-spec#1312 ??

[glebm]

Ah, I haven't pulled from there for a while!
Perhaps sassc, sass-spec, and plugins should be git submodules, then I think they will get updated automatically on git pull? CI could still always pull the latest version.

[glebm]

There is one test:

$ cd sassc; msbuild .\win\sassc.sln /p:Platform=Win32 /p:Configuration=Debug /m:4; cd ..
$ ruby sass-spec/sass-spec.rb -V 3.5 --probe-todo --impl libsass -c sassc\bin\sassc.exe sass-spec/spec
...
The following tests pass but were marked as TODO for libsass:
sass-spec/spec/core_functions/color/hsla/error/one_arg

[mgreter]

Did this also show up on CI? Btw. once you pointed out the missing change from you, I saw how I can reuse OrderNodes for std::includes: std::includes(rset.begin(), rset.end(), lset.begin(), lset.end(), OrderNodes());, sometimes C++ just doesn't feel natural to me :) Adjusting the commit now.

[mgreter]

Rebased and amended once more to include the lost changes from @glebm again and also found out how to reuse struct OrderNodes::operator() for std::includes. @glebm care to take a look again?

[mgreter]

@xzyfer any objections to move this forward?

[xzyfer]

It's too big of a change for me to comment on without context. Feel free to proceed when you and @glebm are confident. 👍