SourceLocations of attribute list delimiters – [[ ... , ... ]] etc

Hi,

when parsing C++11 [[…]] attributes and Microsoft […] attributes, we throw out the source locations of the commas separating the attributes, and of the surrounding bracket tokens.

I’d like to write a fixit that suggests replacing [uuid(…)] with __declspec(uuid(…)), but that requires knowing if uuid() is the only thing in the list, and if so deleting the whole […], and if it’s not the only thing, deleting just the uuid() and either its trailing comma if it’s not the last attribute, or the preceding comma if it is the last one. It’s possible someone will want to do something similar with a C++11 attribute in the future.

Since all C++11 attributes are in the AST, one approach would be to check if there’s a preceding C++11 attribute and if so, start the removal after the last token of the predecessor, and do the same at the other end. But I think that can’t differentiate between [[good, remove_this, good]] and [[good]] [[remove_this]] [[good]]. Also, in either approach and the locations of the [] delimiters aren’t anywhere.

For attributes, it’s a bit harder still since we don’t keep most of them in the AST.

The cleanest way to do this I can think of is to have AttributeClusters (or similar) in the AST and have them store the SourceLocs of the surrounding brackets, and the number of attributes therein.

Does that sound useful? Doing this just for my fixit seems overkill :slight_smile:

Nico

Hi,

when parsing C++11 [[...]] attributes and Microsoft [...] attributes, we
throw out the source locations of the commas separating the attributes, and
of the surrounding bracket tokens.

I'd like to write a fixit that suggests replacing [uuid(...)] with
__declspec(uuid(...)), but that requires knowing if uuid() is the only thing
in the list, and if so deleting the whole [...], and if it's not the only
thing, deleting just the uuid() and either its trailing comma if it's not
the last attribute, or the preceding comma if it is the last one. It's
possible someone will want to do something similar with a C++11 attribute in
the future.

Since all C++11 attributes are in the AST, one approach would be to check if
there's a preceding C++11 attribute and if so, start the removal after the
last token of the predecessor, and do the same at the other end. But I think
that can't differentiate between `[[good, remove_this, good]]` and `[[good]]
[[remove_this]] [[good]]`. Also, in either approach and the locations of the
[] delimiters aren't anywhere.

Correct, the AST does not track enough information to distinguish
between [[one, two]] and [[one]][[two]], for instance.

For attributes, it's a bit harder still since we don't keep most of them
in the AST.

We don't keep any ignored attributes in the AST, so you also cannot
tell between [[known, unknown]] and [[known]].

The cleanest way to do this I can think of is to have AttributeClusters (or
similar) in the AST and have them store the SourceLocs of the surrounding
brackets, and the number of attributes therein.

Does that sound useful? Doing this just for my fixit seems overkill :slight_smile:

It could be useful, but it does seem like possible overkill as well. I
certainly wouldn't complain if we tracked source information about
attributes better, though.

What kind of accuracy are you hoping for? For instance, do you want to
handle constructs like [[foo,bar]][][[baz]], or just things like
__declspec(foo) [[bar]] [baz, quux]?

~Aaron