Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Adding case insensitive 'error' detection #3669

Merged
merged 1 commit into from
Feb 9, 2017

Conversation

bridadan
Copy link
Contributor

@bridadan bridadan commented Jan 31, 2017

Description

GCC Assembler errors were being missed because it prints 'error' with a captial 'E'. This change allows the 'e' to be either lower case or upper case.

Status

READY

Migrations

If this PR changes any APIs or behaviors, give a short description of what API users should do when this PR is merged.

NO

Related PRs

This may conflict with #3653

Todos

  • Tests

Notes

Before, the tools would miss errors like the following when it was printed from the assembler:

/tmp/ccXLitIR.s: Assembler messages:
/tmp/ccXLitIR.s:677: Error: cannot honor width suffix -- `mov r1,#0'
/tmp/ccXLitIR.s:679: Error: cannot honor width suffix -- `mov r0,#0'
/tmp/ccXLitIR.s:727: Error: cannot honor width suffix -- `mov r1,#4'
/tmp/ccXLitIR.s:729: Error: cannot honor width suffix -- `neg r1,r1'
/tmp/ccXLitIR.s:731: Error: cannot honor width suffix -- `mov r5,#4'
/tmp/ccXLitIR.s:737: Error: cannot honor width suffix -- `mov r1,#3'
/tmp/ccXLitIR.s:739: Error: cannot honor width suffix -- `neg r1,r1'
/tmp/ccXLitIR.s:741: Error: cannot honor width suffix -- `mov r5,#3'
/tmp/ccXLitIR.s:743: Error: cannot honor width suffix -- `neg r5,r5'
/tmp/ccXLitIR.s:754: Error: cannot honor width suffix -- `orr r5,r2'
/tmp/ccXLitIR.s:904: Error: cannot honor width suffix -- `mov r1,#4'
/tmp/ccXLitIR.s:906: Error: cannot honor width suffix -- `neg r1,r1'
/tmp/ccXLitIR.s:908: Error: cannot honor width suffix -- `mov r0,#4'
/tmp/ccXLitIR.s:915: Error: cannot honor width suffix -- `mov r1,#3'
/tmp/ccXLitIR.s:916: Error: cannot honor width suffix -- `neg r1,r1'
/tmp/ccXLitIR.s:919: Error: cannot honor width suffix -- `mov r0,#3'
/tmp/ccXLitIR.s:921: Error: cannot honor width suffix -- `neg r0,r0'
/tmp/ccXLitIR.s:926: Error: lo register required -- `add r0,r0,#12'
/tmp/ccXLitIR.s:982: Error: cannot honor width suffix -- `mov r1,#6'
/tmp/ccXLitIR.s:984: Error: cannot honor width suffix -- `neg r1,r1'
/tmp/ccXLitIR.s:986: Error: cannot honor width suffix -- `mov r0,#6'
/tmp/ccXLitIR.s:987: Error: cannot honor width suffix -- `neg r0,r0'

Now it should list every one of the errors.

Copy link
Contributor

@theotherjimmy theotherjimmy left a comment

Choose a reason for hiding this comment

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

lol XD

@bulislaw
Copy link
Member

How about making it all case insensitive?

@bridadan
Copy link
Contributor Author

@bulislaw I thought about it, but I wasn't sure what impact that'd have. I'd rather not make too many changes to this sensitive line at the moment.

@bulislaw
Copy link
Member

maybe at least [Ww] for warnings :)

@bridadan
Copy link
Contributor Author

I thought about that too :) But I don't know if the assembler even outputs warnings that way, have you seen that?

@bulislaw
Copy link
Member

Ok, lets take a step back :) I don't really mind about all of that, but it's shame to add 1-2 days delay while chasing down issues like that, ultimately my goal is to be able to understand what's going on just by looking at CI logs. I don't understand why we just can't keep full logs, how much storage would it take? And i don't mean -v but just full stdout/stderr output from the compiler.

But yes I agree the simplest step we can do is to merge this change, which solves my problem for now.

@0xc0170
Copy link
Contributor

0xc0170 commented Feb 1, 2017

/morph test

@mbed-bot
Copy link

mbed-bot commented Feb 1, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1487

All builds and test passed!

@bridadan
Copy link
Contributor Author

bridadan commented Feb 1, 2017

add 1-2 days delay while chasing down issues like that, ultimately my goal is to be able to understand what's going on just by looking at CI logs

I agree, I've added verbose logs to the CI 'Build' step now, so hopefully this won't block others in the future.

But also if you notice locally that an error is missing without the -v, please don't forget to raise an issue here!

@bulislaw
Copy link
Member

bulislaw commented Feb 2, 2017

Thanks!

@sg-
Copy link
Contributor

sg- commented Feb 2, 2017

@bridadan needs an update!

GCC Assembler errors were being missed because it prints 'error'
with a captial 'E'. This change allows the 'e' to be either lower
case or upper case.
@bridadan
Copy link
Contributor Author

bridadan commented Feb 2, 2017

/morph test

@bridadan
Copy link
Contributor Author

bridadan commented Feb 2, 2017

retest uvisor

@mbed-bot
Copy link

mbed-bot commented Feb 3, 2017

Result: SUCCESS

Your command has finished executing! Here's what you wrote!

/morph test

Output

mbed Build Number: 1497

All builds and test passed!

@adbridge
Copy link
Contributor

adbridge commented Feb 6, 2017

@mazimkhan

@mazimkhan
Copy link

retest uvisor

@sg- sg- merged commit 492db95 into ARMmbed:master Feb 9, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants