sammccall accepted this revision. sammccall added inline comments. This revision is now accepted and ready to land. Herald added a subscriber: cfe-commits.
================ Comment at: clang-tools-extra/clangd/InlayHints.cpp:114 + // void setTimeout(int timeoutMillis); + return Name.substr(3).equals_lower(ParamNames[0]); + } ---------------- nridge wrote: > njames93 wrote: > > This heuristic is far from perfect. There's definitely diminishing returns > > for the more advanced you make it, but I'd argue it makes sense to strip > > any underscore from the Name, e.g, > > `void set_timeout(int timeout)` > Good call, snake_case is definitely worth supporting This doesn't handle multi-word variable names if params use snake case and functions don't, e.g.: e.g. `void setExceptionHandler(EHFunc exception_handler)` This is fine for now but probably worth a comment. Ultimately replacing `equals_lower` with some `sloppy_equals` that ignores case and skips underscores would solve this neatly. ================ Comment at: clang-tools-extra/clangd/InlayHints.cpp:108 + + // In addition to checking that the function has one parameter and its + // name starts with "set", also check that the part after "set" matches ---------------- This is nice and conservative, but a bit style dependent. e.g. `setSema(Sema &S)` Seems fine for now, maybe we want to tweak at some point. ================ Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:344 + void set_parent(S* parent); + void setTimeout(int timeoutMillis); + }; ---------------- can you add setTimeoutMillis(int timeout_millis) just to show the current behavior? Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D100731/new/ https://reviews.llvm.org/D100731 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
