[PATCH] D128663: Removed old test file.

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd created this revision.
Herald added a project: All.
abrahamcd requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128663

Files:
  clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
+++ /dev/null
@@ -1,79 +0,0 @@
-// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
-
-namespace std {
-
-template 
-struct vector {
-  bool empty();
-  void clear();
-};
-
-template 
-bool empty(T &&);
-
-} // namespace std
-
-namespace absl {
-template 
-struct vector {
-  bool empty();
-  void clear();
-};
-
-template 
-bool empty(T &&);
-} // namespace absl
-
-int test_member_empty() {
-  std::vector v;
-
-  v.empty();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 
'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-  absl::vector w;
-
-  w.empty();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 
'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}w.clear();{{$}}
-}
-
-int test_qualified_empty() {
-  absl::vector v;
-
-  std::empty(v);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 
'std::empty' [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-  absl::empty(v);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 
'absl::empty' [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}v.clear();{{$}}
-}
-
-int test_unqualified_empty() {
-  std::vector v;
-
-  empty(v);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 
'std::empty' [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-  absl::vector w;
-
-  empty(w);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 
'absl::empty' [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}w.clear();{{$}}
-
-  {
-using std::empty;
-empty(v);
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 
'std::empty' [bugprone-standalone-empty]
-// CHECK-FIXES: {{^  }}  v.clear();{{$}}
-  }
-
-  {
-using absl::empty;
-empty(w);
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 
'absl::empty' [bugprone-standalone-empty]
-// CHECK-FIXES: {{^  }}  w.clear();{{$}}
-  }
-}


Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
+++ /dev/null
@@ -1,79 +0,0 @@
-// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
-
-namespace std {
-
-template 
-struct vector {
-  bool empty();
-  void clear();
-};
-
-template 
-bool empty(T &&);
-
-} // namespace std
-
-namespace absl {
-template 
-struct vector {
-  bool empty();
-  void clear();
-};
-
-template 
-bool empty(T &&);
-} // namespace absl
-
-int test_member_empty() {
-  std::vector v;
-
-  v.empty();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-  absl::vector w;
-
-  w.empty();
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}w.clear();{{$}}
-}
-
-int test_qualified_empty() {
-  absl::vector v;
-
-  std::empty(v);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-  absl::empty(v);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}v.clear();{{$}}
-}
-
-int test_unqualified_empty() {
-  std::vector v;
-
-  empty(v);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-  absl::vector w;
-
-  empty(w);
-  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-  // CHECK-FIXES: {{^  }}w.clear();{{$}}
-
-  {
-using std::empty;
-empty(v);
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-// CHECK-FIXES: {{^  }}  v.clear();{{$}}
-  }
-
-  {
-using absl::empty;
-empty(w);
-// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-// CHECK-FIXES: {{^  }}  w.clear();{{$}}
-  }
-}
___
cfe-commits mailing list
cf

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 440286.
abrahamcd added a comment.

Removed old test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,202 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  std::vector s;
+
+  s.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty m;
+
+  m.empty();
+  // no-warning
+
+  std::vector_with_clear v;
+
+  v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  std::vector_with_int_empty w;
+
+  w.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  absl::string x;
+
+  x.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  absl::string_with_void_empty y;
+
+  y.empty();
+  // no-warning
+
+  absl::string_with_clear z;
+
+  z.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}z.clear();{{$}}
+
+  absl::string_with_int_empty a;
+
+  a.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}a.clear();{{$}}
+}
+
+int test_qualified_empty() {
+  absl::string_with_clear v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  test::empty(v);
+  // no-warning
+
+  absl::string w;
+
+  std::empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  std::vector_with_clear x;
+
+  empty(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}x.clear();{{$}}
+
+  std::vector_with_int_empty y;
+
+  empty(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}y.clear();{{$}}
+
+  absl::string z;
+
+  empty(z);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+
+  absl::string_with_void_empty a;
+
+  empty(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugpr

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:51-57
+auto Methods = MemberCall->getRecordDecl()->methods();
+auto Clear = llvm::find_if(Methods, [](const CXXMethodDecl *F) {
+  return F->getDeclName().getAsString() == "clear" &&
+ F->getMinRequiredArguments() == 0;
+});
+
+if (Clear != Methods.end()) {

cjdb wrote:
> Note to other reviewers: I suggested this because we couldn't work out a 
> better way to identify if .clear() exists (Abraham can best expand on the 
> issues regarding how a matcher wasn't working).
Yes, I wrote a matcher for the declaration of a .clear()  member function and 
another matcher for a call to .empty(), but it seemed like the check() function 
runs multiple times to match the different matchers. It would find .clear() on 
one run of check() and find .empty() on another, and I couldn't find a way to 
see if .clear() had already been found on a previous run of check().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 440346.
abrahamcd marked 7 inline comments as done.
abrahamcd added a comment.

Addressed review edits and clarity feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,202 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  std::vector s;
+
+  s.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty m;
+
+  m.empty();
+  // no-warning
+
+  std::vector_with_clear v;
+
+  v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  std::vector_with_int_empty w;
+
+  w.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  absl::string x;
+
+  x.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  absl::string_with_void_empty y;
+
+  y.empty();
+  // no-warning
+
+  absl::string_with_clear z;
+
+  z.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}z.clear();{{$}}
+
+  absl::string_with_int_empty a;
+
+  a.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}a.clear();{{$}}
+}
+
+int test_qualified_empty() {
+  absl::string_with_clear v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  test::empty(v);
+  // no-warning
+
+  absl::string w;
+
+  std::empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  std::vector_with_clear x;
+
+  empty(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}x.clear();{{$}}
+
+  std::vector_with_int_empty y;
+
+  empty(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}y.clear();{{$}}
+
+  absl::string z;
+
+  empty(z);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+
+  absl::string_with_void_empty a;
+
+  empty(a);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ig

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 440742.
abrahamcd added a comment.

Progress Update

Hit a roadblock with determining unused return value,
uploading current state for further work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,244 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+bool test = v.empty();
+//(void) v.empty();
+// no-warning
+// NOTE: This test not currently passing.
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+}
+
+int test_qualified_empty() {
+  {
+absl::string_with_clear v;
+
+std::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+absl::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+test::empty(v);
+// no-warning
+  }
+
+  {
+absl::string s;
+
+std::empty(s);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+}
+
+int test_unqualified_empty() {
+  {
+std::vector v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd planned changes to this revision.
abrahamcd added a comment.

Stuck on determining unused return value, uploaded current state for @denik and 
@cjdb to take a look.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 443017.
abrahamcd edited the summary of this revision.
abrahamcd added a comment.

Adds functionality for only warning on the case of unused return value
of the call to `empty()` and removes using-directive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,313 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+}
+
+int test_qualified_empty() {
+  {
+absl::string_with_clear v;
+
+std::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+absl::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+test::empty(v);
+// no-warning
+  }
+
+  {
+absl::string s;
+
+std::empty(s);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+}
+
+int test_unqualified_empty() {
+  {
+std::vector v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 443056.
abrahamcd marked 3 inline comments as done.
abrahamcd added a comment.

Adds argument 0 test case and refactoring based on feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,321 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+}
+
+int test_qualified_empty() {
+  {
+absl::string_with_clear v;
+
+std::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+absl::empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+
+test::empty(v);
+// no-warning
+  }
+
+  {
+absl::string s;
+
+std::empty(s);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+std::empty(0);
+// no-warning
+
+absl::empty(nullptr);
+// no-warning
+  }
+}
+
+int test_unqualified_empty() {
+  {
+std::vector v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added a comment.

Hi all, I just wanted to check in on this again and see if any more feedback or 
progress could be made. Thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D131084: Add support for specifying the severity of a SARIF Result.

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added a comment.

In D131084#3697211 , @vaibhav.y wrote:

> Submitting for review:
>
> Some notes:
>
> There are a couple of ways I think we can acheive this, per the spec:
>
> 1. The reportingDescriptor (rule) objects can be given a default 
> configuration property 
> ,
>  which can set the default warning level and other data such as rule 
> parameters etc.
> 2. The reportingDescriptor objects can omit the default configuration (which 
> then allows operating with warning as default), and the level is then set 
> when the result is reported.
>
> The first approach would be "more correct", what are your thoughts on this? 
> Would we benefit from having per-diagnostic configuration?
>
> There is also the question about the "kind" of results in clang. From my 
> reading of the spec 
> ,
>  it seems that "fail" is the only case that applies to us because:
>
> - "pass": Implies no issue was found.
> - "open": This value is used by proof-based tools. It could also mean 
> additional assertions required
> - "informational": The specified rule was evaluated and produced a purely 
> informational result that does not indicate the presence of a problem
> - "notApplicable": The rule specified by ruleId was not evaluated, because it 
> does not apply to the analysis target.
>
> Of these "open" and "notApplicable" seem to be ones that *could* come to use 
> but I'm not sure where / what kind of diagnostics would use these. Probably 
> clang-tidy's `bugprone-*` suite?
>
> Let me know what you think is a good way to approach this wrt clang's 
> diagnostics system.

@cjdb, @denik, and I discussed the two approaches yesterday, and we thought the 
first would be the better option (add setDefaultConfig to SarifRule instead of 
setDiagnosticLevel in SarifResult). We could make use of the 
defaultConfiguration's "level", "enabled", and "rank" properties to define the 
different diagnostic types that are currently present in Clang, and it would 
consolidate that information in a single rule rather than copying it across 
every result related to that rule. Then we could just have a small set of 
defaultConfigurations that correspond to the current Clang diagnostic 
categories that we can choose from when making new rules.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131084

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 450407.
abrahamcd added a comment.

Checks for `clear()` in base classes as well.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,483 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+}
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_clear_args s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_clear_variable s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+}
+
+int test_qualified_empty() {
+  {
+absl::string_with_clear v;
+
+std::empty(v);
+/

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 450430.
abrahamcd added a comment.

Adds non-dependent base class test case.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,508 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+}
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_clear_args s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_clear_variable s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the re

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp:177-178
+  diag(NonMemberLoc, "ignoring the result of '%0', did you mean 
'clear()'?")
+  << llvm::dyn_cast(NonMemberCall->getCalleeDecl())
+ ->getQualifiedNameAsString()
+  << FixItHint::CreateReplacement(ReplacementRange, ReplacementText);

njames93 wrote:
> Diagnostics can accept args as `const NamedDecl *`, so you can omit the call 
> to `getQualifiedNameAsString()`.
> The use of `dyn_cast` here is suspicious. If the CalleeDecl isn't a 
> `NamedDecl`, this would assert when you try and call 
> `getQualifiedNameAsString`, but equally I can't see any case why the 
> CalleeDecl wouldn't be a `NamedDecl`. @rsmith Can you think of any situation 
> where this could happen?
Seems that without `getQualifiedNameAsString()` I get longer less-readable 
versions of the name, e.g. `'empty &>'` instead of just 
`'std::empty'`. Do you think the extra information is helpful here?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 451209.
abrahamcd marked 5 inline comments as done.
abrahamcd added a comment.

Adds check for the compatibility of qualifiers on the clear method and the 
object it is called on.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,632 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace base
+
+
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_clear_args s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 451212.
abrahamcd added a comment.

Minor formatting/naming fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,631 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace base
+
+namespace qualifiers {
+template 
+struct vector_with_const_clear {
+  bool empty() const;
+  void clear() const;
+};
+
+template 
+struct vector_with_const_empty {
+  bool empty() const;
+  void clear();
+};
+
+template 
+struct vector_with_volatile_clear {
+  bool empty() volatile;
+  void clear() volatile;
+};
+
+template 
+struct vector_with_volatile_empty {
+  bool empty() volatile;
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace qualifiers
+
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+
+  {
+absl::string_with_int_empty s;
+
+s.empty();
+// C

[PATCH] D131632: [WIP] Enable SARIF Diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd created this revision.
Herald added a subscriber: mgorny.
Herald added a project: All.
abrahamcd requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Work in progress to enable Clang to emit SARIF diagnostics.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/SARIFDiagnosticTest.cpp
  clang/unittests/Frontend/sarif-diagnostics.cpp

Index: clang/unittests/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/unittests/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,138 @@
+// RUN: %clang -fdiagnostics-format=sarif %s -o %t.exe -DGTEST
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s 2>
+// %t.diags || true RUN: %t.exe < %t.diags
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace {
+
+constexpr llvm::StringRef BrokenProgram =
+R"(// Example errors below start on line 2
+void main() {
+  int i = hello;
+
+  float test = 1a.0;
+
+  if (true)
+bool Yes = true;
+return;
+
+  bool j = hi;
+}
+})";
+
+TEST(SARIFDiagnosticTest, TestFields) {
+  llvm::SmallString<256> SearchDir;
+  llvm::sys::fs::current_path(SearchDir);
+  
+  SearchDir.append("/../../../bin");
+  // ASSERT_EQ(SearchDir.str(), "hi");
+  llvm::ErrorOr ClangPathOrErr =
+  llvm::sys::findProgramByName("clang", {SearchDir});
+  ASSERT_TRUE(ClangPathOrErr);
+  const std::string &ClangPath = *ClangPathOrErr;
+  // ASSERT_EQ(ClangPath, "hi");
+
+  llvm::ErrorOr EchoPathOrErr =
+  llvm::sys::findProgramByName("echo");
+  ASSERT_TRUE(EchoPathOrErr);
+  const std::string &EchoPath = *EchoPathOrErr;
+
+  int EchoInputFD;
+  llvm::SmallString<32> EchoInputFile, EchoOutputFile;
+  llvm::sys::fs::createTemporaryFile("echo-input", "", EchoInputFD,
+ EchoInputFile);
+  llvm::sys::fs::createTemporaryFile("echo-output", "", EchoOutputFile);
+  llvm::FileRemover InputRemover(EchoInputFile.c_str());
+  llvm::FileRemover OutputRemover(EchoOutputFile.c_str());
+
+  llvm::Optional Redirects[] = {
+  EchoInputFile.str(), EchoOutputFile.str(), llvm::StringRef("")};
+
+  int RunResult = llvm::sys::ExecuteAndWait(EchoPath, {"echo", BrokenProgram},
+llvm::None, Redirects);
+  ASSERT_EQ(RunResult, 0);
+
+  // auto EchoOutputBuf = llvm::MemoryBuffer::getFile(EchoOutputFile.c_str());
+  // ASSERT_TRUE(EchoOutputBuf);
+  // llvm::StringRef EchoOutput = EchoOutputBuf.get()->getBuffer();
+  // ASSERT_EQ(EchoOutput.str(), "hi");
+
+  llvm::SmallString<32> ClangErrFile;
+  llvm::sys::fs::createTemporaryFile("clang-err", "", ClangErrFile);
+  llvm::FileRemover ClangErrRemover(ClangErrFile.c_str());
+
+  llvm::Optional ClangRedirects[] = {
+  EchoOutputFile.str(), llvm::StringRef(""), ClangErrFile.str()};
+  llvm::StringRef Args[] = {"clang",
+"-xc++",
+"-",
+"-fsyntax-only",
+"-Wall",
+"-Wextra",
+"-fdiagnostics-format=sarif"};
+
+  int ClangResult =
+  llvm::sys::ExecuteAndWait(ClangPath, Args, llvm::None, ClangRedirects);
+  ASSERT_EQ(ClangResult, 1);
+
+  // auto ClangOutputBuf = llvm::MemoryBuffer::getFile(ClangOutputFile.c_str());
+  // ASSERT_TRUE(ClangOutputBuf);
+  // llvm::StringRef ClangOutput = ClangOutputBuf.get()->getBuffer();
+  // ASSERT_EQ(ClangOutput.str(), "hi");
+
+  auto ClangErrBuf = llvm::MemoryBuffer::getFile(ClangErrFile.c_str());
+  ASSERT_TRUE(ClangErrBuf);
+  llvm::StringRef ClangErr = ClangErrBuf.get()->getBuffer();
+  ASSERT_EQ(ClangErr.str(), "hi");
+
+  llvm::Expected Value = llvm::json::parse(ClangErr.str());
+  ASSERT_FALSE(!Value);
+
+  llvm::json::Object *SarifDoc = Value->getAsObject();
+
+  const llvm::json::Array *Runs = SarifDoc->getArray("runs");
+  const llvm::json::Object *TheRun = Runs->back().getAsObject();
+  const llvm::json::Array *Results = TheRun->getArray("results");
+  
+  // Check Artifacts
+  const llvm::json::Array *Artifacts = TheRun->getArray("artifacts");
+  const llvm::json::Object *TheArtifact = Artifacts->back().getAsObject();
+  const llvm::json::Object *Location = TheArtifact->getObject("location");
+
+  ASSERT_T

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added a subscriber: denik.
abrahamcd added a comment.

Hi! I'm interning with @cjdb and @denik this summer and I was working on adding 
a `-fdiagnostics-format=sarif` option to start off my project, but I just found 
that a previous abandoned version of this change (D109697 
) was intending to add it. Seeing as the flag 
is no longer included in this version of the change, is it okay for me to go on 
and add it myself, or are you still planning on adding it here? Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

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

[PATCH] D109701: [clang] Emit SARIF Diagnostics: Create `clang::SarifDocumentWriter` interface

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added a comment.

In D109701#3648168 , @vaibhav.y wrote:

> In D109701#3646856 , @abrahamcd 
> wrote:
>
>> Hi! I'm interning with @cjdb and @denik this summer and I was working on 
>> adding a `-fdiagnostics-format=sarif` option to start off my project, but I 
>> just found that a previous abandoned version of this change (D109697 
>> ) was intending to add it. Seeing as the 
>> flag is no longer included in this version of the change, is it okay for me 
>> to go on and add it myself, or are you still planning on adding it here? 
>> Thanks!
>
> Sure, feel free to use D109697  as you see 
> fit!

Great, thank you!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D109701

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

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added a comment.

Gentle ping!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D129886: Adds `-fdiagnostics-format=sarif` flag option and warning.

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd created this revision.
Herald added a project: All.
abrahamcd requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D129886

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/TextDiagnostic.cpp


Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4006,6 +4006,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
+if (std::string(A->getValue()) == "sarif") {
+  D.Diag(diag::warn_drv_sarif_format_unstable);
+}
   }
 
   if (const Arg *A = Args.getLastArg(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5574,8 +5574,8 @@
 
 def fdiagnostics_format : Separate<["-"], "fdiagnostics-format">,
   HelpText<"Change diagnostic formatting to match IDE and command line tools">,
-  Values<"clang,msvc,vi">,
-  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi"]>,
+  Values<"clang,msvc,vi,sarif">,
+  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi", "SARIF"]>,
   MarshallingInfoEnum, "Clang">;
 def fdiagnostics_show_category : Separate<["-"], "fdiagnostics-show-category">,
   HelpText<"Print diagnostic category">,
Index: clang/include/clang/Basic/DiagnosticOptions.h
===
--- clang/include/clang/Basic/DiagnosticOptions.h
+++ clang/include/clang/Basic/DiagnosticOptions.h
@@ -74,7 +74,7 @@
   friend class CompilerInvocation;
 
 public:
-  enum TextDiagnosticFormat { Clang, MSVC, Vi };
+  enum TextDiagnosticFormat { Clang, MSVC, Vi, SARIF };
 
   // Default values.
   enum {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -678,4 +678,7 @@
 def err_drv_invalid_empty_dxil_validator_version : Error<
   "invalid validator version : %0\n"
   "If validator major version is 0, minor version must also be 0.">;
+
+def warn_drv_sarif_format_unstable : Warning<
+  "diagnostic formatting in SARIF mode is currently unstable">;
 }


Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4006,6 +4006,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
+if (std::string(A->getValue()) == "sarif") {
+  D.Diag(diag::warn_drv_sarif_format_unstable);
+}
   }
 
   if (const Arg *A = Args.getLastArg(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5574,8 +5574,8 @@
 
 def fdiagnostics_format : Separate<["-"], "fdiagnostics-format">,
   HelpText<"Change diagnostic formatting to 

[PATCH] D129886: Adds `-fdiagnostics-format=sarif` flag option and warning.

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 445073.
abrahamcd added a comment.

Edited commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129886

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/TextDiagnostic.cpp


Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4006,6 +4006,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
+if (std::string(A->getValue()) == "sarif") {
+  D.Diag(diag::warn_drv_sarif_format_unstable);
+}
   }
 
   if (const Arg *A = Args.getLastArg(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5574,8 +5574,8 @@
 
 def fdiagnostics_format : Separate<["-"], "fdiagnostics-format">,
   HelpText<"Change diagnostic formatting to match IDE and command line tools">,
-  Values<"clang,msvc,vi">,
-  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi"]>,
+  Values<"clang,msvc,vi,sarif">,
+  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi", "SARIF"]>,
   MarshallingInfoEnum, "Clang">;
 def fdiagnostics_show_category : Separate<["-"], "fdiagnostics-show-category">,
   HelpText<"Print diagnostic category">,
Index: clang/include/clang/Basic/DiagnosticOptions.h
===
--- clang/include/clang/Basic/DiagnosticOptions.h
+++ clang/include/clang/Basic/DiagnosticOptions.h
@@ -74,7 +74,7 @@
   friend class CompilerInvocation;
 
 public:
-  enum TextDiagnosticFormat { Clang, MSVC, Vi };
+  enum TextDiagnosticFormat { Clang, MSVC, Vi, SARIF };
 
   // Default values.
   enum {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -678,4 +678,7 @@
 def err_drv_invalid_empty_dxil_validator_version : Error<
   "invalid validator version : %0\n"
   "If validator major version is 0, minor version must also be 0.">;
+
+def warn_drv_sarif_format_unstable : Warning<
+  "diagnostic formatting in SARIF mode is currently unstable">;
 }


Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4006,6 +4006,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
+if (std::string(A->getValue()) == "sarif") {
+  D.Diag(diag::warn_drv_sarif_format_unstable);
+}
   }
 
   if (const Arg *A = Args.getLastArg(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5574,8 +5574,8 @@
 
 def fdiagnostics_format : Separate<["-"], "fdiagnostics-format">,
   HelpText<"Change diagnostic formatting to match IDE and comman

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif for SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 445077.
abrahamcd retitled this revision from "Adds `-fdiagnostics-format=sarif` flag 
option and warning." to "[clang] Add -fdiagnostics-format=sarif for SARIF 
diagnostics".
abrahamcd edited the summary of this revision.
abrahamcd added a comment.

Commit message edits did not go through


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129886

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/TextDiagnostic.cpp


Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4006,6 +4006,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
+if (std::string(A->getValue()) == "sarif") {
+  D.Diag(diag::warn_drv_sarif_format_unstable);
+}
   }
 
   if (const Arg *A = Args.getLastArg(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5574,8 +5574,8 @@
 
 def fdiagnostics_format : Separate<["-"], "fdiagnostics-format">,
   HelpText<"Change diagnostic formatting to match IDE and command line tools">,
-  Values<"clang,msvc,vi">,
-  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi"]>,
+  Values<"clang,msvc,vi,sarif">,
+  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi", "SARIF"]>,
   MarshallingInfoEnum, "Clang">;
 def fdiagnostics_show_category : Separate<["-"], "fdiagnostics-show-category">,
   HelpText<"Print diagnostic category">,
Index: clang/include/clang/Basic/DiagnosticOptions.h
===
--- clang/include/clang/Basic/DiagnosticOptions.h
+++ clang/include/clang/Basic/DiagnosticOptions.h
@@ -74,7 +74,7 @@
   friend class CompilerInvocation;
 
 public:
-  enum TextDiagnosticFormat { Clang, MSVC, Vi };
+  enum TextDiagnosticFormat { Clang, MSVC, Vi, SARIF };
 
   // Default values.
   enum {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -678,4 +678,7 @@
 def err_drv_invalid_empty_dxil_validator_version : Error<
   "invalid validator version : %0\n"
   "If validator major version is 0, minor version must also be 0.">;
+
+def warn_drv_sarif_format_unstable : Warning<
+  "diagnostic formatting in SARIF mode is currently unstable">;
 }


Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4006,6 +4006,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
+if (std::string(A->getValue()) == "sarif") {
+  D.Diag(diag::warn_drv_sarif_format_unstable);
+}
   }
 
   if (const Arg *A = Args.getLastArg(
Index: clang/include/clang/Driver/Options.td
===
--- cla

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 445554.
abrahamcd retitled this revision from "[clang] Add -fdiagnostics-format=sarif 
for SARIF diagnostics" to "[clang] Add -fdiagnostics-format=sarif option for 
future SARIF output".
abrahamcd edited the summary of this revision.
abrahamcd added a comment.

Adds test file and fixes `warning-flags` test failure by adding warning group.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129886

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Driver/fdiagnostics-format-sarif.cpp


Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===
--- /dev/null
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -0,0 +1 @@
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s
\ No newline at end of file
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4006,6 +4006,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
+if (std::string(A->getValue()) == "sarif") {
+  D.Diag(diag::warn_drv_sarif_format_unstable);
+}
   }
 
   if (const Arg *A = Args.getLastArg(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5580,8 +5580,8 @@
 
 def fdiagnostics_format : Separate<["-"], "fdiagnostics-format">,
   HelpText<"Change diagnostic formatting to match IDE and command line tools">,
-  Values<"clang,msvc,vi">,
-  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi"]>,
+  Values<"clang,msvc,vi,sarif">,
+  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi", "SARIF"]>,
   MarshallingInfoEnum, "Clang">;
 def fdiagnostics_show_category : Separate<["-"], "fdiagnostics-show-category">,
   HelpText<"Print diagnostic category">,
Index: clang/include/clang/Basic/DiagnosticOptions.h
===
--- clang/include/clang/Basic/DiagnosticOptions.h
+++ clang/include/clang/Basic/DiagnosticOptions.h
@@ -74,7 +74,7 @@
   friend class CompilerInvocation;
 
 public:
-  enum TextDiagnosticFormat { Clang, MSVC, Vi };
+  enum TextDiagnosticFormat { Clang, MSVC, Vi, SARIF };
 
   // Default values.
   enum {
Index: clang/include/clang/Basic/DiagnosticGroups.td
===
--- clang/include/clang/Basic/DiagnosticGroups.td
+++ clang/include/clang/Basic/DiagnosticGroups.td
@@ -1379,3 +1379,5 @@
 // HLSL diagnostic groups
 // Warnings for HLSL Clang extensions
 def HLSLExtension : DiagGroup<"hlsl-extensions">;
+
+def SarifFormatUnstable : DiagGroup<"sarif-format-unstable">;
\ No newline at end of file
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -678,4 +678,8 @@
 def err_drv_invalid_empty_dxil_validator_version : Error<
   "invalid validator version : %0\n"
   "If validator major version is 0, minor version must also be 0.">;
+
+def warn_drv_sarif_format_unstable : Warning<
+  "diagnostic formatting in SARIF mode is currently unstable">,
+  InGroup>;
 }


Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===
--- /dev/null
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -0,0 +1 @@
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s
\ No newline at end of file
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/Te

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added a comment.

In D129886#3656221 , @vaibhav.y wrote:

> Might be worth hiding it from `--help`, despite the instability warning.

I already couldn't find any mention of `-fdiagnostics-format` or `sarif` when I 
ran `clang --help`.  Is there somewhere else I should look?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129886

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

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticDriverKinds.td:684
+  "diagnostic formatting in SARIF mode is currently unstable">,
+  InGroup>;
 }

Please let me know if there's a better warning group you think this should be 
in.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129886

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

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 445845.
abrahamcd marked 5 inline comments as done.
abrahamcd edited the summary of this revision.
abrahamcd added a comment.

Update test file to check for warning and address other minor comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129886

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Driver/fdiagnostics-format-sarif.cpp


Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===
--- /dev/null
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -0,0 +1,2 @@
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s -### 2>&1 | 
FileCheck %s --check-prefix=WARN
+// WARN: warning: diagnostic formatting in SARIF mode is currently unstable 
[-Wsarif-format-unstable]
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4006,6 +4006,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
+if (StringRef(A->getValue()) == "sarif") {
+  D.Diag(diag::warn_drv_sarif_format_unstable);
+}
   }
 
   if (const Arg *A = Args.getLastArg(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5580,8 +5580,8 @@
 
 def fdiagnostics_format : Separate<["-"], "fdiagnostics-format">,
   HelpText<"Change diagnostic formatting to match IDE and command line tools">,
-  Values<"clang,msvc,vi">,
-  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi"]>,
+  Values<"clang,msvc,vi,sarif">,
+  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi", "SARIF"]>,
   MarshallingInfoEnum, "Clang">;
 def fdiagnostics_show_category : Separate<["-"], "fdiagnostics-show-category">,
   HelpText<"Print diagnostic category">,
Index: clang/include/clang/Basic/DiagnosticOptions.h
===
--- clang/include/clang/Basic/DiagnosticOptions.h
+++ clang/include/clang/Basic/DiagnosticOptions.h
@@ -74,7 +74,7 @@
   friend class CompilerInvocation;
 
 public:
-  enum TextDiagnosticFormat { Clang, MSVC, Vi };
+  enum TextDiagnosticFormat { Clang, MSVC, Vi, SARIF };
 
   // Default values.
   enum {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -678,4 +678,8 @@
 def err_drv_invalid_empty_dxil_validator_version : Error<
   "invalid validator version : %0\n"
   "If validator major version is 0, minor version must also be 0.">;
+
+def warn_drv_sarif_format_unstable : Warning<
+  "diagnostic formatting in SARIF mode is currently unstable">,
+  InGroup>;
 }


Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===
--- /dev/null
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -0,0 +1,2 @@
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s -### 2>&1 | FileCheck %s --check-prefix=WARN
+// WARN: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOp

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 445904.
abrahamcd marked 2 inline comments as done.
abrahamcd added a comment.
Herald added a reviewer: jdoerfert.
Herald added subscribers: abrachet, sstefan1, phosek, ormris.

Enabled both "sarif" and "SARIF" as flag spellings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129886

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Driver/fdiagnostics-format-sarif.cpp

Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===
--- /dev/null
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s -### 2>&1 | FileCheck %s --check-prefix=WARN
+// WARN: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+
+// RUN: %clang -fsyntax-only -fdiagnostics-format=SARIF %s -### 2>&1 | FileCheck %s --check-prefix=WARN2
+// WARN2: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -77,8 +77,8 @@
   if (Args.hasArg(options::OPT_static))
 if (const Arg *A =
 Args.getLastArg(options::OPT_dynamic, options::OPT_mdynamic_no_pic))
-  D.Diag(diag::err_drv_argument_not_allowed_with) << A->getAsString(Args)
-  << "-static";
+  D.Diag(diag::err_drv_argument_not_allowed_with)
+  << A->getAsString(Args) << "-static";
 }
 
 // Add backslashes to escape spaces and other backslashes.
@@ -135,8 +135,8 @@
 /// parameter in reciprocal argument strings. Return false if there is an error
 /// parsing the refinement step. Otherwise, return true and set the Position
 /// of the refinement step in the input string.
-static bool getRefinementStep(StringRef In, const Driver &D,
-  const Arg &A, size_t &Position) {
+static bool getRefinementStep(StringRef In, const Driver &D, const Arg &A,
+  size_t &Position) {
   const char RefinementStepToken = ':';
   Position = In.find(RefinementStepToken);
   if (Position != StringRef::npos) {
@@ -255,7 +255,8 @@
 
 // If the precision was not specified, also mark the double and half entry
 // as found.
-if (ValBase.back() != 'f' && ValBase.back() != 'd' && ValBase.back() != 'h') {
+if (ValBase.back() != 'f' && ValBase.back() != 'd' &&
+ValBase.back() != 'h') {
   OptionStrings[ValBase.str() + 'd'] = true;
   OptionStrings[ValBase.str() + 'h'] = true;
 }
@@ -503,7 +504,7 @@
 }
 
 static bool mustUseNonLeafFramePointerForTarget(const llvm::Triple &Triple) {
-  switch (Triple.getArch()){
+  switch (Triple.getArch()) {
   default:
 return false;
   case llvm::Triple::arm:
@@ -699,7 +700,7 @@
 
 /// Add a CC1 and CC1AS option to specify the coverage file path prefix map.
 static void addCoveragePrefixMapArg(const Driver &D, const ArgList &Args,
-   ArgStringList &CmdArgs) {
+ArgStringList &CmdArgs) {
   for (const Arg *A : Args.filtered(options::OPT_ffile_prefix_map_EQ,
 options::OPT_fcoverage_prefix_map_EQ)) {
 StringRef Map = A->getValue();
@@ -795,13 +796,12 @@
   CSPGOGenerateArg->getOption().matches(options::OPT_fno_profile_generate))
 CSPGOGenerateArg = nullptr;
 
-  auto *ProfileGenerateArg = Args.getLastArg(
-  options::OPT_fprofile_instr_generate,
-  options::OPT_fprofile_instr_generate_EQ,
-  options::OPT_fno_profile_instr_generate);
-  if (ProfileGenerateArg &&
-  ProfileGenerateArg->getOption().matches(
-  options::OPT_fno_profile_instr_generate))
+  auto *ProfileGenerateArg =
+  Args.getLastArg(options::OPT_fprofile_instr_generate,
+  options::OPT_fprofile_instr_generate_EQ,
+

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:4009-4011
+if (StringRef(A->getValue()) == "sarif") {
+  D.Diag(diag::warn_drv_sarif_format_unstable);
+}

cjdb wrote:
> aaron.ballman wrote:
> > Do we want to be kind to people who spell it SARIF (given that it's an 
> > acronym, some folks may spell it that way reflexively)?
> tbh I think it's worth accepting both `sarif` and `SARIF`, if we can.
Ah yeah that would definitely be helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129886

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

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 445917.
abrahamcd marked an inline comment as done.
abrahamcd added a comment.

Removed unintended formatting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129886

Files:
  clang/include/clang/Basic/DiagnosticDriverKinds.td
  clang/include/clang/Basic/DiagnosticOptions.h
  clang/include/clang/Driver/Options.td
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/TextDiagnostic.cpp
  clang/test/Driver/fdiagnostics-format-sarif.cpp


Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===
--- /dev/null
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s -### 2>&1 | 
FileCheck %s --check-prefix=WARN
+// WARN: warning: diagnostic formatting in SARIF mode is currently unstable 
[-Wsarif-format-unstable]
+
+// RUN: %clang -fsyntax-only -fdiagnostics-format=SARIF %s -### 2>&1 | 
FileCheck %s --check-prefix=WARN2
+// WARN2: warning: diagnostic formatting in SARIF mode is currently unstable 
[-Wsarif-format-unstable]
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagnostic.cpp
+++ clang/lib/Frontend/TextDiagnostic.cpp
@@ -815,6 +815,7 @@
 
   emitFilename(PLoc.getFilename(), Loc.getManager());
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
 if (DiagOpts->ShowLine)
   OS << ':' << LineNo;
@@ -837,6 +838,7 @@
   OS << ColNo;
 }
   switch (DiagOpts->getFormat()) {
+  case DiagnosticOptions::SARIF:
   case DiagnosticOptions::Clang:
   case DiagnosticOptions::Vi:OS << ':';break;
   case DiagnosticOptions::MSVC:
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4006,6 +4006,9 @@
   if (const Arg *A = Args.getLastArg(options::OPT_fdiagnostics_format_EQ)) {
 CmdArgs.push_back("-fdiagnostics-format");
 CmdArgs.push_back(A->getValue());
+if (StringRef(A->getValue()) == "sarif" ||
+StringRef(A->getValue()) == "SARIF")
+  D.Diag(diag::warn_drv_sarif_format_unstable);
   }
 
   if (const Arg *A = Args.getLastArg(
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5584,8 +5584,8 @@
 
 def fdiagnostics_format : Separate<["-"], "fdiagnostics-format">,
   HelpText<"Change diagnostic formatting to match IDE and command line tools">,
-  Values<"clang,msvc,vi">,
-  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi"]>,
+  Values<"clang,msvc,vi,sarif,SARIF">,
+  NormalizedValuesScope<"DiagnosticOptions">, NormalizedValues<["Clang", 
"MSVC", "Vi", "SARIF", "SARIF"]>,
   MarshallingInfoEnum, "Clang">;
 def fdiagnostics_show_category : Separate<["-"], "fdiagnostics-show-category">,
   HelpText<"Print diagnostic category">,
Index: clang/include/clang/Basic/DiagnosticOptions.h
===
--- clang/include/clang/Basic/DiagnosticOptions.h
+++ clang/include/clang/Basic/DiagnosticOptions.h
@@ -74,7 +74,7 @@
   friend class CompilerInvocation;
 
 public:
-  enum TextDiagnosticFormat { Clang, MSVC, Vi };
+  enum TextDiagnosticFormat { Clang, MSVC, Vi, SARIF };
 
   // Default values.
   enum {
Index: clang/include/clang/Basic/DiagnosticDriverKinds.td
===
--- clang/include/clang/Basic/DiagnosticDriverKinds.td
+++ clang/include/clang/Basic/DiagnosticDriverKinds.td
@@ -678,4 +678,8 @@
 def err_drv_invalid_empty_dxil_validator_version : Error<
   "invalid validator version : %0\n"
   "If validator major version is 0, minor version must also be 0.">;
+
+def warn_drv_sarif_format_unstable : Warning<
+  "diagnostic formatting in SARIF mode is currently unstable">,
+  InGroup>;
 }


Index: clang/test/Driver/fdiagnostics-format-sarif.cpp
===
--- /dev/null
+++ clang/test/Driver/fdiagnostics-format-sarif.cpp
@@ -0,0 +1,5 @@
+// RUN: %clang -fsyntax-only -fdiagnostics-format=sarif %s -### 2>&1 | FileCheck %s --check-prefix=WARN
+// WARN: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+
+// RUN: %clang -fsyntax-only -fdiagnostics-format=SARIF %s -### 2>&1 | FileCheck %s --check-prefix=WARN2
+// WARN2: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
Index: clang/lib/Frontend/TextDiagnostic.cpp
===
--- clang/lib/Frontend/TextDiagn

[PATCH] D129886: [clang] Add -fdiagnostics-format=sarif option for future SARIF output

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:80-81
 Args.getLastArg(options::OPT_dynamic, 
options::OPT_mdynamic_no_pic))
-  D.Diag(diag::err_drv_argument_not_allowed_with) << A->getAsString(Args)
-  << "-static";
+  D.Diag(diag::err_drv_argument_not_allowed_with)
+  << A->getAsString(Args) << "-static";
 }

vaibhav.y wrote:
> nit: Looks like this an unrelated format change/ rebase issue. There are a 
> few others as well in this file
> 
> Would be better to introduce these through another CR, or use `git 
> clang-format  []` to limit it to the diff.
Thanks! Did not realize format changed all that :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129886

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

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 452833.
abrahamcd retitled this revision from "[WIP] Enable SARIF Diagnostics" to 
"[clang] Enable output of SARIF diagnostics".
abrahamcd edited the summary of this revision.
abrahamcd added a comment.

Removed unused remaining parts of traditional text diagnostics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/SARIFDiagnosticTest.cpp
  clang/unittests/Frontend/sarif-diagnostics.cpp

Index: clang/unittests/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/unittests/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,137 @@
+// RUN: %clang -fdiagnostics-format=sarif %s -o %t.exe -DGTEST
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s 2>
+// %t.diags || true RUN: %t.exe < %t.diags
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace {
+
+constexpr llvm::StringRef BrokenProgram = R"(// Example errors below start on line 2
+void main() {
+  int i = hello;
+
+  float test = 1a.0;
+
+  if (true)
+bool Yes = true;
+return;
+
+  bool j = hi;
+}
+})";
+
+TEST(SARIFDiagnosticTest, TestFields) {
+  llvm::SmallString<256> SearchDir;
+  llvm::sys::fs::current_path(SearchDir);
+  
+  SearchDir.append("/../../../bin");
+  // ASSERT_EQ(SearchDir.str(), "hi");
+  llvm::ErrorOr ClangPathOrErr =
+  llvm::sys::findProgramByName("clang", {SearchDir});
+  ASSERT_TRUE(ClangPathOrErr);
+  const std::string &ClangPath = *ClangPathOrErr;
+  // ASSERT_EQ(ClangPath, "hi");
+
+  llvm::ErrorOr EchoPathOrErr =
+  llvm::sys::findProgramByName("echo");
+  ASSERT_TRUE(EchoPathOrErr);
+  const std::string &EchoPath = *EchoPathOrErr;
+
+  int EchoInputFD;
+  llvm::SmallString<32> EchoInputFile, EchoOutputFile;
+  llvm::sys::fs::createTemporaryFile("echo-input", "", EchoInputFD,
+ EchoInputFile);
+  llvm::sys::fs::createTemporaryFile("echo-output", "", EchoOutputFile);
+  llvm::FileRemover InputRemover(EchoInputFile.c_str());
+  llvm::FileRemover OutputRemover(EchoOutputFile.c_str());
+
+  llvm::Optional Redirects[] = {
+  EchoInputFile.str(), EchoOutputFile.str(), llvm::StringRef("")};
+
+  int RunResult = llvm::sys::ExecuteAndWait(EchoPath, {"echo", BrokenProgram},
+llvm::None, Redirects);
+  ASSERT_EQ(RunResult, 0);
+
+  // auto EchoOutputBuf = llvm::MemoryBuffer::getFile(EchoOutputFile.c_str());
+  // ASSERT_TRUE(EchoOutputBuf);
+  // llvm::StringRef EchoOutput = EchoOutputBuf.get()->getBuffer();
+  // ASSERT_EQ(EchoOutput.str(), "hi");
+
+  llvm::SmallString<32> ClangErrFile;
+  llvm::sys::fs::createTemporaryFile("clang-err", "", ClangErrFile);
+  llvm::FileRemover ClangErrRemover(ClangErrFile.c_str());
+
+  llvm::Optional ClangRedirects[] = {
+  EchoOutputFile.str(), llvm::StringRef(""), ClangErrFile.str()};
+  llvm::StringRef Args[] = {"clang",
+"-xc++",
+"-",
+"-fsyntax-only",
+"-Wall",
+"-Wextra",
+"-fdiagnostics-format=sarif"};
+
+  int ClangResult =
+  llvm::sys::ExecuteAndWait(ClangPath, Args, llvm::None, ClangRedirects);
+  ASSERT_EQ(ClangResult, 1);
+
+  // auto ClangOutputBuf = llvm::MemoryBuffer::getFile(ClangOutputFile.c_str());
+  // ASSERT_TRUE(ClangOutputBuf);
+  // llvm::StringRef ClangOutput = ClangOutputBuf.get()->getBuffer();
+  // ASSERT_EQ(ClangOutput.str(), "hi");
+
+  auto ClangErrBuf = llvm::MemoryBuffer::getFile(ClangErrFile.c_str());
+  ASSERT_TRUE(ClangErrBuf);
+  llvm::StringRef ClangErr = ClangErrBuf.get()->getBuffer();
+  ASSERT_EQ(ClangErr.str(), "hi");
+
+  llvm::Expected Value = llvm::json::parse(ClangErr.str());
+  ASSERT_FALSE(!Value);
+
+  llvm::json::Object *SarifDoc = Value->getAsObject();
+
+  const llvm::json::Array *Runs = SarifDoc->getArray("runs");
+  const llvm::json::Object *TheRun = Runs->back().getAsObject();
+  const llvm::json::Array *Results = TheRun->getArray("results");
+  
+  // Check Artifacts
+  const llvm::json::Array *Artifacts = TheRun->getArray("artifacts");
+  const llvm::json::Obje

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 452840.
abrahamcd added a comment.

Minor cleanup.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/SARIFDiagnosticTest.cpp

Index: clang/unittests/Frontend/SARIFDiagnosticTest.cpp
===
--- /dev/null
+++ clang/unittests/Frontend/SARIFDiagnosticTest.cpp
@@ -0,0 +1,137 @@
+// RUN: %clang -fdiagnostics-format=sarif %s -o %t.exe -DGTEST
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s 2>
+// %t.diags || true RUN: %t.exe < %t.diags
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace {
+
+constexpr llvm::StringRef BrokenProgram = R"(// Example errors below start on line 2
+void main() {
+  int i = hello;
+
+  float test = 1a.0;
+
+  if (true)
+bool Yes = true;
+return;
+
+  bool j = hi;
+}
+})";
+
+TEST(SARIFDiagnosticTest, TestFields) {
+  llvm::SmallString<256> SearchDir;
+  llvm::sys::fs::current_path(SearchDir);
+  
+  SearchDir.append("/../../../bin");
+  // ASSERT_EQ(SearchDir.str(), "hi");
+  llvm::ErrorOr ClangPathOrErr =
+  llvm::sys::findProgramByName("clang", {SearchDir});
+  ASSERT_TRUE(ClangPathOrErr);
+  const std::string &ClangPath = *ClangPathOrErr;
+  // ASSERT_EQ(ClangPath, "hi");
+
+  llvm::ErrorOr EchoPathOrErr =
+  llvm::sys::findProgramByName("echo");
+  ASSERT_TRUE(EchoPathOrErr);
+  const std::string &EchoPath = *EchoPathOrErr;
+
+  int EchoInputFD;
+  llvm::SmallString<32> EchoInputFile, EchoOutputFile;
+  llvm::sys::fs::createTemporaryFile("echo-input", "", EchoInputFD,
+ EchoInputFile);
+  llvm::sys::fs::createTemporaryFile("echo-output", "", EchoOutputFile);
+  llvm::FileRemover InputRemover(EchoInputFile.c_str());
+  llvm::FileRemover OutputRemover(EchoOutputFile.c_str());
+
+  llvm::Optional Redirects[] = {
+  EchoInputFile.str(), EchoOutputFile.str(), llvm::StringRef("")};
+
+  int RunResult = llvm::sys::ExecuteAndWait(EchoPath, {"echo", BrokenProgram},
+llvm::None, Redirects);
+  ASSERT_EQ(RunResult, 0);
+
+  // auto EchoOutputBuf = llvm::MemoryBuffer::getFile(EchoOutputFile.c_str());
+  // ASSERT_TRUE(EchoOutputBuf);
+  // llvm::StringRef EchoOutput = EchoOutputBuf.get()->getBuffer();
+  // ASSERT_EQ(EchoOutput.str(), "hi");
+
+  llvm::SmallString<32> ClangErrFile;
+  llvm::sys::fs::createTemporaryFile("clang-err", "", ClangErrFile);
+  llvm::FileRemover ClangErrRemover(ClangErrFile.c_str());
+
+  llvm::Optional ClangRedirects[] = {
+  EchoOutputFile.str(), llvm::StringRef(""), ClangErrFile.str()};
+  llvm::StringRef Args[] = {"clang",
+"-xc++",
+"-",
+"-fsyntax-only",
+"-Wall",
+"-Wextra",
+"-fdiagnostics-format=sarif"};
+
+  int ClangResult =
+  llvm::sys::ExecuteAndWait(ClangPath, Args, llvm::None, ClangRedirects);
+  ASSERT_EQ(ClangResult, 1);
+
+  // auto ClangOutputBuf = llvm::MemoryBuffer::getFile(ClangOutputFile.c_str());
+  // ASSERT_TRUE(ClangOutputBuf);
+  // llvm::StringRef ClangOutput = ClangOutputBuf.get()->getBuffer();
+  // ASSERT_EQ(ClangOutput.str(), "hi");
+
+  auto ClangErrBuf = llvm::MemoryBuffer::getFile(ClangErrFile.c_str());
+  ASSERT_TRUE(ClangErrBuf);
+  llvm::StringRef ClangErr = ClangErrBuf.get()->getBuffer();
+  ASSERT_EQ(ClangErr.str(), "hi");
+
+  llvm::Expected Value = llvm::json::parse(ClangErr.str());
+  ASSERT_FALSE(!Value);
+
+  llvm::json::Object *SarifDoc = Value->getAsObject();
+
+  const llvm::json::Array *Runs = SarifDoc->getArray("runs");
+  const llvm::json::Object *TheRun = Runs->back().getAsObject();
+  const llvm::json::Array *Results = TheRun->getArray("results");
+  
+  // Check Artifacts
+  const llvm::json::Array *Artifacts = TheRun->getArray("artifacts");
+  const llvm::json::Object *TheArtifact = Artifacts->back().getAsObject();
+  const llvm::json::Object *Location = TheArtifact->getObject("location");
+
+  ASSERT_TRUE(Location->getInteger("index").has_value());
+  ASSERT_TRUE(Location->getString("uri").has_value());
+
+  EXPECT_EQ(Loc

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 452843.
abrahamcd added a comment.

Naming fixes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/SARIFDiagnosticTest.cpp

Index: clang/unittests/Frontend/SARIFDiagnosticTest.cpp
===
--- /dev/null
+++ clang/unittests/Frontend/SARIFDiagnosticTest.cpp
@@ -0,0 +1,123 @@
+// RUN: %clang -fdiagnostics-format=sarif %s -o %t.exe -DGTEST
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s 2>
+// %t.diags || true RUN: %t.exe < %t.diags
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace {
+
+constexpr llvm::StringRef BrokenProgram = R"(// Example errors below start on line 2
+void main() {
+  int i = hello;
+
+  float test = 1a.0;
+
+  if (true)
+bool Yes = true;
+return;
+
+  bool j = hi;
+}
+})";
+
+TEST(SARIFDiagnosticTest, TestFields) {
+  llvm::SmallString<256> SearchDir;
+  llvm::sys::fs::current_path(SearchDir);
+  
+  SearchDir.append("/../../../bin");
+
+  llvm::ErrorOr ClangPathOrErr =
+  llvm::sys::findProgramByName("clang", {SearchDir});
+  ASSERT_TRUE(ClangPathOrErr);
+  const std::string &ClangPath = *ClangPathOrErr;
+
+  llvm::ErrorOr EchoPathOrErr =
+  llvm::sys::findProgramByName("echo");
+  ASSERT_TRUE(EchoPathOrErr);
+  const std::string &EchoPath = *EchoPathOrErr;
+
+  int EchoInputFD;
+  llvm::SmallString<32> EchoInputFile, EchoOutputFile;
+  llvm::sys::fs::createTemporaryFile("echo-input", "", EchoInputFD,
+ EchoInputFile);
+  llvm::sys::fs::createTemporaryFile("echo-output", "", EchoOutputFile);
+  llvm::FileRemover InputRemover(EchoInputFile.c_str());
+  llvm::FileRemover OutputRemover(EchoOutputFile.c_str());
+
+  llvm::Optional Redirects[] = {
+  EchoInputFile.str(), EchoOutputFile.str(), llvm::StringRef("")};
+
+  int RunResult = llvm::sys::ExecuteAndWait(EchoPath, {"echo", BrokenProgram},
+llvm::None, Redirects);
+  ASSERT_EQ(RunResult, 0);
+
+  llvm::SmallString<32> ClangErrFile;
+  llvm::sys::fs::createTemporaryFile("clang-err", "", ClangErrFile);
+  llvm::FileRemover ClangErrRemover(ClangErrFile.c_str());
+
+  llvm::Optional ClangRedirects[] = {
+  EchoOutputFile.str(), llvm::StringRef(""), ClangErrFile.str()};
+  llvm::StringRef Args[] = {"clang",
+"-xc++",
+"-",
+"-fsyntax-only",
+"-Wall",
+"-Wextra",
+"-fdiagnostics-format=sarif"};
+
+  int ClangResult =
+  llvm::sys::ExecuteAndWait(ClangPath, Args, llvm::None, ClangRedirects);
+  ASSERT_EQ(ClangResult, 1);
+
+  auto ClangErrBuf = llvm::MemoryBuffer::getFile(ClangErrFile.c_str());
+  ASSERT_TRUE(ClangErrBuf);
+  llvm::StringRef ClangErr = ClangErrBuf.get()->getBuffer();
+  ASSERT_EQ(ClangErr.str(), "hi");
+
+  llvm::Expected Value = llvm::json::parse(ClangErr.str());
+  ASSERT_FALSE(!Value);
+
+  llvm::json::Object *SarifDoc = Value->getAsObject();
+
+  const llvm::json::Array *Runs = SarifDoc->getArray("runs");
+  const llvm::json::Object *TheRun = Runs->back().getAsObject();
+  const llvm::json::Array *Results = TheRun->getArray("results");
+  
+  // Check Artifacts
+  const llvm::json::Array *Artifacts = TheRun->getArray("artifacts");
+  const llvm::json::Object *TheArtifact = Artifacts->back().getAsObject();
+  const llvm::json::Object *Location = TheArtifact->getObject("location");
+
+  ASSERT_TRUE(Location->getInteger("index").has_value());
+  ASSERT_TRUE(Location->getString("uri").has_value());
+
+  EXPECT_EQ(Location->getInteger("index").value(), 0);
+  EXPECT_EQ(Location->getString("uri").value(), "file://");
+
+  // Check Driver
+  const llvm::json::Object *Driver =
+  TheRun->getObject("tool")->getObject("driver");
+
+  ASSERT_TRUE(Driver->getString("name").has_value());
+  ASSERT_TRUE(Driver->getString("fullName").has_value());
+
+  EXPECT_EQ(Driver->getString("name").value(), "clang");
+  EXPECT_EQ(Driver->getString("fullName").value(), "clang-15");
+
+  // Check Rules
+  const llvm::json::Array *Rules = Driver->getArray("r

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:75
+emitFilename(FE->getName(), Loc.getManager());
+// FIXME: No current way to add file-only location to SARIF object
+  }

@vaibhav.y , just wanted to confirm, the SarifDiagnosticWriter can only add 
locations that include row/column numbers, correct?



Comment at: clang/lib/Frontend/SARIFDiagnosticPrinter.cpp:71
+  // other infrastructure necessary when emitting more rich diagnostics.
+  if (!Info.getLocation().isValid()) { // TODO: What is this case? 
+// SARIFDiag->addDiagnosticWithoutLocation(

Does anyone know specific kinds of diagnostics that might be covered here in 
traditional diagnostics? I'm having trouble thinking of how we can support a 
simpler path for these kinds of diagnostics in SARIF since the Writer requires 
a source manager.



Comment at: clang/test/Frontend/sarif-diagnostics.cpp:3
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: 
{"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":2953,"location":{"index":0,"uri":"file:///usr/local/google/home/abrahamcd/projects/llvm-project/clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":5}}}],"message":{"text":";'main'
 must return 
'int'"},"ruleId":"3463","ruleIndex":0},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":6}}}],"message":{"text":"use
 of undeclared identifier 
'hello'"},"ruleId":"4601","ruleIndex":1},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":8}}}],"message":{"text":"invalid
 digit 'a' in decimal 
constant"},"ruleId":"898","ruleIndex":2},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":12}}}],"message":{"text":"misleading
 indentation; statement is not part of the previous 
'if'"},"ruleId":"1806","ruleIndex":3},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":10}}}],"message":{"text":"previous
 statement is 
here"},"ruleId":"1730","ruleIndex":4},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":11}}}],"message":{"text":"unused
 variable 
'Yes'"},"ruleId":"6536","ruleIndex":5},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":14}}}],"message":{"text":"use
 of undeclared identifier 
'hi'"},"ruleId":"4601","ruleIndex":6},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":16}}}],"message":{"text":"extraneous
 closing brace 
('}')"},"ruleId":"1399","ruleIndex":7}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"fullDescription":{"text":""},"id":"3463","name":""},{"fullDescription":{"text":""},"id":"4601","name":""},{"fullDescription":{"text":""},"id":"898","name":""},{"fullDescription":{"text":""},"id":"1806","name":""},{"fullDescription":{"text":""},"id":"1730","name":""},{"fullDescription":{"text":""},"id":"6536","name":""},{"fullDescription":{"text":""},"id":"4601","name":""},{"fullDescription":{"text":""},"id":"1399","name":""}],"version":"16.0.0"}}}],"version":"2.1.0"}
+

In regards to testing this, my understanding is that we are moving away from 
the unit test GTest testing, correct? I included a FileCheck implementation as 
well, but I wasn't sure how effective just checking the whole SARIF object like 
this would be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

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

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 453126.
abrahamcd added a comment.

Fixed FileCheck test case and added multiple source range error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/SARIFDiagnosticTest.cpp

Index: clang/unittests/Frontend/SARIFDiagnosticTest.cpp
===
--- /dev/null
+++ clang/unittests/Frontend/SARIFDiagnosticTest.cpp
@@ -0,0 +1,123 @@
+// RUN: %clang -fdiagnostics-format=sarif %s -o %t.exe -DGTEST
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s 2>
+// %t.diags || true RUN: %t.exe < %t.diags
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace {
+
+constexpr llvm::StringRef BrokenProgram = R"(// Example errors below start on line 2
+void main() {
+  int i = hello;
+
+  float test = 1a.0;
+
+  if (true)
+bool Yes = true;
+return;
+
+  bool j = hi;
+}
+})";
+
+TEST(SARIFDiagnosticTest, TestFields) {
+  llvm::SmallString<256> SearchDir;
+  llvm::sys::fs::current_path(SearchDir);
+  
+  SearchDir.append("/../../../bin");
+
+  llvm::ErrorOr ClangPathOrErr =
+  llvm::sys::findProgramByName("clang", {SearchDir});
+  ASSERT_TRUE(ClangPathOrErr);
+  const std::string &ClangPath = *ClangPathOrErr;
+
+  llvm::ErrorOr EchoPathOrErr =
+  llvm::sys::findProgramByName("echo");
+  ASSERT_TRUE(EchoPathOrErr);
+  const std::string &EchoPath = *EchoPathOrErr;
+
+  int EchoInputFD;
+  llvm::SmallString<32> EchoInputFile, EchoOutputFile;
+  llvm::sys::fs::createTemporaryFile("echo-input", "", EchoInputFD,
+ EchoInputFile);
+  llvm::sys::fs::createTemporaryFile("echo-output", "", EchoOutputFile);
+  llvm::FileRemover InputRemover(EchoInputFile.c_str());
+  llvm::FileRemover OutputRemover(EchoOutputFile.c_str());
+
+  llvm::Optional Redirects[] = {
+  EchoInputFile.str(), EchoOutputFile.str(), llvm::StringRef("")};
+
+  int RunResult = llvm::sys::ExecuteAndWait(EchoPath, {"echo", BrokenProgram},
+llvm::None, Redirects);
+  ASSERT_EQ(RunResult, 0);
+
+  llvm::SmallString<32> ClangErrFile;
+  llvm::sys::fs::createTemporaryFile("clang-err", "", ClangErrFile);
+  llvm::FileRemover ClangErrRemover(ClangErrFile.c_str());
+
+  llvm::Optional ClangRedirects[] = {
+  EchoOutputFile.str(), llvm::StringRef(""), ClangErrFile.str()};
+  llvm::StringRef Args[] = {"clang",
+"-xc++",
+"-",
+"-fsyntax-only",
+"-Wall",
+"-Wextra",
+"-fdiagnostics-format=sarif"};
+
+  int ClangResult =
+  llvm::sys::ExecuteAndWait(ClangPath, Args, llvm::None, ClangRedirects);
+  ASSERT_EQ(ClangResult, 1);
+
+  auto ClangErrBuf = llvm::MemoryBuffer::getFile(ClangErrFile.c_str());
+  ASSERT_TRUE(ClangErrBuf);
+  llvm::StringRef ClangErr = ClangErrBuf.get()->getBuffer();
+  ASSERT_EQ(ClangErr.str(), "hi");
+
+  llvm::Expected Value = llvm::json::parse(ClangErr.str());
+  ASSERT_FALSE(!Value);
+
+  llvm::json::Object *SarifDoc = Value->getAsObject();
+
+  const llvm::json::Array *Runs = SarifDoc->getArray("runs");
+  const llvm::json::Object *TheRun = Runs->back().getAsObject();
+  const llvm::json::Array *Results = TheRun->getArray("results");
+  
+  // Check Artifacts
+  const llvm::json::Array *Artifacts = TheRun->getArray("artifacts");
+  const llvm::json::Object *TheArtifact = Artifacts->back().getAsObject();
+  const llvm::json::Object *Location = TheArtifact->getObject("location");
+
+  ASSERT_TRUE(Location->getInteger("index").has_value());
+  ASSERT_TRUE(Location->getString("uri").has_value());
+
+  EXPECT_EQ(Location->getInteger("index").value(), 0);
+  EXPECT_EQ(Location->getString("uri").value(), "file://");
+
+  // Check Driver
+  const llvm::json::Object *Driver =
+  TheRun->getObject("tool")->getObject("driver");
+
+  ASSERT_TRUE(Driver->getString("name").has_value());
+  ASSERT_TRUE(Driver->getString("fullName").has_value());
+
+  EXPECT_EQ(Driver->getString("name").value(), "clang");
+  EXPECT_EQ(Driver->getString("fullName").value(), "clang-15");
+
+  // Check Rules
+  c

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd marked 2 inline comments as done.
abrahamcd added a comment.

Hi, checking in on this again, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

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

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 453346.
abrahamcd added a comment.

Removed Clang name from FileCheck test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/SARIFDiagnosticTest.cpp

Index: clang/unittests/Frontend/SARIFDiagnosticTest.cpp
===
--- /dev/null
+++ clang/unittests/Frontend/SARIFDiagnosticTest.cpp
@@ -0,0 +1,123 @@
+// RUN: %clang -fdiagnostics-format=sarif %s -o %t.exe -DGTEST
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s 2>
+// %t.diags || true RUN: %t.exe < %t.diags
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace {
+
+constexpr llvm::StringRef BrokenProgram = R"(// Example errors below start on line 2
+void main() {
+  int i = hello;
+
+  float test = 1a.0;
+
+  if (true)
+bool Yes = true;
+return;
+
+  bool j = hi;
+}
+})";
+
+TEST(SARIFDiagnosticTest, TestFields) {
+  llvm::SmallString<256> SearchDir;
+  llvm::sys::fs::current_path(SearchDir);
+  
+  SearchDir.append("/../../../bin");
+
+  llvm::ErrorOr ClangPathOrErr =
+  llvm::sys::findProgramByName("clang", {SearchDir});
+  ASSERT_TRUE(ClangPathOrErr);
+  const std::string &ClangPath = *ClangPathOrErr;
+
+  llvm::ErrorOr EchoPathOrErr =
+  llvm::sys::findProgramByName("echo");
+  ASSERT_TRUE(EchoPathOrErr);
+  const std::string &EchoPath = *EchoPathOrErr;
+
+  int EchoInputFD;
+  llvm::SmallString<32> EchoInputFile, EchoOutputFile;
+  llvm::sys::fs::createTemporaryFile("echo-input", "", EchoInputFD,
+ EchoInputFile);
+  llvm::sys::fs::createTemporaryFile("echo-output", "", EchoOutputFile);
+  llvm::FileRemover InputRemover(EchoInputFile.c_str());
+  llvm::FileRemover OutputRemover(EchoOutputFile.c_str());
+
+  llvm::Optional Redirects[] = {
+  EchoInputFile.str(), EchoOutputFile.str(), llvm::StringRef("")};
+
+  int RunResult = llvm::sys::ExecuteAndWait(EchoPath, {"echo", BrokenProgram},
+llvm::None, Redirects);
+  ASSERT_EQ(RunResult, 0);
+
+  llvm::SmallString<32> ClangErrFile;
+  llvm::sys::fs::createTemporaryFile("clang-err", "", ClangErrFile);
+  llvm::FileRemover ClangErrRemover(ClangErrFile.c_str());
+
+  llvm::Optional ClangRedirects[] = {
+  EchoOutputFile.str(), llvm::StringRef(""), ClangErrFile.str()};
+  llvm::StringRef Args[] = {"clang",
+"-xc++",
+"-",
+"-fsyntax-only",
+"-Wall",
+"-Wextra",
+"-fdiagnostics-format=sarif"};
+
+  int ClangResult =
+  llvm::sys::ExecuteAndWait(ClangPath, Args, llvm::None, ClangRedirects);
+  ASSERT_EQ(ClangResult, 1);
+
+  auto ClangErrBuf = llvm::MemoryBuffer::getFile(ClangErrFile.c_str());
+  ASSERT_TRUE(ClangErrBuf);
+  llvm::StringRef ClangErr = ClangErrBuf.get()->getBuffer();
+  ASSERT_EQ(ClangErr.str(), "hi");
+
+  llvm::Expected Value = llvm::json::parse(ClangErr.str());
+  ASSERT_FALSE(!Value);
+
+  llvm::json::Object *SarifDoc = Value->getAsObject();
+
+  const llvm::json::Array *Runs = SarifDoc->getArray("runs");
+  const llvm::json::Object *TheRun = Runs->back().getAsObject();
+  const llvm::json::Array *Results = TheRun->getArray("results");
+  
+  // Check Artifacts
+  const llvm::json::Array *Artifacts = TheRun->getArray("artifacts");
+  const llvm::json::Object *TheArtifact = Artifacts->back().getAsObject();
+  const llvm::json::Object *Location = TheArtifact->getObject("location");
+
+  ASSERT_TRUE(Location->getInteger("index").has_value());
+  ASSERT_TRUE(Location->getString("uri").has_value());
+
+  EXPECT_EQ(Location->getInteger("index").value(), 0);
+  EXPECT_EQ(Location->getString("uri").value(), "file://");
+
+  // Check Driver
+  const llvm::json::Object *Driver =
+  TheRun->getObject("tool")->getObject("driver");
+
+  ASSERT_TRUE(Driver->getString("name").has_value());
+  ASSERT_TRUE(Driver->getString("fullName").has_value());
+
+  EXPECT_EQ(Driver->getString("name").value(), "clang");
+  EXPECT_EQ(Driver->getString("fullName").value(), "clang-15");
+
+  // Check Rules
+  const llvm::json::Array *R

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 453788.
abrahamcd added a comment.

Commented out unfinished test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp
  clang/unittests/Frontend/CMakeLists.txt
  clang/unittests/Frontend/SARIFDiagnosticTest.cpp

Index: clang/unittests/Frontend/SARIFDiagnosticTest.cpp
===
--- /dev/null
+++ clang/unittests/Frontend/SARIFDiagnosticTest.cpp
@@ -0,0 +1,122 @@
+// RUN: %clang -fdiagnostics-format=sarif %s -o %t.exe -DGTEST
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s 2>
+// %t.diags || true RUN: %t.exe < %t.diags
+
+#include "llvm/ADT/StringRef.h"
+#include "llvm/Support/ErrorOr.h"
+#include "llvm/Support/FileSystem.h"
+#include "llvm/Support/FileUtilities.h"
+#include "llvm/Support/JSON.h"
+#include "llvm/Support/MemoryBuffer.h"
+#include "llvm/Support/Program.h"
+#include "gmock/gmock.h"
+#include "gtest/gtest.h"
+#include 
+#include 
+
+namespace {
+
+constexpr llvm::StringRef BrokenProgram = R"(// Example errors below start on line 2
+void main() {
+  int i = hello;
+
+  float test = 1a.0;
+
+  if (true)
+bool Yes = true;
+return;
+
+  bool j = hi;
+}
+})";
+
+TEST(SARIFDiagnosticTest, TestFields) {
+  llvm::SmallString<256> SearchDir;
+  llvm::sys::fs::current_path(SearchDir);
+  
+  SearchDir.append("/../../../bin");
+
+  llvm::ErrorOr ClangPathOrErr =
+  llvm::sys::findProgramByName("clang", {SearchDir});
+  ASSERT_TRUE(ClangPathOrErr);
+  const std::string &ClangPath = *ClangPathOrErr;
+
+  llvm::ErrorOr EchoPathOrErr =
+  llvm::sys::findProgramByName("echo");
+  ASSERT_TRUE(EchoPathOrErr);
+  const std::string &EchoPath = *EchoPathOrErr;
+
+  int EchoInputFD;
+  llvm::SmallString<32> EchoInputFile, EchoOutputFile;
+  llvm::sys::fs::createTemporaryFile("echo-input", "", EchoInputFD,
+ EchoInputFile);
+  llvm::sys::fs::createTemporaryFile("echo-output", "", EchoOutputFile);
+  llvm::FileRemover InputRemover(EchoInputFile.c_str());
+  llvm::FileRemover OutputRemover(EchoOutputFile.c_str());
+
+  llvm::Optional Redirects[] = {
+  EchoInputFile.str(), EchoOutputFile.str(), llvm::StringRef("")};
+
+  int RunResult = llvm::sys::ExecuteAndWait(EchoPath, {"echo", BrokenProgram},
+llvm::None, Redirects);
+  ASSERT_EQ(RunResult, 0);
+
+  llvm::SmallString<32> ClangErrFile;
+  llvm::sys::fs::createTemporaryFile("clang-err", "", ClangErrFile);
+  llvm::FileRemover ClangErrRemover(ClangErrFile.c_str());
+
+  llvm::Optional ClangRedirects[] = {
+  EchoOutputFile.str(), llvm::StringRef(""), ClangErrFile.str()};
+  llvm::StringRef Args[] = {"clang",
+"-xc++",
+"-",
+"-fsyntax-only",
+"-Wall",
+"-Wextra",
+"-fdiagnostics-format=sarif"};
+
+  int ClangResult =
+  llvm::sys::ExecuteAndWait(ClangPath, Args, llvm::None, ClangRedirects);
+  ASSERT_EQ(ClangResult, 1);
+
+  auto ClangErrBuf = llvm::MemoryBuffer::getFile(ClangErrFile.c_str());
+  ASSERT_TRUE(ClangErrBuf);
+  llvm::StringRef ClangErr = ClangErrBuf.get()->getBuffer();
+
+  // llvm::Expected Value = llvm::json::parse(ClangErr.str());
+  // ASSERT_FALSE(!Value);
+
+  // llvm::json::Object *SarifDoc = Value->getAsObject();
+
+  // const llvm::json::Array *Runs = SarifDoc->getArray("runs");
+  // const llvm::json::Object *TheRun = Runs->back().getAsObject();
+  // const llvm::json::Array *Results = TheRun->getArray("results");
+  
+  // Check Artifacts
+  // const llvm::json::Array *Artifacts = TheRun->getArray("artifacts");
+  // const llvm::json::Object *TheArtifact = Artifacts->back().getAsObject();
+  // const llvm::json::Object *Location = TheArtifact->getObject("location");
+
+  // ASSERT_TRUE(Location->getInteger("index").has_value());
+  // ASSERT_TRUE(Location->getString("uri").has_value());
+
+  // EXPECT_EQ(Location->getInteger("index").value(), 0);
+  // EXPECT_EQ(Location->getString("uri").value(), "file://");
+
+  // Check Driver
+  // const llvm::json::Object *Driver =
+  // TheRun->getObject("tool")->getObject("driver");
+
+  // ASSERT_TRUE(Driver->getString("name").has_value());
+  // ASSERT_TRUE(Driver->getString("fullName").has_value());
+
+  // EXPECT_EQ(Driver->getString("name").value(), "clang");
+  // EXPECT_EQ(Driver->getString("fullName").value(), "clang-15");
+
+  // Check Rules
+  // const l

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added a comment.

In D131632#3733068 , @aaron.ballman 
wrote:

> Precommit CI found a relevant failure.

I think this was just from the unfinished unit test. I can go ahead and remove 
`clang/unittests/Frontend/SARIFDiagnosticTest.cpp` entirely if we don't plan to 
use unit tests and the FileCheck test in 
`clang/test/Frontend/sarif-diagnostics.cpp` is enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

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

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 454113.
abrahamcd marked 2 inline comments as done.
abrahamcd added a comment.

Added diagnostic level and default configuration to SARIF output.
Removed unit test file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3463","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4601","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6536","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4601","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4536","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3463","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6536","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 454129.
abrahamcd edited the summary of this revision.
abrahamcd added a comment.

Addressed review style comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3463","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4601","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6536","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4601","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4536","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3463","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6536","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration":{"enabled":true,"

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 454635.
abrahamcd marked 2 inline comments as done.
abrahamcd added a comment.

Formatting fixes and synchronization of documentation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,629 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+struct vector_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+template 
+struct vector_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+struct string_with_clear_args {
+  bool empty();
+  void clear(int i);
+};
+
+struct string_with_clear_variable {
+  bool empty();
+  int clear;
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+namespace base {
+template 
+struct base_vector {
+void clear();
+};
+
+template 
+struct base_vector_clear_with_args {
+void clear(int i);
+};
+
+template 
+struct base_vector_clear_variable {
+int clear;
+};
+
+struct base_vector_non_dependent {
+void clear();
+};
+
+template 
+struct vector : base_vector {
+bool empty();
+};
+
+template 
+struct vector_clear_with_args : base_vector_clear_with_args {
+bool empty();
+};
+
+template 
+struct vector_clear_variable : base_vector_clear_variable {
+bool empty();
+};
+
+template 
+struct vector_non_dependent : base_vector_non_dependent {
+bool empty();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace base
+
+namespace qualifiers {
+template 
+struct vector_with_const_clear {
+  bool empty() const;
+  void clear() const;
+};
+
+template 
+struct vector_with_const_empty {
+  bool empty() const;
+  void clear();
+};
+
+template 
+struct vector_with_volatile_clear {
+  bool empty() volatile;
+  void clear() volatile;
+};
+
+template 
+struct vector_with_volatile_empty {
+  bool empty() volatile;
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace qualifiers
+
+
+int test_member_empty() {
+  {
+std::vector v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_void_empty v;
+
+v.empty();
+// no-warning
+  }
+
+  {
+std::vector_with_clear v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_int_empty v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+std::vector_with_clear_args v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+std::vector_with_clear_variable v;
+
+v.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+  }
+
+  {
+absl::string_with_void_empty s;
+
+s.empty();
+// no-warning
+  }
+
+  {
+absl::string_with_clear s;
+
+s.empty();
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'empty()'; did you mean 'clear()'? [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  s.clear();{{$}}
+  }
+

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 454652.
abrahamcd marked 26 inline comments as done.
abrahamcd added a comment.

Addressed review comments on formatting, style, C++ best practices.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3463","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4601","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6536","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4601","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4536","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3463","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6536","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaul

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: clang/include/clang/Frontend/SARIFDiagnosticPrinter.h:63-65
+  void setSarifWriter(SarifDocumentWriter *SarifWriter) {
+Writer = std::unique_ptr(SarifWriter);
+  }

aaron.ballman wrote:
> This interface seems dangerous to me -- the caller has no idea that the 
> pointer will be stashed off as a `unique_ptr` and so the caller might expect 
> to still be able to use its (now not unique) pointer value.
Would it be best to store as a regular pointer instead of a unique pointer in 
this case then?



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:162
+break;
+  llvm_unreachable("Not an existing diagnostic level");
+  }

cjdb wrote:
> aaron.ballman wrote:
> > 
> 
This creates `-Wcovered-switch-default` warning so I just removed the 
unreachable.



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:174
+  if (DiagOpts->AbsolutePath) {
+auto File = SM.getFileManager().getFile(Filename);
+if (File) {

aaron.ballman wrote:
> Please spell out the type since it's not spelled out in the initialization. 
> Also, the next line is checking for a file -- what if `File` is null?
If I am interpreting it correctly (this is adapted from `TextDiagnostic.cpp`), 
I believe the passed-in Filename will just be returned unaltered. This is 
probably one of those functions that would be good to pull out during 
refactoring.



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:189
+  // on Windows we can just use llvm::sys::path::remove_dots(), because,
+  // on that system, both aforementioned paths point to the same place.
+#ifdef _WIN32

aaron.ballman wrote:
> I'm not certain we want a difference in behavior here. For starters, Windows 
> also has hardlinks and symlinks (they're not exactly the same as on *nix 
> systems though), so I think the assumption these paths point to the same 
> place is valid. But also, platform-specific behavior should be exceedingly 
> rare  in most parts of the compiler. Is there significant harm in unifying 
> the behavior across platforms?
There seems to be Windows-specific tests that rely on this behavior, but I'm 
not too sure what the harm in unifying it would be. There's some discussion on 
it here: D59415. 



Comment at: clang/test/Frontend/sarif-diagnostics.cpp:3
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: 
{"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":2953,"location":{"index":0,"uri":"file:///usr/local/google/home/abrahamcd/projects/llvm-project/clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":5}}}],"message":{"text":";'main'
 must return 
'int'"},"ruleId":"3463","ruleIndex":0},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":6}}}],"message":{"text":"use
 of undeclared identifier 
'hello'"},"ruleId":"4601","ruleIndex":1},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":8}}}],"message":{"text":"invalid
 digit 'a' in decimal 
constant"},"ruleId":"898","ruleIndex":2},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":12}}}],"message":{"text":"misleading
 indentation; statement is not part of the previous 
'if'"},"ruleId":"1806","ruleIndex":3},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":10}}}],"message":{"text":"previous
 statement is 
here"},"ruleId":"1730","ruleIndex":4},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":11}}}],"message":{"text":"unused
 variable 
'Yes'"},"ruleId":"6536","ruleIndex":5},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":14}}}],"message":{"text":"use
 of undeclared identifier 
'hi'"},"ruleId":"4601","ruleIndex":6},{"locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":16}}}],"message":{"text":"extraneous
 closing brace 
('}')"},"ruleId":"1399","ruleIndex":7}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"fullDescription":{"text":""},"id":"3463","name":""},{"fullDescription":{"text":""},"id":"4601","name":""},{"fullDescription":{"text":""},"id":"898","name":""},{"fullDescription":{"text":""},"id":"1806","name":""},{"fullDescription":{"text":""},"id":"1730","name":""},{"fu

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 454918.
abrahamcd marked 4 inline comments as done.
abrahamcd added a comment.

Modified Printer's interface to use unique pointers when setting the SARIF 
writer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3463","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4601","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6536","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4601","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4536","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3463","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6536","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","nam

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 454926.
abrahamcd added a comment.

Documented issue with representing PresumedLocs modified by \#line directives.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3463","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4601","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6536","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4601","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4536","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3463","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6536","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration":{"enabled":true,"l

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:116
+
+Locations.push_back(
+CharSourceRange{SourceRange{BeginLoc, EndLoc}, /* ITR = */ false});

I noticed that when processing additional source ranges, regular source 
locations are used instead of the presumed locations. Is this an intentional 
difference from single location diagnostics? (This issue is present in the 
current TextDiagnostics as well.)



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:129-132
+  // FIXME: Enable the proper representation of PresumedLocs modified by a 
+  // #line directive after relevant change is made in SarifDocumentWriter.
+  Locations.push_back(
+  CharSourceRange{SourceRange{DiagLoc, DiagLoc}, /* ITR = */ false});

There's an issue I encountered with adding presumed locations that have been 
modified by a #line directive to SARIF. To add a location, the Writer requires 
a CharSourceRange, which uses regular SourceLocations. If the presumed location 
has a modified line number, the source manager will still interpret it as a 
regular line number in the source file and can cause the wrong column number to 
be added to the SARIF object.

@vaibhav.y Do you have an idea of how involved it would be to add an option to 
the document writer that adds locations without first creating a 
CharSourceRange?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

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

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 454950.
abrahamcd marked 2 inline comments as done.
abrahamcd added a comment.

Added use of std::make_unique.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3463","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4601","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6536","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4601","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4536","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3463","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6536","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4601","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration":{"enabled":true,"level

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 455005.
abrahamcd added a comment.

Removed use of pointer reset in Printer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3464","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4603","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6538","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4603","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4538","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3464","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4603","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6538","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4603","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescript

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 455394.
abrahamcd marked 4 inline comments as done.
abrahamcd added a comment.

Deleted copy/move from renderer and returned OS to reference.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3464","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4603","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6538","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4603","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4538","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3464","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4603","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6538","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4603","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfig

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 455728.
abrahamcd marked 15 inline comments as done.
abrahamcd added a comment.

Addressed some refactoring and reformatting comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3464","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4603","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6538","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4603","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4538","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3464","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4603","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6538","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4603","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: clang/include/clang/Frontend/SARIFDiagnostic.h:45-47
+  void emitCodeContext(FullSourceLoc Loc, DiagnosticsEngine::Level Level,
+   SmallVectorImpl &Ranges,
+   ArrayRef Hints) override {}

aaron.ballman wrote:
> Move this implementation to live with the rest and give it an assertion?
This function is currently being called from the DiagnosticRenderer class that 
both Text and SARIFDiagnostics inherit from. Maybe this could be part of the 
refactoring, making sure that any text-specific function calls are moved to 
TextDiagnostic rather than being in the general Renderer base class?



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:58
+
+  if (Loc.isValid())
+Result = addLocationToResult(Result, Loc, PLoc, Ranges, *Diag);

denik wrote:
> I think we should add a test case when Loc or/and Source Manager is not set.
> If Loc is not set SarifDocumentWriter might not create the `locations` 
> property which is required by the spec.
> If this is the case we need to fix it. But I'm not sure if 
> emitDiagnosticMessage() is going to be called when Loc.isInvalid() ).
I believe if Loc is invalid, it goes to one of those special cases I mentioned 
in this review.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

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

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 455729.
abrahamcd added a comment.

Ran clang-format.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3464","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4603","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6538","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4603","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4538","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3464","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4603","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6538","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4603","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 455955.
abrahamcd marked 10 inline comments as done.
abrahamcd added a comment.

Resolved pending comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3465","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4604","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6539","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4604","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4539","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3465","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6539","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration":{"enabled":true,"level":"

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd added inline comments.



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:58
+
+  if (Loc.isValid())
+Result = addLocationToResult(Result, Loc, PLoc, Ranges, *Diag);

denik wrote:
> abrahamcd wrote:
> > denik wrote:
> > > I think we should add a test case when Loc or/and Source Manager is not 
> > > set.
> > > If Loc is not set SarifDocumentWriter might not create the `locations` 
> > > property which is required by the spec.
> > > If this is the case we need to fix it. But I'm not sure if 
> > > emitDiagnosticMessage() is going to be called when Loc.isInvalid() ).
> > I believe if Loc is invalid, it goes to one of those special cases I 
> > mentioned in this review.
> If it does then "if" is redundant here.
Looked closer and the base Renderer calls this function in both cases when Loc 
is valid and when it is invalid, so the extra check is necessary. In the case 
that Loc is invalid, you're right that results will not have locations. This 
might be something to add to the Writer, I couldn't find a way to add an empty 
locations object.



Comment at: clang/lib/Frontend/SARIFDiagnostic.cpp:191
+#ifdef _WIN32
+  TmpFilename = (*File)->getName();
+  llvm::sys::fs::make_absolute(TmpFilename);

cjdb wrote:
> denik wrote:
> > cjdb wrote:
> > > aaron.ballman wrote:
> > > > Note: this is not a particularly small string when it requires 4k by 
> > > > default.
> > > There's a bug either here or in the function's interface: the function 
> > > returns a `StringRef` to a stack object. Do we really need ownership here?
> > > Note: this is not a particularly small string when it requires 4k by 
> > > default.
> > 
> > This still has to be addressed.
> > There's a bug either here or in the function's interface: the function 
> > returns a StringRef to a stack object. Do we really need ownership here?
> 
> Disregard, I misread `TmpFilename` as `Filename` and thought we were 
> returning a dangling reference. That isn't the case, so there is no bug.
Made it smaller for now, but this should be revisited during refactoring, since 
text diag still uses 4096


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

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

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 455964.
abrahamcd added a comment.

Deleted copy and move for Printer and added asserts.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3465","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4604","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6539","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4604","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4539","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3465","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6539","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"

[PATCH] D131632: [clang] Enable output of SARIF diagnostics

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 455968.
abrahamcd added a comment.

Reset SARIFDiag during EndSourceFile().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131632

Files:
  clang/include/clang/Frontend/SARIFDiagnostic.h
  clang/include/clang/Frontend/SARIFDiagnosticPrinter.h
  clang/lib/Frontend/CMakeLists.txt
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/FrontendAction.cpp
  clang/lib/Frontend/SARIFDiagnostic.cpp
  clang/lib/Frontend/SARIFDiagnosticPrinter.cpp
  clang/test/Frontend/sarif-diagnostics.cpp

Index: clang/test/Frontend/sarif-diagnostics.cpp
===
--- /dev/null
+++ clang/test/Frontend/sarif-diagnostics.cpp
@@ -0,0 +1,28 @@
+// RUN: %clang -fsyntax-only -Wall -Wextra -fdiagnostics-format=sarif %s > %t 2>&1 || true
+// RUN: FileCheck -dump-input=always %s --input-file=%t
+// CHECK: warning: diagnostic formatting in SARIF mode is currently unstable [-Wsarif-format-unstable]
+// CHECK: {"$schema":"https://docs.oasis-open.org/sarif/sarif/v2.1.0/cos02/schemas/sarif-schema-2.1.0.json","runs":[{"artifacts":[{"length":
+// Omit exact length of this file
+// CHECK: ,"location":{"index":0,"uri":"file://
+// Omit filepath to llvm project directory
+// CHECK: clang/test/Frontend/sarif-diagnostics.cpp"},"mimeType":"text/plain","roles":["resultFile"]}],"columnKind":"unicodeCodePoints","results":[{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":12}}}],"message":{"text":"'main' must return 'int'"},"ruleId":"3465","ruleIndex":0},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":11,"startColumn":11,"startLine":13}}}],"message":{"text":"use of undeclared identifier 'hello'"},"ruleId":"4604","ruleIndex":1},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":17,"startColumn":17,"startLine":15}}}],"message":{"text":"invalid digit 'a' in decimal constant"},"ruleId":"898","ruleIndex":2},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":5,"startColumn":5,"startLine":19}}}],"message":{"text":"misleading indentation; statement is not part of the previous 'if'"},"ruleId":"1806","ruleIndex":3},{"level":"note","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":3,"startColumn":3,"startLine":17}}}],"message":{"text":"previous statement is here"},"ruleId":"1730","ruleIndex":4},{"level":"warning","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"startColumn":10,"startLine":18}}}],"message":{"text":"unused variable 'Yes'"},"ruleId":"6539","ruleIndex":5},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":12,"startColumn":12,"startLine":21}}}],"message":{"text":"use of undeclared identifier 'hi'"},"ruleId":"4604","ruleIndex":6},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":1,"startColumn":1,"startLine":23}}}],"message":{"text":"extraneous closing brace ('}')"},"ruleId":"1399","ruleIndex":7},{"level":"error","locations":[{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":6,"endLine":27,"startColumn":5,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":10,"endLine":27,"startColumn":9,"startLine":27}}},{"physicalLocation":{"artifactLocation":{"index":0},"region":{"endColumn":7,"startColumn":7,"startLine":27}}}],"message":{"text":"invalid operands to binary expression ('t1' and 't1')"},"ruleId":"4539","ruleIndex":8}],"tool":{"driver":{"fullName":"","informationUri":"https://clang.llvm.org/docs/UsersManual.html","language":"en-US","name":"clang","rules":[{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"3465","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"898","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"1806","name":""},{"defaultConfiguration":{"enabled":true,"level":"note","rank":-1},"fullDescription":{"text":""},"id":"1730","name":""},{"defaultConfiguration":{"enabled":true,"level":"warning","rank":-1},"fullDescription":{"text":""},"id":"6539","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"4604","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescription":{"text":""},"id":"1399","name":""},{"defaultConfiguration":{"enabled":true,"level":"error","rank":50},"fullDescripti

[PATCH] D128368: added clear check functionality

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd created this revision.
Herald added a subscriber: carlosgalvezp.
Herald added a project: All.
abrahamcd requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128368

Files:
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
@@ -6,76 +6,74 @@
 struct vector {
   bool empty();
   void clear();
-  // CHECK-MESSAGES: HERE
 };
 
-// template 
-// bool empty(T &&);
+template 
+bool empty(T &&);
 
 } // namespace std
 
-// namespace absl {
-// template 
-// struct vector {
-//   bool empty();
-//   void clear();
-//   // CHEscdcCK-MESSAGES: HERE
-// };
+namespace absl {
+template 
+struct vector {
+  bool empty();
+  void clear();
+};
 
-// template 
-// bool empty(T &&);
-// } // namespace absl
+template 
+bool empty(T &&);
+} // namespace absl
 
 int test_member_empty() {
   std::vector v;
 
   v.empty();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
-  // CvfdafHECK-FIXES: {{^  }}v.clear();{{$}}
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
 
-  // absl::vector w;
+  absl::vector w;
 
-  // w.empty();
+  w.empty();
   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
-  // ChafdafHECK-FIXES: {{^  }}w.clear();{{$}}
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
 }
 
-// int test_qualified_empty() {
-//   absl::vector v;
-
-//   std::empty(v);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-//   absl::empty(v);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}v.clear();{{$}}
-// }
-
-// int test_unqualified_empty() {
-//   std::vector v;
-
-//   empty(v);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}v.clear();{{$}}
-
-//   absl::vector w;
-
-//   empty(w);
-//   // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-//   // CHECK-FIXES: {{^  }}w.clear();{{$}}
-
-//   {
-// using std::empty;
-// empty(v);
-// // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
-// // CHECK-FIXES: {{^  }}  v.clear();{{$}}
-//   }
-
-//   {
-// using absl::empty;
-// empty(w);
-// // CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
-// // CHECK-FIXES: {{^  }}  w.clear();{{$}}
-//   }
-// }
+int test_qualified_empty() {
+  absl::vector v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::vector w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  {
+using std::empty;
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+using absl::empty;
+empty(w);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  w.clear();{{$}}
+  }
+}
Index: clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
===
--- clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
+++ clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
@@ -18,6 +18,7 @@
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Basic/SourceLocation.h"
 #include "clang/Lex/Lexer.h"
+#include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallVector.h"
 
 namespace clang {
@@ -25,67 +26,57 @@
 namespace bugprone {
 
 void StandaloneEmptyCheck::registerMat

[PATCH] D128372: Clang-Tidy Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd created this revision.
Herald added subscribers: carlosgalvezp, abrachet, phosek, mgorny.
Herald added a project: All.
abrahamcd requested review of this revision.
Herald added a reviewer: jdoerfert.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang-tools-extra.

Adds a clang-tidy check for the incorrect use of `empty()` on a
container when the result of the call is ignored.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
@@ -0,0 +1,79 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+
+template 
+struct vector {
+  bool empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+template 
+struct vector {
+  bool empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+int test_member_empty() {
+  std::vector v;
+
+  v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::vector w;
+
+  w.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+}
+
+int test_qualified_empty() {
+  absl::vector v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::vector w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  {
+using std::empty;
+empty(v);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  v.clear();{{$}}
+  }
+
+  {
+using absl::empty;
+empty(w);
+// CHECK-MESSAGES: :[[@LINE-1]]:5: warning: ignoring the result of 'absl::empty' [bugprone-standalone-empty]
+// CHECK-FIXES: {{^  }}  w.clear();{{$}}
+  }
+}
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -36,328 +36,329 @@
 .. csv-table::
:header: "Name", "Offers fixes"
 
-   `abseil-cleanup-ctad `_, "Yes"
-   `abseil-duration-addition `_, "Yes"
-   `abseil-duration-comparison `_, "Yes"
-   `abseil-duration-conversion-cast `_, "Yes"
-   `abseil-duration-division `_, "Yes"
-   `abseil-duration-factory-float `_, "Yes"
-   `abseil-duration-factory-scale `_, "Yes"
-   `abseil-duration-subtraction `_, "Yes"
-   `abseil-duration-unnecessary-conversion `_, "Yes"
-   `abseil-faster-strsplit-delimiter `_, "Yes"
-   `abseil-no-internal-dependencies `_,
-   `abseil-no-namespace `_,
-   `abseil-redundant-strcat-calls `_, "Yes"
-   `abseil-str-cat-append `_, "Yes"
-   `abseil-string-find-startswith `_, "Yes"
-   `abseil-string-find-str-contains `_, "Yes"
-   `abseil-time-comparison `_, "Yes"
-   `abseil-time-subtraction `_, "Yes"
-   `abseil-upgrade-duration-conversions `_, "Yes"
-   `altera-id-dependent-backward-branch `_,
-   `altera-kernel-name-restriction `_,
-   `altera-single-work-item-barrier `_,
-   `altera-struct-pack-align `_, "Yes"
-   `altera-unroll-loops `_,
-   `android-cloexec-accept `_, "Yes"
-   `android-cloexec-accept4 `_, "Yes"
-   `android-cloexec-creat `_, "Yes"
-   `android-cloexec-dup `_, "Yes"
-   `android-cloexec-epoll-create `_, "Yes"
-   `android-cloexec-epoll-create1 `_, "Yes"
-   `android-cloexec-fopen `_, "Yes"
-   `android-cloexec-inotify-init `_, "Yes"
-   `android-cloexec-inotify-init1 `_, "Yes"
-   `android-cloexec-memfd-create `_, "Yes"
-   `android-cloexec-open `_, "Yes"
-   `android-cloexec-pipe `_, "Yes"
-   `a

[PATCH] D128372: [Clang-Tidy] Empty Check

[Abraham Corea Diaz via Phabricator via cfe-commits]

abrahamcd updated this revision to Diff 439938.
abrahamcd marked 7 inline comments as done.
abrahamcd retitled this revision from "Clang-Tidy Empty Check" to "[Clang-Tidy] 
Empty Check".
abrahamcd added a comment.
Herald added a subscriber: xazax.hun.

Added functionality to check if member function `clear()` exists before
suggesting it as a fix, including updated tests to reflect that. Fixed
path change and rebase errors from the initial revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128372

Files:
  clang-tools-extra/clang-tidy/bugprone/BugproneTidyModule.cpp
  clang-tools-extra/clang-tidy/bugprone/CMakeLists.txt
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StandaloneEmptyCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone/standalone-empty.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/test/clang-tidy/checkers/bugprone-standalone-empty.cpp
  clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/bugprone/standalone-empty.cpp
@@ -0,0 +1,202 @@
+// RUN: %check_clang_tidy %s bugprone-standalone-empty %t
+
+namespace std {
+template 
+struct vector {
+  bool empty();
+};
+
+template 
+struct vector_with_clear {
+  bool empty();
+  void clear();
+};
+
+template 
+struct vector_with_void_empty {
+  void empty();
+  void clear();
+};
+
+template 
+struct vector_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+
+} // namespace std
+
+namespace absl {
+struct string {
+  bool empty();
+};
+
+struct string_with_clear {
+  bool empty();
+  void clear();
+};
+
+struct string_with_void_empty {
+  void empty();
+  void clear();
+};
+
+struct string_with_int_empty {
+  int empty();
+  void clear();
+};
+
+template 
+bool empty(T &&);
+} // namespace absl
+
+namespace test {
+template 
+void empty(T &&);
+} // namespace test
+
+int test_member_empty() {
+  std::vector s;
+
+  s.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty m;
+
+  m.empty();
+  // no-warning
+
+  std::vector_with_clear v;
+
+  v.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  std::vector_with_int_empty w;
+
+  w.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  absl::string x;
+
+  x.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()' [bugprone-standalone-empty]
+
+  absl::string_with_void_empty y;
+
+  y.empty();
+  // no-warning
+
+  absl::string_with_clear z;
+
+  z.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}z.clear();{{$}}
+
+  absl::string_with_int_empty a;
+
+  a.empty();
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'empty()', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}a.clear();{{$}}
+}
+
+int test_qualified_empty() {
+  absl::string_with_clear v;
+
+  std::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  absl::empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'absl::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}v.clear();{{$}}
+
+  test::empty(v);
+  // no-warning
+
+  absl::string w;
+
+  std::empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+}
+
+int test_unqualified_empty() {
+  std::vector v;
+
+  empty(v);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty' [bugprone-standalone-empty]
+
+  std::vector_with_void_empty w;
+
+  empty(w);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}w.clear();{{$}}
+
+  std::vector_with_clear x;
+
+  empty(x);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the result of 'std::empty', did you mean 'clear()'? [bugprone-standalone-empty]
+  // CHECK-FIXES: {{^  }}x.clear();{{$}}
+
+  std::vector_with_int_empty y;
+
+  empty(y);
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: ignoring the resu