This is an archive of the discontinued LLVM Phabricator instance.

[MC] Support .dcb directives in assembler parser
ClosedPublic

Authored by phosek on Sep 19 2016, 12:22 PM.

Details

Diff Detail

Repository
rL LLVM

Event Timeline

phosek updated this revision to Diff 71867.Sep 19 2016, 12:22 PM
phosek retitled this revision from to [MC] Support .dcb directives in assembler parser.
phosek updated this object.
phosek added a reviewer: echristo.
phosek set the repository for this revision to rL LLVM.
phosek added subscribers: llvm-commits, phosek.
davide added a subscriber: davide.Sep 20 2016, 8:42 PM

Same comments apply here, can you group switch conditions together (for consistency) and add some tests for the other error conditions?

phosek updated this revision to Diff 72088.Sep 21 2016, 11:07 AM

Is there anything else you want me to fix?

Sorry, lld kept me a little busy in the last two days =) I'll take a look at this again by the end of today. Sorry for the delay.

Feel free to submit after fixing my second round of comments, no need for another back and forth. Thanks!

lib/MC/MCParser/AsmParser.cpp
275–276

This is not your fault (partly, but a large amount of the parse* methods don't have a doxygen comment).
Do you mind to add one to this one?

1919–1936

This and the condition at line 1933 can be merged together, no?

phosek updated this revision to Diff 72231.Sep 22 2016, 5:55 PM
phosek marked 2 inline comments as done.
phosek added inline comments.
lib/MC/MCParser/AsmParser.cpp
1919–1936

That's true, it's a different directive but that's probably not an issue here.

This revision was automatically updated to reflect the committed changes.