[PATCH] D141192: [Clang] Add warnings on bad shifts inside enums.

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D141192#4033591 , @shafik wrote:

> So it looks like in `handleIntIntBinOp` we do hit this code:
>
>   unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
>if (SA != RHS) {
>  Info.CCEDiag(E, diag::note_constexpr_large_shift)
><< RHS << E->getType() << LHS.getBitWidth();
>
> So maybe we should figure out why we decide not to emit this diagnostic and 
> fix it there.

We return `true` anyway and so the diagnostics are not printed. This is reached 
from a call to `VerifyIntegerConstantExpression()` IIRC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141192

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

[PATCH] D141215: [clang-repl][WIP] Implement pretty printing

[Jun Zhang via Phabricator via cfe-commits]

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

This patch implements the initial value pretty printing for clang-repl.
In general, when clang-repl finds something need to print, it will
synthesizes a call to `__InterpreterPrettyPrint` and then calls JIT.

Signed-off-by: Jun Zhang 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141215

Files:
  clang/include/clang/Interpreter/Interpreter.h
  clang/lib/Interpreter/CMakeLists.txt
  clang/lib/Interpreter/IncrementalParser.cpp
  clang/lib/Interpreter/IncrementalParser.h
  clang/lib/Interpreter/Interpreter.cpp
  clang/lib/Interpreter/PrettyPrint.cpp
  clang/tools/clang-repl/CMakeLists.txt
  clang/tools/clang-repl/ClangRepl.cpp

Index: clang/tools/clang-repl/ClangRepl.cpp
===
--- clang/tools/clang-repl/ClangRepl.cpp
+++ clang/tools/clang-repl/ClangRepl.cpp
@@ -10,6 +10,7 @@
 //
 //===--===//
 
+#include "clang/AST/Decl.h"
 #include "clang/Basic/Diagnostic.h"
 #include "clang/Frontend/CompilerInstance.h"
 #include "clang/Frontend/FrontendDiagnostic.h"
@@ -18,6 +19,8 @@
 #include "llvm/ExecutionEngine/Orc/LLJIT.h"
 #include "llvm/LineEditor/LineEditor.h"
 #include "llvm/Support/CommandLine.h"
+#include "llvm/Support/Error.h"
+#include "llvm/Support/ErrorHandling.h"
 #include "llvm/Support/ManagedStatic.h" // llvm_shutdown
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/TargetSelect.h" // llvm::Initialize*
@@ -65,6 +68,18 @@
   return (Errs || HasError) ? EXIT_FAILURE : EXIT_SUCCESS;
 }
 
+static void DeclareMagicFunctions(clang::Interpreter &Interp) {
+  llvm::ArrayRef MagicFunctions = {
+  "void __InterpreterPrettyPrint(void*, char);",
+  "void __InterpreterPrettyPrint(void*, int);",
+  "void __InterpreterPrettyPrint(void*, float);",
+  "void __InterpreterPrettyPrint(void*, double);",
+  };
+  for (llvm::StringRef Function : MagicFunctions) {
+llvm::cantFail(Interp.ParseAndExecute(Function));
+  }
+}
+
 llvm::ExitOnError ExitOnErr;
 int main(int argc, const char **argv) {
   ExitOnErr.setBanner("clang-repl: ");
@@ -109,21 +124,51 @@
 
   bool HasError = false;
 
+  DeclareMagicFunctions(*Interp);
   if (OptInputs.empty()) {
 llvm::LineEditor LE("clang-repl");
 // FIXME: Add LE.setListCompleter
 while (llvm::Optional Line = LE.readLine()) {
-  if (*Line == R"(%quit)")
+  llvm::StringRef Code = *Line;
+  if (Code == R"(%quit)")
 break;
-  if (*Line == R"(%undo)") {
+  if (Code == R"(%undo)") {
 if (auto Err = Interp->Undo()) {
   llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
   HasError = true;
 }
 continue;
   }
+  if (!Code.starts_with("#") && !Code.ends_with(";")) {
+clang::Interpreter::PrettyPrintRAII X(*Interp);
+// Capture the expression we want to print.
+auto ExprOrErr = Interp->CaptureExpr(Code);
+if (!ExprOrErr) {
+  llvm::logAllUnhandledErrors(ExprOrErr.takeError(), llvm::errs(),
+  "error: ");
+  HasError = true;
+} else {
+  // Synthesize a CallExpr to `__InterpreterPrettyPrint`.
+  clang::Expr *E = Interp->SynthesizeCall(ExprOrErr.get());
+  // Generate a PartialTranslationUnit from the CallExpr.
+  llvm::Expected SynthesizedPTUOrErr =
+  Interp->GenPTU(E);
+  if (!SynthesizedPTUOrErr) {
+llvm::logAllUnhandledErrors(SynthesizedPTUOrErr.takeError(),
+llvm::errs(), "error: ");
+HasError = true;
+  }
+  // Let JIT handle all the rest.
+  if (auto Err = Interp->Execute(SynthesizedPTUOrErr.get())) {
+llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(),
+"error: ");
+HasError = true;
+  }
+}
+continue;
+  }
 
-  if (auto Err = Interp->ParseAndExecute(*Line)) {
+  if (auto Err = Interp->ParseAndExecute(Code)) {
 llvm::logAllUnhandledErrors(std::move(Err), llvm::errs(), "error: ");
 HasError = true;
   }
Index: clang/tools/clang-repl/CMakeLists.txt
===
--- clang/tools/clang-repl/CMakeLists.txt
+++ clang/tools/clang-repl/CMakeLists.txt
@@ -12,6 +12,7 @@
   )
 
 clang_target_link_libraries(clang-repl PRIVATE
+  clangAST
   clangBasic
   clangFrontend
   clangInterpreter
Index: clang/lib/Interpreter/PrettyPrint.cpp
===
--- /dev/null
+++ clang/lib/Interpreter/PrettyPrint.cpp
@@ -0,0 +1,37 @@
+#include "clang/AST/Type.h"
+#include "llvm/Support/raw_ostream.h"
+
+using namesp

[clang-tools-extra] 29ffafb - [clang-tools-extra] Remove remaining uses of llvm::Optional (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2023-01-07T20:34:53-08:00
New Revision: 29ffafb5754100502da70171b47ee8a0f722c994

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

LOG: [clang-tools-extra] Remove remaining uses of llvm::Optional (NFC)

This patch removes the unused "using" declaration and removes #include
"llvm/ADT/Optional.h".

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

Added: 


Modified: 
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
clang-tools-extra/clang-tidy/ClangTidyCheck.h
clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
clang-tools-extra/clang-tidy/objc/NSInvocationArgumentLifetimeCheck.cpp
clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp
clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp

clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.h
clang-tools-extra/clang-tidy/readability/QualifiedAutoCheck.cpp
clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.h
clang-tools-extra/clang-tidy/utils/RenamerClangTidyCheck.h
clang-tools-extra/clangd/AST.cpp
clang-tools-extra/clangd/ClangdLSPServer.cpp
clang-tools-extra/clangd/ClangdLSPServer.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/CodeComplete.cpp
clang-tools-extra/clangd/CodeComplete.h
clang-tools-extra/clangd/ConfigCompile.cpp
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/Diagnostics.h
clang-tools-extra/clangd/FS.h
clang-tools-extra/clangd/GlobalCompilationDatabase.cpp
clang-tools-extra/clangd/GlobalCompilationDatabase.h
clang-tools-extra/clangd/HeaderSourceSwitch.h
clang-tools-extra/clangd/Hover.cpp
clang-tools-extra/clangd/IncludeFixer.cpp
clang-tools-extra/clangd/IncludeFixer.h
clang-tools-extra/clangd/ParsedAST.h
clang-tools-extra/clangd/Protocol.h
clang-tools-extra/clangd/SemanticHighlighting.cpp
clang-tools-extra/clangd/TUScheduler.cpp
clang-tools-extra/clangd/TUScheduler.h
clang-tools-extra/clangd/XRefs.cpp
clang-tools-extra/clangd/XRefs.h
clang-tools-extra/clangd/index/BackgroundIndexStorage.cpp
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/FileIndex.h
clang-tools-extra/clangd/index/Index.h
clang-tools-extra/clangd/index/YAMLSerialization.cpp
clang-tools-extra/clangd/refactor/Tweak.h
clang-tools-extra/clangd/refactor/tweaks/DefineInline.cpp
clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
clang-tools-extra/clangd/refactor/tweaks/ExpandAutoType.cpp
clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp
clang-tools-extra/clangd/refactor/tweaks/ObjCMemberwiseInitializer.cpp
clang-tools-extra/clangd/tool/Check.cpp
clang-tools-extra/clangd/tool/ClangdMain.cpp
clang-tools-extra/clangd/unittests/ClangdTests.cpp
clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
clang-tools-extra/clangd/unittests/GlobalCompilationDatabaseTests.cpp
clang-tools-extra/clangd/unittests/TestFS.cpp
clang-tools-extra/clangd/unittests/TestFS.h

Removed: 




diff  --git 
a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp 
b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
index d055bf369de4..a0a510c20131 100644
--- 
a/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
+++ 
b/clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
@@ -23,7 +23,6 @@
 #include "clang/Tooling/DiagnosticsYaml.h"
 #include "clang/Tooling/ReplacementsYaml.h"
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/Support/FileSystem.h"
 #include "llvm/Support/MemoryBuffer.h"

diff  --git 
a/clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp 
b/clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
index e067414f7d17..7a6cdf742f42 100644
--- a/clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
+++ b/clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
@@ -16,7 +16,6 @@
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #incl

[clang-tools-extra] 71f5573 - [clang-tools-extra] Add #include (NFC)

[Kazu Hirata via cfe-commits]

Author: Kazu Hirata
Date: 2023-01-07T20:02:20-08:00
New Revision: 71f557355ddaea358c43b151de3a0e045aaa0863

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

LOG: [clang-tools-extra] Add #include  (NFC)

This patch adds #include  to those files containing
llvm::Optional<...> or Optional<...>.

I'll post a separate patch to actually replace llvm::Optional with
std::optional.

This is part of an effort to migrate from llvm::Optional to
std::optional:

https://discourse.llvm.org/t/deprecating-llvm-optional-x-hasvalue-getvalue-getvalueor/63716

Added: 


Modified: 
clang-tools-extra/clang-apply-replacements/lib/Tooling/ApplyReplacements.cpp
clang-tools-extra/clang-doc/HTMLGenerator.cpp
clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllMacros.cpp
clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllMacros.h
clang-tools-extra/clang-include-fixer/find-all-symbols/FindAllSymbols.cpp
clang-tools-extra/clang-query/Query.cpp
clang-tools-extra/clang-query/QueryParser.cpp
clang-tools-extra/clang-query/tool/ClangQuery.cpp
clang-tools-extra/clang-tidy/ClangTidyCheck.cpp
clang-tools-extra/clang-tidy/ClangTidyCheck.h
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
clang-tools-extra/clang-tidy/ExpandModularHeadersPPCallbacks.cpp
clang-tools-extra/clang-tidy/NoLintDirectiveHandler.cpp
clang-tools-extra/clang-tidy/abseil/DurationAdditionCheck.cpp
clang-tools-extra/clang-tidy/abseil/DurationComparisonCheck.cpp
clang-tools-extra/clang-tidy/abseil/DurationConversionCastCheck.cpp
clang-tools-extra/clang-tidy/abseil/DurationFactoryFloatCheck.cpp
clang-tools-extra/clang-tidy/abseil/DurationFactoryScaleCheck.cpp
clang-tools-extra/clang-tidy/abseil/DurationRewriter.cpp
clang-tools-extra/clang-tidy/abseil/DurationRewriter.h
clang-tools-extra/clang-tidy/abseil/DurationSubtractionCheck.cpp
clang-tools-extra/clang-tidy/abseil/FasterStrsplitDelimiterCheck.cpp
clang-tools-extra/clang-tidy/abseil/TimeComparisonCheck.cpp
clang-tools-extra/clang-tidy/abseil/TimeSubtractionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.cpp
clang-tools-extra/clang-tidy/bugprone/BadSignalToKillThreadCheck.h
clang-tools-extra/clang-tidy/bugprone/EasilySwappableParametersCheck.cpp

clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp

clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.h
clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.cpp
clang-tools-extra/clang-tidy/bugprone/ReservedIdentifierCheck.h
clang-tools-extra/clang-tidy/bugprone/SuspiciousIncludeCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SuspiciousMemoryComparisonCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.cpp
clang-tools-extra/clang-tidy/bugprone/UncheckedOptionalAccessCheck.h
clang-tools-extra/clang-tidy/bugprone/UseAfterMoveCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/InitVariablesCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/ProBoundsConstantArrayIndexCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
clang-tools-extra/clang-tidy/google/UpgradeGoogletestCaseCheck.cpp
clang-tools-extra/clang-tidy/misc/RedundantExpressionCheck.cpp
clang-tools-extra/clang-tidy/misc/StaticAssertCheck.cpp
clang-tools-extra/clang-tidy/modernize/DeprecatedIosBaseAliasesCheck.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertCheck.cpp
clang-tools-extra/clang-tidy/modernize/LoopConvertUtils.cpp

clang-tools-extra/clang-tidy/modernize/ReplaceDisallowCopyAndAssignMacroCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseTrailingReturnTypeCheck.h
clang-tools-extra/clang-tidy/mpi/BufferDerefCheck.h
clang-tools-extra/clang-tidy/mpi/TypeMismatchCheck.h
clang-tools-extra/clang-tidy/objc/NSInvocationArgumentLifetimeCheck.cpp
clang-tools-extra/clang-tidy/performance/FasterStringFindCheck.cpp
clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp
clang-tools-extra/clang-tidy/performance/UnnecessaryValueParamCheck.cpp
clang-tools-extra/clang-tidy/readability/ConstReturnTypeCheck.cpp

clang-tools-extra/clang-tidy/readability/FunctionCognitiveComplexityCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
clang-tools-extra/clang-tidy/readability/Identi

[PATCH] D137058: [Driver] [Modules] Support -fmodule-output (1/2)

[H. Vetinari via Phabricator via cfe-commits]

h-vetinari added a comment.

Without undue haste, I just wanted to comment from the peanut gallery that it 
would be amazing if the patches that are necessary for CMake + Clang to use C++ 
modules would make the cut-off for LLVM 16 that's coming up around the 24th of 
January.


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

https://reviews.llvm.org/D137058

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

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Chris Cotter via Phabricator via cfe-commits]

ccotter marked 2 inline comments as done.
ccotter added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp:35
+Capture.getLocation(), SourceMgr, Context.getLangOpts(), tok::amp);
+llvm::errs() << "FOR REF capture loc= "
+ << Capture.getLocation().printToString(SourceMgr)

carlosgalvezp wrote:
> Not having being involved in the development of this check I don't quite 
> understand what this error message means, could you provide a more 
> descriptive message?
> 
> It's also unclear if the program is supposed to abort when entering this 
> branch, or if it's expected that it returns just fine? If it's supposed to 
> return just fine, I think the check should not print anything, to keep the 
> users' logs clean.
My bad - I forgot to remove this trace. Both branches are valid branches for 
normal program execution.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

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

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 487130.
ccotter added a comment.

- rm trace; tidy docs


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-this-with-capture-default.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
@@ -0,0 +1,92 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-this-with-capture-default %t
+
+struct Obj {
+  void lambdas_that_warn_default_capture_copy() {
+int local{};
+int local2{};
+
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };
+
+auto explicit_this_capture_locals1 = [=, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [local, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_locals2 = [=, this]() { return (local+local2) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [local, local2, this]() { return (local+local2) > 10; };
+
+auto explicit_this_capture_local_ref = [=, this, &local]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this, &local]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref2 = [=, &local, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref3 = [=, &local, this, &local2]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, this, &local2]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref4 = [=, &local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, &local2, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref_extra_whitespace = [=, &  local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:62: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&  local, &local2, this]() { return (local+x) > 10; }
+
+auto explicit_this_capture_local_ref_with_comment = [=, & /* byref */ local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:58: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [& /* byref */ local, &local2, this]() { return (local+x) > 10; }
+
+auto implicit_this_capture = [=]() { return x > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { return x > 10; };
+
+auto implicit_this_capture_local = [=]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:41: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-captur

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

Also, this patch is lacking tests for now - mostly bringing it up for 
discussion first - I can add tests if others agree that it seems like a 
reasonable path forward.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141206

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

[PATCH] D141206: [clang] [MinGW] Avoid adding /include and /lib when cross compiling

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo created this revision.
mstorsjo added reviewers: mati865, alvinhochun.
Herald added a subscriber: pengfei.
Herald added a project: All.
mstorsjo requested review of this revision.
Herald added a subscriber: MaskRay.
Herald added a project: clang.

The MinGW compiler driver first tries to deduce the root of
the toolchain installation (either clang itself or a separate
cross mingw gcc installation). On top of this root, a number
of include and lib paths are added (some added unconditionally,
some only if they exist):

- /x86_64-w64-mingw32/include
- /include
- /include/x86_64-w64-windows-gnu

(Some more are also added for libstdc++ and/or libc++.)

The first one is the one commonly used for MinGW targets so
far. For LLVM runtimes installed with the
LLVM_ENABLE_PER_TARGET_RUNTIME_DIR option, the latter two are
used though (this is currently not the default, not yet at least).

For cross compiling, if base is a separate dedicated directory,
this is fine, but when using the sysroots of a distro-installed
cross mingw toolchain, base is /usr - and having /usr/include
in the include path for cross compilation is a potential
source for problems; see
https://github.com/llvm/llvm-project/issues/59871.

If not cross compiling though, /include needs to be included
too. E.g. in the case of msys2, most headers are in e.g.
/mingw64/include while the compiler is /mingw64/bin/clang.

When cross compiling, if the sysroot has been explicitly set
by the user, keep /include too. (In the case of a distro
provided cross gcc toolchain in /usr, the sysroot needs to be set
to /usr and not /usr/x86_64-w64-mingw32 though, to be able to find
libgcc files under /usr/lib/gcc/x86_64-w64-mingw32. So with such a
toolchain, setting the sysroot explicitly does retain the problem.)

All in all - this avoids adding /usr/include and /usr/lib to the
include/lib paths when doing mingw cross compilation with a
distro-provided sysroot in /usr/x86_64-w64-mingw32.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141206

Files:
  clang/lib/Driver/ToolChains/MinGW.cpp


Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -348,6 +348,15 @@
  Exec, CmdArgs, Inputs, Output));
 }
 
+static bool isCrossCompiling(const llvm::Triple &T, bool RequireArchMatch) {
+  llvm::Triple HostTriple(llvm::Triple::normalize(LLVM_HOST_TRIPLE));
+  if (HostTriple.getOS() != llvm::Triple::Win32)
+return true;
+  if (RequireArchMatch && HostTriple.getArch() != T.getArch())
+return true;
+  return false;
+}
+
 // Simplified from 
Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple.
 static bool findGccVersion(StringRef LibDir, std::string &GccLibDir,
std::string &Ver,
@@ -487,7 +496,13 @@
   getFilePaths().push_back(
   (Base + SubdirName + llvm::sys::path::get_separator() + 
"mingw/lib").str());
 
-  getFilePaths().push_back(Base + "lib");
+  // Only include /lib if we're not cross compiling (not even for
+  // windows->windows to a different arch), or if the sysroot has been set
+  // (where we presume the user has pointed it at an arch specific
+  // subdirectory).
+  if (!::isCrossCompiling(getTriple(), /*RequireArchMatch=*/true) ||
+  getDriver().SysRoot.size())
+getFilePaths().push_back(Base + "lib");
 
   NativeLLVMSupport =
   Args.getLastArgValue(options::OPT_fuse_ld_EQ, CLANG_DEFAULT_LINKER)
@@ -649,7 +664,13 @@
   addSystemInclude(DriverArgs, CC1Args,
Base + SubdirName + llvm::sys::path::get_separator() + 
"usr/include");
 
-  addSystemInclude(DriverArgs, CC1Args, Base + "include");
+  // Only include /include if we're not cross compiling (but do allow it
+  // if we're on Windows and building for Windows on another architecture),
+  // or if the sysroot has been set (where we presume the user has pointed it
+  // at an arch specific subdirectory).
+  if (!::isCrossCompiling(getTriple(), /*RequireArchMatch=*/false) ||
+  getDriver().SysRoot.size())
+addSystemInclude(DriverArgs, CC1Args, Base + "include");
 }
 
 void toolchains::MinGW::addClangTargetOptions(


Index: clang/lib/Driver/ToolChains/MinGW.cpp
===
--- clang/lib/Driver/ToolChains/MinGW.cpp
+++ clang/lib/Driver/ToolChains/MinGW.cpp
@@ -348,6 +348,15 @@
  Exec, CmdArgs, Inputs, Output));
 }
 
+static bool isCrossCompiling(const llvm::Triple &T, bool RequireArchMatch) {
+  llvm::Triple HostTriple(llvm::Triple::normalize(LLVM_HOST_TRIPLE));
+  if (HostTriple.getOS() != llvm::Triple::Win32)
+return true;
+  if (RequireArchMatch && HostTriple.getArch() != T.getArch())
+return true;
+  return false;
+}
+
 // Simplified from Generic_GCC::GCCInstallationDetector::ScanLibDirForGCCTriple.
 static bool findGccVe

[PATCH] D141098: [clang-format][NFC] Set DeriveLineEnding to false in config files

[Owen Pan via Phabricator via cfe-commits]

owenpan added a comment.

In D141098#4032951 , @rymiel wrote:

>> I think we should combine `DeriveLineEnding` and `UseCRLF`
>
> Do you mean a single `LineEnding` options which is an enum with `LF`, `CRLF` 
> and `Derive`, or similar?

Yep.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141098

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

[PATCH] D141177: [Clang] Don't tell people to place _Alignas on a struct in diagnostics

[Theodore Luo Wang via Phabricator via cfe-commits]

theo-lw updated this revision to Diff 487119.
theo-lw added a comment.

Didn't realize this test was failing so I've updated it.


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

https://reviews.llvm.org/D141177

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/C/drs/dr4xx.c
  clang/test/Parser/c1x-alignas.c


Index: clang/test/Parser/c1x-alignas.c
===
--- clang/test/Parser/c1x-alignas.c
+++ clang/test/Parser/c1x-alignas.c
@@ -9,5 +9,7 @@
 
 char _Alignas(_Alignof(int)) c5;
 
+_Alignas(int) struct c6; // expected-warning {{1 attribute ignored}}
+
 // CHECK-EXT: '_Alignas' is a C11 extension
 // CHECK-EXT: '_Alignof' is a C11 extension
Index: clang/test/C/drs/dr4xx.c
===
--- clang/test/C/drs/dr4xx.c
+++ clang/test/C/drs/dr4xx.c
@@ -164,11 +164,7 @@
   /* FIXME: This should be accepted as per this DR. */
   int j = (_Alignas(int) int){12}; /* expected-error {{expected expression}} */
 
- /* FIXME: The diagnostic in this case is really bad; moving the specifier to
-  * where the diagnostic recommends causes a different, more inscrutable error
-  * about anonymous structures.
-  */
-  _Alignas(int) struct T { /* expected-warning {{attribute '_Alignas' is 
ignored, place it after "struct" to apply attribute to type declaration}} */
+  _Alignas(int) struct T { /* expected-warning {{1 attribute ignored}} */
 int i;
   };
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5292,17 +5292,29 @@
   // Attributes should be placed after tag to apply to type declaration.
   if (!DS.getAttributes().empty() || !DeclAttrs.empty()) {
 DeclSpec::TST TypeSpecType = DS.getTypeSpecType();
+auto WarnAttributeIgnored = [this, TypeSpecType](const ParsedAttr &AL) {
+  if (AL.isAlignasAttribute()) {
+// Don't use the message with placement with _Alignas.
+// This is because C doesnt let you use _Alignas on type declarations.
+Diag(AL.getLoc(), diag::warn_attribute_ignored)
+<< GetDiagnosticTypeSpecifierID(TypeSpecType);
+  } else {
+Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
+<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  }
+};
+
 if (TypeSpecType == DeclSpec::TST_class ||
 TypeSpecType == DeclSpec::TST_struct ||
 TypeSpecType == DeclSpec::TST_interface ||
 TypeSpecType == DeclSpec::TST_union ||
 TypeSpecType == DeclSpec::TST_enum) {
-  for (const ParsedAttr &AL : DS.getAttributes())
-Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-  for (const ParsedAttr &AL : DeclAttrs)
-Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  for (const ParsedAttr &AL : DS.getAttributes()) {
+WarnAttributeIgnored(AL);
+  }
+  for (const ParsedAttr &AL : DeclAttrs) {
+WarnAttributeIgnored(AL);
+  }
 }
   }
 


Index: clang/test/Parser/c1x-alignas.c
===
--- clang/test/Parser/c1x-alignas.c
+++ clang/test/Parser/c1x-alignas.c
@@ -9,5 +9,7 @@
 
 char _Alignas(_Alignof(int)) c5;
 
+_Alignas(int) struct c6; // expected-warning {{1 attribute ignored}}
+
 // CHECK-EXT: '_Alignas' is a C11 extension
 // CHECK-EXT: '_Alignof' is a C11 extension
Index: clang/test/C/drs/dr4xx.c
===
--- clang/test/C/drs/dr4xx.c
+++ clang/test/C/drs/dr4xx.c
@@ -164,11 +164,7 @@
   /* FIXME: This should be accepted as per this DR. */
   int j = (_Alignas(int) int){12}; /* expected-error {{expected expression}} */
 
- /* FIXME: The diagnostic in this case is really bad; moving the specifier to
-  * where the diagnostic recommends causes a different, more inscrutable error
-  * about anonymous structures.
-  */
-  _Alignas(int) struct T { /* expected-warning {{attribute '_Alignas' is ignored, place it after "struct" to apply attribute to type declaration}} */
+  _Alignas(int) struct T { /* expected-warning {{1 attribute ignored}} */
 int i;
   };
 
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5292,17 +5292,29 @@
   // Attributes should be placed after tag to apply to type declaration.
   if (!DS.getAttributes().empty() || !DeclAttrs.empty()) {
 DeclSpec::TST TypeSpecType = DS.getTypeSpecType();
+auto WarnAttributeIgnored = [this, TypeSpecType](const ParsedAttr &AL) {
+  if (AL.isAlignasAttribute()) {
+// Don't use the message with placement with _Alignas.
+// This is because C doesnt let you use

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp:35
+Capture.getLocation(), SourceMgr, Context.getLangOpts(), tok::amp);
+llvm::errs() << "FOR REF capture loc= "
+ << Capture.getLocation().printToString(SourceMgr)

Not having being involved in the development of this check I don't quite 
understand what this error message means, could you provide a more descriptive 
message?

It's also unclear if the program is supposed to abort when entering this 
branch, or if it's expected that it returns just fine? If it's supposed to 
return just fine, I think the check should not print anything, to keep the 
users' logs clean.



Comment at: clang-tools-extra/docs/ReleaseNotes.rst:114
+
+  Warns when lambda specify a capture default and capture ``this``. The check 
also
+  offers fix-its.

This text should also be at the beginning of the check class documentation (in 
the Check.h, line 18)



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-this-with-capture-default.rst:9
+
+Capture-deafults in member functions can be misleading about
+whether data members are captured by value or reference. For example,

defaults



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };

ccotter wrote:
> carlosgalvezp wrote:
> > ccotter wrote:
> > > ccotter wrote:
> > > > ccotter wrote:
> > > > > ccotter wrote:
> > > > > > carlosgalvezp wrote:
> > > > > > > carlosgalvezp wrote:
> > > > > > > > carlosgalvezp wrote:
> > > > > > > > > "default capture"?
> > > > > > > > I find the check name a bit unintuitive. If you are up for a 
> > > > > > > > rename (you can use `rename_check.py`), I would consider 
> > > > > > > > renaming to something like 
> > > > > > > > `cppcoreguidelines-avoid-default-capture-when-capturing-this`
> > > > > > > > 
> > > > > > > > Like, what should be avoided is not "capturing this", it's 
> > > > > > > > using a default capture.
> > > > > > > > 
> > > > > > > > Would be good to get other reviewers opinion before spending 
> > > > > > > > time on renaming.
> > > > > > > Maybe put it within quotes so clarify it's a C++ keyword? Either 
> > > > > > > backticks `this` or single quotes 'this' would work I think, 
> > > > > > > unless we have some other convention.
> > > > > > I updated all references to capture default to say "capture 
> > > > > > default" as this is how it is spelled in the standard. The 
> > > > > > CppCoreGuideline for F.54 does use "default capture" though, so 
> > > > > > I'll open an issue seeing if that wording should instead say 
> > > > > > "capture default." Also for reference, `git grep` within the 
> > > > > > llvm-project repo shows
> > > > > > 
> > > > > > ```
> > > > > > $ git grep -i 'capture default' | wc -l
> > > > > >   43
> > > > > > $ git grep -i 'default capture' | wc -l
> > > > > >   54
> > > > > > $ git grep -i 'capture.default' | wc -l # E.g., 'capture-default' 
> > > > > > or 'capture default'
> > > > > >  105
> > > > > > ```
> > > > > Updated with a single quote. I didn't find any clang-tidy diagnostics 
> > > > > emitting backticks, but saw usage of single quotes when referring to 
> > > > > identifiers like variable or class names.
> > > > +1 - I admit, I struggled naming this check. More feedback welcome on 
> > > > the name
> > > https://github.com/isocpp/CppCoreGuidelines/pull/2016 for "capture 
> > > default" vs "default capture"
> > Interesting, thanks for investigating this! cppreference also uses that 
> > terminology. 
> > It sounds quite strange to me since that's not how humans read english text 
> > (which is what the diagnostic is meant for). We use "default constructor", 
> > "default arguments", etc. 
> > 
> > Anyhow this is a minor detail that shouldn't block the patch. Thanks for 
> > opening the pull requet towards CppCoreGuidelines!
> Since this is a more minor detail, we could probably follow up on the 
> "capture default" wording on the CppCoreGuidelines issue I opened. I'll add 
> that I *just* changed the lone occurrence of "default capture" in 
> https://en.cppreference.com/mwiki/index.php?title=cpp%2Flanguage%2Flambda&diff=146169&oldid=145543,
>  since the other 15 references were phrased "capture default" or 
> "capture-default."
Another suggestion for the check name: 

"cppcoreguidelines-avoid-capture-default-in-member-functions"




Repository:
  rG LLVM Github Monorepo

CHANGES SINCE LAST ACTION
  https://reviews.llvm

[PATCH] D140059: [APSInt] Fix bug in APSInt mentioned in https://github.com/llvm/llvm-project/issues/59515

[Peter Rong via Phabricator via cfe-commits]

Peter marked an inline comment as done.
Peter added inline comments.



Comment at: clang/lib/CodeGen/CGDebugInfo.cpp:5386
+  if (InitInt)
+InitExpr = DBuilder.createConstantValueExpression(InitInt.value());
+} else if (Init.isFloat())

efriedma wrote:
> I think we actually want the existing behavior here.  Values embedded in 
> debug info aren't really signed or unsigned; they're interpreted by the 
> debugger based on the type of the value.
> 
> Maybe it makes sense to add a new APSInt API for that?  Or I guess we could 
> explicitly write out `Init.getInt().isSigned() ? Init.getInt().getSExtValue() 
> : Init.getInt().getZExtValue();` if we can't think of a reasonable name for 
> the new API...
I have no problem with this proposed new API, we may even name it `uint64_t 
tryInt64Representaiton`. My main concern is that returning a value whose static 
type is unsigned while semantics can be signed OR unsigned is a bit inconsist.

I will use something like `Init.getInt().isSigned() ? 
Init.getInt().getSExtValue() : Init.getInt().getZExtValue();` for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140059

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

[PATCH] D140059: [APSInt] Fix bug in APSInt mentioned in https://github.com/llvm/llvm-project/issues/59515

[Peter Rong via Phabricator via cfe-commits]

Peter updated this revision to Diff 487118.
Peter added a comment.

[DebugInfo] update clang to match semantics changes in APSInt


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140059

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  llvm/include/llvm/ADT/APSInt.h
  llvm/lib/CodeGen/MIRParser/MIParser.cpp
  llvm/unittests/ADT/APSIntTest.cpp

Index: llvm/unittests/ADT/APSIntTest.cpp
===
--- llvm/unittests/ADT/APSIntTest.cpp
+++ llvm/unittests/ADT/APSIntTest.cpp
@@ -62,6 +62,13 @@
   EXPECT_EQ(UINT64_C(0) - 7, APSInt::getUnsigned(-7).getZExtValue());
 }
 
+TEST(APSIntTest, isRepresentableByInt64) {
+  ASSERT_TRUE(APSInt(APInt(3, 7), true).isRepresentableByInt64());
+  ASSERT_TRUE(APSInt(APInt(128, 7), true).isRepresentableByInt64());
+  ASSERT_TRUE(APSInt(APInt(128, 7), false).isRepresentableByInt64());
+  ASSERT_TRUE(APSInt(APInt(64, -1), false).isRepresentableByInt64());
+  ASSERT_FALSE(APSInt(APInt(64, (uint64_t)-1), true).isRepresentableByInt64());
+}
 TEST(APSIntTest, getExtValue) {
   EXPECT_TRUE(APSInt(APInt(3, 7), true).isUnsigned());
   EXPECT_TRUE(APSInt(APInt(3, 7), false).isSigned());
@@ -76,6 +83,16 @@
   EXPECT_EQ(9, APSInt(APInt(4, -7), true).getExtValue());
   EXPECT_EQ(-7, APSInt(APInt(4, -7), false).getExtValue());
 }
+TEST(APSIntTest, tryExtValue) {
+  ASSERT_EQ(-7, APSInt(APInt(64, -7), false).tryExtValue().value_or(42));
+  ASSERT_EQ(42, APSInt(APInt(128, -7), false).tryExtValue().value_or(42));
+  ASSERT_EQ(-1,
+APSInt(APInt::getAllOnes(128), false).tryExtValue().value_or(42));
+  ASSERT_EQ(42, APSInt(APInt(64, -7), true).tryExtValue().value_or(42));
+  ASSERT_EQ(1, APSInt(APInt(128, 1), true).tryExtValue().value_or(42));
+  ASSERT_EQ(42,
+APSInt(APInt::getAllOnes(128), true).tryExtValue().value_or(42));
+}
 
 TEST(APSIntTest, compareValues) {
   auto U = [](uint64_t V) { return APSInt::getUnsigned(V); };
Index: llvm/lib/CodeGen/MIRParser/MIParser.cpp
===
--- llvm/lib/CodeGen/MIRParser/MIParser.cpp
+++ llvm/lib/CodeGen/MIRParser/MIParser.cpp
@@ -1779,9 +1779,12 @@
 bool MIParser::parseImmediateOperand(MachineOperand &Dest) {
   assert(Token.is(MIToken::IntegerLiteral));
   const APSInt &Int = Token.integerValue();
-  if (Int.getMinSignedBits() > 64)
+  if (auto SImm = Int.trySExtValue(); Int.isSigned() && SImm.has_value())
+Dest = MachineOperand::CreateImm(*SImm);
+  else if (auto UImm = Int.tryZExtValue(); !Int.isSigned() && UImm.has_value())
+Dest = MachineOperand::CreateImm(*UImm);
+  else
 return error("integer literal is too large to be an immediate operand");
-  Dest = MachineOperand::CreateImm(Int.getExtValue());
   lex();
   return false;
 }
Index: llvm/include/llvm/ADT/APSInt.h
===
--- llvm/include/llvm/ADT/APSInt.h
+++ llvm/include/llvm/ADT/APSInt.h
@@ -85,12 +85,26 @@
   }
   using APInt::toString;
 
+  /// If this int is representable using an int64_t.
+  bool isRepresentableByInt64() const {
+// For unsigned values with 64 active bits, they technically fit into a
+// int64_t, but the user may get negative numbers and has to manually cast
+// them to unsigned. Let's not bet the user has the sanity to do that and
+// not give them a vague value at the first place.
+return isSigned() ? isSignedIntN(64) : isIntN(63);
+  }
+
   /// Get the correctly-extended \c int64_t value.
   int64_t getExtValue() const {
-assert(getMinSignedBits() <= 64 && "Too many bits for int64_t");
+assert(isRepresentableByInt64() && "Too many bits for int64_t");
 return isSigned() ? getSExtValue() : getZExtValue();
   }
 
+  std::optional tryExtValue() const {
+return isRepresentableByInt64() ? std::optional(getExtValue())
+: std::nullopt;
+  }
+
   APSInt trunc(uint32_t width) const {
 return APSInt(APInt::trunc(width), IsUnsigned);
   }
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -5401,10 +5401,18 @@
   llvm::DIExpression *InitExpr = nullptr;
   if (CGM.getContext().getTypeSize(VD->getType()) <= 64) {
 // FIXME: Add a representation for integer constants wider than 64 bits.
-if (Init.isInt())
-  InitExpr =
-  DBuilder.createConstantValueExpression(Init.getInt().getExtValue());
-else if (Init.isFloat())
+if (Init.isInt()) {
+  llvm::APSInt InitInt = Init.getInt();
+  std::optional InitIntOpt;
+  if (InitInt.isUnsigned())
+InitIntOpt = InitInt.tryZExtValue();
+  else if (auto tmp = InitInt.trySExtValue(); tmp.has_value())
+// Transform a signed optional to unsigned optional. When cpp 23 comes,
+// use std::optional:

[clang] 6fe70cb - clang/AMDGPU: Force disable block enqueue arguments for HIP

[Matt Arsenault via cfe-commits]

Author: Matt Arsenault
Date: 2023-01-07T13:39:05-05:00
New Revision: 6fe70cb465654eafafd272231e23762adeab4290

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

LOG: clang/AMDGPU: Force disable block enqueue arguments for HIP

This is a dirty, dirty hack to workaround bot failures at
-O0. Currently these fields are only used by OpenCL features and
evidently the HIP runtime isn't expecting to see them in HIP
programs. The code objects should be language agnostic, so just force
optimize these out until the runtime is fixed.

Added: 
clang/test/CodeGenHIP/default-attributes.hip

Modified: 
clang/lib/CodeGen/TargetInfo.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/TargetInfo.cpp 
b/clang/lib/CodeGen/TargetInfo.cpp
index aec170ae5570..ee8852903eda 100644
--- a/clang/lib/CodeGen/TargetInfo.cpp
+++ b/clang/lib/CodeGen/TargetInfo.cpp
@@ -9520,6 +9520,15 @@ void AMDGPUTargetCodeGenInfo::setFunctionDeclAttributes(
 if (NumVGPR != 0)
   F->addFnAttr("amdgpu-num-vgpr", llvm::utostr(NumVGPR));
   }
+
+  if (IsHIPKernel) {
+// FIXME: This is a dirty, dirty hack to fix bot failures at -O0 and should
+// be removed. The HIP runtime currently fails to handle the case where one
+// of these fields fails to optimize out. The runtime should tolerate all
+// requested implicit inputs regardless of language.
+F->addFnAttr("amdgpu-no-default-queue");
+F->addFnAttr("amdgpu-no-completion-action");
+  }
 }
 
 void AMDGPUTargetCodeGenInfo::setTargetAttributes(

diff  --git a/clang/test/CodeGenHIP/default-attributes.hip 
b/clang/test/CodeGenHIP/default-attributes.hip
new file mode 100644
index ..b4f4a6201956
--- /dev/null
+++ b/clang/test/CodeGenHIP/default-attributes.hip
@@ -0,0 +1,47 @@
+// NOTE: Assertions have been autogenerated by utils/update_cc_test_checks.py 
UTC_ARGS: --function-signature --check-attributes --check-globals
+// RUN: %clang_cc1 -triple amdgcn-amd-amdhsa -x hip -fcuda-is-device \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=OPTNONE %s
+
+// RUN: %clang_cc1 -O3 -triple amdgcn-amd-amdhsa -x hip -fcuda-is-device \
+// RUN:-emit-llvm -o - %s | FileCheck -check-prefix=OPT %s
+
+#define __device__ __attribute__((device))
+#define __global__ __attribute__((global))
+
+// OPTNONE: Function Attrs: convergent mustprogress noinline nounwind optnone
+// OPTNONE-LABEL: define {{[^@]+}}@_Z4funcv
+// OPTNONE-SAME: () #[[ATTR0:[0-9]+]] {
+// OPTNONE-NEXT:  entry:
+// OPTNONE-NEXT:ret void
+//
+// OPT: Function Attrs: mustprogress nofree norecurse nosync nounwind 
willreturn memory(none)
+// OPT-LABEL: define {{[^@]+}}@_Z4funcv
+// OPT-SAME: () local_unnamed_addr #[[ATTR0:[0-9]+]] {
+// OPT-NEXT:  entry:
+// OPT-NEXT:ret void
+//
+__device__ void func() {
+
+}
+
+// OPTNONE: Function Attrs: convergent mustprogress noinline norecurse 
nounwind optnone
+// OPTNONE-LABEL: define {{[^@]+}}@_Z6kernelv
+// OPTNONE-SAME: () #[[ATTR1:[0-9]+]] {
+// OPTNONE-NEXT:  entry:
+// OPTNONE-NEXT:ret void
+//
+// OPT: Function Attrs: mustprogress nofree norecurse nosync nounwind 
willreturn memory(none)
+// OPT-LABEL: define {{[^@]+}}@_Z6kernelv
+// OPT-SAME: () local_unnamed_addr #[[ATTR1:[0-9]+]] {
+// OPT-NEXT:  entry:
+// OPT-NEXT:ret void
+//
+__global__ void kernel() {
+
+}
+//.
+// OPTNONE: attributes #0 = { convergent mustprogress noinline nounwind 
optnone "no-trapping-math"="true" "stack-protector-buffer-size"="8" }
+// OPTNONE: attributes #1 = { convergent mustprogress noinline norecurse 
nounwind optnone "amdgpu-flat-work-group-size"="1,1024" 
"amdgpu-no-completion-action" "amdgpu-no-default-queue" 
"no-trapping-math"="true" "stack-protector-buffer-size"="8" 
"uniform-work-group-size"="true" }
+//.
+// OPT: attributes #0 = { mustprogress nofree norecurse nosync nounwind 
willreturn memory(none) "no-trapping-math"="true" 
"stack-protector-buffer-size"="8" }
+// OPT: attributes #1 = { mustprogress nofree norecurse nosync nounwind 
willreturn memory(none) "amdgpu-flat-work-group-size"="1,1024" 
"amdgpu-no-completion-action" "amdgpu-no-default-queue" 
"no-trapping-math"="true" "stack-protector-buffer-size"="8" 
"uniform-work-group-size"="true" }



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

[PATCH] D141192: [Clang] Add warnings on bad shifts inside enums.

[Dmitriy Chestnykh via Phabricator via cfe-commits]

chestnykh added a comment.

In D141192#4033591 , @shafik wrote:

> So it looks like in `handleIntIntBinOp` we do hit this code:
>
>   unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
>if (SA != RHS) {
>  Info.CCEDiag(E, diag::note_constexpr_large_shift)
><< RHS << E->getType() << LHS.getBitWidth();
>
> So maybe we should figure out why we decide not to emit this diagnostic and 
> fix it there.

In the comment above: "
// C++11 [expr.shift]p1: Shift width must be less than the bit width of
// the shifted type."
Maybe only since C++11 there is the restriction to shift width? Don't you know 
the standard about it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141192

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

[PATCH] D141192: [Clang] Add warnings on bad shifts inside enums.

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

So it looks like in `handleIntIntBinOp` we do hit this code:

  unsigned SA = (unsigned) RHS.getLimitedValue(LHS.getBitWidth()-1);
   if (SA != RHS) {
 Info.CCEDiag(E, diag::note_constexpr_large_shift)
   << RHS << E->getType() << LHS.getBitWidth();

So maybe we should figure out why we decide not to emit this diagnostic and fix 
it there.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141192

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

[PATCH] D141177: [Clang] Don't tell people to place _Alignas on a struct in diagnostics

[Theodore Luo Wang via Phabricator via cfe-commits]

theo-lw updated this revision to Diff 487103.
theo-lw added a comment.

Accidentally deleted a line causing this to not compile :(


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

https://reviews.llvm.org/D141177

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Parser/c1x-alignas.c


Index: clang/test/Parser/c1x-alignas.c
===
--- clang/test/Parser/c1x-alignas.c
+++ clang/test/Parser/c1x-alignas.c
@@ -9,5 +9,7 @@
 
 char _Alignas(_Alignof(int)) c5;
 
+_Alignas(int) struct c6; // expected-warning {{1 attribute ignored}}
+
 // CHECK-EXT: '_Alignas' is a C11 extension
 // CHECK-EXT: '_Alignof' is a C11 extension
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5292,17 +5292,29 @@
   // Attributes should be placed after tag to apply to type declaration.
   if (!DS.getAttributes().empty() || !DeclAttrs.empty()) {
 DeclSpec::TST TypeSpecType = DS.getTypeSpecType();
+auto WarnAttributeIgnored = [this, TypeSpecType](const ParsedAttr &AL) {
+  if (AL.isAlignasAttribute()) {
+// Don't use the message with placement with _Alignas.
+// This is because C doesnt let you use _Alignas on type declarations.
+Diag(AL.getLoc(), diag::warn_attribute_ignored)
+<< GetDiagnosticTypeSpecifierID(TypeSpecType);
+  } else {
+Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
+<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  }
+};
+
 if (TypeSpecType == DeclSpec::TST_class ||
 TypeSpecType == DeclSpec::TST_struct ||
 TypeSpecType == DeclSpec::TST_interface ||
 TypeSpecType == DeclSpec::TST_union ||
 TypeSpecType == DeclSpec::TST_enum) {
-  for (const ParsedAttr &AL : DS.getAttributes())
-Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-  for (const ParsedAttr &AL : DeclAttrs)
-Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  for (const ParsedAttr &AL : DS.getAttributes()) {
+WarnAttributeIgnored(AL);
+  }
+  for (const ParsedAttr &AL : DeclAttrs) {
+WarnAttributeIgnored(AL);
+  }
 }
   }
 


Index: clang/test/Parser/c1x-alignas.c
===
--- clang/test/Parser/c1x-alignas.c
+++ clang/test/Parser/c1x-alignas.c
@@ -9,5 +9,7 @@
 
 char _Alignas(_Alignof(int)) c5;
 
+_Alignas(int) struct c6; // expected-warning {{1 attribute ignored}}
+
 // CHECK-EXT: '_Alignas' is a C11 extension
 // CHECK-EXT: '_Alignof' is a C11 extension
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -5292,17 +5292,29 @@
   // Attributes should be placed after tag to apply to type declaration.
   if (!DS.getAttributes().empty() || !DeclAttrs.empty()) {
 DeclSpec::TST TypeSpecType = DS.getTypeSpecType();
+auto WarnAttributeIgnored = [this, TypeSpecType](const ParsedAttr &AL) {
+  if (AL.isAlignasAttribute()) {
+// Don't use the message with placement with _Alignas.
+// This is because C doesnt let you use _Alignas on type declarations.
+Diag(AL.getLoc(), diag::warn_attribute_ignored)
+<< GetDiagnosticTypeSpecifierID(TypeSpecType);
+  } else {
+Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
+<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  }
+};
+
 if (TypeSpecType == DeclSpec::TST_class ||
 TypeSpecType == DeclSpec::TST_struct ||
 TypeSpecType == DeclSpec::TST_interface ||
 TypeSpecType == DeclSpec::TST_union ||
 TypeSpecType == DeclSpec::TST_enum) {
-  for (const ParsedAttr &AL : DS.getAttributes())
-Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
-  for (const ParsedAttr &AL : DeclAttrs)
-Diag(AL.getLoc(), diag::warn_declspec_attribute_ignored)
-<< AL << GetDiagnosticTypeSpecifierID(TypeSpecType);
+  for (const ParsedAttr &AL : DS.getAttributes()) {
+WarnAttributeIgnored(AL);
+  }
+  for (const ParsedAttr &AL : DeclAttrs) {
+WarnAttributeIgnored(AL);
+  }
 }
   }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b712aef - [llvm-driver] Mark some tests unsupported

[Alex Brachet via cfe-commits]

Author: Alex Brachet
Date: 2023-01-07T17:45:26Z
New Revision: b712aef5b37e4e98fcc7bd1a6cfc3bac2d7af0d0

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

LOG: [llvm-driver] Mark some tests unsupported

These tests rely on making symlinks to unkown tool names which will
fail when in the llvm-driver build.

Added: 


Modified: 
clang/test/CMakeLists.txt
clang/test/Driver/parse-progname.c
clang/test/lit.cfg.py
clang/test/lit.site.cfg.py.in
llvm/test/tools/llvm-objcopy/tool-name.test

Removed: 




diff  --git a/clang/test/CMakeLists.txt b/clang/test/CMakeLists.txt
index b963a7542589..1d6377b5f2d8 100644
--- a/clang/test/CMakeLists.txt
+++ b/clang/test/CMakeLists.txt
@@ -16,6 +16,7 @@ llvm_canonicalize_cmake_booleans(
   LLVM_ENABLE_THREADS
   LLVM_WITH_Z3
   PPC_LINUX_DEFAULT_IEEELONGDOUBLE
+  LLVM_TOOL_LLVM_DRIVER_BUILD
   )
 
 configure_lit_site_cfg(

diff  --git a/clang/test/Driver/parse-progname.c 
b/clang/test/Driver/parse-progname.c
index ed907ea12e00..34040b81dc73 100644
--- a/clang/test/Driver/parse-progname.c
+++ b/clang/test/Driver/parse-progname.c
@@ -1,4 +1,5 @@
 // REQUIRES: shell, arm-registered-target
+// UNSUPPORTED: llvm-driver
 
 // RUN: mkdir -p %t
 

diff  --git a/clang/test/lit.cfg.py b/clang/test/lit.cfg.py
index 8088ceff5c00..cc55c3c44a41 100644
--- a/clang/test/lit.cfg.py
+++ b/clang/test/lit.cfg.py
@@ -238,6 +238,9 @@ def calculate_arch_features(arch_string):
 if config.clang_vendor_uti:
 config.available_features.add('clang-vendor=' + config.clang_vendor_uti)
 
+if config.have_llvm_driver:
+  config.available_features.add('llvm-driver')
+
 def exclude_unsupported_files_for_aix(dirname):
 for filename in os.listdir(dirname):
 source_path = os.path.join( dirname, filename)

diff  --git a/clang/test/lit.site.cfg.py.in b/clang/test/lit.site.cfg.py.in
index b9ddd0392c2f..89fedd47b008 100644
--- a/clang/test/lit.site.cfg.py.in
+++ b/clang/test/lit.site.cfg.py.in
@@ -39,6 +39,7 @@ config.clang_vendor_uti = "@CLANG_VENDOR_UTI@"
 config.llvm_external_lit = path(r"@LLVM_EXTERNAL_LIT@")
 config.standalone_build = @CLANG_BUILT_STANDALONE@
 config.ppc_linux_default_ieeelongdouble = @PPC_LINUX_DEFAULT_IEEELONGDOUBLE@
+config.have_llvm_driver = @LLVM_TOOL_LLVM_DRIVER_BUILD@
 
 import lit.llvm
 lit.llvm.initialize(lit_config, config)

diff  --git a/llvm/test/tools/llvm-objcopy/tool-name.test 
b/llvm/test/tools/llvm-objcopy/tool-name.test
index 4364d083061a..6a1f72588e23 100644
--- a/llvm/test/tools/llvm-objcopy/tool-name.test
+++ b/llvm/test/tools/llvm-objcopy/tool-name.test
@@ -1,5 +1,5 @@
 ## Don't make symlinks on Windows.
-# UNSUPPORTED: system-windows
+# UNSUPPORTED: system-windows, llvm-driver
 
 # RUN: rm -rf %t
 # RUN: mkdir %t



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

[PATCH] D140925: [CMake] Use Clang to infer the target triple

[Tamás Szelei via Phabricator via cfe-commits]

tamas added inline comments.



Comment at: runtimes/CMakeLists.txt:168
+if(LLVM_ENABLE_PER_TARGET_RUNTIME_DIR AND NOT APPLE)
+  set(CXX_TARGET_TRIPLE ${CMAKE_CXX_COMPILER} --target=${LLVM_RUNTIME_TRIPLE} 
-print-target-triple)
+  execute_process(COMMAND ${CXX_TARGET_TRIPLE}

phosek wrote:
> tamas wrote:
> > I think there is one downside to this: the `normalize` function will not, 
> > in fact, normalize alternative spellings for equivalent architectures (and 
> > likely other components, I haven't checked that):
> > ```
> > + clang -print-target-triple -target amd64-unknown-linux-musl
> > amd64-unknown-linux-musl
> > + clang -print-target-triple -target x86_64-unknown-linux-musl
> > x86_64-unknown-linux-musl
> > ```
> > So the issue where the distribution build can install runtimes in paths 
> > that won't be found by the driver still remains 
> > (https://github.com/llvm/llvm-project/issues/57781). One way to fix this 
> > could be adding a new switch that does a more opinionated normalization 
> > (for example, always picks `x86_64` in the above). Sort of a reverse for 
> > `Triple::parseArch` etc. where it always returns one certain spelling when 
> > there are multiple common alternatives. Alternatively, `normalize` could be 
> > changed to do this, but that might subtle breaking consequences for users 
> > which are hard to foresee.
> I think this is an orthogonal issue to the one that this change is trying to 
> address. Clang currently uses normalized triples to for its include 
> directories (libc++ headers) and runtime libraries. This change ensures that 
> the triple spelling used by CMake and Clang matches, but it doesn't change 
> the normalization logic.
> 
> From Clang's perspective, `amd64-unknown-linux-musl` and 
> `x86_64-unknown-linux-musl` are two different triples and so it'd use 
> different search directories. We could consider changing the normalization 
> rules to normalize these to the same triple, but that should be done 
> separately (and would likely require further discussion to understand the 
> potential consequences of such a change).
I don't mean to raise this as a potential blocker and this is certainly 
somewhat orthogonal. However, I think this not true: 

> From Clang's perspective, amd64-unknown-linux-musl and 
> x86_64-unknown-linux-musl are two different triples

`amd64` and `x86_64` are very explictly parsed as the same: 
https://github.com/llvm/llvm-project/blob/6dc85bd3fde7df2999fda07e9e9f2e83d52c6125/llvm/lib/TargetParser/Triple.cpp#L454

I'm not sure if it's a good idea to change the current normalization logic, 
hence my suggestion for a new flag. But this is probably a discussion for 
discourse/discord.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140925

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

[PATCH] D127812: [AArch64] FMV support and necessary target features dependencies.

[Pavel Iliin via Phabricator via cfe-commits]

ilinpv added a comment.

In D127812#4031994 , @smeenai wrote:

> You're right that it conceptually makes sense for this to be in `cpu_model.c` 
> though. An alternative would be providing an option for compiler-rt to be 
> built without the multiversioning support, e.g. if it's built with `-mno-fmv` 
> itself. Does that seem reasonable?

Sounds good to me, please look into the patch adding such cmake option 
https://reviews.llvm.org/D141199
Using cmake `-DCOMPILER_RT_DISABLE_AARCH64_FMV=On` will exclude Aarch64 
multiversioning support from compiler-rt build.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D127812

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

[PATCH] D140795: [Flang] Add user option -funderscoring/-fnounderscoring to enable/disable ExternalNameConversionPass

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

In D140795#4031392 , @kkwli0 wrote:

> The purpose of this option is to control the trailing underscore being 
> appended to external names (e.g. procedure names, common block names). The 
> option in gfortran is documented in 
> https://gcc.gnu.org/onlinedocs/gfortran/Code-Gen-Options.html.

Thanks for this explanation - much appreciated! Could this short description be 
added to the summary? Also,  could you add a note stating whether the semantics 
of this option in Flang are consistent with GFortran. Ta!

> However, I don't think the patch does what we want. Given a procedure name 
> `foo`, the `-fno-underscoring` option will give `_QPfoo` instead of `foo`. We 
> will look into it.

Names in Flang are mangled at the FIR level. I couldn't find any documentation 
for this, but the ExternalNameConversion 

 pass could be helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D140795

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

[PATCH] D141198: [Clang][RISCV][NFC] Reorganize test case for rvv intrinsics

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

eopXD added a comment.

I have created a branch under my forked repo. for people to checkout this 
commit conveniently.

https://github.com/eopXD/llvm-project/tree/reorganize-testcase


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141198

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

[PATCH] D141198: [Clang][RISCV][NFC] Reorganize test case for rvv intrinsics

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

eopXD added a comment.

The patch application failure probably is due to the large amount of changes in 
this patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141198

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

[PATCH] D140860: [Diagnostics][NFC] Fix -Wlogical-op-parentheses warning inconsistency for const and constexpr values

[Takuya Shimizu via Phabricator via cfe-commits]

hazohelet added a comment.

I have yet to do thorough checks using this patched clang to build significant 
code bases.
It will likely take quite a bit of time as I am not used to editing build tool 
files.

Instead, I used `grep` to find potentially newly-warned codes.
The `grep` command is shown below. It is intended to match C/C++ codes with 
both `&&` and `||` within which 0, 1, or 2 matching parentheses exist in a 
single line.
`grep -r --include=\*.cpp --include=\*.cc  --include=\*.c -e "&&[^()]*||" -e 
"||[^()]*&&" -e "&&[^()]*([^()]*)[^()]*||" -e "||[^()]*([^()]*)[^()]*&&" -e 
"&&[^()]*([^()]*([^()]*)[^()]*)[^()]*||" -e 
"||[^()]*([^()]*([^()]*)[^()]*)[^()]*&&"`

I applied this `grep` command to the following popular GitHub repositories 
`NVIDIA/TensorRT`, `bulletphysics/bullet3`, `apple/foundationdb`, `grpc/grpc`, 
`microsoft/lightgbm`, `rui314/mold`, `oneapi-src/oneTBB`, 
`SerenityOS/serenity`, `tensorflow/tensorflow`.
I found the 7 examples of missing parentheses at `&&` within `||` in the 
matched strings (many of the matchings exist in preprocessor conditionals, 
which is out of the scope of this patch)
They are listed at the end of this comment.
The last case in tensorflow is an `assert(a || b && "reason for the assert")` 
known idiom and is handled correctly.
Because the newly-warned constants are compile-time constants not dependent on 
function arguments, the other 6 cases will also get warnings from the clang 
before this patch.

It suggests that this patch does not generate extensive new warnings in 
existent codebases.

oneTBB:
https://github.com/oneapi-src/oneTBB/blob/e6e493f96ec8b7e2e2b4d048ed49356eb54ec2a0/examples/common/gui/xvideo.cpp#L359
https://github.com/oneapi-src/oneTBB/blob/e6e493f96ec8b7e2e2b4d048ed49356eb54ec2a0/src/tbbmalloc/frontend.cpp#L1266
tensorflow:
https://github.com/tensorflow/tensorflow/blob/fb2c3b1e3140af73f949981d8428379cbb28228b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc#L4130
https://github.com/tensorflow/tensorflow/blob/fb2c3b1e3140af73f949981d8428379cbb28228b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc#L4074
https://github.com/tensorflow/tensorflow/blob/fb2c3b1e3140af73f949981d8428379cbb28228b/tensorflow/compiler/tf2tensorrt/convert/convert_nodes_test.cc#L9456
https://github.com/tensorflow/tensorflow/blob/fb2c3b1e3140af73f949981d8428379cbb28228b/tensorflow/compiler/jit/deadness_analysis.cc#L1423
https://github.com/tensorflow/tensorflow/blob/fb2c3b1e3140af73f949981d8428379cbb28228b/tensorflow/core/ir/tf_op_wrapper.cc#L25


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

https://reviews.llvm.org/D140860

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

[PATCH] D141198: [Clang][RISCV][NFC] Reorganize test case for rvv intrinsics

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

eopXD added a comment.

The testing time difference is shown below. I personally think 3 more minutes 
in testing is tolerable when considering `check-all`. The testing time increase 
comes from full test coverage to the intrinsics. Before this patch, some test 
cases only includes parts of the {SEW, LMUL} combination of an intrinsic to 
reduce avoid long compile time. We no longer suffer from long compile time as 
the intrinsics are now lazily added.

Before

  $ bin/llvm-lit ../clang/test/CodeGen/RISCV/rvv-intrinsics-overloaded 
../clang/test/CodeGen/RISCV/rvv-intrinsics 
../clang/test/CodeGen/RISCV/rvv-intrinsics-handcrafted
  llvm-lit: 
/scratch/eopc/upstream-llvm-project/llvm/utils/lit/lit/llvm/config.py:459: 
note: using clang: /scratch/eopc/upstream-llvm-project/build/bin/clang
  -- Testing: 326 tests, 48 workers --
  Testing Time: 25.83s
Passed: 326

After

  $ bin/llvm-lit ../clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated
  llvm-lit: 
/scratch/eopc/upstream-llvm-project/llvm/utils/lit/lit/llvm/config.py:459: 
note: using clang: /scratch/eopc/upstream-llvm-project/build/bin/clang
  -- Testing: 566 tests, 48 workers --
  Testing Time: 225.61s
Passed: 566


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141198

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

[PATCH] D141198: [Clang][RISCV][NFC] Reorganize test case for rvv intrinsics

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

eopXD updated this revision to Diff 487094.
eopXD added a comment.

Rebase to latest main.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141198

Files:
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vadc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vand.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vcompress.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vcpop.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfabs.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfclass.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfcvt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfirst.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmax.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmerge.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmin.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmul.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfncvt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfneg.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrec7.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredmax.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredmin.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredosum.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredusum.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrsqrt7.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfsgnj.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfslide1down.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfslide1up.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfsqrt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwcvt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwmul.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwnmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwnmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwredosum.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwredusum.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-a

[PATCH] D141198: [NFC][Clang][RISCV] Reorganize test case for rvv intrinsics

[Yueh-Ting (eop) Chen via Phabricator via cfe-commits]

eopXD created this revision.
eopXD added reviewers: craig.topper, kito-cheng, frasercrmck, reames, jrtc27.
Herald added subscribers: sunshaoce, VincentWu, StephenFan, vkmr, evandro, 
luismarques, apazos, sameer.abuasal, s.egerton, Jim, benna, psnobl, jocewei, 
PkmX, arphaman, the_o, brucehoult, MartinMosbeck, rogfer01, edward-jones, 
zzheng, shiva0217, niosHD, sabuasal, simoncook, johnrusso, rbar, asb, 
arichardson.
Herald added a project: All.
eopXD requested review of this revision.
Herald added subscribers: cfe-commits, pcwang-thead, MaskRay.
Herald added a project: clang.

Separating auto-generated test cases and hand-craft ones. The
auto-generated ones are basic API tests generated from the intrinsic
generator [0].

This re-organization allows direct copy-and-paste from the produced
outputs of the generator in future API changes, which is discussed
and needs to be implemented towards a v1.0 RVV intrinsic.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141198

Files:
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vaadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vadc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vand.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vasub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vcompress.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vcpop.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfabs.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfclass.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfcvt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfirst.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmax.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmerge.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmin.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmul.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfmv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfncvt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfneg.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmacc.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmadd.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmsac.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfnmsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrdiv.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrec7.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredmax.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredmin.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredosum.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfredusum.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrsqrt7.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfrsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfsgnj.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfslide1down.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfslide1up.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfsqrt.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfsub.c
  
clang/test/CodeGen/RISCV/rvv-intrinsics-autogenerated/non-policy/non-overloaded/vfwadd.c
  
clang/test/CodeGen/RISCV/

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

[Vincent Hong via Phabricator via cfe-commits]

v1nh1shungry added a comment.

Thank you for reviewing and giving suggestions! @Febbe

Please take a look at my reply. Sorry if I misunderstood anything!




Comment at: 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:215
   (Twine("static_cast<") + TyAsString + ">(").str())
-   << FixItHint::CreateInsertion(IndexExpr->getEndLoc(), ")");
 else

Febbe wrote:
> Actually, I find it pretty weird/suspicious, that ` -> 
> getEndLoc()` does not return the end of its `Expr`. The code looked correct 
> already.
> Can you elaborate, if this is a bug in the `getEndLoc()` of those `Expr`s? It 
> might be better to fix it there directly.
> 
I'm really not an expert on `SourceLocation`. I just took a rough look at these 
`getEndLoc()`s.

Take `1 + 2` as an example, this is a `BinaryOperator` with two 
`IntegerLiteral`s. When we call `Expr::getEndLoc()` which is actually 
`Stmt::getEndLoc()` on it, it will call `BinaryOperator::getEndLoc()` and this 
actually returns the `RHS->getEndLoc()`, that is  the result of 
`IntegerLiteral(2)->getEndLoc()`. And the interesting stuff is that 
`IntegerLiteral::getBeginLoc()` and `IntegerLiteral::getEndLoc()` return the 
same `Loc`. Although I'm not sure which location these will return, I guess 
that's why `BinaryOperator::getEndLoc()` doesn't return the end of the 
expression.

Sorry if I misunderstood anything! After all, I didn't find documents saying 
which location these `Expr`s' `getEndLoc()` should return. I think it's better 
to have experts take a look at this.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone/implicit-widening-of-multiplication-result-array-subscript-expression.cpp:18
   // CHECK-NOTES-C:(ptrdiff_t)( )
   // CHECK-NOTES-CXX:  static_cast( )
   // CHECK-NOTES-ALL: :[[@LINE-5]]:16: note: perform multiplication in a wider 
type

Actually I have tried adding

```
// CHECK-FIXES-C: return &base[(ptrdiff_t)(a * b)];
// CHECK-FIXES-CXX: return &base[static_cast(a * b)];
```

under this line, but the test failed, and when I took a look at 
`build/tools/clang/tools/extra/test/clang-tidy/checkers/bugprone/Output/implicit-widening-of-multiplication-result-array-subscript-expression.cpp.tmp.cpp`,
 I found that these codes didn't get modified. And I took a look at other files 
which have `CHECK-FIXES` lines, I found the codes in the corresponding 
temporary files got fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Chris Cotter via Phabricator via cfe-commits]

ccotter added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };

carlosgalvezp wrote:
> ccotter wrote:
> > ccotter wrote:
> > > ccotter wrote:
> > > > ccotter wrote:
> > > > > carlosgalvezp wrote:
> > > > > > carlosgalvezp wrote:
> > > > > > > carlosgalvezp wrote:
> > > > > > > > "default capture"?
> > > > > > > I find the check name a bit unintuitive. If you are up for a 
> > > > > > > rename (you can use `rename_check.py`), I would consider renaming 
> > > > > > > to something like 
> > > > > > > `cppcoreguidelines-avoid-default-capture-when-capturing-this`
> > > > > > > 
> > > > > > > Like, what should be avoided is not "capturing this", it's using 
> > > > > > > a default capture.
> > > > > > > 
> > > > > > > Would be good to get other reviewers opinion before spending time 
> > > > > > > on renaming.
> > > > > > Maybe put it within quotes so clarify it's a C++ keyword? Either 
> > > > > > backticks `this` or single quotes 'this' would work I think, unless 
> > > > > > we have some other convention.
> > > > > I updated all references to capture default to say "capture default" 
> > > > > as this is how it is spelled in the standard. The CppCoreGuideline 
> > > > > for F.54 does use "default capture" though, so I'll open an issue 
> > > > > seeing if that wording should instead say "capture default." Also for 
> > > > > reference, `git grep` within the llvm-project repo shows
> > > > > 
> > > > > ```
> > > > > $ git grep -i 'capture default' | wc -l
> > > > >   43
> > > > > $ git grep -i 'default capture' | wc -l
> > > > >   54
> > > > > $ git grep -i 'capture.default' | wc -l # E.g., 'capture-default' or 
> > > > > 'capture default'
> > > > >  105
> > > > > ```
> > > > Updated with a single quote. I didn't find any clang-tidy diagnostics 
> > > > emitting backticks, but saw usage of single quotes when referring to 
> > > > identifiers like variable or class names.
> > > +1 - I admit, I struggled naming this check. More feedback welcome on the 
> > > name
> > https://github.com/isocpp/CppCoreGuidelines/pull/2016 for "capture default" 
> > vs "default capture"
> Interesting, thanks for investigating this! cppreference also uses that 
> terminology. 
> It sounds quite strange to me since that's not how humans read english text 
> (which is what the diagnostic is meant for). We use "default constructor", 
> "default arguments", etc. 
> 
> Anyhow this is a minor detail that shouldn't block the patch. Thanks for 
> opening the pull requet towards CppCoreGuidelines!
Since this is a more minor detail, we could probably follow up on the "capture 
default" wording on the CppCoreGuidelines issue I opened. I'll add that I 
*just* changed the lone occurrence of "default capture" in 
https://en.cppreference.com/mwiki/index.php?title=cpp%2Flanguage%2Flambda&diff=146169&oldid=145543,
 since the other 15 references were phrased "capture default" or 
"capture-default."


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

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

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Carlos Galvez via Phabricator via cfe-commits]

carlosgalvezp added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };

ccotter wrote:
> ccotter wrote:
> > ccotter wrote:
> > > ccotter wrote:
> > > > carlosgalvezp wrote:
> > > > > carlosgalvezp wrote:
> > > > > > carlosgalvezp wrote:
> > > > > > > "default capture"?
> > > > > > I find the check name a bit unintuitive. If you are up for a rename 
> > > > > > (you can use `rename_check.py`), I would consider renaming to 
> > > > > > something like 
> > > > > > `cppcoreguidelines-avoid-default-capture-when-capturing-this`
> > > > > > 
> > > > > > Like, what should be avoided is not "capturing this", it's using a 
> > > > > > default capture.
> > > > > > 
> > > > > > Would be good to get other reviewers opinion before spending time 
> > > > > > on renaming.
> > > > > Maybe put it within quotes so clarify it's a C++ keyword? Either 
> > > > > backticks `this` or single quotes 'this' would work I think, unless 
> > > > > we have some other convention.
> > > > I updated all references to capture default to say "capture default" as 
> > > > this is how it is spelled in the standard. The CppCoreGuideline for 
> > > > F.54 does use "default capture" though, so I'll open an issue seeing if 
> > > > that wording should instead say "capture default." Also for reference, 
> > > > `git grep` within the llvm-project repo shows
> > > > 
> > > > ```
> > > > $ git grep -i 'capture default' | wc -l
> > > >   43
> > > > $ git grep -i 'default capture' | wc -l
> > > >   54
> > > > $ git grep -i 'capture.default' | wc -l # E.g., 'capture-default' or 
> > > > 'capture default'
> > > >  105
> > > > ```
> > > Updated with a single quote. I didn't find any clang-tidy diagnostics 
> > > emitting backticks, but saw usage of single quotes when referring to 
> > > identifiers like variable or class names.
> > +1 - I admit, I struggled naming this check. More feedback welcome on the 
> > name
> https://github.com/isocpp/CppCoreGuidelines/pull/2016 for "capture default" 
> vs "default capture"
Interesting, thanks for investigating this! cppreference also uses that 
terminology. 
It sounds quite strange to me since that's not how humans read english text 
(which is what the diagnostic is meant for). We use "default constructor", 
"default arguments", etc. 

Anyhow this is a minor detail that shouldn't block the patch. Thanks for 
opening the pull requet towards CppCoreGuidelines!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

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

[PATCH] D141058: [clang-tidy] fix wrong fixup for bugprone-implicit-widening-of-multiplication-result

[Fabian Keßler via Phabricator via cfe-commits]

Febbe added a comment.

In D141058#4028768 , @v1nh1shungry 
wrote:

> Sorry, I don't know how to add tests for such fixes. Could anyone please give 
> me some hints? Thanks!

It seems like the tests for this check do not contain checks for the fixups 
itself.

Normally, they are tested with `CHECK-FIXES: ` for example:

  return F? Mov: Movable{}; 
  // CHECK-MESSAGES: [[@LINE-1]]:13: warning: Parameter 'Mov' is copied on last 
use, consider moving it instead. [performance-unnecessary-copy-on-last-use] 
  // CHECK-FIXES: return F? std::move(Mov): Movable{};

In the case of this test file, you have to append the suffixes to the 
`CHECK-FIXES` to `CHECK-FIXES-` where `` is one of `C`, `CXX`, 
`ALL`

  return F? Mov: Movable{}; 
  // CHECK-MESSAGES-CXX: [[@LINE-1]]:13: warning: Parameter 'Mov' is copied on 
last use, consider moving it instead. 
[performance-unnecessary-copy-on-last-use] 
  // CHECK-FIXES-CXX: return F? std::move(Mov): Movable{};




Comment at: 
clang-tools-extra/clang-tidy/bugprone/ImplicitWideningOfMultiplicationResultCheck.cpp:215
   (Twine("static_cast<") + TyAsString + ">(").str())
-   << FixItHint::CreateInsertion(IndexExpr->getEndLoc(), ")");
 else

Actually, I find it pretty weird/suspicious, that ` -> 
getEndLoc()` does not return the end of its `Expr`. The code looked correct 
already.
Can you elaborate, if this is a bug in the `getEndLoc()` of those `Expr`s? It 
might be better to fix it there directly.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141058

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

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Chris Cotter via Phabricator via cfe-commits]

ccotter marked an inline comment as done and an inline comment as not done.
ccotter added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };

ccotter wrote:
> ccotter wrote:
> > ccotter wrote:
> > > carlosgalvezp wrote:
> > > > carlosgalvezp wrote:
> > > > > carlosgalvezp wrote:
> > > > > > "default capture"?
> > > > > I find the check name a bit unintuitive. If you are up for a rename 
> > > > > (you can use `rename_check.py`), I would consider renaming to 
> > > > > something like 
> > > > > `cppcoreguidelines-avoid-default-capture-when-capturing-this`
> > > > > 
> > > > > Like, what should be avoided is not "capturing this", it's using a 
> > > > > default capture.
> > > > > 
> > > > > Would be good to get other reviewers opinion before spending time on 
> > > > > renaming.
> > > > Maybe put it within quotes so clarify it's a C++ keyword? Either 
> > > > backticks `this` or single quotes 'this' would work I think, unless we 
> > > > have some other convention.
> > > I updated all references to capture default to say "capture default" as 
> > > this is how it is spelled in the standard. The CppCoreGuideline for F.54 
> > > does use "default capture" though, so I'll open an issue seeing if that 
> > > wording should instead say "capture default." Also for reference, `git 
> > > grep` within the llvm-project repo shows
> > > 
> > > ```
> > > $ git grep -i 'capture default' | wc -l
> > >   43
> > > $ git grep -i 'default capture' | wc -l
> > >   54
> > > $ git grep -i 'capture.default' | wc -l # E.g., 'capture-default' or 
> > > 'capture default'
> > >  105
> > > ```
> > Updated with a single quote. I didn't find any clang-tidy diagnostics 
> > emitting backticks, but saw usage of single quotes when referring to 
> > identifiers like variable or class names.
> +1 - I admit, I struggled naming this check. More feedback welcome on the name
https://github.com/isocpp/CppCoreGuidelines/pull/2016 for "capture default" vs 
"default capture"


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

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

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Chris Cotter via Phabricator via cfe-commits]

ccotter marked an inline comment as done.
ccotter added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };

carlosgalvezp wrote:
> carlosgalvezp wrote:
> > carlosgalvezp wrote:
> > > "default capture"?
> > I find the check name a bit unintuitive. If you are up for a rename (you 
> > can use `rename_check.py`), I would consider renaming to something like 
> > `cppcoreguidelines-avoid-default-capture-when-capturing-this`
> > 
> > Like, what should be avoided is not "capturing this", it's using a default 
> > capture.
> > 
> > Would be good to get other reviewers opinion before spending time on 
> > renaming.
> Maybe put it within quotes so clarify it's a C++ keyword? Either backticks 
> `this` or single quotes 'this' would work I think, unless we have some other 
> convention.
I updated all references to capture default to say "capture default" as this is 
how it is spelled in the standard. The CppCoreGuideline for F.54 does use 
"default capture" though, so I'll open an issue seeing if that wording should 
instead say "capture default." Also for reference, `git grep` within the 
llvm-project repo shows

```
$ git grep -i 'capture default' | wc -l
  43
$ git grep -i 'default capture' | wc -l
  54
$ git grep -i 'capture.default' | wc -l # E.g., 'capture-default' or 'capture 
default'
 105
```



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };

ccotter wrote:
> carlosgalvezp wrote:
> > carlosgalvezp wrote:
> > > carlosgalvezp wrote:
> > > > "default capture"?
> > > I find the check name a bit unintuitive. If you are up for a rename (you 
> > > can use `rename_check.py`), I would consider renaming to something like 
> > > `cppcoreguidelines-avoid-default-capture-when-capturing-this`
> > > 
> > > Like, what should be avoided is not "capturing this", it's using a 
> > > default capture.
> > > 
> > > Would be good to get other reviewers opinion before spending time on 
> > > renaming.
> > Maybe put it within quotes so clarify it's a C++ keyword? Either backticks 
> > `this` or single quotes 'this' would work I think, unless we have some 
> > other convention.
> I updated all references to capture default to say "capture default" as this 
> is how it is spelled in the standard. The CppCoreGuideline for F.54 does use 
> "default capture" though, so I'll open an issue seeing if that wording should 
> instead say "capture default." Also for reference, `git grep` within the 
> llvm-project repo shows
> 
> ```
> $ git grep -i 'capture default' | wc -l
>   43
> $ git grep -i 'default capture' | wc -l
>   54
> $ git grep -i 'capture.default' | wc -l # E.g., 'capture-default' or 'capture 
> default'
>  105
> ```
Updated with a single quote. I didn't find any clang-tidy diagnostics emitting 
backticks, but saw usage of single quotes when referring to identifiers like 
variable or class names.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp:9
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this 
should not specify a capture default 
[cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };

ccotter wrote:
> ccotter wrote:
> > carlosgalvezp wrote:
> > > carlosgalvezp wrote:
> > > > carlosgalvezp wrote:
> > > > > "default capture"?
> > > > I find the check name a bit unintuitive. If you are up for a rename 
> > > > (you can use `rename_check.py`), I would consider renaming to something 
> > > > like `cppcoreguidelines-avoid-default-capture-when-capturing-this`
> > > > 
> > > > Like, what should be avoided is not "capturing this", it's using a 
> > > > default capture.
> > > > 
> > > > Would be good to get other reviewers opinion before spending time on 
> > > > renaming.
> > > Maybe put it within quotes so clarify it's a C++ keyword? Either 
> > > backticks `this` or single quotes 'this' would work I think, unless we 
> > > have some other convention.
> > I updated all references to capture default to say "capture default" as 
> > this is how it is spelled in the standard. The CppCoreGuideline for F.54 
> > does use "default capture" though, so I'll open an i

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 487084.
ccotter added a comment.

- typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-this-with-capture-default.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
@@ -0,0 +1,92 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-this-with-capture-default %t
+
+struct Obj {
+  void lambdas_that_warn_default_capture_copy() {
+int local{};
+int local2{};
+
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };
+
+auto explicit_this_capture_locals1 = [=, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [local, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_locals2 = [=, this]() { return (local+local2) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [local, local2, this]() { return (local+local2) > 10; };
+
+auto explicit_this_capture_local_ref = [=, this, &local]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this, &local]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref2 = [=, &local, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref3 = [=, &local, this, &local2]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, this, &local2]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref4 = [=, &local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, &local2, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref_extra_whitespace = [=, &  local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:62: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&  local, &local2, this]() { return (local+x) > 10; }
+
+auto explicit_this_capture_local_ref_with_comment = [=, & /* byref */ local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:58: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [& /* byref */ local, &local2, this]() { return (local+x) > 10; }
+
+auto implicit_this_capture = [=]() { return x > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { return x > 10; };
+
+auto implicit_this_capture_local = [=]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:41: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-cap

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 487082.
ccotter added a comment.

oops, bad 'arc diff' again


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-this-with-capture-default.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
@@ -0,0 +1,92 @@
+// RUN: %check_clang_tidy -std=c++11-or-later %s cppcoreguidelines-avoid-capture-this-with-capture-default %t
+
+struct Obj {
+  void lambdas_that_warn_default_capture_copy() {
+int local{};
+int local2{};
+
+auto explicit_this_capture = [=, this]() { };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { };
+
+auto explicit_this_capture_locals1 = [=, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [local, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_locals2 = [=, this]() { return (local+local2) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [local, local2, this]() { return (local+local2) > 10; };
+
+auto explicit_this_capture_local_ref = [=, this, &local]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this, &local]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref2 = [=, &local, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref3 = [=, &local, this, &local2]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, this, &local2]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref4 = [=, &local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&local, &local2, this]() { return (local+x) > 10; };
+
+auto explicit_this_capture_local_ref_extra_whitespace = [=, &  local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:62: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [&  local, &local2, this]() { return (local+x) > 10; }
+
+auto explicit_this_capture_local_ref_with_comment = [=, & /* byref */ local, &local2, this]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:58: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [& /* byref */ local, &local2, this]() { return (local+x) > 10; }
+
+auto implicit_this_capture = [=]() { return x > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-FIXES: [this]() { return x > 10; };
+
+auto implicit_this_capture_local = [=]() { return (local+x) > 10; };
+// CHECK-MESSAGES: :[[@LINE-1]]:41: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-c

[PATCH] D141133: [clang-tidy] Implement CppCoreGuideline F.54

[Chris Cotter via Phabricator via cfe-commits]

ccotter updated this revision to Diff 487081.
ccotter added a comment.

- Use "capture default" consistently and update diagnostic message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D141133

Files:
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.cpp
  
clang-tools-extra/clang-tidy/cppcoreguidelines/AvoidCaptureThisWithCaptureDefaultCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines/avoid-capture-this-with-capture-default.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines/avoid-capture-this-with-capture-default.cpp
@@ -6,47 +6,47 @@
 int local2{};
 
 auto explicit_this_capture = [=, this]() { };
-// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:35: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
 // CHECK-FIXES: [this]() { };
 
 auto explicit_this_capture_locals1 = [=, this]() { return (local+x) > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
 // CHECK-FIXES: [local, this]() { return (local+x) > 10; };
 
 auto explicit_this_capture_locals2 = [=, this]() { return (local+local2) > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:43: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
 // CHECK-FIXES: [local, local2, this]() { return (local+local2) > 10; };
 
 auto explicit_this_capture_local_ref = [=, this, &local]() { return (local+x) > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:45: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
 // CHECK-FIXES: [this, &local]() { return (local+x) > 10; };
 
 auto explicit_this_capture_local_ref2 = [=, &local, this]() { return (local+x) > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
 // CHECK-FIXES: [&local, this]() { return (local+x) > 10; };
 
 auto explicit_this_capture_local_ref3 = [=, &local, this, &local2]() { return (local+x) > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
 // CHECK-FIXES: [&local, this, &local2]() { return (local+x) > 10; };
 
 auto explicit_this_capture_local_ref4 = [=, &local, &local2, this]() { return (local+x) > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-MESSAGES: :[[@LINE-1]]:46: warning: lambdas that capture 'this' should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
 // CHECK-FIXES: [&local, &local2, this]() { return (local+x) > 10; };
 
 auto explicit_this_capture_local_ref_extra_whitespace = [=, &  local, &local2, this]() { return (local+x) > 10; };
-// CHECK-MESSAGES: :[[@LINE-1]]:62: warning: lambdas that capture this should not specify a capture default [cppcoreguidelines-avoid-capture-this-with-capture-default]
+// CHECK-MESSA

[PATCH] D141194: [clang][Interp] Implement bitcasts

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

I really don't know much about these, so limited tests included.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141194

Files:
  clang/lib/AST/Interp/ByteCodeExprGen.cpp
  clang/test/AST/Interp/literals.cpp


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -90,6 +90,11 @@
 static_assert(p != nullptr, "");
 static_assert(*p == 10, "");
 
+constexpr const void *cp = (void *)p;
+// FIXME: This should be an error in the new interpreter.
+constexpr const int *pm = (int*)cp; // ref-error {{ must be initialized by a 
constant expression}} \
+// ref-note {{cast from 'const void *' is 
not allowed}}
+
 constexpr const int* getIntPointer() {
   return &m;
 }
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -150,6 +150,7 @@
   case CK_NonAtomicToAtomic:
   case CK_NoOp:
   case CK_UserDefinedConversion:
+  case CK_BitCast:
 return this->visit(SubExpr);
 
   case CK_IntegralToBoolean:


Index: clang/test/AST/Interp/literals.cpp
===
--- clang/test/AST/Interp/literals.cpp
+++ clang/test/AST/Interp/literals.cpp
@@ -90,6 +90,11 @@
 static_assert(p != nullptr, "");
 static_assert(*p == 10, "");
 
+constexpr const void *cp = (void *)p;
+// FIXME: This should be an error in the new interpreter.
+constexpr const int *pm = (int*)cp; // ref-error {{ must be initialized by a constant expression}} \
+// ref-note {{cast from 'const void *' is not allowed}}
+
 constexpr const int* getIntPointer() {
   return &m;
 }
Index: clang/lib/AST/Interp/ByteCodeExprGen.cpp
===
--- clang/lib/AST/Interp/ByteCodeExprGen.cpp
+++ clang/lib/AST/Interp/ByteCodeExprGen.cpp
@@ -150,6 +150,7 @@
   case CK_NonAtomicToAtomic:
   case CK_NoOp:
   case CK_UserDefinedConversion:
+  case CK_BitCast:
 return this->visit(SubExpr);
 
   case CK_IntegralToBoolean:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D141193: [clang][Interp] Implement __builtin_assume

[Timm Bäder via Phabricator via cfe-commits]

tbaeder created this revision.
tbaeder added reviewers: aaron.ballman, erichkeane, tahonermann, shafik.
Herald added a project: All.
tbaeder requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

  Just ignore it.
  
  As part of this, move the Ret and RetVoid implementation to Interp.h, so
  they can be shared with InterpBuiltin.cpp.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141193

Files:
  clang/lib/AST/Interp/Interp.cpp
  clang/lib/AST/Interp/Interp.h
  clang/lib/AST/Interp/InterpBuiltin.cpp
  clang/test/AST/Interp/builtins.cpp

Index: clang/test/AST/Interp/builtins.cpp
===
--- clang/test/AST/Interp/builtins.cpp
+++ clang/test/AST/Interp/builtins.cpp
@@ -22,3 +22,10 @@
 bool is_this_constant() {
   return __builtin_is_constant_evaluated(); // CHECK: ret i1 false
 }
+
+constexpr bool assume() {
+  __builtin_assume(true);
+  __builtin_assume(false);
+  return true;
+}
+static_assert(assume(), "");
Index: clang/lib/AST/Interp/InterpBuiltin.cpp
===
--- clang/lib/AST/Interp/InterpBuiltin.cpp
+++ clang/lib/AST/Interp/InterpBuiltin.cpp
@@ -13,35 +13,17 @@
 namespace clang {
 namespace interp {
 
-/// This is a slightly simplified version of the Ret() we have in Interp.cpp
-/// If they end up diverging in the future, we should get rid of the code
-/// duplication.
-template ::T>
-static bool Ret(InterpState &S, CodePtr &PC) {
-  S.CallStackDepth--;
-  const T &Ret = S.Stk.pop();
+bool InterpretBuiltin(InterpState &S, CodePtr &PC, unsigned BuiltinID) {
+  APValue Dummy;
 
-  assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
-  if (!S.checkingPotentialConstantExpression())
-S.Current->popArgs();
-
-  InterpFrame *Caller = S.Current->Caller;
-  assert(Caller);
-
-  PC = S.Current->getRetPC();
-  delete S.Current;
-  S.Current = Caller;
-  S.Stk.push(Ret);
-
-  return true;
-}
-
-bool InterpretBuiltin(InterpState &S, CodePtr PC, unsigned BuiltinID) {
   switch (BuiltinID) {
   case Builtin::BI__builtin_is_constant_evaluated:
 S.Stk.push(Boolean::from(S.inConstantContext()));
-Ret(S, PC);
-return true;
+return Ret(S, PC, Dummy);
+  case Builtin::BI__builtin_assume:
+return RetVoid(S, PC, Dummy);
+  default:
+return false;
   }
 
   return false;
Index: clang/lib/AST/Interp/Interp.h
===
--- clang/lib/AST/Interp/Interp.h
+++ clang/lib/AST/Interp/Interp.h
@@ -109,10 +109,58 @@
 /// Interpreter entry point.
 bool Interpret(InterpState &S, APValue &Result);
 
-bool InterpretBuiltin(InterpState &S, CodePtr PC, unsigned BuiltinID);
+/// Interpret a builtin function.
+bool InterpretBuiltin(InterpState &S, CodePtr &PC, unsigned BuiltinID);
 
 enum class ArithOp { Add, Sub };
 
+//===--===//
+// Returning values
+//===--===//
+
+template ::T>
+bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
+  S.CallStackDepth--;
+  const T &Ret = S.Stk.pop();
+
+  assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
+  if (Builtin || !S.checkingPotentialConstantExpression())
+S.Current->popArgs();
+
+  if (InterpFrame *Caller = S.Current->Caller) {
+PC = S.Current->getRetPC();
+delete S.Current;
+S.Current = Caller;
+S.Stk.push(Ret);
+  } else {
+delete S.Current;
+S.Current = nullptr;
+if (!ReturnValue(Ret, Result))
+  return false;
+  }
+  return true;
+}
+
+template 
+inline bool RetVoid(InterpState &S, CodePtr &PC, APValue &Result) {
+  S.CallStackDepth--;
+
+  assert(S.Current->getFrameOffset() == S.Stk.size() && "Invalid frame");
+  if (Builtin || !S.checkingPotentialConstantExpression())
+S.Current->popArgs();
+
+  if (InterpFrame *Caller = S.Current->Caller) {
+PC = S.Current->getRetPC();
+delete S.Current;
+S.Current = Caller;
+  } else {
+delete S.Current;
+S.Current = nullptr;
+  }
+  return true;
+}
+
 //===--===//
 // Add, Sub, Mul
 //===--===//
Index: clang/lib/AST/Interp/Interp.cpp
===
--- clang/lib/AST/Interp/Interp.cpp
+++ clang/lib/AST/Interp/Interp.cpp
@@ -26,51 +26,6 @@
 using namespace clang;
 using namespace clang::interp;
 
-//===--===//
-// Ret
-//===--===//
-
-template ::T>
-static bool Ret(InterpState &S, CodePtr &PC, APValue &Result) {
-  S.CallStackDepth--;
-  const T &Ret = S.Stk.pop();
-
-  assert(S.Current->getFrameOffset() == S.Stk.size() && "Inva

[PATCH] D141192: [Clang] Add warnings on bad shifts inside enums.

[Dmitriy Chestnykh via Phabricator via cfe-commits]

chestnykh created this revision.
chestnykh added a reviewer: MaskRay.
Herald added a subscriber: StephenFan.
Herald added a project: All.
chestnykh requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Those warnings were gone because DiagRuntimeBehavior
doesn't emit warnings in ConstantEvaluated case.
PR#59863


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D141192

Files:
  clang/lib/Sema/SemaExpr.cpp
  clang/test/Sema/shift-count-negative.c
  clang/test/Sema/shift-count-overflow.c
  clang/test/Sema/shift-negative-value.c

Index: clang/test/Sema/shift-negative-value.c
===
--- /dev/null
+++ clang/test/Sema/shift-negative-value.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wshift-negative-value %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wshift-negative-value %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+enum shiftof {
+X = (-1<<29) //expected-warning {{shifting a negative signed value is undefined}}
+};
+
Index: clang/test/Sema/shift-count-overflow.c
===
--- /dev/null
+++ clang/test/Sema/shift-count-overflow.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wshift-count-overflow %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wshift-count-overflow %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+enum shiftof {
+X = (1<<32) //expected-warning {{shift count >= width of type}}
+};
+
Index: clang/test/Sema/shift-count-negative.c
===
--- /dev/null
+++ clang/test/Sema/shift-count-negative.c
@@ -0,0 +1,9 @@
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wshift-count-negative %s
+// RUN: %clang_cc1 -x c -fsyntax-only -verify -Wall %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wshift-count-negative %s
+// RUN: %clang_cc1 -x c++ -fsyntax-only -verify -Wall %s
+
+enum shiftof {
+X = (1<<-29) //expected-warning {{shift count is negative}}
+};
+
Index: clang/lib/Sema/SemaExpr.cpp
===
--- clang/lib/Sema/SemaExpr.cpp
+++ clang/lib/Sema/SemaExpr.cpp
@@ -11428,26 +11428,35 @@
   return false;
 }
 
-static void DiagnoseBadShiftValues(Sema& S, ExprResult &LHS, ExprResult &RHS,
-   SourceLocation Loc, BinaryOperatorKind Opc,
-   QualType LHSType) {
+enum BadShiftValueKind {
+  BSV_Ok,
+  BSV_ShiftIsNegative,
+  BSV_ShiftLHSIsNegative,
+  BSV_ShiftSizeGTTypeWidth,
+};
+
+static BadShiftValueKind DiagnoseBadShiftValues(Sema &S, ExprResult &LHS,
+ExprResult &RHS,
+SourceLocation Loc,
+BinaryOperatorKind Opc,
+QualType LHSType) {
   // OpenCL 6.3j: shift values are effectively % word size of LHS (more defined),
   // so skip remaining warnings as we don't want to modify values within Sema.
   if (S.getLangOpts().OpenCL)
-return;
+return BSV_Ok;
 
   // Check right/shifter operand
   Expr::EvalResult RHSResult;
   if (RHS.get()->isValueDependent() ||
   !RHS.get()->EvaluateAsInt(RHSResult, S.Context))
-return;
+return BSV_Ok;
   llvm::APSInt Right = RHSResult.Val.getInt();
 
   if (Right.isNegative()) {
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_negative)
 << RHS.get()->getSourceRange());
-return;
+return BSV_ShiftIsNegative;
   }
 
   QualType LHSExprType = LHS.get()->getType();
@@ -11463,12 +11472,12 @@
 S.DiagRuntimeBehavior(Loc, RHS.get(),
   S.PDiag(diag::warn_shift_gt_typewidth)
 << RHS.get()->getSourceRange());
-return;
+return BSV_ShiftSizeGTTypeWidth;
   }
 
   // FIXME: We probably need to handle fixed point types specially here.
   if (Opc != BO_Shl || LHSExprType->isFixedPointType())
-return;
+return BSV_Ok;
 
   // When left shifting an ICE which is signed, we can check for overflow which
   // according to C++ standards prior to C++2a has undefined behavior
@@ -11480,7 +11489,7 @@
   if (LHS.get()->isValueDependent() ||
   LHSType->hasUnsignedIntegerRepresentation() ||
   !LHS.get()->EvaluateAsInt(LHSResult, S.Context))
-return;
+return BSV_Ok;
   llvm::APSInt Left = LHSResult.Val.getInt();
 
   // Don't warn if signed overflow is defined, then all the rest of the
@@ -11488,7 +11497,7 @@
   // Also don't warn in C++20 mode (and newer), as signed left shifts
   // always wrap and never overflow.
   if (S.getLangOpts().isSignedOverflowDefined(

[PATCH] D141092: Optionally install clang-tblgen to aid cross-compiling

[James Le Cuirot via Phabricator via cfe-commits]

chewi added a comment.

I forgot to add: I notice you no longer need llvm-tblgen already present when 
cross-compiling LLVM, but sadly, that still isn't the case for clang-tblgen. I 
tried leveraging CrossCompile.cmake to perform a "NATIVE" Clang build, but I 
think a lot more work would be needed to do it this way.


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

https://reviews.llvm.org/D141092

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

[PATCH] D141092: Optionally install clang-tblgen to aid cross-compiling

[James Le Cuirot via Phabricator via cfe-commits]

chewi updated this revision to Diff 487073.
chewi edited the summary of this revision.
chewi added a reviewer: nikic.
chewi added a comment.
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Reworked and retested against the main branch. Thankfully, it is simpler now.


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

https://reviews.llvm.org/D141092

Files:
  llvm/cmake/modules/TableGen.cmake


Index: llvm/cmake/modules/TableGen.cmake
===
--- llvm/cmake/modules/TableGen.cmake
+++ llvm/cmake/modules/TableGen.cmake
@@ -190,7 +190,8 @@
 endif()
   endif()
 
-  if (ADD_TABLEGEN_DESTINATION AND NOT LLVM_INSTALL_TOOLCHAIN_ONLY AND 
LLVM_BUILD_UTILS)
+  if (ADD_TABLEGEN_DESTINATION AND NOT LLVM_INSTALL_TOOLCHAIN_ONLY AND
+  (LLVM_BUILD_UTILS OR ${target} IN_LIST LLVM_DISTRIBUTION_COMPONENTS))
 set(export_arg)
 if(ADD_TABLEGEN_EXPORT)
   get_target_export_arg(${target} ${ADD_TABLEGEN_EXPORT} export_arg)


Index: llvm/cmake/modules/TableGen.cmake
===
--- llvm/cmake/modules/TableGen.cmake
+++ llvm/cmake/modules/TableGen.cmake
@@ -190,7 +190,8 @@
 endif()
   endif()
 
-  if (ADD_TABLEGEN_DESTINATION AND NOT LLVM_INSTALL_TOOLCHAIN_ONLY AND LLVM_BUILD_UTILS)
+  if (ADD_TABLEGEN_DESTINATION AND NOT LLVM_INSTALL_TOOLCHAIN_ONLY AND
+  (LLVM_BUILD_UTILS OR ${target} IN_LIST LLVM_DISTRIBUTION_COMPONENTS))
 set(export_arg)
 if(ADD_TABLEGEN_EXPORT)
   get_target_export_arg(${target} ${ADD_TABLEGEN_EXPORT} export_arg)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D139926: [clangd] Add semantic tokens for angle brackets

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D139926#4032473 , @ckandeler wrote:

> In D139926#4030782 , @nridge wrote:
>
>> It's true that there is an ambiguity between `<` and `>` as operators, vs. 
>> template arg/param list delimiters, but, at least in terms of user 
>> understanding of code, my sense is that the highlighting of the 
>> **preceding** token should be sufficient to disambiguate -- i.e. it would be 
>> some sort of type name in the template case, vs. a variable / literal / 
>> punctuation ending an expression in the operator case.
>
> We used to do this sort of heuristic in our old libclang-based 
> implementation, and it turned out to be rather messy, with a surprising 
> amount of exceptions having to be added.

To clarify, I'm not suggesting that any client-side logic use this heuristic, 
only that it's probably good enough for a human reader to disambiguate without 
needing to assign angle brackets and comparison operators different colors.

>>> This is needed for clients that would like to visualize matching opening 
>>> and closing angle brackets, which can be valuable in non-trivial template 
>>> declarations or instantiations.
>>
>> For this use case, could an editor make use of the recently added operator 
>> tokens (https://reviews.llvm.org/D136594) instead, inferring that a `<` 
>> token which does not have an `operator` semantic token is a template 
>> delimiter?
>
> I have a suspicion that this will lead to false positives for invalid code.

Ah, interesting. I guess in the case of invalid code (no/malformed AST) you 
wouldn't get AngleBracket tokens either, so this gives you a three-way signal 
(Operator vs. AngleBracket vs. no semantic token) that's richer than just the 
two-way signal of Operator vs. no Operator.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D139926

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