aaron.ballman added a comment. In D69764#2938973 <https://reviews.llvm.org/D69764#2938973>, @MyDeveloperDay wrote:
> With the introduction of `CVQualifierOrder` the need for > `CVQualifierAlignment:` doesn't make so much sense > > CVQualifierAlignment: Left > CVQualifierOrder: [ "inline", "static", "volatile", "const" ] > > I'm leaning toward introducing <type> into the `CVQualifierOrder` allowing > for some qualifiers to be Left of the type and some to be Right (this is not > dissimilar to what was discussed some time ago, and similar to the concept of > the resharper image above) > > So we would be able to have: > > CVQualifierOrder: [ "inline", "static", "volatile", "<type>", "const", > "restrict" ] > > By doing so `inline,static,and volatile` are Left and `const,restrict` are > Right and so a single setting of Left and Right isn't applicable. > > The whole feature could then be determined ONLY using the `CVQualifierOrder` > however it doesn't feel like there is a good way to say "Don't change order" > (unless of course I use an empty `CVQualifierOrder` list), but this doesn't > feel very off by default enough, I like the explicitness of `Leave`. > > My gut feeling is to make `Left` and `Right` options define a predetermined > CVQualifierOrder, and introduce `Custom` as a new CVQualifierAlignment > setting which would require you to define the Order, this has the advantage > of driving a sort of semi standard as to what we mean by `Left` and `Right` > > Left would by default define: > > CVQualifierOrder: [ "inline", "static", "volatile", "const", "restrict", > "<type>"] > > Right would define: > > CVQualifierOrder: [ "inline", "static", "<type>", "volatile", "const", > "restrict" ] > > (we might want to discuss what these defaults should be, I'm not a language > lawyer!) > > I realise "inline" and "static" are to the left in the Right alignment, but > I'm not sure them being to the right is even legal (its certainly not common) > or what is most likely wanted. > > I also think if a `CVQualifierOrder` is defined and the > `CVQualifierAlignment` is not `Custom` that should be a configuration error > to ensure there isn't any ambiguity > > Does anyone have any thoughts on this approach? I think the general idea (letting the user specify the order of elements relative to one another rather than trying to come up with individual controls for each qualifier), I think that'll be easier for people to configure. I do worry about calling it "qualifier order" though as some of the things are definitely not qualifiers (like `static` or `inline` which are specifiers). A few questions about using an ordered list like this are: 0) should we diagnose typos (e.g., the user writes `inlnie` instead of `inline`)?, 1) should we diagnose duplications (e.g., the user lists `friend` twice)? Btw, https://github.com/llvm/llvm-project/blob/main/clang/include/clang/Sema/DeclSpec.h#L229 has quite a bit of information about the kinds of qualifiers and specifiers Clang deals with at the parsing stage, in case you were looking for a list of things to care about. > Oh man you are killing me with good suggestions ;-).... > So this will be hard with the current implementation because [ [ noreturn ] ] > are all separate tokens. However maybe if we COULD merge the tokens into 1 > token (like we do in FormatTokenLexer) even it it was just temporarily for > this pass we maybe could do that. I'd like to second the request for attribute support, even if it's follow-up work. Things like address space attributes, GC attributes, `__unaligned`, etc are all type qualifiers (I think we have a list of the attributes which are qualifiers in Type.h), so it's a natural extension. However, attributes in general are somewhat tricky because the attribute style (`__attribute__` vs `[[]]`) can be syntactically location sensitive. e.g., `[[noreturn]] void func()` means something different from `void [[noreturn]]` func()` and we have some attributes that can appertain to a declaration or a type (so the positioning can have silent semantic changes). > I'm in no rush, I want to let the RFC have a good amount time for people to > read and to gather all the feedback, but I'm encouraged enough to think my > efforts won't be wasted and I want to address some of @aaron.ballman concerns > to extend a olive branch in the hope that I could at least partially gain > approval. Not being in a rush sounds like a great plan! FWIW, I think we're moving steadily towards approval. I mentioned it on the RFC thread, but I'll mention it again here: thank you for starting the RFC process to have the broader discussion! CHANGES SINCE LAST ACTION https://reviews.llvm.org/D69764/new/ https://reviews.llvm.org/D69764 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
