This is an archive of the discontinued LLVM Phabricator instance.

[ATTR] Automatic line feed after pragma-like attribute.
ClosedPublic

Authored by ABataev on Oct 8 2015, 12:16 AM.

Details

Summary

Automatically insert line feed after pretty printing of all pragma-like attributes.

Diff Detail

Repository
rL LLVM

Event Timeline

ABataev updated this revision to Diff 36828.Oct 8 2015, 12:16 AM
ABataev retitled this revision from to [ATTR] Automatic line feed after pragma-like attribute, NFC..
ABataev updated this object.
ABataev added a reviewer: aaron.ballman.
ABataev added a subscriber: cfe-commits.
aaron.ballman accepted this revision.Oct 8 2015, 6:02 AM
aaron.ballman edited edge metadata.

LGTM, thanks! If you would be so kind as to add a test for #pragma init_seg("bss"), that would be great -- it seems its printPrettyPragma never put a newline in there for that attribute, and so I would guess there's lacking test coverage that this change should fix.

~Aaron

This revision is now accepted and ready to land.Oct 8 2015, 6:02 AM
ABataev added a subscriber: ABataev.Oct 8 2015, 6:14 AM

Ok, will do

Best regards,

Alexey Bataev

Software Engineer
Intel Compiler Team

08.10.2015 16:02, Aaron Ballman пишет:

aaron.ballman accepted this revision.
aaron.ballman added a comment.
This revision is now accepted and ready to land.

LGTM, thanks! If you would be so kind as to add a test for #pragma init_seg("bss"), that would be great -- it seems its printPrettyPragma never put a newline in there for that attribute, and so I would guess there's lacking test coverage that this change should fix.

~Aaron

http://reviews.llvm.org/D13546

ABataev updated this revision to Diff 36850.Oct 8 2015, 6:45 AM
ABataev retitled this revision from [ATTR] Automatic line feed after pragma-like attribute, NFC. to [ATTR] Automatic line feed after pragma-like attribute..
ABataev edited edge metadata.

Printing of pragma-like attributes for declarations did not worked at all. Fixed it and added test for printing #pragma init_seg.

Wow, good catch on the fact that this didn't work at all! Thank you for tackling it.

lib/AST/DeclPrinter.cpp
197 ↗(On Diff #36850)

Would it make more sense to add a prettyPrintPragmas(Decl *D) function instead?

test/SemaCXX/pragma-init_seg.cpp
1 ↗(On Diff #36850)

I think this should be a new test under Misc\. We have ast-print-pragmas.cpp that looks like a good place for it to go.

ABataev marked 2 inline comments as done.Oct 8 2015, 1:53 PM
ABataev added inline comments.
lib/AST/DeclPrinter.cpp
197 ↗(On Diff #36850)

Ok, will do

test/SemaCXX/pragma-init_seg.cpp
1 ↗(On Diff #36850)

Ok

ABataev updated this revision to Diff 36924.Oct 8 2015, 9:08 PM
ABataev marked 2 inline comments as done.

Update after review

Looks great, thank you!

~Aaron

This revision was automatically updated to reflect the committed changes.