[PATCH] D103789: [clangd] Type hints for C++14 return type deduction

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:79
+  if (!Deduced.isNull()) {
+SourceRange R = D->getReturnTypeSourceRange();
+// For operator auto(), have to get location of `auto` a different way.

sammccall wrote:
> nit: bool TrailingReturnType = D->getReturnTypeSourceRange().isValid()?
Sorry, I don't really understand this comment.

If you're thinking about the test case where a trailing return type is present 
and we avoid producing a hint in that case (`f3` in 
`TypeHints.ReturnTypeDeduction`), that happens because `getContainedAutoType()` 
returns null, so we don't get into this block at all.



Comment at: clang-tools-extra/clangd/unittests/InlayHintTests.cpp:505
+  )cpp",
+  ExpectedHint{": int", "ret1a"}, ExpectedHint{": int", "ret1b"},
+  ExpectedHint{": int &", "ret2"}, ExpectedHint{": int", "retConv"});

sammccall wrote:
> This reads as "auto[: int] f1(int x);", which doesn't look much like familiar 
> syntax, C++ or otherwise.
> 
> I guess we could try `auto f1(int x)[-> int];`?
> 
> (From playing with these in vscode, I'm not sure I find the punctuation very 
> useful)
I like this suggestion, thanks!

I agree it's nice if the hint is in a place where it would have been valid 
syntax.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103789

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

[PATCH] D103789: [clangd] Type hints for C++14 return type deduction

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 351788.
nridge added a comment.

Put hint in trailing return type position


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103789

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


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -478,14 +478,31 @@
 }
 
 TEST(TypeHints, ReturnTypeDeduction) {
-  // FIXME: Not handled yet.
-  // This test is currently here mostly because a naive implementation
-  // might have us print something not super helpful like the function type.
-  assertTypeHints(R"cpp(
-auto func(int x) {
-  return x + 1;
-}
-  )cpp");
+  assertTypeHints(
+  R"cpp(
+auto f1(int x$ret1a[[)]];  // Hint forward declaration too
+auto f1(int x$ret1b[[)]] { return x + 1; }
+
+// Include pointer operators in hint
+int s;
+auto& f2($ret2[[)]] { return s; }
+
+// Do not hint `auto` for trailing return type.
+auto f3() -> int;
+
+// `auto` conversion operator
+struct A {
+  operator auto($retConv[[)]] { return 42; }
+};
+
+// FIXME: Dependent types do not work yet.
+template 
+struct S {
+  auto method() { return T(); }
+};
+  )cpp",
+  ExpectedHint{"-> int", "ret1a"}, ExpectedHint{"-> int", "ret1b"},
+  ExpectedHint{"-> int &", "ret2"}, ExpectedHint{"-> int", "retConv"});
 }
 
 TEST(TypeHints, DependentType) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -72,6 +72,19 @@
 return true;
   }
 
+  bool VisitFunctionDecl(FunctionDecl *D) {
+if (auto *AT = D->getReturnType()->getContainedAutoType()) {
+  QualType Deduced = AT->getDeducedType();
+  if (!Deduced.isNull()) {
+addInlayHint(D->getFunctionTypeLoc().getRParenLoc(),
+ InlayHintKind::TypeHint,
+ "-> " + D->getReturnType().getAsString(TypeHintPolicy));
+  }
+}
+
+return true;
+  }
+
   bool VisitVarDecl(VarDecl *D) {
 // Do not show hints for the aggregate in a structured binding.
 // In the future, we may show hints for the individual bindings.


Index: clang-tools-extra/clangd/unittests/InlayHintTests.cpp
===
--- clang-tools-extra/clangd/unittests/InlayHintTests.cpp
+++ clang-tools-extra/clangd/unittests/InlayHintTests.cpp
@@ -478,14 +478,31 @@
 }
 
 TEST(TypeHints, ReturnTypeDeduction) {
-  // FIXME: Not handled yet.
-  // This test is currently here mostly because a naive implementation
-  // might have us print something not super helpful like the function type.
-  assertTypeHints(R"cpp(
-auto func(int x) {
-  return x + 1;
-}
-  )cpp");
+  assertTypeHints(
+  R"cpp(
+auto f1(int x$ret1a[[)]];  // Hint forward declaration too
+auto f1(int x$ret1b[[)]] { return x + 1; }
+
+// Include pointer operators in hint
+int s;
+auto& f2($ret2[[)]] { return s; }
+
+// Do not hint `auto` for trailing return type.
+auto f3() -> int;
+
+// `auto` conversion operator
+struct A {
+  operator auto($retConv[[)]] { return 42; }
+};
+
+// FIXME: Dependent types do not work yet.
+template 
+struct S {
+  auto method() { return T(); }
+};
+  )cpp",
+  ExpectedHint{"-> int", "ret1a"}, ExpectedHint{"-> int", "ret1b"},
+  ExpectedHint{"-> int &", "ret2"}, ExpectedHint{"-> int", "retConv"});
 }
 
 TEST(TypeHints, DependentType) {
Index: clang-tools-extra/clangd/InlayHints.cpp
===
--- clang-tools-extra/clangd/InlayHints.cpp
+++ clang-tools-extra/clangd/InlayHints.cpp
@@ -72,6 +72,19 @@
 return true;
   }
 
+  bool VisitFunctionDecl(FunctionDecl *D) {
+if (auto *AT = D->getReturnType()->getContainedAutoType()) {
+  QualType Deduced = AT->getDeducedType();
+  if (!Deduced.isNull()) {
+addInlayHint(D->getFunctionTypeLoc().getRParenLoc(),
+ InlayHintKind::TypeHint,
+ "-> " + D->getReturnType().getAsString(TypeHintPolicy));
+  }
+}
+
+return true;
+  }
+
   bool VisitVarDecl(VarDecl *D) {
 // Do not show hints for the aggregate in a structured binding.
 // In the future, we may show hints for the individual bindings.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D99810: [ifs] Prepare llvm-ifs for elfabi/ifs merging.

[Petr Hosek via Phabricator via cfe-commits]

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



Comment at: llvm/include/llvm/InterfaceStub/ELFObjHandler.h:25
 
 namespace elfabi {
 

Shouldn't this be in namespace `ifs`?



Comment at: llvm/include/llvm/InterfaceStub/IFSStub.h:82-83
+  return true;
+}
+inline bool operator!=(const IFSTarget &Lhs, const IFSTarget &Rhs) {
+  return !(Lhs == Rhs);

Use empty line between functions.



Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:57-61
+  if (!Triple && !ObjectFormat && !Arch && !ArchString && !Endianness &&
+  !BitWidth) {
+return true;
+  }
+  return false;

This can be simplified.



Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:65
+uint8_t elfabi::convertIFSBitWidthToELF(IFSBitWidthType BitWidth) {
+  return BitWidth == IFSBitWidthType::IFS32 ? ELF::ELFCLASS32 : 
ELF::ELFCLASS64;
+}

I'd consider using `switch` here so if someone adds a new entry to the 
`IFSBitWidthType` enum, it gets caught by the compiler.



Comment at: llvm/lib/InterfaceStub/IFSStub.cpp:69
+uint8_t elfabi::convertIFSEndiannessToELF(IFSEndiannessType Endianness) {
+  return Endianness == IFSEndiannessType::Little ? ELF::ELFDATA2LSB
+ : ELF::ELFDATA2MSB;

Ditto here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99810

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

[PATCH] D104116: AMD k8 family does not support SSE4.x which are required by x86-64-v2+

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added reviewers: pengfei, craig.topper.
RKSimon added a comment.

This leaves the question - what hardware should we align each of the 
CK_x86_64_v* targets with?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104116

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

[clang] 669771c - [clang][deps] NFC: Preserve the original frontend action

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T10:49:25+02:00
New Revision: 669771cfe75b48ac4c195ce9e8824319be973f4d

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

LOG: [clang][deps] NFC: Preserve the original frontend action

This patch stops adjusting the frontend action when `clang-scan-deps` is 
configured to use the full output format.

In a future patch, the dependency scanner needs to check whether the original 
compiler invocation builds a PCH. That's impossible when `-Eonly` et al. 
override `-emit-pch`.

The `-Eonly` flag is not needed - the dependency scanner explicitly sets up its 
own frontend action anyways.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D103461

Added: 


Modified: 
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 2501ec3470e1..ce8be72bc291 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -462,6 +462,10 @@ int main(int argc, const char **argv) {
   std::make_unique(
   std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
+
+  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
+  // instead of parsing and adjusting the raw command-line. This will make it
+  // possible to remove some code specific to clang-cl and Windows.
   AdjustingCompilations->appendArgumentsAdjuster(
   [&ResourceDirCache](const tooling::CommandLineArguments &Args,
   StringRef FileName) {
@@ -532,7 +536,7 @@ int main(int argc, const char **argv) {
 #else
 AdjustedArgs.push_back("/dev/null");
 #endif
-if (!HasMT && !HasMQ) {
+if (!HasMT && !HasMQ && Format == ScanningOutputFormat::Make) {
   // We're interested in source dependencies of an object file.
   std::string FileNameArg;
   if (!HasMD) {
@@ -553,8 +557,6 @@ int main(int argc, const char **argv) {
   }
 }
 AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-Eonly");
-AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
 AdjustedArgs.push_back("-Wno-error");
 



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

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Jan Svoboda via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG669771cfe75b: [clang][deps] NFC: Preserve the original 
frontend action (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D103461?vs=349229&id=351795#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103461

Files:
  clang/tools/clang-scan-deps/ClangScanDeps.cpp


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -462,6 +462,10 @@
   std::make_unique(
   std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
+
+  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
+  // instead of parsing and adjusting the raw command-line. This will make it
+  // possible to remove some code specific to clang-cl and Windows.
   AdjustingCompilations->appendArgumentsAdjuster(
   [&ResourceDirCache](const tooling::CommandLineArguments &Args,
   StringRef FileName) {
@@ -532,7 +536,7 @@
 #else
 AdjustedArgs.push_back("/dev/null");
 #endif
-if (!HasMT && !HasMQ) {
+if (!HasMT && !HasMQ && Format == ScanningOutputFormat::Make) {
   // We're interested in source dependencies of an object file.
   std::string FileNameArg;
   if (!HasMD) {
@@ -553,8 +557,6 @@
   }
 }
 AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-Eonly");
-AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
 AdjustedArgs.push_back("-Wno-error");
 


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -462,6 +462,10 @@
   std::make_unique(
   std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
+
+  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
+  // instead of parsing and adjusting the raw command-line. This will make it
+  // possible to remove some code specific to clang-cl and Windows.
   AdjustingCompilations->appendArgumentsAdjuster(
   [&ResourceDirCache](const tooling::CommandLineArguments &Args,
   StringRef FileName) {
@@ -532,7 +536,7 @@
 #else
 AdjustedArgs.push_back("/dev/null");
 #endif
-if (!HasMT && !HasMQ) {
+if (!HasMT && !HasMQ && Format == ScanningOutputFormat::Make) {
   // We're interested in source dependencies of an object file.
   std::string FileNameArg;
   if (!HasMD) {
@@ -553,8 +557,6 @@
   }
 }
 AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-Eonly");
-AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
 AdjustedArgs.push_back("-Wno-error");
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103461: [clang][deps] NFC: Preserve the original frontend action

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D103461#2815913 , @dexonsmith 
wrote:

> Okay, LGTM. (Sorry for the delay, I've been out.)
>
> In D103461#2793254 , @jansvoboda11 
> wrote:
>
>> My reason for the FIXME is that we could get rid of bunch of 
>> Windows-specific logic by adjusting `CompilerInvocation` instead.
>
> Please add that motivation to the FIXME itself.

No problem, thanks for the review(s)!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103461

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

[PATCH] D102186: [clang][c++20] Fix false warning for unused private fields when a class has only defaulted comparison operators.

[Hans Wennborg via Phabricator via cfe-commits]

hans added inline comments.



Comment at: clang/test/SemaCXX/warn-unused-private-field.cpp:7
+
+class EqDefaultCompare {
+  int unused_;

The test case feels a little strange to me, I mean the member variable is still 
pretty unused even though I suppose the comparison function technically uses 
it.. At least the test case in the bug report had a constructor which set the 
value. Perhaps with that it makes more sense.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102186

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

[clang] 78668c8 - [clang][modules][pch] Allow loading PCH with different modules cache path

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T11:04:56+02:00
New Revision: 78668c822af9504f77a554f5924e1097365d9c33

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

LOG: [clang][modules][pch] Allow loading PCH with different modules cache path

It's useful to be able to load explicitly-built PCH files into an implicit 
build (e.g. during dependency scanning). That's currently impossible, since the 
explicitly-built PCH has an empty modules cache path, while the current 
compilation has (and needs to have) a valid path, triggering an error in the 
`PCHValidator`.

This patch adds a preprocessor option and command-line flag that can be used to 
omit this check.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D103802

Added: 
clang/test/Modules/Inputs/pch-typedef.h
clang/test/Modules/module-pch-different-cache-path.c

Modified: 
clang/include/clang/Driver/Options.td
clang/include/clang/Lex/PreprocessorOptions.h
clang/lib/Serialization/ASTReader.cpp

Removed: 




diff  --git a/clang/include/clang/Driver/Options.td 
b/clang/include/clang/Driver/Options.td
index 3968aa3431ea..329caae5f9fe 100644
--- a/clang/include/clang/Driver/Options.td
+++ b/clang/include/clang/Driver/Options.td
@@ -5499,6 +5499,10 @@ def fallow_pch_with_errors : Flag<["-"], 
"fallow-pch-with-compiler-errors">,
   HelpText<"Accept a PCH file that was created with compiler errors">,
   MarshallingInfoFlag>,
   ImpliedByAnyOf<[fallow_pcm_with_errors.KeyPath]>;
+def fallow_pch_with_
diff erent_modules_cache_path :
+  Flag<["-"], "fallow-pch-with-
diff erent-modules-cache-path">,
+  HelpText<"Accept a PCH file that was created with a 
diff erent modules cache path">,
+  
MarshallingInfoFlag>;
 def dump_deserialized_pch_decls : Flag<["-"], "dump-deserialized-decls">,
   HelpText<"Dump declarations that are deserialized from PCH, for testing">,
   MarshallingInfoFlag>;

diff  --git a/clang/include/clang/Lex/PreprocessorOptions.h 
b/clang/include/clang/Lex/PreprocessorOptions.h
index 7f024989bf9b..99085b98fc7a 100644
--- a/clang/include/clang/Lex/PreprocessorOptions.h
+++ b/clang/include/clang/Lex/PreprocessorOptions.h
@@ -106,6 +106,10 @@ class PreprocessorOptions {
   /// When true, a PCH with compiler errors will not be rejected.
   bool AllowPCHWithCompilerErrors = false;
 
+  /// When true, a PCH with modules cache path 
diff erent to the current
+  /// compilation will not be rejected.
+  bool AllowPCHWithDifferentModulesCachePath = false;
+
   /// Dump declarations that are deserialized from PCH, for testing.
   bool DumpDeserializedPCHDecls = false;
 

diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 55e0d084ea4b..80ee56a9b7df 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -783,9 +783,11 @@ static bool checkHeaderSearchOptions(const 
HeaderSearchOptions &HSOpts,
  StringRef SpecificModuleCachePath,
  StringRef ExistingModuleCachePath,
  DiagnosticsEngine *Diags,
- const LangOptions &LangOpts) {
+ const LangOptions &LangOpts,
+ const PreprocessorOptions &PPOpts) {
   if (LangOpts.Modules) {
-if (SpecificModuleCachePath != ExistingModuleCachePath) {
+if (SpecificModuleCachePath != ExistingModuleCachePath &&
+!PPOpts.AllowPCHWithDifferentModulesCachePath) {
   if (Diags)
 Diags->Report(diag::err_pch_modulecache_mismatch)
   << SpecificModuleCachePath << ExistingModuleCachePath;
@@ -802,7 +804,7 @@ bool PCHValidator::ReadHeaderSearchOptions(const 
HeaderSearchOptions &HSOpts,
   return checkHeaderSearchOptions(HSOpts, SpecificModuleCachePath,
   
PP.getHeaderSearchInfo().getModuleCachePath(),
   Complain ? &Reader.Diags : nullptr,
-  PP.getLangOpts());
+  PP.getLangOpts(), PP.getPreprocessorOpts());
 }
 
 void PCHValidator::ReadCounter(const ModuleFile &M, unsigned Value) {
@@ -5142,8 +5144,8 @@ namespace {
  StringRef SpecificModuleCachePath,
  bool Complain) override {
   return checkHeaderSearchOptions(HSOpts, SpecificModuleCachePath,
-  ExistingModuleCachePath,
-  nullptr, ExistingLangOpts);
+  ExistingModuleCachePath, nullptr,
+  ExistingLangOpts, ExistingPPOpts);
 }
 
 bool ReadPreprocessorOpt

[PATCH] D103802: [clang][modules][pch] Allow loading PCH with different modules cache path

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG78668c822af9: [clang][modules][pch] Allow loading PCH with 
different modules cache path (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103802

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/PreprocessorOptions.h
  clang/lib/Serialization/ASTReader.cpp
  clang/test/Modules/Inputs/pch-typedef.h
  clang/test/Modules/module-pch-different-cache-path.c


Index: clang/test/Modules/module-pch-different-cache-path.c
===
--- /dev/null
+++ clang/test/Modules/module-pch-different-cache-path.c
@@ -0,0 +1,18 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+//
+// RUN: %clang_cc1 -x objective-c-header -emit-pch %S/Inputs/pch-typedef.h -o 
%t/pch-typedef.h.gch \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/one
+//
+// RUN: not %clang_cc1 -x objective-c -fsyntax-only %s -include-pch 
%t/pch-typedef.h.gch \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/two 2>&1 \
+// RUN:   | FileCheck %s -check-prefixes=CHECK-ERROR
+//
+// RUN: %clang_cc1 -x objective-c -fsyntax-only %s -include-pch 
%t/pch-typedef.h.gch \
+// RUN:   -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/two 2>&1 
-fallow-pch-with-different-modules-cache-path \
+// RUN:   | FileCheck %s -check-prefixes=CHECK-SUCCESS -allow-empty
+
+pch_int x = 0;
+
+// CHECK-ERROR: PCH was compiled with module cache path '{{.*}}', but the path 
is currently '{{.*}}'
+// CHECK-SUCCESS-NOT: PCH was compiled with module cache path '{{.*}}', but 
the path is currently '{{.*}}'
Index: clang/test/Modules/Inputs/pch-typedef.h
===
--- /dev/null
+++ clang/test/Modules/Inputs/pch-typedef.h
@@ -0,0 +1 @@
+typedef int pch_int;
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -783,9 +783,11 @@
  StringRef SpecificModuleCachePath,
  StringRef ExistingModuleCachePath,
  DiagnosticsEngine *Diags,
- const LangOptions &LangOpts) {
+ const LangOptions &LangOpts,
+ const PreprocessorOptions &PPOpts) {
   if (LangOpts.Modules) {
-if (SpecificModuleCachePath != ExistingModuleCachePath) {
+if (SpecificModuleCachePath != ExistingModuleCachePath &&
+!PPOpts.AllowPCHWithDifferentModulesCachePath) {
   if (Diags)
 Diags->Report(diag::err_pch_modulecache_mismatch)
   << SpecificModuleCachePath << ExistingModuleCachePath;
@@ -802,7 +804,7 @@
   return checkHeaderSearchOptions(HSOpts, SpecificModuleCachePath,
   
PP.getHeaderSearchInfo().getModuleCachePath(),
   Complain ? &Reader.Diags : nullptr,
-  PP.getLangOpts());
+  PP.getLangOpts(), PP.getPreprocessorOpts());
 }
 
 void PCHValidator::ReadCounter(const ModuleFile &M, unsigned Value) {
@@ -5142,8 +5144,8 @@
  StringRef SpecificModuleCachePath,
  bool Complain) override {
   return checkHeaderSearchOptions(HSOpts, SpecificModuleCachePath,
-  ExistingModuleCachePath,
-  nullptr, ExistingLangOpts);
+  ExistingModuleCachePath, nullptr,
+  ExistingLangOpts, ExistingPPOpts);
 }
 
 bool ReadPreprocessorOptions(const PreprocessorOptions &PPOpts,
Index: clang/include/clang/Lex/PreprocessorOptions.h
===
--- clang/include/clang/Lex/PreprocessorOptions.h
+++ clang/include/clang/Lex/PreprocessorOptions.h
@@ -106,6 +106,10 @@
   /// When true, a PCH with compiler errors will not be rejected.
   bool AllowPCHWithCompilerErrors = false;
 
+  /// When true, a PCH with modules cache path different to the current
+  /// compilation will not be rejected.
+  bool AllowPCHWithDifferentModulesCachePath = false;
+
   /// Dump declarations that are deserialized from PCH, for testing.
   bool DumpDeserializedPCHDecls = false;
 
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5499,6 +5499,10 @@
   HelpText<"Accept a PCH file that was created with compiler errors">,
   Marshall

[clang] 9223209 - [clang][deps] Handle precompiled headers' AST files

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T11:28:39+02:00
New Revision: 9223209be11e93c1b701054c6fff88d46ee54658

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

LOG: [clang][deps] Handle precompiled headers' AST files

The `PreprocessOnlyAction` doesn't support loading the AST file of a 
precompiled header. This is problematic for dependency scanning, since the 
`#include` manufactured for the PCH is treated as textual. This means the PCH 
contents get scanned with each TU, which is redundant. Moreover, dependencies 
of the PCH end up being considered dependency of the TU.

To handle AST file of PCH properly, this patch creates new `FrontendAction` 
that behaves the same way `PreprocessorOnlyAction` does, but treats the 
manufactured PCH `#include` as a normal compilation would (by not claiming it 
only uses a preprocessor and creating the default AST consumer).

The AST file is now reported as a file dependency of the TU.

Depends on D103519.

Reviewed By: Bigcheese

Differential Revision: https://reviews.llvm.org/D103524

Added: 


Modified: 
clang/include/clang/Frontend/FrontendActions.h
clang/lib/Frontend/FrontendActions.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/modules-pch.c

Removed: 




diff  --git a/clang/include/clang/Frontend/FrontendActions.h 
b/clang/include/clang/Frontend/FrontendActions.h
index 25ca95980806..ff8d4417eaa4 100644
--- a/clang/include/clang/Frontend/FrontendActions.h
+++ b/clang/include/clang/Frontend/FrontendActions.h
@@ -34,6 +34,17 @@ class InitOnlyAction : public FrontendAction {
   bool usesPreprocessorOnly() const override { return false; }
 };
 
+/// Preprocessor-based frontend action that also loads PCH files.
+class ReadPCHAndPreprocessAction : public FrontendAction {
+  void ExecuteAction() override;
+
+  std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
+ StringRef InFile) override;
+
+public:
+  bool usesPreprocessorOnly() const override { return false; }
+};
+
 class DumpCompilerOptionsAction : public FrontendAction {
   std::unique_ptr CreateASTConsumer(CompilerInstance &CI,
  StringRef InFile) override {

diff  --git a/clang/lib/Frontend/FrontendActions.cpp 
b/clang/lib/Frontend/FrontendActions.cpp
index b3f6cfcf57e3..6df57cbb45ae 100644
--- a/clang/lib/Frontend/FrontendActions.cpp
+++ b/clang/lib/Frontend/FrontendActions.cpp
@@ -62,6 +62,27 @@ InitOnlyAction::CreateASTConsumer(CompilerInstance &CI, 
StringRef InFile) {
 void InitOnlyAction::ExecuteAction() {
 }
 
+// Basically PreprocessOnlyAction::ExecuteAction.
+void ReadPCHAndPreprocessAction::ExecuteAction() {
+  Preprocessor &PP = getCompilerInstance().getPreprocessor();
+
+  // Ignore unknown pragmas.
+  PP.IgnorePragmas();
+
+  Token Tok;
+  // Start parsing the specified input file.
+  PP.EnterMainSourceFile();
+  do {
+PP.Lex(Tok);
+  } while (Tok.isNot(tok::eof));
+}
+
+std::unique_ptr
+ReadPCHAndPreprocessAction::CreateASTConsumer(CompilerInstance &CI,
+  StringRef InFile) {
+  return std::make_unique();
+}
+
 
//===--===//
 // AST Consumer Actions
 
//===--===//

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index ecaeeed1a061..0df515a4c99f 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -73,6 +73,8 @@ class DependencyScanningAction : public tooling::ToolAction {
 if (!Compiler.hasDiagnostics())
   return false;
 
+Compiler.getPreprocessorOpts().AllowPCHWithDifferentModulesCachePath = 
true;
+
 // Use the dependency scanning optimized file system if we can.
 if (DepFS) {
   const CompilerInvocation &CI = Compiler.getInvocation();
@@ -133,7 +135,7 @@ class DependencyScanningAction : public tooling::ToolAction 
{
 // the impact of strict context hashing.
 Compiler.getHeaderSearchOpts().ModulesStrictContextHash = true;
 
-auto Action = std::make_unique();
+auto Action = std::make_unique();
 const bool Result = Compiler.ExecuteAction(*Action);
 if (!DepFS)
   FileMgr->clearStatCache();

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index dfd853822327..3271075d9281 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollec

[PATCH] D103524: [clang][deps] Handle precompiled headers' AST files

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG9223209be11e: [clang][deps] Handle precompiled headers' 
AST files (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D103524?vs=349260&id=351800#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103524

Files:
  clang/include/clang/Frontend/FrontendActions.h
  clang/lib/Frontend/FrontendActions.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/modules-pch.c

Index: clang/test/ClangScanDeps/modules-pch.c
===
--- clang/test/ClangScanDeps/modules-pch.c
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -9,5 +9,52 @@
 // Scan dependencies of the TU:
 //
 // RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_tu.json > %t/cdb.json
+// RUN: echo -%t > %t/result_tu.json
+// FIXME: Make this work with '-mode preprocess-minimized-sources'.
 // RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
-// RUN:   -generate-modules-path-args -module-files-dir %t/build
+// RUN:   -generate-modules-path-args -module-files-dir %t/build -mode preprocess >> %t/result_tu.json
+// RUN: cat %t/result_tu.json | sed 's:\?:/:g' | FileCheck %s -check-prefix=CHECK-TU
+//
+// CHECK-TU:  -[[PREFIX:.*]]
+// CHECK-TU-NEXT: {
+// CHECK-TU-NEXT:   "modules": [
+// CHECK-TU-NEXT: {
+// CHECK-TU-NEXT:   "clang-module-deps": [],
+// CHECK-TU-NEXT:   "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-TU-NEXT:   "command-line": [
+// CHECK-TU-NEXT: "-cc1",
+// CHECK-TU:  "-emit-module",
+// CHECK-TU:  "-fmodule-name=ModTU",
+// CHECK-TU:  "-fno-implicit-modules",
+// CHECK-TU:],
+// CHECK-TU-NEXT:   "context-hash": "[[HASH_MOD_TU:.*]]",
+// CHECK-TU-NEXT:   "file-deps": [
+// CHECK-TU-NEXT: "[[PREFIX]]/mod_tu.h",
+// CHECK-TU-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-TU-NEXT:   ],
+// CHECK-TU-NEXT:   "name": "ModTU"
+// CHECK-TU-NEXT: }
+// CHECK-TU-NEXT:   ],
+// CHECK-TU-NEXT:   "translation-units": [
+// CHECK-TU-NEXT: {
+// CHECK-TU-NEXT:   "clang-context-hash": "[[HASH_TU:.*]]",
+// CHECK-TU-NEXT:   "clang-module-deps": [
+// CHECK-TU-NEXT: {
+// CHECK-TU-NEXT:   "context-hash": "[[HASH_MOD_TU]]",
+// CHECK-TU-NEXT:   "module-name": "ModTU"
+// CHECK-TU-NEXT: }
+// CHECK-TU-NEXT:   ],
+// CHECK-TU-NEXT:   "command-line": [
+// CHECK-TU-NEXT: "-fno-implicit-modules",
+// CHECK-TU-NEXT: "-fno-implicit-module-maps",
+// CHECK-TU-NEXT: "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_TU]]/ModTU-{{.*}}.pcm",
+// CHECK-TU-NEXT: "-fmodule-map-file=[[PREFIX]]/module.modulemap"
+// CHECK-TU-NEXT:   ],
+// CHECK-TU-NEXT:   "file-deps": [
+// CHECK-TU-NEXT: "[[PREFIX]]/tu.c",
+// CHECK-TU-NEXT: "[[PREFIX]]/pch.h.gch"
+// CHECK-TU-NEXT:   ],
+// CHECK-TU-NEXT:   "input-file": "[[PREFIX]]/tu.c"
+// CHECK-TU-NEXT: }
+// CHECK-TU-NEXT:   ]
+// CHECK-TU-NEXT: }
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -156,6 +156,9 @@
   MDC.MainFile = std::string(
   Instance.getSourceManager().getFileEntryForID(MainFileID)->getName());
 
+  if (!Instance.getPreprocessorOpts().ImplicitPCHInclude.empty())
+MDC.FileDeps.push_back(Instance.getPreprocessorOpts().ImplicitPCHInclude);
+
   for (const Module *M : DirectModularDeps)
 handleTopLevelModule(M);
 
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -73,6 +73,8 @@
 if (!Compiler.hasDiagnostics())
   return false;
 
+Compiler.getPreprocessorOpts().AllowPCHWithDifferentModulesCachePath = true;
+
 // Use the dependency scanning optimized file system if we can.
 if (DepFS) {
   const CompilerInvocation &CI = Compiler.getInvocation();
@@ -133,7 +135,7 @@
 // the impact of strict context hashing.
 Compiler.getHeaderSearchOpts().ModulesStrictContextHash = true;
 
-auto Action = std::make_unique();
+auto Action = std::make_unique();
 const bool Result = Compiler.ExecuteAction(*Action);
 if (!DepFS)
   FileMgr->clearStatCache();
Index: clang/lib/Frontend/FrontendActions.cpp

[PATCH] D104209: [clang-format] distinguish function type casts after 21c18d5a04316891110cecc2bf37ce51533decba

[Krasimir Georgiev via Phabricator via cfe-commits]

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

https://github.com/llvm/llvm-project/commit/21c18d5a04316891110cecc2bf37ce51533decba
improved the detection of multiplication in function all argument lists,
but unintentionally regressed the handling of function type casts (there
were no tests covering those).
This patch improves the detection of function type casts and adds a few tests.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104209

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8707,6 +8707,11 @@
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   verifyFormat("void f() { f(float(1), a * a); }");
+
+  verifyFormat("f((void (*)(int))g);");
+  verifyFormat("f((void (&)(int))g);");
+  verifyFormat("f((void (^)(int))g);");
+
   // FIXME: Is there a way to make this work?
   // verifyIndependentOfContext("MACRO(A *a);");
   verifyFormat("MACRO(A &B);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -397,9 +397,13 @@
   !CurrentToken->Next->HasUnescapedNewline &&
   !CurrentToken->Next->isTrailingComment())
 HasMultipleParametersOnALine = true;
+  bool ProbablyFunctionTypeLParen =
+  (CurrentToken->is(tok::l_paren) && CurrentToken->Next &&
+   CurrentToken->Next->isOneOf(tok::star, tok::amp, tok::caret));
   if ((CurrentToken->Previous->isOneOf(tok::kw_const, tok::kw_auto) ||
CurrentToken->Previous->isSimpleTypeSpecifier()) &&
-  !CurrentToken->isOneOf(tok::l_brace, tok::l_paren))
+  !(CurrentToken->is(tok::l_brace) ||
+(CurrentToken->is(tok::l_paren) && !ProbablyFunctionTypeLParen)))
 Contexts.back().IsExpression = false;
   if (CurrentToken->isOneOf(tok::semi, tok::colon)) {
 MightBeObjCForRangeLoop = false;


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -8707,6 +8707,11 @@
   verifyIndependentOfContext("MACRO('0' <= c && c <= '9');");
   verifyFormat("void f() { f(float{1}, a * a); }");
   verifyFormat("void f() { f(float(1), a * a); }");
+
+  verifyFormat("f((void (*)(int))g);");
+  verifyFormat("f((void (&)(int))g);");
+  verifyFormat("f((void (^)(int))g);");
+
   // FIXME: Is there a way to make this work?
   // verifyIndependentOfContext("MACRO(A *a);");
   verifyFormat("MACRO(A &B);");
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -397,9 +397,13 @@
   !CurrentToken->Next->HasUnescapedNewline &&
   !CurrentToken->Next->isTrailingComment())
 HasMultipleParametersOnALine = true;
+  bool ProbablyFunctionTypeLParen =
+  (CurrentToken->is(tok::l_paren) && CurrentToken->Next &&
+   CurrentToken->Next->isOneOf(tok::star, tok::amp, tok::caret));
   if ((CurrentToken->Previous->isOneOf(tok::kw_const, tok::kw_auto) ||
CurrentToken->Previous->isSimpleTypeSpecifier()) &&
-  !CurrentToken->isOneOf(tok::l_brace, tok::l_paren))
+  !(CurrentToken->is(tok::l_brace) ||
+(CurrentToken->is(tok::l_paren) && !ProbablyFunctionTypeLParen)))
 Contexts.back().IsExpression = false;
   if (CurrentToken->isOneOf(tok::semi, tok::colon)) {
 MightBeObjCForRangeLoop = false;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a83ef21 - Fix -Wswitch warning after 092c303955cd18be6c0b923b1c0a1b96e2c91893.

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2021-06-14T11:52:49+02:00
New Revision: a83ef21ff82e4283044fd31470fc6c1bc4b99c51

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

LOG: Fix -Wswitch warning after 092c303955cd18be6c0b923b1c0a1b96e2c91893.

Added: 


Modified: 
clang/lib/Basic/Targets/X86.cpp

Removed: 




diff  --git a/clang/lib/Basic/Targets/X86.cpp b/clang/lib/Basic/Targets/X86.cpp
index 8f7749bd6066..3143a70adf85 100644
--- a/clang/lib/Basic/Targets/X86.cpp
+++ b/clang/lib/Basic/Targets/X86.cpp
@@ -516,6 +516,10 @@ void X86TargetInfo::getTargetDefines(const LangOptions 
&Opts,
   case CK_x86_64:
 defineCPUMacros(Builder, "k8");
 break;
+  case CK_x86_64_v2:
+  case CK_x86_64_v3:
+  case CK_x86_64_v4:
+break;
   case CK_AMDFAM10:
 defineCPUMacros(Builder, "amdfam10");
 break;



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

[clang] 4629554 - [clang][deps] Handle modular dependencies present in PCH

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T11:59:35+02:00
New Revision: 4629554f0b664c94ada7c44fe40855d7a9a39820

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

LOG: [clang][deps] Handle modular dependencies present in PCH

When a translation unit uses a PCH and imports the same modules as the PCH, 
we'd prefer to resolve to those modules instead of inventing new modules and 
reporting them as modular dependencies. Since the PCH modules have already been 
built nudge the compiler to reuse them when deciding whether to build a new 
module and don't report them as regular modular dependencies.

Depends on D103524 & D103802.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D103526

Added: 
clang/test/ClangScanDeps/Inputs/modules-pch/cdb_pch.json
clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu_with_common.json
clang/test/ClangScanDeps/Inputs/modules-pch/mod_common_1.h
clang/test/ClangScanDeps/Inputs/modules-pch/mod_common_2.h
clang/test/ClangScanDeps/Inputs/modules-pch/mod_pch.h
clang/test/ClangScanDeps/Inputs/modules-pch/mod_tu_with_common.h
clang/test/ClangScanDeps/Inputs/modules-pch/tu_with_common.c

Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
clang/test/ClangScanDeps/Inputs/modules-pch/module.modulemap
clang/test/ClangScanDeps/Inputs/modules-pch/pch.h
clang/test/ClangScanDeps/modules-pch.c

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
index 89a70fb723c4..f88dc472c80b 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
@@ -31,6 +31,10 @@ struct FullDependencies {
   /// directly depends on, not including transitive dependencies.
   std::vector FileDeps;
 
+  /// A collection of prebuilt modules this translation unit directly depends
+  /// on, not including transitive dependencies.
+  std::vector PrebuiltModuleDeps;
+
   /// A list of modules this translation unit directly depends on, not 
including
   /// transitive dependencies.
   ///

diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index 689119330c41..e51040d2c0f5 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -37,6 +37,8 @@ class DependencyConsumer {
   virtual void handleFileDependency(const DependencyOutputOptions &Opts,
 StringRef Filename) = 0;
 
+  virtual void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) = 0;
+
   virtual void handleModuleDependency(ModuleDeps MD) = 0;
 
   virtual void handleContextHash(std::string Hash) = 0;

diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h 
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index b8b7882e54bd..73bc41d1f955 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -29,6 +29,18 @@ namespace dependencies {
 
 class DependencyConsumer;
 
+/// Modular dependency that has already been built prior to the dependency 
scan.
+struct PrebuiltModuleDep {
+  std::string ModuleName;
+  std::string PCMFile;
+  std::string ModuleMapFile;
+
+  explicit PrebuiltModuleDep(const Module *M)
+  : ModuleName(M->getTopLevelModuleName()),
+PCMFile(M->getASTFile()->getName()),
+ModuleMapFile(M->PresumedModuleMapFile) {}
+};
+
 /// This is used to identify a specific module.
 struct ModuleID {
   /// The name of the module. This may include `:` for C++20 module partitions,
@@ -74,6 +86,10 @@ struct ModuleDeps {
   /// on, not including transitive dependencies.
   llvm::StringSet<> FileDeps;
 
+  /// A collection of prebuilt modular dependencies this module directly 
depends
+  /// on, not including transitive dependencies.
+  std::vector PrebuiltModuleDeps;
+
   /// A list of module identifiers this module directly depends on, not
   /// including transitive dependencies.
   ///
@@ -150,9 +166,15 @@ class ModuleDepCollectorPP final : public PPCallbacks {
   Modu

[PATCH] D103526: [clang][deps] Handle modular dependencies present in PCH

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4629554f0b66: [clang][deps] Handle modular dependencies 
present in PCH (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D103526?vs=350247&id=351806#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103526

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_pch.json
  clang/test/ClangScanDeps/Inputs/modules-pch/cdb_tu_with_common.json
  clang/test/ClangScanDeps/Inputs/modules-pch/mod_common_1.h
  clang/test/ClangScanDeps/Inputs/modules-pch/mod_common_2.h
  clang/test/ClangScanDeps/Inputs/modules-pch/mod_pch.h
  clang/test/ClangScanDeps/Inputs/modules-pch/mod_tu_with_common.h
  clang/test/ClangScanDeps/Inputs/modules-pch/module.modulemap
  clang/test/ClangScanDeps/Inputs/modules-pch/pch.h
  clang/test/ClangScanDeps/Inputs/modules-pch/tu_with_common.c
  clang/test/ClangScanDeps/modules-pch.c

Index: clang/test/ClangScanDeps/modules-pch.c
===
--- clang/test/ClangScanDeps/modules-pch.c
+++ clang/test/ClangScanDeps/modules-pch.c
@@ -1,10 +1,124 @@
 // RUN: rm -rf %t && mkdir %t
 // RUN: cp %S/Inputs/modules-pch/* %t
 
+// Scan dependencies of the PCH:
+//
+// RUN: sed "s|DIR|%/t|g" %S/Inputs/modules-pch/cdb_pch.json > %t/cdb.json
+// RUN: echo -%t > %t/result_pch.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full \
+// RUN:   -generate-modules-path-args -module-files-dir %t/build -mode preprocess >> %t/result_pch.json
+// RUN: cat %t/result_pch.json | sed 's:\?:/:g' | FileCheck %s -check-prefix=CHECK-PCH
+//
+// CHECK-PCH:  -[[PREFIX:.*]]
+// CHECK-PCH-NEXT: {
+// CHECK-PCH-NEXT:   "modules": [
+// CHECK-PCH-NEXT: {
+// CHECK-PCH-NEXT:   "clang-module-deps": [],
+// CHECK-PCH-NEXT:   "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-PCH-NEXT:   "command-line": [
+// CHECK-PCH-NEXT: "-cc1"
+// CHECK-PCH:  "-emit-module"
+// CHECK-PCH:  "-fmodules"
+// CHECK-PCH:  "-fmodule-name=ModCommon1"
+// CHECK-PCH:  "-fno-implicit-modules"
+// CHECK-PCH:],
+// CHECK-PCH-NEXT:   "context-hash": "[[HASH_MOD_COMMON_1:.*]]",
+// CHECK-PCH-NEXT:   "file-deps": [
+// CHECK-PCH-NEXT: "[[PREFIX]]/mod_common_1.h",
+// CHECK-PCH-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-PCH-NEXT:   ],
+// CHECK-PCH-NEXT:   "name": "ModCommon1"
+// CHECK-PCH-NEXT: },
+// CHECK-PCH-NEXT: {
+// CHECK-PCH-NEXT:   "clang-module-deps": [],
+// CHECK-PCH-NEXT:   "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-PCH-NEXT:   "command-line": [
+// CHECK-PCH-NEXT: "-cc1"
+// CHECK-PCH:  "-emit-module"
+// CHECK-PCH:  "-fmodules"
+// CHECK-PCH:  "-fmodule-name=ModCommon2"
+// CHECK-PCH:  "-fno-implicit-modules"
+// CHECK-PCH:],
+// CHECK-PCH-NEXT:   "context-hash": "[[HASH_MOD_COMMON_2:.*]]",
+// CHECK-PCH-NEXT:   "file-deps": [
+// CHECK-PCH-NEXT: "[[PREFIX]]/mod_common_2.h",
+// CHECK-PCH-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-PCH-NEXT:   ],
+// CHECK-PCH-NEXT:   "name": "ModCommon2"
+// CHECK-PCH-NEXT: },
+// CHECK-PCH-NEXT: {
+// CHECK-PCH-NEXT:   "clang-module-deps": [
+// CHECK-PCH-NEXT: {
+// CHECK-PCH-NEXT:   "context-hash": "[[HASH_MOD_COMMON_2]]",
+// CHECK-PCH-NEXT:   "module-name": "ModCommon2"
+// CHECK-PCH-NEXT: }
+// CHECK-PCH-NEXT:   ],
+// CHECK-PCH-NEXT:   "clang-modulemap-file": "[[PREFIX]]/module.modulemap",
+// CHECK-PCH-NEXT:   "command-line": [
+// CHECK-PCH-NEXT: "-cc1"
+// CHECK-PCH:  "-fmodule-map-file=[[PREFIX]]/module.modulemap"
+// CHECK-PCH:  "-emit-module"
+// CHECK-PCH:  "-fmodule-file=[[PREFIX]]/build/[[HASH_MOD_COMMON_2]]/ModCommon2-{{.*}}.pcm"
+// CHECK-PCH:  "-fmodules"
+// CHECK-PCH:  "-fmodule-name=ModPCH"
+// CHECK-PCH:  "-fno-implicit-modules"
+// CHECK-PCH:],
+// CHECK-PCH-NEXT:   "context-hash": "[[HASH_MOD_PCH:.*]]",
+// CHECK-PCH-NEXT:   "file-deps": [
+// CHECK-PCH-NEXT: "[[PREFIX]]/mod_pch.h",
+// CHECK-PCH-NEXT: "[[PREFIX]]/module.modulemap"
+// CHECK-PCH-NEXT:   ],
+// CHECK-PCH-NEXT:

[PATCH] D104044: [clang-format] Fix the issue that empty lines being removed at the beginning of namespace

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

I think we can agree its complicated, how about you take your unit tests and 
show us what the "code change" looks like to fix the bug

If that gets overly convoluted then perhaps we can bring the idea forward of a 
more generalised approach.


Repository:
  rZORG LLVM Github Zorg

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

https://reviews.llvm.org/D104044

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

[PATCH] D43957: Fixing issue where a space was not added before a global namespace variable when SpacesInParentheses is set

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

This issue was resolved by 
https://github.com/llvm/llvm-project/commit/d5e9ff4fe20e66d53a245645c95f0bb816b747cb
 could you please close out this review


Repository:
  rC Clang

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

https://reviews.llvm.org/D43957

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

[clang] 8ddbb44 - [Analyzer][solver] Simplify existing eq classes and constraints when a new constraint is added

[Gabor Marton via cfe-commits]

Author: Gabor Marton
Date: 2021-06-14T12:19:09+02:00
New Revision: 8ddbb442b6e87efc9c6599280740c6f4fc40963d

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

LOG: [Analyzer][solver] Simplify existing eq classes and constraints when a new 
constraint is added

Update `setConstraint` to simplify existing equivalence classes when a
new constraint is added. In this patch we iterate over all existing
equivalence classes and constraints and try to simplfy them with
simplifySVal. This solves problematic cases where we have two symbols in
the tree, e.g.:
```
int test_rhs_further_constrained(int x, int y) {
  if (x + y != 0)
return 0;
  if (y != 0)
return 0;
  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
  return 0;
}
```

Differential Revision: https://reviews.llvm.org/D103314

Added: 
clang/test/Analysis/find-binop-constraints.cpp

Modified: 

clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Removed: 




diff  --git 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
index 4a118074463d..a4957c697c0a 100644
--- 
a/clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
+++ 
b/clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
@@ -256,7 +256,7 @@ class RangeSet {
   ///  by FoldingSet.
   void Profile(llvm::FoldingSetNodeID &ID) const { Profile(ID, *this); }
 
-  /// getConcreteValue - If a symbol is contrained to equal a specific integer
+  /// getConcreteValue - If a symbol is constrained to equal a specific integer
   ///  constant then this method returns that value.  Otherwise, it returns
   ///  NULL.
   const llvm::APSInt *getConcreteValue() const {

diff  --git a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp 
b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
index 1088565d1a23..ac7767f0d3c4 100644
--- a/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ b/clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ImmutableSet.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/StringExtras.h"
+#include "llvm/ADT/SmallSet.h"
 #include "llvm/Support/Compiler.h"
 #include "llvm/Support/raw_ostream.h"
 #include 
@@ -582,9 +583,17 @@ class EquivalenceClass : public llvm::FoldingSetNode {
   LLVM_NODISCARD inline ClassSet
   getDisequalClasses(DisequalityMapTy Map, ClassSet::Factory &Factory) const;
 
+  LLVM_NODISCARD static inline Optional areEqual(ProgramStateRef State,
+   EquivalenceClass First,
+   EquivalenceClass 
Second);
   LLVM_NODISCARD static inline Optional
   areEqual(ProgramStateRef State, SymbolRef First, SymbolRef Second);
 
+  /// Iterate over all symbols and try to simplify them.
+  LLVM_NODISCARD ProgramStateRef simplify(SValBuilder &SVB,
+  RangeSet::Factory &F,
+  ProgramStateRef State);
+
   /// Check equivalence data for consistency.
   LLVM_NODISCARD LLVM_ATTRIBUTE_UNUSED static bool
   isClassDataConsistent(ProgramStateRef State);
@@ -1375,6 +1384,12 @@ RangeSet 
SymbolicRangeInferrer::VisitBinaryOperator(Range LHS,
 //  Constraint manager implementation details
 
//===--===//
 
+static SymbolRef simplify(ProgramStateRef State, SymbolRef Sym) {
+  SValBuilder &SVB = State->getStateManager().getSValBuilder();
+  SVal SimplifiedVal = SVB.simplifySVal(State, SVB.makeSymbolVal(Sym));
+  return SimplifiedVal.getAsSymbol();
+}
+
 class RangeConstraintManager : public RangedConstraintManager {
 public:
   RangeConstraintManager(ExprEngine *EE, SValBuilder &SVB)
@@ -1488,6 +1503,9 @@ class RangeConstraintManager : public 
RangedConstraintManager {
   // This is an infeasible assumption.
   return nullptr;
 
+if (SymbolRef SimplifiedSym = simplify(State, Sym))
+  Sym = SimplifiedSym;
+
 if (ProgramStateRef NewState = setConstraint(State, Sym, NewConstraint)) {
   if (auto Equality = EqualityInfo::extract(Sym, Int, Adjustment)) {
 // If the original assumption is not Sym + Adjustment !=/ Int,
@@ -1554,9 +1572,47 @@ class RangeConstraintManager : public 
RangedConstraintManager {
 return State->set(Constraints);
   }
 
+  // Associate a constraint to a symbolic expression. First, we set the
+  // constraint in the State, then we try to simplify existing symbol

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

[Gabor Marton via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG8ddbb442b6e8: [Analyzer][solver] Simplify existing eq 
classes and constraints when a new… (authored by martong).

Changed prior to commit:
  https://reviews.llvm.org/D103314?vs=350966&id=351809#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

Files:
  
clang/include/clang/StaticAnalyzer/Core/PathSensitive/RangedConstraintManager.h
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/find-binop-constraints.cpp

Index: clang/test/Analysis/find-binop-constraints.cpp
===
--- /dev/null
+++ clang/test/Analysis/find-binop-constraints.cpp
@@ -0,0 +1,163 @@
+// RUN: %clang_analyze_cc1 %s \
+// RUN:   -analyzer-checker=core \
+// RUN:   -analyzer-checker=debug.ExprInspection \
+// RUN:   -analyzer-config eagerly-assume=false \
+// RUN:   -verify
+
+void clang_analyzer_eval(bool);
+void clang_analyzer_warnIfReached();
+
+int test_legacy_behavior(int x, int y) {
+  if (y != 0)
+return 0;
+  if (x + y != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return y / (x + y);  // expected-warning{{Division by zero}}
+}
+
+int test_rhs_further_constrained(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (y != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_lhs_further_constrained(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (x != 0)
+return 0;
+  clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_lhs_and_rhs_further_constrained(int x, int y) {
+  if (x % y != 1)
+return 0;
+  if (x != 1)
+return 0;
+  if (y != 2)
+return 0;
+  clang_analyzer_eval(x % y == 1); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 2); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_commutativity(int x, int y) {
+  if (x + y != 0)
+return 0;
+  if (y != 0)
+return 0;
+  clang_analyzer_eval(y + x == 0); // expected-warning{{TRUE}}
+  clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+  return 0;
+}
+
+int test_binop_when_height_is_2_r(int a, int x, int y, int z) {
+  switch (a) {
+  case 1: {
+if (x + y + z != 0)
+  return 0;
+if (z != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 2: {
+if (x + y + z != 0)
+  return 0;
+if (y != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 3: {
+if (x + y + z != 0)
+  return 0;
+if (x != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 4: {
+if (x + y + z != 0)
+  return 0;
+if (x + y != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x + y == 0); // expected-warning{{TRUE}}
+break;
+  }
+  case 5: {
+if (z != 0)
+  return 0;
+if (x + y + z != 0)
+  return 0;
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+if (y != 0)
+  return 0;
+clang_analyzer_eval(y == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(z == 0); // expected-warning{{TRUE}}
+clang_analyzer_eval(x + y + z == 0); // expected-warning{{TRUE}}
+break;
+  }
+
+  }
+  return 0;
+}
+
+void test_equivalence_classes_are_updated(int a, int b, int c, int d) {
+  if (a + b != c)
+return;
+  if (a != d)
+return;
+  if (b != 0)
+return;
+  clang_analyzer_eval(c == d); // expected-warning{{TRUE}}
+  // Keep the symbols and the constraints! alive.
+  (void)(a * b * c * d);
+  return;
+}
+
+void test_contradiction(int a, int b, int c, int d) {
+  if (a + b != c)
+return;
+  if (a == c)
+return;
+  clang_analyzer_warnIfReached(); // expected-warning{{REACHABLE}}
+
+  // Bring in the contradiction.
+  if (b != 0)
+return;
+  clang_analyzer_warnIfReached(); // no-warning, i.e. UNREACHABLE
+  // Keep the symbols and the constraints! alive.
+  (void)(a * b * c * d);
+  return;
+}
+
+void test_deferred_contradiction(int e0, int b0, int b1) {
+
+  int e1 = e0 - b0; // e1 is bound to (reg_$0) - (reg_$1)
+  (void)(b0 == 2);  // bifurcate
+
+  int e2 = e1 - b1;
+  if (e2 > 0) { // b1 != e1
+clang_anal

[PATCH] D103314: [Analyzer][solver] Simplify existing constraints when a new constraint is added

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

> This patch is the first step of a sequence of patches, and not intended to be 
> commited as a standalone change.

Although I planned to commit this in a lock-step when subsequent patches are 
also accepted, it makes sense to commit now since it's an obvious improvement 
and the performance penalty remains below a reasonable limit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103314

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

[clang] 35fa3e6 - [clang][deps] Move stripping of diagnostic serialization from `clang-scan-deps` to `DependencyScanning` library

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T12:23:32+02:00
New Revision: 35fa3e60d1612dcc4f8e233b046423d948ca9a9b

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

LOG: [clang][deps] Move stripping of diagnostic serialization from 
`clang-scan-deps` to `DependencyScanning` library

To prevent the creation of diagnostics file, `clang-scan-deps` strips the 
corresponding command-line argument. This behavior is useful even when using 
the C++ `DependencyScanner` library.

This patch transforms stripping of command-line in `clang-scan-deps` into 
stripping of `CompilerInvocation` in `DependencyScanning`.

AFAIK, the `clang-cl` driver doesn't even accept `--serialize-diagnostics`, so 
I've removed the test. (It would fail with an unknown command-line argument 
otherwise.)

Note: Since we're generating command-lines for modular dependencies from 
`CompilerInvocation`, the `--serialize-diagnostics` will be dropped. This was 
already happening in `clang-scan-deps` before this patch, but it will now 
happen also when using `DependencyScanning` library directly. This is resolved 
in D104036.

Reviewed By: dexonsmith, arphaman

Differential Revision: https://reviews.llvm.org/D104012

Added: 


Modified: 
clang/include/clang/Tooling/ArgumentsAdjusters.h
clang/lib/Tooling/ArgumentsAdjusters.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/test/ClangScanDeps/Inputs/strip_diag_serialize.json
clang/test/ClangScanDeps/strip_diag_serialize.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git a/clang/include/clang/Tooling/ArgumentsAdjusters.h 
b/clang/include/clang/Tooling/ArgumentsAdjusters.h
index c48a8725aae9..bf0886034324 100644
--- a/clang/include/clang/Tooling/ArgumentsAdjusters.h
+++ b/clang/include/clang/Tooling/ArgumentsAdjusters.h
@@ -43,10 +43,6 @@ ArgumentsAdjuster getClangSyntaxOnlyAdjuster();
 /// arguments.
 ArgumentsAdjuster getClangStripOutputAdjuster();
 
-/// Gets an argument adjuster which removes command line arguments related to
-/// diagnostic serialization.
-ArgumentsAdjuster getClangStripSerializeDiagnosticAdjuster();
-
 /// Gets an argument adjuster which removes dependency-file
 /// related command line arguments.
 ArgumentsAdjuster getClangStripDependencyFileAdjuster();

diff  --git a/clang/lib/Tooling/ArgumentsAdjusters.cpp 
b/clang/lib/Tooling/ArgumentsAdjusters.cpp
index d94673bd2ab9..7f5dc4d62f11 100644
--- a/clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ b/clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -86,22 +86,6 @@ ArgumentsAdjuster getClangStripOutputAdjuster() {
   };
 }
 
-ArgumentsAdjuster getClangStripSerializeDiagnosticAdjuster() {
-  return [](const CommandLineArguments &Args, StringRef /*unused*/) {
-CommandLineArguments AdjustedArgs;
-for (size_t i = 0, e = Args.size(); i < e; ++i) {
-  StringRef Arg = Args[i];
-  if (Arg == "--serialize-diagnostics") {
-// Skip the diagnostic output argument.
-++i;
-continue;
-  }
-  AdjustedArgs.push_back(Args[i]);
-}
-return AdjustedArgs;
-  };
-}
-
 ArgumentsAdjuster getClangStripDependencyFileAdjuster() {
   return [](const CommandLineArguments &Args, StringRef /*unused*/) {
 auto UsingClDriver = (getDriverMode(Args) == "cl");

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index ebcd16e5ab6f..40466b00f0e5 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -85,6 +85,8 @@ class DependencyScanningAction : public tooling::ToolAction {
 
 // Don't print 'X warnings and Y errors generated'.
 Compiler.getDiagnosticOpts().ShowCarets = false;
+// Don't write out diagnostic file.
+Compiler.getDiagnosticOpts().DiagnosticSerializationFile.clear();
 // Create the compiler's actual diagnostics engine.
 Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
 if (!Compiler.hasDiagnostics())

diff  --git a/clang/test/ClangScanDeps/Inputs/strip_diag_serialize.json 
b/clang/test/ClangScanDeps/Inputs/strip_diag_serialize.json
index 7af1acdc378a..a774d95a3b02 100644
--- a/clang/test/ClangScanDeps/Inputs/strip_diag_serialize.json
+++ b/clang/test/ClangScanDeps/Inputs/strip_diag_serialize.json
@@ -3,10 +3,5 @@
   "directory": "DIR",
   "command": "clang -E -fsyntax-only DIR/strip_diag_serialize_input.cpp 
--serialize-diagnostics /does/not/exist",
   "file": "DIR/strip_diag_serialize_input.cpp"
-},
-{
-  "directory": "DIR",
-  "command": "clang-cl /E --serialize-diagnostics A:/does/not/exist -- 
DIR/strip_diag_serialize_input_clangcl.cpp",
-  "file": "DIR/strip_diag_serialize_i

[PATCH] D104012: [clang][deps] Move stripping of diagnostic serialization from `clang-scan-deps` to `DependencyScanning` library

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG35fa3e60d161: [clang][deps] Move stripping of diagnostic 
serialization from `clang-scan-deps`… (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104012

Files:
  clang/include/clang/Tooling/ArgumentsAdjusters.h
  clang/lib/Tooling/ArgumentsAdjusters.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/test/ClangScanDeps/Inputs/strip_diag_serialize.json
  clang/test/ClangScanDeps/strip_diag_serialize.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -571,8 +571,6 @@
 AdjustedArgs.insert(AdjustedArgs.end(), FlagsEnd, Args.end());
 return AdjustedArgs;
   });
-  AdjustingCompilations->appendArgumentsAdjuster(
-  tooling::getClangStripSerializeDiagnosticAdjuster());
 
   SharedStream Errs(llvm::errs());
   // Print out the dependency results to STDOUT by default.
Index: clang/test/ClangScanDeps/strip_diag_serialize.cpp
===
--- clang/test/ClangScanDeps/strip_diag_serialize.cpp
+++ clang/test/ClangScanDeps/strip_diag_serialize.cpp
@@ -8,6 +8,5 @@
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 1 2>&1 | FileCheck %s
 // CHECK-NOT: unable to open file
 // CHECK: strip_diag_serialize_input.cpp
-// CHECK: strip_diag_serialize_input_clangcl.cpp
 
 #warning "diagnostic"
Index: clang/test/ClangScanDeps/Inputs/strip_diag_serialize.json
===
--- clang/test/ClangScanDeps/Inputs/strip_diag_serialize.json
+++ clang/test/ClangScanDeps/Inputs/strip_diag_serialize.json
@@ -3,10 +3,5 @@
   "directory": "DIR",
   "command": "clang -E -fsyntax-only DIR/strip_diag_serialize_input.cpp 
--serialize-diagnostics /does/not/exist",
   "file": "DIR/strip_diag_serialize_input.cpp"
-},
-{
-  "directory": "DIR",
-  "command": "clang-cl /E --serialize-diagnostics A:/does/not/exist -- 
DIR/strip_diag_serialize_input_clangcl.cpp",
-  "file": "DIR/strip_diag_serialize_input_clangcl.cpp"
 }
 ]
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -85,6 +85,8 @@
 
 // Don't print 'X warnings and Y errors generated'.
 Compiler.getDiagnosticOpts().ShowCarets = false;
+// Don't write out diagnostic file.
+Compiler.getDiagnosticOpts().DiagnosticSerializationFile.clear();
 // Create the compiler's actual diagnostics engine.
 Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
 if (!Compiler.hasDiagnostics())
Index: clang/lib/Tooling/ArgumentsAdjusters.cpp
===
--- clang/lib/Tooling/ArgumentsAdjusters.cpp
+++ clang/lib/Tooling/ArgumentsAdjusters.cpp
@@ -86,22 +86,6 @@
   };
 }
 
-ArgumentsAdjuster getClangStripSerializeDiagnosticAdjuster() {
-  return [](const CommandLineArguments &Args, StringRef /*unused*/) {
-CommandLineArguments AdjustedArgs;
-for (size_t i = 0, e = Args.size(); i < e; ++i) {
-  StringRef Arg = Args[i];
-  if (Arg == "--serialize-diagnostics") {
-// Skip the diagnostic output argument.
-++i;
-continue;
-  }
-  AdjustedArgs.push_back(Args[i]);
-}
-return AdjustedArgs;
-  };
-}
-
 ArgumentsAdjuster getClangStripDependencyFileAdjuster() {
   return [](const CommandLineArguments &Args, StringRef /*unused*/) {
 auto UsingClDriver = (getDriverMode(Args) == "cl");
Index: clang/include/clang/Tooling/ArgumentsAdjusters.h
===
--- clang/include/clang/Tooling/ArgumentsAdjusters.h
+++ clang/include/clang/Tooling/ArgumentsAdjusters.h
@@ -43,10 +43,6 @@
 /// arguments.
 ArgumentsAdjuster getClangStripOutputAdjuster();
 
-/// Gets an argument adjuster which removes command line arguments related to
-/// diagnostic serialization.
-ArgumentsAdjuster getClangStripSerializeDiagnosticAdjuster();
-
 /// Gets an argument adjuster which removes dependency-file
 /// related command line arguments.
 ArgumentsAdjuster getClangStripDependencyFileAdjuster();


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -571,8 +571,6 @@
 AdjustedArgs.insert(AdjustedArgs.end(),

[clang] d8bab69 - [clang][deps] Move invocation adjustments from `clang-scan-deps` to `DependencyScanning` library

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T12:23:33+02:00
New Revision: d8bab69ead22a10dc4cdb2e36f6ea6fdfe774e2e

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

LOG: [clang][deps] Move invocation adjustments from `clang-scan-deps` to 
`DependencyScanning` library

The `clang-scan-deps` tool has some logic that parses and modifies the original 
Clang command-line. The goal is to setup `DependencyOutputOptions` by injecting 
`-M -MT ` and prevent the creation of output files.

This patch moves the logic into the `DependencyScanning` library, and uses the 
parsed `CompilerInvocation` instead of the raw command-line. The code simpler 
and can be used from the C++ API as well.

The `-o /dev/null` arguments are not necessary, since the `DependencyScanning` 
library only runs a preprocessing action, so there's no way it'll produce an 
actual object file.

Related: The `-M` argument implies `-w`, which would appear on the command-line 
of modular dependencies even though it was not on the original TU command line 
(see D104036).

Some related tests were updated.

Reviewed By: arphaman

Differential Revision: https://reviews.llvm.org/D104030

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/test/ClangScanDeps/modules-pch.c
clang/test/ClangScanDeps/modules.cpp
clang/test/ClangScanDeps/regular_cdb.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 40466b00f0e5..e9392ee824a6 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -62,6 +62,26 @@ class ImportCollectingListener : public ASTReaderListener {
   std::map &PrebuiltModuleFiles;
 };
 
+/// Transform arbitrary file name into an object-like file name.
+static std::string makeObjFileName(StringRef FileName) {
+  SmallString<128> ObjFileName(FileName);
+  llvm::sys::path::replace_extension(ObjFileName, "o");
+  return std::string(ObjFileName.str());
+}
+
+/// Deduce the dependency target based on the output file and input files.
+static std::string
+deduceDepTarget(const std::string &OutputFile,
+const SmallVectorImpl &InputFiles) {
+  if (OutputFile != "-")
+return OutputFile;
+
+  if (InputFiles.empty() || !InputFiles.front().isFile())
+return "clang-scan-deps\\ dependency";
+
+  return makeObjFileName(InputFiles.front().getFile());
+}
+
 /// A clang tool that runs the preprocessor in a mode that's optimized for
 /// dependency scanning for the given compiler invocation.
 class DependencyScanningAction : public tooling::ToolAction {
@@ -150,9 +170,11 @@ class DependencyScanningAction : public 
tooling::ToolAction {
 // and thus won't write out the extra '.d' files to disk.
 auto Opts = std::make_unique(
 std::move(Compiler.getInvocation().getDependencyOutputOpts()));
-// We need at least one -MT equivalent for the generator to work.
+// We need at least one -MT equivalent for the generator of make dependency
+// files to work.
 if (Opts->Targets.empty())
-  Opts->Targets = {"clang-scan-deps dependency"};
+  Opts->Targets = {deduceDepTarget(Compiler.getFrontendOpts().OutputFile,
+   Compiler.getFrontendOpts().Inputs)};
 
 switch (Format) {
 case ScanningOutputFormat::Make:

diff  --git a/clang/test/ClangScanDeps/modules-pch.c 
b/clang/test/ClangScanDeps/modules-pch.c
index 34a0043acd5c..8b8b75310adb 100644
--- a/clang/test/ClangScanDeps/modules-pch.c
+++ b/clang/test/ClangScanDeps/modules-pch.c
@@ -9,6 +9,9 @@
 // RUN:   -generate-modules-path-args -module-files-dir %t/build -mode 
preprocess >> %t/result_pch.json
 // RUN: cat %t/result_pch.json | sed 's:\?:/:g' | FileCheck %s 
-check-prefix=CHECK-PCH
 //
+// Check we didn't build the PCH during dependency scanning.
+// RUN: not cat %/t/pch.h.gch
+//
 // CHECK-PCH:  -[[PREFIX:.*]]
 // CHECK-PCH-NEXT: {
 // CHECK-PCH-NEXT:   "modules": [

diff  --git a/clang/test/ClangScanDeps/modules.cpp 
b/clang/test/ClangScanDeps/modules.cpp
index f0d97dc0c5c2..b7daf51b4a80 100644
--- a/clang/test/ClangScanDeps/modules.cpp
+++ b/clang/test/ClangScanDeps/modules.cpp
@@ -42,13 +42,14 @@
 
 #include "header.h"
 
-// CHECK1: modules_cdb_input2.cpp
+// CHECK1: modules_cdb_input2.o:
 // CHECK1-NEXT: modules_cdb_input2.cpp
 // CHECK1-NEXT: Inputs{{/|\\}}module.modulemap
 // CHECK1-NEXT: Inputs{{/|\\}}header2.h
 // CHECK1: Inputs{{/|\\}}header.h
 
-// CHECK2: modules_cdb_input.cpp
+// CHECK2: {{.*}}.o:
+// CHECK2-NEXT: modules_cdb_input.cpp
 // CHECK2-NEXT: Inputs{{/|\\}}modu

[clang] cf7d970 - [clang][deps] Move injection of `-Wno-error` from `clang-scan-deps` to `DependencyScanning` library

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T12:23:33+02:00
New Revision: cf7d9704688db746b26d739a6e154ad54b6b676e

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

LOG: [clang][deps] Move injection of `-Wno-error` from `clang-scan-deps` to 
`DependencyScanning` library

This moves another piece of logic specific to `clang-scan-deps` into the 
`DependencyScanning` library. This makes it easier to check how the original 
command-line looked like in the library and will enable the library to stop 
inventing `-Wno-error` for modular dependencies (see D104036).

Reviewed By: arphaman

Differential Revision: https://reviews.llvm.org/D104031

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index e9392ee824a6..0011a5672bf2 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -107,6 +107,8 @@ class DependencyScanningAction : public tooling::ToolAction 
{
 Compiler.getDiagnosticOpts().ShowCarets = false;
 // Don't write out diagnostic file.
 Compiler.getDiagnosticOpts().DiagnosticSerializationFile.clear();
+// Don't treat warnings as errors.
+Compiler.getDiagnosticOpts().Warnings.push_back("no-error");
 // Create the compiler's actual diagnostics engine.
 Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
 if (!Compiler.hasDiagnostics())

diff  --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index d9f1b586d71a..8a1575083d8e 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -504,7 +504,6 @@ int main(int argc, const char **argv) {
 }
 AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
-AdjustedArgs.push_back("-Wno-error");
 
 if (!HasResourceDir) {
   StringRef ResourceDir =



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

[PATCH] D104030: [clang][deps] Move invocation adjustments from `clang-scan-deps` to `DependencyScanning` library

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGd8bab69ead22: [clang][deps] Move invocation adjustments from 
`clang-scan-deps` to… (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D104030?vs=351164&id=351811#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104030

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/test/ClangScanDeps/modules-pch.c
  clang/test/ClangScanDeps/modules.cpp
  clang/test/ClangScanDeps/regular_cdb.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -201,13 +201,6 @@
 
 } // end anonymous namespace
 
-/// \returns object-file path derived from source-file path.
-static std::string getObjFilePath(StringRef SrcFile) {
-  SmallString<128> ObjFileName(SrcFile);
-  llvm::sys::path::replace_extension(ObjFileName, "o");
-  return std::string(ObjFileName.str());
-}
-
 class SingleCommandCompilationDatabase : public tooling::CompilationDatabase {
 public:
   SingleCommandCompilationDatabase(tooling::CompileCommand Cmd)
@@ -463,16 +456,10 @@
   std::move(Compilations));
   ResourceDirectoryCache ResourceDirCache;
 
-  // FIXME: Adjust the resulting CompilerInvocation in DependencyScanningAction
-  // instead of parsing and adjusting the raw command-line. This will make it
-  // possible to remove some code specific to clang-cl and Windows.
   AdjustingCompilations->appendArgumentsAdjuster(
   [&ResourceDirCache](const tooling::CommandLineArguments &Args,
   StringRef FileName) {
 std::string LastO = "";
-bool HasMT = false;
-bool HasMQ = false;
-bool HasMD = false;
 bool HasResourceDir = false;
 bool ClangCLMode = false;
 auto FlagsEnd = llvm::find(Args, "--");
@@ -503,58 +490,17 @@
 if (!LastO.empty() && !llvm::sys::path::has_extension(LastO))
   LastO.append(".obj");
   }
-  if (Arg == "/clang:-MT")
-HasMT = true;
-  if (Arg == "/clang:-MQ")
-HasMQ = true;
-  if (Arg == "/clang:-MD")
-HasMD = true;
-} else {
-  if (LastO.empty()) {
-if (Arg == "-o" && I != R)
-  LastO = I[-1]; // Next argument (reverse iterator)
-else if (Arg.startswith("-o"))
-  LastO = Arg.drop_front(2).str();
-  }
-  if (Arg == "-MT")
-HasMT = true;
-  if (Arg == "-MQ")
-HasMQ = true;
-  if (Arg == "-MD")
-HasMD = true;
 }
 if (Arg == "-resource-dir")
   HasResourceDir = true;
   }
 }
-// If there's no -MT/-MQ Driver would add -MT with the value of the last
-// -o option.
 tooling::CommandLineArguments AdjustedArgs(Args.begin(), FlagsEnd);
-AdjustedArgs.push_back("-o");
-#ifdef _WIN32
-AdjustedArgs.push_back("nul");
-#else
-AdjustedArgs.push_back("/dev/null");
-#endif
-if (!HasMT && !HasMQ && Format == ScanningOutputFormat::Make) {
-  // We're interested in source dependencies of an object file.
-  std::string FileNameArg;
-  if (!HasMD) {
-// FIXME: We are missing the directory unless the -o value is an
-// absolute path.
-FileNameArg = !LastO.empty() ? LastO : getObjFilePath(FileName);
-  } else {
-FileNameArg = std::string(FileName);
-  }
-  if (ClangCLMode) {
-AdjustedArgs.push_back("/clang:-M");
-AdjustedArgs.push_back("/clang:-MT");
-AdjustedArgs.push_back(Twine("/clang:", FileNameArg).str());
-  } else {
-AdjustedArgs.push_back("-M");
-AdjustedArgs.push_back("-MT");
-AdjustedArgs.push_back(std::move(FileNameArg));
-  }
+// The clang-cl driver passes "-o -" to the frontend. Inject the real
+// file here to ensure "-MT" can be deduced if need be.
+if (ClangCLMode && !LastO.empty()) {
+  AdjustedArgs.push_back("/clang:-o");
+  AdjustedArgs.push_back("/clang:" + LastO);
 }
 AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
Index: clang/test/ClangScanDeps/regular_cdb.cpp
===
--- clang/test/ClangScanDeps/regular_cdb.cpp
+++ clang/test/ClangScanDeps/regular_cdb.cpp
@@ -58,14 +58,14 @@
 
 #include "header.h"
 
-//

[clang] 6c6dcfc - [clang][deps] Move enabling system header deps from `clang-scan-deps` to `DependencyScanning` library

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T12:23:33+02:00
New Revision: 6c6dcfc4ce750bb7dc15d0a4ad631c66beed70d4

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

LOG: [clang][deps] Move enabling system header deps from `clang-scan-deps` to 
`DependencyScanning` library

This patch moves enabling system header deps from `clang-scan-deps` into the 
`DependencyScanning` library. This will make it easier to preserve semantics of 
the original TU command-line for modular dependencies (see D104036).

Reviewed By: arphaman

Differential Revision: https://reviews.llvm.org/D104033

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/tools/clang-scan-deps/ClangScanDeps.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 0011a5672bf2..a2f9b1c0e074 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -177,6 +177,7 @@ class DependencyScanningAction : public tooling::ToolAction 
{
 if (Opts->Targets.empty())
   Opts->Targets = {deduceDepTarget(Compiler.getFrontendOpts().OutputFile,
Compiler.getFrontendOpts().Inputs)};
+Opts->IncludeSystemHeaders = true;
 
 switch (Format) {
 case ScanningOutputFormat::Make:

diff  --git a/clang/tools/clang-scan-deps/ClangScanDeps.cpp 
b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
index 8a1575083d8e..49c475768662 100644
--- a/clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ b/clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -502,8 +502,6 @@ int main(int argc, const char **argv) {
   AdjustedArgs.push_back("/clang:-o");
   AdjustedArgs.push_back("/clang:" + LastO);
 }
-AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-sys-header-deps");
 
 if (!HasResourceDir) {
   StringRef ResourceDir =



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

[PATCH] D104031: [clang][deps] Move injection of `-Wno-error` from `clang-scan-deps` to `DependencyScanning` library

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGcf7d9704688d: [clang][deps] Move injection of `-Wno-error` 
from `clang-scan-deps` to… (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104031

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -504,7 +504,6 @@
 }
 AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
-AdjustedArgs.push_back("-Wno-error");
 
 if (!HasResourceDir) {
   StringRef ResourceDir =
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -107,6 +107,8 @@
 Compiler.getDiagnosticOpts().ShowCarets = false;
 // Don't write out diagnostic file.
 Compiler.getDiagnosticOpts().DiagnosticSerializationFile.clear();
+// Don't treat warnings as errors.
+Compiler.getDiagnosticOpts().Warnings.push_back("no-error");
 // Create the compiler's actual diagnostics engine.
 Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
 if (!Compiler.hasDiagnostics())


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -504,7 +504,6 @@
 }
 AdjustedArgs.push_back("-Xclang");
 AdjustedArgs.push_back("-sys-header-deps");
-AdjustedArgs.push_back("-Wno-error");
 
 if (!HasResourceDir) {
   StringRef ResourceDir =
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -107,6 +107,8 @@
 Compiler.getDiagnosticOpts().ShowCarets = false;
 // Don't write out diagnostic file.
 Compiler.getDiagnosticOpts().DiagnosticSerializationFile.clear();
+// Don't treat warnings as errors.
+Compiler.getDiagnosticOpts().Warnings.push_back("no-error");
 // Create the compiler's actual diagnostics engine.
 Compiler.createDiagnostics(DiagConsumer, /*ShouldOwnClient=*/false);
 if (!Compiler.hasDiagnostics())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104033: [clang][deps] Move enabling system header deps from `clang-scan-deps` to `DependencyScanning` library

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c6dcfc4ce75: [clang][deps] Move enabling system header deps 
from `clang-scan-deps` to… (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104033

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -502,8 +502,6 @@
   AdjustedArgs.push_back("/clang:-o");
   AdjustedArgs.push_back("/clang:" + LastO);
 }
-AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-sys-header-deps");
 
 if (!HasResourceDir) {
   StringRef ResourceDir =
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -177,6 +177,7 @@
 if (Opts->Targets.empty())
   Opts->Targets = {deduceDepTarget(Compiler.getFrontendOpts().OutputFile,
Compiler.getFrontendOpts().Inputs)};
+Opts->IncludeSystemHeaders = true;
 
 switch (Format) {
 case ScanningOutputFormat::Make:


Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -502,8 +502,6 @@
   AdjustedArgs.push_back("/clang:-o");
   AdjustedArgs.push_back("/clang:" + LastO);
 }
-AdjustedArgs.push_back("-Xclang");
-AdjustedArgs.push_back("-sys-header-deps");
 
 if (!HasResourceDir) {
   StringRef ResourceDir =
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -177,6 +177,7 @@
 if (Opts->Targets.empty())
   Opts->Targets = {deduceDepTarget(Compiler.getFrontendOpts().OutputFile,
Compiler.getFrontendOpts().Inputs)};
+Opts->IncludeSystemHeaders = true;
 
 switch (Format) {
 case ScanningOutputFormat::Make:
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1d8882b - [clang][deps] Fix failing modules.cpp test

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T12:55:56+02:00
New Revision: 1d8882b5e44e577f226b7a5a83c27df3b16b3ab6

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

LOG: [clang][deps] Fix failing modules.cpp test

Commit d8bab69ead22a10dc4cdb2e36f6ea6fdfe774e2e updated the 
ClangScanDeps/modules.cpp test. The new `{{.*}}` regex is supposed to only 
match `modules_cdb_input.o`, `a.o` or `b.o`. However, due to non-determinism, 
this can sometimes also match `modules_cdb_input2.o`, causing match failure on 
the next line. This commit changes the regex to only match one of the three 
valid cases.

Buildbot failure: https://lab.llvm.org/buildbot/#/builders/109/builds/16675

Added: 


Modified: 
clang/test/ClangScanDeps/modules.cpp

Removed: 




diff  --git a/clang/test/ClangScanDeps/modules.cpp 
b/clang/test/ClangScanDeps/modules.cpp
index b7daf51b4a80..af7cdcb9d68c 100644
--- a/clang/test/ClangScanDeps/modules.cpp
+++ b/clang/test/ClangScanDeps/modules.cpp
@@ -48,7 +48,7 @@
 // CHECK1-NEXT: Inputs{{/|\\}}header2.h
 // CHECK1: Inputs{{/|\\}}header.h
 
-// CHECK2: {{.*}}.o:
+// CHECK2: {{(modules_cdb_input)|(a)|(b)}}.o:
 // CHECK2-NEXT: modules_cdb_input.cpp
 // CHECK2-NEXT: Inputs{{/|\\}}module.modulemap
 // CHECK2-NEXT: Inputs{{/|\\}}header.h



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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.
Herald added a subscriber: sstefan1.

Tracking a possible bug, https://bugs.llvm.org/show_bug.cgi?id=50702


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

  namespace test {
class Test {
public:
  Test() = default;
};
  } // namespace test

Getting miss formatted when BeforeLambdaBody is true


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3741
tok::colon, tok::l_square, tok::at) ||
+ (Style.BraceWrapping.BeforeLambdaBody && Right.is(tok::l_brace)) ||
  (Left.is(tok::r_paren) &&

I feel this could be way too aggressive! This is basically saying if I have 
BeforeLamdaBody is true then I will always break before the `{`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

[PATCH] D103616: [analyzer] Reimplement trackExpressionValue as ExpressionHandler

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2069-2070
+// this point.
+// TODO: Shouldn't we track control dependencies of every bug location,
+// rather than only tracked expressions?
+if (LVState->getAnalysisManager()

I recall that  reports that don't invoke trackExpressionValue were rather small 
in scope (usually a few LOC), and those  in particular tended to get bigger but 
not better. A lot of effort went into making sure we're only placing notes 
where we were reasonably sure they added value -- its been a while though, so a 
fresh look at this wouldn't hurt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103616

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

[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:367-368
+/// \param Origin Only adds notes when the last store happened in a
+///different stackframe to this one. Disregarded if the tracking kind
+///is thorough.
+///This is useful, because for non-tracked regions, notes about

Different or nested? (D64272)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103618

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

[PATCH] D44609: [clang-format] New option BeforeLambdaBody to manage lambda line break inside function parameter call (in Allman style)

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3312
+  auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
+  if (Style.BraceWrapping.BeforeLambdaBody &&
+  (isAllmanBraceIncludedBreakableLambda(Left, ShortLambdaOption) ||

actually I think its more likely this that is causing the incorrect breaking..I 
don't see anything in these checks which is validating if the tokens are 
actually part of a LambdaBody or did I miss something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D44609

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

[PATCH] D104036: [clang][deps] Prevent unintended modifications of the original TU command-line

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added inline comments.



Comment at: 
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h:194
  CompilerInstance &I, DependencyConsumer &C,
- std::map>
- OriginalPrebuiltModuleFiles);
+ CompilerInvocation OriginalInvocation);
 

dexonsmith wrote:
> I wonder if it'd be better to take `CompilerInvocation&&` here. Then the 
> caller is required to either pass `std::move` or make a deep copy at the call 
> site, and it's perhaps more clear that there's a deep copy being made.
That's much clearer indeed. Thanks for the suggestion!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104036

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

[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:367-368
+/// \param Origin Only adds notes when the last store happened in a
+///different stackframe to this one. Disregarded if the tracking kind
+///is thorough.
+///This is useful, because for non-tracked regions, notes about

Szelethus wrote:
> Different or nested? (D64272)
I'm not sure I understand this question, but it is simply a refactoring, all 
parameters come from previous interfaces.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103618

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

[clang] 41115ef - [clang][deps] NFC: Check the correct context hashes in tests

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T13:58:19+02:00
New Revision: 41115efca642981c2165cdf9ef2b2148605faa12

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

LOG: [clang][deps] NFC: Check the correct context hashes in tests

The `clang-scan-deps` tests for the full output format were written under the 
assumption that most TUs/modules have the same context hash. This is no longer 
true, since we're changing the original compilation options. This patch updates 
the tests, which no longer conflate multiple context hashes into a single 
FileCheck variable.

Added: 


Modified: 
clang/test/ClangScanDeps/modules-full.cpp
clang/test/ClangScanDeps/modules-inferred.m

Removed: 




diff  --git a/clang/test/ClangScanDeps/modules-full.cpp 
b/clang/test/ClangScanDeps/modules-full.cpp
index df2821cbef63..be0b79dd6fb0 100644
--- a/clang/test/ClangScanDeps/modules-full.cpp
+++ b/clang/test/ClangScanDeps/modules-full.cpp
@@ -42,7 +42,7 @@
 // CHECK-NEXT: {
 // CHECK-NEXT:   "clang-module-deps": [
 // CHECK-NEXT: {
-// CHECK-NEXT:   "context-hash": "[[CONTEXT_HASH_H1:[A-Z0-9]+]]",
+// CHECK-NEXT:   "context-hash": "[[HASH_H2_DINCLUDE:[A-Z0-9]+]]",
 // CHECK-NEXT:   "module-name": "header2"
 // CHECK-NEXT: }
 // CHECK-NEXT:   ],
@@ -54,12 +54,12 @@
 // CHECK-CUSTOM:   "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
 // CHECK:  "-emit-module"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
-// CHECK-ABS:  
"-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H1]]/header2-{{[A-Z0-9]+}}.pcm"
-// CHECK-CUSTOM:   
"-fmodule-file=[[PREFIX]]/custom/[[CONTEXT_HASH_H1]]/header2-{{[A-Z0-9]+}}.pcm"
+// CHECK-ABS:  
"-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm"
+// CHECK-CUSTOM:   
"-fmodule-file=[[PREFIX]]/custom/[[HASH_H2_DINCLUDE]]/header2-{{[A-Z0-9]+}}.pcm"
 // CHECK:  "-fmodule-name=header1"
 // CHECK:  "-fno-implicit-modules"
 // CHECK:],
-// CHECK-NEXT:   "context-hash": "[[CONTEXT_HASH_H1]]",
+// CHECK-NEXT:   "context-hash": "[[HASH_H1_DINCLUDE:[A-Z0-9]+]]",
 // CHECK-NEXT:   "file-deps": [
 // CHECK-NEXT: "[[PREFIX]]/Inputs/header.h",
 // CHECK-NEXT: "[[PREFIX]]/Inputs/module.modulemap"
@@ -75,7 +75,7 @@
 // CHECK:  "-fmodule-name=header1",
 // CHECK:  "-fno-implicit-modules",
 // CHECK:],
-// CHECK-NEXT:   "context-hash": "[[CONTEXT_HASH_H2:[A-Z0-9]+]]",
+// CHECK-NEXT:   "context-hash": "[[HASH_H1:[A-Z0-9]+]]",
 // CHECK-NEXT:   "file-deps": [
 // CHECK-NEXT: "[[PREFIX]]/Inputs/header.h",
 // CHECK-NEXT: "[[PREFIX]]/Inputs/module.modulemap"
@@ -91,7 +91,7 @@
 // CHECK:  "-fmodule-name=header2",
 // CHECK:  "-fno-implicit-modules",
 // CHECK:],
-// CHECK-NEXT:   "context-hash": "[[CONTEXT_HASH_H1]]",
+// CHECK-NEXT:   "context-hash": "[[HASH_H2_DINCLUDE]]",
 // CHECK-NEXT:   "file-deps": [
 // CHECK-NEXT: "[[PREFIX]]/Inputs/header2.h",
 // CHECK-NEXT: "[[PREFIX]]/Inputs/module.modulemap"
@@ -101,10 +101,10 @@
 // CHECK-NEXT:   ],
 // CHECK-NEXT:   "translation-units": [
 // CHECK-NEXT: {
-// CHECK-NEXT:   "clang-context-hash": "[[CONTEXT_HASH_H2]]",
+// CHECK-NEXT:   "clang-context-hash": "[[HASH_TU:[A-Z0-9]+]]",
 // CHECK-NEXT:   "clang-module-deps": [
 // CHECK-NEXT: {
-// CHECK-NEXT:   "context-hash": "[[CONTEXT_HASH_H2]]",
+// CHECK-NEXT:   "context-hash": "[[HASH_H1]]",
 // CHECK-NEXT:   "module-name": "header1"
 // CHECK-NEXT: }
 // CHECK-NEXT:   ],
@@ -112,8 +112,8 @@
 // CHECK-NEXT: "-fno-implicit-modules"
 // CHECK-NEXT: "-fno-implicit-module-maps"
 // CHECK-NO-ABS-NOT:   "-fmodule-file={{.*}}"
-// CHECK-ABS-NEXT: 
"-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[CONTEXT_HASH_H2]]/header1-{{[A-Z0-9]+}}.pcm"
-// CHECK-CUSTOM-NEXT:  
"-fmodule-file=[[PREFIX]]/custom/[[CONTEXT_HASH_H2]]/header1-{{[A-Z0-9]+}}.pcm"
+// CHECK-ABS-NEXT: 
"-fmodule-file=[[PREFIX]]/module-cache{{(_clangcl)?}}/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
+// CHECK-CUSTOM-NEXT:  
"-fmodule-file=[[PREFIX]]/custom/[[HASH_H1]]/header1-{{[A-Z0-9]+}}.pcm"
 // CHECK-NO-ABS-NOT:   "-fmodule-map-file={{.*}}"
 // CHECK-ABS-NEXT: "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
 // CHECK-CUSTOM-NEXT:  "-fmodule-map-file=[[PREFIX]]/Inputs/module.modulemap"
@@ -124,10 +124,10 @@
 // CHECK-NEXT:   "input-file": "[[PREFIX]]/modules_cdb_input.cpp"
 // CHECK-NEXT: },
 // CHECK-NEXT: {
-// CHECK-NEXT:   "clang-context-hash": "[[CONTEXT_HASH_H2]]",
+// CHECK-NEXT: 

[clang] 80c0c63 - [clang][deps] Prevent unintended modifications of the original TU command-line

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T13:58:19+02:00
New Revision: 80c0c639687ef52f5c432ea059ff9cb53125d08e

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

LOG: [clang][deps] Prevent unintended modifications of the original TU 
command-line

One of the goals of the dependency scanner is to generate command-lines that 
can be used to explicitly build modular dependencies of a translation unit. The 
only modifications to these command-lines should be for the purposes of 
explicit modular build.

However, the current version of dependency scanner leaks its implementation 
details into the command-lines.

The first problem is that the `clang-scan-deps` tool adjusts the original 
textual command-line (adding `-Eonly -M -MT  -sys-header-deps 
-Wno-error -o /dev/null `, removing `--serialize-diagnostics`) in order to set 
up the `DependencyScanning` library. This has been addressed in D103461, 
D104012, D104030, D104031, D104033. With these patches, the 
`DependencyScanning` library receives the unmodified `CompilerInvocation`, sets 
it up and uses it for the implicit modular build.

Finally, to prevent leaking the implementation details to the resulting 
command-lines, this patch generates them from the **original** unmodified 
`CompilerInvocation` rather than from the one that drives the implicit build.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D104036

Added: 
clang/test/ClangScanDeps/Inputs/preserved-args/cdb.json.template
clang/test/ClangScanDeps/Inputs/preserved-args/mod.h
clang/test/ClangScanDeps/Inputs/preserved-args/module.modulemap
clang/test/ClangScanDeps/Inputs/preserved-args/tu.c
clang/test/ClangScanDeps/preserved-args.c

Modified: 
clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h 
b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
index 73bc41d1f955..a9f2b4d0c6fc 100644
--- a/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
+++ b/clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
@@ -191,8 +191,7 @@ class ModuleDepCollector final : public DependencyCollector 
{
 public:
   ModuleDepCollector(std::unique_ptr Opts,
  CompilerInstance &I, DependencyConsumer &C,
- std::map>
- OriginalPrebuiltModuleFiles);
+ CompilerInvocation &&OriginalCI);
 
   void attachToPreprocessor(Preprocessor &PP) override;
   void attachToASTReader(ASTReader &R) override;
@@ -215,9 +214,8 @@ class ModuleDepCollector final : public DependencyCollector 
{
   std::unordered_map ModularDeps;
   /// Options that control the dependency output generation.
   std::unique_ptr Opts;
-  /// The mapping between prebuilt module names and module files that were
-  /// present in the original CompilerInvocation.
-  std::map> OriginalPrebuiltModuleFiles;
+  /// The original Clang invocation passed to dependency scanner.
+  CompilerInvocation OriginalInvocation;
 
   /// Checks whether the module is known as being prebuilt.
   bool isPrebuiltModule(const Module *M);

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index a2f9b1c0e074..bbe498d12f15 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -47,9 +47,11 @@ class DependencyConsumerForwarder : public 
DependencyFileGenerator {
 
 /// A listener that collects the names and paths to imported modules.
 class ImportCollectingListener : public ASTReaderListener {
+  using PrebuiltModuleFilesT =
+  decltype(HeaderSearchOptions::PrebuiltModuleFiles);
+
 public:
-  ImportCollectingListener(
-  std::map &PrebuiltModuleFiles)
+  ImportCollectingListener(PrebuiltModuleFilesT &PrebuiltModuleFiles)
   : PrebuiltModuleFiles(PrebuiltModuleFiles) {}
 
   bool needsImportVisitation() const override { return true; }
@@ -59,7 +61,7 @@ class ImportCollectingListener : public ASTReaderListener {
   }
 
 private:
-  std::map &PrebuiltModuleFiles;
+  PrebuiltModuleFilesT &PrebuiltModuleFiles;
 };
 
 /// Transform arbitrary file name into an object-like file name.
@@ -99,6 +101,9 @@ class DependencyScanningAction : public tooling::ToolAction {
  FileManager *FileMgr,
  std::shared_ptr PCHContainerOps,
  DiagnosticConsumer *DiagConsumer) override {
+// Make a deep copy of the origi

[PATCH] D104036: [clang][deps] Prevent unintended modifications of the original TU command-line

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG80c0c639687e: [clang][deps] Prevent unintended modifications 
of the original TU command-line (authored by jansvoboda11).

Changed prior to commit:
  https://reviews.llvm.org/D104036?vs=351174&id=351833#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104036

Files:
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/preserved-args/cdb.json.template
  clang/test/ClangScanDeps/Inputs/preserved-args/mod.h
  clang/test/ClangScanDeps/Inputs/preserved-args/module.modulemap
  clang/test/ClangScanDeps/Inputs/preserved-args/tu.c
  clang/test/ClangScanDeps/preserved-args.c

Index: clang/test/ClangScanDeps/preserved-args.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/preserved-args.c
@@ -0,0 +1,26 @@
+// RUN: rm -rf %t && mkdir %t
+// RUN: cp -r %S/Inputs/preserved-args/* %t
+// RUN: sed -e "s|DIR|%/t|g" %t/cdb.json.template > %t/cdb.json
+
+// RUN: echo -%t > %t/result.json
+// RUN: clang-scan-deps -compilation-database %t/cdb.json -format experimental-full >> %t/result.json
+// RUN: cat %t/result.json | sed 's:\?:/:g' | FileCheck %s
+
+// CHECK:  -[[PREFIX:.*]]
+// CHECK-NEXT: {
+// CHECK-NEXT:   "modules": [
+// CHECK-NEXT: {
+// CHECK:"command-line": [
+// CHECK-NEXT: "-cc1"
+// CHECK:  "-serialize-diagnostic-file"
+// CHECK-NEXT: "[[PREFIX]]/tu.dia"
+// CHECK:  "-fmodule-file=Foo=[[PREFIX]]/foo.pcm"
+// CHECK:  "-MT"
+// CHECK-NEXT: "my_target"
+// CHECK:  "-dependency-file"
+// CHECK-NEXT: "[[PREFIX]]/tu.d"
+// CHECK:],
+// CHECK:"name": "Mod"
+// CHECK-NEXT: }
+// CHECK-NEXT:   ]
+// CHECK:  }
Index: clang/test/ClangScanDeps/Inputs/preserved-args/tu.c
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/preserved-args/tu.c
@@ -0,0 +1 @@
+#include "mod.h"
Index: clang/test/ClangScanDeps/Inputs/preserved-args/module.modulemap
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/preserved-args/module.modulemap
@@ -0,0 +1,3 @@
+module Mod {
+  header "mod.h"
+}
Index: clang/test/ClangScanDeps/Inputs/preserved-args/mod.h
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/preserved-args/mod.h
@@ -0,0 +1 @@
+// mod.h
Index: clang/test/ClangScanDeps/Inputs/preserved-args/cdb.json.template
===
--- /dev/null
+++ clang/test/ClangScanDeps/Inputs/preserved-args/cdb.json.template
@@ -0,0 +1,7 @@
+[
+  {
+"directory": "DIR",
+"command": "clang -MD -MT my_target -serialize-diagnostics DIR/tu.dia -fsyntax-only DIR/tu.c -fmodules -fimplicit-module-maps -fmodules-cache-path=DIR/cache -fmodule-file=Foo=DIR/foo.pcm -o DIR/tu.o",
+"file": "DIR/tu.c"
+  }
+]
Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -20,8 +20,8 @@
 
 CompilerInvocation ModuleDepCollector::makeInvocationForModuleBuildWithoutPaths(
 const ModuleDeps &Deps) const {
-  // Make a deep copy of the invocation.
-  CompilerInvocation CI(Instance.getInvocation());
+  // Make a deep copy of the original Clang invocation.
+  CompilerInvocation CI(OriginalInvocation);
 
   // Remove options incompatible with explicit module build.
   CI.getFrontendOpts().Inputs.clear();
@@ -39,9 +39,6 @@
 CI.getFrontendOpts().ModuleMapFiles.push_back(PrebuiltModule.ModuleMapFile);
   }
 
-  // Restore the original set of prebuilt module files.
-  CI.getHeaderSearchOpts().PrebuiltModuleFiles = OriginalPrebuiltModuleFiles;
-
   CI.getPreprocessorOpts().ImplicitPCHInclude.clear();
 
   return CI;
@@ -269,10 +266,9 @@
 
 ModuleDepCollector::ModuleDepCollector(
 std::unique_ptr Opts, CompilerInstance &I,
-DependencyConsumer &C,
-std::map> OriginalPrebuiltModuleFiles)
+DependencyConsumer &C, CompilerInvocation &&OriginalCI)
 : Instance(I), Consumer(C), Opts(std::move(Opts)),
-  OriginalPrebuiltModuleFiles(std::move(OriginalPrebuiltModuleFiles)) {}
+  OriginalInvocation(std::move(OriginalCI)) {}
 
 void ModuleDepCollector::attachToPreprocessor(Preprocessor &PP) {
   PP.addPPCallbacks(std::make_unique(Instance, *this));
Index: clang/lib/Tooling/DependencyScanning/Depende

[PATCH] D104036: [clang][deps] Prevent unintended modifications of the original TU command-line

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

Either this or your concurrent commit broke check-clang: 
http://45.33.8.238/linux/48839/step_7.txt

Please take a look and revert for now if it takes a while to fix.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104036

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

[PATCH] D103792: [clang][AST] Set correct DeclContext in ASTImporter lookup table for template params.

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:5961-5962
 
   if (Error Err = ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))
 return std::move(Err);
 

Do you think that the alternative approach that is used with `TypedefNameDecl` 
could work here?
```
  // Do not import the DeclContext, we will import it once the TypedefNameDecl
  // is created.
  if (Error Err = ImportDeclParts(D, Name, ToD, Loc))
return std::move(Err);
```
With that perhaps we could avoid the hassle with the `update`. Could you please 
investigate that?



Comment at: clang/lib/AST/ASTImporter.cpp:6011
+  // FunctionTemplateDecl is created, but was set already when the class
+  // template was created. So here is is not the TU (default value) any more.
+  // FIXME: The DeclContext of the parameters is now set finally to the

typo: `is is` -> `it is`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103792

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

[PATCH] D104036: [clang][deps] Prevent unintended modifications of the original TU command-line

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D104036#2816602 , @thakis wrote:

> Either this or your concurrent commit broke check-clang: 
> http://45.33.8.238/linux/48839/step_7.txt
>
> Please take a look and revert for now if it takes a while to fix.

This test has been intermittently failing since yesterday: 
https://lab.llvm.org/buildbot/#/builders/109/builds/16653
I've notified the author here: 
https://reviews.llvm.org/rG673c5ba58497298a684f8b8dfddbfb11cd89950e


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104036

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

[clang] f39dcf8 - [clang][deps] NFC: Fix an XFAIL test on Windows

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T14:37:26+02:00
New Revision: f39dcf85f994b464946cd6702bbceadc518ce904

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

LOG: [clang][deps] NFC: Fix an XFAIL test on Windows

The `sed` command ensures Windows-specific path separators (single and double 
backslashes) are replaced by forward slashes in the output file. FileCheck can 
continue using forward slashes in paths this way.

Added: 


Modified: 
clang/test/ClangScanDeps/modules-full.cpp

Removed: 




diff  --git a/clang/test/ClangScanDeps/modules-full.cpp 
b/clang/test/ClangScanDeps/modules-full.cpp
index be0b79dd6fb0..6841d2ff42c6 100644
--- a/clang/test/ClangScanDeps/modules-full.cpp
+++ b/clang/test/ClangScanDeps/modules-full.cpp
@@ -13,26 +13,23 @@
 // RUN: echo %t.dir > %t.result
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 4 -format 
experimental-full \
 // RUN:   -mode preprocess-minimized-sources >> %t.result
-// RUN: cat %t.result | sed 's/\\/\//g' | FileCheck 
--check-prefixes=CHECK,CHECK-NO-ABS %s
+// RUN: cat %t.result | sed 's:\?:/:g' | FileCheck 
--check-prefixes=CHECK,CHECK-NO-ABS %s
 //
 // RUN: echo %t.dir > %t.result
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 4 -format 
experimental-full \
 // RUN:   -generate-modules-path-args -mode preprocess-minimized-sources >> 
%t.result
-// RUN: cat %t.result | sed 's/\\/\//g' | FileCheck 
--check-prefixes=CHECK,CHECK-ABS %s
+// RUN: cat %t.result | sed 's:\?:/:g' | FileCheck 
--check-prefixes=CHECK,CHECK-ABS %s
 //
 // RUN: echo %t.dir > %t.result
 // RUN: clang-scan-deps -compilation-database %t.cdb -j 4 -format 
experimental-full \
 // RUN:   -generate-modules-path-args -module-files-dir %t.dir/custom \
 // RUN:   -mode preprocess-minimized-sources >> %t.result
-// RUN: cat %t.result | sed 's/\\/\//g' | FileCheck 
--check-prefixes=CHECK,CHECK-CUSTOM %s
+// RUN: cat %t.result | sed 's:\?:/:g' | FileCheck 
--check-prefixes=CHECK,CHECK-CUSTOM %s
 //
 // RUN: echo %t.dir > %t_clangcl.result
 // RUN: clang-scan-deps -compilation-database %t_clangcl.cdb -j 4 -format 
experimental-full \
 // RUN:   -mode preprocess-minimized-sources >> %t_clangcl.result
-// RUN: cat %t_clangcl.result | sed 's/\\/\//g' | FileCheck 
--check-prefixes=CHECK,CHECK-NO-ABS %s
-
-// FIXME: Backslash issues.
-// XFAIL: system-windows
+// RUN: cat %t_clangcl.result | sed 's:\?:/:g' | FileCheck 
--check-prefixes=CHECK,CHECK-NO-ABS %s
 
 #include "header.h"
 



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

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 351838.
mgartmann marked 5 inline comments as done.
mgartmann added a comment.

- removed empty configs from tests
- moved documentation to Google's check
- extended matchers so that namespaces for classes and conversion operators can 
be specified
- adjusted documentation and tests

Unfortunately, I was not able to implement functionality to ignore constructors 
and operates by using Regex in the given time.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp
  clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-constructor-and-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconstructors-option.cpp
  
clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/google-explicit-constructor-ignoredconversionoperators-option.cpp
@@ -0,0 +1,95 @@
+// RUN: %check_clang_tidy -check-suffix=DEFAULT %s google-explicit-constructor %t
+
+// RUN: %check_clang_tidy -check-suffix=IGNORED %s \
+// RUN: google-explicit-constructor %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN:   {key: google-explicit-constructor.IgnoredConversionOperators, value: "Foo::A::operator bool;Foo::A::operator type-parameter-0-0;B::operator double;B::operator A"} \
+// RUN: ]}'
+
+namespace Foo {
+struct A {
+  A() {}
+  A(int x, int y) {}
+
+  explicit A(void *x) {}
+  explicit A(void *x, void *y) {}
+
+  A(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit A(int x1);
+
+  operator bool() const { return true; }
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator bool() const { return true; }
+
+  operator double() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit operator double() const;
+
+  template 
+  operator Ty() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator type-parameter-0-0' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator Ty() const;
+};
+
+inline A::A(int x1) {}
+} // namespace Foo
+
+using Foo::A;
+struct B {
+  B() {}
+  B(int x, int y) {}
+
+  explicit B(void *x) {}
+  explicit B(void *x, void *y) {}
+
+  B(int x1);
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: single-argument constructors must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit B(int x1);
+
+  operator bool() const { return true; }
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-MESSAGES-IGNORED: :[[@LINE-2]]:3: warning: 'operator bool' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES: {{^  }}explicit operator bool() const { return true; }
+
+  operator double() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]:3: warning: 'operator double' must be marked explicit to avoid unintentional implicit conversions [google-explicit-constructor]
+  // CHECK-FIXES-DEFAULT: {{^  }}explicit operator double() const;
+
+  operator A() const;
+  // CHECK-MESSAGES-DEFAULT: :[[@LINE-1]]

[PATCH] D102779: [clang-tidy] cppcoreguidelines-explicit-constructor-and-conversion: new alias

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/google/ExplicitConstructorCheck.cpp:28-30
+  std::string FullOperatorName =
+  Node.getParent()->getNameAsString().append("::").append(
+  Node.getNameAsString());

aaron.ballman wrote:
> Rather than trying to do this by hand, does `Decl::print()` give you the 
> functionality you need? For example, this will likely not work well for 
> classes defined within a namespace (it may ignore the wrong class due to not 
> checking the namespace). Another thing to consider are templates and how to 
> handle those. e.g.,
> ```
> struct Foo {
>   template 
>   operator Ty() const; // How to silence the diagnostic here?
> };
> ```
> Thankfully, specializations can't differ in their explicitness, so you don't 
> have to also worry about:
> ```
> struct Foo {
>   template 
>   explicit operator Ty() const; // This one's explicit
> };
> 
> template <>
> Foo::operator int() const; // Thankfully, this inherits the explicit from the 
> primary template.
> ```
Thanks for your comment, @aaron.ballman!

I was able to use `printQualifiedName` to get a `Node`'s qualified name, 
including its namespace. 

Templated operators are not represented by their name visible in the source 
code (e.g., `Ty`) in the AST. Instead, their name in the AST is something like 
`type-parameter-0-0`. As it is now, templated operators have to be ignored with 
the latter name, which is also displayed in the check's diagnostic message.
This was described in the documentation accordingly.

I was not able to find a feasible way to enable users to exclude templated 
operators by their original name (e.g., `Ty`). Does anyone have an idea how 
this could be done?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102779

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

[PATCH] D103630: [analyzer] Refactor trackRValueExpression into ExpressionHandler

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2263-2264
 Tracker::Tracker(PathSensitiveBugReport &Report) : Report(Report) {
   addHighPriorityHandler();
+  addLowPriorityHandler();
   // TODO: split trackExpressionValue and FindLastStoreBRVisitor into handlers

I checked this commit out, and failed to see how these low/high priority 
handlers work out in practice. I tried to
* Make the default handler low prio as well
* Make the PRValue high prio
* All other combinations of the those values
* Change the order of adding these handlers

No tests failed. Is there any one patch so far that demonstrates why we need 
this? A unit test?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103630

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

[PATCH] D103630: [analyzer] Refactor trackRValueExpression into ExpressionHandler

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2263-2264
 Tracker::Tracker(PathSensitiveBugReport &Report) : Report(Report) {
   addHighPriorityHandler();
+  addLowPriorityHandler();
   // TODO: split trackExpressionValue and FindLastStoreBRVisitor into handlers

Szelethus wrote:
> I checked this commit out, and failed to see how these low/high priority 
> handlers work out in practice. I tried to
> * Make the default handler low prio as well
> * Make the PRValue high prio
> * All other combinations of the those values
> * Change the order of adding these handlers
> 
> No tests failed. Is there any one patch so far that demonstrates why we need 
> this? A unit test?
add with high priority == add to the beginning of the queue
add with low priority == add to the end of the queue

This means that it doesn't matter with what priority we add the very first one.
I guess with these two it doesn't matter which order they have one against 
another, that's why tests don't fail.  This code here keeps the same order it 
was originally in `trackExpressionValue` to maintain parity.

In the future commits, `InterestingLValueHandler` HAVE TO BE before before 
`DefaultExpressionHandler` and there are tests that validate it.  Actually this 
is the only place that I know that really cares about the order and that's why 
I needed to add priorities and the way for one handler to cancel all further 
handlers.

Does this answer your question?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103630

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

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay created this revision.
MyDeveloperDay added reviewers: curdeius, HazardyKnusperkeks, Wawha, krasimir.
MyDeveloperDay added projects: clang, clang-format.
MyDeveloperDay requested review of this revision.

https://bugs.llvm.org/show_bug.cgi?id=50702

I believe D44609: [clang-format] New option BeforeLambdaBody to manage lambda 
line break inside function parameter call (in Allman style) 
 may be too aggressive with brace wrapping 
rules which doesn't always apply to Lamdbas

The introduction of BeforeLambdaBody and AllowShortLambdasOnASingleLine has 
impact on brace handling on other block types, which I suspect we didn't see 
before as people may not be using the BeforeLambdaBody  style

>From what I can tell this can be seen by the unit test I change as its not 
>honouring the orginal LLVM brace wrapping style for the `Fct()` function

I added a unit test from PR50702 and have removed some of the code (which has 
zero impact on the unit test, which kind of suggests its unnecessary), some 
additional attempt has been made to try and ensure we'll only break on what is 
actually a LamdbaLBrace


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104222

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18856,8 +18856,7 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
-  verifyFormat("void Fct()\n"
-   "{\n"
+  verifyFormat("void Fct() {\n"
"  return {[]()\n"
"  {\n"
"return 17;\n"
@@ -19061,6 +19060,16 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_None;
+  verifyFormat("namespace test {\n"
+   "class Test {\n"
+   "public:\n"
+   "  Test() = default;\n"
+   "};\n"
+   "} // namespace test",
+   LLVMWithBeforeLambdaBody);
 }
 
 TEST_F(FormatTest, LambdaWithLineComments) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3754,13 +3754,6 @@
   if (Right.is(TT_InlineASMBrace))
 return Right.HasUnescapedNewline;
 
-  auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
-  if (Style.BraceWrapping.BeforeLambdaBody &&
-  (isAllmanBraceIncludedBreakableLambda(Left, ShortLambdaOption) ||
-   isAllmanBraceIncludedBreakableLambda(Right, ShortLambdaOption))) {
-return true;
-  }
-
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
 return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
@@ -4186,7 +4179,7 @@
 return false;
 
   auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
-  if (Style.BraceWrapping.BeforeLambdaBody) {
+  if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) {
 if (isAllmanLambdaBrace(Left))
   return !isItAnEmptyLambdaAllowed(Left, ShortLambdaOption);
 if (isAllmanLambdaBrace(Right))
@@ -4198,7 +4191,6 @@
  Right.isMemberAccess() ||
  Right.isOneOf(TT_TrailingReturnArrow, TT_LambdaArrow, tok::lessless,
tok::colon, tok::l_square, tok::at) ||
- (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) ||
  (Left.is(tok::r_paren) &&
   Right.isOneOf(tok::identifier, tok::kw_const)) ||
  (Left.is(tok::l_paren) && !Right.is(tok::r_paren)) ||


Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18856,8 +18856,7 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
-  verifyFormat("void Fct()\n"
-   "{\n"
+  verifyFormat("void Fct() {\n"
"  return {[]()\n"
"  {\n"
"return 17;\n"
@@ -19061,6 +19060,16 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_None;
+  verifyFormat("namespace test {\n"
+   "class Test {\n"
+   "public:\n"
+   "  Test() = default;\n"
+   "};\n"
+   "} // namespace test",
+   LLVMWithBeforeLambdaBody);
 }
 
 TEST_F(FormatTest, LambdaWit

[PATCH] D103618: [analyzer] Change FindLastStoreBRVisitor to use Tracker

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

I know I'm late to the party, and am just mostly thinking aloud. Awesome patch 
series!




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:331-341
+/// Visitor that tracks expressions and values.
+class TrackingBugReporterVisitor : public BugReporterVisitor {
+private:
+  TrackerRef ParentTracker;
+
+public:
+  TrackingBugReporterVisitor(TrackerRef ParentTracker)

Maybe we could draw more attention to the fact this is a major component of bug 
report construction? Dividers like this maybe:

```
//===--===//
// Visitor core components.
//===--===//

class BugReporterVisitor {};
// ...

class TrackingBugReporterVisitor {};

//===--===//
// Specific visitors.
//===--===//
```



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:340
+
+  Tracker &getParentTracker() { return *ParentTracker; }
+};

Why is this a //parent// tracker? Could visitors have their own child trackers? 



Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:367-368
+/// \param Origin Only adds notes when the last store happened in a
+///different stackframe to this one. Disregarded if the tracking kind
+///is thorough.
+///This is useful, because for non-tracked regions, notes about

vsavchenko wrote:
> Szelethus wrote:
> > Different or nested? (D64272)
> I'm not sure I understand this question, but it is simply a refactoring, all 
> parameters come from previous interfaces.
Oh well, then its likely me that made this mistake :^)

Both `f`'s and `h`'s stack frame is different to `g`'s, but only `f` is nested. 
As a top level function, the bug path would start from `h`, so arrows and 
function call events would be present in it. `f` may be pruned (no diagnostic 
pieces would point to it), if the analyzer decides that nothing interesting 
happens in it (not this, but maybe a similar example). Following this logic, 
we're looking for nested stack frames in particular, to tell the analyzer not 
to prune them.
```lang=c++
void f(int **p) {
  *p = NULL; // At the point of assignment, the stackframe of f is nested in 
g's stackframe
}

void h() {
  *p = malloc(...); 
  g(p); // h calls g, g's stackframe is nested in h's
}

void g(int **p) {
  f(p);
  **p = 5; // warning: nullptr reference
}
```

The comment is a little dumb, as it explicitly mentions "nested" a sentence 
later.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103618

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

[PATCH] D103630: [analyzer] Refactor trackRValueExpression into ExpressionHandler

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:2263-2264
 Tracker::Tracker(PathSensitiveBugReport &Report) : Report(Report) {
   addHighPriorityHandler();
+  addLowPriorityHandler();
   // TODO: split trackExpressionValue and FindLastStoreBRVisitor into handlers

vsavchenko wrote:
> Szelethus wrote:
> > I checked this commit out, and failed to see how these low/high priority 
> > handlers work out in practice. I tried to
> > * Make the default handler low prio as well
> > * Make the PRValue high prio
> > * All other combinations of the those values
> > * Change the order of adding these handlers
> > 
> > No tests failed. Is there any one patch so far that demonstrates why we 
> > need this? A unit test?
> add with high priority == add to the beginning of the queue
> add with low priority == add to the end of the queue
> 
> This means that it doesn't matter with what priority we add the very first 
> one.
> I guess with these two it doesn't matter which order they have one against 
> another, that's why tests don't fail.  This code here keeps the same order it 
> was originally in `trackExpressionValue` to maintain parity.
> 
> In the future commits, `InterestingLValueHandler` HAVE TO BE before before 
> `DefaultExpressionHandler` and there are tests that validate it.  Actually 
> this is the only place that I know that really cares about the order and 
> that's why I needed to add priorities and the way for one handler to cancel 
> all further handlers.
> 
> Does this answer your question?
Perfectly, thank you! :) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103630

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

[clang] f95ff81 - [clang][deps] NFC: Handle `DependencyOutputOptions` only once

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T15:16:08+02:00
New Revision: f95ff81627212a8db9f942aafa91392096538847

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

LOG: [clang][deps] NFC: Handle `DependencyOutputOptions` only once

There's no need to pass `DependencyOutputOptions` to each call of 
`handleFileDependency`, since the options don't ever change.

This patch adds new `handleDependencyOutputOpts` method to the 
`DependencyConsumer` interface and the dependency scanner uses it to report the 
options only once.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D104104

Added: 


Modified: 
clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp

Removed: 




diff  --git 
a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h 
b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
index e51040d2c0f5..5903ad13c1d8 100644
--- a/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ b/clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -34,8 +34,10 @@ class DependencyConsumer {
 public:
   virtual ~DependencyConsumer() {}
 
-  virtual void handleFileDependency(const DependencyOutputOptions &Opts,
-StringRef Filename) = 0;
+  virtual void
+  handleDependencyOutputOpts(const DependencyOutputOptions &Opts) = 0;
+
+  virtual void handleFileDependency(StringRef Filename) = 0;
 
   virtual void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) = 0;
 

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
index 409ccd393e78..2fd12f7e12b1 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -54,10 +54,12 @@ llvm::Expected 
DependencyScanningTool::getDependencyFile(
   /// Prints out all of the gathered dependencies into a string.
   class MakeDependencyPrinterConsumer : public DependencyConsumer {
   public:
-void handleFileDependency(const DependencyOutputOptions &Opts,
-  StringRef File) override {
-  if (!this->Opts)
-this->Opts = std::make_unique(Opts);
+void
+handleDependencyOutputOpts(const DependencyOutputOptions &Opts) override {
+  this->Opts = std::make_unique(Opts);
+}
+
+void handleFileDependency(StringRef File) override {
   Dependencies.push_back(std::string(File));
 }
 
@@ -74,8 +76,7 @@ llvm::Expected 
DependencyScanningTool::getDependencyFile(
 void handleContextHash(std::string Hash) override {}
 
 void printDependencies(std::string &S) {
-  if (!Opts)
-return;
+  assert(Opts && "Handled dependency output options.");
 
   class DependencyPrinter : public DependencyFileGenerator {
   public:
@@ -128,8 +129,10 @@ DependencyScanningTool::getFullDependencies(
 FullDependencyPrinterConsumer(const llvm::StringSet<> &AlreadySeen)
 : AlreadySeen(AlreadySeen) {}
 
-void handleFileDependency(const DependencyOutputOptions &Opts,
-  StringRef File) override {
+void
+handleDependencyOutputOpts(const DependencyOutputOptions &Opts) override {}
+
+void handleFileDependency(StringRef File) override {
   Dependencies.push_back(std::string(File));
 }
 

diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index bbe498d12f15..48abc095231f 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -32,11 +32,12 @@ class DependencyConsumerForwarder : public 
DependencyFileGenerator {
   : DependencyFileGenerator(*Opts), Opts(std::move(Opts)), C(C) {}
 
   void finishedMainFile(DiagnosticsEngine &Diags) override {
+C.handleDependencyOutputOpts(*Opts);
 llvm::SmallString<256> CanonPath;
 for (const auto &File : getDependencies()) {
   CanonPath = File;
   llvm::sys::path::remove_dots(CanonPath, /*remove_dot_dot=*/true);
-  C.handleFileDependency(*Opts, CanonPath);
+  C.handleFileDependency(CanonPath);
 }
   }
 

diff  --git a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp 
b/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
index 21770540c100..690bd6d476e5 100644
--- a/clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ 

[clang] 85208b9 - [clang][deps] NFC: Stop using moved-from object

[Jan Svoboda via cfe-commits]

Author: Jan Svoboda
Date: 2021-06-14T15:16:08+02:00
New Revision: 85208b96b85f6f3e502cf4b7fd5f440434d1c7e5

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

LOG: [clang][deps] NFC: Stop using moved-from object

The dependency scanning worker uses `std::move` to "reset" 
`DependencyOutputOptions` in the `CompilerInstance` that performs the implicit 
build. It's probably preferable to replace the object with value-initialized 
instance, rather than depending on the behavior of a moved-from object.

Reviewed By: dexonsmith

Differential Revision: https://reviews.llvm.org/D104106

Added: 


Modified: 
clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp

Removed: 




diff  --git a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp 
b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
index 48abc095231f..cd3463aa13e6 100644
--- a/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ b/clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -171,8 +171,8 @@ class DependencyScanningAction : public tooling::ToolAction 
{
 // invocation to the collector. The options in the invocation are reset,
 // which ensures that the compiler won't create new dependency collectors,
 // and thus won't write out the extra '.d' files to disk.
-auto Opts = std::make_unique(
-std::move(Compiler.getInvocation().getDependencyOutputOpts()));
+auto Opts = std::make_unique();
+std::swap(*Opts, Compiler.getInvocation().getDependencyOutputOpts());
 // We need at least one -MT equivalent for the generator of make dependency
 // files to work.
 if (Opts->Targets.empty())



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

[PATCH] D104106: [clang][deps] NFC: Stop using moved-from object

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG85208b96b85f: [clang][deps] NFC: Stop using moved-from 
object (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104106

Files:
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -171,8 +171,8 @@
 // invocation to the collector. The options in the invocation are reset,
 // which ensures that the compiler won't create new dependency collectors,
 // and thus won't write out the extra '.d' files to disk.
-auto Opts = std::make_unique(
-std::move(Compiler.getInvocation().getDependencyOutputOpts()));
+auto Opts = std::make_unique();
+std::swap(*Opts, Compiler.getInvocation().getDependencyOutputOpts());
 // We need at least one -MT equivalent for the generator of make dependency
 // files to work.
 if (Opts->Targets.empty())


Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -171,8 +171,8 @@
 // invocation to the collector. The options in the invocation are reset,
 // which ensures that the compiler won't create new dependency collectors,
 // and thus won't write out the extra '.d' files to disk.
-auto Opts = std::make_unique(
-std::move(Compiler.getInvocation().getDependencyOutputOpts()));
+auto Opts = std::make_unique();
+std::swap(*Opts, Compiler.getInvocation().getDependencyOutputOpts());
 // We need at least one -MT equivalent for the generator of make dependency
 // files to work.
 if (Opts->Targets.empty())
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104104: [clang][deps] NFC: Handle `DependencyOutputOptions` only once

[Jan Svoboda via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rGf95ff8162721: [clang][deps] NFC: Handle 
`DependencyOutputOptions` only once (authored by jansvoboda11).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104104

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -173,11 +173,13 @@
   for (const Module *M : DirectModularDeps)
 handleTopLevelModule(M);
 
+  MDC.Consumer.handleDependencyOutputOpts(*MDC.Opts);
+
   for (auto &&I : MDC.ModularDeps)
 MDC.Consumer.handleModuleDependency(I.second);
 
   for (auto &&I : MDC.FileDeps)
-MDC.Consumer.handleFileDependency(*MDC.Opts, I);
+MDC.Consumer.handleFileDependency(I);
 
   for (auto &&I : DirectPrebuiltModularDeps)
 MDC.Consumer.handlePrebuiltModuleDependency(PrebuiltModuleDep{I});
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
@@ -32,11 +32,12 @@
   : DependencyFileGenerator(*Opts), Opts(std::move(Opts)), C(C) {}
 
   void finishedMainFile(DiagnosticsEngine &Diags) override {
+C.handleDependencyOutputOpts(*Opts);
 llvm::SmallString<256> CanonPath;
 for (const auto &File : getDependencies()) {
   CanonPath = File;
   llvm::sys::path::remove_dots(CanonPath, /*remove_dot_dot=*/true);
-  C.handleFileDependency(*Opts, CanonPath);
+  C.handleFileDependency(CanonPath);
 }
   }
 
Index: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
===
--- clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
+++ clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
@@ -54,10 +54,12 @@
   /// Prints out all of the gathered dependencies into a string.
   class MakeDependencyPrinterConsumer : public DependencyConsumer {
   public:
-void handleFileDependency(const DependencyOutputOptions &Opts,
-  StringRef File) override {
-  if (!this->Opts)
-this->Opts = std::make_unique(Opts);
+void
+handleDependencyOutputOpts(const DependencyOutputOptions &Opts) override {
+  this->Opts = std::make_unique(Opts);
+}
+
+void handleFileDependency(StringRef File) override {
   Dependencies.push_back(std::string(File));
 }
 
@@ -74,8 +76,7 @@
 void handleContextHash(std::string Hash) override {}
 
 void printDependencies(std::string &S) {
-  if (!Opts)
-return;
+  assert(Opts && "Handled dependency output options.");
 
   class DependencyPrinter : public DependencyFileGenerator {
   public:
@@ -128,8 +129,10 @@
 FullDependencyPrinterConsumer(const llvm::StringSet<> &AlreadySeen)
 : AlreadySeen(AlreadySeen) {}
 
-void handleFileDependency(const DependencyOutputOptions &Opts,
-  StringRef File) override {
+void
+handleDependencyOutputOpts(const DependencyOutputOptions &Opts) override {}
+
+void handleFileDependency(StringRef File) override {
   Dependencies.push_back(std::string(File));
 }
 
Index: clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
===
--- clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
+++ clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
@@ -34,8 +34,10 @@
 public:
   virtual ~DependencyConsumer() {}
 
-  virtual void handleFileDependency(const DependencyOutputOptions &Opts,
-StringRef Filename) = 0;
+  virtual void
+  handleDependencyOutputOpts(const DependencyOutputOptions &Opts) = 0;
+
+  virtual void handleFileDependency(StringRef Filename) = 0;
 
   virtual void handlePrebuiltModuleDependency(PrebuiltModuleDep PMD) = 0;
 


Index: clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
===
--- clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
+++ clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
@@ -173,11 +173,13 @@
   for (const Module *M : DirectModularDeps)
 handleTopLevelModule(M);
 
+  MDC.Consumer.hand

[PATCH] D104116: AMD k8 family does not support SSE4.x which are required by x86-64-v2+

[serge via Phabricator via cfe-commits]

serge-sans-paille added a comment.

In D104116#2816291 , @RKSimon wrote:

> This leaves the question - what hardware should we align each of the 
> CK_x86_64_v* targets with?

I'm not quite sure I get it. If I understand correctly, each `CK_x86_64_v*` 
implies a mimal set of requirements, but doesn't map to a particular hardware, 
right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104116

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

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

[Hans Wennborg via Phabricator via cfe-commits]

hans added subscribers: rjmccall, hans.
hans added a comment.

We're seeing new build errors in Chromium after this 
(http://crbug.com/1219457). Here's a reduced example:

  $ cat /tmp/a.mm
  #include 
  
  struct Callback {
// Move-only!
Callback(const Callback&) = delete;
Callback& operator=(const Callback&) = delete;
Callback(Callback&&) = default;
Callback& operator=(Callback&&) = default;
  
void operator()() {}
  };
  
  void f(Callback c) {
__block auto x = std::move(c);
  
(void)^(void) {
  x();
};
  }
  
  $ build.release/bin/clang++ -c /tmp/a.mm -target x86_64-apple-darwin10 
-stdlib=libc++
  /tmp/a.mm:14:16: error: call to deleted constructor of 'Callback'
__block auto x = std::move(c);
 ^
  /tmp/a.mm:5:3: note: 'Callback' has been explicitly marked deleted here
Callback(const Callback&) = delete;
^
  1 error generated.

I don't know enough about blocks to know whether the new error makes sense or 
not.

+rjmccall who knows about blocks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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

[PATCH] D98068: Remove asserts for LocalInstantiationScope

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

ping

I will provide a detailed description of the issue:

What happen are:

First, fun1 is instantiated as `fun1<0,1>`, which calls lambda function `f(f, 
Number<1>)`.

This causes lambda function `f` instantiated as `f(f, Number<1>)`. clang uses 
LocalInstantiationScope to track local variable instantiations, i.e., map var 
decls in the template to the corresponding var decls in the instantiation. In 
this case, clang adds two decls to the map: `fs` -> `f`, and `i` -> 
instantiation of `i` with type `Number<1>`.

This further causes instantiation of lambda function `f` as `f(f, Number<0>)` 
since `f` contains call of `fs(fs, Number<0>`.  clang adds two decls to its 
LocalInstantiationScope: `fs` -> `f`, and `i` -> instantiation of `i` with 
`Number<0>`.

Since `f` is lambda function, clang assumes its instantiation sees the decls in 
the LocalInstantiationScope of its enclosing template function instantiation, 
i.e. `f(f,Number<1>)`. However, clang assumes each decl can only show up in 
LocalInstantiationScope and any enclosing LocalInstantiationScope once, and 
clang checks and asserts that. In this case, since `f` shows up in 
LocalInstantiationScope of `f(f, Number<0>)` and `f(f, Number<1>)` twice. This 
causes assertion.

`LocalInstantiationScope' has a data member `CombineWithOuterScope`, which 
determines whether a template function instantiation sees the var decls 
instantiated in its enclosing function template instantiation. For non-lambda 
functions, this is false, therefor non-lambda functions will not have such 
issue. For non-recursive lambda function instantiation, such issue will not 
happen since the var decls will not show up twice.

Functionally, it seems to be OK to have var decls show up twice in 
`LocalInstantiationScope` for recursive labmda instantiations. For the inner 
level lambda instantiation, when looking up the var decls, the local var 
instantiation will be found first. The var instantiation in the outer 
LocalInstantiationScope will not be found. This is like the inner instantiation 
hides the outer instantiation. To me, it seems we only need to disable the 
assertion for the specific case of recursive lambda instantiation.

Any thoughts? Thanks.


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

https://reviews.llvm.org/D98068

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

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

[Arthur O'Dwyer via Phabricator via cfe-commits]

Quuxplusone added a comment.

@hans: FYI, that looks related to the immediately following D99005 
, not D99696  
specifically. I suspect that reverting D99005  
(but leaving D99696  in trunk) would suffice 
to fix that symptom. But I agree I don't see why it should be happening. That 
block code looks reasonable to me at first glance and wasn't intended to be 
affected by D99005  AFAIK.
@mizvekov: See why I'm skeptical about merging big patches back-to-back and/or 
on Fridays? ;)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

> @hans: FYI, that looks related to the immediately following D99005 
> , not D99696 
>  specifically.

No, my test case passes at 0d9e8f5f4b68252c6caa1ef81a30777b2f5d7242 
 but fails 
at 1e50c3d785f4563873ab1ce86559f2a1285b5678 
 (D99696 
), so this does seem to be the one that 
introduced the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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

[PATCH] D104198: [Matrix] Add documentation for compound assignment and type conversion of matrix types

[Florian Hahn via Phabricator via cfe-commits]

fhahn added a comment.

Thanks for the patch! some comments inline.




Comment at: clang/docs/LanguageExtensions.rst:527
+The matrix type extension supports compound assignments for addition, 
subtraction, and multiplication, provided
+their types are consistent.
+

it might also be helpful to include an example with matrix / scalar operations? 
Division is also supported for matrix/scalar combinations (but not for 
matrix/matrix)



Comment at: clang/docs/LanguageExtensions.rst:533
+
+  void f(m4x4_t a, m4x4_t b) {
+a += b;

perhaps return the result, so it is not a no-op?



Comment at: clang/docs/LanguageExtensions.rst:547
+
+  void f1(ix5x5 i, fx5x5 f) {
+f = (fx5x5)i;

can we instead return the casted value, so the code is not a no-op?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104198

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

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann updated this revision to Diff 351869.
mgartmann marked 10 inline comments as done.
mgartmann added a comment.

- changed name of check to `cppcoreguidelines-virtual-class-destructor`
- removed matcher for class or struct in AST matcher
- changed string concatenations to use `llvm::twine`
- adjusted documentation and removed unnecessary semicolons in it


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

Files:
  clang-tools-extra/clang-tidy/cppcoreguidelines/CMakeLists.txt
  clang-tools-extra/clang-tidy/cppcoreguidelines/CppCoreGuidelinesTidyModule.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/VirtualClassDestructorCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-virtual-class-destructor.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/cppcoreguidelines-virtual-class-destructor.cpp
@@ -0,0 +1,204 @@
+// RUN: %check_clang_tidy %s cppcoreguidelines-virtual-class-destructor %t -- --fix-notes
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateVirtualBaseStruct {
+  virtual void f();
+
+private:
+  virtual ~PrivateVirtualBaseStruct() {}
+};
+
+struct PublicVirtualBaseStruct { // OK
+  virtual void f();
+  virtual ~PublicVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualBaseStruct() {}
+  // CHECK-FIXES: ~ProtectedVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'ProtectedVirtualDefaultBaseStruct' is protected and virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it protected and non-virtual
+struct ProtectedVirtualDefaultBaseStruct {
+  virtual void f();
+
+protected:
+  virtual ~ProtectedVirtualDefaultBaseStruct() = default;
+  // CHECK-FIXES: ~ProtectedVirtualDefaultBaseStruct() = default;
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PrivateNonVirtualBaseStruct' is private and prevents using the type [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-MESSAGES: :[[@LINE+2]]:8: note: make it protected
+// As we have 2 conflicting fixes in notes, no fix is applied.
+struct PrivateNonVirtualBaseStruct {
+  virtual void f();
+
+private:
+  ~PrivateNonVirtualBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+2]]:8: warning: destructor of 'PublicNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+1]]:8: note: make it public and virtual
+struct PublicNonVirtualBaseStruct {
+  virtual void f();
+  ~PublicNonVirtualBaseStruct() {}
+  // CHECK-FIXES: virtual ~PublicNonVirtualBaseStruct() {}
+};
+
+struct PublicNonVirtualNonBaseStruct { // OK according to C.35, since this struct does not have any virtual methods.
+  void f();
+  ~PublicNonVirtualNonBaseStruct() {}
+};
+
+// CHECK-MESSAGES: :[[@LINE+4]]:8: warning: destructor of 'PublicImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+3]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicImplicitNonVirtualBaseStruct() = default;
+struct PublicImplicitNonVirtualBaseStruct {
+  virtual void f();
+};
+
+// CHECK-MESSAGES: :[[@LINE+5]]:8: warning: destructor of 'PublicASImplicitNonVirtualBaseStruct' is public and non-virtual [cppcoreguidelines-virtual-class-destructor]
+// CHECK-MESSAGES: :[[@LINE+4]]:8: note: make it public and virtual
+// CHECK-FIXES: struct PublicASImplicitNonVirtualBaseStruct {
+// CHECK-FIXES-NEXT: virtual ~PublicASImplicitNonVirtualBaseStruct() = default;
+// CHECK-FIXES-NEXT: private:
+struct PublicASImplicitNonVirtualBaseStruct {
+private:
+  virtual void f();
+};
+
+struct ProtectedNonVirtualBaseStruct { // OK
+  virtual void f();
+
+protected:

[PATCH] D102325: [clang-tidy] cppcoreguidelines-virtual-base-class-destructor: a new check

[Marco Gartmann via Phabricator via cfe-commits]

mgartmann added a comment.

Thanks a lot for your feedback, @aaron.ballman!

I was able to incorporate it in the updated diff.

On your advice, I also ran this check on a large open source project using the 
`run-clang-tidy.py` script. I decided to do it on the llvm project itself as 
follows:

  marco@nb:~/llvm-project/build$ python3 run-clang-tidy.py 
-checks="-*,cppcoreguidelines-virtual-class-destructor"  -header-filter=".*" > 
result.txt
  marco@nb:~/llvm-project/build$ cat result.txt | grep "private and prevents" | 
wc -l
  797

Most of these 797 lines are duplicates, since these warnings come from header 
files included in (apparently) many other files. 
Deduplicated, the following seven destructors were private and triggered this 
check's warning:

- llvm-project/llvm/include/llvm/Analysis/RegionInfo.h:1024:23: warning: 
destructor of 'RegionInfoBase>' is private 
...
- llvm-project/llvm/include/llvm/Analysis/RegionInfo.h:674:7: warning: 
destructor of 'RegionInfoBase' is private ...
- llvm-project/llvm/include/llvm/CodeGen/MachineRegionInfo.h:177:23: warning: 
destructor of 'RegionInfoBase>' is 
private ...
- llvm-project/llvm/lib/Target/AVR/MCTargetDesc/AVRMCExpr.h:19:7: warning: 
destructor of 'AVRMCExpr' is private ...
- llvm-project/llvm/utils/unittest/googletest/include/gtest/gtest.h:1257:18: 
warning: destructor of 'UnitTest' is private ...
- llvm-project/llvm/lib/CodeGen/InlineSpiller.cpp:159:7: warning: destructor of 
'InlineSpiller' is private ...
- llvm-project/llvm/lib/Target/X86/MCTargetDesc/X86MCTargetDesc.cpp:387:7: 
warning: destructor of 'X86MCInstrAnalysis' is private ...

I assume that these destructors are private by on purpose. Please correct me if 
I am wrong. 
A programmer working on the LLVM project would therefore see seven warnings for 
private destructors over the whole project.

Is this number of private destructors which are flagged acceptable to you?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102325

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

[clang] c60dd3b - Revert "[clang] NRVO: Improvements and handling of more cases."

[Hans Wennborg via cfe-commits]

Author: Hans Wennborg
Date: 2021-06-14T16:46:58+02:00
New Revision: c60dd3b2626a4d9eefd9f82f9a406b0d28d3fd72

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

LOG: Revert "[clang] NRVO: Improvements and handling of more cases."

This change caused build errors related to move-only __block variables,
see discussion on https://reviews.llvm.org/D99696

> This expands NRVO propagation for more cases:
>
> Parse analysis improvement:
> * Lambdas and Blocks with dependent return type can have their variables
>   marked as NRVO Candidates.
>
> Variable instantiation improvements:
> * Fixes crash when instantiating NRVO variables in Blocks.
> * Functions, Lambdas, and Blocks which have auto return type have their
>   variables' NRVO status propagated. For Blocks with non-auto return type,
>   as a limitation, this propagation does not consider the actual return
>   type.
>
> This also implements exclusion of VarDecls which are references to
> dependent types.
>
> Signed-off-by: Matheus Izvekov 
>
> Reviewed By: Quuxplusone
>
> Differential Revision: https://reviews.llvm.org/D99696

This also reverts the follow-on change which was hard to tease apart
form the one above:

> "[clang] Implement P2266 Simpler implicit move"
>
> This Implements 
> [[http://www.open-std.org/jtc1/sc22/wg21/docs/papers/2021/p2266r1.html|P2266 
> Simpler implicit move]].
>
> Signed-off-by: Matheus Izvekov 
>
> Reviewed By: Quuxplusone
>
> Differential Revision: https://reviews.llvm.org/D99005

This reverts commits 1e50c3d785f4563873ab1ce86559f2a1285b5678 and
bf20631782183cd19e0bb7219e908c2bbb01a75f.

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/SemaExprCXX.cpp
clang/lib/Sema/SemaStmt.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/SemaType.cpp
clang/test/CXX/class/class.init/class.copy.elision/p3.cpp
clang/test/CXX/dcl.dcl/dcl.spec/dcl.type/dcl.spec.auto/p7-cxx14.cpp
clang/test/CXX/drs/dr3xx.cpp
clang/test/CXX/expr/expr.prim/expr.prim.lambda/p4-cxx14.cpp
clang/test/CXX/temp/temp.decls/temp.mem/p5.cpp
clang/test/CodeGen/nrvo-tracking.cpp
clang/test/SemaCXX/constant-expression-cxx11.cpp
clang/test/SemaCXX/constant-expression-cxx14.cpp
clang/test/SemaCXX/coroutine-rvo.cpp
clang/test/SemaCXX/coroutines.cpp
clang/test/SemaCXX/deduced-return-type-cxx14.cpp
clang/test/SemaCXX/return-stack-addr.cpp
clang/test/SemaCXX/warn-return-std-move.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index db389922ae3a1..6ade9d7691266 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -3455,6 +3455,12 @@ class Sema final {
   bool DiagnoseMultipleUserDefinedConversion(Expr *From, QualType ToType);
   bool isSameOrCompatibleFunctionType(CanQualType Param, CanQualType Arg);
 
+  ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
+ const VarDecl *NRVOCandidate,
+ QualType ResultType,
+ Expr *Value,
+ bool AllowNRVO = true);
+
   bool CanPerformAggregateInitializationForOverloadResolution(
   const InitializedEntity &Entity, InitListExpr *From);
 
@@ -4754,30 +4760,28 @@ class Sema final {
SourceLocation Loc,
unsigned NumParams);
 
-  struct NamedReturnInfo {
-const VarDecl *Candidate;
-
-enum Status : uint8_t { None, MoveEligible, MoveEligibleAndCopyElidable };
-Status S;
-
-bool isMoveEligible() const { return S != None; };
-bool isCopyElidable() const { return S == MoveEligibleAndCopyElidable; }
+  enum CopyElisionSemanticsKind {
+CES_Strict = 0,
+CES_AllowParameters = 1,
+CES_AllowDifferentTypes = 2,
+CES_AllowExceptionVariables = 4,
+CES_AllowRValueReferenceType = 8,
+CES_ImplicitlyMovableCXX11CXX14CXX17 =
+(CES_AllowParameters | CES_AllowDifferentTypes),
+CES_ImplicitlyMovableCXX20 =
+(CES_AllowParameters | CES_AllowDifferentTypes |
+ CES_AllowExceptionVariables | CES_AllowRValueReferenceType),
   };
-  NamedReturnInfo getNamedReturnInfo(Expr *&E, bool ForceCXX2b = false);
-  NamedReturnInfo getNamedReturnInfo(const VarDecl *VD,
- bool ForceCXX20 = false);
-  const VarDecl *getCopyElisionCandidate(NamedReturnInfo &Info,
- QualType ReturnType);
 
-  ExprResult PerformMoveOrCopyInitialization(const InitializedEntity &Entity,
-   

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.

Since it seems more discussion is needed here, I've reverted in 
c60dd3b2626a4d9eefd9f82f9a406b0d28d3fd72 
. Since 
they were hard to tease apart, the revert is for both D99696 
 and D99005 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

[Francois JEAN via Phabricator via cfe-commits]

Wawha added inline comments.



Comment at: clang/lib/Format/TokenAnnotator.cpp:3760
-  (isAllmanBraceIncludedBreakableLambda(Left, ShortLambdaOption) ||
-   isAllmanBraceIncludedBreakableLambda(Right, ShortLambdaOption))) {
-return true;

Without this code, you should be able to remove functions 
isAllmanBraceIncludedBreakableLambda(), isItAInlineLambdaAllowed() and 
isOneChildWithoutMustBreakBefore().

Thank you for the fix.

When I implement that part, I have some difficulty to handle inline lambda and 
BreakBeforeLambdaBody. At the beginning, I implement the solution without 
inline lambda option (not yet present). But I have to handle it before merging 
the patch. And, this modification was added at this moment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104222

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

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Sorry for the absence, took my time to catch up with this series! Really 
interesting insight into how a new interface is being designed! I need some 
time to digest things, but can't wait to help on the remaining patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103605

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

[PATCH] D103605: [analyzer] Introduce a new interface for tracking

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103605#2816979 , @Szelethus wrote:

> Sorry for the absence, took my time to catch up with this series! Really 
> interesting insight into how a new interface is being designed! I need some 
> time to digest things, but can't wait to help on the remaining patches.

Hey, thanks!  Here is one example of how it can be useful for checkers (D104136 
), maybe it can help to form a full picture.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103605

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

[PATCH] D99696: [clang] NRVO: Improvements and handling of more cases.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

When `__block` variables get moved to the heap, they're supposed to be moved if 
possible, not copied.  It looks like PerformMoveOrCopyInitialization requires 
NRVO info to be passed in to ever do a move.  Maybe it's being passed in wrong 
when building `__block` copy expressions in some situation.

Was there really not a test case covering those semantics?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D99696

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

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

@Wawha do you have a project that perhaps uses this work your did? I would like 
to ensure I didn't break anything


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104222

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

[PATCH] D103792: [clang][AST] Set correct DeclContext in ASTImporter lookup table for template params.

[Balázs Kéri via Phabricator via cfe-commits]

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:5961-5962
 
   if (Error Err = ImportDeclParts(D, DC, LexicalDC, Name, ToD, Loc))
 return std::move(Err);
 

martong wrote:
> Do you think that the alternative approach that is used with 
> `TypedefNameDecl` could work here?
> ```
>   // Do not import the DeclContext, we will import it once the TypedefNameDecl
>   // is created.
>   if (Error Err = ImportDeclParts(D, Name, ToD, Loc))
> return std::move(Err);
> ```
> With that perhaps we could avoid the hassle with the `update`. Could you 
> please investigate that?
The problem is not with the DC of the `FunctionTemplateDecl`, it is with the 
template parameters. It looks not safe to pass nullptr as `DeclContext` for 
these, this is only possible if the import is done. (Probably the solution in 
`TypedefNameDecl` is not perfect: If something exists already in the "To TU" 
that was not imported, specially a namespace that contains a typedef with same 
name, it may not be found before the import.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103792

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

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay updated this revision to Diff 351895.
MyDeveloperDay added a comment.

Remove unused functions


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

https://reviews.llvm.org/D104222

Files:
  clang/lib/Format/TokenAnnotator.cpp
  clang/unittests/Format/FormatTest.cpp

Index: clang/unittests/Format/FormatTest.cpp
===
--- clang/unittests/Format/FormatTest.cpp
+++ clang/unittests/Format/FormatTest.cpp
@@ -18856,8 +18856,7 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
-  verifyFormat("void Fct()\n"
-   "{\n"
+  verifyFormat("void Fct() {\n"
"  return {[]()\n"
"  {\n"
"return 17;\n"
@@ -19061,6 +19060,16 @@
"  });\n"
"});",
LLVMWithBeforeLambdaBody);
+
+  LLVMWithBeforeLambdaBody.AllowShortLambdasOnASingleLine =
+  FormatStyle::ShortLambdaStyle::SLS_None;
+  verifyFormat("namespace test {\n"
+   "class Test {\n"
+   "public:\n"
+   "  Test() = default;\n"
+   "};\n"
+   "} // namespace test",
+   LLVMWithBeforeLambdaBody);
 }
 
 TEST_F(FormatTest, LambdaWithLineComments) {
Index: clang/lib/Format/TokenAnnotator.cpp
===
--- clang/lib/Format/TokenAnnotator.cpp
+++ clang/lib/Format/TokenAnnotator.cpp
@@ -3557,42 +3557,11 @@
   return Tok.Children.empty() && ShortLambdaOption != FormatStyle::SLS_None;
 }
 
-static bool
-isItAInlineLambdaAllowed(const FormatToken &Tok,
- FormatStyle::ShortLambdaStyle ShortLambdaOption) {
-  return (ShortLambdaOption == FormatStyle::SLS_Inline &&
-  IsFunctionArgument(Tok)) ||
- (ShortLambdaOption == FormatStyle::SLS_All);
-}
-
-static bool isOneChildWithoutMustBreakBefore(const FormatToken &Tok) {
-  if (Tok.Children.size() != 1)
-return false;
-  FormatToken *curElt = Tok.Children[0]->First;
-  while (curElt) {
-if (curElt->MustBreakBefore)
-  return false;
-curElt = curElt->Next;
-  }
-  return true;
-}
 static bool isAllmanLambdaBrace(const FormatToken &Tok) {
   return (Tok.is(tok::l_brace) && Tok.is(BK_Block) &&
   !Tok.isOneOf(TT_ObjCBlockLBrace, TT_DictLiteral));
 }
 
-static bool isAllmanBraceIncludedBreakableLambda(
-const FormatToken &Tok, FormatStyle::ShortLambdaStyle ShortLambdaOption) {
-  if (!isAllmanLambdaBrace(Tok))
-return false;
-
-  if (isItAnEmptyLambdaAllowed(Tok, ShortLambdaOption))
-return false;
-
-  return !isItAInlineLambdaAllowed(Tok, ShortLambdaOption) ||
- !isOneChildWithoutMustBreakBefore(Tok);
-}
-
 bool TokenAnnotator::mustBreakBefore(const AnnotatedLine &Line,
  const FormatToken &Right) {
   const FormatToken &Left = *Right.Previous;
@@ -3754,13 +3723,6 @@
   if (Right.is(TT_InlineASMBrace))
 return Right.HasUnescapedNewline;
 
-  auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
-  if (Style.BraceWrapping.BeforeLambdaBody &&
-  (isAllmanBraceIncludedBreakableLambda(Left, ShortLambdaOption) ||
-   isAllmanBraceIncludedBreakableLambda(Right, ShortLambdaOption))) {
-return true;
-  }
-
   if (isAllmanBrace(Left) || isAllmanBrace(Right))
 return (Line.startsWith(tok::kw_enum) && Style.BraceWrapping.AfterEnum) ||
(Line.startsWith(tok::kw_typedef, tok::kw_enum) &&
@@ -4186,7 +4148,7 @@
 return false;
 
   auto ShortLambdaOption = Style.AllowShortLambdasOnASingleLine;
-  if (Style.BraceWrapping.BeforeLambdaBody) {
+  if (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) {
 if (isAllmanLambdaBrace(Left))
   return !isItAnEmptyLambdaAllowed(Left, ShortLambdaOption);
 if (isAllmanLambdaBrace(Right))
@@ -4198,7 +4160,6 @@
  Right.isMemberAccess() ||
  Right.isOneOf(TT_TrailingReturnArrow, TT_LambdaArrow, tok::lessless,
tok::colon, tok::l_square, tok::at) ||
- (Style.BraceWrapping.BeforeLambdaBody && Right.is(TT_LambdaLBrace)) ||
  (Left.is(tok::r_paren) &&
   Right.isOneOf(tok::identifier, tok::kw_const)) ||
  (Left.is(tok::l_paren) && !Right.is(tok::r_paren)) ||
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104222: [clang-format] [PR50702] Lamdba processing does not respect AfterClass and AfterNamespace

[Zhihao Yuan via Phabricator via cfe-commits]

lichray added a comment.

LGTM


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

https://reviews.llvm.org/D104222

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

[PATCH] D103021: [clang-tidy] performance-unnecessary-copy-initialization: Search whole function body for variable initializations.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnnecessaryCopyInitialization.cpp:98-101
   auto Matches =
   match(findAll(declStmt(has(varDecl(equalsNode(&InitializingVar
 .bind("declStmt")),
+Body, Context);

flx wrote:
> ymandel wrote:
> > flx wrote:
> > > ymandel wrote:
> > > > Consider inspecting the `DeclContext`s instead, which should be much 
> > > > more efficient than searching the entire block.  Pass the 
> > > > `FunctionDecl` as an argument instead of `Body`, since it is a 
> > > > `DeclContext`.  e.g. `const DeclContext &Fun`
> > > > 
> > > > Then, either
> > > > 1. Call `Fun.containsDecl(InitializingVar)`, or
> > > > 2. Search through the contexts yourself; something like:
> > > > ```
> > > > DeclContext* DC = InitializingVar->getDeclContext(); 
> > > > while (DC != nullptr && DC != &Fun)
> > > >   DC = DC->getLexicalParent();
> > > > if (DC == nullptr)
> > > >   // The reference or pointer is not initialized anywhere witin the 
> > > > function. We
> > > >   // assume its pointee is not modified then.
> > > >   return true;
> > > > ```
> > > Are #1 and #2 equivalent? From the implementation and comment I cannot 
> > > tell whether #1 would cover cases where the variable is not declared 
> > > directly in the function, but in child block:
> > > 
> > > ```
> > > void Fun() {
> > >  {
> > >var i;
> > >{
> > >  i.usedHere();
> > >}  
> > >  } 
> > > }
> > > ```
> > > 
> > > I'm also reading this as an optimization to more quickly determine 
> > > whether we can stop here. We still need to find the matches for the next 
> > > steps, but I  think I could then limit matching to the DeclContext I 
> > > found here. Is this correct? For this I would actually need the 
> > > DeclContext result from #2.
> > A. I think you're right that #2 is more suited to what you need. I wasn't 
> > sure, so included both. Agreed that the comments are ambiguous.
> > B. yes, this is just an optimization. it may be premature for that matter; 
> > just that match can be expensive and this seemed a more direct expression 
> > of the algorithm.
> I was not able to pass the (possibly more narrow) DeclContext to the match 
> function as scope since match does not support DeclContexts.
> 
> I implemented  isDeclaredInFunction() which iterates through the decl 
> contexts as you suggested. I'm not sure though whether we should start with 
> VarDecl::getDeclContext() or VarDecl::getLexicalDeclContext()?
> 
> While looking at VarDecl::getLexicalDeclContext() I discovered is VarDecl has 
> the following method:
> 
> ```
>   /// Returns true for local variable declarations other than parameters. 
>
>   /// Note that this includes static variables inside of functions. It also   
>
>   /// includes variables inside blocks.   
>
>   /// 
>
>   ///   void foo() { int x; static int y; extern int z; } 
>
>   bool isLocalVarDecl() const;
> ```
> 
> I think this is exactly what we'd want here. What do you think?
> 
You mean instead of `isDeclaredInFunction`?  If so -- yes, that seems right.  
But, if so, are you still planning to bind "declStmt" with the `match`, or will 
you replace that with something more direct?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103021

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

[PATCH] D104116: AMD k8 family does not support SSE4.x which are required by x86-64-v2+

[Stella Stamenova via Phabricator via cfe-commits]

stella.stamenova added a comment.

This change is generating warnings when building with clang:

  /usr/bin/clang++ -DCLANG_ROUND_TRIP_CC1_ARGS=ON -DGTEST_HAS_RTTI=0 -D_DEBUG 
-D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
-D__STDC_LIMIT_MACROS -Itools/clang/lib/Basic 
-I/mnt/vss/_work/1/s/clang/lib/Basic -I/mnt/vss/_work/1/s/clang/include 
-Itools/clang/include -Iinclude -I/mnt/vss/_work/1/s/llvm/include -fPIC 
-fvisibility-inlines-hidden -Werror -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -fdiagnostics-color -fno-common -Woverloaded-virtual 
-Wno-nested-anon-types -g  -fno-exceptions -fno-rtti -std=c++14 -MD -MT 
tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Targets/X86.cpp.o -MF 
tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Targets/X86.cpp.o.d -o 
tools/clang/lib/Basic/CMakeFiles/obj.clangBasic.dir/Targets/X86.cpp.o -c 
/mnt/vss/_work/1/s/clang/lib/Basic/Targets/X86.cpp
  /mnt/vss/_work/1/s/clang/lib/Basic/Targets/X86.cpp:396:11: error: enumeration 
values 'CK_x86_64_v2', 'CK_x86_64_v3', and 'CK_x86_64_v4' not handled in switch 
[-Werror,-Wswitch]
switch (CPU) {

Rather than removing the cases from the switch entirely, they should still be 
handled without defining k8.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104116

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

[PATCH] D103087: [clang-tidy] performances-unnecessary-* checks: Extend isOnlyUsedAsConst to expressions and catch const methods returning non-const references/pointers.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D103087#2813901 , @flx wrote:

> In D103087#2793673 , @ymandel wrote:
>
>> I have some concerns about the cost of this checks as it used matching over 
>> entire contexts quite extensively.  At this point, the facilities involved 
>> seem quite close to doing dataflow analysis and I wonder if you might be 
>> better off with a very different implementation. Regardless, have you done 
>> any perfomance testing to see the impact on real code?
>
> That's a fair point. Is there prior art in terms of dataflow analysis in 
> ClangTidy or LLVM I could take a look at?

Added Dmitri to speak to prior art.

> In terms of measuring performance, do you have suggestions how to measure 
> this? I can add a counter that counts the recursion depth that is reached to 
> see how often this happens in practice.

I would simply run clang-tidy over a reasonable size set of files and see the 
timing w/ and w/o this change. But, I think that clang-tidy may have some 
built-in perf monitoring as well (specifically, a way to display the cost of 
each check, or the top most costly ones).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103087

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

[PATCH] D104240: [OPENMP][C++20]Add support for CXXRewrittenBinaryOperator in ranged for loops.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added reviewers: jdoerfert, mikerice.
Herald added subscribers: guansong, yaxunl.
ABataev requested review of this revision.
Herald added a subscriber: sstefan1.
Herald added a project: clang.

Added support for CXXRewrittenBinaryOperator as a condition in ranged
for loops. This is a new kind of expression, need to extend support for

  C++20 constructs.

It fixes PR49970: range-based for compilation fails for libstdc++ vector
with -std=c++20.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104240

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/for_ast_print_cxx20.cpp

Index: clang/test/OpenMP/for_ast_print_cxx20.cpp
===
--- /dev/null
+++ clang/test/OpenMP/for_ast_print_cxx20.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -verify -fopenmp --std=c++20 -ast-print %s -Wsign-conversion | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++20 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -std=c++20 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// RUN: %clang_cc1 -verify -fopenmp-simd --std=c++20 -ast-print %s -Wsign-conversion | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++20 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -std=c++20 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template  class iterator {
+public:
+  T &operator*() const;
+  iterator &operator++();
+};
+template 
+bool operator==(const iterator &, const iterator &);
+template 
+unsigned long operator-(const iterator &, const iterator &);
+template 
+iterator operator+(const iterator &, unsigned long);
+class vector {
+public:
+  vector();
+  iterator begin();
+  iterator end();
+};
+// CHECK: void foo() {
+void foo() {
+// CHECK-NEXT: vector vec;
+  vector vec;
+// CHECK-NEXT: #pragma omp for
+#pragma omp for
+// CHECK-NEXT: for (int i : vec)
+  for (int i : vec)
+;
+}
+#endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7668,53 +7668,43 @@
   Condition = S;
   S = getExprAsWritten(S);
   SourceLocation CondLoc = S->getBeginLoc();
-  if (auto *BO = dyn_cast(S)) {
-if (BO->isRelationalOp()) {
-  if (getInitLCDecl(BO->getLHS()) == LCDecl)
-return setUB(BO->getRHS(),
- (BO->getOpcode() == BO_LT || BO->getOpcode() == BO_LE),
- (BO->getOpcode() == BO_LT || BO->getOpcode() == BO_GT),
- BO->getSourceRange(), BO->getOperatorLoc());
-  if (getInitLCDecl(BO->getRHS()) == LCDecl)
-return setUB(BO->getLHS(),
- (BO->getOpcode() == BO_GT || BO->getOpcode() == BO_GE),
- (BO->getOpcode() == BO_LT || BO->getOpcode() == BO_GT),
- BO->getSourceRange(), BO->getOperatorLoc());
-} else if (IneqCondIsCanonical && BO->getOpcode() == BO_NE)
-  return setUB(
-  getInitLCDecl(BO->getLHS()) == LCDecl ? BO->getRHS() : BO->getLHS(),
-  /*LessOp=*/llvm::None,
-  /*StrictOp=*/true, BO->getSourceRange(), BO->getOperatorLoc());
+  auto &&CheckAndSetCond = [this, IneqCondIsCanonical](
+   BinaryOperatorKind Opcode, const Expr *LHS,
+   const Expr *RHS, SourceRange SR,
+   SourceLocation OpLoc) -> llvm::Optional {
+if (BinaryOperator::isRelationalOp(Opcode)) {
+  if (getInitLCDecl(LHS) == LCDecl)
+return setUB(const_cast(RHS),
+ (Opcode == BO_LT || Opcode == BO_LE),
+ (Opcode == BO_LT || Opcode == BO_GT), SR, OpLoc);
+  if (getInitLCDecl(RHS) == LCDecl)
+return setUB(const_cast(LHS),
+ (Opcode == BO_GT || Opcode == BO_GE),
+ (Opcode == BO_LT || Opcode == BO_GT), SR, OpLoc);
+} else if (IneqCondIsCanonical && Opcode == BO_NE) {
+  return setUB(const_cast(getInitLCDecl(LHS) == LCDecl ? RHS : LHS),
+   /*LessOp=*/llvm::None,
+   /*StrictOp=*/true, SR, OpLoc);
+}
+return llvm::None;
+  };
+  llvm::Optional Res;
+  if (auto *RBO = dyn_cast(S)) {
+CXXRewrittenBinaryOperator::DecomposedForm DF = RBO->getDecomposedForm();
+Res = CheckAndSetCond(DF.Opcode, DF.LHS, DF.RHS, RBO->getSourceRange(),
+  RBO->getOperatorLoc());
+  } else if (auto *BO = dyn_cast(S)) {
+Res = CheckAndSetCond(BO->getOpcode(), BO->getLHS(), BO->getRHS(),
+  BO->getSourceRange(), BO->getOperatorLoc());
   } else if (auto *CE = dyn_cast(S)) {
 if (CE->getNumArgs() == 2) {
-  auto Op = CE->getOperator();
-  switch (Op) {
-  case OO_Greater:
-  case OO_GreaterEqual:
-  case OO_Less:
-  case OO_LessEqual:
-if (getInitLC

[PATCH] D103702: [AArch64][SVE] Wire up vscale_range attribute to SVE min/max vector queries

[Bradley Smith via Phabricator via cfe-commits]

bsmith updated this revision to Diff 351910.
bsmith marked 7 inline comments as done.
bsmith added a comment.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

- Move attribute/command line logic into AArch64TargetMachine.
- Fix issue with subtarget key appending integers.
- Add end-to-end test.
- Make new subtarget options optional.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103702

Files:
  clang/test/CodeGen/aarch64-sve-vector-bits-codegen.c
  llvm/lib/Target/AArch64/AArch64Subtarget.cpp
  llvm/lib/Target/AArch64/AArch64Subtarget.h
  llvm/lib/Target/AArch64/AArch64TargetMachine.cpp
  llvm/test/CodeGen/AArch64/sve-vscale-attr.ll

Index: llvm/test/CodeGen/AArch64/sve-vscale-attr.ll
===
--- /dev/null
+++ llvm/test/CodeGen/AArch64/sve-vscale-attr.ll
@@ -0,0 +1,144 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc < %s | FileCheck %s --check-prefixes=CHECK,CHECK-NOARG
+; RUN: llc -aarch64-sve-vector-bits-min=512 < %s | FileCheck %s --check-prefixes=CHECK,CHECK-ARG
+
+target triple = "aarch64-unknown-linux-gnu"
+
+define void @func_vscale_none(<16 x i32>* %a, <16 x i32>* %b) #0 {
+; CHECK-NOARG-LABEL: func_vscale_none:
+; CHECK-NOARG:   // %bb.0:
+; CHECK-NOARG-NEXT:ldp q0, q1, [x0]
+; CHECK-NOARG-NEXT:ldp q2, q3, [x1]
+; CHECK-NOARG-NEXT:ldp q4, q5, [x0, #32]
+; CHECK-NOARG-NEXT:ldp q7, q6, [x1, #32]
+; CHECK-NOARG-NEXT:add v1.4s, v1.4s, v3.4s
+; CHECK-NOARG-NEXT:add v0.4s, v0.4s, v2.4s
+; CHECK-NOARG-NEXT:add v2.4s, v5.4s, v6.4s
+; CHECK-NOARG-NEXT:add v3.4s, v4.4s, v7.4s
+; CHECK-NOARG-NEXT:stp q3, q2, [x0, #32]
+; CHECK-NOARG-NEXT:stp q0, q1, [x0]
+; CHECK-NOARG-NEXT:ret
+;
+; CHECK-ARG-LABEL: func_vscale_none:
+; CHECK-ARG:   // %bb.0:
+; CHECK-ARG-NEXT:ptrue p0.s, vl16
+; CHECK-ARG-NEXT:ld1w { z0.s }, p0/z, [x0]
+; CHECK-ARG-NEXT:ld1w { z1.s }, p0/z, [x1]
+; CHECK-ARG-NEXT:add z0.s, p0/m, z0.s, z1.s
+; CHECK-ARG-NEXT:st1w { z0.s }, p0, [x0]
+; CHECK-ARG-NEXT:ret
+  %op1 = load <16 x i32>, <16 x i32>* %a
+  %op2 = load <16 x i32>, <16 x i32>* %b
+  %res = add <16 x i32> %op1, %op2
+  store <16 x i32> %res, <16 x i32>* %a
+  ret void
+}
+
+attributes #0 = { "target-features"="+sve" }
+
+define void @func_vscale1_1(<16 x i32>* %a, <16 x i32>* %b) #1 {
+; CHECK-LABEL: func_vscale1_1:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:ldp q0, q1, [x0]
+; CHECK-NEXT:ldp q2, q3, [x1]
+; CHECK-NEXT:ldp q4, q5, [x0, #32]
+; CHECK-NEXT:ldp q7, q6, [x1, #32]
+; CHECK-NEXT:add v1.4s, v1.4s, v3.4s
+; CHECK-NEXT:add v0.4s, v0.4s, v2.4s
+; CHECK-NEXT:add v2.4s, v5.4s, v6.4s
+; CHECK-NEXT:add v3.4s, v4.4s, v7.4s
+; CHECK-NEXT:stp q3, q2, [x0, #32]
+; CHECK-NEXT:stp q0, q1, [x0]
+; CHECK-NEXT:ret
+  %op1 = load <16 x i32>, <16 x i32>* %a
+  %op2 = load <16 x i32>, <16 x i32>* %b
+  %res = add <16 x i32> %op1, %op2
+  store <16 x i32> %res, <16 x i32>* %a
+  ret void
+}
+
+attributes #1 = { "target-features"="+sve" vscale_range(1,1) }
+
+define void @func_vscale2_2(<16 x i32>* %a, <16 x i32>* %b) #2 {
+; CHECK-LABEL: func_vscale2_2:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:ptrue p0.s, vl8
+; CHECK-NEXT:add x8, x0, #32 // =32
+; CHECK-NEXT:add x9, x1, #32 // =32
+; CHECK-NEXT:ld1w { z0.s }, p0/z, [x0]
+; CHECK-NEXT:ld1w { z1.s }, p0/z, [x8]
+; CHECK-NEXT:ld1w { z2.s }, p0/z, [x1]
+; CHECK-NEXT:ld1w { z3.s }, p0/z, [x9]
+; CHECK-NEXT:add z0.s, p0/m, z0.s, z2.s
+; CHECK-NEXT:add z1.s, p0/m, z1.s, z3.s
+; CHECK-NEXT:st1w { z0.s }, p0, [x0]
+; CHECK-NEXT:st1w { z1.s }, p0, [x8]
+; CHECK-NEXT:ret
+  %op1 = load <16 x i32>, <16 x i32>* %a
+  %op2 = load <16 x i32>, <16 x i32>* %b
+  %res = add <16 x i32> %op1, %op2
+  store <16 x i32> %res, <16 x i32>* %a
+  ret void
+}
+
+attributes #2 = { "target-features"="+sve" vscale_range(2,2) }
+
+define void @func_vscale2_4(<16 x i32>* %a, <16 x i32>* %b) #3 {
+; CHECK-LABEL: func_vscale2_4:
+; CHECK:   // %bb.0:
+; CHECK-NEXT:ptrue p0.s, vl8
+; CHECK-NEXT:add x8, x0, #32 // =32
+; CHECK-NEXT:add x9, x1, #32 // =32
+; CHECK-NEXT:ld1w { z0.s }, p0/z, [x0]
+; CHECK-NEXT:ld1w { z1.s }, p0/z, [x8]
+; CHECK-NEXT:ld1w { z2.s }, p0/z, [x1]
+; CHECK-NEXT:ld1w { z3.s }, p0/z, [x9]
+; CHECK-NEXT:add z0.s, p0/m, z0.s, z2.s
+; CHECK-NEXT:add z1.s, p0/m, z1.s, z3.s
+; CHECK-NEXT:st1w { z0.s }, p0, [x0]
+; CHECK-NEXT:st1w { z1.s }, p0, [x8]
+; CHECK-NEXT:ret
+  %op1 = load <16 x i32>, <16 x i32>* %a
+  %op2 = load <16 x i32>, <16 x i32>* %b
+  %res = add <16 x i32> %op1, %op2
+  store <16 x i32> %res, <16 x i32>* %a
+  ret void
+}
+
+attributes #3 = { "target-features"="+sve" vscale_range(2,4) }
+
+define void @func_vscale4_4(<16 x i32>* %a, <16 x i32>* %b) #4 {
+; CHECK-LABEL: f

[PATCH] D103702: [AArch64][SVE] Wire up vscale_range attribute to SVE min/max vector queries

[Bradley Smith via Phabricator via cfe-commits]

bsmith added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.cpp:226
+ : SVEVectorBitsMaxOpt),
   TargetTriple(TT), FrameLowering(),
   InstrInfo(initializeSubtargetDependencies(FS, CPU)), TSInfo(),

sdesmalen wrote:
> nit: Does this need an assert that Min|MaxSVEVectorSizeInBits is zero when 
> SVE is not enabled in the feature string?
In principal you could, however I'm not sure it adds any value, as the 
accessors already assert that SVE is enabled. (And in principal this is a 
generic attribute, not an SVE one).



Comment at: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp:357
+  Attribute VScaleRangeAttr = F.getFnAttribute(Attribute::VScaleRange);
+  if (VScaleRangeAttr.isValid())
+std::tie(MinSVEVectorSize, MaxSVEVectorSize) =

paulwalker-arm wrote:
> I don't know if this is possible but I feel we need a `HasSVE` like check 
> here?
I'm not sure this is really doable here without picking apart the feature 
string, I think it makes more sense to just set the values and assert when 
using the accessors without SVE enabled.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103702

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

[PATCH] D104242: Removes an unused variable and alters a lit test to simplify and avoid a buildbot error

[Fred Grim via Phabricator via cfe-commits]

feg208 created this revision.
feg208 added reviewers: HazardyKnusperkeks, Bigcheese.
feg208 requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104242

Files:
  clang/lib/Format/WhitespaceManager.cpp
  clang/test/Format/struct-array-initializer.cpp


Index: clang/test/Format/struct-array-initializer.cpp
===
--- clang/test/Format/struct-array-initializer.cpp
+++ clang/test/Format/struct-array-initializer.cpp
@@ -1,9 +1,5 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Right}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Left}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Right}" %s | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: 
Left}" %s | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
 struct test {
   int a;
   int b;
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1068,9 +1068,6 @@
 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces;
   ++CellIter;
   for (auto i = 1U; i < CellDescs.CellCount; i++, ++CellIter) {
-unsigned NetWidth = 0U;
-if (isSplitCell(*CellIter))
-  NetWidth = getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
 auto MaxNetWidth = getMaximumNetWidth(
 Cells.begin(), CellIter, CellDescs.InitialSpaces, CellDescs.CellCount);
 auto ThisNetWidth =


Index: clang/test/Format/struct-array-initializer.cpp
===
--- clang/test/Format/struct-array-initializer.cpp
+++ clang/test/Format/struct-array-initializer.cpp
@@ -1,9 +1,5 @@
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Right}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
-// RUN: grep -Ev "// *[A-Z-]+:" %s \
-// RUN:   | clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Left}" %s \
-// RUN:   | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Right}" %s | FileCheck -strict-whitespace -check-prefix=CHECK1 %s
+// RUN: clang-format -style="{BasedOnStyle: LLVM, AlignArrayOfStructures: Left}" %s | FileCheck -strict-whitespace -check-prefix=CHECK2 %s
 struct test {
   int a;
   int b;
Index: clang/lib/Format/WhitespaceManager.cpp
===
--- clang/lib/Format/WhitespaceManager.cpp
+++ clang/lib/Format/WhitespaceManager.cpp
@@ -1068,9 +1068,6 @@
 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces;
   ++CellIter;
   for (auto i = 1U; i < CellDescs.CellCount; i++, ++CellIter) {
-unsigned NetWidth = 0U;
-if (isSplitCell(*CellIter))
-  NetWidth = getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
 auto MaxNetWidth = getMaximumNetWidth(
 Cells.begin(), CellIter, CellDescs.InitialSpaces, CellDescs.CellCount);
 auto ThisNetWidth =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D104125: [PowerPC] Moving defineXLCompatMacros() definition

[Lei Huang via Phabricator via cfe-commits]

lei accepted this revision.
lei added a comment.
This revision is now accepted and ready to land.

LGTM
Thanks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104125

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

[PATCH] D103702: [AArch64][SVE] Wire up vscale_range attribute to SVE min/max vector queries

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64Subtarget.h:298-299
+   bool LittleEndian,
+   unsigned MinSVEVectorSizeInBitsOverride = 0,
+   unsigned MaxSVEVectorSizeInBitsOverride = 0);
 

Out of interest are these defaults ever relied upon?



Comment at: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp:380-386
+  assert(MinSVEVectorSize % 128 == 0 &&
+ "SVE requires vector length in multiples of 128!");
+  assert(MaxSVEVectorSize % 128 == 0 &&
+ "SVE requires vector length in multiples of 128!");
+  assert((MaxSVEVectorSize >= MinSVEVectorSize ||
+  MaxSVEVectorSize == 0) &&
+ "Minimum SVE vector size should not be larger than its maximum!");

These asserts are fine but you'll see from the original implementations of 
`getM..SVEVectorSizeInBits` that I do not rely on the user passing the correct 
values.  Instead I also always process the sizes to ensure the values of 
`MinSVEVectorSize` and `MaxSVEVectorSize`are sane.  Can you do likewise here?



Comment at: llvm/lib/Target/AArch64/AArch64TargetMachine.cpp:357
+  Attribute VScaleRangeAttr = F.getFnAttribute(Attribute::VScaleRange);
+  if (VScaleRangeAttr.isValid())
+std::tie(MinSVEVectorSize, MaxSVEVectorSize) =

bsmith wrote:
> paulwalker-arm wrote:
> > I don't know if this is possible but I feel we need a `HasSVE` like check 
> > here?
> I'm not sure this is really doable here without picking apart the feature 
> string, I think it makes more sense to just set the values and assert when 
> using the accessors without SVE enabled.
Fair enough.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103702

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

[clang] 20f7b5f - [Clang] Test case for -Wunused-but-set-variable, warn for volatile.

[George Burgess IV via cfe-commits]

Author: Michael Benfield
Date: 2021-06-14T10:25:59-07:00
New Revision: 20f7b5f3f9c8ebbbe7bf6648c824b815385d4bf7

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

LOG: [Clang] Test case for -Wunused-but-set-variable, warn for volatile.

Differential Revision: https://reviews.llvm.org/D103623

Added: 


Modified: 
clang/test/Sema/warn-unused-but-set-variables.c

Removed: 




diff  --git a/clang/test/Sema/warn-unused-but-set-variables.c 
b/clang/test/Sema/warn-unused-but-set-variables.c
index 93595a251df0..a8d05243321f 100644
--- a/clang/test/Sema/warn-unused-but-set-variables.c
+++ b/clang/test/Sema/warn-unused-but-set-variables.c
@@ -23,6 +23,10 @@ int f0() {
   int a;
   w = (a = 0);
 
+  // Following gcc, warn for a volatile variable.
+  volatile int b; // expected-warning{{variable 'b' set but not used}}
+  b = 0;
+
   int x;
   x = 0;
   return x;



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

[PATCH] D103623: [Clang] Test case for -Wunused-but-set-variable, warn for volatile.

[George Burgess IV via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG20f7b5f3f9c8: [Clang] Test case for 
-Wunused-but-set-variable, warn for volatile. (authored by mbenfield, committed 
by george.burgess.iv).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103623

Files:
  clang/test/Sema/warn-unused-but-set-variables.c


Index: clang/test/Sema/warn-unused-but-set-variables.c
===
--- clang/test/Sema/warn-unused-but-set-variables.c
+++ clang/test/Sema/warn-unused-but-set-variables.c
@@ -23,6 +23,10 @@
   int a;
   w = (a = 0);
 
+  // Following gcc, warn for a volatile variable.
+  volatile int b; // expected-warning{{variable 'b' set but not used}}
+  b = 0;
+
   int x;
   x = 0;
   return x;


Index: clang/test/Sema/warn-unused-but-set-variables.c
===
--- clang/test/Sema/warn-unused-but-set-variables.c
+++ clang/test/Sema/warn-unused-but-set-variables.c
@@ -23,6 +23,10 @@
   int a;
   w = (a = 0);
 
+  // Following gcc, warn for a volatile variable.
+  volatile int b; // expected-warning{{variable 'b' set but not used}}
+  b = 0;
+
   int x;
   x = 0;
   return x;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[Tim Wojtulewicz via Phabricator via cfe-commits]

timwoj added a comment.

> A crash (https://bugs.llvm.org/show_bug.cgi?id=50663) in 12.0.1 is fixed by 
> your changes here when merged to the 12 branch, I'm not sure if this needs to 
> be backported to 12 I guess it might depend on if we think it's critical 
> enough or if there will more 12 release candidates before 13 @timwoj any 
> thoughts?

It should apply cleanly to 12.0 as far as I know, since if I remember right I 
had asked if it could be merged before 12.0 was released. I don't have a whole 
lot of free cycles at the moment to help with it if it needs a lot of changes 
though. My vote is yes to backport it, if it counts for anything.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94500

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

[clang] 44f197e - [OpenMP] Fix C-only clang assert on parsing use_allocator clause of target directive

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2021-06-14T10:36:27-07:00
New Revision: 44f197e94b83d389b59ce6a2a1977f972e6d34e3

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

LOG: [OpenMP] Fix C-only clang assert on parsing use_allocator clause of target 
directive

The parser code assumes building with C++ compiler and asserts when using clang 
(not clang++) on C file. I made the code dependent on input language. This 
shows up for amdgpu target.

Reviewed By: ABataev

Differential Revision: https://reviews.llvm.org/D103899

Added: 
clang/test/OpenMP/target_uses_allocators.c

Modified: 
clang/lib/Parse/ParseOpenMP.cpp

Removed: 




diff  --git a/clang/lib/Parse/ParseOpenMP.cpp b/clang/lib/Parse/ParseOpenMP.cpp
index d3a456fca49c..d2152fd11793 100644
--- a/clang/lib/Parse/ParseOpenMP.cpp
+++ b/clang/lib/Parse/ParseOpenMP.cpp
@@ -2708,7 +2708,8 @@ OMPClause 
*Parser::ParseOpenMPUsesAllocatorClause(OpenMPDirectiveKind DKind) {
 return nullptr;
   SmallVector Data;
   do {
-ExprResult Allocator = ParseCXXIdExpression();
+ExprResult Allocator =
+getLangOpts().CPlusPlus ? ParseCXXIdExpression() : ParseExpression();
 if (Allocator.isInvalid()) {
   SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,
 StopBeforeMatch);
@@ -2720,7 +2721,8 @@ OMPClause 
*Parser::ParseOpenMPUsesAllocatorClause(OpenMPDirectiveKind DKind) {
   BalancedDelimiterTracker T(*this, tok::l_paren,
  tok::annot_pragma_openmp_end);
   T.consumeOpen();
-  ExprResult AllocatorTraits = ParseCXXIdExpression();
+  ExprResult AllocatorTraits =
+  getLangOpts().CPlusPlus ? ParseCXXIdExpression() : ParseExpression();
   T.consumeClose();
   if (AllocatorTraits.isInvalid()) {
 SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,

diff  --git a/clang/test/OpenMP/target_uses_allocators.c 
b/clang/test/OpenMP/target_uses_allocators.c
new file mode 100644
index ..5d1aaa37d845
--- /dev/null
+++ b/clang/test/OpenMP/target_uses_allocators.c
@@ -0,0 +1,44 @@
+// Test host codegen.
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50  -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu
+
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+enum omp_allocator_handle_t {
+  omp_null_allocator = 0,
+  omp_default_mem_alloc = 1,
+  omp_large_cap_mem_alloc = 2,
+  omp_const_mem_alloc = 3,
+  omp_high_bw_mem_alloc = 4,
+  omp_low_lat_mem_alloc = 5,
+  omp_cgroup_mem_alloc = 6,
+  omp_pteam_mem_alloc = 7,
+  omp_thread_mem_alloc = 8,
+  KMP_ALLOCATOR_MAX_HANDLE = __UINTPTR_MAX__
+};
+
+// CHECK: define {{.*}}[[FIE:@.+]]()
+void fie() {
+  int x;
+  #pragma omp target uses_allocators(omp_null_allocator) 
allocate(omp_null_allocator: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_default_mem_alloc) 
allocate(omp_default_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_large_cap_mem_alloc) 
allocate(omp_large_cap_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_const_mem_alloc) 
allocate(omp_const_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_high_bw_mem_alloc) 
allocate(omp_high_bw_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_low_lat_mem_alloc) 
allocate(omp_low_lat_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_cgroup_mem_alloc) 
allocate(omp_cgroup_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_pteam_mem_alloc) 
allocate(omp_pteam_mem_alloc: x) firstprivate(x)
+  {}
+}
+
+#endif



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

[PATCH] D103899: [OpenMP] Fix C-only clang assert on parsing use_allocator clause of target directive

[Alexey Bataev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG44f197e94b83: [OpenMP] Fix C-only clang assert on parsing 
use_allocator clause of target… (authored by ABataev).
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103899

Files:
  clang/lib/Parse/ParseOpenMP.cpp
  clang/test/OpenMP/target_uses_allocators.c


Index: clang/test/OpenMP/target_uses_allocators.c
===
--- /dev/null
+++ clang/test/OpenMP/target_uses_allocators.c
@@ -0,0 +1,44 @@
+// Test host codegen.
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50  -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu 
-emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple 
powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu
+
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+enum omp_allocator_handle_t {
+  omp_null_allocator = 0,
+  omp_default_mem_alloc = 1,
+  omp_large_cap_mem_alloc = 2,
+  omp_const_mem_alloc = 3,
+  omp_high_bw_mem_alloc = 4,
+  omp_low_lat_mem_alloc = 5,
+  omp_cgroup_mem_alloc = 6,
+  omp_pteam_mem_alloc = 7,
+  omp_thread_mem_alloc = 8,
+  KMP_ALLOCATOR_MAX_HANDLE = __UINTPTR_MAX__
+};
+
+// CHECK: define {{.*}}[[FIE:@.+]]()
+void fie() {
+  int x;
+  #pragma omp target uses_allocators(omp_null_allocator) 
allocate(omp_null_allocator: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_default_mem_alloc) 
allocate(omp_default_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_large_cap_mem_alloc) 
allocate(omp_large_cap_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_const_mem_alloc) 
allocate(omp_const_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_high_bw_mem_alloc) 
allocate(omp_high_bw_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_low_lat_mem_alloc) 
allocate(omp_low_lat_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_cgroup_mem_alloc) 
allocate(omp_cgroup_mem_alloc: x) firstprivate(x)
+  {}
+  #pragma omp target uses_allocators(omp_pteam_mem_alloc) 
allocate(omp_pteam_mem_alloc: x) firstprivate(x)
+  {}
+}
+
+#endif
Index: clang/lib/Parse/ParseOpenMP.cpp
===
--- clang/lib/Parse/ParseOpenMP.cpp
+++ clang/lib/Parse/ParseOpenMP.cpp
@@ -2708,7 +2708,8 @@
 return nullptr;
   SmallVector Data;
   do {
-ExprResult Allocator = ParseCXXIdExpression();
+ExprResult Allocator =
+getLangOpts().CPlusPlus ? ParseCXXIdExpression() : ParseExpression();
 if (Allocator.isInvalid()) {
   SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,
 StopBeforeMatch);
@@ -2720,7 +2721,8 @@
   BalancedDelimiterTracker T(*this, tok::l_paren,
  tok::annot_pragma_openmp_end);
   T.consumeOpen();
-  ExprResult AllocatorTraits = ParseCXXIdExpression();
+  ExprResult AllocatorTraits =
+  getLangOpts().CPlusPlus ? ParseCXXIdExpression() : ParseExpression();
   T.consumeClose();
   if (AllocatorTraits.isInvalid()) {
 SkipUntil(tok::comma, tok::r_paren, tok::annot_pragma_openmp_end,


Index: clang/test/OpenMP/target_uses_allocators.c
===
--- /dev/null
+++ clang/test/OpenMP/target_uses_allocators.c
@@ -0,0 +1,44 @@
+// Test host codegen.
+// RUN: %clang_cc1 -verify -fopenmp -fopenmp-version=50  -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-llvm %s -o - | FileCheck %s --check-prefix CHECK --check-prefix CHECK-64
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -fopenmp-version=50 -x c++ -triple powerpc64le-unknown-unknown -fopenmp-targets=powerpc64le-ibm-linux-gnu
+
+// expected-no-diagnostics
+#ifndef HEADER
+#define HEADER
+
+enum omp_allocator_handle_t {
+  omp_null_allocator = 0,
+  omp_default_mem_alloc = 1,
+  omp_large_cap_mem_alloc = 2,
+  omp_const_mem_alloc = 3,
+  omp_high_bw_mem_alloc = 4,
+  omp_low_lat_mem_alloc = 5,
+  omp_cgroup_mem_alloc = 6,
+  omp_pteam_mem_alloc = 7,
+  omp_thread_mem_alloc = 8,
+  KMP_ALLOCATOR_MAX_HANDLE = __UINTPTR_MAX__
+};
+
+// CHECK: define {{.*}}[[FIE:@.+]]()
+void fie() {
+  int x;
+  #pragma omp target uses_allocators(omp_null_

[clang] d650ccf - [NFC] Remove unused variable

[Vitaly Buka via cfe-commits]

Author: Vitaly Buka
Date: 2021-06-14T10:57:26-07:00
New Revision: d650ccf6390bb1e4454dd735cfcec9eda9af8ca3

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

LOG: [NFC] Remove unused variable

To fix 'set but not used' warning on sanitizer-x86_64-linux-android bot.

Added: 


Modified: 
clang/lib/Format/WhitespaceManager.cpp

Removed: 




diff  --git a/clang/lib/Format/WhitespaceManager.cpp 
b/clang/lib/Format/WhitespaceManager.cpp
index 222c4c4e8e29a..b079eac9803c4 100644
--- a/clang/lib/Format/WhitespaceManager.cpp
+++ b/clang/lib/Format/WhitespaceManager.cpp
@@ -1068,9 +1068,6 @@ void 
WhitespaceManager::alignArrayInitializersLeftJustified(
 Changes[CellIter->Index].Spaces = CellDescs.InitialSpaces;
   ++CellIter;
   for (auto i = 1U; i < CellDescs.CellCount; i++, ++CellIter) {
-unsigned NetWidth = 0U;
-if (isSplitCell(*CellIter))
-  NetWidth = getNetWidth(Cells.begin(), CellIter, CellDescs.InitialSpaces);
 auto MaxNetWidth = getMaximumNetWidth(
 Cells.begin(), CellIter, CellDescs.InitialSpaces, CellDescs.CellCount);
 auto ThisNetWidth =



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

[PATCH] D94500: [clang-format] Rework Whitesmiths mode to use line-level values in UnwrappedLineParser

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a subscriber: tstellar.
MyDeveloperDay added a comment.

I think we've missed the deadline for 12.0.1 which I believe was last friday. I 
did a practice merge and it didn't quite go in cleanly to 12.0.X branch

Normally we would mark the bug in the bug tracker for the next release, this is 
what I did for another bug and @tstellar I think picked it up and merge it for 
us.

I'm not sure if we are doing a 12.0.2 release but I've added that as the 
"Blocks" tag in the bug tracker, one of us can merge it if Tom wants us to.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D94500

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

[PATCH] D103936: [compiler-rt][hwasan] Define fuchsia implementations of required hwasan functions

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 351931.
leonardchan marked 10 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103936

Files:
  compiler-rt/lib/hwasan/CMakeLists.txt
  compiler-rt/lib/hwasan/hwasan_fuchsia.cpp


Index: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
===
--- /dev/null
+++ compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
@@ -0,0 +1,68 @@
+//===-- hwasan_fuchsia.cpp --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file
+/// This file is a part of HWAddressSanitizer and contains Fuchsia-specific
+/// code.
+///
+//===--===//
+
+#include "sanitizer_common/sanitizer_fuchsia.h"
+#if SANITIZER_FUCHSIA
+
+#include "hwasan.h"
+#include "hwasan_interface_internal.h"
+#include "hwasan_report.h"
+#include "hwasan_thread.h"
+#include "hwasan_thread_list.h"
+
+namespace __hwasan {
+
+bool InitShadow() {
+  // TODO(leonardchan): Remove the runtime variable altogether once we 
implement
+  // something similar to SHADOW_OFFSET for hwasan. Ideally, the runtime would
+  // just know this value is zero everywhere it's used.
+  __hwasan_shadow_memory_dynamic_address = 0;
+
+  // Call this explicitly to set the ShadowBounds global so we can reference it
+  // now. Otherwise, ShadowBounds will be a zero-initialized global.
+  ShadowBounds = __sanitizer_shadow_bounds();
+  CHECK_NE(__sanitizer::ShadowBounds.shadow_limit, 0);
+
+  return true;
+}
+
+bool MemIsApp(uptr p) {
+  CHECK(GetTagFromPointer(p) == 0);
+  return __sanitizer::ShadowBounds.shadow_limit <= p &&
+ p <= __sanitizer::GetMaxUserVirtualAddress();
+}
+
+// Not implemented because Fuchsia does not use signal handlers.
+void HwasanOnDeadlySignal(int signo, void *info, void *context) {}
+
+// Not implemented because Fuchsia does not use interceptors.
+void InitializeInterceptors() {}
+
+// Not implemented because this is only relevant for Android.
+void AndroidTestTlsSlot() {}
+
+// TSD was normally used on linux as a means of calling the hwasan thread exit
+// handler passed to pthread_key_create. This is not needed on Fuchsia because
+// we will be using __sanitizer_thread_exit_hook.
+void HwasanTSDInit() {}
+void HwasanTSDThreadInit() {}
+
+// On linux, this just would call `atexit(HwasanAtExit)`. The functions in
+// HwasanAtExit are unimplemented for Fuchsia and effectively no-ops, so this
+// function is unneeded.
+void InstallAtExitHandler() {}
+
+}  // namespace __hwasan
+
+#endif  // SANITIZER_FUCHSIA
Index: compiler-rt/lib/hwasan/CMakeLists.txt
===
--- compiler-rt/lib/hwasan/CMakeLists.txt
+++ compiler-rt/lib/hwasan/CMakeLists.txt
@@ -7,6 +7,7 @@
   hwasan_allocation_functions.cpp
   hwasan_dynamic_shadow.cpp
   hwasan_exceptions.cpp
+  hwasan_fuchsia.cpp
   hwasan_globals.cpp
   hwasan_interceptors.cpp
   hwasan_interceptors_vfork.S


Index: compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
===
--- /dev/null
+++ compiler-rt/lib/hwasan/hwasan_fuchsia.cpp
@@ -0,0 +1,68 @@
+//===-- hwasan_fuchsia.cpp --*- C++ -*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+///
+/// \file
+/// This file is a part of HWAddressSanitizer and contains Fuchsia-specific
+/// code.
+///
+//===--===//
+
+#include "sanitizer_common/sanitizer_fuchsia.h"
+#if SANITIZER_FUCHSIA
+
+#include "hwasan.h"
+#include "hwasan_interface_internal.h"
+#include "hwasan_report.h"
+#include "hwasan_thread.h"
+#include "hwasan_thread_list.h"
+
+namespace __hwasan {
+
+bool InitShadow() {
+  // TODO(leonardchan): Remove the runtime variable altogether once we implement
+  // something similar to SHADOW_OFFSET for hwasan. Ideally, the runtime would
+  // just know this value is zero everywhere it's used.
+  __hwasan_shadow_memory_dynamic_address = 0;
+
+  // Call this explicitly to set the ShadowBounds global so we can reference it
+  // now. Otherwise, ShadowBounds will be a zero-initialized global.
+  ShadowBounds = __sanitizer_shadow_bounds();
+  CHECK_NE(__sanitizer::ShadowBounds.shadow_limit, 0);
+
+  return true;
+}
+
+bool MemIsApp(uptr p) {
+  CHECK(GetTagFromPoint

[PATCH] D104240: [OPENMP][C++20]Add support for CXXRewrittenBinaryOperator in ranged for loops.

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert accepted this revision.
jdoerfert added a comment.
This revision is now accepted and ready to land.

LG, thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104240

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

[PATCH] D98068: Exampt asserts for recursive lambdas about LocalInstantiationScope

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 351934.
yaxunl retitled this revision from "Remove asserts for LocalInstantiationScope" 
to "Exampt asserts for recursive lambdas about LocalInstantiationScope".
yaxunl edited the summary of this revision.
yaxunl added a comment.

re-enable the assert but with exception for recursive lambda functions


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

https://reviews.llvm.org/D98068

Files:
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/test/SemaCXX/recursive-lambda.cpp


Index: clang/test/SemaCXX/recursive-lambda.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/recursive-lambda.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// Check recursive instantiation of lambda does not cause assertion.
+// lambda function `f` in `fun1` is instantiated twice: first
+// as f(f, Number<1>), then as f(f, Number<0>). The
+// LocalInstantiationScopes of these two instantiations both contain
+// `f` and `i`. Clang should not assert for that.
+
+template 
+struct Number
+{
+   static constexpr unsigned value = v;
+};
+
+template 
+constexpr auto fun1(Number = Number<0>{}, Number  = Number<1>{})
+{
+  auto f = [&](auto fs, auto i) {
+if constexpr(i.value > 0)
+{
+  return fs(fs, Number{});
+}
+  };
+
+  return f(f, Number{});
+}
+
+
+void fun2() {
+  fun1();
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3624,11 +3624,13 @@
   llvm::PointerUnion &Stored = LocalDecls[D];
   if (Stored.isNull()) {
 #ifndef NDEBUG
-// It should not be present in any surrounding scope either.
+// It should not be present in any surrounding scope either except for
+// recursive lambda functions.
 LocalInstantiationScope *Current = this;
 while (Current->CombineWithOuterScope && Current->Outer) {
   Current = Current->Outer;
-  assert(Current->LocalDecls.find(D) == Current->LocalDecls.end() &&
+  assert((isLambdaCallOperator(D->getDeclContext()) ||
+  Current->LocalDecls.find(D) == Current->LocalDecls.end()) &&
  "Instantiated local in inner and outer scopes");
 }
 #endif
@@ -3649,10 +3651,12 @@
 
 void LocalInstantiationScope::MakeInstantiatedLocalArgPack(const Decl *D) {
 #ifndef NDEBUG
-  // This should be the first time we've been told about this decl.
+  // This should be the first time we've been told about this decl except for
+  // recursive lambda functions.
   for (LocalInstantiationScope *Current = this;
Current && Current->CombineWithOuterScope; Current = Current->Outer)
-assert(Current->LocalDecls.find(D) == Current->LocalDecls.end() &&
+assert((isLambdaCallOperator(D->getDeclContext()) ||
+Current->LocalDecls.find(D) == Current->LocalDecls.end()) &&
"Creating local pack after instantiation of local");
 #endif
 


Index: clang/test/SemaCXX/recursive-lambda.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/recursive-lambda.cpp
@@ -0,0 +1,34 @@
+// RUN: %clang_cc1 -std=c++17 -fsyntax-only -verify %s
+
+// expected-no-diagnostics
+
+// Check recursive instantiation of lambda does not cause assertion.
+// lambda function `f` in `fun1` is instantiated twice: first
+// as f(f, Number<1>), then as f(f, Number<0>). The
+// LocalInstantiationScopes of these two instantiations both contain
+// `f` and `i`. Clang should not assert for that.
+
+template 
+struct Number
+{
+   static constexpr unsigned value = v;
+};
+
+template 
+constexpr auto fun1(Number = Number<0>{}, Number  = Number<1>{})
+{
+  auto f = [&](auto fs, auto i) {
+if constexpr(i.value > 0)
+{
+  return fs(fs, Number{});
+}
+  };
+
+  return f(f, Number{});
+}
+
+
+void fun2() {
+  fun1();
+}
Index: clang/lib/Sema/SemaTemplateInstantiate.cpp
===
--- clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -3624,11 +3624,13 @@
   llvm::PointerUnion &Stored = LocalDecls[D];
   if (Stored.isNull()) {
 #ifndef NDEBUG
-// It should not be present in any surrounding scope either.
+// It should not be present in any surrounding scope either except for
+// recursive lambda functions.
 LocalInstantiationScope *Current = this;
 while (Current->CombineWithOuterScope && Current->Outer) {
   Current = Current->Outer;
-  assert(Current->LocalDecls.find(D) == Current->LocalDecls.end() &&
+  assert((isLambdaCallOperator(D->getDeclContext()) ||
+  Current->LocalDecls.find(D) == Current->LocalDecls.end()) &&
  "Instantiated local in inner and outer scopes");
 }
 #endif
@@ -3649,10 +3651,12 @@
 
 void Local

[PATCH] D104248: [compiler-rt][hwasan] Move Thread::Init into hwasan_linux.cpp

[Leonard Chan via Phabricator via cfe-commits]

leonardchan created this revision.
leonardchan added reviewers: eugenis, vitalybuka, pcc.
leonardchan added a project: Sanitizers.
Herald added a subscriber: dberris.
leonardchan requested review of this revision.
Herald added a subscriber: Sanitizers.

This allows for other implementations to define their own version of 
`Thread::Init`. This will be the case for Fuchsia where much of the thread 
initialization can be broken up between different thread hooks 
(`__sanitizer_before_thread_create_hook`, `__sanitizer_thread_create_hook`, 
`__sanitizer_thread_start_hook`). Namely, setting up the heap ring buffer and 
stack info and can be setup before thread creation. The stack ring buffer can 
also be setup before thread creation, but storing it into `__hwasan_tls` can 
only be done on the thread start hook since it's only then we can access 
`__hwasan_tls` for that thread correctly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104248

Files:
  compiler-rt/lib/hwasan/hwasan_linux.cpp
  compiler-rt/lib/hwasan/hwasan_thread.cpp

Index: compiler-rt/lib/hwasan/hwasan_thread.cpp
===
--- compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -34,51 +34,6 @@
 stack_allocations_->push(0);
 }
 
-void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size) {
-  CHECK_EQ(0, unique_id_);  // try to catch bad stack reuse
-  CHECK_EQ(0, stack_top_);
-  CHECK_EQ(0, stack_bottom_);
-
-  static u64 unique_id;
-  unique_id_ = unique_id++;
-  if (auto sz = flags()->heap_history_size)
-heap_allocations_ = HeapAllocationsRingBuffer::New(sz);
-
-  HwasanTSDThreadInit();  // Only needed with interceptors.
-  uptr *ThreadLong = GetCurrentThreadLongPtr();
-  // The following implicitly sets (this) as the current thread.
-  stack_allocations_ = new (ThreadLong)
-  StackAllocationsRingBuffer((void *)stack_buffer_start, stack_buffer_size);
-  // Check that it worked.
-  CHECK_EQ(GetCurrentThread(), this);
-
-  // ScopedTaggingDisable needs GetCurrentThread to be set up.
-  ScopedTaggingDisabler disabler;
-
-  uptr tls_size;
-  uptr stack_size;
-  GetThreadStackAndTls(IsMainThread(), &stack_bottom_, &stack_size, &tls_begin_,
-   &tls_size);
-  stack_top_ = stack_bottom_ + stack_size;
-  tls_end_ = tls_begin_ + tls_size;
-
-  if (stack_bottom_) {
-int local;
-CHECK(AddrIsInStack((uptr)&local));
-CHECK(MemIsApp(stack_bottom_));
-CHECK(MemIsApp(stack_top_ - 1));
-  }
-
-  if (flags()->verbose_threads) {
-if (IsMainThread()) {
-  Printf("sizeof(Thread): %zd sizeof(HeapRB): %zd sizeof(StackRB): %zd\n",
- sizeof(Thread), heap_allocations_->SizeInBytes(),
- stack_allocations_->size() * sizeof(uptr));
-}
-Print("Creating  : ");
-  }
-}
-
 void Thread::ClearShadowForThreadStackAndTLS() {
   if (stack_top_ != stack_bottom_)
 TagMemory(stack_bottom_, stack_top_ - stack_bottom_, 0);
Index: compiler-rt/lib/hwasan/hwasan_linux.cpp
===
--- compiler-rt/lib/hwasan/hwasan_linux.cpp
+++ compiler-rt/lib/hwasan/hwasan_linux.cpp
@@ -427,6 +427,50 @@
   HandleDeadlySignal(info, context, GetTid(), &OnStackUnwind, nullptr);
 }
 
+void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size) {
+  CHECK_EQ(0, unique_id_);  // try to catch bad stack reuse
+  CHECK_EQ(0, stack_top_);
+  CHECK_EQ(0, stack_bottom_);
+
+  static u64 unique_id;
+  unique_id_ = unique_id++;
+  if (auto sz = flags()->heap_history_size)
+heap_allocations_ = HeapAllocationsRingBuffer::New(sz);
+
+  HwasanTSDThreadInit();  // Only needed with interceptors.
+  uptr *ThreadLong = GetCurrentThreadLongPtr();
+  // The following implicitly sets (this) as the current thread.
+  stack_allocations_ = new (ThreadLong)
+  StackAllocationsRingBuffer((void *)stack_buffer_start, stack_buffer_size);
+  // Check that it worked.
+  CHECK_EQ(GetCurrentThread(), this);
+
+  // ScopedTaggingDisable needs GetCurrentThread to be set up.
+  ScopedTaggingDisabler disabler;
+
+  uptr tls_size;
+  uptr stack_size;
+  GetThreadStackAndTls(IsMainThread(), &stack_bottom_, &stack_size, &tls_begin_,
+   &tls_size);
+  stack_top_ = stack_bottom_ + stack_size;
+  tls_end_ = tls_begin_ + tls_size;
+
+  if (stack_bottom_) {
+int local;
+CHECK(AddrIsInStack((uptr)&local));
+CHECK(MemIsApp(stack_bottom_));
+CHECK(MemIsApp(stack_top_ - 1));
+  }
+
+  if (flags()->verbose_threads) {
+if (IsMainThread()) {
+  Printf("sizeof(Thread): %zd sizeof(HeapRB): %zd sizeof(StackRB): %zd\n",
+ sizeof(Thread), heap_allocations_->SizeInBytes(),
+ stack_allocations_->size() * sizeof(uptr));
+}
+Print("Creating  : ");
+  }
+}
 
 } // namespace __hwasan
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bi

[PATCH] D103615: [Clang] Add option for vector compare compatibility.

[Bardia Mahjour via Phabricator via cfe-commits]

bmahjour added inline comments.



Comment at: clang/include/clang/Basic/LangOptions.h:248
+  enum class VectorCompareCompatKind {
+// Default clang behaviour.
+// All vector compares produce scalars except vector Pixel and vector bool.

How about adding `Default = Mixed` at the end and moving the comment with it?



Comment at: clang/include/clang/Basic/LangOptions.h:249
+// Default clang behaviour.
+// All vector compares produce scalars except vector Pixel and vector bool.
+// The types vector Pixel and vector bool return vector results.

Pixel -> pixel



Comment at: clang/include/clang/Basic/LangOptions.h:250
+// All vector compares produce scalars except vector Pixel and vector bool.
+// The types vector Pixel and vector bool return vector results.
+Mixed,

[nit] Pixel -> pixel



Comment at: clang/lib/Sema/SemaExpr.cpp:12221
+  // which behavior is prefered.
+  switch (getLangOpts().getVectorCompareCompat()) {
+  case LangOptions::VectorCompareCompatKind::Mixed:

Check for `getLangOpts().AltiVec` before the switch?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103615

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

[PATCH] D104248: [compiler-rt][hwasan] Move Thread::Init into hwasan_linux.cpp

[Roland McGrath via Phabricator via cfe-commits]

mcgrathr added inline comments.



Comment at: compiler-rt/lib/hwasan/hwasan_linux.cpp:430
 
+void Thread::Init(uptr stack_buffer_start, uptr stack_buffer_size) {
+  CHECK_EQ(0, unique_id_);  // try to catch bad stack reuse

Most of this code can actually be reused for Fuchsia (just not necessarily in 
Thread::Init).
It's probably better to split it up for reuse rather than just moving the whole 
thing to linux-specific.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104248

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

[PATCH] D104253: [Clang][Codegen] emit noprofile IR Fn Attr for no_instrument_function Fn Attr

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers created this revision.
nickdesaulniers added reviewers: MaskRay, melver, void.
Herald added a subscriber: wenlei.
nickdesaulniers requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

noprofile IR attribute already exists to prevent profiling with PGO;
emit that when a function uses the no_instrument_function function
attribute.

The Linux kernel would like to avoid compiler generated code in
functions annotated with such attribute. We already respect this for
libcalls to fentry() and mcount().

Link: 
https://lore.kernel.org/lkml/cakwvodmpti93n2l0_yqkrzldmpxzror7zggszonyaw2pgsh...@mail.gmail.com/
Signed-off-by: Nick Desaulniers 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D104253

Files:
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGen/fprofile-generate.c


Index: clang/test/CodeGen/fprofile-generate.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile-generate.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fprofile-instrument=llvm -emit-llvm -o - %s | FileCheck %s
+int g(int);
+
+int __attribute__((__no_instrument_function__))
+__attribute__((no_instrument_function))
+no_instr(int a) {
+// CHECK: define {{.*}} i32 @no_instr(i32 %a) [[ATTR:#[0-9]+]]
+  int sum = 0;
+  for (int i = 0; i < a; i++)
+sum += g(i);
+  return sum;
+}
+
+int instr(int a) {
+// CHECK: define {{.*}} i32 @instr(i32 %a) [[ATTR2:#[0-9]+]]
+  int sum = 0;
+  for (int i = 0; i < a; i++)
+sum += g(i);
+  return sum;
+}
+// CHECK: attributes [[ATTR]] = {{.*}} noprofile
+// CHECK-NOT: attributes [[ATTR2]] = {{.*}} noprofile
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))


Index: clang/test/CodeGen/fprofile-generate.c
===
--- /dev/null
+++ clang/test/CodeGen/fprofile-generate.c
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -fprofile-instrument=llvm -emit-llvm -o - %s | FileCheck %s
+int g(int);
+
+int __attribute__((__no_instrument_function__))
+__attribute__((no_instrument_function))
+no_instr(int a) {
+// CHECK: define {{.*}} i32 @no_instr(i32 %a) [[ATTR:#[0-9]+]]
+  int sum = 0;
+  for (int i = 0; i < a; i++)
+sum += g(i);
+  return sum;
+}
+
+int instr(int a) {
+// CHECK: define {{.*}} i32 @instr(i32 %a) [[ATTR2:#[0-9]+]]
+  int sum = 0;
+  for (int i = 0; i < a; i++)
+sum += g(i);
+  return sum;
+}
+// CHECK: attributes [[ATTR]] = {{.*}} noprofile
+// CHECK-NOT: attributes [[ATTR2]] = {{.*}} noprofile
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -893,6 +893,9 @@
   if (D && D->hasAttr())
 Fn->addFnAttr("cfi-canonical-jump-table");
 
+  if (D && D->hasAttr())
+Fn->addFnAttr(llvm::Attribute::NoProfile);
+
   if (getLangOpts().OpenCL) {
 // Add metadata for a kernel function.
 if (const FunctionDecl *FD = dyn_cast_or_null(D))
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 4e15560 - [OPENMP][C++20]Add support for CXXRewrittenBinaryOperator in ranged for loops.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2021-06-14T11:50:27-07:00
New Revision: 4e155608796b79d7e369f4e42980ce670bff7172

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

LOG: [OPENMP][C++20]Add support for CXXRewrittenBinaryOperator in ranged for 
loops.

Added support for CXXRewrittenBinaryOperator as a condition in ranged
for loops. This is a new kind of expression, need to extend support for
  C++20 constructs.
It fixes PR49970: range-based for compilation fails for libstdc++ vector
with -std=c++20.

Differential Revision: https://reviews.llvm.org/D104240

Added: 
clang/test/OpenMP/for_ast_print_cxx20.cpp

Modified: 
clang/lib/Sema/SemaOpenMP.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaOpenMP.cpp b/clang/lib/Sema/SemaOpenMP.cpp
index a67f0910bd07a..0e39b8ef572dc 100644
--- a/clang/lib/Sema/SemaOpenMP.cpp
+++ b/clang/lib/Sema/SemaOpenMP.cpp
@@ -7668,53 +7668,43 @@ bool OpenMPIterationSpaceChecker::checkAndSetCond(Expr 
*S) {
   Condition = S;
   S = getExprAsWritten(S);
   SourceLocation CondLoc = S->getBeginLoc();
-  if (auto *BO = dyn_cast(S)) {
-if (BO->isRelationalOp()) {
-  if (getInitLCDecl(BO->getLHS()) == LCDecl)
-return setUB(BO->getRHS(),
- (BO->getOpcode() == BO_LT || BO->getOpcode() == BO_LE),
- (BO->getOpcode() == BO_LT || BO->getOpcode() == BO_GT),
- BO->getSourceRange(), BO->getOperatorLoc());
-  if (getInitLCDecl(BO->getRHS()) == LCDecl)
-return setUB(BO->getLHS(),
- (BO->getOpcode() == BO_GT || BO->getOpcode() == BO_GE),
- (BO->getOpcode() == BO_LT || BO->getOpcode() == BO_GT),
- BO->getSourceRange(), BO->getOperatorLoc());
-} else if (IneqCondIsCanonical && BO->getOpcode() == BO_NE)
-  return setUB(
-  getInitLCDecl(BO->getLHS()) == LCDecl ? BO->getRHS() : BO->getLHS(),
-  /*LessOp=*/llvm::None,
-  /*StrictOp=*/true, BO->getSourceRange(), BO->getOperatorLoc());
+  auto &&CheckAndSetCond = [this, IneqCondIsCanonical](
+   BinaryOperatorKind Opcode, const Expr *LHS,
+   const Expr *RHS, SourceRange SR,
+   SourceLocation OpLoc) -> llvm::Optional {
+if (BinaryOperator::isRelationalOp(Opcode)) {
+  if (getInitLCDecl(LHS) == LCDecl)
+return setUB(const_cast(RHS),
+ (Opcode == BO_LT || Opcode == BO_LE),
+ (Opcode == BO_LT || Opcode == BO_GT), SR, OpLoc);
+  if (getInitLCDecl(RHS) == LCDecl)
+return setUB(const_cast(LHS),
+ (Opcode == BO_GT || Opcode == BO_GE),
+ (Opcode == BO_LT || Opcode == BO_GT), SR, OpLoc);
+} else if (IneqCondIsCanonical && Opcode == BO_NE) {
+  return setUB(const_cast(getInitLCDecl(LHS) == LCDecl ? RHS : 
LHS),
+   /*LessOp=*/llvm::None,
+   /*StrictOp=*/true, SR, OpLoc);
+}
+return llvm::None;
+  };
+  llvm::Optional Res;
+  if (auto *RBO = dyn_cast(S)) {
+CXXRewrittenBinaryOperator::DecomposedForm DF = RBO->getDecomposedForm();
+Res = CheckAndSetCond(DF.Opcode, DF.LHS, DF.RHS, RBO->getSourceRange(),
+  RBO->getOperatorLoc());
+  } else if (auto *BO = dyn_cast(S)) {
+Res = CheckAndSetCond(BO->getOpcode(), BO->getLHS(), BO->getRHS(),
+  BO->getSourceRange(), BO->getOperatorLoc());
   } else if (auto *CE = dyn_cast(S)) {
 if (CE->getNumArgs() == 2) {
-  auto Op = CE->getOperator();
-  switch (Op) {
-  case OO_Greater:
-  case OO_GreaterEqual:
-  case OO_Less:
-  case OO_LessEqual:
-if (getInitLCDecl(CE->getArg(0)) == LCDecl)
-  return setUB(CE->getArg(1), Op == OO_Less || Op == OO_LessEqual,
-   Op == OO_Less || Op == OO_Greater, CE->getSourceRange(),
-   CE->getOperatorLoc());
-if (getInitLCDecl(CE->getArg(1)) == LCDecl)
-  return setUB(CE->getArg(0), Op == OO_Greater || Op == 
OO_GreaterEqual,
-   Op == OO_Less || Op == OO_Greater, CE->getSourceRange(),
-   CE->getOperatorLoc());
-break;
-  case OO_ExclaimEqual:
-if (IneqCondIsCanonical)
-  return setUB(getInitLCDecl(CE->getArg(0)) == LCDecl ? CE->getArg(1)
-  : CE->getArg(0),
-   /*LessOp=*/llvm::None,
-   /*StrictOp=*/true, CE->getSourceRange(),
-   CE->getOperatorLoc());
-break;
-  default:
-break;
-  }
+  Res = CheckAndSetCond(
+  BinaryOperator::getOverloadedOpcode(CE->getOperator()), 
CE->getArg(0),

[PATCH] D104240: [OPENMP][C++20]Add support for CXXRewrittenBinaryOperator in ranged for loops.

[Alexey Bataev via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG4e155608796b: [OPENMP][C++20]Add support for 
CXXRewrittenBinaryOperator in ranged for loops. (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104240

Files:
  clang/lib/Sema/SemaOpenMP.cpp
  clang/test/OpenMP/for_ast_print_cxx20.cpp

Index: clang/test/OpenMP/for_ast_print_cxx20.cpp
===
--- /dev/null
+++ clang/test/OpenMP/for_ast_print_cxx20.cpp
@@ -0,0 +1,40 @@
+// RUN: %clang_cc1 -verify -fopenmp --std=c++20 -ast-print %s -Wsign-conversion | FileCheck %s
+// RUN: %clang_cc1 -fopenmp -x c++ -std=c++20 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp -std=c++20 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+
+// RUN: %clang_cc1 -verify -fopenmp-simd --std=c++20 -ast-print %s -Wsign-conversion | FileCheck %s
+// RUN: %clang_cc1 -fopenmp-simd -x c++ -std=c++20 -emit-pch -o %t %s
+// RUN: %clang_cc1 -fopenmp-simd -std=c++20 -include-pch %t -fsyntax-only -verify %s -ast-print | FileCheck %s
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+template  class iterator {
+public:
+  T &operator*() const;
+  iterator &operator++();
+};
+template 
+bool operator==(const iterator &, const iterator &);
+template 
+unsigned long operator-(const iterator &, const iterator &);
+template 
+iterator operator+(const iterator &, unsigned long);
+class vector {
+public:
+  vector();
+  iterator begin();
+  iterator end();
+};
+// CHECK: void foo() {
+void foo() {
+// CHECK-NEXT: vector vec;
+  vector vec;
+// CHECK-NEXT: #pragma omp for
+#pragma omp for
+// CHECK-NEXT: for (int i : vec)
+  for (int i : vec)
+;
+}
+#endif
Index: clang/lib/Sema/SemaOpenMP.cpp
===
--- clang/lib/Sema/SemaOpenMP.cpp
+++ clang/lib/Sema/SemaOpenMP.cpp
@@ -7668,53 +7668,43 @@
   Condition = S;
   S = getExprAsWritten(S);
   SourceLocation CondLoc = S->getBeginLoc();
-  if (auto *BO = dyn_cast(S)) {
-if (BO->isRelationalOp()) {
-  if (getInitLCDecl(BO->getLHS()) == LCDecl)
-return setUB(BO->getRHS(),
- (BO->getOpcode() == BO_LT || BO->getOpcode() == BO_LE),
- (BO->getOpcode() == BO_LT || BO->getOpcode() == BO_GT),
- BO->getSourceRange(), BO->getOperatorLoc());
-  if (getInitLCDecl(BO->getRHS()) == LCDecl)
-return setUB(BO->getLHS(),
- (BO->getOpcode() == BO_GT || BO->getOpcode() == BO_GE),
- (BO->getOpcode() == BO_LT || BO->getOpcode() == BO_GT),
- BO->getSourceRange(), BO->getOperatorLoc());
-} else if (IneqCondIsCanonical && BO->getOpcode() == BO_NE)
-  return setUB(
-  getInitLCDecl(BO->getLHS()) == LCDecl ? BO->getRHS() : BO->getLHS(),
-  /*LessOp=*/llvm::None,
-  /*StrictOp=*/true, BO->getSourceRange(), BO->getOperatorLoc());
+  auto &&CheckAndSetCond = [this, IneqCondIsCanonical](
+   BinaryOperatorKind Opcode, const Expr *LHS,
+   const Expr *RHS, SourceRange SR,
+   SourceLocation OpLoc) -> llvm::Optional {
+if (BinaryOperator::isRelationalOp(Opcode)) {
+  if (getInitLCDecl(LHS) == LCDecl)
+return setUB(const_cast(RHS),
+ (Opcode == BO_LT || Opcode == BO_LE),
+ (Opcode == BO_LT || Opcode == BO_GT), SR, OpLoc);
+  if (getInitLCDecl(RHS) == LCDecl)
+return setUB(const_cast(LHS),
+ (Opcode == BO_GT || Opcode == BO_GE),
+ (Opcode == BO_LT || Opcode == BO_GT), SR, OpLoc);
+} else if (IneqCondIsCanonical && Opcode == BO_NE) {
+  return setUB(const_cast(getInitLCDecl(LHS) == LCDecl ? RHS : LHS),
+   /*LessOp=*/llvm::None,
+   /*StrictOp=*/true, SR, OpLoc);
+}
+return llvm::None;
+  };
+  llvm::Optional Res;
+  if (auto *RBO = dyn_cast(S)) {
+CXXRewrittenBinaryOperator::DecomposedForm DF = RBO->getDecomposedForm();
+Res = CheckAndSetCond(DF.Opcode, DF.LHS, DF.RHS, RBO->getSourceRange(),
+  RBO->getOperatorLoc());
+  } else if (auto *BO = dyn_cast(S)) {
+Res = CheckAndSetCond(BO->getOpcode(), BO->getLHS(), BO->getRHS(),
+  BO->getSourceRange(), BO->getOperatorLoc());
   } else if (auto *CE = dyn_cast(S)) {
 if (CE->getNumArgs() == 2) {
-  auto Op = CE->getOperator();
-  switch (Op) {
-  case OO_Greater:
-  case OO_GreaterEqual:
-  case OO_Less:
-  case OO_LessEqual:
-if (getInitLCDecl(CE->getArg(0)) == LCDecl)
-  return setUB(CE->getArg(1), Op == OO_Less || Op == OO_LessEqual,
-   Op == OO_Less || Op == O

[PATCH] D104248: [compiler-rt][hwasan] Move Thread::Init into hwasan_linux.cpp

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 351953.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104248

Files:
  compiler-rt/lib/hwasan/hwasan_thread.cpp
  compiler-rt/lib/hwasan/hwasan_thread.h


Index: compiler-rt/lib/hwasan/hwasan_thread.h
===
--- compiler-rt/lib/hwasan/hwasan_thread.h
+++ compiler-rt/lib/hwasan/hwasan_thread.h
@@ -23,8 +23,13 @@
 
 class Thread {
  public:
-  void Init(uptr stack_buffer_start, uptr stack_buffer_size);  // Must be 
called from the thread itself.
+  void Init(uptr stack_buffer_start, uptr stack_buffer_size);
   void InitRandomState();
+  void InitStackAndTls();
+
+  // Must be called from the thread itself.
+  void InitStackRingBuffer(uptr stack_buffer_start, uptr stack_buffer_size);
+
   void Destroy();
 
   uptr stack_top() { return stack_top_; }
Index: compiler-rt/lib/hwasan/hwasan_thread.cpp
===
--- compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -44,6 +44,20 @@
   if (auto sz = flags()->heap_history_size)
 heap_allocations_ = HeapAllocationsRingBuffer::New(sz);
 
+  InitStackAndTls();
+  InitStackAndRingBuffer(stack_buffer_start, stack_buffer_size);
+}
+
+void Thread::InitStackAndTls() {
+  uptr tls_size;
+  uptr stack_size;
+  GetThreadStackAndTls(IsMainThread(), &stack_bottom_, &stack_size, 
&tls_begin_,
+   &tls_size);
+  stack_top_ = stack_bottom_ + stack_size;
+  tls_end_ = tls_begin_ + tls_size;
+}
+
+void Thread::InitStackRingBuffer(uptr stack_buffer_start, uptr 
stack_buffer_size) {
   HwasanTSDThreadInit();  // Only needed with interceptors.
   uptr *ThreadLong = GetCurrentThreadLongPtr();
   // The following implicitly sets (this) as the current thread.
@@ -55,13 +69,6 @@
   // ScopedTaggingDisable needs GetCurrentThread to be set up.
   ScopedTaggingDisabler disabler;
 
-  uptr tls_size;
-  uptr stack_size;
-  GetThreadStackAndTls(IsMainThread(), &stack_bottom_, &stack_size, 
&tls_begin_,
-   &tls_size);
-  stack_top_ = stack_bottom_ + stack_size;
-  tls_end_ = tls_begin_ + tls_size;
-
   if (stack_bottom_) {
 int local;
 CHECK(AddrIsInStack((uptr)&local));


Index: compiler-rt/lib/hwasan/hwasan_thread.h
===
--- compiler-rt/lib/hwasan/hwasan_thread.h
+++ compiler-rt/lib/hwasan/hwasan_thread.h
@@ -23,8 +23,13 @@
 
 class Thread {
  public:
-  void Init(uptr stack_buffer_start, uptr stack_buffer_size);  // Must be called from the thread itself.
+  void Init(uptr stack_buffer_start, uptr stack_buffer_size);
   void InitRandomState();
+  void InitStackAndTls();
+
+  // Must be called from the thread itself.
+  void InitStackRingBuffer(uptr stack_buffer_start, uptr stack_buffer_size);
+
   void Destroy();
 
   uptr stack_top() { return stack_top_; }
Index: compiler-rt/lib/hwasan/hwasan_thread.cpp
===
--- compiler-rt/lib/hwasan/hwasan_thread.cpp
+++ compiler-rt/lib/hwasan/hwasan_thread.cpp
@@ -44,6 +44,20 @@
   if (auto sz = flags()->heap_history_size)
 heap_allocations_ = HeapAllocationsRingBuffer::New(sz);
 
+  InitStackAndTls();
+  InitStackAndRingBuffer(stack_buffer_start, stack_buffer_size);
+}
+
+void Thread::InitStackAndTls() {
+  uptr tls_size;
+  uptr stack_size;
+  GetThreadStackAndTls(IsMainThread(), &stack_bottom_, &stack_size, &tls_begin_,
+   &tls_size);
+  stack_top_ = stack_bottom_ + stack_size;
+  tls_end_ = tls_begin_ + tls_size;
+}
+
+void Thread::InitStackRingBuffer(uptr stack_buffer_start, uptr stack_buffer_size) {
   HwasanTSDThreadInit();  // Only needed with interceptors.
   uptr *ThreadLong = GetCurrentThreadLongPtr();
   // The following implicitly sets (this) as the current thread.
@@ -55,13 +69,6 @@
   // ScopedTaggingDisable needs GetCurrentThread to be set up.
   ScopedTaggingDisabler disabler;
 
-  uptr tls_size;
-  uptr stack_size;
-  GetThreadStackAndTls(IsMainThread(), &stack_bottom_, &stack_size, &tls_begin_,
-   &tls_size);
-  stack_top_ = stack_bottom_ + stack_size;
-  tls_end_ = tls_begin_ + tls_size;
-
   if (stack_bottom_) {
 int local;
 CHECK(AddrIsInStack((uptr)&local));
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits