This is an archive of the discontinued LLVM Phabricator instance.

[clang-format] Do not merge target-name and : for C# attributes
ClosedPublic

Authored by jbcoe on Mar 2 2020, 8:50 AM.

Details

Summary

Re-use token type TT_AttributeColon for C# attribute target colons.

Diff Detail

Event Timeline

jbcoe created this revision.Mar 2 2020, 8:50 AM
Herald added a project: Restricted Project. · View Herald TranscriptMar 2 2020, 8:50 AM
Herald added a subscriber: cfe-commits. · View Herald Transcript
krasimir added inline comments.Mar 2 2020, 4:20 PM
clang/unittests/Format/FormatTestCSharp.cpp
281

isn't the space after the : a regression?
Looking at
https://docs.microsoft.com/en-us/dotnet/standard/assembly/set-attributes
it looks like such attributes are spelled [assembly:Blah]

How is an example where such an attribute is not the last thing in the file?

I'm not entirely clear on which edit caused this.

krasimir added inline comments.Mar 2 2020, 4:23 PM
clang/unittests/Format/FormatTestCSharp.cpp
281

The space maybe is coming from
TokenAnnotator.spaceRequiredBetween, which may need to be adjusted for C#.

jbcoe updated this revision to Diff 247832.Mar 3 2020, 2:22 AM
jbcoe marked 2 inline comments as done.

Do not allow spaces around C# attribute colons.

clang/lib/Format/TokenAnnotator.cpp
391

This is needed to avoid

[assembly:InternalsVisibleTo("SomeAssembly, PublicKey=SomePublicKeyThatExceedsTheColumnLimit")]

being classified as a ObjCMethodExpr (as seen when running clang-format with -debug).

The formatting results are currently the same for ObjCMethodExpr and CSharpAttributes.

clang/unittests/Format/FormatTestCSharp.cpp
281

There seems to be some inconsistency between the docs you linked (which has no space) and code that forms part of projects like Roslyn (which has the space).

I'll defer making this change (if we want it until later).

This feels better than merging

I think visual studio tends to create files via the new project wizard that do not have a space before but do have a space after the :

[assembly: ComVisible(false)]

krasimir accepted this revision.Mar 3 2020, 11:41 AM

Thank you for the explanations! Looking good!

This revision is now accepted and ready to land.Mar 3 2020, 11:41 AM
This revision was automatically updated to reflect the committed changes.