[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added a comment.

FYI, once nice addition of the parsing of macro bodies is that it paves the way 
for
a modernize-macro-to-function check that converts function-like macros that
compute values to template functions.  Once this change has landed, I'll be 
putting
up a review for that.


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

https://reviews.llvm.org/D124500

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

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood updated this revision to Diff 426263.
LegalizeAdulthood added a comment.

Recognize comma operator expressions


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

https://reviews.llvm.org/D124500

Files:
  clang-tools-extra/clang-tidy/modernize/CMakeLists.txt
  clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp
  clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.h
  clang-tools-extra/clang-tidy/modernize/MacroToEnumCheck.cpp
  clang-tools-extra/docs/clang-tidy/checks/modernize-macro-to-enum.rst
  clang-tools-extra/test/clang-tidy/checkers/modernize-macro-to-enum.cpp
  clang-tools-extra/unittests/clang-tidy/CMakeLists.txt
  clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp

Index: clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
===
--- /dev/null
+++ clang-tools-extra/unittests/clang-tidy/ModernizeModuleTest.cpp
@@ -0,0 +1,214 @@
+//=== ModernizeModuleTest.cpp - clang-tidy ===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+#include "ClangTidyTest.h"
+#include "modernize/IntegralLiteralExpressionMatcher.h"
+#include "clang/Lex/Lexer.h"
+#include "gtest/gtest.h"
+
+#include 
+#include 
+#include 
+#include 
+
+namespace clang {
+namespace tidy {
+namespace test {
+
+static std::vector tokenify(const char *Text) {
+  LangOptions LangOpts;
+  std::vector Includes;
+  LangOptions::setLangDefaults(LangOpts, Language::CXX, llvm::Triple(),
+   Includes, LangStandard::lang_cxx20);
+  Lexer Lex(SourceLocation{}, LangOpts, Text, Text, Text + std::strlen(Text));
+  std::vector Tokens;
+  bool End = false;
+  while (!End) {
+Token Tok;
+End = Lex.LexFromRawLexer(Tok);
+Tokens.push_back(Tok);
+  }
+
+  return Tokens;
+}
+
+static bool matchText(const char *Text) {
+  std::vector Tokens{tokenify(Text)};
+  modernize::IntegralLiteralExpressionMatcher Matcher(Tokens);
+
+  return Matcher.match();
+}
+
+namespace {
+
+struct Param {
+  bool Matched;
+  const char *Text;
+};
+
+class MatcherTest : public ::testing::TestWithParam {};
+
+} // namespace
+
+static const Param Params[] = {
+// Accept integral literals.
+{true, "1"},
+{true, "0177"},
+{true, "0xdeadbeef"},
+{true, "0b1011"},
+{true, "'c'"},
+// Reject non-integral literals.
+{false, "1.23"},
+{false, "0x1p3"},
+{false, R"("string")"},
+{false, "1i"},
+
+// Accept literals with these unary operators.
+{true, "-1"},
+{true, "+1"},
+{true, "~1"},
+{true, "!1"},
+// Reject invalid unary operators.
+{false, "1-"},
+{false, "1+"},
+{false, "1~"},
+{false, "1!"},
+
+// Accept valid binary operators.
+{true, "1+1"},
+{true, "1-1"},
+{true, "1*1"},
+{true, "1/1"},
+{true, "1%2"},
+{true, "1<<1"},
+{true, "1>>1"},
+{true, "1<=>1"},
+{true, "1<1"},
+{true, "1>1"},
+{true, "1<=1"},
+{true, "1>=1"},
+{true, "1==1"},
+{true, "1!=1"},
+{true, "1&1"},
+{true, "1^1"},
+{true, "1|1"},
+{true, "1&&1"},
+{true, "1||1"},
+{true, "1+ +1"}, // A space is needed to avoid being tokenized as ++ or --.
+{true, "1- -1"},
+{true, "1,1"},
+// Reject invalid binary operators.
+{false, "1+"},
+{false, "1-"},
+{false, "1*"},
+{false, "1/"},
+{false, "1%"},
+{false, "1<<"},
+{false, "1>>"},
+{false, "1<=>"},
+{false, "1<"},
+{false, "1>"},
+{false, "1<="},
+{false, "1>="},
+{false, "1=="},
+{false, "1!="},
+{false, "1&"},
+{false, "1^"},
+{false, "1|"},
+{false, "1&&"},
+{false, "1||"},
+{false, "1,"},
+{false, ",1"},
+
+// Accept valid ternary operators.
+{true, "1?1:1"},
+{true, "1?:1"}, // A gcc extension treats x ? : y as x ? x : y.
+// Reject invalid ternary operators.
+{false, "?"},
+{false, "?1"},
+{false, "?:"},
+{false, "?:1"},
+{false, "?1:"},
+{false, "?1:1"},
+{false, "1?"},
+{false, "1?1"},
+{false, "1?:"},
+{false, "1?1:"},
+
+// Accept parenthesized expressions.
+{true, "(1)"},
+{true, "((+1))"},
+{true, "((+(1)))"},
+{true, "(-1)"},
+{true, "-(1)"},
+{true, "(+1)"},
+{true, "((+1))"},
+{true, "+(1)"},
+{true, "(~1)"},
+{true, "~(1)"},
+{true, "(!1)"},
+{true, "!(1)"},
+{true, "(1+1)"},
+{true, "(1-1)"},
+{true, "(1*1)"},
+{true, "(1/1)"},
+{true, "(1%2)"},
+{true, "(1<<1)"},
+{true, "(1>>1)"},
+{true, "(1<=>1)"},
+{true, "(1<1)"},
+{true, "(1>1)"},
+{true, "(1<=1)"},
+{true, 

[PATCH] D124500: [clang-tidy] Support expressions of literals in modernize-macro-to-enum

[Richard via Phabricator via cfe-commits]

LegalizeAdulthood added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/modernize/IntegralLiteralExpressionMatcher.cpp:204
+  return true;
+}
+

LegalizeAdulthood wrote:
> aaron.ballman wrote:
> > LegalizeAdulthood wrote:
> > > aaron.ballman wrote:
> > > > Comma operator?
> > > Remember that the use case here is identifying expressions that are 
> > > initializers for an enum.  If you were doing a code review and you saw 
> > > this:
> > > ```
> > > enum {
> > > FOO = (2, 3)
> > > };
> > > ```
> > > Would you be OK with that?  I wouldn't.  Clang even warns about it: 
> > > https://godbolt.org/z/Y641cb8Y9
> > > 
> > > Therefore I deliberately left comma operator out of the grammar.
> > This is another case where I think you're predicting that users won't be 
> > using the full expressivity of the language and we'll get bug reports 
> > later. Again, in insolation, I tend to agree that I wouldn't be happy 
> > seeing that code. However, users write some very creative code and there's 
> > no technical reason why we can't or shouldn't handle comma operators.
> "Don't let the perfect be the enemy of the good."
> 
> My inclination is to simply explicitly state that comma operator is not 
> recognized in the documentation.  It's already implicit by it's absence from 
> the list of recognized operators.
> 
> Again, the worst that happens is that your macro isn't converted.
> 
> I'm open to being convinced that it's important, but you haven't convinced me 
> yet `:)`
It wasn't much extra work/code to add comma operator support so I've done that.


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

https://reviews.llvm.org/D124500

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui added inline comments.



Comment at: clang/test/OpenMP/predefined_macro.c:7
 // RUN: %clang_cc1 -fopenmp-simd -fopenmp-version=50 -verify -o - %s
-// expected-no-diagnostics
+// expected-warning@+5 {{'#elsif' directive not found, did you mean '#elif'?}}
 #ifdef FOPENMP

I am not sure if this typo was intended.

When I renamed `elsif` to `elif`, `#error "_OPENMP has incorrect value"` on 
line `13` was evaluated.

Therefore, if this typo was intended, just suppressing the warning with 
`expected-warning` would be better. However, if this typo was NOT intended, I 
think I should make `_OPENMP` equal to `201107`. It is also possible that this 
test was mistakenly written.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124726

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

[PATCH] D124726: Suggest typoed directives in preprocessor conditionals

[Ken Matsui via Phabricator via cfe-commits]

ken-matsui created this revision.
ken-matsui added a reviewer: aaron.ballman.
Herald added a subscriber: mgorny.
Herald added a project: All.
ken-matsui requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch implements suggestion for typoed directives in preprocessor 
conditionals. The related issue is: 
https://github.com/llvm/llvm-project/issues/51598.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124726

Files:
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticLexKinds.td
  clang/include/clang/Lex/Preprocessor.h
  clang/include/clang/Tooling/LevDistance.h
  clang/lib/Lex/CMakeLists.txt
  clang/lib/Lex/PPDirectives.cpp
  clang/lib/Tooling/CMakeLists.txt
  clang/lib/Tooling/LevDistance.cpp
  clang/test/OpenMP/predefined_macro.c
  clang/test/Preprocessor/suggest-typoed-directive.c
  clang/unittests/Tooling/CMakeLists.txt
  clang/unittests/Tooling/LevDistanceTest.cpp

Index: clang/unittests/Tooling/LevDistanceTest.cpp
===
--- /dev/null
+++ clang/unittests/Tooling/LevDistanceTest.cpp
@@ -0,0 +1,60 @@
+//===- unittest/Tooling/LevDistanceTest.cpp ---===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/Tooling/LevDistance.h"
+#include "gtest/gtest.h"
+#include 
+
+using namespace clang;
+
+using tooling::levdistance::findSimilarStr;
+using tooling::levdistance::levDistance;
+
+namespace {
+
+TEST(LevDistanceTest, levDistance) {
+  EXPECT_EQ(0ul, levDistance("", ""));
+  EXPECT_EQ(1ul, levDistance("", "aaab"));
+  EXPECT_EQ(2ul, levDistance("aabc", "aacb"));
+  EXPECT_EQ(2ul, levDistance("aabc", "abca"));
+  EXPECT_EQ(3ul, levDistance("aabc", "adef"));
+  EXPECT_EQ(4ul, levDistance("abcd", "efgh"));
+}
+
+TEST(LevDistanceTest, findSimilarStr) {
+  std::array candidates{"aaab", "aaabc"};
+  EXPECT_EQ(std::string("aaab"), findSimilarStr(candidates, ""));
+
+  candidates = {"aab", "abc"};
+  EXPECT_EQ(std::string("aab"), findSimilarStr(candidates, "aaa"));
+
+  candidates = {"ab", "bc"};
+  EXPECT_EQ(std::string("ab"), findSimilarStr(candidates, "aa"));
+
+  candidates = {"b", "c"};
+  EXPECT_EQ(None, findSimilarStr(candidates, "a"));
+}
+
+TEST(LevDistanceTest, findSimilarStrToMacros) {
+  const std::array candidates{
+  "if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"};
+
+  EXPECT_EQ(std::string("elifdef"), findSimilarStr(candidates, "elfidef"));
+  EXPECT_EQ(std::string("elifdef"), findSimilarStr(candidates, "elifdef"));
+  EXPECT_EQ(None, findSimilarStr(candidates, "special_compiler_directive"));
+}
+
+TEST(LevDistanceTest, findSimilarStrInCaseInsensitive) {
+  const std::array candidates{
+  "if", "ifdef", "ifndef", "elif", "elifdef", "elifndef", "else", "endif"};
+
+  EXPECT_EQ(std::string("elifdef"), findSimilarStr(candidates, "elifdef"));
+  EXPECT_EQ(std::string("elifdef"), findSimilarStr(candidates, "ELIFDEF"));
+}
+
+} // end anonymous namespace
Index: clang/unittests/Tooling/CMakeLists.txt
===
--- clang/unittests/Tooling/CMakeLists.txt
+++ clang/unittests/Tooling/CMakeLists.txt
@@ -18,6 +18,7 @@
   FixItTest.cpp
   HeaderIncludesTest.cpp
   StandardLibraryTest.cpp
+  LevDistanceTest.cpp
   LexicallyOrderedRecursiveASTVisitorTest.cpp
   LookupTest.cpp
   QualTypeNamesTest.cpp
Index: clang/test/Preprocessor/suggest-typoed-directive.c
===
--- /dev/null
+++ clang/test/Preprocessor/suggest-typoed-directive.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -std=c99 -fsyntax-only -verify -pedantic %s
+
+// expected-warning@+11 {{'#id' directive not found, did you mean '#if'?}}
+// expected-warning@+11 {{'#ifd' directive not found, did you mean '#if'?}}
+// expected-warning@+11 {{'#ifde' directive not found, did you mean '#ifdef'?}}
+// expected-warning@+11 {{'#elid' directive not found, did you mean '#elif'?}}
+// expected-warning@+11 {{'#elsif' directive not found, did you mean '#elif'?}}
+// expected-warning@+11 {{'#elseif' directive not found, did you mean '#elif'?}}
+// expected-warning@+11 {{'#elfidef' directive not found, did you mean '#elifdef'?}}
+// expected-warning@+11 {{'#elfindef' directive not found, did you mean '#elifdef'?}}
+// expected-warning@+11 {{'#elsi' directive not found, did you mean '#else'?}}
+// expected-warning@+11 {{'#endi' directive not found, did you mean '#endif'?}}
+#ifdef UNDEFINED
+#id
+#ifd
+#ifde
+#elid
+#elsif
+#elseif
+#elfidef
+#elfindef
+#elsi
+#endi
+#endif
+
+#if special_compiler
+#special_compiler_directive // no diagnostics
+#endif
+

[PATCH] D124724: [Clang][OpenMP] Add the support for floating-point variables for specific atomic clauses

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 created this revision.
tianshilei1992 added reviewers: ABataev, jdoerfert.
Herald added subscribers: guansong, yaxunl.
Herald added a project: All.
tianshilei1992 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1.
Herald added a project: clang.

Currently when using `atomic update` with floating-point variables, if
the operation is add or sub, `cmpxchg`, instead of `atomicrmw` is emitted, as
shown in [1].  In fact, about three years ago, llvm-svn: 351850 added the
support for FP operations. This patch adds the support in OpenMP as well.

[1] https://godbolt.org/z/M7b4ba9na


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124724

Files:
  clang/lib/CodeGen/CGStmtOpenMP.cpp
  clang/test/OpenMP/atomic_capture_codegen.cpp
  clang/test/OpenMP/atomic_update_codegen.cpp
  clang/test/OpenMP/for_reduction_codegen.cpp
  clang/test/OpenMP/parallel_reduction_codegen.cpp
  clang/test/OpenMP/reduction_implicit_map.cpp
  clang/test/OpenMP/sections_reduction_codegen.cpp

Index: clang/test/OpenMP/sections_reduction_codegen.cpp
===
--- clang/test/OpenMP/sections_reduction_codegen.cpp
+++ clang/test/OpenMP/sections_reduction_codegen.cpp
@@ -268,11 +268,9 @@
 // CHECK1-NEXT:[[T_VAR15:%.*]] = alloca float, align 4
 // CHECK1-NEXT:[[DOTOMP_REDUCTION_RED_LIST:%.*]] = alloca [4 x i8*], align 8
 // CHECK1-NEXT:[[REF_TMP:%.*]] = alloca [[STRUCT_S]], align 4
+// CHECK1-NEXT:[[REF_TMP16:%.*]] = alloca [[STRUCT_S]], align 4
 // CHECK1-NEXT:[[ATOMIC_TEMP:%.*]] = alloca float, align 4
 // CHECK1-NEXT:[[TMP:%.*]] = alloca float, align 4
-// CHECK1-NEXT:[[REF_TMP17:%.*]] = alloca [[STRUCT_S]], align 4
-// CHECK1-NEXT:[[ATOMIC_TEMP27:%.*]] = alloca float, align 4
-// CHECK1-NEXT:[[_TMP28:%.*]] = alloca float, align 4
 // CHECK1-NEXT:store i32* [[DOTGLOBAL_TID_]], i32** [[DOTGLOBAL_TID__ADDR]], align 8
 // CHECK1-NEXT:store i32* [[DOTBOUND_TID_]], i32** [[DOTBOUND_TID__ADDR]], align 8
 // CHECK1-NEXT:store float* [[T_VAR]], float** [[T_VAR_ADDR]], align 8
@@ -401,77 +399,59 @@
 // CHECK1-NEXT:br label [[DOTOMP_REDUCTION_DEFAULT]]
 // CHECK1:   .omp.reduction.case2:
 // CHECK1-NEXT:[[TMP43:%.*]] = load float, float* [[T_VAR2]], align 4
-// CHECK1-NEXT:[[TMP44:%.*]] = bitcast float* [[TMP0]] to i32*
-// CHECK1-NEXT:[[ATOMIC_LOAD:%.*]] = load atomic i32, i32* [[TMP44]] monotonic, align 4
+// CHECK1-NEXT:[[TMP44:%.*]] = atomicrmw fadd float* [[TMP0]], float [[TMP43]] monotonic, align 4
+// CHECK1-NEXT:call void @__kmpc_critical(%struct.ident_t* @[[GLOB3]], i32 [[TMP7]], [8 x i32]* @.gomp_critical_user_.atomic_reduction.var)
+// CHECK1-NEXT:[[CALL15:%.*]] = call noundef nonnull align 4 dereferenceable(4) %struct.S* @_ZN1SIfEanERKS0_(%struct.S* noundef nonnull align 4 dereferenceable(4) [[TMP1]], %struct.S* noundef nonnull align 4 dereferenceable(4) [[VAR3]])
+// CHECK1-NEXT:[[TMP45:%.*]] = bitcast %struct.S* [[TMP1]] to i8*
+// CHECK1-NEXT:[[TMP46:%.*]] = bitcast %struct.S* [[CALL15]] to i8*
+// CHECK1-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 [[TMP45]], i8* align 4 [[TMP46]], i64 4, i1 false)
+// CHECK1-NEXT:call void @__kmpc_end_critical(%struct.ident_t* @[[GLOB3]], i32 [[TMP7]], [8 x i32]* @.gomp_critical_user_.atomic_reduction.var)
+// CHECK1-NEXT:call void @__kmpc_critical(%struct.ident_t* @[[GLOB3]], i32 [[TMP7]], [8 x i32]* @.gomp_critical_user_.atomic_reduction.var)
+// CHECK1-NEXT:[[CALL17:%.*]] = call noundef float @_ZN1SIfEcvfEv(%struct.S* noundef nonnull align 4 dereferenceable(4) [[TMP2]])
+// CHECK1-NEXT:[[TOBOOL18:%.*]] = fcmp une float [[CALL17]], 0.00e+00
+// CHECK1-NEXT:br i1 [[TOBOOL18]], label [[LAND_RHS19:%.*]], label [[LAND_END22:%.*]]
+// CHECK1:   land.rhs19:
+// CHECK1-NEXT:[[CALL20:%.*]] = call noundef float @_ZN1SIfEcvfEv(%struct.S* noundef nonnull align 4 dereferenceable(4) [[VAR14]])
+// CHECK1-NEXT:[[TOBOOL21:%.*]] = fcmp une float [[CALL20]], 0.00e+00
+// CHECK1-NEXT:br label [[LAND_END22]]
+// CHECK1:   land.end22:
+// CHECK1-NEXT:[[TMP47:%.*]] = phi i1 [ false, [[DOTOMP_REDUCTION_CASE2]] ], [ [[TOBOOL21]], [[LAND_RHS19]] ]
+// CHECK1-NEXT:[[CONV23:%.*]] = uitofp i1 [[TMP47]] to float
+// CHECK1-NEXT:call void @_ZN1SIfEC1Ef(%struct.S* noundef nonnull align 4 dereferenceable(4) [[REF_TMP16]], float noundef [[CONV23]])
+// CHECK1-NEXT:[[TMP48:%.*]] = bitcast %struct.S* [[TMP2]] to i8*
+// CHECK1-NEXT:[[TMP49:%.*]] = bitcast %struct.S* [[REF_TMP16]] to i8*
+// CHECK1-NEXT:call void @llvm.memcpy.p0i8.p0i8.i64(i8* align 4 [[TMP48]], i8* align 4 [[TMP49]], i64 4, i1 false)
+// CHECK1-NEXT:call void @_ZN1SIfED1Ev(%struct.S* noundef nonnull align 4 dereferenceable(4) [[REF_TMP16]]) #[[ATTR4]]
+// CHECK1-NEXT:call void @__kmpc_end_critical(%struct.ident_t* @[[GLOB3]], i32 [[TMP7]], [8 x i32]* 

[PATCH] D124708: Fix "the the" typo in documentation and user facing strings

[Brian Tracy via Phabricator via cfe-commits]

briantracy updated this revision to Diff 426259.
briantracy added a comment.

Updated sanitizer tests to include newly fixed error message strings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124708

Files:
  clang/docs/LanguageExtensions.rst
  clang/docs/MatrixTypes.rst
  clang/docs/UndefinedBehaviorSanitizer.rst
  clang/docs/analyzer/checkers.rst
  clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp
  clang/test/Analysis/cert/env34-c.c
  libcxx/include/__iterator/advance.h
  libcxx/src/filesystem/operations.cpp
  llvm/docs/AMDGPUUsage.rst
  llvm/docs/BugLifeCycle.rst
  llvm/docs/CodingStandards.rst
  llvm/docs/CommandGuide/llvm-profdata.rst
  llvm/include/llvm/Analysis/Loads.h
  llvm/include/llvm/ExecutionEngine/Orc/Core.h
  llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
  llvm/tools/llvm-profdata/llvm-profdata.cpp

Index: llvm/tools/llvm-profdata/llvm-profdata.cpp
===
--- llvm/tools/llvm-profdata/llvm-profdata.cpp
+++ llvm/tools/llvm-profdata/llvm-profdata.cpp
@@ -994,7 +994,7 @@
   "zero-counter-threshold", cl::init(0.7), cl::Hidden,
   cl::desc("For the function which is cold in instr profile but hot in "
"sample profile, if the ratio of the number of zero counters "
-   "divided by the the total number of counters is above the "
+   "divided by the total number of counters is above the "
"threshold, the profile of the function will be regarded as "
"being harmful for performance and will be dropped."));
   cl::opt SupplMinSizeThreshold(
Index: llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
===
--- llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
+++ llvm/lib/CodeGen/LiveDebugValues/InstrRefBasedImpl.h
@@ -196,7 +196,7 @@
 };
 
 /// Meta qualifiers for a value. Pair of whatever expression is used to qualify
-/// the the value, and Boolean of whether or not it's indirect.
+/// the value, and Boolean of whether or not it's indirect.
 class DbgValueProperties {
 public:
   DbgValueProperties(const DIExpression *DIExpr, bool Indirect)
Index: llvm/include/llvm/ExecutionEngine/Orc/Core.h
===
--- llvm/include/llvm/ExecutionEngine/Orc/Core.h
+++ llvm/include/llvm/ExecutionEngine/Orc/Core.h
@@ -1331,7 +1331,7 @@
   lookupInitSymbols(ExecutionSession ,
 const DenseMap );
 
-  /// Performs an async lookup for the the given symbols in each of the given
+  /// Performs an async lookup for the given symbols in each of the given
   /// JITDylibs, calling the given handler once all lookups have completed.
   static void
   lookupInitSymbolsAsync(unique_function OnComplete,
Index: llvm/include/llvm/Analysis/Loads.h
===
--- llvm/include/llvm/Analysis/Loads.h
+++ llvm/include/llvm/Analysis/Loads.h
@@ -75,9 +75,9 @@
 /// within the specified loop) would access only dereferenceable memory, and
 /// be properly aligned on every iteration of the specified loop regardless of
 /// its placement within the loop. (i.e. does not require predication beyond
-/// that required by the the header itself and could be hoisted into the header
+/// that required by the header itself and could be hoisted into the header
 /// if desired.)  This is more powerful than the variants above when the
-/// address loaded from is analyzeable by SCEV.  
+/// address loaded from is analyzeable by SCEV.
 bool isDereferenceableAndAlignedInLoop(LoadInst *LI, Loop *L,
ScalarEvolution ,
DominatorTree );
Index: llvm/docs/CommandGuide/llvm-profdata.rst
===
--- llvm/docs/CommandGuide/llvm-profdata.rst
+++ llvm/docs/CommandGuide/llvm-profdata.rst
@@ -170,7 +170,7 @@
 .. option:: --zero-counter-threshold=
 
  For the function which is cold in instr profile but hot in sample profile, if
- the ratio of the number of zero counters divided by the the total number of
+ the ratio of the number of zero counters divided by the total number of
  counters is above the threshold, the profile of the function will be regarded
  as being harmful for performance and will be dropped.
 
Index: llvm/docs/CodingStandards.rst
===
--- llvm/docs/CodingStandards.rst
+++ llvm/docs/CodingStandards.rst
@@ -1312,7 +1312,7 @@
 ... use I ...
 
 Usage of ``std::for_each()``/``llvm::for_each()`` functions is discouraged,
-unless the the callable object already exists.
+unless the callable object already exists.
 
 Don't evaluate ``end()`` every time through a loop
 ^^

[PATCH] D124721: [OpenMP] Allow compiling multiple target architectures with OpenMP

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 created this revision.
jhuber6 added reviewers: jdoerfert, JonChesterfield, yaxunl, saiislam, 
tianshilei1992, tra.
Herald added subscribers: kerbowa, guansong, jvesely.
Herald added a project: All.
jhuber6 requested review of this revision.
Herald added subscribers: cfe-commits, sstefan1, MaskRay.
Herald added a project: clang.

This patch adds support for OpenMP to use the `--offload-arch` and
`--no-offload-arch` options. Traditionally, OpenMP has only supported
compiling for a single architecture via the `-Xopenmp-target` option.
Now we can pass in a bound architecture and use that if given, otherwise
we default to the value of the `-march` option as before.

Note that this only applies the basic support, the OpenMP target runtime
does not yet know how to choose between multiple architectures.
Additionally other parts of the offloading toolchain (e.g. LTO) require
the `-march` option, these should be worked out later.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124721

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/AMDGPUOpenMP.cpp
  clang/lib/Driver/ToolChains/Cuda.cpp
  clang/test/Driver/amdgpu-openmp-toolchain-new.c
  clang/test/Driver/openmp-offload-gpu-new.c

Index: clang/test/Driver/openmp-offload-gpu-new.c
===
--- clang/test/Driver/openmp-offload-gpu-new.c
+++ clang/test/Driver/openmp-offload-gpu-new.c
@@ -10,6 +10,10 @@
 // RUN:  -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_52 \
 // RUN:  --libomptarget-nvptx-bc-path=%S/Inputs/libomptarget/libomptarget-nvptx-test.bc %s 2>&1 \
 // RUN:   | FileCheck %s
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda \
+// RUN:  --offload-arch=sm_52 \
+// RUN:  --libomptarget-nvptx-bc-path=%S/Inputs/libomptarget/libomptarget-nvptx-test.bc %s 2>&1 \
+// RUN:   | FileCheck %s
 
 // verify the tools invocations
 // CHECK: clang{{.*}}"-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-emit-llvm-bc"{{.*}}"-x" "c"
@@ -40,6 +44,15 @@
 // CHECK-BINDINGS: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[DEVICE_OBJ]]"], output: "[[HOST_OBJ:.*]]"
 // CHECK-BINDINGS: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
 
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda --offload-arch=sm_52 --offload-arch=sm_70 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-ARCH-BINDINGS
+// CHECK-ARCH-BINDINGS: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[INPUT:.*]]"], output: "[[HOST_BC:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[DEVICE_BC_SM_52:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[DEVICE_BC_SM_52]]"], output: "[[DEVICE_OBJ_SM_52:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "clang", inputs: ["[[INPUT]]", "[[HOST_BC]]"], output: "[[DEVICE_BC_SM_70:.*]]"
+// CHECK-ARCH-BINDINGS: "nvptx64-nvidia-cuda" - "NVPTX::Assembler", inputs: ["[[DEVICE_BC_SM_70]]"], output: "[[DEVICE_OBJ_SM_70:.*]]"
+// CHECK-ARCH-BINDINGS: "x86_64-unknown-linux-gnu" - "clang", inputs: ["[[HOST_BC]]", "[[DEVICE_OBJ_SM_52]]", "[[DEVICE_OBJ_SM_70]]"], output: "[[HOST_OBJ:.*]]"
+// CHECK-ARCH-BINDINGS: "x86_64-unknown-linux-gnu" - "Offload::Linker", inputs: ["[[HOST_OBJ]]"], output: "a.out"
+
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -emit-llvm -S -fopenmp -fopenmp-targets=nvptx64-nvidia-cuda -Xopenmp-target=nvptx64-nvidia-cuda -march=sm_52 -nogpulib %s 2>&1 | FileCheck %s --check-prefix=CHECK-EMIT-LLVM-IR
 // CHECK-EMIT-LLVM-IR: clang{{.*}}"-cc1"{{.*}}"-triple" "nvptx64-nvidia-cuda"{{.*}}"-emit-llvm"
 
Index: clang/test/Driver/amdgpu-openmp-toolchain-new.c
===
--- clang/test/Driver/amdgpu-openmp-toolchain-new.c
+++ clang/test/Driver/amdgpu-openmp-toolchain-new.c
@@ -3,6 +3,9 @@
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \
 // RUN:  -Xopenmp-target=amdgcn-amd-amdhsa -march=gfx906 --libomptarget-amdgpu-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
 // RUN:   | FileCheck %s
+// RUN:   %clang -### --target=x86_64-unknown-linux-gnu -fopenmp -fopenmp-targets=amdgcn-amd-amdhsa \
+// RUN:  --offload-arch=gfx906 --libomptarget-amdgpu-bc-path=%S/Inputs/hip_dev_lib %s 2>&1 \
+// RUN:   | FileCheck %s
 
 // verify the tools invocations
 // CHECK: "-cc1" "-triple" "x86_64-unknown-linux-gnu"{{.*}}"-emit-llvm-bc"{{.*}}"-x" "c"
@@ -34,6 +37,7 @@
 // CHECK-NOGPULIB-NOT: "-cc1" "-triple" "amdgcn-amd-amdhsa"{{.*}}"-target-cpu" "gfx803" "-fcuda-is-device" "-mlink-builtin-bitcode"{{.*}}libomptarget-amdgpu-gfx803.bc"{{.*}}
 
 // RUN:   %clang -### --target=x86_64-unknown-linux-gnu -ccc-print-bindings -fopenmp 

[PATCH] D124719: [clang,doc] PCH usage documentation update

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko updated this revision to Diff 426248.
ivanmurashko added a comment.

Minor changes at the doc


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124719

Files:
  clang/docs/UsersManual.rst


Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -1219,23 +1219,22 @@
 Using a PCH File
 
 
-A PCH file can then be used as a prefix header when a :option:`-include`
+A PCH file can then be used as a prefix header when a :option:`-include-pch`
 option is passed to ``clang``:
 
 .. code-block:: console
 
-  $ clang -include test.h test.c -o test
+  $ clang -include-pch test.h.pch test.c -o test
 
-The ``clang`` driver will first check if a PCH file for ``test.h`` is
+The ``clang`` driver will check if the PCH file ``test.h.pch`` is
 available; if so, the contents of ``test.h`` (and the files it includes)
-will be processed from the PCH file. Otherwise, Clang falls back to
-directly processing the content of ``test.h``. This mirrors the behavior
-of GCC.
+will be processed from the PCH file. Otherwise, Clang will report an error.
 
 .. note::
 
   Clang does *not* automatically use PCH files for headers that are directly
-  included within a source file. For example:
+  included within a source file or indirectly via :option:`-include`.
+  For example:
 
   .. code-block:: console
 
@@ -1244,9 +1243,6 @@
 #include "test.h"
 $ clang test.c -o test
 
-  In this example, ``clang`` will not automatically use the PCH file for
-  ``test.h`` since ``test.h`` was included directly in the source file and not
-  specified on the command line using :option:`-include`.
 
 Relocatable PCH Files
 ^


Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -1219,23 +1219,22 @@
 Using a PCH File
 
 
-A PCH file can then be used as a prefix header when a :option:`-include`
+A PCH file can then be used as a prefix header when a :option:`-include-pch`
 option is passed to ``clang``:
 
 .. code-block:: console
 
-  $ clang -include test.h test.c -o test
+  $ clang -include-pch test.h.pch test.c -o test
 
-The ``clang`` driver will first check if a PCH file for ``test.h`` is
+The ``clang`` driver will check if the PCH file ``test.h.pch`` is
 available; if so, the contents of ``test.h`` (and the files it includes)
-will be processed from the PCH file. Otherwise, Clang falls back to
-directly processing the content of ``test.h``. This mirrors the behavior
-of GCC.
+will be processed from the PCH file. Otherwise, Clang will report an error.
 
 .. note::
 
   Clang does *not* automatically use PCH files for headers that are directly
-  included within a source file. For example:
+  included within a source file or indirectly via :option:`-include`.
+  For example:
 
   .. code-block:: console
 
@@ -1244,9 +1243,6 @@
 #include "test.h"
 $ clang test.c -o test
 
-  In this example, ``clang`` will not automatically use the PCH file for
-  ``test.h`` since ``test.h`` was included directly in the source file and not
-  specified on the command line using :option:`-include`.
 
 Relocatable PCH Files
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124719: [clang,doc] PCH usage documentation update

[Ivan Murashko via Phabricator via cfe-commits]

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

The documentation has an incorrect example of PCH files usage via `-include` 
option. The possibility has not been available since llvm-svn: 174385 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124719

Files:
  clang/docs/UsersManual.rst


Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -1219,23 +1219,22 @@
 Using a PCH File
 
 
-A PCH file can then be used as a prefix header when a :option:`-include`
+A PCH file can then be used as a prefix header when a :option:`-include-pch`
 option is passed to ``clang``:
 
 .. code-block:: console
 
-  $ clang -include test.h test.c -o test
+  $ clang -include-pch test.h.pch test.c -o test
 
-The ``clang`` driver will first check if a PCH file for ``test.h`` is
-available; if so, the contents of ``test.h`` (and the files it includes)
-will be processed from the PCH file. Otherwise, Clang falls back to
-directly processing the content of ``test.h``. This mirrors the behavior
-of GCC.
+The ``clang`` driver will check if the PCH file ``test.h.pch`` is
+available; if so, the contents of ``test.h.pch`` (and the files it includes)
+will be processed from the PCH file. Otherwise, Clang will report an error.
 
 .. note::
 
   Clang does *not* automatically use PCH files for headers that are directly
-  included within a source file. For example:
+  included within a source file or indirectly via :option:`-include`.
+  For example:
 
   .. code-block:: console
 
@@ -1244,9 +1243,6 @@
 #include "test.h"
 $ clang test.c -o test
 
-  In this example, ``clang`` will not automatically use the PCH file for
-  ``test.h`` since ``test.h`` was included directly in the source file and not
-  specified on the command line using :option:`-include`.
 
 Relocatable PCH Files
 ^


Index: clang/docs/UsersManual.rst
===
--- clang/docs/UsersManual.rst
+++ clang/docs/UsersManual.rst
@@ -1219,23 +1219,22 @@
 Using a PCH File
 
 
-A PCH file can then be used as a prefix header when a :option:`-include`
+A PCH file can then be used as a prefix header when a :option:`-include-pch`
 option is passed to ``clang``:
 
 .. code-block:: console
 
-  $ clang -include test.h test.c -o test
+  $ clang -include-pch test.h.pch test.c -o test
 
-The ``clang`` driver will first check if a PCH file for ``test.h`` is
-available; if so, the contents of ``test.h`` (and the files it includes)
-will be processed from the PCH file. Otherwise, Clang falls back to
-directly processing the content of ``test.h``. This mirrors the behavior
-of GCC.
+The ``clang`` driver will check if the PCH file ``test.h.pch`` is
+available; if so, the contents of ``test.h.pch`` (and the files it includes)
+will be processed from the PCH file. Otherwise, Clang will report an error.
 
 .. note::
 
   Clang does *not* automatically use PCH files for headers that are directly
-  included within a source file. For example:
+  included within a source file or indirectly via :option:`-include`.
+  For example:
 
   .. code-block:: console
 
@@ -1244,9 +1243,6 @@
 #include "test.h"
 $ clang test.c -o test
 
-  In this example, ``clang`` will not automatically use the PCH file for
-  ``test.h`` since ``test.h`` was included directly in the source file and not
-  specified on the command line using :option:`-include`.
 
 Relocatable PCH Files
 ^
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123200: [compiler-rt][builtins] Add several helper functions for AVR

[Fangrui Song via Phabricator via cfe-commits]

MaskRay accepted this revision.
MaskRay added inline comments.
This revision is now accepted and ready to land.



Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:171
   check_symbol_exists(__ve__ "" __VE)
+  check_symbol_exists(__AVR__ "" __AVR)
   if(__ARM)

Keep the list in alphabetical order. Move avr to the beginning. Ignore some 
entries which are unordered.



Comment at: compiler-rt/cmake/Modules/CompilerRTUtils.cmake:216
 add_default_target_arch(ve)
+  elseif(__AVR)
+add_default_target_arch(avr)

Keep the list in alphabetical order. Move avr to the beginning. Ignore some 
entries which are unordered.



Comment at: compiler-rt/cmake/builtin-config-ix.cmake:55
 set(VE ve)
+set(AVR avr)
 

Keep the list in alphabetical order. Move avr to the beginning. Ignore some 
entries which are unordered.



Comment at: compiler-rt/lib/builtins/CMakeLists.txt:671
 
+set(avr_SOURCES
+  avr/mulqi3.S

Keep the `*_SOURCES` in alphabetical order. Move avr to the beginning. Ignore 
some entries which are unordered.


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

https://reviews.llvm.org/D123200

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

IIRC, the reason it works it that way is that "warnings which default to an 
error" are really "errors which you can explicitly downgrade to a warning".  
Maybe those ought to be different categories, or maybe we ought to just be 
telling people to downgrade this specific diagnostic instead of generally using 
`-Wno-error`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122983

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

Hmm, the commit message says that Wno-error should work but this is not really 
the case :(.

> (they can disable the warning or use -Wno-error to downgrade the
> error


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122983

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

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122895#3484097 , @aaron.ballman 
wrote:

> In D122895#3484076 , @manojgupta 
> wrote:
>
>> Tried locally but I still see the warning with -fno-knr-functions. It also 
>> says that the argument is unused.
>>
>> bin/clang --version
>> clang version 15.0.0 (https://github.com/llvm/llvm-project.git 
>> a9d68a5524dea113cace5983697786599cbdce9a 
>> )
>> Target: x86_64-unknown-linux-gnu
>>
>> $ cat pr.c
>> void foo(void);
>>
>> void foo() 
>> {
>> }
>> $ bin/clang -c pr.c -Wstrict-prototypes -fno-knr-functions
>> clang-14: warning: argument unused during compilation: '-fno-knr-functions' 
>> [-Wunused-command-line-argument]
>> pr.c:3:9: warning: a function declaration without a prototype is deprecated 
>> in all versions of C [-Wstrict-prototypes]
>> void foo()
>>
>>   ^
>>void
>>
>> 1 warning generated.
>>
>> It works if -fno-knr-functions is passed with Xclang .  Is it intentional 
>> that -fno-knr-functions is only a cc1 option? That makes it very hard for us 
>> to enable it.
>>
>> $ bin/clang -c pr.c -Wstrict-prototypes -Xclang -fno-knr-functions (no 
>> warnings)
>
> No, that's not at all intentional -- it should be exposed as a driver flag. I 
> can reproduce the issue locally and will fix this today (it's very strange 
> because the option is listed as a CoreOption should it should be exposed 
> through the driver). I'm very sorry for the trouble, but thank you for 
> catching this!

Sheesh, I added the driver tests to make sure we don't accept a negated version 
of the flag, but I didn't add the test to validate that the driver accepted the 
flag, which is why I didn't catch this before. I've corrected the problem and 
added test coverage in 786954db06ab253dbd62d059036e06f6bbd9223c 
. Thanks 
for letting me know about the issue!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122895

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

[clang] 786954d - Accept -fno-knr-functions as a driver flag as well

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-04-30T13:37:55-04:00
New Revision: 786954db06ab253dbd62d059036e06f6bbd9223c

URL: 
https://github.com/llvm/llvm-project/commit/786954db06ab253dbd62d059036e06f6bbd9223c
DIFF: 
https://github.com/llvm/llvm-project/commit/786954db06ab253dbd62d059036e06f6bbd9223c.diff

LOG: Accept -fno-knr-functions as a driver flag as well

Due to a think-o, it was only being accepted as a -cc1 flag. This adds
the proper forwarding from the driver to the frontend and adds test
coverage for the option.

Added: 


Modified: 
clang/lib/Driver/ToolChains/Clang.cpp
clang/test/Driver/no-knr-functions.c

Removed: 




diff  --git a/clang/lib/Driver/ToolChains/Clang.cpp 
b/clang/lib/Driver/ToolChains/Clang.cpp
index c5f38762a5d61..388a29261a720 100644
--- a/clang/lib/Driver/ToolChains/Clang.cpp
+++ b/clang/lib/Driver/ToolChains/Clang.cpp
@@ -5311,6 +5311,8 @@ void Clang::ConstructJob(Compilation , const JobAction 
,
   if (Freestanding)
 CmdArgs.push_back("-ffreestanding");
 
+  Args.AddLastArg(CmdArgs, options::OPT_fno_knr_functions);
+
   // This is a coarse approximation of what llvm-gcc actually does, both
   // -fasynchronous-unwind-tables and -fnon-call-exceptions interact in more
   // complicated ways.

diff  --git a/clang/test/Driver/no-knr-functions.c 
b/clang/test/Driver/no-knr-functions.c
index 99f397d38d626..5ebcc8916be67 100644
--- a/clang/test/Driver/no-knr-functions.c
+++ b/clang/test/Driver/no-knr-functions.c
@@ -7,5 +7,9 @@
 // RUN: not %clang -fknr-functions -x c %s 2>&1 | FileCheck 
--check-prefixes=POS %s
 // RUN: not %clang -fknr-functions -std=c89 -x c %s 2>&1 | FileCheck 
--check-prefixes=POS %s
 
+// Ensure that the driver flag is actually accepted though.
+// RUN: %clang -fno-knr-functions -### %s 2>&1 | FileCheck 
--check-prefixes=ACCEPTED %s
+
 // NONO: error: unknown argument: '-fno-no-knr-functions'
 // POS: error: unknown argument: '-fknr-functions'
+// ACCEPTED-NOT: warning: argument unused during compilation: 
'-fno-knr-functions'



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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122983#3484064 , @manojgupta 
wrote:

> We are finding a lot of failures in our ToT builds with this change. here is 
> an example for a configure script:
>
> $ cat tent.c
> int  main ()
> {
>  tgetent(0,0);
>  return 0;
> }
> $ bin/clang -c tent.c -Wno-error
> tent.c:3:2: error: call to undeclared function 'tgetent'; ISO C99 and later 
> do not support implicit function declarations 
> [-Wimplicit-function-declaration]
>  tgetent(0,0);
>  ^
> 1 error generated.
>
> It feels very surprising that Wno-error does not suppress this warning. Is 
> that expected?

Yes and no. Warnings which default to an error have very surprising behavior 
(at least to me) in terms of `-Werror` and `-w`. Specifying `-Wno-error` 
doesn't downgrade these diagnostics into a warning 
(https://godbolt.org/z/c43zTqTj1) and specifying `-w` doesn't disable the 
warning (https://godbolt.org/z/Y8YYYecTd). As you can see, that behavior is not 
specific to this patch but is just a general behavior with these kinds of 
diagnostics. I also find the behavior surprising (in both cases), but the last 
time I asked around about it, it sounded like this behavior was intentional for 
some reasons. More exploration is needed to know whether we should make any 
changes there.

In the meantime, `-Wno-error=implicit-function-declaration` should downgrade 
the error to a warning for you (but as you noticed, you can't later re-upgrade 
it into an error; so yes, these things behave a bit strangely IMO).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122983

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

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122895#3484076 , @manojgupta 
wrote:

> Tried locally but I still see the warning with -fno-knr-functions. It also 
> says that the argument is unused.
>
> bin/clang --version
> clang version 15.0.0 (https://github.com/llvm/llvm-project.git 
> a9d68a5524dea113cace5983697786599cbdce9a 
> )
> Target: x86_64-unknown-linux-gnu
>
> $ cat pr.c
> void foo(void);
>
> void foo() 
> {
> }
> $ bin/clang -c pr.c -Wstrict-prototypes -fno-knr-functions
> clang-14: warning: argument unused during compilation: '-fno-knr-functions' 
> [-Wunused-command-line-argument]
> pr.c:3:9: warning: a function declaration without a prototype is deprecated 
> in all versions of C [-Wstrict-prototypes]
> void foo()
>
>   ^
>void
>
> 1 warning generated.
>
> It works if -fno-knr-functions is passed with Xclang .  Is it intentional 
> that -fno-knr-functions is only a cc1 option? That makes it very hard for us 
> to enable it.
>
> $ bin/clang -c pr.c -Wstrict-prototypes -Xclang -fno-knr-functions (no 
> warnings)

No, that's not at all intentional -- it should be exposed as a driver flag. I 
can reproduce the issue locally and will fix this today (it's very strange 
because the option is listed as a CoreOption should it should be exposed 
through the driver). I'm very sorry for the trouble, but thank you for catching 
this!

In D122895#3484077 , @manojgupta 
wrote:

> Following behavior is also surprising:
>
> ` -Werror -Wimplicit-function-declaration` does not rep-promote it to an 
> error either if I suppress it globally with 
> -Wno-error=implicit-function-declaration.

On its face, I agree that it's surprising, but that's the general behavior of 
`-Werror` when warning flags default to an error, and is not specific to the 
changes here: https://godbolt.org/z/ronq687cj


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122895

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

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

Following behavior is also surprising:


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122895

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

[PATCH] D122895: [C89/C2x] Improve diagnostics around strict prototypes in C

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

Tried locally but I still see the warning with -fno-knr-functions. It also says 
that the argument is unused.

bin/clang --version
clang version 15.0.0 (https://github.com/llvm/llvm-project.git 
a9d68a5524dea113cace5983697786599cbdce9a 
)
Target: x86_64-unknown-linux-gnu

$ cat pr.c
void foo(void);

void foo() 
{
}
$ bin/clang -c pr.c -Wstrict-prototypes -fno-knr-functions
clang-14: warning: argument unused during compilation: '-fno-knr-functions' 
[-Wunused-command-line-argument]
pr.c:3:9: warning: a function declaration without a prototype is deprecated in 
all versions of C [-Wstrict-prototypes]
void foo()

  ^
   void

1 warning generated.

It works if -fno-knr-functions is passed with Xclang .  Is it intentional that 
-fno-knr-functions is only a cc1 option? That makes it very hard for us to 
enable it.

$ bin/clang -c pr.c -Wstrict-prototypes -Xclang -fno-knr-functions (no warnings)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122895

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Dávid Bolvanský via Phabricator via cfe-commits]

xbolva00 added a comment.

In D122983#3484064 , @manojgupta 
wrote:

> We are finding a lot of failures in our ToT builds with this change. here is 
> an example for a configure script:
>
> $ cat tent.c
> int  main ()
> {
>  tgetent(0,0);
>  return 0;
> }
> $ bin/clang -c tent.c -Wno-error
> tent.c:3:2: error: call to undeclared function 'tgetent'; ISO C99 and later 
> do not support implicit function declarations 
> [-Wimplicit-function-declaration]
>  tgetent(0,0);
>  ^
> 1 error generated.
>
> It feels very surprising that Wno-error does not suppress this warning. Is 
> that expected?

Yéah, this is unfortunate if -Wno-error does nothing :/ other options from 
release notes work for you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122983

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Manoj Gupta via Phabricator via cfe-commits]

manojgupta added a comment.

We are finding a lot of failures in our ToT builds with this change. here is an 
example for a configure script:

$ cat tent.c
int  main ()
{
 tgetent(0,0);
 return 0;
}
$ bin/clang -c tent.c -Wno-error
tent.c:3:2: error: call to undeclared function 'tgetent'; ISO C99 and later do 
not support implicit function declarations [-Wimplicit-function-declaration]
 tgetent(0,0);
 ^
1 error generated.

It feels very surprising that Wno-error does not suppress this warning. Is that 
expected?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122983

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

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

[Stefan Haller via Phabricator via cfe-commits]

stefanhaller updated this revision to Diff 426233.
stefanhaller added a comment.

Fix documentation of ThreadPriority::Low (was: Background), and move it up from
set_thread_priority to ThreadPriority.

(Is it ok that there's no documentation left for set_thread_priority? I couldn't
come up with anything that's not trivial and redundant, e.g. "Sets the priority
of the current thread.")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124715

Files:
  clang-tools-extra/clangd/index/Background.h
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Support/Threading.h
  llvm/lib/Support/Unix/Threading.inc
  llvm/lib/Support/Windows/Threading.inc

Index: llvm/lib/Support/Windows/Threading.inc
===
--- llvm/lib/Support/Windows/Threading.inc
+++ llvm/lib/Support/Windows/Threading.inc
@@ -121,7 +121,7 @@
   // priorities of the thread as they were before the thread entered background
   // processing mode.
   return SetThreadPriority(GetCurrentThread(),
-   Priority == ThreadPriority::Background
+   Priority == ThreadPriority::Low
? THREAD_MODE_BACKGROUND_BEGIN
: THREAD_MODE_BACKGROUND_END)
  ? SetThreadPriorityResult::SUCCESS
Index: llvm/lib/Support/Unix/Threading.inc
===
--- llvm/lib/Support/Unix/Threading.inc
+++ llvm/lib/Support/Unix/Threading.inc
@@ -18,6 +18,7 @@
 #if defined(__APPLE__)
 #include 
 #include 
+#include 
 #endif
 
 #include 
@@ -258,27 +259,20 @@
   // SCHED_OTHER   the standard round-robin time-sharing policy;
   return !pthread_setschedparam(
  pthread_self(),
- Priority == ThreadPriority::Background ? SCHED_IDLE : SCHED_OTHER,
+ Priority == ThreadPriority::Low ? SCHED_IDLE : SCHED_OTHER,
  )
  ? SetThreadPriorityResult::SUCCESS
  : SetThreadPriorityResult::FAILURE;
 #elif defined(__APPLE__)
-  // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getpriority.2.html
-  // When setting a thread into background state the scheduling priority is set
-  // to lowest value, disk and network IO are throttled. Network IO will be
-  // throttled for any sockets the thread opens after going into background
-  // state. Any previously opened sockets are not affected.
-
-  // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/getiopolicy_np.3.html
-  // I/Os with THROTTLE policy are called THROTTLE I/Os. If a THROTTLE I/O
-  // request occurs within a small time window (usually a fraction of a second)
-  // of another NORMAL I/O request, the thread that issues the THROTTLE I/O is
-  // forced to sleep for a certain interval. This slows down the thread that
-  // issues the THROTTLE I/O so that NORMAL I/Os can utilize most of the disk
-  // I/O bandwidth.
-  return !setpriority(PRIO_DARWIN_THREAD, 0,
-  Priority == ThreadPriority::Background ? PRIO_DARWIN_BG
- : 0)
+  // https://developer.apple.com/documentation/apple-silicon/tuning-your-code-s-performance-for-apple-silicon
+  // Utility - Applies to work that takes anywhere from a few seconds to a few
+  // minutes to complete. Examples include downloading a document or importing
+  // data. This class offers a balance between responsiveness, performance, and
+  // energy efficiency.
+  return !pthread_set_qos_class_self_np(Priority == ThreadPriority::Low
+? QOS_CLASS_UTILITY
+: QOS_CLASS_DEFAULT,
+0)
  ? SetThreadPriorityResult::SUCCESS
  : SetThreadPriorityResult::FAILURE;
 #endif
Index: llvm/include/llvm/Support/Threading.h
===
--- llvm/include/llvm/Support/Threading.h
+++ llvm/include/llvm/Support/Threading.h
@@ -233,15 +233,15 @@
   unsigned get_cpus();
 
   enum class ThreadPriority {
-Background = 0,
+/// Try to lower current thread's priority such that it does not affect 
+/// foreground tasks significantly. Can be used for long-running, 
+/// latency-insensitive tasks to make sure cpu is not hogged by this task.
+Low = 0,
+
+/// Try to restore current thread's priority to default scheduling 
+/// priority.
 Default = 1,
   };
-  /// If priority is Background tries to lower current threads priority such
-  /// that it does not affect foreground tasks significantly. Can be used for
-  /// long-running, latency-insensitive tasks to make sure cpu is not hogged by
-  /// this task.
-  /// If the priority is default tries to restore 

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

[David Goldman via Phabricator via cfe-commits]

dgoldman added inline comments.



Comment at: llvm/include/llvm/Support/Threading.h:239
   };
   /// If priority is Background tries to lower current threads priority such
   /// that it does not affect foreground tasks significantly. Can be used for

Should be "Low" now

Wonder if this comment should be moved up into ThreadPriority?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124715

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

[PATCH] D124715: Use QoS class Utility for ThreadPriority::Low on Mac

[Stefan Haller via Phabricator via cfe-commits]

stefanhaller created this revision.
Herald added subscribers: dexonsmith, usaxena95, kadircet, arphaman, hiraditya.
Herald added a project: All.
stefanhaller requested review of this revision.
Herald added subscribers: cfe-commits, llvm-commits, ilya-biryukov.
Herald added projects: clang, LLVM, clang-tools-extra.

On Apple Silicon Macs, using a Darwin thread priority of PRIO_DARWIN_BG seems to
map directly to the QoS class Background. With this priority, the thread is
confined to efficiency cores only, which makes background indexing take forever.

Change this to use QoS class Utility instead; this makes the thread run on all
cores, but still lowers priority enough to keep the machine responsive, and not
interfere with user-initiated actions.

Also, rename ThreadPriority::Background to ThreadPriority::Low; this avoids
confusion with the QoS class Background, and allows to reintroduce
ThreadPriority::Background later, should there be a need for it.

We might consider changing the Windows and Linux implementations too (e.g. using
SCHED_BATCH instead of SCHED_IDLE on Linux). I didn't do that here because I
don't have access to these systems to test it on.

See also https://github.com/clangd/clangd/issues/1119.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124715

Files:
  clang-tools-extra/clangd/index/Background.h
  clang/tools/libclang/CIndex.cpp
  llvm/include/llvm/Support/Threading.h
  llvm/lib/Support/Unix/Threading.inc
  llvm/lib/Support/Windows/Threading.inc

Index: llvm/lib/Support/Windows/Threading.inc
===
--- llvm/lib/Support/Windows/Threading.inc
+++ llvm/lib/Support/Windows/Threading.inc
@@ -121,7 +121,7 @@
   // priorities of the thread as they were before the thread entered background
   // processing mode.
   return SetThreadPriority(GetCurrentThread(),
-   Priority == ThreadPriority::Background
+   Priority == ThreadPriority::Low
? THREAD_MODE_BACKGROUND_BEGIN
: THREAD_MODE_BACKGROUND_END)
  ? SetThreadPriorityResult::SUCCESS
Index: llvm/lib/Support/Unix/Threading.inc
===
--- llvm/lib/Support/Unix/Threading.inc
+++ llvm/lib/Support/Unix/Threading.inc
@@ -18,6 +18,7 @@
 #if defined(__APPLE__)
 #include 
 #include 
+#include 
 #endif
 
 #include 
@@ -258,27 +259,20 @@
   // SCHED_OTHER   the standard round-robin time-sharing policy;
   return !pthread_setschedparam(
  pthread_self(),
- Priority == ThreadPriority::Background ? SCHED_IDLE : SCHED_OTHER,
+ Priority == ThreadPriority::Low ? SCHED_IDLE : SCHED_OTHER,
  )
  ? SetThreadPriorityResult::SUCCESS
  : SetThreadPriorityResult::FAILURE;
 #elif defined(__APPLE__)
-  // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man2/getpriority.2.html
-  // When setting a thread into background state the scheduling priority is set
-  // to lowest value, disk and network IO are throttled. Network IO will be
-  // throttled for any sockets the thread opens after going into background
-  // state. Any previously opened sockets are not affected.
-
-  // https://developer.apple.com/library/archive/documentation/System/Conceptual/ManPages_iPhoneOS/man3/getiopolicy_np.3.html
-  // I/Os with THROTTLE policy are called THROTTLE I/Os. If a THROTTLE I/O
-  // request occurs within a small time window (usually a fraction of a second)
-  // of another NORMAL I/O request, the thread that issues the THROTTLE I/O is
-  // forced to sleep for a certain interval. This slows down the thread that
-  // issues the THROTTLE I/O so that NORMAL I/Os can utilize most of the disk
-  // I/O bandwidth.
-  return !setpriority(PRIO_DARWIN_THREAD, 0,
-  Priority == ThreadPriority::Background ? PRIO_DARWIN_BG
- : 0)
+  // https://developer.apple.com/documentation/apple-silicon/tuning-your-code-s-performance-for-apple-silicon
+  // Utility - Applies to work that takes anywhere from a few seconds to a few
+  // minutes to complete. Examples include downloading a document or importing
+  // data. This class offers a balance between responsiveness, performance, and
+  // energy efficiency.
+  return !pthread_set_qos_class_self_np(Priority == ThreadPriority::Low
+? QOS_CLASS_UTILITY
+: QOS_CLASS_DEFAULT,
+0)
  ? SetThreadPriorityResult::SUCCESS
  : SetThreadPriorityResult::FAILURE;
 #endif
Index: llvm/include/llvm/Support/Threading.h
===
--- llvm/include/llvm/Support/Threading.h
+++ 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDecl.cpp:15319-15321
   // OpenCL v2.0 s6.9.u - Implicit function declaration is not supported.
   else if (getLangOpts().OpenCL)
 diag_id = diag::err_opencl_implicit_function_decl;

aaron.ballman wrote:
> rsmith wrote:
> > Should we even be calling this function in OpenCL mode? It seems a bit 
> > inconsistent that we avoid calling this in C++ and C2x mode, and that we 
> > call it but error in OpenCL mode.
> > 
> > Maybe there should be a function on `LangOptions` to ask if implicit 
> > function declarations are permitted in the current language mode, to make 
> > it easy to opt out the right cases? (Happy for this to be a follow-on 
> > change if you agree.)
> I agree that it does seem inconsistent. I can look into making that change in 
> a follow-up.
I've fixed that up in a9d68a5524dea113cace5983697786599cbdce9a, thanks for the 
suggestion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122983

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

[clang-tools-extra] a9d68a5 - Generalize calls to ImplicitlyDefineFunction

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-04-30T10:03:51-04:00
New Revision: a9d68a5524dea113cace5983697786599cbdce9a

URL: 
https://github.com/llvm/llvm-project/commit/a9d68a5524dea113cace5983697786599cbdce9a
DIFF: 
https://github.com/llvm/llvm-project/commit/a9d68a5524dea113cace5983697786599cbdce9a.diff

LOG: Generalize calls to ImplicitlyDefineFunction

In C++ and C2x, we would avoid calling ImplicitlyDefineFunction at all,
but in OpenCL mode we would still call the function and have it produce
an error diagnostic. Instead, we now have a helper function to
determine when implicit function definitions are allowed and we use
that to determine whether to call ImplicitlyDefineFunction so that the
behavior is more consistent across language modes.

This changes the diagnostic behavior from telling the users that an
implicit function declaration is not allowed in OpenCL to reporting use
of an unknown identifier and going through typo correction, as done in
C++ and C2x.

Added: 


Modified: 
clang-tools-extra/clangd/IncludeFixer.cpp
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/LangOptions.h
clang/lib/Sema/SemaDecl.cpp
clang/lib/Sema/SemaExpr.cpp
clang/test/Driver/default-includes.cl
clang/test/Preprocessor/macro_variadic.cl
clang/test/SemaOpenCL/arm-integer-dot-product.cl
clang/test/SemaOpenCL/clang-builtin-version.cl
clang/test/SemaOpenCL/fdeclare-opencl-builtins.cl
clang/test/SemaOpenCL/invalid-block.cl
clang/test/SemaOpenCL/to_addr_builtin.cl

Removed: 




diff  --git a/clang-tools-extra/clangd/IncludeFixer.cpp 
b/clang-tools-extra/clangd/IncludeFixer.cpp
index 69ec27a024e1f..7fcb01e6e957a 100644
--- a/clang-tools-extra/clangd/IncludeFixer.cpp
+++ b/clang-tools-extra/clangd/IncludeFixer.cpp
@@ -198,7 +198,6 @@ std::vector IncludeFixer::fix(DiagnosticsEngine::Level 
DiagLevel,
   case diag::err_no_member_template_suggest:
   case diag::warn_implicit_function_decl:
   case diag::ext_implicit_function_decl_c99:
-  case diag::err_opencl_implicit_function_decl:
 dlog("Unresolved name at {0}, last typo was {1}",
  Info.getLocation().printToString(Info.getSourceManager()),
  LastUnresolvedName

diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index 69093a6e51dc3..084d67af570e2 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -10198,8 +10198,6 @@ def 
err_opencl_scalar_type_rank_greater_than_vector_type : Error<
 "element. (%0 and %1)">;
 def err_bad_kernel_param_type : Error<
   "%0 cannot be used as the type of a kernel parameter">;
-def err_opencl_implicit_function_decl : Error<
-  "implicit declaration of function %0 is invalid in OpenCL">;
 def err_record_with_pointers_kernel_param : Error<
   "%select{struct|union}0 kernel parameters may not contain pointers">;
 def note_within_field_of_type : Note<

diff  --git a/clang/include/clang/Basic/LangOptions.h 
b/clang/include/clang/Basic/LangOptions.h
index 9d852d72e593f..eb4c7c4c7d93e 100644
--- a/clang/include/clang/Basic/LangOptions.h
+++ b/clang/include/clang/Basic/LangOptions.h
@@ -527,6 +527,12 @@ class LangOptions : public LangOptionsBase {
 return CPlusPlus || C2x || DisableKNRFunctions;
   }
 
+  /// Returns true if implicit function declarations are allowed in the current
+  /// language mode.
+  bool implicitFunctionsAllowed() const {
+return !requiresStrictPrototypes() && !OpenCL;
+  }
+
   /// Check if return address signing is enabled.
   bool hasSignReturnAddress() const {
 return getSignReturnAddressScope() != SignReturnAddressScopeKind::None;

diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index da6d35ca98a44..5890bbc7d574b 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -934,8 +934,9 @@ Sema::NameClassification Sema::ClassifyName(Scope *S, 
CXXScopeSpec ,
   //   appeared.
   //
   // We also allow this in C99 as an extension. However, this is not
-  // allowed in C2x as there are no functions without prototypes there.
-  if (!getLangOpts().C2x) {
+  // allowed in all language modes as functions without prototypes may not
+  // be supported.
+  if (getLangOpts().implicitFunctionsAllowed()) {
 if (NamedDecl *D = ImplicitlyDefineFunction(NameLoc, *Name, S))
   return NameClassification::NonType(D);
   }
@@ -15273,7 +15274,8 @@ void Sema::ActOnFinishDelayedAttribute(Scope *S, Decl 
*D,
 NamedDecl *Sema::ImplicitlyDefineFunction(SourceLocation Loc,
   IdentifierInfo , Scope *S) {
   // It is not valid to implicitly define a function in C2x.
-  assert(!LangOpts.C2x && "Cannot implicitly define a function in C2x");
+  assert(LangOpts.implicitFunctionsAllowed() &&
+ "Implicit 

[PATCH] D124690: [clangd] add inlay hints for std::forward-ed parameter packs

[Tobias Ribizel via Phabricator via cfe-commits]

upsj created this revision.
upsj added a reviewer: nridge.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
upsj updated this revision to Diff 426127.
upsj added a comment.
upsj added a reviewer: sammccall.
upsj updated this revision to Diff 426224.
upsj updated this revision to Diff 426225.
upsj published this revision for review.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

remove accidentally committed file


upsj added a comment.

remove dependency on D124359  and add tests


upsj added a comment.

remove unnecessary changes




Comment at: clang-tools-extra/clangd/InlayHints.cpp:388
+// arguments
+auto ForwardedParmMatcher = compoundStmt(forEachDescendant(
+invocation(

We tend not to use ASTMatchers for these kind of tasks in clangd (except when 
embedding clang-tidy checks of course).

I guess the biggest technical reason is it's hard to reason about their 
performance so they end up slow and opaque (and indeed the clang-tidy checks 
are slow and we haven't got a clear idea how to fix it). It's also easy to 
match too much.

But part of it is just convention - because we have more RecursiveASTVisitors, 
the maintainers are more familiar with the quirks there than the quirks of 
matchers.

---

To be clear, I don't see any specific problems with this code! But we still 
might ask to change it as there are costs to mixing styles, and we don't want 
to end up in a situation where a bug fix requires choosing between an expensive 
hasAncestor() matcher and rewriting the logic.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:388
+// arguments
+auto ForwardedParmMatcher = compoundStmt(forEachDescendant(
+invocation(

sammccall wrote:
> We tend not to use ASTMatchers for these kind of tasks in clangd (except when 
> embedding clang-tidy checks of course).
> 
> I guess the biggest technical reason is it's hard to reason about their 
> performance so they end up slow and opaque (and indeed the clang-tidy checks 
> are slow and we haven't got a clear idea how to fix it). It's also easy to 
> match too much.
> 
> But part of it is just convention - because we have more 
> RecursiveASTVisitors, the maintainers are more familiar with the quirks there 
> than the quirks of matchers.
> 
> ---
> 
> To be clear, I don't see any specific problems with this code! But we still 
> might ask to change it as there are costs to mixing styles, and we don't want 
> to end up in a situation where a bug fix requires choosing between an 
> expensive hasAncestor() matcher and rewriting the logic.
That makes sense. From the Visitor standpoint, do I understand correctly that 
you mean something like remembering the top-level templated `FunctionDecl` (or 
stack of `FunctionDecl`s if we have something like nested Lambdas?) and check 
every `CallExpr` and `CXXConstructExpr` for `ParmVarDecls` or 
`std::forward(ParmVarDecl)` matching the parameters of the higher-level 
`FunctionDecls`? That means basically lazily evaluating the parameter names, so 
more storage inside the Visitor, but allows us to do recursive resolution in a 
rather straightforward fashion.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:255
+  )cpp",
+   ExpectedHint{"a: ", "param"});
+}

I haven't been able to figure out why, but for some reason the 
`CXXConstructExpr` and `CXXTemporaryObjectExpr` are not being picked up by the 
`RecursiveASTVisitor`, while in a similar situation below, the `CallExpr` gets 
picked up by `VisitCallExpr`.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:270
+  )cpp",
+   ExpectedHint{"a: ", "wrapped"},
+   ExpectedHint{"a: ", "param"});

This is a bit of an unfortunate side-effect of looking at instantiated 
templates.


This adds special-case treatment for parameter packs in make_unique-like 
functions to forward parameter names to inlay hints.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124690

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -162,6 +162,159 @@
ExpectedHint{"good: ", "good"});
 }
 
+TEST(ParameterHints, NoNameVariadicDeclaration) {
+  // No hint for anonymous variadic parameter
+  assertParameterHints(R"cpp(
+template 
+void foo(Args&& ...);
+void bar() {
+  foo(42);
+}
+  )cpp");
+}
+
+TEST(ParameterHints, NoNameVariadicForwarded) {
+  // No hint for 

[PATCH] D124688: [clangd] parse all make_unique-like functions in preamble

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Great! If this were to be a public, user-controlled feature we'd make it part 
of the config file  which is a bit more 
involved, but I think this is rather just a developer toggle until it's time to 
turn it on by default, so a command-line flag seems fine.

This needs to be passed from the main binary => ClangdServer => 
Preamble::build, as something like `bool AlwaysParseForwardingFunctions`.

I think the most appropriate place to pass this to Preamble::build is in 
ParseOptions defined in `Compiler.h` (currently empty) which is embedded in 
ParseInputs. Something like `bool AlwaysParseForwardingFunctions`.
And the usual way to pass command line flags into ClangdServer is via 
ClangdServer::Options.

See 
https://github.com/llvm/llvm-project/commit/e6be5c7cd6d227144f874623e2764890f80cad32
 where we removed a couple of ParseOptions, I think you'd basically want the 
opposite of that.
(Sorry about the plumbing, configuration is always a pain).

For testing, you want the ability to turn this option on in TestTU. I'd just 
add `ParseOptions TestTU::ParseOpts` as a public member that your test can set, 
and use it from `TestTU::inputs()` instead of the current `Inputs.Opts = 
ParseOptions();`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124688

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

[PATCH] D124359: [clangd] Add inlay hints for mutable reference parameters

[Tobias Ribizel via Phabricator via cfe-commits]

upsj updated this revision to Diff 426217.
upsj added a comment.

don't add reference inlay hints for r-value refs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124359

Files:
  clang-tools-extra/clangd/InlayHints.cpp
  clang-tools-extra/clangd/unittests/InlayHintTests.cpp

Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -138,6 +138,38 @@
   )cpp");
 }
 
+TEST(ParameterHints, NoNameConstReference) {
+  // No hint for anonymous const l-value ref parameter.
+  assertParameterHints(R"cpp(
+void foo(const int&);
+void bar() {
+  foo(42);
+}
+  )cpp");
+}
+
+TEST(ParameterHints, NoNameReference) {
+  // Reference hint for anonymous l-value ref parameter.
+  assertParameterHints(R"cpp(
+void foo(int&);
+void bar() {
+  int i;
+  foo($param[[i]]);
+}
+  )cpp",
+   ExpectedHint{"&", "param"});
+}
+
+TEST(ParameterHints, NoNameRValueReference) {
+  // No reference hint for anonymous r-value ref parameter.
+  assertParameterHints(R"cpp(
+void foo(int&&);
+void bar() {
+  foo($param[[42]]);
+}
+  )cpp");
+}
+
 TEST(ParameterHints, NameInDefinition) {
   // Parameter name picked up from definition if necessary.
   assertParameterHints(R"cpp(
@@ -162,6 +194,40 @@
ExpectedHint{"good: ", "good"});
 }
 
+TEST(ParameterHints, NameConstReference) {
+  // Only name hint for const l-value ref parameter.
+  assertParameterHints(R"cpp(
+void foo(const int& param);
+void bar() {
+  foo($param[[42]]);
+}
+  )cpp",
+   ExpectedHint{"param: ", "param"});
+}
+
+TEST(ParameterHints, NameReference) {
+  // Reference and name hint for l-value ref parameter.
+  assertParameterHints(R"cpp(
+void foo(int& param);
+void bar() {
+  int i;
+  foo($param[[i]]);
+}
+  )cpp",
+   ExpectedHint{"param: &", "param"});
+}
+
+TEST(ParameterHints, NameRValueReference) {
+  // Only name hint for r-value ref parameter.
+  assertParameterHints(R"cpp(
+void foo(int&& param);
+void bar() {
+  foo($param[[42]]);
+}
+  )cpp",
+   ExpectedHint{"param: ", "param"});
+}
+
 TEST(ParameterHints, Operator) {
   // No hint for operator call with operator syntax.
   assertParameterHints(R"cpp(
@@ -301,6 +367,21 @@
ExpectedHint{"param: ", "param"});
 }
 
+TEST(ParameterHints, ArgMatchesParamReference) {
+  assertParameterHints(R"cpp(
+void foo(int& param);
+void foo2(const int& param);
+void bar() {
+  int param;
+  // show reference hint on mutable reference
+  foo($param[[param]]);
+  // but not on const reference
+  foo2(param);
+}
+  )cpp",
+   ExpectedHint{"&", "param"});
+}
+
 TEST(ParameterHints, LeadingUnderscore) {
   assertParameterHints(R"cpp(
 void foo(int p1, int _p2, int __p3);
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -10,6 +10,7 @@
 #include "Config.h"
 #include "HeuristicResolver.h"
 #include "ParsedAST.h"
+#include "clang/AST/Decl.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecursiveASTVisitor.h"
@@ -392,6 +393,7 @@
 // Don't show hints for variadic parameters.
 size_t FixedParamCount = getFixedParamCount(Callee);
 size_t ArgCount = std::min(FixedParamCount, Args.size());
+auto Params = Callee->parameters();
 
 NameVec ParameterNames = chooseParameterNames(Callee, ArgCount);
 
@@ -402,12 +404,18 @@
 
 for (size_t I = 0; I < ArgCount; ++I) {
   StringRef Name = ParameterNames[I];
-  if (!shouldHint(Args[I], Name))
-continue;
+  bool NameHint = shouldNameHint(Args[I], Name);
+  std::string Suffix = ": ";
+  if (!NameHint) {
+Name = "";
+Suffix = "";
+  }
+  Suffix += getRefSuffix(Params[I]);
 
-  addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
-   InlayHintKind::ParameterHint, /*Prefix=*/"", Name,
-   /*Suffix=*/": ");
+  if (!Name.empty() || !Suffix.empty()) {
+addInlayHint(Args[I]->getSourceRange(), HintSide::Left,
+ InlayHintKind::ParameterHint, /*Prefix=*/"", Name, Suffix);
+  }
 }
   }
 
@@ -434,12 +442,21 @@
 return WhatItIsSetting.equals_insensitive(ParamNames[0]);
   }
 
-  bool shouldHint(const Expr *Arg, StringRef ParamName) {
+  StringRef getRefSuffix(const ParmVarDecl *Param) {
+// If the parameter is a non-const reference type, print an inlay hint
+auto Type = Param->getType();
+

[PATCH] D124688: [clangd] parse all make_unique-like functions in preamble

[Tobias Ribizel via Phabricator via cfe-commits]

upsj added a comment.

Thanks for checking! Putting it behind a flag was my intention from the start 
anyways, since ideally I would like to traverse the AST through something like 
emplace_back ->construct/realloc_insert -> allocator::construct until I reach a 
non-forwarding function.
Do you have some pointers on what needs to be done to add a new flag? I am 
still kind of new to LLVM development :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124688

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

[PATCH] D124708: Fix "the the" typo in documentation and user facing strings

[Mark de Wever via Phabricator via cfe-commits]

Mordante added a comment.

Thanks for these fixes. SGTM, but I want to see the CI pass.




Comment at: clang/lib/StaticAnalyzer/Checkers/cert/InvalidPtrChecker.cpp:135
   FD->getNameForDiagnostic(Out, FD->getASTContext().getLangOpts(), true);
-  Out << "' call may invalidate the the result of the previous " << '\'';
+  Out << "' call may invalidate the result of the previous " << '\'';
   FD->getNameForDiagnostic(Out, FD->getASTContext().getLangOpts(), true);

I'm not familiar with the static analyzers, so I don't know whether this breaks 
a test.
I see the pre-commit CI couldn't apply your patch. Can you rebase this patch on 
main so the CI runs?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124708

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

[PATCH] D124688: [clangd] parse all make_unique-like functions in preamble

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Trying this on a few files, this seems like it increases preamble sizes up to 
1% or so:

   | Before| After
  -+---+--
  AST.cpp  | 42249648  | 42419732
  XRefsTests.cpp   | 56525116  | 56763768
  SelectionDAGISel.cpp | 37546764  | 37668564
  internal_s.cc| 59582216  | 60024876
  internal_m.cc| 192084984 | 193850560
  internal_l.cc| 365811816 | 368841388

I can't see any reason to think that RAM/CPU usage would be out of proportion 
to this.
A 1% regression here isn't trivial but seems worthwhile for significant 
functional improvements.
I suppose we can add:

- diagnostics (already with this patch)
- inlay hints
- signature help

WDYT about keeping this behind a flag until these are working?
For diagnostics alone, I'm not sure this is a good tradeoff (otherwise why 
would we restrict it to variadics? FWIW allowing all function templates to be 
parsed is +4% to preamble size for internal_l.cc.

I'm less concerned about the effects on the main file, partly because preambles 
are a bigger performance cliff, and partly because we're probably only 
regressing in cases where the user really is seeing benefits too.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124688

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

[PATCH] D124688: [clangd] parse all make_unique-like functions in preamble

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: adamcz.
sammccall added a comment.

It was intentional to only (initially) support std::make_unique as its 
implementation is trivial. Whereas I looked at std::make_shared and it appears 
to result in instantiating a lot of templates.
This was more "I'm sure this is useful and cheap" than "I'm sure covering more 
functions is too expensive" though.

I suspect that parsing all forwarding function bodies is quite expensive (well, 
parsing them => instantiating them => instantiating everything from their 
bodies).
We can measure this in a couple of ways:

- some work will happen while parsing headers, this will show up as increased 
preamble size which is easy to measure reliably
- some will only happen when parsing main files, this will cause increased RAM 
and latency in main-file parses, which is less reproducible

I'll try patching this and at least see what happens to preamble sizes on some 
big files

@adamcz FYI (Adam was looking at forwarding functions, for completion/signature 
help, but isn't anymore I believe)




Comment at: clang-tools-extra/clangd/Preamble.cpp:144
+  // ... with a template parameter pack...
+  if (FT->getTemplateParameters()->hasParameterPack()) {
+auto PackIt = std::find_if(

this is a linear search, and so is the next line, let's just do it once :-)



Comment at: clang-tools-extra/clangd/Preamble.cpp:144
+  // ... with a template parameter pack...
+  if (FT->getTemplateParameters()->hasParameterPack()) {
+auto PackIt = std::find_if(

sammccall wrote:
> this is a linear search, and so is the next line, let's just do it once :-)
I feel that checking template params -> injected template args -> params is a 
bit roundabout. (And it involves some searching and construction of the 
injected template args).

Possibly more direct, and I think equivalent:
- get the last function param, check that it's a pack and unwrap
- check that its (non-ref) type is a TemplateTypeParmType
- verify that the param is from the function template rather than elsewhere, by 
comparing type->getDepth() == funcTemplate->getTemplateParameters()->getDepth()
I think all these are constant-time, too


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124688

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

[PATCH] D124679: [clangd] More precisely enable clang warnings through ClangTidy options

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Thanks for revert, and apologies for missing the bots.

(I did of course run the tests locally, but in a weird config that didn't 
trigger the failure)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124679

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

[clang-tools-extra] 816399c - Reland [clangd] More precisely enable clang warnings through ClangTidy options

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-04-30T11:07:11+02:00
New Revision: 816399cac2475499437c5a62e21a165f2de6460a

URL: 
https://github.com/llvm/llvm-project/commit/816399cac2475499437c5a62e21a165f2de6460a
DIFF: 
https://github.com/llvm/llvm-project/commit/816399cac2475499437c5a62e21a165f2de6460a.diff

LOG: Reland [clangd] More precisely enable clang warnings through ClangTidy 
options

This reverts commit 26c82f3d1de11cdada57e499b63a05d24e18b656.

When tests enable 'Checks: *', we may get extra diagnostics.

Added: 


Modified: 
clang-tools-extra/clangd/ParsedAST.cpp
clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
clang/include/clang/Basic/DiagnosticIDs.h
clang/lib/Basic/DiagnosticIDs.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/ParsedAST.cpp 
b/clang-tools-extra/clangd/ParsedAST.cpp
index 235362b5d599c..54d8e391cd94a 100644
--- a/clang-tools-extra/clangd/ParsedAST.cpp
+++ b/clang-tools-extra/clangd/ParsedAST.cpp
@@ -233,12 +233,69 @@ class ReplayPreamble : private PPCallbacks {
   std::vector MainFileTokens;
 };
 
+// Filter for clang diagnostics groups enabled by CTOptions.Checks.
+//
+// These are check names like clang-diagnostics-unused.
+// Note that unlike -Wunused, clang-diagnostics-unused does not imply
+// subcategories like clang-diagnostics-unused-function.
+//
+// This is used to determine which diagnostics can be enabled by ExtraArgs in
+// the clang-tidy configuration.
+class TidyDiagnosticGroups {
+  // Whether all diagnostic groups are enabled by default.
+  // True if we've seen clang-diagnostic-*.
+  bool Default = false;
+  // Set of diag::Group whose enablement != Default.
+  // If Default is false, this is foo where we've seen clang-diagnostic-foo.
+  llvm::DenseSet Exceptions;
+
+public:
+  TidyDiagnosticGroups(llvm::StringRef Checks) {
+constexpr llvm::StringLiteral CDPrefix = "clang-diagnostic-";
+
+llvm::StringRef Check;
+while (!Checks.empty()) {
+  std::tie(Check, Checks) = Checks.split(',');
+  if (Check.empty())
+continue;
+
+  bool Enable = !Check.consume_front("-");
+  bool Glob = Check.consume_back("*");
+  if (Glob) {
+// Is this clang-diagnostic-*, or *, or so?
+// (We ignore all other types of globs).
+if (CDPrefix.startswith(Check)) {
+  Default = Enable;
+  Exceptions.clear();
+}
+continue;
+  }
+
+  // In "*,clang-diagnostic-foo", the latter is a no-op.
+  if (Default == Enable)
+continue;
+  // The only non-glob entries we care about are clang-diagnostic-foo.
+  if (!Check.consume_front(CDPrefix))
+continue;
+
+  if (auto Group = DiagnosticIDs::getGroupForWarningOption(Check))
+Exceptions.insert(static_cast(*Group));
+}
+  }
+
+  bool operator()(diag::Group GroupID) const {
+return Exceptions.contains(static_cast(GroupID)) ? !Default
+   : Default;
+  }
+};
+
 // Find -W and -Wno- options in ExtraArgs and apply them to 
Diags.
 //
 // This is used to handle ExtraArgs in clang-tidy configuration.
 // We don't use clang's standard handling of this as we want slightly 
diff erent
 // behavior (e.g. we want to exclude these from -Wno-error).
 void applyWarningOptions(llvm::ArrayRef ExtraArgs,
+ llvm::function_ref EnabledGroups,
  DiagnosticsEngine ) {
   for (llvm::StringRef Group : ExtraArgs) {
 // Only handle args that are of the form -W[no-].
@@ -259,6 +316,9 @@ void applyWarningOptions(llvm::ArrayRef 
ExtraArgs,
   if (Enable) {
 if (Diags.getDiagnosticLevel(ID, SourceLocation()) <
 DiagnosticsEngine::Warning) {
+  auto Group = DiagnosticIDs::getGroupForDiag(ID);
+  if (!Group || !EnabledGroups(*Group))
+continue;
   Diags.setSeverity(ID, diag::Severity::Warning, SourceLocation());
   if (Diags.getWarningsAsErrors())
 NeedsWerrorExclusion = true;
@@ -354,18 +414,25 @@ ParsedAST::build(llvm::StringRef Filename, const 
ParseInputs ,
 //   - ExtraArgs: ["-Wfoo"] causes clang to produce the warnings
 //   - Checks: "clang-diagnostic-foo" prevents clang-tidy filtering them 
out
 //
-// We treat these as clang warnings, so the Checks part is not relevant.
-// We must enable the warnings specified in ExtraArgs.
+// In clang-tidy, diagnostics are emitted if they pass both checks.
+// When groups contain subgroups, -Wparent includes the child, but
+// clang-diagnostic-parent does not.
 //
-// We *don't* want to change the compile command directly. this can have
+// We *don't* want to change the compile command directly. This can have
 // too many unexpected effects: breaking the command, interactions with
 // -- and -Werror, etc. Besides, we've already parsed the 

[clang] a60ef98 - ClangDriverTests:ToolChainTest.cpp: Fix warnings. [-Wsign-compare]

[NAKAMURA Takumi via cfe-commits]

Author: NAKAMURA Takumi
Date: 2022-04-30T17:19:26+09:00
New Revision: a60ef98bb11378b24045372a53c55923df13ddc3

URL: 
https://github.com/llvm/llvm-project/commit/a60ef98bb11378b24045372a53c55923df13ddc3
DIFF: 
https://github.com/llvm/llvm-project/commit/a60ef98bb11378b24045372a53c55923df13ddc3.diff

LOG: ClangDriverTests:ToolChainTest.cpp: Fix warnings. [-Wsign-compare]

EXPECT_EQ(num,num) is aware of signedness, even if rhs is a constant.

Added: 


Modified: 
clang/unittests/Driver/ToolChainTest.cpp

Removed: 




diff  --git a/clang/unittests/Driver/ToolChainTest.cpp 
b/clang/unittests/Driver/ToolChainTest.cpp
index 7abcb3ee0d975..c652b093dc993 100644
--- a/clang/unittests/Driver/ToolChainTest.cpp
+++ b/clang/unittests/Driver/ToolChainTest.cpp
@@ -544,7 +544,7 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
 if (A)
   EXPECT_STREQ(A->getValue(), "1.1");
   }
-  EXPECT_EQ(Diags.getNumErrors(), 0);
+  EXPECT_EQ(Diags.getNumErrors(), 0u);
 
   // Invalid tests.
   Args = TheDriver.ParseArgStrings({"-validator-version", "0.1"}, false,
@@ -555,7 +555,7 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
 DAL->append(A);
 
   TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None);
-  EXPECT_EQ(Diags.getNumErrors(), 1);
+  EXPECT_EQ(Diags.getNumErrors(), 1u);
   EXPECT_STREQ(DiagConsumer->Errors.back().c_str(),
"invalid validator version : 0.1\nIf validator major version is 
"
"0, minor version must also be 0.");
@@ -570,7 +570,7 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
 DAL->append(A);
 
   TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None);
-  EXPECT_EQ(Diags.getNumErrors(), 2);
+  EXPECT_EQ(Diags.getNumErrors(), 2u);
   EXPECT_STREQ(DiagConsumer->Errors.back().c_str(),
"invalid validator version : 1\nFormat of validator version is "
"\".\" (ex:\"1.4\").");
@@ -585,7 +585,7 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
 DAL->append(A);
 
   TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None);
-  EXPECT_EQ(Diags.getNumErrors(), 3);
+  EXPECT_EQ(Diags.getNumErrors(), 3u);
   EXPECT_STREQ(
   DiagConsumer->Errors.back().c_str(),
   "invalid validator version : -Tlib_6_7\nFormat of validator version is "
@@ -601,7 +601,7 @@ TEST(DxcModeTest, ValidatorVersionValidation) {
 DAL->append(A);
 
   TranslatedArgs = TC.TranslateArgs(*DAL, "0", Action::OffloadKind::OFK_None);
-  EXPECT_EQ(Diags.getNumErrors(), 4);
+  EXPECT_EQ(Diags.getNumErrors(), 4u);
   EXPECT_STREQ(
   DiagConsumer->Errors.back().c_str(),
   "invalid validator version : foo\nFormat of validator version is "



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