[PATCH] D75465: [clang-format] Do not merge target-name and : for C# attributes
Jonathan B Coe via Phabricator via cfe-commits
cfe-commits at lists.llvm.org
Tue Mar 3 02:38:08 PST 2020
jbcoe added inline comments.
================
Comment at: clang/lib/Format/TokenAnnotator.cpp:391
if (!AttrTok)
- return false;
+ return true;
----------------
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.
================
Comment at: clang/unittests/Format/FormatTestCSharp.cpp:281
Style.ColumnLimit = 10;
- verifyFormat(R"([assembly:InternalsVisibleTo(
+ verifyFormat(R"([assembly: InternalsVisibleTo(
"SomeAssembly, PublicKey=SomePublicKeyThatExceedsTheColumnLimit")])",
----------------
krasimir wrote:
> krasimir wrote:
> > 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.
> The space maybe is coming from
> [[ https://github.com/llvm/llvm-project/blob/ff9f4b5489cda4d9b31595b54ec0296dc012588d/clang/lib/Format/TokenAnnotator.h#L177 | TokenAnnotator.spaceRequiredBetween ]], which may need to be adjusted for C#.
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).
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D75465/new/
https://reviews.llvm.org/D75465
More information about the cfe-commits
mailing list