nridge planned changes to this revision. nridge added inline comments.
================ Comment at: clang-tools-extra/clangd/unittests/ClangdTests.cpp:1343 #undef CMDMACRO $inactive3[[#ifdef CMDMACRO int inactiveInt2; ---------------- daiyousei-qz wrote: > hokein wrote: > > While this patch is an improvement, I wonder we should move it further. > > > > Has been thinking about it more, we seem to have some inconsistent behavior > > on highlighting the `#if`, `#else`, `#endif` lines: > > > > - in `$inactive1` case, the `#endif` is marked as inactive (IMO, the > > highlighting in the editor is confusing, it looks like we're missing a > > match `endif`, thus I prefer to mark it as active to correspond to the > > active `#if` branch); > > - in `$inactive3` case, the line `#elif PREAMBLEMACRO > 0` is marked as > > inactive, this is inconsistent with `$inactive1` case; > > > > I think it would be nice to have a consistent model. One approach is to > > always consider `#if`, `#elif`, `#endif`, `#endif` lines active (in the > > implementation side, this would mean we always use the line range > > [skipp_range.start.line+1, skipp_range.end.line - 1]). > > > > What do you think about this? > > > > > +1. My perspective is that C++ source code is actually a meta-language > (preprocessor) that describes generation of C++ language code. That is, > `#if`, `#else`, `#endif` and .etc are always in a sense "executed" to > generate the actual code. So they shouldn't be marked as inactive. I agree, what you describe is a nice additional improvement. I believe it would also resolve the long-standing issue https://github.com/clangd/clangd/issues/773. I will update the patch. Repository: rG LLVM Github Monorepo CHANGES SINCE LAST ACTION https://reviews.llvm.org/D151190/new/ https://reviews.llvm.org/D151190 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
