ckandeler added inline comments.
================ Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:76 ConstructorOrDestructor, + UserProvided, ---------------- sammccall wrote: > sammccall wrote: > > sammccall wrote: > > > Can you give some background here or on the bug tracker about what kind > > > of distinction you're trying to draw here and why it's important? > > > (Most clients are never going to benefit from nonstandard modifiers so > > > they should be pretty compelling) > > as well as being jargony, "user-provided" has a specific technical meaning > > that I don't think you intend here. For example, `auto operator<=>(const > > S&) const = default` is not user-provided (defaulted on first declaration). > > https://eel.is/c++draft/dcl.fct.def.default#5 > > > > If we need this and can't get away with reusing `defaultLibrary` (which > > would include `std::`) then maybe we should add something like `builtin` > > which seems quite reusable. > Since we often can't say whether an operator is user-provided or not (in > templates), we should consider what we want the highlighting to be in these > cases. > (If templates should be highlighted as built-in, we should prefer a modifier > like `UserProvided`, if they should be highlighted as user-provided, we > should prefer a modifier like `Builtin`) > as well as being jargony, "user-provided" has a specific technical meaning > that I don't think you intend here. For example, `auto operator<=>(const S&) > const = default` is not user-provided (defaulted on first declaration). > https://eel.is/c++draft/dcl.fct.def.default#5 > > If we need this and can't get away with reusing `defaultLibrary` (which would > include `std::`) then maybe we should add something like `builtin` which > seems quite reusable. I use "userProvided" here in the sense of "not built-in" or "overloaded". I do not cling to the term, and have also suggested the opposite default of using "builtin" in another comment. ================ Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:76 ConstructorOrDestructor, + UserProvided, ---------------- ckandeler wrote: > sammccall wrote: > > sammccall wrote: > > > sammccall wrote: > > > > Can you give some background here or on the bug tracker about what kind > > > > of distinction you're trying to draw here and why it's important? > > > > (Most clients are never going to benefit from nonstandard modifiers so > > > > they should be pretty compelling) > > > as well as being jargony, "user-provided" has a specific technical > > > meaning that I don't think you intend here. For example, `auto > > > operator<=>(const S&) const = default` is not user-provided (defaulted on > > > first declaration). https://eel.is/c++draft/dcl.fct.def.default#5 > > > > > > If we need this and can't get away with reusing `defaultLibrary` (which > > > would include `std::`) then maybe we should add something like `builtin` > > > which seems quite reusable. > > Since we often can't say whether an operator is user-provided or not (in > > templates), we should consider what we want the highlighting to be in these > > cases. > > (If templates should be highlighted as built-in, we should prefer a > > modifier like `UserProvided`, if they should be highlighted as > > user-provided, we should prefer a modifier like `Builtin`) > > as well as being jargony, "user-provided" has a specific technical meaning > > that I don't think you intend here. For example, `auto operator<=>(const > > S&) const = default` is not user-provided (defaulted on first declaration). > > https://eel.is/c++draft/dcl.fct.def.default#5 > > > > If we need this and can't get away with reusing `defaultLibrary` (which > > would include `std::`) then maybe we should add something like `builtin` > > which seems quite reusable. > > I use "userProvided" here in the sense of "not built-in" or "overloaded". I > do not cling to the term, and have also suggested the opposite default of > using "builtin" in another comment. > Since we often can't say whether an operator is user-provided or not (in > templates), we should consider what we want the highlighting to be in these > cases. True, I have not considered this. > (If templates should be highlighted as built-in, we should prefer a modifier > like `UserProvided`, if they should be highlighted as user-provided, we > should prefer a modifier like `Builtin`) Intuitively, it seems we should be conservative and not claim the operator is overloaded unless we know it is. So "built-in" might then mean "we can't prove it's not a built-in". It's probably not worth to introduce a modifier CouldBeEitherWay just to explicitly express ambiguity ;) Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D136594/new/ https://reviews.llvm.org/D136594 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
