[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp added a comment.

Thanks for the review @nridge! I'd really appreciate if you could help me with 
the commit (I don't have commit access).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157990

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D157990: [clangd] Add --query-driver flag to clangd-indexer

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp created this revision.
tetsuo-cpp added reviewers: sammccall, nridge.
tetsuo-cpp added projects: clang-tools-extra, All.
Herald added subscribers: kadircet, arphaman.
tetsuo-cpp requested review of this revision.
Herald added subscribers: MaskRay, ilya-biryukov.

When using `clangd` for cross-compiled projects, it's necessary to use the 
`--query-driver` flag so that `clangd` can extract the compiler's built-in 
header paths. However, there's no such flag for `clangd-indexer` so we're 
unable to build a working static index for these projects.

This patch adds a `--query-driver` flag to `clangd-indexer` for this scenario.

I saw some tests under `clang-tools-extra/clangd/test/` but I think the 
cross-compilation case is a bit more complex to test. Let me know if you'd like 
me to look into this further.

Resolves: https://github.com/clangd/clangd/issues/1717


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D157990

Files:
  clang-tools-extra/clangd/indexer/IndexerMain.cpp


Index: clang-tools-extra/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/clangd/indexer/IndexerMain.cpp
@@ -38,6 +38,16 @@
"binary RIFF format")),
llvm::cl::init(IndexFileFormat::RIFF));
 
+static llvm::cl::list QueryDriverGlobs{
+"query-driver",
+llvm::cl::desc(
+"Comma separated list of globs for white-listing gcc-compatible "
+"drivers that are safe to execute. Drivers matching any of these globs 
"
+"will be used to extract system includes. e.g. "
+"/usr/bin/**/clang-*,/path/to/repo/**/g++-*"),
+llvm::cl::CommaSeparated,
+};
+
 class IndexActionFactory : public tooling::FrontendActionFactory {
 public:
   IndexActionFactory(IndexFileIn ) : Result(Result) {}
@@ -144,12 +154,16 @@
 
   // Collect symbols found in each translation unit, merging as we go.
   clang::clangd::IndexFileIn Data;
+  auto Mangler = std::make_shared(
+  clang::clangd::CommandMangler::detect());
+  Mangler->SystemIncludeExtractor = clang::clangd::getSystemIncludeExtractor(
+  static_cast>(
+  clang::clangd::QueryDriverGlobs));
   auto Err = Executor->get()->execute(
   std::make_unique(Data),
   clang::tooling::ArgumentsAdjuster(
-  [Mangler = std::make_shared(
-   clang::clangd::CommandMangler::detect())](
-  const std::vector , llvm::StringRef File) {
+  [Mangler = std::move(Mangler)](const std::vector ,
+ llvm::StringRef File) {
 clang::tooling::CompileCommand Cmd;
 Cmd.CommandLine = Args;
 Mangler->operator()(Cmd, File);


Index: clang-tools-extra/clangd/indexer/IndexerMain.cpp
===
--- clang-tools-extra/clangd/indexer/IndexerMain.cpp
+++ clang-tools-extra/clangd/indexer/IndexerMain.cpp
@@ -38,6 +38,16 @@
"binary RIFF format")),
llvm::cl::init(IndexFileFormat::RIFF));
 
+static llvm::cl::list QueryDriverGlobs{
+"query-driver",
+llvm::cl::desc(
+"Comma separated list of globs for white-listing gcc-compatible "
+"drivers that are safe to execute. Drivers matching any of these globs "
+"will be used to extract system includes. e.g. "
+"/usr/bin/**/clang-*,/path/to/repo/**/g++-*"),
+llvm::cl::CommaSeparated,
+};
+
 class IndexActionFactory : public tooling::FrontendActionFactory {
 public:
   IndexActionFactory(IndexFileIn ) : Result(Result) {}
@@ -144,12 +154,16 @@
 
   // Collect symbols found in each translation unit, merging as we go.
   clang::clangd::IndexFileIn Data;
+  auto Mangler = std::make_shared(
+  clang::clangd::CommandMangler::detect());
+  Mangler->SystemIncludeExtractor = clang::clangd::getSystemIncludeExtractor(
+  static_cast>(
+  clang::clangd::QueryDriverGlobs));
   auto Err = Executor->get()->execute(
   std::make_unique(Data),
   clang::tooling::ArgumentsAdjuster(
-  [Mangler = std::make_shared(
-   clang::clangd::CommandMangler::detect())](
-  const std::vector , llvm::StringRef File) {
+  [Mangler = std::move(Mangler)](const std::vector ,
+ llvm::StringRef File) {
 clang::tooling::CompileCommand Cmd;
 Cmd.CommandLine = Args;
 Mangler->operator()(Cmd, File);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp added a comment.

Friendly ping @njames93.
Does this look ok to you? If so, could you please help me with the commit?


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

https://reviews.llvm.org/D83188

___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp marked an inline comment as done.
tetsuo-cpp added a comment.

@aaron.ballman 
Thanks! I took your suggestion and I think this looks cleaner.


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

https://reviews.llvm.org/D83188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp updated this revision to Diff 280082.
tetsuo-cpp added a comment.

Cleanup check conditional.


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

https://reviews.llvm.org/D83188

Files:
  clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
@@ -74,9 +74,31 @@
 bool *b;
   } d = { SomeFunction() };
 
-  if (d.b)
+  if (d.b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: dubious check of 'bool *' against 'nullptr'
+// CHECK-FIXES: if (*d.b) {
+  }
+
+  if (d.b) {
 (void)*d.b; // no-warning
+  }
 
-#define CHECK(b) if (b) {}
+#define CHECK(b) \
+  if (b) {   \
+  }
   CHECK(c)
 }
+
+struct H {
+  bool *b;
+  void foo() const {
+if (b) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: dubious check of 'bool *' against 'nullptr'
+  // CHECK-FIXES: if (*b) {
+}
+
+if (b) {
+  (void)*b; // no-warning
+}
+  }
+};
Index: clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
@@ -20,53 +20,68 @@
   Finder->addMatcher(
   traverse(
   ast_type_traits::TK_AsIs,
-  ifStmt(hasCondition(findAll(implicitCastExpr(
- unless(hasParent(unaryOperator(hasOperatorName("!",
- hasSourceExpression(expr(
- hasType(pointerType(pointee(booleanType(,
- ignoringParenImpCasts(declRefExpr().bind("expr",
- hasCastKind(CK_PointerToBoolean,
- unless(isInTemplateInstantiation()))
+  ifStmt(
+  hasCondition(findAll(implicitCastExpr(
+  unless(hasParent(unaryOperator(hasOperatorName("!",
+  hasSourceExpression(expr(
+  hasType(pointerType(pointee(booleanType(,
+  ignoringParenImpCasts(anyOf(declRefExpr().bind("expr"),
+  memberExpr().bind("expr"),
+  hasCastKind(CK_PointerToBoolean,
+  unless(isInTemplateInstantiation()))
   .bind("if")),
   this);
 }
 
-void BoolPointerImplicitConversionCheck::check(
-const MatchFinder::MatchResult ) {
-  auto *If = Result.Nodes.getNodeAs("if");
-  auto *Var = Result.Nodes.getNodeAs("expr");
-
+static void checkImpl(const MatchFinder::MatchResult , const Expr *Ref,
+  const IfStmt *If,
+  const ast_matchers::internal::Matcher ,
+  ClangTidyCheck ) {
   // Ignore macros.
-  if (Var->getBeginLoc().isMacroID())
+  if (Ref->getBeginLoc().isMacroID())
 return;
 
-  // Only allow variable accesses for now, no function calls or member exprs.
+  // Only allow variable accesses and member exprs for now, no function calls.
   // Check that we don't dereference the variable anywhere within the if. This
   // avoids false positives for checks of the pointer for nullptr before it is
   // dereferenced. If there is a dereferencing operator on this variable don't
   // emit a diagnostic. Also ignore array subscripts.
-  const Decl *D = Var->getDecl();
-  auto DeclRef = ignoringParenImpCasts(declRefExpr(to(equalsNode(D;
-  if (!match(findAll(
- unaryOperator(hasOperatorName("*"), hasUnaryOperand(DeclRef))),
+  if (!match(findAll(unaryOperator(hasOperatorName("*"),
+   hasUnaryOperand(RefMatcher))),
  *If, *Result.Context)
.empty() ||
-  !match(findAll(arraySubscriptExpr(hasBase(DeclRef))), *If,
+  !match(findAll(arraySubscriptExpr(hasBase(RefMatcher))), *If,
  *Result.Context)
.empty() ||
   // FIXME: We should still warn if the paremater is implicitly converted to
   // bool.
-  !match(findAll(callExpr(hasAnyArgument(ignoringParenImpCasts(DeclRef,
- *If, *Result.Context)
+  !match(
+   findAll(callExpr(hasAnyArgument(ignoringParenImpCasts(RefMatcher,
+   *If, *Result.Context)
.empty() ||
-  !match(findAll(cxxDeleteExpr(has(ignoringParenImpCasts(expr(DeclRef),
- *If, *Result.Context)
+  !match(
+   findAll(cxxDeleteExpr(has(ignoringParenImpCasts(expr(RefMatcher),
+   *If, *Result.Context)
.empty())
 

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp added a comment.

Friendly ping. Could I please get this looked at?


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

https://reviews.llvm.org/D83188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp added a comment.

Friendly ping!


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

https://reviews.llvm.org/D83188



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp updated this revision to Diff 276022.
tetsuo-cpp added a comment.

Address comments: Rename variables, remove unnecessary tests and avoid 
unnecessary check.


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

https://reviews.llvm.org/D83188

Files:
  clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
@@ -74,9 +74,31 @@
 bool *b;
   } d = { SomeFunction() };
 
-  if (d.b)
+  if (d.b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: dubious check of 'bool *' against 'nullptr'
+// CHECK-FIXES: if (*d.b) {
+  }
+
+  if (d.b) {
 (void)*d.b; // no-warning
+  }
 
-#define CHECK(b) if (b) {}
+#define CHECK(b) \
+  if (b) {   \
+  }
   CHECK(c)
 }
+
+struct H {
+  bool *b;
+  void foo() const {
+if (b) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: dubious check of 'bool *' against 'nullptr'
+  // CHECK-FIXES: if (*b) {
+}
+
+if (b) {
+  (void)*b; // no-warning
+}
+  }
+};
Index: clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
@@ -20,53 +20,73 @@
   Finder->addMatcher(
   traverse(
   ast_type_traits::TK_AsIs,
-  ifStmt(hasCondition(findAll(implicitCastExpr(
- unless(hasParent(unaryOperator(hasOperatorName("!",
- hasSourceExpression(expr(
- hasType(pointerType(pointee(booleanType(,
- ignoringParenImpCasts(declRefExpr().bind("expr",
- hasCastKind(CK_PointerToBoolean,
- unless(isInTemplateInstantiation()))
+  ifStmt(
+  hasCondition(findAll(implicitCastExpr(
+  unless(hasParent(unaryOperator(hasOperatorName("!",
+  hasSourceExpression(expr(
+  hasType(pointerType(pointee(booleanType(,
+  ignoringParenImpCasts(anyOf(declRefExpr().bind("expr"),
+  memberExpr().bind("expr"),
+  hasCastKind(CK_PointerToBoolean,
+  unless(isInTemplateInstantiation()))
   .bind("if")),
   this);
 }
 
-void BoolPointerImplicitConversionCheck::check(
-const MatchFinder::MatchResult ) {
-  auto *If = Result.Nodes.getNodeAs("if");
-  auto *Var = Result.Nodes.getNodeAs("expr");
-
+static void checkImpl(const MatchFinder::MatchResult , const Expr *Ref,
+  const IfStmt *If,
+  const ast_matchers::internal::Matcher ,
+  ClangTidyCheck ) {
   // Ignore macros.
-  if (Var->getBeginLoc().isMacroID())
+  if (Ref->getBeginLoc().isMacroID())
 return;
 
-  // Only allow variable accesses for now, no function calls or member exprs.
+  // Only allow variable accesses and member exprs for now, no function calls.
   // Check that we don't dereference the variable anywhere within the if. This
   // avoids false positives for checks of the pointer for nullptr before it is
   // dereferenced. If there is a dereferencing operator on this variable don't
   // emit a diagnostic. Also ignore array subscripts.
-  const Decl *D = Var->getDecl();
-  auto DeclRef = ignoringParenImpCasts(declRefExpr(to(equalsNode(D;
-  if (!match(findAll(
- unaryOperator(hasOperatorName("*"), hasUnaryOperand(DeclRef))),
+  if (!match(findAll(unaryOperator(hasOperatorName("*"),
+   hasUnaryOperand(RefMatcher))),
  *If, *Result.Context)
.empty() ||
-  !match(findAll(arraySubscriptExpr(hasBase(DeclRef))), *If,
+  !match(findAll(arraySubscriptExpr(hasBase(RefMatcher))), *If,
  *Result.Context)
.empty() ||
   // FIXME: We should still warn if the paremater is implicitly converted to
   // bool.
-  !match(findAll(callExpr(hasAnyArgument(ignoringParenImpCasts(DeclRef,
- *If, *Result.Context)
+  !match(
+   findAll(callExpr(hasAnyArgument(ignoringParenImpCasts(RefMatcher,
+   *If, *Result.Context)
.empty() ||
-  !match(findAll(cxxDeleteExpr(has(ignoringParenImpCasts(expr(DeclRef),
- *If, *Result.Context)
+  !match(
+   

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp updated this revision to Diff 275592.
tetsuo-cpp added a comment.

Remove the unnecessary template.


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

https://reviews.llvm.org/D83188

Files:
  clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
@@ -70,13 +70,72 @@
 *(c) = false; // no-warning
   }
 
+#define CHECK(b) \
+  if (b) {   \
+  }
+  CHECK(c)
+
   struct {
 bool *b;
   } d = { SomeFunction() };
 
-  if (d.b)
-(void)*d.b; // no-warning
+  if (d.b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: dubious check of 'bool *' against 'nullptr'
+// CHECK-FIXES: if (*d.b) {
+  }
 
-#define CHECK(b) if (b) {}
-  CHECK(c)
+  if (F() && d.b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: dubious check of 'bool *' against 'nullptr'
+// CHECK-FIXES: if (F() && *d.b) {
+  }
+
+  // TODO: warn here.
+  if (d.b) {
+G(d.b);
+  }
+
+#define TESTMACRO2 if (d.b || F())
+
+  TESTMACRO2 {
+  }
+
+  t(d.b);
+
+  if (!d.b) {
+// no-warning
+  }
+
+  if (d.b) {
+*d.b = true; // no-warning
+  }
+
+  if (d.b) {
+d.b[0] = false; // no-warning
+  }
+
+  if (d.b) {
+SomeOtherFunction(d.b); // no-warning
+  }
+
+  if (d.b) {
+delete[] d.b; // no-warning
+  }
+
+  if (d.b) {
+*(d.b) = false; // no-warning
+  }
 }
+
+struct H {
+  bool *b;
+  void foo() const {
+if (b) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: dubious check of 'bool *' against 'nullptr'
+  // CHECK-FIXES: if (*b) {
+}
+
+if (b) {
+  (void)*b; // no-warning
+}
+  }
+};
Index: clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
@@ -20,33 +20,32 @@
   Finder->addMatcher(
   traverse(
   ast_type_traits::TK_AsIs,
-  ifStmt(hasCondition(findAll(implicitCastExpr(
- unless(hasParent(unaryOperator(hasOperatorName("!",
- hasSourceExpression(expr(
- hasType(pointerType(pointee(booleanType(,
- ignoringParenImpCasts(declRefExpr().bind("expr",
- hasCastKind(CK_PointerToBoolean,
- unless(isInTemplateInstantiation()))
+  ifStmt(
+  hasCondition(findAll(implicitCastExpr(
+  unless(hasParent(unaryOperator(hasOperatorName("!",
+  hasSourceExpression(expr(
+  hasType(pointerType(pointee(booleanType(,
+  ignoringParenImpCasts(anyOf(declRefExpr().bind("expr"),
+  memberExpr().bind("expr"),
+  hasCastKind(CK_PointerToBoolean,
+  unless(isInTemplateInstantiation()))
   .bind("if")),
   this);
 }
 
-void BoolPointerImplicitConversionCheck::check(
-const MatchFinder::MatchResult ) {
-  auto *If = Result.Nodes.getNodeAs("if");
-  auto *Var = Result.Nodes.getNodeAs("expr");
-
+static void checkImpl(const MatchFinder::MatchResult , const Expr *Var,
+  const IfStmt *If,
+  const ast_matchers::internal::Matcher ,
+  ClangTidyCheck ) {
   // Ignore macros.
   if (Var->getBeginLoc().isMacroID())
 return;
 
-  // Only allow variable accesses for now, no function calls or member exprs.
+  // Only allow variable accesses and member exprs for now, no function calls.
   // Check that we don't dereference the variable anywhere within the if. This
   // avoids false positives for checks of the pointer for nullptr before it is
   // dereferenced. If there is a dereferencing operator on this variable don't
   // emit a diagnostic. Also ignore array subscripts.
-  const Decl *D = Var->getDecl();
-  auto DeclRef = ignoringParenImpCasts(declRefExpr(to(equalsNode(D;
   if (!match(findAll(
  unaryOperator(hasOperatorName("*"), hasUnaryOperand(DeclRef))),
  *If, *Result.Context)
@@ -64,11 +63,30 @@
.empty())
 return;
 
-  diag(Var->getBeginLoc(), "dubious check of 'bool *' against 'nullptr', did "
-   "you mean to dereference it?")
+  Check.diag(Var->getBeginLoc(),
+ "dubious check of 'bool *' against 'nullptr', did "
+ "you mean to dereference it?")
   << 

[PATCH] D83188: [clang-tidy] bugprone-bool-pointer-implicit-conversion doesn't handle members

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp created this revision.
tetsuo-cpp added reviewers: njames93, MaskRay.
tetsuo-cpp added projects: clang, clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.

Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=45189
This patch is adding support for members to the 
`bugprone-bool-pointer-implicit-conversion` check. Apologies if I've done this 
in a weird/wrong way, I'm still getting my head around the matching DSL.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D83188

Files:
  clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-bool-pointer-implicit-conversion.cpp
@@ -70,13 +70,72 @@
 *(c) = false; // no-warning
   }
 
+#define CHECK(b) \
+  if (b) {   \
+  }
+  CHECK(c)
+
   struct {
 bool *b;
   } d = { SomeFunction() };
 
-  if (d.b)
-(void)*d.b; // no-warning
+  if (d.b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: dubious check of 'bool *' against 'nullptr'
+// CHECK-FIXES: if (*d.b) {
+  }
 
-#define CHECK(b) if (b) {}
-  CHECK(c)
+  if (F() && d.b) {
+// CHECK-MESSAGES: :[[@LINE-1]]:14: warning: dubious check of 'bool *' against 'nullptr'
+// CHECK-FIXES: if (F() && *d.b) {
+  }
+
+  // TODO: warn here.
+  if (d.b) {
+G(d.b);
+  }
+
+#define TESTMACRO2 if (d.b || F())
+
+  TESTMACRO2 {
+  }
+
+  t(d.b);
+
+  if (!d.b) {
+// no-warning
+  }
+
+  if (d.b) {
+*d.b = true; // no-warning
+  }
+
+  if (d.b) {
+d.b[0] = false; // no-warning
+  }
+
+  if (d.b) {
+SomeOtherFunction(d.b); // no-warning
+  }
+
+  if (d.b) {
+delete[] d.b; // no-warning
+  }
+
+  if (d.b) {
+*(d.b) = false; // no-warning
+  }
 }
+
+struct H {
+  bool *b;
+  void foo() const {
+if (b) {
+  // CHECK-MESSAGES: :[[@LINE-1]]:9: warning: dubious check of 'bool *' against 'nullptr'
+  // CHECK-FIXES: if (*b) {
+}
+
+if (b) {
+  (void)*b; // no-warning
+}
+  }
+};
Index: clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/BoolPointerImplicitConversionCheck.cpp
@@ -20,33 +20,33 @@
   Finder->addMatcher(
   traverse(
   ast_type_traits::TK_AsIs,
-  ifStmt(hasCondition(findAll(implicitCastExpr(
- unless(hasParent(unaryOperator(hasOperatorName("!",
- hasSourceExpression(expr(
- hasType(pointerType(pointee(booleanType(,
- ignoringParenImpCasts(declRefExpr().bind("expr",
- hasCastKind(CK_PointerToBoolean,
- unless(isInTemplateInstantiation()))
+  ifStmt(
+  hasCondition(findAll(implicitCastExpr(
+  unless(hasParent(unaryOperator(hasOperatorName("!",
+  hasSourceExpression(expr(
+  hasType(pointerType(pointee(booleanType(,
+  ignoringParenImpCasts(anyOf(declRefExpr().bind("expr"),
+  memberExpr().bind("expr"),
+  hasCastKind(CK_PointerToBoolean,
+  unless(isInTemplateInstantiation()))
   .bind("if")),
   this);
 }
 
-void BoolPointerImplicitConversionCheck::check(
-const MatchFinder::MatchResult ) {
-  auto *If = Result.Nodes.getNodeAs("if");
-  auto *Var = Result.Nodes.getNodeAs("expr");
-
+template 
+static void checkImpl(const MatchFinder::MatchResult ,
+  const RefExpr *Var, const IfStmt *If,
+  const ast_matchers::internal::Matcher ,
+  ClangTidyCheck ) {
   // Ignore macros.
   if (Var->getBeginLoc().isMacroID())
 return;
 
-  // Only allow variable accesses for now, no function calls or member exprs.
+  // Only allow variable accesses and member exprs for now, no function calls.
   // Check that we don't dereference the variable anywhere within the if. This
   // avoids false positives for checks of the pointer for nullptr before it is
   // dereferenced. If there is a dereferencing operator on this variable don't
   // emit a diagnostic. Also ignore array subscripts.
-  const Decl *D = Var->getDecl();
-  auto DeclRef = ignoringParenImpCasts(declRefExpr(to(equalsNode(D;
   if (!match(findAll(
  unaryOperator(hasOperatorName("*"), hasUnaryOperand(DeclRef))),
  *If, *Result.Context)
@@ -64,11 +64,30 @@

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp added a comment.

In D75901#1920397 , @njames93 wrote:

> In D75901#1916119 , @tetsuo-cpp 
> wrote:
>
> > @njames93 @MaskRay 
> >  Thanks for helping me with testing. I'll remember this for next time.
> >
> > I also saw this pattern of retrieving `LangOptions` from an `ASTContext` in 
> > some other checks (`RedundantExpressionCheck` and `StaticAssertCheck`) but 
> > I didn't change them to use `getLangOpts` since the `ASTContext` is used 
> > for other stuff.
>
>
> Its also not a good idea to change unrelated code in a review


Understood. Thanks @njames93.
Could you please help me commit this change?


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

https://reviews.llvm.org/D75901



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp marked an inline comment as done.
tetsuo-cpp added a comment.

@njames93 @MaskRay 
Thanks for helping me with testing. I'll remember this for next time.

I also saw this pattern of retrieving `LangOptions` from an `ASTContext` in 
some other checks (`RedundantExpressionCheck` and `StaticAssertCheck`) but I 
didn't change them to use `getLangOpts` since the `ASTContext` is used for 
other stuff.


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

https://reviews.llvm.org/D75901



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp updated this revision to Diff 249538.
tetsuo-cpp added a comment.

Address review comments.


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

https://reviews.llvm.org/D75901

Files:
  clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
  
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp


Index: 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
===
--- /dev/null
+++ 
clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
@@ -0,0 +1,6 @@
+// RUN: %check_clang_tidy -std=c++98,c++03 %s 
misc-unconventional-assign-operator %t
+
+struct BadArgument {
+  BadArgument =(BadArgument &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 
'BadArgument const&' or 'BadArgument'
+};
Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -75,7 +75,10 @@
   } else {
 static const char *const Messages[][2] = {
 {"ReturnType", "operator=() should return '%0&'"},
-{"ArgumentType", "operator=() should take '%0 const&', '%0&&' or 
'%0'"},
+{"ArgumentType",
+ getLangOpts().CPlusPlus11
+ ? "operator=() should take '%0 const&', '%0&&' or '%0'"
+ : "operator=() should take '%0 const&' or '%0'"},
 {"cv", "operator=() should not be marked '%1'"}};
 
 const auto *Method = Result.Nodes.getNodeAs("method");


Index: clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/misc-unconventional-assign-operator-precxx11.cpp
@@ -0,0 +1,6 @@
+// RUN: %check_clang_tidy -std=c++98,c++03 %s misc-unconventional-assign-operator %t
+
+struct BadArgument {
+  BadArgument =(BadArgument &);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: operator=() should take 'BadArgument const&' or 'BadArgument'
+};
Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -75,7 +75,10 @@
   } else {
 static const char *const Messages[][2] = {
 {"ReturnType", "operator=() should return '%0&'"},
-{"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"},
+{"ArgumentType",
+ getLangOpts().CPlusPlus11
+ ? "operator=() should take '%0 const&', '%0&&' or '%0'"
+ : "operator=() should take '%0 const&' or '%0'"},
 {"cv", "operator=() should not be marked '%1'"}};
 
 const auto *Method = Result.Nodes.getNodeAs("method");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75901: [clang-tidy] misc-unconventional-assign-operator suggest to use rvalue references in C++03 mode

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp created this revision.
tetsuo-cpp added a project: clang-tools-extra.
Herald added subscribers: cfe-commits, xazax.hun.
Herald added a project: clang.
tetsuo-cpp added reviewers: njames93, MaskRay.

Bugzilla: https://bugs.llvm.org/show_bug.cgi?id=27702
I wasn't sure how this type of thing is usually tested. So any advice would be 
appreciated.
`check-llvm`, `check-clang` and `check-clang-tools` are clean for me.
**C++98**

  tetsuo@garland-c-16-sgp1-01:~/dev/llvm-project/test$ cat compile_commands.json
  [
  {
"directory": "/home/tetsuo/dev/llvm-project/test",
"command": "/usr/bin/c++  -std=gnu++98 -o 
CMakeFiles/test.dir/test.cpp.o -c /home/tetsuo/dev/llvm-project/test/test.cpp",
"file": "/home/tetsuo/dev/llvm-project/test/test.cpp"
  }
  ]
  tetsuo@garland-c-16-sgp1-01:~/dev/llvm-project/test$ ../build/bin/clang-tidy 
--checks=misc-unconventional-assign-operator test.cpp
  3053 warnings generated.
  /home/tetsuo/dev/llvm-project/test/test.cpp:7:3: warning: operator=() should 
take 'Foo const&' or 'Foo' [misc-unconventional-assign-operator]
Foo =(Foo ) {
^
  Suppressed 3052 warnings (3052 in non-user code).
  Use -header-filter=.* to display errors from all non-system headers. Use 
-system-headers to display errors from system headers as well.

**C++17**

  tetsuo@garland-c-16-sgp1-01:~/dev/llvm-project/test$ cat compile_commands.json
  [
  {
"directory": "/home/tetsuo/dev/llvm-project/test",
"command": "/usr/bin/c++  -std=gnu++17 -o 
CMakeFiles/test.dir/test.cpp.o -c /home/tetsuo/dev/llvm-project/test/test.cpp",
"file": "/home/tetsuo/dev/llvm-project/test/test.cpp"
  }
  ]
  tetsuo@garland-c-16-sgp1-01:~/dev/llvm-project/test$ ../build/bin/clang-tidy 
--checks=misc-unconventional-assign-operator test.cpp
  5377 warnings generated.
  /home/tetsuo/dev/llvm-project/test/test.cpp:7:3: warning: operator=() should 
take 'Foo const&', 'Foo&&' or 'Foo' [misc-unconventional-assign-operator]
Foo =(Foo ) {
^
  Suppressed 5376 warnings (5376 in non-user code).
  Use -header-filter=.* to display errors from all non-system headers. Use 
-system-headers to display errors from system headers as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75901

Files:
  clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp


Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -73,9 +73,14 @@
   if (const auto *RetStmt = Result.Nodes.getNodeAs("returnStmt")) {
 diag(RetStmt->getBeginLoc(), "operator=() should always return '*this'");
   } else {
+const ASTContext *ASTCtx = Result.Context;
+const LangOptions  = ASTCtx->getLangOpts();
 static const char *const Messages[][2] = {
 {"ReturnType", "operator=() should return '%0&'"},
-{"ArgumentType", "operator=() should take '%0 const&', '%0&&' or 
'%0'"},
+{"ArgumentType",
+ Opts.CPlusPlus11
+ ? "operator=() should take '%0 const&', '%0&&' or '%0'"
+ : "operator=() should take '%0 const&' or '%0'"},
 {"cv", "operator=() should not be marked '%1'"}};
 
 const auto *Method = Result.Nodes.getNodeAs("method");


Index: clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
===
--- clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
+++ clang-tools-extra/clang-tidy/misc/UnconventionalAssignOperatorCheck.cpp
@@ -73,9 +73,14 @@
   if (const auto *RetStmt = Result.Nodes.getNodeAs("returnStmt")) {
 diag(RetStmt->getBeginLoc(), "operator=() should always return '*this'");
   } else {
+const ASTContext *ASTCtx = Result.Context;
+const LangOptions  = ASTCtx->getLangOpts();
 static const char *const Messages[][2] = {
 {"ReturnType", "operator=() should return '%0&'"},
-{"ArgumentType", "operator=() should take '%0 const&', '%0&&' or '%0'"},
+{"ArgumentType",
+ Opts.CPlusPlus11
+ ? "operator=() should take '%0 const&', '%0&&' or '%0'"
+ : "operator=() should take '%0 const&' or '%0'"},
 {"cv", "operator=() should not be marked '%1'"}};
 
 const auto *Method = Result.Nodes.getNodeAs("method");
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp added a comment.

@MyDeveloperDay
Thanks for your suggestions! I think it's better now.
This is my first LLVM patch (hopefully first of many) so I don't have commit 
permissions yet. If you're still happy with this change, could you please 
commit on my behalf?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67037



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp updated this revision to Diff 219582.
tetsuo-cpp added a comment.

Addressed review comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D67037

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1408,6 +1408,30 @@
"test_label:;\n"
"  int i = 0;\n"
"}");
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentGotoLabels = false;
+  verifyFormat("void f() {\n"
+   "  some_code();\n"
+   "test_label:\n"
+   "  some_other_code();\n"
+   "  {\n"
+   "some_more_code();\n"
+   "another_label:\n"
+   "some_more_code();\n"
+   "  }\n"
+   "}",
+   Style);
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label:\n"
+   "  some_other_code();\n"
+   "}",
+   Style);
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label:;\n"
+   "  int i = 0;\n"
+   "}");
 }
 
 //===--===//
@@ -11769,6 +11793,7 @@
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
   CHECK_PARSE_BOOL(DisableFormat);
   CHECK_PARSE_BOOL(IndentCaseLabels);
+  CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -106,7 +106,7 @@
   void parseTryCatch();
   void parseForOrWhileLoop();
   void parseDoWhile();
-  void parseLabel();
+  void parseLabel(bool LeftAlignLabel = false);
   void parseCaseLabel();
   void parseSwitch();
   void parseNamespace();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1351,7 +1351,7 @@
   (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
-  parseLabel();
+  parseLabel(!Style.IndentGotoLabels);
   return;
 }
 // Recognize function-like macro usages without trailing semicolon as
@@ -1970,11 +1970,13 @@
   parseStructuralElement();
 }
 
-void UnwrappedLineParser::parseLabel() {
+void UnwrappedLineParser::parseLabel(bool LeftAlignLabel) {
   nextToken();
   unsigned OldLineLevel = Line->Level;
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
 --Line->Level;
+  if (LeftAlignLabel)
+Line->Level = 0;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Line->Level,
Style.BraceWrapping.AfterCaseLabel,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -453,6 +453,7 @@
 IO.mapOptional("IncludeCategories", Style.IncludeStyle.IncludeCategories);
 IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex);
 IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
+IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
 IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
 IO.mapOptional("IndentWidth", Style.IndentWidth);
 IO.mapOptional("IndentWrappedFunctionNames",
@@ -725,6 +726,7 @@
   LLVMStyle.IncludeStyle.IncludeIsMainRegex = "(Test)?$";
   LLVMStyle.IncludeStyle.IncludeBlocks = tooling::IncludeStyle::IBS_Preserve;
   LLVMStyle.IndentCaseLabels = false;
+  LLVMStyle.IndentGotoLabels = true;
   LLVMStyle.IndentPPDirectives = FormatStyle::PPDIS_None;
   LLVMStyle.IndentWrappedFunctionNames = false;
   LLVMStyle.IndentWidth = 2;
Index: clang/include/clang/Format/Format.h
===
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -1265,6 +1265,22 @@
   /// \endcode
   bool IndentCaseLabels;
 
+  /// Indent goto labels.
+  ///
+  /// When ``false``, goto labels are flushed left.
+  /// \code
+  ///   

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp added a comment.

Friendly ping. Could I please get this looked at?


Repository:
  rC Clang

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

https://reviews.llvm.org/D67037



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D67037: [ClangFormat] Add new style option IndentGotoLabels

[Alex Cameron via Phabricator via cfe-commits]

tetsuo-cpp created this revision.
tetsuo-cpp added a reviewer: klimek.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This option determines whether goto labels are indented according to scope. 
Setting this option to false causes goto labels to be flushed to the left.
This is mostly copied from this patch 
 submitted 
by Christian Neukirchen that didn't make its way into trunk.

  true:  false:
  int f() {  vs. int f() {
if (foo()) {   if (foo()) {
label1:  label1:
  bar(); bar();
}  }
  label2:label2:
return 1;  return 1;
  }  }


Repository:
  rC Clang

https://reviews.llvm.org/D67037

Files:
  clang/docs/ClangFormatStyleOptions.rst
  clang/include/clang/Format/Format.h
  clang/lib/Format/Format.cpp
  clang/lib/Format/UnwrappedLineParser.cpp
  clang/lib/Format/UnwrappedLineParser.h
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -1408,6 +1408,30 @@
"test_label:;\n"
"  int i = 0;\n"
"}");
+  FormatStyle Style = getLLVMStyle();
+  Style.IndentGotoLabels = false;
+  verifyFormat("void f() {\n"
+   "  some_code();\n"
+   "test_label:\n"
+   "  some_other_code();\n"
+   "  {\n"
+   "some_more_code();\n"
+   "another_label:\n"
+   "some_more_code();\n"
+   "  }\n"
+   "}",
+   Style);
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label:\n"
+   "  some_other_code();\n"
+   "}",
+   Style);
+  verifyFormat("{\n"
+   "  some_code();\n"
+   "test_label:;\n"
+   "  int i = 0;\n"
+   "}");
 }
 
 //===--===//
@@ -11769,6 +11793,7 @@
   CHECK_PARSE_BOOL_FIELD(DerivePointerAlignment, "DerivePointerBinding");
   CHECK_PARSE_BOOL(DisableFormat);
   CHECK_PARSE_BOOL(IndentCaseLabels);
+  CHECK_PARSE_BOOL(IndentGotoLabels);
   CHECK_PARSE_BOOL(IndentWrappedFunctionNames);
   CHECK_PARSE_BOOL(KeepEmptyLinesAtTheStartOfBlocks);
   CHECK_PARSE_BOOL(ObjCSpaceAfterProperty);
Index: clang/lib/Format/UnwrappedLineParser.h
===
--- clang/lib/Format/UnwrappedLineParser.h
+++ clang/lib/Format/UnwrappedLineParser.h
@@ -106,7 +106,7 @@
   void parseTryCatch();
   void parseForOrWhileLoop();
   void parseDoWhile();
-  void parseLabel();
+  void parseLabel(bool GotoLabel = false);
   void parseCaseLabel();
   void parseSwitch();
   void parseNamespace();
Index: clang/lib/Format/UnwrappedLineParser.cpp
===
--- clang/lib/Format/UnwrappedLineParser.cpp
+++ clang/lib/Format/UnwrappedLineParser.cpp
@@ -1351,7 +1351,7 @@
   (TokenCount == 2 && Line->Tokens.front().Tok->is(tok::comment))) {
 if (FormatTok->Tok.is(tok::colon) && !Line->MustBeDeclaration) {
   Line->Tokens.begin()->Tok->MustBreakBefore = true;
-  parseLabel();
+  parseLabel(true);
   return;
 }
 // Recognize function-like macro usages without trailing semicolon as
@@ -1970,11 +1970,13 @@
   parseStructuralElement();
 }
 
-void UnwrappedLineParser::parseLabel() {
+void UnwrappedLineParser::parseLabel(bool GotoLabel) {
   nextToken();
   unsigned OldLineLevel = Line->Level;
   if (Line->Level > 1 || (!Line->InPPDirective && Line->Level > 0))
 --Line->Level;
+  if (GotoLabel && !Style.IndentGotoLabels)
+Line->Level = 0;
   if (CommentsBeforeNextToken.empty() && FormatTok->Tok.is(tok::l_brace)) {
 CompoundStatementIndenter Indenter(this, Line->Level,
Style.BraceWrapping.AfterCaseLabel,
Index: clang/lib/Format/Format.cpp
===
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -453,6 +453,7 @@
 IO.mapOptional("IncludeCategories", Style.IncludeStyle.IncludeCategories);
 IO.mapOptional("IncludeIsMainRegex", Style.IncludeStyle.IncludeIsMainRegex);
 IO.mapOptional("IndentCaseLabels", Style.IndentCaseLabels);
+IO.mapOptional("IndentGotoLabels", Style.IndentGotoLabels);
 IO.mapOptional("IndentPPDirectives", Style.IndentPPDirectives);
 IO.mapOptional("IndentWidth", Style.IndentWidth);