[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