[PATCH] D113896: [clangd] cleanup of header guard names

Christian Kühnel via Phabricator via cfe-commits cfe-commits at lists.llvm.org
Wed Nov 17 06:36:25 PST 2021


kuhnel added inline comments.


================
Comment at: clang-tools-extra/clangd/PathMapping.h:3
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_PATHMAPPING_H
+
 //===--- PathMapping.h - apply path mappings to LSP messages -===//
----------------
Looks like the include guards were missing here. I suppose this is something we want to add.


================
Comment at: clang-tools-extra/clangd/test/Inputs/background-index/sub_dir/foo.h:1
-#ifndef FOO_H
-#define FOO_H
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_TEST_INPUTS_BACKGROUND_INDEX_SUB_DIR_FOO_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_TEST_INPUTS_BACKGROUND_INDEX_SUB_DIR_FOO_H
----------------
sammccall wrote:
> Please don't modify tests, they don't need to follow LLVM style and should be as simple as possible
I was iterating over all .cpp and .h files, so the test data ended up in the changes as well.

I suppose the only way to avoid this is to use the `run-clang-tidy.py` wraper script instead


================
Comment at: clang-tools-extra/clangd/unittests/LSPClient.h:2
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_LSPCLIENT_H
+
----------------
Same here. I suppose we want header guards in this one.


================
Comment at: clang-tools-extra/clangd/unittests/TestScheme.h:1
+#ifndef LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTSCHEME_H
+#define LLVM_CLANG_TOOLS_EXTRA_CLANGD_UNITTESTS_TESTSCHEME_H
----------------
sammccall wrote:
> What's up with this change?
> If this file is empty it should just be deleted instead
Deleting the file


Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm.org/D113896/new/

https://reviews.llvm.org/D113896



More information about the cfe-commits mailing list