[PATCH] D39571: [clangd] DidChangeConfiguration Notification

Ilya Biryukov via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Feb 14 04:35:03 PST 2018


ilya-biryukov added a comment.

In https://reviews.llvm.org/D39571#1006667, @simark wrote:

>   It seems to me like
>
> changes in command line arguments (adding -DMACRO=1) are not taken into account
>  when changing configuration.


It looks like a bug in the preamble handling. (It does not check if macros were redefined).
You can workaround that by making sure the preamble ends before your code starts (preamble only captures preprocessor directives, so any C++ decl will end it):

    Annotations SourceAnnotations(R"cpp(
  int avoid_preamble;
  
  #ifndef MACRO
  $before[[static void bob() {}]]
  #else
  $after[[static void bob() {}]]
  #endif
  /// ....
  )cpp"





================
Comment at: unittests/clangd/XRefsTests.cpp:260
 
+TEST(DidChangeConfiguration, DifferentDeclaration) {
+  Annotations SourceAnnotations(R"cpp(
----------------
I'd move it to `ClangdTests.cpp`, generic `ClangdServer` tests usually go there. It's fine to `#include "Annotations.h"` there, too, even though it hasn't been used before.



================
Comment at: unittests/clangd/XRefsTests.cpp:271
+
+  MockCompilationDatabase CDB(/*UseRelPaths=*/true);
+  MockFSProvider FS;
----------------
Specifying `/*UseRelPath=*/true` is not necessary for this test, default value should do.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D39571





More information about the cfe-commits mailing list