Skip to content

Conversation

@neusdan
Copy link
Contributor

@neusdan neusdan commented Jan 8, 2016

@randombit
Copy link
Owner

The build.h modification looks fine as does the assert.

I do not think noreturn is appropriate on any of the encode_into calls; they are throwing simply due to encoding of those values not being implemented, but I don't want to commit to them not ever being implemented in the future with a noreturn API contract.

Similarly it does not seem right on Credentials_Manager::psk, since while the default throws, a subclass might well return if a PSK is defined. I have no idea what a compiler does with a virtual noreturn, but I would guess it either ignores the noreturn or happily generates code assuming no subclass will ever return either. I find it kind of weird GCC would suggest this one.

For http_transact_fail it makes sense, but I wonder why on earth this is exposed to callers? It is just an internal implementation detail I'm pretty sure. (I did this, I just can't remember what I was thinking at the time.) Probably it should be pulled out of the header entirely and be an anon namespace function in the source file, as there is no reason for any application to ever call it. I'll handle this.

Please trim the PR to just buildh.in and assert.h for now.

…ture

In addition don't declare virtual functions noreturn
@neusdan
Copy link
Contributor Author

neusdan commented Jan 8, 2016

Good points. What about the warning flag? Do you wan't to have it enabled?

@neusdan
Copy link
Contributor Author

neusdan commented Jan 8, 2016

somethings wrong with the amalgamation build

@neusdan
Copy link
Contributor Author

neusdan commented Jan 8, 2016

ok, that was dumb ;-) it goes before the function declaration

@randombit randombit merged commit f391635 into randombit:master Jan 9, 2016
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.

2 participants