[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