Skip to content

Conversation

burner
Copy link
Member

@burner burner commented Aug 25, 2015

@Hackerpilot
Copy link
Contributor

@quickfur quickfur changed the title fix Issue 14940 fix Issue 14940: Can't call logger with more complex objects Aug 26, 2015
@@ -1799,8 +1799,7 @@ Params:
_isNull = false;
}

template toString()
{
template toString() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpick: opening brace on own line, please.

@burner
Copy link
Member Author

burner commented Aug 26, 2015

@quickfur @Hackerpilot done

@mathias-baumann-sociomantic

While this fixes the specific issue, isn't the deeper problem that @safe is required at all?

@burner
Copy link
Member Author

burner commented Aug 26, 2015

@mathias-baumann-sociomantic not really the OutputRange delegate passed to Nullable is not @safe and as this delegate is called by toString it can not be @safe

@mihails-strasuns-sociomantic
Copy link
Contributor

Code duplication looks a bit worrying here. Can't dmd infer attributes for delegate here? (considering toString is already a template)

@burner
Copy link
Member Author

burner commented Aug 26, 2015

@mihails-strasuns-sociomantic no it didn't making the delegate a template parameter also didn't work. The alias this was then used instead of the toString

@DmitryOlshansky
Copy link
Member

Auto-merge toggled on

@DmitryOlshansky
Copy link
Member

Puttin aside the general problem with delegates, merging this

DmitryOlshansky added a commit that referenced this pull request Sep 2, 2015
fix Issue 14940: Can't call logger with more complex objects
@DmitryOlshansky DmitryOlshansky merged commit 1e647d9 into dlang:master Sep 2, 2015
@burner
Copy link
Member Author

burner commented Sep 2, 2015

thanks

@burner burner deleted the issue14940 branch October 23, 2015 09:29
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.

6 participants