nridge added inline comments.
================ Comment at: clang-tools-extra/clangd/SemanticHighlighting.h:76 ConstructorOrDestructor, + UserProvided, ---------------- ckandeler wrote: > 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 ;) > 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`) In my mind, "go-to-definition on this operator symbol will take me to a function declaration/definition" is a good match for "I want this colored differently". (Which would imply treating dependent operator calls where we can't figure out an overloaded operator target even heuristically, as "built-in".) 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
