Skip to content
This repository was archived by the owner on Aug 31, 2021. It is now read-only.

Turn on -Werror=switch#4724

Merged
montegoulding merged 3 commits into
livecode:developfrom
montegoulding:switch
Nov 2, 2016
Merged

Turn on -Werror=switch#4724
montegoulding merged 3 commits into
livecode:developfrom
montegoulding:switch

Conversation

@montegoulding

Copy link
Copy Markdown
Contributor

No description provided.

@montegoulding

Copy link
Copy Markdown
Contributor Author

Looks like xcpretty output is too long for me to see the issues on mac which I'm guessing are test failures and for some reason Linux isn't working out that the switch satisfies all the enumeration values... perhaps I need to add MCUnreachableReturn there to satisfy the compiler?

@montegoulding

Copy link
Copy Markdown
Contributor Author

Ah... raw log tells me it was one of the IDE tests but the log output appears to be broken so it doesn't tell me which one.

@montegoulding

Copy link
Copy Markdown
Contributor Author

Some googling found this so I did the MCUnreachableReturn thing.

Comment thread engine/src/funcs.cpp
break;
case CT_CODEUNIT:
MCSyntaxFactoryEvalMethod(ctxt, kMCStringsEvalCodeunitOffsetMethodInfo);
break;

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This can be CT_BTYE as well I think.

@montegoulding montegoulding force-pushed the switch branch 2 times, most recently from e3353ac to 44fa2a1 Compare October 21, 2016 12:21
@montegoulding

Copy link
Copy Markdown
Contributor Author

@livecodeali this passed once everything was back up from Dyn DNS DDoS attack but I'm restarting Travis just to make doubly sure

@montegoulding

Copy link
Copy Markdown
Contributor Author

Seems to pass ok

Comment thread lcidlc/src/Interface.cpp
fprintf(stderr, "Java mapped methods must be java\n");
break;
case kInterfaceErrorNone:
MCUnreachableReturn(false);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Surely this should be something like:

fprintf(stderr, "No error occurred\n");

Aborting because there wasn't an error seems a little bit extreme.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Doesn't there have to be something seriously wrong with your code to be reporting an error when there wasn't one?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, and since this is lcidlc rather than libexternal this is probably not a big deal ;-)

@peter-b

peter-b commented Oct 28, 2016

Copy link
Copy Markdown
Contributor

@livecode-vulcan review ok aac360e

@peter-b peter-b added this to the 9.0.0-dp-2 milestone Oct 28, 2016
@livecode-vulcan

Copy link
Copy Markdown
Contributor

💙 review by @peter-b ok aac360e

livecode-vulcan added a commit that referenced this pull request Oct 28, 2016
@livecode-vulcan

Copy link
Copy Markdown
Contributor

😞 test failure aac360e

@montegoulding

Copy link
Copy Markdown
Contributor Author

@livecode-vulcan try aac360e

livecode-vulcan added a commit that referenced this pull request Nov 1, 2016
@livecode-vulcan

Copy link
Copy Markdown
Contributor

😞 test failure aac360e

@peter-b

peter-b commented Nov 2, 2016

Copy link
Copy Markdown
Contributor

@livecode-vulcan review ok 7310951

@livecode-vulcan

Copy link
Copy Markdown
Contributor

💙 review by @peter-b ok 7310951

livecode-vulcan added a commit that referenced this pull request Nov 2, 2016
@livecode-vulcan

Copy link
Copy Markdown
Contributor

😎 test success 7310951

@montegoulding montegoulding merged commit bb4e563 into livecode:develop Nov 2, 2016
@montegoulding montegoulding deleted the switch branch November 2, 2016 19:28
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants