Skip to content

remove helper functions for assert and unittest#6530

Merged
andralex merged 1 commit intodlang:masterfrom
WalterBright:rm-assert-unittest
Feb 12, 2017
Merged

remove helper functions for assert and unittest#6530
andralex merged 1 commit intodlang:masterfrom
WalterBright:rm-assert-unittest

Conversation

@WalterBright
Copy link
Member

This:

  1. removes the always-generated assert and unittest helper functions
  2. replaces them with direct calls
  3. replaces file argument with moduleinfo pointer

This makes using assert and unittest more pay-as-you-go, which can result in more code generated (though not more code executed). Of course, the extra code doesn't matter for unittest asserts.

Undone is the array bounds check, which I'll do if this goes through.

@WalterBright
Copy link
Member Author

This isn't ready yet.

@MartinNowak
Copy link
Member

  1. replaces them with direct calls

Direct calls to _d_unittest* and _d_assert* you mean.

  1. replaces file argument with moduleinfo pointer

Why? We have several known issues/drawbacks w/ dragged in ModuleInfos. Are you trying to avoid redundant strings? I think linkers already deduplicate strings.
If you don't trust those, moving the module name to it's own symbol might be an alternative.

@WalterBright
Copy link
Member Author

I changed the ModuleInfo pointer to a simple char* for the filename. This eliminates dependency on ModuleInfo, and eliminates problems with accessing the internal functions in shared libraries.

But it is blocked by dlang/druntime#1761

@andralex
Copy link
Member

This is a welcome simplification.

@WalterBright
Copy link
Member Author

@andralex ready to go!

@andralex andralex merged commit d0df42d into dlang:master Feb 12, 2017
@MartinNowak
Copy link
Member

What about array bounds checks?

@UplinkCoder
Copy link
Member

This causes my test-setup to fail :(

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants