AaronBallman wrote: > I'd like to highlight that many tools still only care about declarations > themselves and not the attributes.
Good to know. > The ones that fiddle with attributes need to do it in a special and > complicated way already. Making this less complicated definitely creates some > value. But I think the immediate issue isn't about making that easy, but > rather keeping most tools that don't care about attributes simple. > > Hence I think having consistency in source ranges associated with > declarations to eliminate special cases in most tools is more beneficial (and > possibly simpler). I'd probably lean towards keeping them out of the source > ranges associated with decls, as that's the current state of the world. > Unless we're deliberately going to include them in declarations source > range's systematically, I think we're more likely to regress users down the > line. We're currently inconsistent regarding attributes; sometimes we include the attributes in the range, sometimes we don't. But declaration source ranges are tough to reason about in general because there's many different moving parts. Consider: ``` #pragma clang section bss = "test" static int i = 12; ``` If we include attributes in the declaration, shouldn't we also include pragmas which impact the declaration like above (this one adds an implicit attribute)? I think where we've traditionally fallen on this is that the source range for any given AST node covers all of the tokens used to produce that node. This means leading or trailing tokens are the awkward cases; are they used to produce the node or not? So currently, for `static int i = 12, j = 100 /*NOLINT*/;`, the source range for the `VarDecl` for `j` starts at `static` and ends at `100`. Perhaps surprising, but understandable. But what about: `/* NOLINT */ [[]] static int i = 12, j = 100 /* NOLINT */;`? Should that include the NOLINT comments? Tools often associate comments with declarations for special handling, so I think there are arguments either way. But what about the empty attribute specifier? Is that salient to the declaration? In the earlier example, should the pragma be included because it impacts the declaration? What about a `#pragma clang attribute` slapping attributes onto the declaration? Then there are problems like unknown attributes; should the source range for `[[unknown::unknown]] int i;` include the attribute? And so on. So we could either improve the current approach of using the source range which encompasses the maximum contiguous sequence of tokens comprising the full definition. Or we could consider changing it so that the declaration source range is the minimum contiguous sequence of tokens to form a valid declaration. e.g., `static int i = 100;` could have the range cover `int i` and `static auto i = 100;` could have the range cover `auto i = 100`. But I suspect the least amount of churn for tools is to continue to go with the maximal range and improve what we track at the edges. But I also suspect that will always be best-effort and tools are going to have to handle those on a case-by-case basis sometimes. https://github.com/llvm/llvm-project/pull/133107 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
