[PATCH] D75844: [clang] Set begin loc on GNU attribute parsed attrs

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Thank you for picking this back up!



================
Comment at: clang/include/clang/Parse/Parser.h:2708
+
   bool MaybeParseGNUAttributes(ParsedAttributes &attrs,
                                SourceLocation *endLoc = nullptr,
----------------
aaron.ballman wrote:
> We might as well take the opportunity to fix the style issues.
Can we add a comment above here saying that use of this interface is discouraged and callers should use the interface that takes the attributes with range information. Eventually, we should be getting rid of the range-less parsing functionality.


================
Comment at: clang/include/clang/Parse/Parser.h:2708-2714
   bool MaybeParseGNUAttributes(ParsedAttributes &attrs,
                                SourceLocation *endLoc = nullptr,
                                LateParsedAttrList *LateAttrs = nullptr) {
+    if (Tok.is(tok::kw___attribute)) {
+      ParsedAttributesWithRange attrsWithRange(AttrFactory);
+      ParseGNUAttributes(attrs, endLoc, LateAttrs);
+      attrs.takeAllFrom(attrsWithRange);
----------------
We might as well take the opportunity to fix the style issues.


================
Comment at: clang/include/clang/Parse/Parser.h:2720-2721
+
+  bool MaybeParseGNUAttributes(ParsedAttributesWithRange &attrs,
+                               SourceLocation *endLoc = nullptr,
+                               LateParsedAttrList *LateAttrs = nullptr) {
----------------



================
Comment at: clang/include/clang/Parse/Parser.h:2730
+
   void ParseGNUAttributes(ParsedAttributes &attrs,
+                          SourceLocation *endLoc = nullptr,
----------------
aaron.ballman wrote:
> Same
A similar comment here would be handy.


================
Comment at: clang/include/clang/Parse/Parser.h:2730-2734
   void ParseGNUAttributes(ParsedAttributes &attrs,
+                          SourceLocation *endLoc = nullptr,
+                          LateParsedAttrList *LateAttrs = nullptr,
+                          Declarator *D = nullptr) {
+    ParsedAttributesWithRange attrsWithRange(AttrFactory);
----------------
Same


================
Comment at: clang/include/clang/Parse/Parser.h:2739-2740
+
+  void ParseGNUAttributes(ParsedAttributesWithRange &attrs,
                           SourceLocation *endLoc = nullptr,
                           LateParsedAttrList *LateAttrs = nullptr,
----------------



================
Comment at: clang/lib/Parse/ParseDecl.cpp:165-167
+void Parser::ParseGNUAttributes(ParsedAttributesWithRange &attrs,
                                 SourceLocation *endLoc,
+                                LateParsedAttrList *LateAttrs, Declarator *D) {
----------------
Don't feel obligated to fix the naming style issues here -- that's quite a bit more churn. But if you did fix it, I wouldn't complain either.


================
Comment at: clang/test/AST/sourceranges.cpp:112
+// CHECK-1Z: NamespaceDecl {{.*}} attributed_case
+namespace attributed_case {
+void f(int n) {
----------------
This test looks to be failing the CI pipeline currently.


CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D75844/new/

https://reviews.llvm.org/D75844