[PATCH] D105354: [clang][AST] Add support for DecompositionDecl to ASTImporter.

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

balazske added inline comments.



Comment at: clang/lib/AST/ASTImporter.cpp:2305-2309
+  BindingDecl *ToD;
+  if (GetImportedOrCreateDecl(ToD, D, Importer.getToContext(), DC, Loc,
+  Name.getAsIdentifierInfo()))
+return ToD;
+

shafik wrote:
> martong wrote:
> > So, we moved the import of the binding before importing the decomposition 
> > decl to avoid an infinite recursion. But why can't we have an infinit 
> > recursion this way?
> > 
> > Perhaps, it would be useful to have a test case that triggered the infinity 
> > in case of the original order of the import calls.
> Yes, I agree, I would also like to understand better why this avoids the 
> infinite recursion problem, a test case would be helpful as well as an 
> explanation of the steps that leads us to the problem.
With the import at original place, in `VisitVarDecl` the bindings (which are 
`BindingDecl`) are imported before create of the `DecompositionDecl` instance, 
and in `VisitBindingDecl` the decomposition (a `DecompositionDecl` that is 
`VarDecl`) is imported before create of the `BindingDecl` instance. This causes 
the recursion with the most simple import of a `DecompositionDecl`. This is 
triggered by the existing simple test (actually I discovered the problem at the 
test failure). If the decomposition is imported after create of the 
`BindingDecl` the same `BindingDecl` is not imported again because it already 
exists.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105354

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

[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

[Deep Majumder via Phabricator via cfe-commits]

RedDocMD updated this revision to Diff 357422.
RedDocMD added a comment.

Tests implemented


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105421

Files:
  clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/smart-ptr.cpp

Index: clang/test/Analysis/smart-ptr.cpp
===
--- clang/test/Analysis/smart-ptr.cpp
+++ clang/test/Analysis/smart-ptr.cpp
@@ -3,6 +3,11 @@
 // RUN:   -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
 // RUN:   -std=c++11 -verify %s
 
+// RUN: %clang_analyze_cc1 -analyzer-checker=core,debug.ExprInspection\
+// RUN:   -analyzer-checker cplusplus.Move,alpha.cplusplus.SmartPtr\
+// RUN:   -analyzer-config cplusplus.SmartPtrModeling:ModelSmartPtrDereference=true\
+// RUN:   -std=c++20 -verify %s
+
 #include "Inputs/system-header-simulator-cxx.h"
 
 void clang_analyzer_warnIfReached();
@@ -457,3 +462,17 @@
 P->foo(); // expected-warning {{Dereference of null smart pointer 'P' [alpha.cplusplus.SmartPtr]}}
   }
 }
+
+#if __cplusplus >= 202002L
+
+void testOstreamOverload(std::unique_ptr P) {
+  auto &Cout = std::cout;
+  auto &PtrCout = std::cout << P;
+  auto &StringCout = std::cout << "hello";
+  // We are testing the fact that in our modelling of
+  // operator<<(basic_ostream &, const unique_ptr &)
+  // we set the return SVal to the SVal of the ostream arg.
+  clang_analyzer_eval(&Cout == &PtrCout);// expected-warning {{TRUE}}
+  clang_analyzer_eval(&Cout == &StringCout); // expected-warning {{UNKNOWN}}
+}
+#endif
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -981,6 +981,22 @@
 } // namespace std
 #endif
 
+namespace std {
+template 
+class basic_ostream;
+
+using ostream = basic_ostream;
+
+extern std::ostream cout;
+
+ostream &operator<<(ostream &, const string &);
+
+#if __cplusplus >= 202002L
+template 
+ostream &operator<<(ostream &, const std::unique_ptr &);
+#endif
+} // namespace std
+
 #ifdef TEST_INLINABLE_ALLOCATORS
 namespace std {
   void *malloc(size_t);
Index: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
+++ clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp
@@ -68,6 +68,7 @@
   bool updateMovedSmartPointers(CheckerContext &C, const MemRegion *ThisRegion,
 const MemRegion *OtherSmartPtrRegion) const;
   void handleBoolConversion(const CallEvent &Call, CheckerContext &C) const;
+  bool handleOstreamOperator(const CallEvent &Call, CheckerContext &C) const;
 
   using SmartPtrMethodHandlerFn =
   void (SmartPtrModeling::*)(const CallEvent &Call, CheckerContext &) const;
@@ -81,6 +82,31 @@
 
 REGISTER_MAP_WITH_PROGRAMSTATE(TrackedRegionMap, const MemRegion *, SVal)
 
+// Checks if RD has name in Names and is in std namespace
+static bool hasStdClassWithName(const CXXRecordDecl *RD,
+const ArrayRef &Names) {
+  if (!RD || !RD->getDeclContext()->isStdNamespace())
+return false;
+  if (RD->getDeclName().isIdentifier()) {
+StringRef Name = RD->getName();
+return llvm::any_of(Names, [&Name](StringRef GivenName) -> bool {
+  return Name == GivenName;
+});
+  }
+  return false;
+}
+
+const SmallVector StdPtrNames = {"shared_ptr", "unique_ptr",
+   "weak_ptr"};
+
+static bool isStdSmartPtr(const CXXRecordDecl *RD) {
+  return hasStdClassWithName(RD, StdPtrNames);
+}
+
+static bool isStdSmartPtr(const Expr *E) {
+  return isStdSmartPtr(E->getType()->getAsCXXRecordDecl());
+}
+
 // Define the inter-checker API.
 namespace clang {
 namespace ento {
@@ -89,16 +115,7 @@
   const auto *MethodDecl = dyn_cast_or_null(Call.getDecl());
   if (!MethodDecl || !MethodDecl->getParent())
 return false;
-
-  const auto *RecordDecl = MethodDecl->getParent();
-  if (!RecordDecl || !RecordDecl->getDeclContext()->isStdNamespace())
-return false;
-
-  if (RecordDecl->getDeclName().isIdentifier()) {
-StringRef Name = RecordDecl->getName();
-return Name == "shared_ptr" || Name == "unique_ptr" || Name == "weak_ptr";
-  }
-  return false;
+  return isStdSmartPtr(MethodDecl->getParent());
 }
 
 bool isNullSmartPtr(const ProgramStateRef State, const MemRegion *ThisRegion) {
@@ -175,9 +192,37 @@
   return CD && CD->getConversionType()->isBooleanType();
 }
 
+const SmallVector BasicOstreamName = {"basic_ostream"};
+
+bool isStdBasicOstream(const Expr *E) {
+  const auto *RD = E->getType()->getAsCXXRecordDecl();
+  return hasStdClassWithName(RD, BasicOstreamName);

[PATCH] D105679: [clangd] Add CMake option to (not) link in clang-tidy checks

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added reviewers: kadircet, nridge.
Herald added subscribers: usaxena95, arphaman, mgorny.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

This reduces the size of the dependency graph and makes incremental
development a little more pleasant (less rebuilding).

This introduces a bit of complexity/fragility as some tests verify
clang-tidy behavior. I attempted to isolate these and build/run as much
of the tests as possible in both configs to prevent rot.

Expectation is that (some) developers will use this locally, but
buildbots etc will keep testing clang-tidy.

Fixes https://github.com/clangd/clangd/issues/233


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105679

Files:
  clang-tools-extra/clangd/CMakeLists.txt
  clang-tools-extra/clangd/Features.cpp
  clang-tools-extra/clangd/Features.inc.in
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/test/diagnostics-tidy.test
  clang-tools-extra/clangd/test/diagnostics.test
  clang-tools-extra/clangd/test/lit.cfg.py
  clang-tools-extra/clangd/test/lit.site.cfg.py.in
  clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -10,6 +10,7 @@
 #include "Config.h"
 #include "Diagnostics.h"
 #include "FeatureModule.h"
+#include "Features.h"
 #include "ParsedAST.h"
 #include "Protocol.h"
 #include "SourceCode.h"
@@ -114,6 +115,19 @@
   return Res;
 }
 
+// Normally returns the provided diagnostics matcher.
+// If clang-tidy checks are not linked in, returns a matcher for no diagnostics!
+// This is intended for tests where the diagnostics come from clang-tidy checks.
+// We don't #ifdef each individual test as it's intrusive and we want to ensure
+// that as much of the test is still compiled an run as possible.
+::testing::Matcher>
+ifTidyChecks(::testing::Matcher> M,
+ ::testing::Matcher> Else = IsEmpty()) {
+  if (!CLANGD_TIDY_CHECKS)
+return Else;
+  return M;
+}
+
 TEST(DiagnosticsTest, DiagnosticRanges) {
   // Check we report correct ranges, including various edge-cases.
   Annotations Test(R"cpp(
@@ -121,7 +135,6 @@
 #define ID(X) X
 namespace test{};
 void $decl[[foo]]();
-class T{$explicit[[]]$constructor[[T]](int a);};
 int main() {
   $typo[[go\
 o]]();
@@ -163,13 +176,7 @@
   AllOf(Diag(Test.range("macro"),
  "use of undeclared identifier 'fod'; did you mean 'foo'?"),
 WithFix(Fix(Test.range("macroarg"), "foo",
-"change 'fod' to 'foo'"))),
-  // We make sure here that the entire token is highlighted
-  AllOf(Diag(Test.range("constructor"),
- "single-argument constructors must be marked explicit to "
- "avoid unintentional implicit conversions"),
-WithFix(Fix(Test.range("explicit"), "explicit ",
-"insert 'explicit '");
+"change 'fod' to 'foo'");
 }
 
 TEST(DiagnosticsTest, FlagsMatter) {
@@ -212,10 +219,10 @@
   // Verify that we filter out the duplicated diagnostic message.
   EXPECT_THAT(
   *TU.build().getDiagnostics(),
-  UnorderedElementsAre(::testing::AllOf(
+  ifTidyChecks(UnorderedElementsAre(::testing::AllOf(
   Diag(Test.range(),
"floating point literal has suffix 'f', which is not uppercase"),
-  DiagSource(Diag::ClangTidy;
+  DiagSource(Diag::ClangTidy);
 
   Test = Annotations(R"cpp(
 template
@@ -232,10 +239,10 @@
   // duplicated messages, verify that we deduplicate them.
   EXPECT_THAT(
   *TU.build().getDiagnostics(),
-  UnorderedElementsAre(::testing::AllOf(
+  ifTidyChecks(UnorderedElementsAre(::testing::AllOf(
   Diag(Test.range(),
"floating point literal has suffix 'f', which is not uppercase"),
-  DiagSource(Diag::ClangTidy;
+  DiagSource(Diag::ClangTidy);
 }
 
 TEST(DiagnosticsTest, ClangTidy) {
@@ -249,6 +256,8 @@
   return $doubled[[sizeof]](sizeof(int));
 }
 
+class T{$explicit[[]]$constructor[[T]](int a);};
+
 // misc-no-recursion uses a custom traversal from the TUDecl
 void foo();
 void $bar[[bar]]() {
@@ -262,43 +271,57 @@
   TU.HeaderFilename = "assert.h"; // Suppress "not found" error.
   TU.ClangTidyProvider = addTidyChecks("bugprone-sizeof-expression,"
"bugprone-macro-repeated-side-effects,"
+   "google-explicit-constructor,"
"modern

[PATCH] D105681: [clangd] Add platform triple (host & target) to version info

[Sam McCall via Phabricator via cfe-commits]

sammccall created this revision.
sammccall added a reviewer: kadircet.
Herald added subscribers: usaxena95, mstorsjo, arphaman.
sammccall requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Useful in logs to understand issues around some platforms we don't have much
experience with (e.g. m1, mingw)


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105681

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/Features.cpp
  clang-tools-extra/clangd/Features.h
  clang-tools-extra/clangd/index/remote/Client.cpp
  clang-tools-extra/clangd/tool/ClangdMain.cpp


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -679,7 +679,8 @@
   llvm::sys::SetInterruptFunction(&requestShutdown);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
 OS << versionString() << "\n"
-   << "Features: " << featureString() << "\n";
+   << "Features: " << featureString() << "\n"
+   << "Platform: " << platformString() << "\n";
   });
   const char *FlagsEnvVar = "CLANGD_FLAGS";
   const char *Overview =
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/Client.cpp
+++ clang-tools-extra/clangd/index/remote/Client.cpp
@@ -74,6 +74,7 @@
 grpc::ClientContext Context;
 Context.AddMetadata("version", versionString());
 Context.AddMetadata("features", featureString());
+Context.AddMetadata("platform", platformString());
 std::chrono::system_clock::time_point StartTime =
 std::chrono::system_clock::now();
 auto Deadline = StartTime + DeadlineWaitingTime;
Index: clang-tools-extra/clangd/Features.h
===
--- clang-tools-extra/clangd/Features.h
+++ clang-tools-extra/clangd/Features.h
@@ -19,6 +19,10 @@
 // Returns a version string for clangd, e.g. "clangd 10.0.0"
 std::string versionString();
 
+// Returns the platform triple for clangd, e.g. "x86_64-pc-linux-gnu"
+// May include both the host and target triple if they differ.
+std::string platformString();
+
 // Returns a string describing the compile-time configuration.
 // e.g. mac+debug+asan+grpc
 std::string featureString();
Index: clang-tools-extra/clangd/Features.cpp
===
--- clang-tools-extra/clangd/Features.cpp
+++ clang-tools-extra/clangd/Features.cpp
@@ -9,12 +9,26 @@
 #include "Features.h"
 #include "clang/Basic/Version.h"
 #include "llvm/Support/Compiler.h"
+#include "llvm/Support/Host.h"
 
 namespace clang {
 namespace clangd {
 
 std::string versionString() { return clang::getClangToolFullVersion("clangd"); 
}
 
+std::string platformString() {
+  static std::string PlatformString = []() {
+std::string Host = llvm::sys::getProcessTriple();
+std::string Target = llvm::sys::getDefaultTargetTriple();
+if (Host != Target) {
+  Host += "; target=";
+  Host += Target;
+}
+return Host;
+  }();
+  return PlatformString;
+}
+
 std::string featureString() {
   return
 #if defined(_WIN32)
Index: clang-tools-extra/clangd/ClangdLSPServer.cpp
===
--- clang-tools-extra/clangd/ClangdLSPServer.cpp
+++ clang-tools-extra/clangd/ClangdLSPServer.cpp
@@ -619,9 +619,10 @@
 
   llvm::json::Object Result{
   {{"serverInfo",
-llvm::json::Object{{"name", "clangd"},
-   {"version", llvm::formatv("{0} {1}", 
versionString(),
- featureString())}}},
+llvm::json::Object{
+{"name", "clangd"},
+{"version", llvm::formatv("{0} {1} {2}", versionString(),
+  featureString(), platformString())}}},
{"capabilities", std::move(ServerCaps)}}};
   if (Opts.Encoding)
 Result["offsetEncoding"] = *Opts.Encoding;


Index: clang-tools-extra/clangd/tool/ClangdMain.cpp
===
--- clang-tools-extra/clangd/tool/ClangdMain.cpp
+++ clang-tools-extra/clangd/tool/ClangdMain.cpp
@@ -679,7 +679,8 @@
   llvm::sys::SetInterruptFunction(&requestShutdown);
   llvm::cl::SetVersionPrinter([](llvm::raw_ostream &OS) {
 OS << versionString() << "\n"
-   << "Features: " << featureString() << "\n";
+   << "Features: " << featureString() << "\n"
+   << "Platform: " << platformString() << "\n";
   });
   const char *FlagsEnvVar = "CLANGD_FLAGS";
   const char *Overview =
Index: clang-tools-extra/clangd/index/remote/Client.cpp
===
--- clang-tools-extra/clangd/index/remote/

[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Great, thanks for addressing my comments! I still have a couple of minor 
suggestions though.




Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:99-100
+
+const SmallVector StdPtrNames = {"shared_ptr", "unique_ptr",
+   "weak_ptr"};
+

If it's a compile-time constant, let's make sure it is.
nit: I prefer global scope constants to be CAPITALIZED, so it's easier to spot 
them in the code.



Comment at: clang/lib/StaticAnalyzer/Checkers/SmartPtrModeling.cpp:195
 
+const SmallVector BasicOstreamName = {"basic_ostream"};
+

Same here + don't call it "Name" (singular).  It is a) an array and b) in the 
future, we might add more things to it, so we shouldn't need to rename it 
everywhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105421

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

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

@vsavchenko any update on this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D105679: [clangd] Add CMake option to (not) link in clang-tidy checks

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added inline comments.



Comment at: clang-tools-extra/clangd/unittests/ConfigCompileTests.cpp:280
 
+#if CLANGD_TIDY_CHECKS
 TEST_F(ConfigCompileTests, Tidy) {

why do we need to disable this test? it doesn't really build an ast or assert 
on the diagnostics from clang-tidy in any way, seems to be purely about configs.



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:151
   auto TU = TestTU::withCode(Test.code());
   TU.ClangTidyProvider = addTidyChecks("google-explicit-constructor");
   EXPECT_THAT(

you probably want to drop this too ?



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:167
-"change 'fod' to 'foo'"))),
-  // We make sure here that the entire token is highlighted
-  AllOf(Diag(Test.range("constructor"),

ah, this was actually checking for a particular change in the way we transform 
clang(-tidy) provided diag ranges to editor-friendly ones.

any diagnostic without any range info and containing a fix which is only 
insertion should be fine (one that i could find is `for_range_dereference`).



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:259
 
+class T{$explicit[[]]$constructor[[T]](int a);};
+

ah, should've kept reading before leaving the comment above. feel free to 
ignore that (or just drop this test and rely only on clang diagnostics for 
checking that behaviour?)



Comment at: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp:301
+  "macro 'SQUARE' defined here"))),
   Diag(Test.range("macroarg"),
+   "multiple unsequenced modifications to 'y'"),

i think we should suppress this with `-Wno-unsequenced` (and get rid of the 
`Else` part). Moreover this is the only test requiring an Else bit (if I didn't 
miss any). WDYT about just having a new file `TidyIntegrationTests` or 
`TidyDiagnosticTests` and moving all of these there, or having two different 
sections in this file to only enable these tests when tidy is on. It would 
imply tests that want to check a mix of tidy and clang diagnostics needs to go 
into the tidy section and cannot be tested on non-tidy builds, but that sounds 
like the price we need to pay anyway, whereas introducing the `ifTidyChecks` 
complicates things a little more (IMO).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105679

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

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Valeriy Savchenko via Phabricator via cfe-commits]

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

Great!  Thanks for addressing all of the comments!




Comment at: clang/lib/StaticAnalyzer/Core/LoopUnrolling.cpp:185-186
+  return FD->getType()->isReferenceType();
+} else {
+  assert(false && "Unknown captured variable");
+}

AbbasSabra wrote:
> vsavchenko wrote:
> > AbbasSabra wrote:
> > > vsavchenko wrote:
> > > > But actually, it's a bit different with this one. I don't know exact 
> > > > details and rules how clang sema fills in the class for lambda.
> > > > According to [[ https://en.cppreference.com/w/cpp/language/lambda | 
> > > > cppreference ]]:
> > > > > For the entities that are captured by reference (with the default 
> > > > > capture [&] or when using the character &, e.g. [&a, &b, &c]), it is 
> > > > > unspecified if additional data members are declared in the closure 
> > > > > type
> > > > 
> > > > It can be pretty much specified in clang, that's true, but it looks 
> > > > like in `DeadStoreChecker` we have a very similar situation and we do 
> > > > not assume that captured variable always have a corresponding field.
> > > Yes, I based this on the fact that DeadStoreChecker considers it 
> > > possible, but I have never faced a case where it does not have a 
> > > corresponding field.
> > It still would be good to have some proof that it is indeed like this or 
> > simply fallback into returning true (which you already do when in doubt).
> As I said, I believe it cannot happen but I assumed based on the other 
> checker that there is something I don't know.
> I think based on !!getCaptureFields!! the implementation we are iterating 
> over all captures. For that algorithm to work number of captures should be 
> equal to number of fields
> ```
> void CXXRecordDecl::getCaptureFields(
>llvm::DenseMap &Captures,
>FieldDecl *&ThisCapture) const {
>   Captures.clear();
>   ThisCapture = nullptr;
> 
>   LambdaDefinitionData &Lambda = getLambdaData();
>   RecordDecl::field_iterator Field = field_begin();
>   for (const LambdaCapture *C = Lambda.Captures, *CEnd = C + 
> Lambda.NumCaptures;
>C != CEnd; ++C, ++Field) {
> if (C->capturesThis())
>   ThisCapture = *Field;
> else if (C->capturesVariable())
>   Captures[C->getCapturedVar()] = *Field;
>   }
>   assert(Field == field_end());
> }
> ```
> 
> I dropped the defensive code
OK, sounds reasonable!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[PATCH] D105533: [clang] Fix an infinite loop during typo-correction

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

In D105533#2865803 , @dgoldman wrote:

> Thanks! Just to confirm, the non-simplified example is also fixed, right?
>
>   struct a {
> int xxx;
>   };
>   
>   int g_107;
>   int g_108;
>   int g_109;
>   
>   struct a g_999;
>   
>   void b() { (g_910.xxx = g_910.xxx1); }

yeah, it works, added to the test.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105533

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

[PATCH] D105533: [clang] Fix an infinite loop during typo-correction

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 357460.
hokein added a comment.

add the non-simplified sample.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105533

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/Sema/typo-correction-ambiguity.c
  clang/test/Sema/typo-correction-no-hang.c


Index: clang/test/Sema/typo-correction-no-hang.c
===
--- /dev/null
+++ clang/test/Sema/typo-correction-no-hang.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// PR50797
+struct a {
+  int xxx; // expected-note {{'xxx' declared here}}
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999; // expected-note 4{{'g_999' declared here}}
+
+void b() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of undeclared 
identifier 'g_910'; did you mean 'g_999'}}
+
+void c() { (g_910.xxx = g_910.xxx1); } //expected-error 2{{use of undeclared 
identifier 'g_910'; did you mean 'g_999'}} \
+ expected-error {{no member named 
'xxx1' in 'struct a'; did you mean 'xxx'}}
Index: clang/test/Sema/typo-correction-ambiguity.c
===
--- clang/test/Sema/typo-correction-ambiguity.c
+++ clang/test/Sema/typo-correction-ambiguity.c
@@ -12,3 +12,16 @@
v_2_0(v_195,  // expected-error {{use of undeclared identifier 'v_195'}}
  v_231);  // expected-error {{use of undeclared identifier 
'v_231'}}
 }
+
+// Test: no typo-correction diagnostics are emitted for ambiguous typos.
+struct a {
+  int xxx;
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999;
+struct a g_998;
+void PR50797() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of 
undeclared identifier 'g_910'}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8343,6 +8343,7 @@
 
 AmbiguousTypoExprs.remove(TE);
 SemaRef.getTypoExprState(TE).Consumer->restoreSavedPosition();
+TransformCache[TE] = SavedTransformCache[TE];
   }
   TransformCache = std::move(SavedTransformCache);
 }


Index: clang/test/Sema/typo-correction-no-hang.c
===
--- /dev/null
+++ clang/test/Sema/typo-correction-no-hang.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// PR50797
+struct a {
+  int xxx; // expected-note {{'xxx' declared here}}
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999; // expected-note 4{{'g_999' declared here}}
+
+void b() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of undeclared identifier 'g_910'; did you mean 'g_999'}}
+
+void c() { (g_910.xxx = g_910.xxx1); } //expected-error 2{{use of undeclared identifier 'g_910'; did you mean 'g_999'}} \
+ expected-error {{no member named 'xxx1' in 'struct a'; did you mean 'xxx'}}
Index: clang/test/Sema/typo-correction-ambiguity.c
===
--- clang/test/Sema/typo-correction-ambiguity.c
+++ clang/test/Sema/typo-correction-ambiguity.c
@@ -12,3 +12,16 @@
v_2_0(v_195,  // expected-error {{use of undeclared identifier 'v_195'}}
  v_231);  // expected-error {{use of undeclared identifier 'v_231'}}
 }
+
+// Test: no typo-correction diagnostics are emitted for ambiguous typos.
+struct a {
+  int xxx;
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999;
+struct a g_998;
+void PR50797() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of undeclared identifier 'g_910'}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8343,6 +8343,7 @@
 
 AmbiguousTypoExprs.remove(TE);
 SemaRef.getTypoExprState(TE).Consumer->restoreSavedPosition();
+TransformCache[TE] = SavedTransformCache[TE];
   }
   TransformCache = std::move(SavedTransformCache);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] f4877c7 - [clang] Improve `-Wnull-dereference` diag to be more in-line with reality

[Roman Lebedev via cfe-commits]

Author: Roman Lebedev
Date: 2021-07-09T12:51:12+03:00
New Revision: f4877c78c0fc98be47b926439bbfe33d5e1d1b6d

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

LOG: [clang] Improve `-Wnull-dereference` diag to be more in-line with reality

* Drop any mention of `volatile`.
  Please refer to https://reviews.llvm.org/D105338
* Drop address space check - it really doesn't affect the behavior,
  the store will still be dropped: https://godbolt.org/z/dP8fevxG4

Added: 


Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExpr.cpp
clang/test/Analysis/NewDelete-checker-test.cpp
clang/test/Analysis/conditional-path-notes.c
clang/test/Analysis/cxx-for-range.cpp
clang/test/Analysis/diagnostics/no-prune-paths.c
clang/test/Analysis/inlining/path-notes.cpp
clang/test/Analysis/objc-arc.m
clang/test/Analysis/objc-for.m
clang/test/Analysis/taint-generic.c
clang/test/Analysis/valist-uninitialized.c
clang/test/Parser/expressions.c
clang/test/Sema/exprs.c
clang/test/Sema/offsetof.c
clang/test/SemaCXX/member-pointer.cpp

Removed: 




diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index a9d738895033..d33d48846b18 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6744,13 +6744,13 @@ def ext_typecheck_indirection_through_void_pointer : 
ExtWarn<
   "ISO C++ does not allow indirection on operand of type %0">,
   InGroup>;
 def warn_indirection_through_null : Warning<
-  "indirection of non-volatile null pointer will be deleted, not trap">,
+  "indirection of null pointer will be deleted, not trap">,
   InGroup;
 def warn_binding_null_to_reference : Warning<
   "binding dereferenced null pointer to reference has undefined behavior">,
   InGroup;
 def note_indirection_through_null : Note<
-  "consider using __builtin_trap() or qualifying pointer with 'volatile'">;
+  "consider using __builtin_trap()">;
 def warn_pointer_indirection_from_incompatible_type : Warning<
   "dereference of type %1 that was reinterpret_cast from type %0 has undefined 
"
   "behavior">,

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index a3a26d21422f..d0efe4c02a1e 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -533,21 +533,16 @@ ExprResult Sema::DefaultFunctionArrayConversion(Expr *E, 
bool Diagnose) {
 }
 
 static void CheckForNullPointerDereference(Sema &S, Expr *E) {
-  // Check to see if we are dereferencing a null pointer.  If so,
-  // and if not volatile-qualified, this is undefined behavior that the
-  // optimizer will delete, so warn about it.  People sometimes try to use this
-  // to get a deterministic trap and are surprised by clang's behavior.  This
-  // only handles the pattern "*null", which is a very syntactic check.
+  // Check to see if we are dereferencing a null pointer.
+  // If so, this is undefined behavior that the optimizer will delete,
+  // so warn about it. People sometimes try to use this to get a deterministic
+  // trap and are surprised by clang's behavior. This only handles the pattern
+  // "*null", which is a very syntactic check.
   const auto *UO = dyn_cast(E->IgnoreParenCasts());
   if (UO && UO->getOpcode() == UO_Deref &&
   UO->getSubExpr()->getType()->isPointerType()) {
-const LangAS AS =
-UO->getSubExpr()->getType()->getPointeeType().getAddressSpace();
-if ((!isTargetAddressSpace(AS) ||
- (isTargetAddressSpace(AS) && toTargetAddressSpace(AS) == 0)) &&
-UO->getSubExpr()->IgnoreParenCasts()->isNullPointerConstant(
-S.Context, Expr::NPC_ValueDependentIsNotNull) &&
-!UO->getType().isVolatileQualified()) {
+if (UO->getSubExpr()->IgnoreParenCasts()->isNullPointerConstant(
+S.Context, Expr::NPC_ValueDependentIsNotNull)) {
   S.DiagRuntimeBehavior(UO->getOperatorLoc(), UO,
 S.PDiag(diag::warn_indirection_through_null)
 << UO->getSubExpr()->getSourceRange());

diff  --git a/clang/test/Analysis/NewDelete-checker-test.cpp 
b/clang/test/Analysis/NewDelete-checker-test.cpp
index 86df9d01dfb0..44a176a6eef8 100644
--- a/clang/test/Analysis/NewDelete-checker-test.cpp
+++ b/clang/test/Analysis/NewDelete-checker-test.cpp
@@ -83,7 +83,7 @@ void testGlobalPointerPlacementNew() {
 //- Other cases
 void testNewMemoryIsInHeap() {
   int *p = new int;
-  if (global != p) // condition is always true as 'p' wraps a heap region that 
+  if (global != p) // condition is always true as 'p' wraps a heap region that
// is 
diff erent from a region wrapped by 'global'
 global = 

[PATCH] D105236: [PowerPC] Implement Load and Reserve and Store Conditional Builtins

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added inline comments.



Comment at: llvm/include/llvm/IR/IntrinsicsPowerPC.td:1533
+  def int_ppc_lwarx : GCCBuiltin<"__builtin_ppc_lwarx">,
+  Intrinsic<[llvm_i32_ty], [llvm_ptr_ty], [IntrNoMem]>;
+  def int_ppc_ldarx : GCCBuiltin<"__builtin_ppc_ldarx">,

I don't know why I missed this in the review, but this is very wrong! This is a 
load intrinsic that is marked as one that doesn't touch memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105236

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov updated this revision to Diff 357463.
ASDenysPetrov added a comment.

Added more descriptive comments. Fixed 
`RangeConstraintManager::updateExistingConstraints` function.


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

https://reviews.llvm.org/D103096

Files:
  clang/include/clang/StaticAnalyzer/Checkers/SValExplainer.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymExpr.h
  clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h
  clang/lib/StaticAnalyzer/Checkers/ExprInspectionChecker.cpp
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/symbol-integral-cast.cpp

Index: clang/test/Analysis/symbol-integral-cast.cpp
===
--- /dev/null
+++ clang/test/Analysis/symbol-integral-cast.cpp
@@ -0,0 +1,353 @@
+// RUN: %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection -analyzer-config eagerly-assume=false -analyzer-config support-symbolic-integer-casts=true -verify %s
+
+template 
+void clang_analyzer_eval(T);
+void clang_analyzer_warnIfReached();
+
+typedef short int16_t;
+typedef int int32_t;
+typedef unsigned short uint16_t;
+typedef unsigned int uint32_t;
+
+void test1(int x) {
+  // Even if two lower bytes of `x` equal to zero, it doesn't mean that
+  // the entire `x` is zero. We are not able to know the exact value of x.
+  // It can be one of  65536 possible values like [0, 65536, 131072, ...]
+  // and so on. To avoid huge range sets we still assume `x` in the range
+  // [INT_MIN, INT_MAX].
+  if (!(short)x) {
+if (!x)
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+else
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  }
+}
+
+void test2(int x) {
+  // If two lower bytes of `x` equal to zero, and we know x to be 65537,
+  // which is not truncated to short as zero. Thus the branch is infisible.
+  short s = x;
+  if (!s) {
+if (x == 65537)
+  clang_analyzer_warnIfReached(); // no-warning
+else
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  }
+}
+
+void test3(int x, short s) {
+  s = x;
+  if ((short)x > -10 && s < 10) {
+if (x > 0 && x < 10) {
+  // If the range of the whole variable was constrained then reason again
+  // about truncated bytes to make the ranges more precise.
+  clang_analyzer_eval((short)x <= 0); // expected-warning {{FALSE}}
+}
+  }
+}
+
+void test4(unsigned x) {
+  if ((char)x > 8) {
+// Constraint the range of the lowest byte of `x` to [9, CHAR_MAX].
+// The original range of `x` still remains [0, UINT_MAX].
+clang_analyzer_eval((char)x < 42); // expected-warning {{UNKNOWN}}
+if (x < 42) {
+  // Constraint the original range to [0, 42] and update (re-constraint)
+  // the range of the lowest byte of 'x' to [9, 42].
+  clang_analyzer_eval((char)x < 42); // expected-warning {{TRUE}}
+}
+  }
+}
+
+void test5(unsigned x) {
+  if ((char)x > -10 && (char)x < 10) {
+if ((short)x == 8) {
+  // If the range of higher bytes(short) was constrained then reason again
+  // about smaller truncated ranges(char) to make it more precise.
+  clang_analyzer_eval((char)x == 8);  // expected-warning {{TRUE}}
+  clang_analyzer_eval((short)x == 8); // expected-warning {{TRUE}}
+  // We still assume full version of `x` in the range [INT_MIN, INT_MAX].
+  clang_analyzer_eval(x == 8); // expected-warning {{UNKNOWN}}
+}
+  }
+}
+
+void test6(int x) {
+  // Even if two lower bytes of `x` less than zero, it doesn't mean that `x`
+  // can't be greater than zero. Thence we don't change the native range of
+  // `x` and this branch is feasible.
+  if (x > 0)
+if ((short)x < 0)
+  clang_analyzer_eval(x > 0); // expected-warning {{TRUE}}
+}
+
+void test7(int x) {
+  // The range of two lower bytes of `x` [1, SHORT_MAX] is enough to cover
+  // all possible values of char [CHAR_MIN, CHAR_MAX]. So the lowest byte
+  // can be lower than zero.
+  if ((short)x > 0) {
+if ((char)x < 0)
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+else
+  clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+  }
+}
+
+void test8(int x) {
+  // Promotion from `signed int` to `signed long long` also reasoning about the
+  // original range, because we know the fact that even after promotion it
+  // remains in the range [INT_MIN, INT_MAX].
+  if ((long long)x < 0)
+clang_analyzer_eval(x < 0); // expected-warning {{TRUE}}
+}
+
+void test9(signed int x) {
+  // Any cast `signed` to `unsigned` produces an unsigned range, which is
+  // [0, UNSIGNED_MAX] and can not be lower than zero.
+  if ((unsigned long long)x < 0)
+clang_analyzer_warnIfReached(); // no-warning
+  else
+clang_analyzer_warnIfReached(); // expected-warning {{REACHABLE}}
+
+  if ((unsigned int)x < 0)
+cla

[clang] 47653db - [clang] Fix an infinite loop during typo-correction

[Haojian Wu via cfe-commits]

Author: Haojian Wu
Date: 2021-07-09T12:03:57+02:00
New Revision: 47653db6d2a3964c14cca5ffa73e79aeee292e8b

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

LOG: [clang] Fix an infinite loop during typo-correction

See https://bugs.llvm.org/show_bug.cgi?id=50797#c6

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

Added: 
clang/test/Sema/typo-correction-no-hang.c

Modified: 
clang/lib/Sema/SemaExprCXX.cpp
clang/test/Sema/typo-correction-ambiguity.c

Removed: 




diff  --git a/clang/lib/Sema/SemaExprCXX.cpp b/clang/lib/Sema/SemaExprCXX.cpp
index 2e9e9a4a88df..e6bf5c6a7ad8 100644
--- a/clang/lib/Sema/SemaExprCXX.cpp
+++ b/clang/lib/Sema/SemaExprCXX.cpp
@@ -8343,6 +8343,7 @@ class TransformTypos : public 
TreeTransform {
 
 AmbiguousTypoExprs.remove(TE);
 SemaRef.getTypoExprState(TE).Consumer->restoreSavedPosition();
+TransformCache[TE] = SavedTransformCache[TE];
   }
   TransformCache = std::move(SavedTransformCache);
 }

diff  --git a/clang/test/Sema/typo-correction-ambiguity.c 
b/clang/test/Sema/typo-correction-ambiguity.c
index bebbf25ce291..c25fe652bd9f 100644
--- a/clang/test/Sema/typo-correction-ambiguity.c
+++ b/clang/test/Sema/typo-correction-ambiguity.c
@@ -12,3 +12,16 @@ int v_3_0() {
v_2_0(v_195,  // expected-error {{use of undeclared identifier 'v_195'}}
  v_231);  // expected-error {{use of undeclared identifier 
'v_231'}}
 }
+
+// Test: no typo-correction diagnostics are emitted for ambiguous typos.
+struct a {
+  int xxx;
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999;
+struct a g_998;
+void PR50797() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of 
undeclared identifier 'g_910'}}

diff  --git a/clang/test/Sema/typo-correction-no-hang.c 
b/clang/test/Sema/typo-correction-no-hang.c
new file mode 100644
index ..f3d3bf00abe8
--- /dev/null
+++ b/clang/test/Sema/typo-correction-no-hang.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// PR50797
+struct a {
+  int xxx; // expected-note {{'xxx' declared here}}
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999; // expected-note 4{{'g_999' declared here}}
+
+void b() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of undeclared 
identifier 'g_910'; did you mean 'g_999'}}
+
+void c() { (g_910.xxx = g_910.xxx1); } //expected-error 2{{use of undeclared 
identifier 'g_910'; did you mean 'g_999'}} \
+ expected-error {{no member named 
'xxx1' in 'struct a'; did you mean 'xxx'}}



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

[PATCH] D105533: [clang] Fix an infinite loop during typo-correction

[Haojian Wu 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 rG47653db6d2a3: [clang] Fix an infinite loop during 
typo-correction (authored by hokein).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105533

Files:
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/Sema/typo-correction-ambiguity.c
  clang/test/Sema/typo-correction-no-hang.c


Index: clang/test/Sema/typo-correction-no-hang.c
===
--- /dev/null
+++ clang/test/Sema/typo-correction-no-hang.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// PR50797
+struct a {
+  int xxx; // expected-note {{'xxx' declared here}}
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999; // expected-note 4{{'g_999' declared here}}
+
+void b() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of undeclared 
identifier 'g_910'; did you mean 'g_999'}}
+
+void c() { (g_910.xxx = g_910.xxx1); } //expected-error 2{{use of undeclared 
identifier 'g_910'; did you mean 'g_999'}} \
+ expected-error {{no member named 
'xxx1' in 'struct a'; did you mean 'xxx'}}
Index: clang/test/Sema/typo-correction-ambiguity.c
===
--- clang/test/Sema/typo-correction-ambiguity.c
+++ clang/test/Sema/typo-correction-ambiguity.c
@@ -12,3 +12,16 @@
v_2_0(v_195,  // expected-error {{use of undeclared identifier 'v_195'}}
  v_231);  // expected-error {{use of undeclared identifier 
'v_231'}}
 }
+
+// Test: no typo-correction diagnostics are emitted for ambiguous typos.
+struct a {
+  int xxx;
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999;
+struct a g_998;
+void PR50797() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of 
undeclared identifier 'g_910'}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8343,6 +8343,7 @@
 
 AmbiguousTypoExprs.remove(TE);
 SemaRef.getTypoExprState(TE).Consumer->restoreSavedPosition();
+TransformCache[TE] = SavedTransformCache[TE];
   }
   TransformCache = std::move(SavedTransformCache);
 }


Index: clang/test/Sema/typo-correction-no-hang.c
===
--- /dev/null
+++ clang/test/Sema/typo-correction-no-hang.c
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 -fsyntax-only -verify %s
+
+// PR50797
+struct a {
+  int xxx; // expected-note {{'xxx' declared here}}
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999; // expected-note 4{{'g_999' declared here}}
+
+void b() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of undeclared identifier 'g_910'; did you mean 'g_999'}}
+
+void c() { (g_910.xxx = g_910.xxx1); } //expected-error 2{{use of undeclared identifier 'g_910'; did you mean 'g_999'}} \
+ expected-error {{no member named 'xxx1' in 'struct a'; did you mean 'xxx'}}
Index: clang/test/Sema/typo-correction-ambiguity.c
===
--- clang/test/Sema/typo-correction-ambiguity.c
+++ clang/test/Sema/typo-correction-ambiguity.c
@@ -12,3 +12,16 @@
v_2_0(v_195,  // expected-error {{use of undeclared identifier 'v_195'}}
  v_231);  // expected-error {{use of undeclared identifier 'v_231'}}
 }
+
+// Test: no typo-correction diagnostics are emitted for ambiguous typos.
+struct a {
+  int xxx;
+};
+
+int g_107;
+int g_108;
+int g_109;
+
+struct a g_999;
+struct a g_998;
+void PR50797() { (g_910.xxx = g_910.xxx); } //expected-error 2{{use of undeclared identifier 'g_910'}}
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -8343,6 +8343,7 @@
 
 AmbiguousTypoExprs.remove(TE);
 SemaRef.getTypoExprState(TE).Consumer->restoreSavedPosition();
+TransformCache[TE] = SavedTransformCache[TE];
   }
   TransformCache = std::move(SavedTransformCache);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

Can you please explain why you do the same thing in two different ways?




Comment at: 
clang/include/clang/StaticAnalyzer/Core/PathSensitive/SymbolManager.h:293-296
+SymbolRef Sym = Operand;
+while (isa(Sym))
+  Sym = cast(Sym)->Operand;
+return Sym;




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

https://reviews.llvm.org/D103096

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@vsavchenko




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:2797-2799
+ProgramStateRef
+RangeConstraintManager::updateExistingConstraints(ProgramStateRef State,
+  SymbolRef Sym, RangeSet R) {

ASDenysPetrov wrote:
> vsavchenko wrote:
> > OK, but I still don't understand one thing.
> > Here you go over all "smaller" types and artificially create constraints 
> > for them, and at the same time in `VisitSymbolCast` you do the opposite 
> > operation?  Why?  Shouldn't the map have constraints for smaller types 
> > already because of this action?  Why do we need to do both?
> > 
> I've been preparing an answer for you, but suddenly you inspired me on some 
> impovements. Thanks.
I've fixed `RangeConstraintManager::updateExistingConstraints`. There was a 
mistake when I update smaller types from the **root** symbol, but correct 
symbol is the **given** symbol which is before calling `ignoreCast()`.
May be now it would be more clear for you.


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

https://reviews.llvm.org/D103096

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103096#2866704 , @ASDenysPetrov 
wrote:

> @vsavchenko

That's not the question I'm asking.  Why do you need to set constraints for 
other symbolic expressions, when `SymbolicInferrer` can look them up on its 
own?  Which cases will fail if we remove that part altogether?


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

https://reviews.llvm.org/D103096

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

[PATCH] D105629: [TSan] Add SystemZ support

[Ilya Leoshkevich via Phabricator via cfe-commits]

iii updated this revision to Diff 357471.
iii added a comment.

- Reserve the address space "tail" in the "[TSan] Define C/C++ address ranges 
for SystemZ" patch.
- Drop the "[TSan] Simulate OOM in mmap_interceptor()" patch.
- Group "[TSan] Use zeroext for function parameters" with the common code 
patches.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105629

Files:
  clang/lib/Driver/ToolChains/Linux.cpp
  compiler-rt/cmake/config-ix.cmake
  compiler-rt/lib/sanitizer_common/sanitizer_common_interceptors.inc
  compiler-rt/lib/sanitizer_common/sanitizer_linux.cpp
  compiler-rt/lib/sanitizer_common/sanitizer_platform_limits_posix.h
  compiler-rt/lib/tsan/CMakeLists.txt
  compiler-rt/lib/tsan/rtl/tsan_interceptors_posix.cpp
  compiler-rt/lib/tsan/rtl/tsan_interface.h
  compiler-rt/lib/tsan/rtl/tsan_platform.h
  compiler-rt/lib/tsan/rtl/tsan_platform_linux.cpp
  compiler-rt/lib/tsan/rtl/tsan_platform_posix.cpp
  compiler-rt/lib/tsan/rtl/tsan_rtl_s390x.S
  compiler-rt/test/tsan/ignore_lib0.cpp
  compiler-rt/test/tsan/ignore_lib1.cpp
  compiler-rt/test/tsan/ignore_lib5.cpp
  compiler-rt/test/tsan/map32bit.cpp
  compiler-rt/test/tsan/mmap_large.cpp
  llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
  llvm/utils/gn/secondary/compiler-rt/lib/tsan/BUILD.gn

Index: llvm/utils/gn/secondary/compiler-rt/lib/tsan/BUILD.gn
===
--- llvm/utils/gn/secondary/compiler-rt/lib/tsan/BUILD.gn
+++ llvm/utils/gn/secondary/compiler-rt/lib/tsan/BUILD.gn
@@ -125,6 +125,8 @@
 sources += [ "rtl/tsan_rtl_ppc64.S" ]
   } else if (target_cpu == "mips64") {
 sources += [ "rtl/tsan_rtl_mips64.S" ]
+  } else if (target_cpu == "s390x") {
+sources += [ "rtl/tsan_rtl_s390x.S" ]
   }
 
   # To be able to include sanitizer_common.
Index: llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
===
--- llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
+++ llvm/lib/Transforms/Instrumentation/ThreadSanitizer.cpp
@@ -311,12 +311,21 @@
 Type *Ty = Type::getIntNTy(M.getContext(), BitSize);
 Type *PtrTy = Ty->getPointerTo();
 SmallString<32> AtomicLoadName("__tsan_atomic" + BitSizeStr + "_load");
-TsanAtomicLoad[i] =
-M.getOrInsertFunction(AtomicLoadName, Attr, Ty, PtrTy, OrdTy);
+{
+  AttributeList AL = Attr;
+  AL = AL.addParamAttribute(M.getContext(), 1, Attribute::ZExt);
+  TsanAtomicLoad[i] =
+  M.getOrInsertFunction(AtomicLoadName, AL, Ty, PtrTy, OrdTy);
+}
 
 SmallString<32> AtomicStoreName("__tsan_atomic" + BitSizeStr + "_store");
-TsanAtomicStore[i] = M.getOrInsertFunction(
-AtomicStoreName, Attr, IRB.getVoidTy(), PtrTy, Ty, OrdTy);
+{
+  AttributeList AL = Attr;
+  AL = AL.addParamAttribute(M.getContext(), 1, Attribute::ZExt);
+  AL = AL.addParamAttribute(M.getContext(), 2, Attribute::ZExt);
+  TsanAtomicStore[i] = M.getOrInsertFunction(
+  AtomicStoreName, AL, IRB.getVoidTy(), PtrTy, Ty, OrdTy);
+}
 
 for (unsigned Op = AtomicRMWInst::FIRST_BINOP;
  Op <= AtomicRMWInst::LAST_BINOP; ++Op) {
@@ -339,24 +348,44 @@
   else
 continue;
   SmallString<32> RMWName("__tsan_atomic" + itostr(BitSize) + NamePart);
-  TsanAtomicRMW[Op][i] =
-  M.getOrInsertFunction(RMWName, Attr, Ty, PtrTy, Ty, OrdTy);
+  {
+AttributeList AL = Attr;
+AL = AL.addParamAttribute(M.getContext(), 1, Attribute::ZExt);
+AL = AL.addParamAttribute(M.getContext(), 2, Attribute::ZExt);
+TsanAtomicRMW[Op][i] =
+M.getOrInsertFunction(RMWName, AL, Ty, PtrTy, Ty, OrdTy);
+  }
 }
 
 SmallString<32> AtomicCASName("__tsan_atomic" + BitSizeStr +
   "_compare_exchange_val");
-TsanAtomicCAS[i] = M.getOrInsertFunction(AtomicCASName, Attr, Ty, PtrTy, Ty,
- Ty, OrdTy, OrdTy);
+{
+  AttributeList AL = Attr;
+  AL = AL.addParamAttribute(M.getContext(), 1, Attribute::ZExt);
+  AL = AL.addParamAttribute(M.getContext(), 2, Attribute::ZExt);
+  AL = AL.addParamAttribute(M.getContext(), 3, Attribute::ZExt);
+  AL = AL.addParamAttribute(M.getContext(), 4, Attribute::ZExt);
+  TsanAtomicCAS[i] = M.getOrInsertFunction(AtomicCASName, AL, Ty, PtrTy, Ty,
+   Ty, OrdTy, OrdTy);
+}
   }
   TsanVptrUpdate =
   M.getOrInsertFunction("__tsan_vptr_update", Attr, IRB.getVoidTy(),
 IRB.getInt8PtrTy(), IRB.getInt8PtrTy());
   TsanVptrLoad = M.getOrInsertFunction("__tsan_vptr_read", Attr,
IRB.getVoidTy(), IRB.getInt8PtrTy());
-  TsanAtomicThreadFence = M.getOrInsertFunction("__tsan_atomic_thread_fence",
-Attr, IRB.ge

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

To be more sure of not breaking something we'd likely have to reduce the cases 
where tok::identifier was checked, it depends if "not catching every case" is 
caught is more acceptable. I certainly see that elsewhere in clang (like 
identifying where override is missing)


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

https://reviews.llvm.org/D69764

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

[PATCH] D102273: [analyzer] LoopUnrolling: fix crash when a loop counter is captured in a lambda by reference

[Abbas Sabra via Phabricator via cfe-commits]

AbbasSabra added a comment.

In D102273#2866532 , @vsavchenko 
wrote:

> Great!  Thanks for addressing all of the comments!

Thank you for the review! Can you take care of merging it? I don't have the 
required permission.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102273

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

[clang] 329f819 - [NFC][Clang][CodegenOpenCL] Fix test not to rely on volatile store not being removed

[Roman Lebedev via cfe-commits]

Author: Roman Lebedev
Date: 2021-07-09T14:16:54+03:00
New Revision: 329f8197ef59f9bd23328b52d623ba768b51dbb2

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

LOG: [NFC][Clang][CodegenOpenCL] Fix test not to rely on volatile store not 
being removed

Added: 


Modified: 
clang/test/CodeGenOpenCL/convergent.cl

Removed: 




diff  --git a/clang/test/CodeGenOpenCL/convergent.cl 
b/clang/test/CodeGenOpenCL/convergent.cl
index 1905d7dd81aa..a69b3d784e8c 100644
--- a/clang/test/CodeGenOpenCL/convergent.cl
+++ b/clang/test/CodeGenOpenCL/convergent.cl
@@ -3,11 +3,10 @@
 
 // This is initially assumed convergent, but can be deduced to not require it.
 
-// CHECK-LABEL: define{{.*}} spir_func void @non_convfun() local_unnamed_addr 
#0
+// CHECK-LABEL: define{{.*}} spir_func void @non_convfun(i32* %p) 
local_unnamed_addr #0
 // CHECK: ret void
 __attribute__((noinline))
-void non_convfun(void) {
-  volatile int* p;
+void non_convfun(volatile int* p) {
   *p = 0;
 }
 
@@ -28,29 +27,29 @@ void g(void);
 //  non_convfun();
 //}
 //
-// CHECK-LABEL: define{{.*}} spir_func void @test_merge_if(i32 %a) 
local_unnamed_addr #1 {
+// CHECK-LABEL: define{{.*}} spir_func void @test_merge_if(i32 %a, i32* %p) 
local_unnamed_addr #1 {
 // CHECK: %[[tobool:.+]] = icmp eq i32 %a, 0
 // CHECK: br i1 %[[tobool]], label %[[if_end3_critedge:.+]], label 
%[[if_then:.+]]
 
 // CHECK: [[if_then]]:
 // CHECK: tail call spir_func void @f()
-// CHECK: tail call spir_func void @non_convfun()
+// CHECK: tail call spir_func void @non_convfun(i32* %p)
 // CHECK: tail call spir_func void @g()
 
 // CHECK: br label %[[if_end3:.+]]
 
 // CHECK: [[if_end3_critedge]]:
-// CHECK: tail call spir_func void @non_convfun()
+// CHECK: tail call spir_func void @non_convfun(i32* %p)
 // CHECK: br label %[[if_end3]]
 
 // CHECK: [[if_end3]]:
 // CHECK: ret void
 
-void test_merge_if(int a) {
+void test_merge_if(int a, volatile int* p) {
   if (a) {
 f();
   }
-  non_convfun();
+  non_convfun(p);
   if (a) {
 g();
   }



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

[PATCH] D105648: [OpenMP] Support OpenMP 5.1 attributes

[Aaron Ballman via Phabricator via cfe-commits]

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



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:2670-2676
+
+  // The next token may be an OpenMP pragma annotation token. That would
+  // normally be handled from ParseCXXClassMemberDeclarationWithPragmas, but in
+  // this case, it came from an *attribute* rather than a pragma. Handle it 
now.
+  if (Tok.is(tok::annot_pragma_openmp_from_attr))
+return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, attrs);
+

erichkeane wrote:
> jdoerfert wrote:
> > cor3ntin wrote:
> > > The comment raises 2 questions: should it be called `annot_openmp_attr` 
> > > instead? Does the comment describe what this does?
> > > I imagine long terms attributes might be the ""normal"" scenario?
> > > I imagine long terms attributes might be the ""normal"" scenario?
> > 
> > We can't assume that (C) and for C++ only time will tell.
> FWIW, C23 is getting C++ style attributes as well, so we MIGHT be able to 
> make that assumption some day :D
> 
> That said, the fact that these are called "PRAGMA_ANNOTATION" in 
> TokenKinds.def seems misnamed to me anymore, which I think is the confusion.  
> It is a little strange that the `annot` is added automatically, but the 
> `pragma` isn't... 
> 
> Either way, I think it is debatable what the `pragma` in these relates to.  
> My opinion is that it applies to the syntax (since the rest are #pragma 
> SOMETHING), not that it is a `PRAGMA_ANNOTATION`, and I liked 
> `annot_attr_openmp` to match `annot_pragma_openmp`, but I don't feel terribly 
> strongly.  See our conversation on TokenKinds for the other half of this 
> discussion.
> FWIW, C23 is getting C++ style attributes as well, so we MIGHT be able to 
> make that assumption some day :D

And FWIW, I'm explicitly supporting OpenMP 5.1 attributes when 
-fdouble-square-bracket-attributes is enabled along with OpenMP 5.1 (which 
includes C23 mode by default). I just noticed that the OpenMP spec doesn't 
require this support in C, so should I remove that support in this patch (we 
can enable it as an extension when we allow it in older OpenMP modes), should I 
diagnose this as an extension only in C mode, or should I enable this as an 
extension in all OpenMP modes and add diagnostics for it?

> That said, the fact that these are called "PRAGMA_ANNOTATION" in 
> TokenKinds.def seems misnamed to me anymore, which I think is the confusion. 
> It is a little strange that the annot is added automatically, but the pragma 
> isn't...

The fact that at least two people have found the name and definition of the 
token confusing means I'll be changing it. I think `ANNOTATION(attr_openmp)` 
will actually work out fine. The only use of the `PRAGMA_ANNOTATION` macro is 
in the definition of `tok::isPragmaAnnotation()` and the only places we call 
that function are places we're already looking for 
`tok::annot_pragma_openmp_from_attr` explicitly prior to the call anyway. The 
one oddity to this is that it starts with an `annot_attr_openmp` token and ends 
with an `annot_pragma_openmp_end` token -- but I don't think that should cause 
massive confusion (the end token is only interesting to the OpenMP parsing 
methods and those are designed around pragma terminology anyway).



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:4285
+// created for us by the caller.
+return true;
+  }

erichkeane wrote:
> aaron.ballman wrote:
> > aaron.ballman wrote:
> > > erichkeane wrote:
> > > > ABataev wrote:
> > > > > erichkeane wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > erichkeane wrote:
> > > > > > > > Another place to make the same comment :D  I wonder if giving a 
> > > > > > > > diagnostic on attempting to use omp::directive in a previous 
> > > > > > > > version should have either an extension warning or more 
> > > > > > > > explicit warning would be a good idea?
> > > > > > > Ah, now I see what you're after. We could do that, but I think 
> > > > > > > deciding whether to expose this in older modes is more of an open 
> > > > > > > question. I went with the conservative approach first, but we 
> > > > > > > could relax this later.
> > > > > > Well, I was going for 1 of two things:
> > > > > > 
> > > > > > 1- allow in older modes, then do a warning that you're using it in 
> > > > > > the wrong version.
> > > > > > 
> > > > > > 2- Customize the error from "unknown attribute directive" to 
> > > > > > something like, "attribute omp::directive is only available in 
> > > > > > OMP5.1 mode".
> > > > > I think we can enable it for the previous versions, at least as an 
> > > > > extension. Thoughts?
> > > > It'll mean a larger test-surface I'd think, but there _IS_ precedent 
> > > > both ways.  
> > > > IMO, we are probably better off doing #2 above (a better 'unknown 
> > > > attribute' diagnostic), then doing it as an extension if GCC makes that 
> > > > choice, or there is a compelling reaosn.
> > >

[PATCH] D105648: [OpenMP] Support OpenMP 5.1 attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 357479.
aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

Change `annot_pragma_openmp_from_attr` into `annot_attr_openmp` based on review 
feedback.


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

https://reviews.llvm.org/D105648

Files:
  clang/docs/OpenMPSupport.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/OpenMP/allocate_codegen_attr.cpp
  clang/test/OpenMP/assumes_messages_attr.c
  clang/test/OpenMP/critical_codegen_attr.cpp
  clang/test/OpenMP/masked_messages_attr.cpp
  clang/test/OpenMP/openmp_attribute.cpp
  clang/test/OpenMP/openmp_attribute_parsing.cpp
  clang/test/OpenMP/target_map_names_attr.cpp
  clang/test/OpenMP/taskloop_reduction_messages_attr.cpp
  
clang/test/OpenMP/teams_distribute_parallel_for_simd_num_teams_messages_attr.cpp
  clang/test/OpenMP/unroll_codegen_unroll_for_attr.cpp

Index: clang/test/OpenMP/unroll_codegen_unroll_for_attr.cpp
===
--- /dev/null
+++ clang/test/OpenMP/unroll_codegen_unroll_for_attr.cpp
@@ -0,0 +1,237 @@
+// Check code generation
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -emit-llvm %s -o - | FileCheck %s --check-prefix=IR
+
+// Check same results after serialization round-trip
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -emit-pch -o %t %s
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -include-pch %t -emit-llvm %s -o - | FileCheck %s --check-prefix=IR
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+// placeholder for loop body code.
+extern "C" void body(...) {}
+
+
+// IR-LABEL: @func(
+// IR-NEXT:  [[ENTRY:.*]]:
+// IR-NEXT:%[[START_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[END_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[STEP_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_IV:.+]] = alloca i32, align 4
+// IR-NEXT:%[[TMP:.+]] = alloca i32, align 4
+// IR-NEXT:%[[I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_1:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_2:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_3:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTUNROLLED_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_6:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_8:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_12:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_14:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTUNROLLED_IV__UNROLLED_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_LB:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_UB:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_STRIDE:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_IS_LAST:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTUNROLLED_IV__UNROLLED_IV_I18:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTUNROLL_INNER_IV__UNROLLED_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTUNROLL_INNER_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[TMP0:.+]] = call i32 @__kmpc_global_thread_num(%struct.ident_t* @2)
+// IR-NEXT:store i32 %[[START:.+]], i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[END:.+]], i32* %[[END_ADDR]], align 4
+// IR-NEXT:store i32 %[[STEP:.+]], i32* %[[STEP_ADDR]], align 4
+// IR-NEXT:%[[TMP1:.+]] = load i32, i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP1]], i32* %[[I]], align 4
+// IR-NEXT:%[[TMP2:.+]] = load i32, i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP2]], i32* %[[DOTCAPTURE_EXPR_]], align 4
+// IR-NEXT:%[[TMP3:.+]] = load i32, i32* %[[END_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP3]], i32* %[[DOTCAPTURE_EXPR_1]], align 4
+// IR-NEXT:%[[TMP4:.+]] = load i32, i32* %[[STEP_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP4]], i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[TMP5:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_1]], align 4
+// IR-NEXT:%[[TMP6:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_]], align 4
+// IR-NEXT:%[[SUB:.+]] = sub i32 %[[TMP5]], %[[TMP6]]
+// IR-NEXT:%[[SUB4:.+]] = sub i32 %[[SUB]], 1
+// IR-NEXT:%[[TMP7:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[ADD:.+]] = add i32 %[[SUB4]], %[[TMP7]]
+// IR-NEXT:%[[TMP8:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[DIV:.+]] = udiv i32 %[[ADD]], %[[TMP8]]
+// IR-NEXT:%[[SUB5:.+]] = sub i32 %[[DIV]], 

[PATCH] D92777: [clang-tidy] Add bugprone-strlen-in-array-index checker

[Whisperity via Phabricator via cfe-commits]

whisperity added a comment.

The tests are a bit slim. We definitely need more cases... what if the array 
isn't allocated locally, but received as a parameter? I take that we need both 
an `fgets` and a `strlen` call in the same function(?) to emit diagnostics. Are 
we sure it will work if the array is originally from somewhere else?




Comment at: clang-tools-extra/clang-tidy/bugprone/StrlenInArrayIndexCheck.cpp:21
+  Finder->addMatcher(
+  arraySubscriptExpr(
+  allOf(hasBase(implicitCastExpr(hasSourceExpression(

While certainly not natural, if we're already here, it wouldn't take more than 
a few additional lines to match the equivalent but "expanded" version of array 
indexing:

```
*(buf + strlen(buf) - 1) = 0;
```



Comment at: clang-tools-extra/clang-tidy/bugprone/StrlenInArrayIndexCheck.cpp:22
+  arraySubscriptExpr(
+  allOf(hasBase(implicitCastExpr(hasSourceExpression(
+declRefExpr(to(varDecl().bind("buf")),

What's a hard `implicitCastExpr` doing here? Does a cast always happen? I feel 
this might be a bit too restrictive... there are matcher adaptors like 
`ignoringParenImpCasts`, that route should be investigated.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/StrlenInArrayIndexCheck.cpp:26-28
+allOf(callee(functionDecl(
+  hasAnyName("::strlen", "::std::strlen",
+ "::wcslen", "::std::wcslen"))),

`strlen_s`, `wcslen_s`? If I am reading [[ 
http://en.cppreference.com/w/c/string/byte/strlen | this correctly ]] then in 
case the input buffer starts with a `NUL` (and the //safe size// is large 
enough) then it behaves the same way `strlen` and will still return 0.

If we are already here, it might be worth to also check for [[ 
http://pubs.opengroup.org/onlinepubs/9699919799/functions/strlen.html | strnlen 
]]. It is POSIX and BSD specific, but behaves the same way as `strlen_s`.



Comment at: 
clang-tools-extra/clang-tidy/bugprone/StrlenInArrayIndexCheck.cpp:31-32
+ equalsBoundNode("buf",
+anyOf(hasDescendant(binaryOperator(hasOperatorName("-"))),
+  binaryOperator(hasOperatorName("-")))
+  .bind("arrayExpr"),

Why is the inner matcher duplicated here? What cases do we have a direct (?) 
`-` and a non-direct (descendant) one?



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/bugprone-strlen-in-array-index.cpp:16-17
+
+// Examples were copied from the cert-fio37-c issue (with some added calls):
+// 
https://wiki.sei.cmu.edu/confluence/display/c/FIO37-C.+Do+not+assume+that+fgets%28%29+or+fgetws%28%29+returns+a+nonempty+string+when+successful
+

Such code examples might be covered by non-permissive copyright, which might be 
incompatible with LLVM's licence. I was unable to find anything wrt. copyright 
on SEI CERT's Confluence, which in this case makes the work to be //(strictly) 
copyrighted// by default. I am not sure what the actual situation is, but such 
a **direct mention** of copying //could// lead to problems.


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

https://reviews.llvm.org/D92777

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

[PATCH] D105648: [OpenMP] Support OpenMP 5.1 attributes

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:2670-2676
+
+  // The next token may be an OpenMP pragma annotation token. That would
+  // normally be handled from ParseCXXClassMemberDeclarationWithPragmas, but in
+  // this case, it came from an *attribute* rather than a pragma. Handle it 
now.
+  if (Tok.is(tok::annot_pragma_openmp_from_attr))
+return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, attrs);
+

aaron.ballman wrote:
> erichkeane wrote:
> > jdoerfert wrote:
> > > cor3ntin wrote:
> > > > The comment raises 2 questions: should it be called `annot_openmp_attr` 
> > > > instead? Does the comment describe what this does?
> > > > I imagine long terms attributes might be the ""normal"" scenario?
> > > > I imagine long terms attributes might be the ""normal"" scenario?
> > > 
> > > We can't assume that (C) and for C++ only time will tell.
> > FWIW, C23 is getting C++ style attributes as well, so we MIGHT be able to 
> > make that assumption some day :D
> > 
> > That said, the fact that these are called "PRAGMA_ANNOTATION" in 
> > TokenKinds.def seems misnamed to me anymore, which I think is the 
> > confusion.  It is a little strange that the `annot` is added automatically, 
> > but the `pragma` isn't... 
> > 
> > Either way, I think it is debatable what the `pragma` in these relates to.  
> > My opinion is that it applies to the syntax (since the rest are #pragma 
> > SOMETHING), not that it is a `PRAGMA_ANNOTATION`, and I liked 
> > `annot_attr_openmp` to match `annot_pragma_openmp`, but I don't feel 
> > terribly strongly.  See our conversation on TokenKinds for the other half 
> > of this discussion.
> > FWIW, C23 is getting C++ style attributes as well, so we MIGHT be able to 
> > make that assumption some day :D
> 
> And FWIW, I'm explicitly supporting OpenMP 5.1 attributes when 
> -fdouble-square-bracket-attributes is enabled along with OpenMP 5.1 (which 
> includes C23 mode by default). I just noticed that the OpenMP spec doesn't 
> require this support in C, so should I remove that support in this patch (we 
> can enable it as an extension when we allow it in older OpenMP modes), should 
> I diagnose this as an extension only in C mode, or should I enable this as an 
> extension in all OpenMP modes and add diagnostics for it?
> 
> > That said, the fact that these are called "PRAGMA_ANNOTATION" in 
> > TokenKinds.def seems misnamed to me anymore, which I think is the 
> > confusion. It is a little strange that the annot is added automatically, 
> > but the pragma isn't...
> 
> The fact that at least two people have found the name and definition of the 
> token confusing means I'll be changing it. I think `ANNOTATION(attr_openmp)` 
> will actually work out fine. The only use of the `PRAGMA_ANNOTATION` macro is 
> in the definition of `tok::isPragmaAnnotation()` and the only places we call 
> that function are places we're already looking for 
> `tok::annot_pragma_openmp_from_attr` explicitly prior to the call anyway. The 
> one oddity to this is that it starts with an `annot_attr_openmp` token and 
> ends with an `annot_pragma_openmp_end` token -- but I don't think that should 
> cause massive confusion (the end token is only interesting to the OpenMP 
> parsing methods and those are designed around pragma terminology anyway).
> And FWIW, I'm explicitly supporting OpenMP 5.1 attributes when 
> -fdouble-square-bracket-attributes is enabled along with OpenMP 5.1 (which 
> includes C23 mode by default). I just noticed that the OpenMP spec doesn't 
> require this support in C, so should I remove that support in this patch (we 
> can enable it as an extension when we allow it in older OpenMP modes), should 
> I diagnose this as an extension only in C mode, or should I enable this as an 
> extension in all OpenMP modes and add diagnostics for it?

I would support all this stuff as an extension and emit a warning/note for the 
old versions, possibly ignored by default.


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

https://reviews.llvm.org/D105648

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

[PATCH] D105692: [analyzer][solver][NFC] Introduce ConstraintAssignor

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, 
ASDenysPetrov, manas, RedDocMD.
Herald added subscribers: dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, 
rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The new component is a symmetric response to SymbolicRangeInferrer.
While the latter is the unified component, which answers all the
questions what does the solver knows about a particular symbolic
expression, assignor associates new constraints (aka "assumes")
with symbolic expressions and can imply additional knowledge that
the solver can extract and use later on.

- Why do we need it and why is SymbolicRangeInferrer not enough?

As it is noted before, the inferrer only helps us to get the most
precise range information based on the existing knowledge and on the
mathematical foundations of different operations that symbolic
expressions actually represent.  It doesn't introduce new constraints.

The assignor, on the other hand, can impose constraints on other
symbols using the same domain knowledge.

- But for some expressions, SymbolicRangeInferrer looks into constraints for 
similar expressions, why can't we do that for all the cases?

That's correct!  But in order to do something like this, we should
have a finite number of possible "similar expressions".

Let's say we are asked about `$a - $b` and we know something about
`$b - $a`.  The inferrer can invert this expression and check
constraints for `$b - $a`.  This is simple!
But let's say we are asked about `$a` and we know that `$a * $b != 0`.
In this situation, we can imply that `$a != 0`, but the inferrer shouldn't
try every possible symbolic expression `X` to check if `$a * X` or
`X * $a` is constrained to non-zero.

With the assignor mechanism, we can catch this implication right at
the moment we associate `$a * $b` with non-zero range, and set similar
constraints for `$a` and `$b` as well.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105692

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -669,6 +669,17 @@
   return getConstraint(State, EquivalenceClass::find(State, Sym));
 }
 
+LLVM_NODISCARD ProgramStateRef setConstraint(ProgramStateRef State,
+ EquivalenceClass Class,
+ RangeSet Constraint) {
+  return State->set(Class, Constraint);
+}
+
+LLVM_NODISCARD ProgramStateRef setConstraints(ProgramStateRef State,
+  ConstraintRangeTy Constraints) {
+  return State->set(Constraints);
+}
+
 //===--===//
 //   Equality/diseqiality abstraction
 //===--===//
@@ -1373,6 +1384,182 @@
   return {RangeFactory, ValueFactory.getValue(Min), ValueFactory.getValue(Max)};
 }
 
+//===--===//
+//New constraint handler
+//===--===//
+
+/// ConstraintAssignorBase is a small utility class that unifies visitor
+/// for ranges with a visitor for constraints (rangeset/range/constant).
+///
+/// It is designed to have one derived class, but generally it cna have more.
+/// Derived class can control which types we handle by defining methods of the
+/// following form:
+///
+///   bool handle${SYMBOL}To${CONSTRAINT}(const SYMBOL *Sym,
+///   CONSTRAINT Constraint);
+///
+/// where SYMBOL is the type of the symbol (e.g. SymSymExpr, SymbolCast, etc.)
+///   CONSTRAINT is the type of constraint (RangeSet/Range/Const)
+///   return value signifies whether we should try other handle methods
+///  (i.e. false would mean to stop right after calling this method)
+template  class ConstraintAssignorBase {
+public:
+  using Const = const llvm::APSInt &;
+
+#define DISPATCH(CLASS) return assign##CLASS##Impl(cast(Sym), Constraint)
+
+#define ASSIGN(CLASS, TO, SYM, CONSTRAINT) \
+  if (!static_cast(this)->assign##CLASS##To##TO(SYM, CONSTRAINT))   \
+  return false
+
+  void assign(SymbolRef Sym, RangeSet Constraint) {
+assignImpl(Sym, Constraint);
+  }
+
+  bool assignImpl(SymbolRef Sym, RangeSet Constraint) {
+switch (Sym->getKind()) {
+#define SYMBOL(Id, Parent) \
+  case SymExpr::Id##Kind:

[PATCH] D105648: [OpenMP] Support OpenMP 5.1 attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 8 inline comments as done.
aaron.ballman added inline comments.



Comment at: clang/lib/Parse/ParseDeclCXX.cpp:2670-2676
+
+  // The next token may be an OpenMP pragma annotation token. That would
+  // normally be handled from ParseCXXClassMemberDeclarationWithPragmas, but in
+  // this case, it came from an *attribute* rather than a pragma. Handle it 
now.
+  if (Tok.is(tok::annot_pragma_openmp_from_attr))
+return ParseOpenMPDeclarativeDirectiveWithExtDecl(AS, attrs);
+

ABataev wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > jdoerfert wrote:
> > > > cor3ntin wrote:
> > > > > The comment raises 2 questions: should it be called 
> > > > > `annot_openmp_attr` instead? Does the comment describe what this does?
> > > > > I imagine long terms attributes might be the ""normal"" scenario?
> > > > > I imagine long terms attributes might be the ""normal"" scenario?
> > > > 
> > > > We can't assume that (C) and for C++ only time will tell.
> > > FWIW, C23 is getting C++ style attributes as well, so we MIGHT be able to 
> > > make that assumption some day :D
> > > 
> > > That said, the fact that these are called "PRAGMA_ANNOTATION" in 
> > > TokenKinds.def seems misnamed to me anymore, which I think is the 
> > > confusion.  It is a little strange that the `annot` is added 
> > > automatically, but the `pragma` isn't... 
> > > 
> > > Either way, I think it is debatable what the `pragma` in these relates 
> > > to.  My opinion is that it applies to the syntax (since the rest are 
> > > #pragma SOMETHING), not that it is a `PRAGMA_ANNOTATION`, and I liked 
> > > `annot_attr_openmp` to match `annot_pragma_openmp`, but I don't feel 
> > > terribly strongly.  See our conversation on TokenKinds for the other half 
> > > of this discussion.
> > > FWIW, C23 is getting C++ style attributes as well, so we MIGHT be able to 
> > > make that assumption some day :D
> > 
> > And FWIW, I'm explicitly supporting OpenMP 5.1 attributes when 
> > -fdouble-square-bracket-attributes is enabled along with OpenMP 5.1 (which 
> > includes C23 mode by default). I just noticed that the OpenMP spec doesn't 
> > require this support in C, so should I remove that support in this patch 
> > (we can enable it as an extension when we allow it in older OpenMP modes), 
> > should I diagnose this as an extension only in C mode, or should I enable 
> > this as an extension in all OpenMP modes and add diagnostics for it?
> > 
> > > That said, the fact that these are called "PRAGMA_ANNOTATION" in 
> > > TokenKinds.def seems misnamed to me anymore, which I think is the 
> > > confusion. It is a little strange that the annot is added automatically, 
> > > but the pragma isn't...
> > 
> > The fact that at least two people have found the name and definition of the 
> > token confusing means I'll be changing it. I think 
> > `ANNOTATION(attr_openmp)` will actually work out fine. The only use of the 
> > `PRAGMA_ANNOTATION` macro is in the definition of 
> > `tok::isPragmaAnnotation()` and the only places we call that function are 
> > places we're already looking for `tok::annot_pragma_openmp_from_attr` 
> > explicitly prior to the call anyway. The one oddity to this is that it 
> > starts with an `annot_attr_openmp` token and ends with an 
> > `annot_pragma_openmp_end` token -- but I don't think that should cause 
> > massive confusion (the end token is only interesting to the OpenMP parsing 
> > methods and those are designed around pragma terminology anyway).
> > And FWIW, I'm explicitly supporting OpenMP 5.1 attributes when 
> > -fdouble-square-bracket-attributes is enabled along with OpenMP 5.1 (which 
> > includes C23 mode by default). I just noticed that the OpenMP spec doesn't 
> > require this support in C, so should I remove that support in this patch 
> > (we can enable it as an extension when we allow it in older OpenMP modes), 
> > should I diagnose this as an extension only in C mode, or should I enable 
> > this as an extension in all OpenMP modes and add diagnostics for it?
> 
> I would support all this stuff as an extension and emit a warning/note for 
> the old versions, possibly ignored by default.
Okay, I can do that.


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

https://reviews.llvm.org/D105648

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

[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko created this revision.
vsavchenko added reviewers: NoQ, xazax.hun, martong, steakhal, Szelethus, 
ASDenysPetrov, manas, RedDocMD.
Herald added subscribers: dkrupp, donat.nagy, mikhail.ramalho, a.sidorin, 
rnkovacs, szepet, baloghadamsoftware.
vsavchenko requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch simplifies the way we deal with (dis)equalities.
Due to the symmetry between constraint handler and range inferrer,
we can have very similar implementations of logic handling
questions about (dis)equality and assumptions involving (dis)equality.

It also helps us to remove one more visitor, and removes uncertainty
that we got all the right places to put `trackNE` and `trackEQ`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105693

Files:
  clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
  clang/test/Analysis/equality_tracking.c

Index: clang/test/Analysis/equality_tracking.c
===
--- clang/test/Analysis/equality_tracking.c
+++ clang/test/Analysis/equality_tracking.c
@@ -219,3 +219,17 @@
   if (c < 0)
 ;
 }
+
+void implyDisequalityFromGT(int a, int b) {
+  if (a > b) {
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
+  }
+}
+
+void implyDisequalityFromLT(int a, int b) {
+  if (a < b) {
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
+  }
+}
Index: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
===
--- clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
+++ clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp
@@ -688,54 +688,26 @@
 ///
 /// Equality check can have different forms (like a == b or a - b) and this
 /// class encapsulates those away if the only thing the user wants to check -
-/// whether it's equality/diseqiality or not and have an easy access to the
-/// compared symbols.
-struct EqualityInfo {
-public:
-  SymbolRef Left, Right;
-  // true for equality and false for disequality.
-  bool IsEquality = true;
-
-  void invert() { IsEquality = !IsEquality; }
-  /// Extract equality information from the given symbol and the constants.
-  ///
-  /// This function assumes the following expression Sym + Adjustment != Int.
-  /// It is a default because the most widespread case of the equality check
-  /// is (A == B) + 0 != 0.
-  static Optional extract(SymbolRef Sym, const llvm::APSInt &Int,
-const llvm::APSInt &Adjustment) {
-// As of now, the only equality form supported is Sym + 0 != 0.
-if (!Int.isNullValue() || !Adjustment.isNullValue())
-  return llvm::None;
-
-return extract(Sym);
-  }
-  /// Extract equality information from the given symbol.
-  static Optional extract(SymbolRef Sym) {
-return EqualityExtractor().Visit(Sym);
+/// whether it's equality/diseqiality or not.
+///
+/// \returns true if assuming this Sym to be true means equality of operands
+///  false if it means disequality of operands
+///  None otherwise
+Optional meansEquality(const SymSymExpr *Sym) {
+  switch (Sym->getOpcode()) {
+  case BO_Sub:
+// This case is: A - B != 0 -> disequality check.
+return false;
+  case BO_EQ:
+// This case is: A == B != 0 -> equality check.
+return true;
+  case BO_NE:
+// This case is: A != B != 0 -> diseqiality check.
+return false;
+  default:
+return llvm::None;
   }
-
-private:
-  class EqualityExtractor
-  : public SymExprVisitor> {
-  public:
-Optional VisitSymSymExpr(const SymSymExpr *Sym) const {
-  switch (Sym->getOpcode()) {
-  case BO_Sub:
-// This case is: A - B != 0 -> disequality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), false};
-  case BO_EQ:
-// This case is: A == B != 0 -> equality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), true};
-  case BO_NE:
-// This case is: A != B != 0 -> diseqiality check.
-return EqualityInfo{Sym->getLHS(), Sym->getRHS(), false};
-  default:
-return llvm::None;
-  }
-}
-  };
-};
+}
 
 //===--===//
 //Intersection functions
@@ -866,7 +838,13 @@
   }
 
   RangeSet VisitSymSymExpr(const SymSymExpr *Sym) {
-return VisitBinaryOperator(Sym);
+return intersect(
+RangeFactory,
+// If Sym is (dis)equality, we might have some information
+// on that in our equality classes data structure.
+getRangeForEqualities(Sym),
+// And we should always check what we can get from the operands.
+VisitBinaryOperator(Sym));
   }
 
 private:
@@ -907,9 +885,6 @@
 // calculate the effective rang

[PATCH] D105695: [clang][tooling] Accept Clang invocations with "-fno-integrated-as"

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 created this revision.
jansvoboda11 added reviewers: Bigcheese, dexonsmith, bmahjour.
jansvoboda11 added a project: clang.
jansvoboda11 requested review of this revision.

When "-fno-integrated-as" is passed to the Clang driver (or set by default by a 
specific toolchain), it will construct an assembler job in addition to the cc1 
job.

This patch handles such case in the Clang tooling library which by default 
expects only a single job to be present.

This fixes a test failure in `ClangScanDeps/headerwithname.cpp` and 
`ClangScanDeps/headerwithnamefollowedbyinclude.cpp` on AIX reported here: 
https://reviews.llvm.org/D103461#2841918


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105695

Files:
  clang/lib/Tooling/Tooling.cpp
  clang/unittests/Tooling/ToolingTest.cpp


Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -258,6 +258,31 @@
   EXPECT_TRUE(Consumer.SawSourceManager);
 }
 
+TEST(ToolInvocation, AllowExternalAssembler) {
+  auto OverlayFS = llvm::makeIntrusiveRefCnt(
+  llvm::vfs::getRealFileSystem());
+  auto InMemoryFS = llvm::makeIntrusiveRefCnt();
+  OverlayFS->pushOverlay(InMemoryFS);
+  auto Files =
+  llvm::makeIntrusiveRefCnt(FileSystemOptions(), OverlayFS);
+
+  std::vector Args;
+  Args.push_back("tool-executable");
+  Args.push_back("-fno-integrated-as");
+  Args.push_back("-c");
+  Args.push_back("test.cpp");
+
+  // We're not matching the "-c" argument with `clang::EmitObjAction` in order
+  // to avoid linking the clangCodeGen library. This does not affect the
+  // command-line handling code under test.
+  clang::tooling::ToolInvocation Invocation(
+  Args, std::make_unique(), Files.get());
+  InMemoryFS->addFile("test.cpp", 0,
+  llvm::MemoryBuffer::getMemBuffer("int main() {}\n"));
+
+  EXPECT_TRUE(Invocation.run());
+}
+
 struct VerifyEndCallback : public SourceFileCallbacks {
   VerifyEndCallback() : BeginCalled(0), EndCalled(0), Matched(false) {}
   bool handleBeginSource(CompilerInstance &CI) override {
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -93,8 +93,9 @@
   const driver::JobList &Jobs = Compilation->getJobs();
   const driver::ActionList &Actions = Compilation->getActions();
   bool OffloadCompilation = false;
+  bool ExternalAssembler = false;
   if (Jobs.size() > 1) {
-for (auto A : Actions){
+for (const auto *A : Actions) {
   // On MacOSX real actions may end up being wrapped in BindArchAction
   if (isa(A))
 A = *A->input_begin();
@@ -115,10 +116,15 @@
 OffloadCompilation = true;
 break;
   }
+  if (isa(A)) {
+ExternalAssembler = true;
+break;
+  }
 }
   }
+  bool MultipleJobsAllowed = OffloadCompilation || ExternalAssembler;
   if (Jobs.size() == 0 || !isa(*Jobs.begin()) ||
-  (Jobs.size() > 1 && !OffloadCompilation)) {
+  (Jobs.size() > 1 && !MultipleJobsAllowed)) {
 SmallString<256> error_msg;
 llvm::raw_svector_ostream error_stream(error_msg);
 Jobs.Print(error_stream, "; ", true);


Index: clang/unittests/Tooling/ToolingTest.cpp
===
--- clang/unittests/Tooling/ToolingTest.cpp
+++ clang/unittests/Tooling/ToolingTest.cpp
@@ -258,6 +258,31 @@
   EXPECT_TRUE(Consumer.SawSourceManager);
 }
 
+TEST(ToolInvocation, AllowExternalAssembler) {
+  auto OverlayFS = llvm::makeIntrusiveRefCnt(
+  llvm::vfs::getRealFileSystem());
+  auto InMemoryFS = llvm::makeIntrusiveRefCnt();
+  OverlayFS->pushOverlay(InMemoryFS);
+  auto Files =
+  llvm::makeIntrusiveRefCnt(FileSystemOptions(), OverlayFS);
+
+  std::vector Args;
+  Args.push_back("tool-executable");
+  Args.push_back("-fno-integrated-as");
+  Args.push_back("-c");
+  Args.push_back("test.cpp");
+
+  // We're not matching the "-c" argument with `clang::EmitObjAction` in order
+  // to avoid linking the clangCodeGen library. This does not affect the
+  // command-line handling code under test.
+  clang::tooling::ToolInvocation Invocation(
+  Args, std::make_unique(), Files.get());
+  InMemoryFS->addFile("test.cpp", 0,
+  llvm::MemoryBuffer::getMemBuffer("int main() {}\n"));
+
+  EXPECT_TRUE(Invocation.run());
+}
+
 struct VerifyEndCallback : public SourceFileCallbacks {
   VerifyEndCallback() : BeginCalled(0), EndCalled(0), Matched(false) {}
   bool handleBeginSource(CompilerInstance &CI) override {
Index: clang/lib/Tooling/Tooling.cpp
===
--- clang/lib/Tooling/Tooling.cpp
+++ clang/lib/Tooling/Tooling.cpp
@@ -93,8 +93,9 @@
   const driver::JobList &Jobs = Compilation->getJobs();
   const driver::ActionList &Actions = Compilation

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

[Jan Svoboda via Phabricator via cfe-commits]

jansvoboda11 added a comment.

In D103461#2862067 , @jansvoboda11 
wrote:

> In D103461#2841918 , @bmahjour 
> wrote:
>
>> @jansvoboda11 This change is causing the following LIT tests to fail on AIX:
>>
>>   Clang :: ClangScanDeps/headerwithdirname.cpp
>>   Clang :: ClangScanDeps/headerwithdirnamefollowedbyinclude.cpp
>>
>> The reason seems to be related to the fact that `-fno-integrated-as` is on 
>> by default on that platform. I get the same failure on Linux if I change the 
>> "compilation database" file to add `-fno-integrated-as` to the "command" 
>> line:
>>
>>   > /build_llvm/bin/clang-scan-deps -compilation-database 
>> ./headerwithdirname.cpp.tmp.cdb -j 1
>>   > Error while scanning dependencies for 
>> /build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir/headerwithdirname_input.cpp:
>>   error: unable to handle compilation, expected exactly one compiler job in 
>> ' "clang" "-cc1" "-triple" "powerpc64le-unknown-linux-gnu" "-S" ...;  
>> "/usr/bin/as" "-a64" "-mppc64" "-mlittle-endian" "-mpower8" "-I" 
>> "/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir"
>>  "-I" 
>> "/build_llvm/tools/clang/test/ClangScanDeps/Output/headerwithdirname.cpp.tmp.dir/foodir"
>>  "-I" "Inputs" "-o" "headerwithdirname_input.o" 
>> "/tmp/headerwithdirname_input-2e0050.s"; '
>
> Thanks for the report and the reproducer. I'll try to get a fix ready 
> tomorrow.

Fixed here: https://reviews.llvm.org/D105695


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] D105648: [OpenMP] Support OpenMP 5.1 attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 357487.
aaron.ballman added a comment.

Enable attribute support as an extension (with the usual diagnostics). Note 
that this adds some new diagnostic groups for OpenMP as well.


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

https://reviews.llvm.org/D105648

Files:
  clang/docs/OpenMPSupport.rst
  clang/include/clang/Basic/DiagnosticGroups.td
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Basic/TokenKinds.def
  clang/include/clang/Parse/Parser.h
  clang/lib/Basic/Attributes.cpp
  clang/lib/Parse/ParseCXXInlineMethods.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseOpenMP.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/OpenMP/allocate_codegen_attr.cpp
  clang/test/OpenMP/assumes_messages_attr.c
  clang/test/OpenMP/critical_codegen_attr.cpp
  clang/test/OpenMP/masked_messages_attr.cpp
  clang/test/OpenMP/openmp_attribute.cpp
  clang/test/OpenMP/openmp_attribute_compat.cpp
  clang/test/OpenMP/openmp_attribute_parsing.cpp
  clang/test/OpenMP/target_map_names_attr.cpp
  clang/test/OpenMP/taskloop_reduction_messages_attr.cpp
  
clang/test/OpenMP/teams_distribute_parallel_for_simd_num_teams_messages_attr.cpp
  clang/test/OpenMP/unroll_codegen_unroll_for_attr.cpp

Index: clang/test/OpenMP/unroll_codegen_unroll_for_attr.cpp
===
--- /dev/null
+++ clang/test/OpenMP/unroll_codegen_unroll_for_attr.cpp
@@ -0,0 +1,237 @@
+// Check code generation
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -emit-llvm %s -o - | FileCheck %s --check-prefix=IR
+
+// Check same results after serialization round-trip
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -emit-pch -o %t %s
+// RUN: %clang_cc1 -verify -triple x86_64-pc-linux-gnu -fopenmp -fopenmp-version=51 -include-pch %t -emit-llvm %s -o - | FileCheck %s --check-prefix=IR
+// expected-no-diagnostics
+
+#ifndef HEADER
+#define HEADER
+
+// placeholder for loop body code.
+extern "C" void body(...) {}
+
+
+// IR-LABEL: @func(
+// IR-NEXT:  [[ENTRY:.*]]:
+// IR-NEXT:%[[START_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[END_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[STEP_ADDR:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_IV:.+]] = alloca i32, align 4
+// IR-NEXT:%[[TMP:.+]] = alloca i32, align 4
+// IR-NEXT:%[[I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_1:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_2:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_3:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTUNROLLED_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_6:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_8:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_12:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTCAPTURE_EXPR_14:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTUNROLLED_IV__UNROLLED_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_LB:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_UB:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_STRIDE:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTOMP_IS_LAST:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTUNROLLED_IV__UNROLLED_IV_I18:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTUNROLL_INNER_IV__UNROLLED_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[DOTUNROLL_INNER_IV_I:.+]] = alloca i32, align 4
+// IR-NEXT:%[[TMP0:.+]] = call i32 @__kmpc_global_thread_num(%struct.ident_t* @2)
+// IR-NEXT:store i32 %[[START:.+]], i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[END:.+]], i32* %[[END_ADDR]], align 4
+// IR-NEXT:store i32 %[[STEP:.+]], i32* %[[STEP_ADDR]], align 4
+// IR-NEXT:%[[TMP1:.+]] = load i32, i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP1]], i32* %[[I]], align 4
+// IR-NEXT:%[[TMP2:.+]] = load i32, i32* %[[START_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP2]], i32* %[[DOTCAPTURE_EXPR_]], align 4
+// IR-NEXT:%[[TMP3:.+]] = load i32, i32* %[[END_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP3]], i32* %[[DOTCAPTURE_EXPR_1]], align 4
+// IR-NEXT:%[[TMP4:.+]] = load i32, i32* %[[STEP_ADDR]], align 4
+// IR-NEXT:store i32 %[[TMP4]], i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[TMP5:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_1]], align 4
+// IR-NEXT:%[[TMP6:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_]], align 4
+// IR-NEXT:%[[SUB:.+]] = sub i32 %[[TMP5]], %[[TMP6]]
+// IR-NEXT:%[[SUB4:.+]] = sub i32 %[[SUB]], 1
+// IR-NEXT:%[[TMP7:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:%[[ADD:.+]] = add i32 %[[SUB4]], %[[TMP7]]
+// IR-NEXT:%[[TMP8:.+]] = load i32, i32* %[[DOTCAPTURE_EXPR_2]], align 4
+// IR-NEXT:

[PATCH] D105648: [OpenMP] Support OpenMP 5.1 attributes

[Alexey Bataev via Phabricator via cfe-commits]

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

LG, but wait for others' comments/suggestions/etc.


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

https://reviews.llvm.org/D105648

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

[PATCH] D103465: [OpaquePtr] Track pointee types in Clang

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/Address.h:26
   llvm::Value *Pointer;
+  llvm::Type *PointeeType;
   CharUnits Alignment;

I think this will still end up being a problem when we try to look into the 
type for multi-dimension pointers.  Should we have this just store the 
`QualType` of the value instead, so we can unpack it later?  If we ever needed 
the `llvm::Type` of the Pointee, a `convertTypeForMem` is all that is standing 
in our way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103465

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

[PATCH] D105648: [OpenMP] Support OpenMP 5.1 attributes

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D105648#2866893 , @ABataev wrote:

> LG, but wait for others' comments/suggestions/etc.

Thank you for the review!


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

https://reviews.llvm.org/D105648

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

[PATCH] D105479: [clang-tidy] [PR50069] readability-braces-around-statements doesn't work well with [[likely]] [[unlikely]]

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thank you for the fix!


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

https://reviews.llvm.org/D105479

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

[PATCH] D105648: [OpenMP] Support OpenMP 5.1 attributes

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin accepted this revision.
cor3ntin added a comment.

Thanks for changing the name. It looks good!


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

https://reviews.llvm.org/D105648

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

[PATCH] D105660: [PowerPC][AIX] Add warning when alignment is incompatible with XL

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:3255-3256
+def warn_not_xl_compatible
+: Warning<"requested alignment of arguments 16 bytes or greater is not"
+  " compatible with previous versions of the AIX XL compiler">,
+  InGroup>;

Should we be talking about the AIX XL compiler in a Clang diagnostic?



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:3957
 
+  if (Context.getTargetInfo().getTriple().isOSAIX() && (AlignVal >= 16))
+Diag(AttrLoc, diag::warn_not_xl_compatible) << E->getSourceRange();




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105660

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

[PATCH] D105127: Implement P1401R5

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1491
+def err_constexpr_if_condition_expression_is_not_constant : Error<
+  "constexpr if condition is not a constant expression convertible to bool">;
 def err_static_assert_failed : Error<"static_assert failed%select{ %1|}0">;

rsmith wrote:
> mizvekov wrote:
> > cor3ntin wrote:
> > > mizvekov wrote:
> > > > Looks a bit easier to parse the english there.
> > > I would rather not change that, to remain consistent with existing 
> > > diagnostics involving `constexpr if`
> > > But I agree it might be good to change them all
> > I see, yeah agreed.
> Would it be reasonable to drop the "convertible to bool" part here? We know 
> the problem is that the (converted) expression is not a constant expression, 
> not that the expression can't be converted to bool, because we handle the 
> conversion to bool separately before we get to this diagnostic; I think the 
> diagnostic would be clearer if it didn't mention the conversion.
+1 -- I think the "convertible to bool" may trip some users up on how to 
correct the issue.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:3925
+  // is contextually converted to bool and the converted expression shall be
+  // a constant expression
+  //





Comment at: clang/lib/Sema/SemaExprCXX.cpp:3930
+  ExprResult E = PerformContextuallyConvertToBool(CondExpr);
+  if (!IsConstexpr || !E.isUsable() || E.get()->isValueDependent())
+return E;





Comment at: clang/lib/Sema/SemaExprCXX.cpp:3922
   //
   // FIXME: Return this value to the caller so they don't need to recompute it.
+

mizvekov wrote:
> This FIXME got pushed out of the thing it used to refer to.
+1, I think this comment should move down to the `APSInt` declaration so it's 
clear which value we want returned to the caller someday.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127

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

[PATCH] D105127: Implement P1401R5

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman requested changes to this revision.
aaron.ballman added a comment.
This revision now requires changes to proceed.

Making it clear that there are still some minor changes left to be made.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127

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

[PATCH] D105127: Implement P1401R5

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 357501.
cor3ntin marked 3 inline comments as done.
cor3ntin added a comment.

Improve diagnostic message, fix comments, use isInvalid instead of isUsable


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1296,7 +1296,7 @@
 
   Narrowing contextual conversions to bool
   https://wg21.link/P1401R5";>P1401R5
-  No
+  Clang 13
 
 
   Trimming whitespaces before line splicing
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -188,3 +188,14 @@
 }
 void callFoo4() { foo4(42); }
 // expected-note@-1{{in instantiation of function template specialization 'foo4' requested here}}
+
+static_assert(42, "message");
+static_assert(42.0, "message"); // expected-warning {{implicit conversion from 'double' to 'bool' changes value from 42 to true}}
+constexpr int *p = 0;
+static_assert(p, "message"); // expected-error {{static_assert failed}}
+
+struct NotBool {
+} notBool;
+constexpr NotBool constexprNotBool;
+static_assert(notBool, "message");  // expected-error {{value of type 'struct NotBool' is not contextually convertible to 'bool'}}
+static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}}
Index: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
===
--- clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
+++ clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
@@ -38,18 +38,38 @@
 }
 #else
 namespace ccce {
+
+struct S {
+} s;
   void f() {
 if (5) {}
-if constexpr (5) {} // expected-error {{cannot be narrowed}}
+if constexpr (5) {
+}
   }
   template void g() {
-if constexpr (N) {} // expected-error {{cannot be narrowed}}
+if constexpr (N) {
+}
   }
-  template void g<5>(); // expected-note {{instantiation of}}
+  template void g<5>();
   void h() {
-if constexpr (4.3) {} // expected-error{{conversion from 'double' to 'bool' is not allowed in a converted constant expression}}
+if constexpr (4.3) { //expected-warning {{implicit conversion from 'double' to 'bool' changes value}}
+}
 constexpr void *p = nullptr;
-if constexpr (p) {} // expected-error{{conversion from 'void *const' to 'bool' is not allowed in a converted constant expression}}
+if constexpr (p) {
+}
+  }
+
+  void not_constant(int b, S s) { //  expected-note 2{{declared here}}
+if constexpr (bool(b)) {  // expected-error {{constexpr if condition is not a constant expression}} expected-note {{cannot be used in a constant expression}}
+}
+if constexpr (b) { // expected-error {{constexpr if condition is not a constant expression}} expected-note {{cannot be used in a constant expression}}
+}
+if constexpr (s) { // expected-error {{value of type 'ccce::S' is not contextually convertible to 'bool'}}
+}
+
+constexpr S constexprS;
+if constexpr (constexprS) { // expected-error {{value of type 'const ccce::S' is not contextually convertible to 'bool'}}
+}
   }
 }
 
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5634,12 +5634,8 @@
   //  implicitly converted to type T, where the converted
   //  expression is a constant expression and the implicit conversion
   //  sequence contains only [... list of conversions ...].
-  // C++1z [stmt.if]p2:
-  //  If the if statement is of the form if constexpr, the value of the
-  //  condition shall be a contextually converted constant expression of type
-  //  bool.
   ImplicitConversionSequence ICS =
-  CCE == Sema::CCEK_ConstexprIf || CCE == Sema::CCEK_ExplicitBool
+  CCE == Sema::CCEK_ExplicitBool
   ? TryContextuallyConvertToBool(S, From)
   : TryCopyInitialization(S, From, T,
   /*SuppressUserConversions=*/false,
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -3911,7 +3911,7 @@
 
 /// CheckCXXBooleanCondition - Returns true if a conversion to bool is invalid.
 ExprResult Sema::CheckCXXBooleanCondition(Expr *CondExpr, b

[PATCH] D105127: Implement P1401R5

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin marked 5 inline comments as done.
cor3ntin added inline comments.



Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:1491
+def err_constexpr_if_condition_expression_is_not_constant : Error<
+  "constexpr if condition is not a constant expression convertible to bool">;
 def err_static_assert_failed : Error<"static_assert failed%select{ %1|}0">;

aaron.ballman wrote:
> rsmith wrote:
> > mizvekov wrote:
> > > cor3ntin wrote:
> > > > mizvekov wrote:
> > > > > Looks a bit easier to parse the english there.
> > > > I would rather not change that, to remain consistent with existing 
> > > > diagnostics involving `constexpr if`
> > > > But I agree it might be good to change them all
> > > I see, yeah agreed.
> > Would it be reasonable to drop the "convertible to bool" part here? We know 
> > the problem is that the (converted) expression is not a constant 
> > expression, not that the expression can't be converted to bool, because we 
> > handle the conversion to bool separately before we get to this diagnostic; 
> > I think the diagnostic would be clearer if it didn't mention the conversion.
> +1 -- I think the "convertible to bool" may trip some users up on how to 
> correct the issue.
Sorry, I missed that feedback earlier, fixed now


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

In D103096#2866730 , @vsavchenko 
wrote:

> In D103096#2866704 , @ASDenysPetrov 
> wrote:
>
>> @vsavchenko
>
> That's not the question I'm asking.  Why do you need to set constraints for 
> other symbolic expressions, when `SymbolicInferrer` can look them up on its 
> own?  Which cases will fail if we remove that part altogether?

I see. Here is what fails in case if we don't update other constraints:

  void test(int x) {
if ((char)x > -10 && (char)x < 10) {
  if ((short)x == 8) {
// If you remove updateExistingConstraints,
// then `c` won't be 8. It would be [-10, 10] instead.
char c = x;
if (c != 8)
  clang_analyzer_warnIfReached(); // should no-warning, but fail
  }
}
  }


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

https://reviews.llvm.org/D103096

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

[PATCH] D105477: [AIX] Define __LONGDOUBLE64 macro

[Chris Bowler via Phabricator via cfe-commits]

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

The change made me think we should add the negative test with 
-mlong-double-128, but then I thought we should probably make that an error 
until we've sorted through the AIX binary compat implications and tested it.  
So the patch you've proposed makes sense to me and we should follow up with 
handling for mlong-double-128.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105477

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103096#2867021 , @ASDenysPetrov 
wrote:

> In D103096#2866730 , @vsavchenko 
> wrote:
>
>> In D103096#2866704 , 
>> @ASDenysPetrov wrote:
>>
>>> @vsavchenko
>>
>> That's not the question I'm asking.  Why do you need to set constraints for 
>> other symbolic expressions, when `SymbolicInferrer` can look them up on its 
>> own?  Which cases will fail if we remove that part altogether?
>
> I see. Here is what fails in case if we don't update other constraints:
>
>   void test(int x) {
> if ((char)x > -10 && (char)x < 10) {
>   if ((short)x == 8) {
> // If you remove updateExistingConstraints,
> // then `c` won't be 8. It would be [-10, 10] instead.
> char c = x;
> if (c != 8)
>   clang_analyzer_warnIfReached(); // should no-warning, but fail
>   }
> }
>   }

OK, it's something! Good!
I still want to hear a good explanation why is it done this way.  Here `c` is 
mapped to `(char)x`, and we have `[-10, 10]` directly associated with it, but 
we also have `(short)x` associated with `[8, 8]`.  Why can't `VisitSymbolCast` 
look up constraints for `(short)x` it already looks up for constraints for 
different casts already.




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1203
+if (!Opts.ShouldSupportSymbolicIntegerCasts)
+  return VisitSymExpr(Sym);
+

Why do you use `VisitSymExpr` here?  You want to interrupt all `Visits or... 
I'm not sure I fully understand.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1215
+  QualType T = RootSym->getType();
+  if (!T->isIntegralOrEnumerationType())
+return VisitSymExpr(Sym);

Can we get a test for that?



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1216
+  if (!T->isIntegralOrEnumerationType())
+return VisitSymExpr(Sym);
+

Same goes here.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1241
+  // Find the first constraint and exit the loop.
+  RSPtr = getConstraint(State, S);
+}

Why do you get associated constraint directly without consulting with what 
`SymbolRangeInferrer` can tell you about it?


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

https://reviews.llvm.org/D103096

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

Generally, with this patch we kinda have several constraints for each cast of a 
single symbol. And we shall care for all of that constraints and timely update 
them (if possible).
For instance, we have `int x` and met casts of this symbol in code:

  int x;
  (char)x; // we can reason about the 1st byte
  (short)x; // we can reason about the 2 lowest bytes
  (ushort)x; // we can reason about the 2 lowest bytes (in this case we may not 
store for unsigned separately, as we already stored 2 bytes for signed)

That's like we have a knowledge of a lower //part// of the integer. And every 
time we have a new constraints, for example, for `(short)x;` (aka 2 bytes) then 
we have to update all the constraints that have two bytes or lower (`(char)x`in 
this case) as well to make them consistent.


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

https://reviews.llvm.org/D103096

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

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

In D69764#2863648 , @owenpan wrote:

> Has this been tested against a large code base? It also needs an unqualified 
> LGTM before it can be merged.

D105701: [clang-format] test revision (NOT FOR COMMIT) to demonstrate east/west 
const fixer capability  demonstrates 
transforming clang-format itself to east const.

Actually transformation of the whole of the clang subfolder is actually holding 
up pretty well. I'm not seeing an violations (not sure if I transformed all the 
files) but certainly so much so that creating a review that covered all of it 
was way too big.

Testing on a large code base can be hard especially one as large as LLVM where 
its not currently fully clang-formatted in the first place.

Of course the lit tests get mangled as the test code gets swapped but the 
//CHECK-FIXES doesn't

Like I mentioned before, by gut feeling is that this option is MOST useful in 
preventing violation to your current style from creeping in than going to the 
extreme of transforming a whole project from east to west or vice versa.


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

https://reviews.llvm.org/D69764

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

[PATCH] D105478: [clang] Make CXXRecrdDecl invalid if it contains any undeduced fields.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz updated this revision to Diff 357511.
adamcz added a comment.

Fix a more generic case of invalid static initializer


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105478

Files:
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/test/AST/ast-dump-undeduced-expr.cpp
  clang/test/SemaCXX/cxx11-crashes.cpp


Index: clang/test/SemaCXX/cxx11-crashes.cpp
===
--- clang/test/SemaCXX/cxx11-crashes.cpp
+++ clang/test/SemaCXX/cxx11-crashes.cpp
@@ -104,3 +104,20 @@
   bool baz() { return __has_nothrow_constructor(B); }
   bool qux() { return __has_nothrow_copy(B); }
 }
+
+namespace undeduced_field {
+template
+struct Foo {
+  typedef T type;
+};
+
+struct Bar {
+  Bar();
+  // The missing expression makes A undeduced.
+  static constexpr auto A = ;  // expected-error {{expected expression}}
+  Foo::type B;  // The type of B is also undeduced (wrapped in 
Elaborated).
+};
+
+// This used to crash when trying to get the layout of B.
+Bar x;
+}
Index: clang/test/AST/ast-dump-undeduced-expr.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-undeduced-expr.cpp
@@ -0,0 +1,7 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck 
%s
+
+struct Foo {
+  static constexpr auto Bar = ;
+};
+
+// CHECK: -VarDecl {{.*}} invalid Bar 'const auto' static constexpr
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2982,9 +2982,11 @@
   ExprResult Init = ParseCXXMemberInitializer(
   ThisDecl, DeclaratorInfo.isDeclarationOfFunction(), EqualLoc);
 
-  if (Init.isInvalid())
+  if (Init.isInvalid()) {
+if (ThisDecl)
+  ThisDecl->setInvalidDecl();
 SkipUntil(tok::comma, StopAtSemi | StopBeforeMatch);
-  else if (ThisDecl)
+  } else if (ThisDecl)
 Actions.AddInitializerToDecl(ThisDecl, Init.get(), 
EqualLoc.isInvalid());
 } else if (ThisDecl && DS.getStorageClassSpec() == DeclSpec::SCS_static)
   // No initializer.


Index: clang/test/SemaCXX/cxx11-crashes.cpp
===
--- clang/test/SemaCXX/cxx11-crashes.cpp
+++ clang/test/SemaCXX/cxx11-crashes.cpp
@@ -104,3 +104,20 @@
   bool baz() { return __has_nothrow_constructor(B); }
   bool qux() { return __has_nothrow_copy(B); }
 }
+
+namespace undeduced_field {
+template
+struct Foo {
+  typedef T type;
+};
+
+struct Bar {
+  Bar();
+  // The missing expression makes A undeduced.
+  static constexpr auto A = ;  // expected-error {{expected expression}}
+  Foo::type B;  // The type of B is also undeduced (wrapped in Elaborated).
+};
+
+// This used to crash when trying to get the layout of B.
+Bar x;
+}
Index: clang/test/AST/ast-dump-undeduced-expr.cpp
===
--- /dev/null
+++ clang/test/AST/ast-dump-undeduced-expr.cpp
@@ -0,0 +1,7 @@
+// RUN: not %clang_cc1 -triple x86_64-unknown-unknown -ast-dump %s | FileCheck %s
+
+struct Foo {
+  static constexpr auto Bar = ;
+};
+
+// CHECK: -VarDecl {{.*}} invalid Bar 'const auto' static constexpr
Index: clang/lib/Parse/ParseDeclCXX.cpp
===
--- clang/lib/Parse/ParseDeclCXX.cpp
+++ clang/lib/Parse/ParseDeclCXX.cpp
@@ -2982,9 +2982,11 @@
   ExprResult Init = ParseCXXMemberInitializer(
   ThisDecl, DeclaratorInfo.isDeclarationOfFunction(), EqualLoc);
 
-  if (Init.isInvalid())
+  if (Init.isInvalid()) {
+if (ThisDecl)
+  ThisDecl->setInvalidDecl();
 SkipUntil(tok::comma, StopAtSemi | StopBeforeMatch);
-  else if (ThisDecl)
+  } else if (ThisDecl)
 Actions.AddInitializerToDecl(ThisDecl, Init.get(), EqualLoc.isInvalid());
 } else if (ThisDecl && DS.getStorageClassSpec() == DeclSpec::SCS_static)
   // No initializer.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D105478: [clang] Make CXXRecrdDecl invalid if it contains any undeduced fields.

[Adam Czachorowski via Phabricator via cfe-commits]

adamcz added a comment.

OK, I think we've got it now :-)

I kept the original crash test in the change, but I'm not sure if it's 
appropriate anymore. Let me know if you think I should remove it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105478

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@vsavchenko

> I still want to hear a good explanation why is it done this way.  Here `c` is 
> mapped to `(char)x`, and we have `[-10, 10]` directly associated with it, but 
> we also have `(short)x` associated with `[8, 8]`.  Why can't 
> `VisitSymbolCast` look up constraints for `(short)x` it already looks up for 
> constraints for different casts already.

Hm, you've confused me. I'll make some debugging and report.


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

https://reviews.llvm.org/D103096

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103096#2867104 , @ASDenysPetrov 
wrote:

> Generally, with this patch we kinda have several constraints for each cast of 
> a single symbol. And we shall care for all of that constraints and timely 
> update them (if possible).
> For instance, we have `int x` and meet casts of this symbol in code:
>
>   int x;
>   (char)x; // we can reason about the 1st byte
>   (short)x; // we can reason about the 2 lowest bytes
>   (ushort)x; // we can reason about the 2 lowest bytes (in this case we may 
> not store for unsigned separately, as we already stored 2 bytes for signed)
>
> That's like we have a knowledge of a lower //part// of the integer. And every 
> time we have a new constraints, for example, for `(short)x;` (aka 2 bytes) 
> then we have to update all the constraints that have two bytes or lower 
> (`(char)x`in this case) as well to make them consistent.

What we do in `Inferrer` is that we try to look at many sources of information 
and `intersect` their ranges.  And I repeat my question again in a bit 
different form, why can't it look up constraints for `(char)x` and for 
`(short)x` and intersect them?
You should admit you never really address this question.  Why can't 
`VisitSymolCast` do everything?


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

https://reviews.llvm.org/D103096

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

[PATCH] D105127: Implement P1401R5

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

This LGTM aside from a small nit, but you should wait a bit to land in case 
other reviewers have comments.




Comment at: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp:42-43
+
+struct S {
+} s;
   void f() {

This looks like it still needs some indentation to match the nearby local 
style. Also, do you need to declare the variable `s`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127

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

[PATCH] D105127: Implement P1401R5

[Aaron Ballman via Phabricator via cfe-commits]

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

Oops, I chanted the magic incantation but forgot to press the button. LGTM 
again. ;-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103096#2867136 , @ASDenysPetrov 
wrote:

> @vsavchenko
>
>> I still want to hear a good explanation why is it done this way.  Here `c` 
>> is mapped to `(char)x`, and we have `[-10, 10]` directly associated with it, 
>> but we also have `(short)x` associated with `[8, 8]`.  Why can't 
>> `VisitSymbolCast` look up constraints for `(short)x` it already looks up for 
>> constraints for different casts already.
>
> Hm, you've confused me. I'll make some debugging and report.

It should not be about debugging, it's your code!  Why did you write it this 
way!?


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

https://reviews.llvm.org/D103096

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

[PATCH] D103986: [PowerPC] Floating Point Builtins for XL Compat.

[Quinn Pham via Phabricator via cfe-commits]

quinnp updated this revision to Diff 357516.
quinnp added a comment.

Adressing some review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103986

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.h
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/builtins-ppc-xlcompat-fp.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelDAGToDAG.cpp
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/lib/Target/PowerPC/PPCInstrVSX.td
  llvm/test/CodeGen/builtins-ppc-xlcompat-fp.ll

Index: llvm/test/CodeGen/builtins-ppc-xlcompat-fp.ll
===
--- /dev/null
+++ llvm/test/CodeGen/builtins-ppc-xlcompat-fp.ll
@@ -0,0 +1,113 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
+; RUN:   -mcpu=pwr7 < %s | FileCheck %s --check-prefix=CHECK-PWR7
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown \
+; RUN:   -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK-PWR8
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   -mcpu=pwr7 < %s | FileCheck %s --check-prefix=CHECK-PWR7
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix \
+; RUN:   -mcpu=pwr7 < %s | FileCheck %s --check-prefix=CHECK-PWR7
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-unknown \
+; RUN:   -mattr=-vsx -mcpu=pwr7 < %s | FileCheck %s --check-prefix=CHECK-NOVSX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-unknown \
+; RUN:   -mattr=-vsx -mcpu=pwr8 < %s | FileCheck %s --check-prefix=CHECK-NOVSX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix \
+; RUN:   -mattr=-vsx -mcpu=pwr7 < %s | FileCheck %s --check-prefix=CHECK-NOVSX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix \
+; RUN:   -mattr=-vsx -mcpu=pwr7 < %s | FileCheck %s --check-prefix=CHECK-NOVSX
+
+define dso_local double @test_fsel(double %a, double %b, double %c) local_unnamed_addr {
+; CHECK-PWR7-LABEL: test_fsel:
+; CHECK-PWR7:   # %bb.0: # %entry
+; CHECK-PWR7-NEXT:fsel 1, 1, 2, 3
+; CHECK-PWR7-NEXT:blr
+;
+; CHECK-PWR8-LABEL: test_fsel:
+; CHECK-PWR8:   # %bb.0: # %entry
+; CHECK-PWR8-NEXT:fsel 1, 1, 2, 3
+; CHECK-PWR8-NEXT:blr
+;
+; CHECK-NOVSX-LABEL: test_fsel:
+; CHECK-NOVSX:   # %bb.0: # %entry
+; CHECK-NOVSX-NEXT:fsel 1, 1, 2, 3
+; CHECK-NOVSX-NEXT:blr
+
+entry:
+  %0 = tail call double @llvm.ppc.fsel(double %a, double %b, double %c)
+
+  ret double %0
+}
+
+declare double @llvm.ppc.fsel(double, double, double)
+
+define dso_local float @test_fsels(float %a, float %b, float %c) local_unnamed_addr {
+; CHECK-PWR7-LABEL: test_fsels:
+; CHECK-PWR7:   # %bb.0: # %entry
+; CHECK-PWR7-NEXT:fsel 1, 1, 2, 3
+; CHECK-PWR7-NEXT:blr
+;
+; CHECK-PWR8-LABEL: test_fsels:
+; CHECK-PWR8:   # %bb.0: # %entry
+; CHECK-PWR8-NEXT:fsel 1, 1, 2, 3
+; CHECK-PWR8-NEXT:blr
+;
+; CHECK-NOVSX-LABEL: test_fsels:
+; CHECK-NOVSX:   # %bb.0: # %entry
+; CHECK-NOVSX-NEXT:fsel 1, 1, 2, 3
+; CHECK-NOVSX-NEXT:blr
+
+entry:
+  %0 = tail call float @llvm.ppc.fsels(float %a, float %b, float %c)
+
+  ret float %0
+}
+
+declare float @llvm.ppc.fsels(float, float, float)
+
+define dso_local double @test_frsqrte(double %a) local_unnamed_addr {
+; CHECK-PWR7-LABEL: test_frsqrte:
+; CHECK-PWR7:   # %bb.0: # %entry
+; CHECK-PWR7-NEXT:xsrsqrtedp 1, 1
+; CHECK-PWR7-NEXT:blr
+;
+; CHECK-PWR8-LABEL: test_frsqrte:
+; CHECK-PWR8:   # %bb.0: # %entry
+; CHECK-PWR8-NEXT:xsrsqrtedp 1, 1
+; CHECK-PWR8-NEXT:blr
+;
+; CHECK-NOVSX-LABEL: test_frsqrte:
+; CHECK-NOVSX:   # %bb.0: # %entry
+; CHECK-NOVSX-NEXT:frsqrte 1, 1
+; CHECK-NOVSX-NEXT:blr
+
+entry:
+  %0 = tail call double @llvm.ppc.frsqrte(double %a)
+
+  ret double %0
+}
+
+declare double @llvm.ppc.frsqrte(double)
+
+define dso_local float @test_frsqrtes(float %a) local_unnamed_addr {
+; CHECK-PWR7-LABEL: test_frsqrtes:
+; CHECK-PWR7:   # %bb.0: # %entry
+; CHECK-PWR7-NEXT:frsqrtes 1, 1
+; CHECK-PWR7-NEXT:blr
+;
+; CHECK-PWR8-LABEL: test_frsqrtes:
+; CHECK-PWR8:   # %bb.0: # %entry
+; CHECK-PWR8-NEXT:xsrsqrtesp 1, 1
+; CHECK-PWR8-NEXT:blr
+;
+; CHECK-NOVSX-LABEL: test_frsqrtes:
+; CHECK-NOVSX:   # %bb.0: # %entry
+; CHECK-NOVSX-NEXT:frsqrtes 1, 1
+; CHECK-NOVSX-NEXT:blr
+
+entry:
+  %0 = tail call float @llvm.ppc.frsqrtes(float %a)
+
+  ret float %0
+}
+
+declare float @llvm.ppc.frsqrtes(float)
Index: llvm/lib/Target/PowerPC/PPCInstrVSX.td
===
--- llvm/lib/Target/PowerPC/PPCInstrVSX.td
+++ llvm/lib/Target/PowerPC/PPCInstrVSX.td
@@ -2850,6 +2850,8 @@
 def : Pat<(v2i64 (PPCvcmp_rec v2i64:$vA, v2i64:$vB, 199)),
   (VCMPGTUB_rec DblwdCmp.MRGEQ, (v2i64 (XXLXORz)))>;
 

[PATCH] D94098: [Clang][AArch64] Inline assembly support for the ACLE type 'data512_t'.

[Alexandros Lamprineas via Phabricator via cfe-commits]

labrinea added a comment.

In D94098#2865372 , @efriedma wrote:

> I'm confused what your goal here is, exactly.  The point of allowing 512-bit 
> inline asm operands is presumably to allow writing efficient code involving 
> inline asm... but you're intentionally destroying any potential efficiency by 
> forcing it to be passed/returned in memory.  If the user wanted to do that, 
> they could just use an "m" constraint.
>
> It looks like SelectionDAG currently crashes if you try to pass an array as 
> an inline asm operand, but that should be possible to fix, I think.

I have explained in the description why I am doing this: i512 is not a 
qualified type and so it is not possible to emit the store instruction required 
for output operands (line 2650 in the original code of 
clang/lib/CodeGen/CGStmt.cpp). As I said clang has already tests in place for 
this case (clang/test/CodeGen/X86/x86_64-PR42672.c - function big_struct), so I 
don't see how I am destroying the efficient codegen, which only applies to 
small sized integers (because they have a qualified type). Can you suggest a 
better solution?

Regarding the Selection DAG, my patches https://reviews.llvm.org/D94096 and 
https://reviews.llvm.org/D94097 are adding support for this use case in the 
backend. @t.p.northover has raised a concern there too, so maybe my original 
set of patches (including a dedicated IR type) in the RFC 
https://lists.llvm.org/pipermail/llvm-dev/2020-November/146860.html were a 
better fit?


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

https://reviews.llvm.org/D94098

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

[PATCH] D105692: [analyzer][solver][NFC] Introduce ConstraintAssignor

[Gabor Marton via Phabricator via cfe-commits]

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

Generally it looks okay to me. However, I have a question:
In `SimpleSValBuilder::evalBinOpNN` we do infer values to symbols. E.g. if we 
have an expression `y / x` and `y` is known to be `0` then we assign `0` to the 
division expression. On one hand, this logic is independent from whichever 
solver implementation we use. On the other hand, we do the value inferring 
based on the fact that `y` is a constrainted to be in `[0, 0]` so in this sense 
this infer logic should be in the solver.
What do you think, perhaps another round of refactoring should move that logic 
into the solver?




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1388
+//===--===//
+//New constraint handler
+//===--===//

`New` now, but it might be old after a while.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1394
+///
+/// It is designed to have one derived class, but generally it cna have more.
+/// Derived class can control which types we handle by defining methods of the

typo, `can`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105692

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

[PATCH] D105693: [analyzer][solver][NFC] Refactor how we detect (dis)equalities

[Gabor Marton via Phabricator via cfe-commits]

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

LGTM!




Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:687
 
 /// A small helper structure representing symbolic equality.
 ///

This is no longer a `structure`.



Comment at: clang/test/Analysis/equality_tracking.c:223-235
+void implyDisequalityFromGT(int a, int b) {
+  if (a > b) {
+clang_analyzer_eval(a == b); // expected-warning{{FALSE}}
+clang_analyzer_eval(a != b); // expected-warning{{TRUE}}
+  }
+}
+

Thanks, for the new test cases!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105693

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

[clang] 97c675d - Revert "Revert "Temporarily do not drop volatile stores before unreachable""

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2021-07-09T11:44:34-04:00
New Revision: 97c675d3d43fe02a0ff0a8350d79344c845758af

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

LOG: Revert "Revert "Temporarily do not drop volatile stores before 
unreachable""

This reverts commit 52aeacfbf5ce5f949efe0eae029e56db171ea1f7.
There isn't full agreement on a path forward yet, but there is agreement that
this shouldn't land as-is.  See discussion on https://reviews.llvm.org/D105338

Also reverts unreviewed "[clang] Improve `-Wnull-dereference` diag to be more 
in-line with reality"
This reverts commit f4877c78c0fc98be47b926439bbfe33d5e1d1b6d.

And all the related changes to tests:
This reverts commit 9a0152799f8e4a59e0483728c9f11c8a7805616f.
This reverts commit 3f7c9cc27422f7302cf5a683eeb3978e6cb84270.
This reverts commit 329f8197ef59f9bd23328b52d623ba768b51dbb2.
This reverts commit aa9f58cc2c48ca6cfc853a2467cd775dc7622746.
This reverts commit 2df37d5ddd38091aafbb7d338660e58836f4ac80.
This reverts commit a72a44181264fd83e05be958c2712cbd4560aba7.

Added: 
compiler-rt/test/fuzzer/NullDerefTest.cpp
compiler-rt/test/fuzzer/null-deref.test

Modified: 
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/lib/Sema/SemaExpr.cpp
clang/test/Analysis/NewDelete-checker-test.cpp
clang/test/Analysis/conditional-path-notes.c
clang/test/Analysis/cxx-for-range.cpp
clang/test/Analysis/diagnostics/no-prune-paths.c
clang/test/Analysis/inlining/path-notes.cpp
clang/test/Analysis/objc-arc.m
clang/test/Analysis/objc-for.m
clang/test/Analysis/taint-generic.c
clang/test/Analysis/valist-uninitialized.c
clang/test/CodeGenOpenCL/convergent.cl
clang/test/Parser/expressions.c
clang/test/Sema/exprs.c
clang/test/Sema/offsetof.c
clang/test/SemaCXX/member-pointer.cpp
compiler-rt/test/asan/TestCases/Windows/dll_control_c.cpp
compiler-rt/test/fuzzer/ShallowOOMDeepCrash.cpp
compiler-rt/test/fuzzer/coverage.test
compiler-rt/test/fuzzer/fork.test
compiler-rt/test/fuzzer/fuzzer-seed.test
compiler-rt/test/fuzzer/fuzzer-segv.test
compiler-rt/test/fuzzer/fuzzer-singleinputs.test
compiler-rt/test/fuzzer/minimize_crash.test
compiler-rt/test/sanitizer_common/TestCases/Linux/signal_line.cpp
llvm/lib/Transforms/InstCombine/InstructionCombining.cpp
llvm/lib/Transforms/Utils/Local.cpp
llvm/lib/Transforms/Utils/SimplifyCFG.cpp
llvm/test/CodeGen/AArch64/branch-relax-alignment.ll
llvm/test/CodeGen/AArch64/branch-relax-bcc.ll
llvm/test/CodeGen/AMDGPU/early-inline.ll
llvm/test/CodeGen/X86/indirect-branch-tracking-eh2.ll
llvm/test/Transforms/InstCombine/volatile_store.ll
llvm/test/Transforms/SimplifyCFG/trapping-load-unreachable.ll
llvm/utils/unittest/googletest/src/gtest.cc

Removed: 
compiler-rt/test/fuzzer/TrapTest.cpp
compiler-rt/test/fuzzer/trap.test



diff  --git a/clang/include/clang/Basic/DiagnosticSemaKinds.td 
b/clang/include/clang/Basic/DiagnosticSemaKinds.td
index d33d48846b18b..a9d7388950331 100644
--- a/clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ b/clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -6744,13 +6744,13 @@ def ext_typecheck_indirection_through_void_pointer : 
ExtWarn<
   "ISO C++ does not allow indirection on operand of type %0">,
   InGroup>;
 def warn_indirection_through_null : Warning<
-  "indirection of null pointer will be deleted, not trap">,
+  "indirection of non-volatile null pointer will be deleted, not trap">,
   InGroup;
 def warn_binding_null_to_reference : Warning<
   "binding dereferenced null pointer to reference has undefined behavior">,
   InGroup;
 def note_indirection_through_null : Note<
-  "consider using __builtin_trap()">;
+  "consider using __builtin_trap() or qualifying pointer with 'volatile'">;
 def warn_pointer_indirection_from_incompatible_type : Warning<
   "dereference of type %1 that was reinterpret_cast from type %0 has undefined 
"
   "behavior">,

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index d0efe4c02a1e3..a3a26d21422f0 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -533,16 +533,21 @@ ExprResult Sema::DefaultFunctionArrayConversion(Expr *E, 
bool Diagnose) {
 }
 
 static void CheckForNullPointerDereference(Sema &S, Expr *E) {
-  // Check to see if we are dereferencing a null pointer.
-  // If so, this is undefined behavior that the optimizer will delete,
-  // so warn about it. People sometimes try to use this to get a deterministic
-  // trap and are surprised by clang's behavior. This only handles the pattern
-  // "*null", which is a very syntactic check.
+  // Check to see if we are dereferencing a null pointer.  If so,
+  // and if not volatile-qualified, this is undefined beh

[PATCH] D105562: [OPENMP]Fix overlapped mapping for dereferenced pointer members.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev updated this revision to Diff 357532.
ABataev added a comment.
Herald added a project: OpenMP.
Herald added a subscriber: openmp-commits.

Rebase + added runtime test intended to be fixed by the patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105562

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_map_codegen_29.cpp
  openmp/libomptarget/test/mapping/target_pointers_members_map.cpp

Index: openmp/libomptarget/test/mapping/target_pointers_members_map.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/target_pointers_members_map.cpp
@@ -0,0 +1,55 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  short *a;
+  long d1, d2;
+} DV_A;
+
+typedef struct {
+  DV_A b;
+  long d3;
+} C;
+
+typedef struct {
+  C *c;
+  long d4, d5;
+} DV_B;
+
+int main() {
+
+  short arr1[10] = {10, 11, 12, 13, 14, 15, 16, 17, 18, 19};
+  short arr2[10] = {20, 31, 22, 23, 24, 25, 26, 27, 28, 29};
+
+  C c1[2];
+  c1[0].b.a = (short *)arr1;
+  c1[1].b.a = (short *)arr2;
+  c1[0].b.d1 = 111;
+
+  DV_B dvb1;
+  dvb1.c = (C *)&c1;
+
+  // CHECK: 10 111
+  printf("%d %ld %p %p %p %p\n", dvb1.c[0].b.a[0], dvb1.c[0].b.d1, &dvb1,
+ &dvb1.c[0], &dvb1.c[0].b, &dvb1.c[0].b.a[0]);
+#pragma omp target map(to  \
+   : dvb1, dvb1.c [0:2])   \
+map(tofrom \
+: dvb1.c[0].b.a [0:10], dvb1.c[1].b.a [0:10])
+  {
+// CHECK: 10 111
+printf("%d %ld %p %p %p %p\n", dvb1.c[0].b.a[0], dvb1.c[0].b.d1, &dvb1,
+   &dvb1.c[0], &dvb1.c[0].b, &dvb1.c[0].b.a[0]);
+dvb1.c[0].b.a[0] = 333;
+dvb1.c[0].b.d1 = 444;
+  }
+  // CHECK: 333 111
+  printf("%d %ld %p %p %p %p\n", dvb1.c[0].b.a[0], dvb1.c[0].b.d1, &dvb1,
+ &dvb1.c[0], &dvb1.c[0].b, &dvb1.c[0].b.a[0]);
+}
Index: clang/test/OpenMP/target_map_codegen_29.cpp
===
--- clang/test/OpenMP/target_map_codegen_29.cpp
+++ clang/test/OpenMP/target_map_codegen_29.cpp
@@ -38,9 +38,9 @@
 
 // CK30-LABEL: @.__omp_offloading_{{.*}}map_with_deep_copy{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
 // The first element: 0x20 - OMP_MAP_TARGET_PARAM
-// 2-4: 0x10003 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_TO | OMP_MAP_FROM - copies all the data in structs excluding deep-copied elements (from &s to &s.ptrBase1, from &s.ptr to &s.ptr1, from &s.ptr1 to end of s).
-// 5-6: 0x10013 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_PTR_AND_OBJ | OMP_MAP_TO | OMP_MAP_FROM - deep copy of the pointers + pointee.
-// CK30: [[MTYPE00:@.+]] = private {{.*}}constant [6 x i64] [i64 32, i64 281474976710659, i64 281474976710659, i64 281474976710659, i64 281474976710675, i64 281474976710675]
+// 2: 0x10003 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_TO | OMP_MAP_FROM - copies all the data in structs excluding deep-copied elements (from &s to end of s).
+// 3-4: 0x10013 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_PTR_AND_OBJ | OMP_MAP_TO | OMP_MAP_FROM - deep copy of the pointers + pointee.
+// CK30: [[MTYPE00:@.+]] = private {{.*}}constant [4 x i64] [i64 32, i64 281474976710659, i64 281474976710675, i64 281474976710675]
 
 typedef struct {
   int *ptrBase;
@@ -55,18 +55,18 @@
   int *ptr1;
 } StructWithPtr;
 
-// CK30-DAG: call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}}, i64 -1, i8* @.__omp_offloading_{{.*}}map_with_deep_copy{{.*}}_l{{[0-9]+}}.region_id, i32 6, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], i64* [[GEPS:%.+]], i64* getelementptr inbounds ([6 x i64], [6 x i64]* [[MTYPE00]], i32 0, i32 0), i8** null, i8** null)
-// CK30-DAG: [[GEPS]] = getelementptr inbounds [6 x i{{64|32}}], [6 x i64]* [[SIZES:%.+]], i32 0, i32 0
-// CK30-DAG: [[GEPP]] = getelementptr inbounds [6 x i8*], [6 x i8*]* [[PTRS:%.+]], i32 0, i32 0
-// CK30-DAG: [[GEPBP]] = getelementptr inbounds [6 x i8*], [6 x i8*]* [[BASES:%.+]], i32 0, i32 0
+// CK30-DAG: call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}}, i64 -1, i8* @.__omp_offloading_{{.*}}map_with_deep_copy{{.*}}_l{{[0-9]+}}.region_id, i32 4, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], i64* [[GEPS:%.+]], i64* getelementptr inbounds ([4 x i64], [4 x i64]* [[MTYPE00]], i32 0, i32 0), i8** null, i8** null)
+// CK30-DAG: [[GEPS]] = getelementptr inbounds [4 x i{{64|32}}], [4 x i64]* [[SIZES:%.+]], i32 0, i32 0
+// CK30-DAG: [[GEPP]] = getelementptr inbounds [4 x i8*], [4 x i8*]* [[PTRS:%.+]], i32 0, i32 0
+// CK30-DAG:

[PATCH] D105708: [analyzer][NFC] Display the correct function name even in crash dumps

[Balázs Benics via Phabricator via cfe-commits]

steakhal created this revision.
steakhal added reviewers: NoQ, vsavchenko, martong, Szelethus, ASDenysPetrov.
Herald added subscribers: manas, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, rnkovacs, szepet, baloghadamsoftware, xazax.hun, whisperity.
steakhal requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

The `-analyzer-display-progress` displayed the function name of the currently 
analyzed function.
It differs in C and C++. In C++, it prints the argument types as well in a 
comma-separated list.
While in C, only the function name is displayed, without the brackets.

E.g.:
C++: foo(), foo(int, float)
C: foo

In crash traces, the analyzer dumps the location contexts, but the string is 
not enough for `-analyze-function` in C++ mode.
This patch addresses the issue by dumping the proper function names even in 
stack traces.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105708

Files:
  clang/include/clang/Analysis/AnalysisDeclContext.h
  clang/lib/Analysis/AnalysisDeclContext.cpp
  clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
  clang/test/Analysis/crash-trace.c

Index: clang/test/Analysis/crash-trace.c
===
--- clang/test/Analysis/crash-trace.c
+++ clang/test/Analysis/crash-trace.c
@@ -1,4 +1,7 @@
-// RUN: not --crash %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection %s 2>&1 | FileCheck %s
+// RUN: not --crash %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection \
+// RUN:   -x c   %s 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-C-ONLY
+// RUN: not --crash %clang_analyze_cc1 -analyzer-checker=debug.ExprInspection \
+// RUN:   -x c++ %s 2>&1 | FileCheck %s --check-prefixes=CHECK,CHECK-CXX-ONLY
 // REQUIRES: crash-recovery
 
 // Stack traces require back traces.
@@ -6,17 +9,22 @@
 
 void clang_analyzer_crash(void);
 
-void inlined() {
+void inlined(int x, float y) {
   clang_analyzer_crash();
 }
 
 void test() {
-  inlined();
+  inlined(0, 0);
 }
 
-// CHECK: 0.	Program arguments: {{.*}}clang
-// CHECK-NEXT: 1.	 parser at end of file
-// CHECK-NEXT: 2. While analyzing stack: 
-// CHECK-NEXT:  #0 Calling inlined at line 14
-// CHECK-NEXT:  #1 Calling test
-// CHECK-NEXT: 3.	{{.*}}crash-trace.c:{{[0-9]+}}:3: Error evaluating statement
+// CHECK:0. Program arguments: {{.*}}clang
+// CHECK-NEXT:   1.  parser at end of file
+// CHECK-NEXT:   2. While analyzing stack:
+//
+// CHECK-C-ONLY-NEXT:   #0 Calling inlined at line 17
+// CHECK-C-ONLY-NEXT:   #1 Calling test
+//
+// CHECK-CXX-ONLY-NEXT: #0 Calling inlined(int, float) at line 17
+// CHECK-CXX-ONLY-NEXT: #1 Calling test()
+//
+// CHECK-NEXT:   3. {{.*}}crash-trace.c:{{[0-9]+}}:3: Error evaluating statement
Index: clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
===
--- clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
+++ clang/lib/StaticAnalyzer/Frontend/AnalysisConsumer.cpp
@@ -209,8 +209,8 @@
   } else
 assert(Mode == (AM_Syntax | AM_Path) && "Unexpected mode!");
 
-  llvm::errs() << ": " << Loc.getFilename() << ' ' << getFunctionName(D)
-   << '\n';
+  llvm::errs() << ": " << Loc.getFilename() << ' '
+   << AnalysisDeclContext::getFunctionName(D) << '\n';
 }
   }
 
@@ -568,63 +568,10 @@
   Mgr.reset();
 }
 
-std::string AnalysisConsumer::getFunctionName(const Decl *D) {
-  std::string Str;
-  llvm::raw_string_ostream OS(Str);
-
-  if (const FunctionDecl *FD = dyn_cast(D)) {
-OS << FD->getQualifiedNameAsString();
-
-// In C++, there are overloads.
-if (Ctx->getLangOpts().CPlusPlus) {
-  OS << '(';
-  for (const auto &P : FD->parameters()) {
-if (P != *FD->param_begin())
-  OS << ", ";
-OS << P->getType().getAsString();
-  }
-  OS << ')';
-}
-
-  } else if (isa(D)) {
-PresumedLoc Loc = Ctx->getSourceManager().getPresumedLoc(D->getLocation());
-
-if (Loc.isValid()) {
-  OS << "block (line: " << Loc.getLine() << ", col: " << Loc.getColumn()
- << ')';
-}
-
-  } else if (const ObjCMethodDecl *OMD = dyn_cast(D)) {
-
-// FIXME: copy-pasted from CGDebugInfo.cpp.
-OS << (OMD->isInstanceMethod() ? '-' : '+') << '[';
-const DeclContext *DC = OMD->getDeclContext();
-if (const auto *OID = dyn_cast(DC)) {
-  OS << OID->getName();
-} else if (const auto *OID = dyn_cast(DC)) {
-  OS << OID->getName();
-} else if (const auto *OC = dyn_cast(DC)) {
-  if (OC->IsClassExtension()) {
-OS << OC->getClassInterface()->getName();
-  } else {
-OS << OC->getIdentifier()->getNameStart() << '('
-   << OC->getIdentifier()->getNameStart() << ')';
-  }
-} else if (const auto *OCD = dyn_cast(DC)) {
-  OS << OCD->getClassInterface()->getName() << '('
- << OCD->getName() << ')';

[PATCH] D105692: [analyzer][solver][NFC] Introduce ConstraintAssignor

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1388
+//===--===//
+//New constraint handler
+//===--===//

martong wrote:
> `New` now, but it might be old after a while.
Whoops, this is how I first named the class.  It is not new handler, but new 
constraint.  Because of this ambiguity I actually renamed it 😅


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105692

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

[PATCH] D105127: Implement P1401R5

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin updated this revision to Diff 357536.
cor3ntin added a comment.

Fix test formatting


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105127

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaExprCXX.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/www/cxx_status.html

Index: clang/www/cxx_status.html
===
--- clang/www/cxx_status.html
+++ clang/www/cxx_status.html
@@ -1296,7 +1296,7 @@
 
   Narrowing contextual conversions to bool
   https://wg21.link/P1401R5";>P1401R5
-  No
+  Clang 13
 
 
   Trimming whitespaces before line splicing
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -188,3 +188,14 @@
 }
 void callFoo4() { foo4(42); }
 // expected-note@-1{{in instantiation of function template specialization 'foo4' requested here}}
+
+static_assert(42, "message");
+static_assert(42.0, "message"); // expected-warning {{implicit conversion from 'double' to 'bool' changes value from 42 to true}}
+constexpr int *p = 0;
+static_assert(p, "message"); // expected-error {{static_assert failed}}
+
+struct NotBool {
+} notBool;
+constexpr NotBool constexprNotBool;
+static_assert(notBool, "message");  // expected-error {{value of type 'struct NotBool' is not contextually convertible to 'bool'}}
+static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}}
Index: clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
===
--- clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
+++ clang/test/CXX/stmt.stmt/stmt.select/stmt.if/p2.cpp
@@ -38,18 +38,38 @@
 }
 #else
 namespace ccce {
+
+  struct S {
+  };
   void f() {
 if (5) {}
-if constexpr (5) {} // expected-error {{cannot be narrowed}}
+if constexpr (5) {
+}
   }
   template void g() {
-if constexpr (N) {} // expected-error {{cannot be narrowed}}
+if constexpr (N) {
+}
   }
-  template void g<5>(); // expected-note {{instantiation of}}
+  template void g<5>();
   void h() {
-if constexpr (4.3) {} // expected-error{{conversion from 'double' to 'bool' is not allowed in a converted constant expression}}
+if constexpr (4.3) { //expected-warning {{implicit conversion from 'double' to 'bool' changes value}}
+}
 constexpr void *p = nullptr;
-if constexpr (p) {} // expected-error{{conversion from 'void *const' to 'bool' is not allowed in a converted constant expression}}
+if constexpr (p) {
+}
+  }
+
+  void not_constant(int b, S s) { //  expected-note 2{{declared here}}
+if constexpr (bool(b)) {  // expected-error {{constexpr if condition is not a constant expression}} expected-note {{cannot be used in a constant expression}}
+}
+if constexpr (b) { // expected-error {{constexpr if condition is not a constant expression}} expected-note {{cannot be used in a constant expression}}
+}
+if constexpr (s) { // expected-error {{value of type 'ccce::S' is not contextually convertible to 'bool'}}
+}
+
+constexpr S constexprS;
+if constexpr (constexprS) { // expected-error {{value of type 'const ccce::S' is not contextually convertible to 'bool'}}
+}
   }
 }
 
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -5634,12 +5634,8 @@
   //  implicitly converted to type T, where the converted
   //  expression is a constant expression and the implicit conversion
   //  sequence contains only [... list of conversions ...].
-  // C++1z [stmt.if]p2:
-  //  If the if statement is of the form if constexpr, the value of the
-  //  condition shall be a contextually converted constant expression of type
-  //  bool.
   ImplicitConversionSequence ICS =
-  CCE == Sema::CCEK_ConstexprIf || CCE == Sema::CCEK_ExplicitBool
+  CCE == Sema::CCEK_ExplicitBool
   ? TryContextuallyConvertToBool(S, From)
   : TryCopyInitialization(S, From, T,
   /*SuppressUserConversions=*/false,
Index: clang/lib/Sema/SemaExprCXX.cpp
===
--- clang/lib/Sema/SemaExprCXX.cpp
+++ clang/lib/Sema/SemaExprCXX.cpp
@@ -3911,7 +3911,7 @@
 
 /// CheckCXXBooleanCondition - Returns true if a conversion to bool is invalid.
 ExprResult Sema::CheckCXXBooleanCondition(Expr *CondExpr, bool IsConstexpr) {
-  // C++ 6.4p4:
+  // C++11 6.4p4:
   // The value of a condition that is an 

[PATCH] D104975: Implement P1949

[Corentin Jabot via Phabricator via cfe-commits]

cor3ntin added a comment.

In D104975#2842425 , @jfb wrote:

> It would be more user-friendly to say which character is not allowed in the 
> diagnostic.

Agreed, done!

> Do we need to have a backward-compatible flag to preserve the old behavior, 
> or do we believe that nobody will be affected by the change? We should make 
> this choice explicitly and state it in the patch description.

I have been wondering about that.
I came to the conclusion it would probably not be worth it. Until fairly 
recently Unicode identifiers in GCC were not really usable and therefore not 
used afaik.
I haven't seen people use emojis or other interesting symbols in non-toy code.

I tried to make that clearer in the commit message


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104975

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

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D102107#2832740 , @ABataev wrote:

> In D102107#2832286 , @jdoerfert 
> wrote:
>
>> In D102107#2824581 , @ABataev 
>> wrote:
>>
>>> In D102107#2823706 , @jdoerfert 
>>> wrote:
>>>
 In D102107#2821976 , @ABataev 
 wrote:

> We used this kind of codegen initially but later found out that it causes 
> a large overhead when gathering pointers into a record. What about hybrid 
> scheme where the first args are passed as arguments and others (if any) 
> are gathered into a record?

 I'm confused, maybe I misunderstand the problem. The parallel function 
 arguments need to go from the main thread to the workers somehow, I don't 
 see how this is done w/o a record. This patch makes it explicit though.
>>>
>>> Pass it in a record for workers only? And use a hybrid scheme for all other 
>>> parallel regions.
>>
>> I still do not follow. What does it mean for workers only? What is a hybrid 
>> scheme? And, probably most importantly, how would we not eventually put 
>> everything into a record anyway?
>
> On the host you don’t need to put everything into a record, especially for 
> small parallel regions. Pass some first args in registers and only the 
> remaining args gather into the record. For workers just pass all args in the 
> record.

Could you please respond to my question so we make progress here. We *always* 
have to pass things in a record, do you agree? If we pack the things eventually 
to pass it to the workers, why would we not pack it right away and avoid 
complexity? Passing varargs, then packing them later (with the same thread) 
into a record to give it to the workers is arguably introducing cost. What is 
the benefit of a hybrid approach given that it is (theoretically) more costly 
and arguably more complex?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[PATCH] D104500: [clang] Apply P1825 as Defect Report from C++11 up to C++20.

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D104500#2865917 , @mizvekov wrote:

> Well I thought that meant exactly that libc++ does not support C++98, it only 
> works on clang because it provides so much of C++11 as an extension.
>
> I did not remove this just on my own whim by the way, the information I had 
> and the reason the patch was approved as is, is because as I said, those 
> extensions were considered best effort and not officially supported.
> @rsmith thoughts?

I don't know what the state of those extensions is from Clang's perspective, 
however one thing is clear - we use those extensions in libc++ very heavily. If 
any such extension is removed, libc++ will stop working in C++03 mode, which 
effectively means that most of the code compiled with Clang is going to break. 
That's a pretty serious problem.

In D104500#2866103 , @mizvekov wrote:

> Though if I may suggest, if we ever hope to sunset C++98, then to stop 
> providing these C++11 extensions would be a good first step...

Yes, I agree those extensions are terrible. However, libc++ has supported C++11 
library features with an approximative C++03 language mode ever since it was 
created. Removing support for something as fundamental as `std::unique_ptr` in 
C++03 mode is likely going to break a huge number of things, so we can't really 
do that. If there is a desire to sunset C++03, I could get behind that, but it 
would be an effort on its own and I don't think reducing what C++11 features we 
implement in C++03 mode would be part of that.

In the summary, you say:

> Note that we also remove implicit moves from C++98 as an extension 
> altogether, since the expanded first overload resolution from P1825 
>  can cause some meaning changes in C++98. For 
> example it can change which copy constructor is picked when both const and 
> non-const ones are available.

So basically, you decided to downright remove the extension because it caused 
some code to change meaning? However, in doing so, all the code that was 
previously moving implicitly will now perform a copy instead (unless the copy 
constructor is deleted, in which case the code breaks, as in libc++). That's a 
pretty big behavior change. Is that accurate, or am I misunderstanding 
something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104500

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

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D102107#2867382 , @jdoerfert wrote:

> In D102107#2832740 , @ABataev wrote:
>
>> In D102107#2832286 , @jdoerfert 
>> wrote:
>>
>>> In D102107#2824581 , @ABataev 
>>> wrote:
>>>
 In D102107#2823706 , @jdoerfert 
 wrote:

> In D102107#2821976 , @ABataev 
> wrote:
>
>> We used this kind of codegen initially but later found out that it 
>> causes a large overhead when gathering pointers into a record. What 
>> about hybrid scheme where the first args are passed as arguments and 
>> others (if any) are gathered into a record?
>
> I'm confused, maybe I misunderstand the problem. The parallel function 
> arguments need to go from the main thread to the workers somehow, I don't 
> see how this is done w/o a record. This patch makes it explicit though.

 Pass it in a record for workers only? And use a hybrid scheme for all 
 other parallel regions.
>>>
>>> I still do not follow. What does it mean for workers only? What is a hybrid 
>>> scheme? And, probably most importantly, how would we not eventually put 
>>> everything into a record anyway?
>>
>> On the host you don’t need to put everything into a record, especially for 
>> small parallel regions. Pass some first args in registers and only the 
>> remaining args gather into the record. For workers just pass all args in the 
>> record.
>
> Could you please respond to my question so we make progress here. We *always* 
> have to pass things in a record, do you agree?

On the GPU device, yes. And I'm absolutely fine with packing args for the GPU 
device. But the patch packs the args not only for the GPU devices but also for 
the host and other devices which may not require packing/unpacking. For such 
devices/host better to avoid packing/unpacking as it introduces overhead in 
many cases.

> If we pack the things eventually to pass it to the workers, why would we not 
> pack it right away and avoid complexity? Passing varargs, then packing them 
> later (with the same thread) into a record to give it to the workers is 
> arguably introducing cost. What is the benefit of a hybrid approach given 
> that it is (theoretically) more costly and arguably more complex?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added a comment.

@vsavchenko

> Why did you write it this way!?

I want the map contains only valid constraints at any time, so we can easely 
get them without traversing with all variants intersecting with each other. I'm 
gonna move `updateExistingConstraints ` logic to `VisitSymbolCast`. I think 
your suggestion can even improve the feature and cover some more cases. I'll 
add more tests in the next update. Thanks!


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

https://reviews.llvm.org/D103096

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

[PATCH] D101037: [clang-tidy] Change shebang from python to python3

[Keith Smiley via Phabricator via cfe-commits]

keith added a comment.

Doesn't seem like anything changed on the mailing list side, should we land 
this now?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D101037

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

[PATCH] D105708: [analyzer][NFC] Display the correct function name even in crash dumps

[Artem Dergachev via Phabricator via cfe-commits]

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

Amazing, thanks a lot!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105708

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

[PATCH] D105049: [NFC] Remove extra semicolons in clang/lib/APINotes/APINotesFormat.h

[Saleem Abdulrasool via Phabricator via cfe-commits]

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

Thanks for the cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105049

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

[PATCH] D105191: [Clang][OpenMP] Add support for Static Device Libraries

[George Rokos via Phabricator via cfe-commits]

grokos added inline comments.



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1689
+   : "lib" + libname + "-" + archname + "-" + gpuname,
+  "a");
+

"a" --> ".a" (add a dot)



Comment at: clang/lib/Driver/ToolChains/CommonArgs.cpp:1796-1798
+if (SDL_Name != "omp" && SDL_Name != "cudart" && SDL_Name != "m" &&
+SDL_Name != "gcc" && SDL_Name != "gcc_s" && SDL_Name != "pthread" &&
+SDL_Name != "hip_hcc") {

I'm with @jdoerfert here, you can use a set of library names which are known to 
not have device-specific SDLs and check whether that set contains `SDL_Name`. 
Also, `SDL_Names` can be a set of unique entries, this way even if you try to 
add the same library twice it won't be added. This quadratic-complexity loop 
looks ugly...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105191

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

[PATCH] D105421: [analyzer] Handle << operator for std::unique_ptr

[Artem Dergachev via Phabricator via cfe-commits]

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

Great, thanks!




Comment at: clang/test/Analysis/smart-ptr.cpp:472-474
+  // We are testing the fact that in our modelling of
+  // operator<<(basic_ostream &, const unique_ptr &)
+  // we set the return SVal to the SVal of the ostream arg.

Another thing to test could be that global variables are not invalidated. Eg.:

```lang=c++
int glob;
void foo(std::unique_ptr P) {
  int x = glob;
  std::cout << P;
  int y = glob;
  clang_analyzer_eval(x == y); // expected-warning{{TRUE}}
}



Comment at: clang/test/Analysis/smart-ptr.cpp:476
+  clang_analyzer_eval(&Cout == &PtrCout);// expected-warning {{TRUE}}
+  clang_analyzer_eval(&Cout == &StringCout); // expected-warning {{UNKNOWN}}
+}

This deserves a `FIXME:` because the expected result is still `TRUE`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105421

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

[PATCH] D105091: [RISCV] Pass -u to linker correctly.

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added inline comments.



Comment at: clang/test/Driver/riscv-args.c:5
 // RUN: %clang -### -target riscv32 \
 // RUN:   --gcc-toolchain= -Xlinker --defsym=FOO=10 -T a.lds %s 2>&1 \
 // RUN:   | FileCheck -check-prefix=CHECK-LD %s

kito-cheng wrote:
> MaskRay wrote:
> > MaskRay wrote:
> > > Just place all linker options on one run line.
> > > 
> > > It decreases the number of RUN lines and additionally checks the order 
> > > (though usually unimportant).
> > The test hasn't been updated?
> Oh, I only update the testcase I added.
You can place all link options on the same RUN line.

This additionally checks the order.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105091

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

[PATCH] D105637: [clang][Analyzer] Add symbol uninterestingness to bug report.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D105637#2864684 , @balazske wrote:

> Could not make a simple test for the change. This file F17831281: 
> BugReportInterestingnessTest.cpp  is what 
> I could do, but it prints the needed text to the console. Is it possible to 
> get the note texts in the program?

You could define your own diagnostic consumer in the unittest and intercept all 
the notes. But at this point i'd rather turn this into a LIT test by turning 
your checker mock into a `debug.ExprInspection` item and using 
`-analyzer-output=text` to test notes:

  void foo(x) {
clang_analyzer_noteIfInteresting(x); // no-note
clang_analyzer_markNotInteresting(x);
clang_analyzer_noteIfInteresting(x); // expected-note{{INTERESTING}}
clang_analyzer_markInteresting(x);
clang_analyzer_warnIfReached(); // expected-warning{{TRUE}}
  } // expected-note@-1{{TRUE}}


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105637

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

[PATCH] D105695: [clang][tooling] Accept Clang invocations with "-fno-integrated-as"

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith added inline comments.



Comment at: clang/lib/Tooling/Tooling.cpp:119-122
+  if (isa(A)) {
+ExternalAssembler = true;
+break;
+  }

Seems like this could (unexpectedly?) let through a command with multiple 
`-cc1`s (if I'm reading the code right). Consider these two cases:
```
lang=zsh
% clang -x c a.c b.c -c -fno-integrated-as -###
% clang -arch x86_64 -arch arm64 -x c /dev/null -c -fno-integrated-as -###
```
The first compiles two separate files; the second compiles a file with two 
separate architectures. But IIUC, the logic below doesn't expect this and will 
now let them through (ultimately, there's no reason scan-deps couldn't be 
updated to handle this! but that's not in scope for this patch, and maybe 
doesn't apply generally to clang-tooling).

Also, I'm not sure it's good that it was rejected, but I think the previous 
code would reject the following command-line and the new code will accept it:
```
% clang -x c /dev/null -###
```

I wonder if it'd be simpler to search for a supported action, and reject if 
zero or multiple are found. Something like:
```
lang=c++
const driver::Command *Cmd = nullptr;
for (const Action *A : Actions) {
  A = lookThrough(A);
  if (!shouldIgnore(*A)) // ignore some actions...
continue;
  if (shouldReject(*A)) // if needed, error on some actions...
return error(...);

  if (Cmd) // error if we hit a second candidate
return error(...);
  Cmd = cast(*A);
}
if (!Cmd) // error if we found no candidate
  return error(...);
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105695

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Denys Petrov via Phabricator via cfe-commits]

ASDenysPetrov added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp:1241
+  // Find the first constraint and exit the loop.
+  RSPtr = getConstraint(State, S);
+}

vsavchenko wrote:
> Why do you get associated constraint directly without consulting with what 
> `SymbolRangeInferrer` can tell you about it?
What do you mean? I didn't get. Could you give an example?


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

https://reviews.llvm.org/D103096

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

[PATCH] D105635: [PowerPC][AIX] Fix Zero-width bit fields wrt MaxFieldAlign.

[Zarko Todorovski via Phabricator via cfe-commits]

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

LGTM but I have a strong preference that 
`clang/test/Layout/aix-packed-bitfields.c` be committed separately if my 
understanding is right.




Comment at: clang/test/Layout/aix-bitfield-alignment.c:236
+
+#pragma align(packed)
+struct H {

nit: I prefer having these test cases together with the other `#pragma 
align/pack` test cases.



Comment at: clang/test/Layout/aix-packed-bitfields.c:1
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -fdump-record-layouts \
+// RUN: -fsyntax-only -fxl-pragma-pack -x c %s | FileCheck  %s

AFAIU this test case is not affected by your changes here.  Maybe commit this 
as a separate NFC patch? 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105635

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

[PATCH] D103096: [analyzer] Implement cast for ranges of symbolic integers.

[Valeriy Savchenko via Phabricator via cfe-commits]

vsavchenko added a comment.

In D103096#2867441 , @ASDenysPetrov 
wrote:

> @vsavchenko
>
>> Why did you write it this way!?
>
> I want the map contains only valid constraints at any time, so we can easely 
> get them without traversing with all variants intersecting with each other. 
> I'm gonna move `updateExistingConstraints ` logic to `VisitSymbolCast`. I 
> think your suggestion can even improve the feature and cover some more cases. 
> I'll add more tests in the next update. Thanks!

`[-10, 10]` is also valid, right?  You can't keep things at their best all the 
time.  And if you want all constraints directly in the map then what's all this 
logic in `VisitSymbolCast`?  That's why I keep asking why do you need both 
parts of this solution and didn't get any answer so far.
I'm hands down for the incremental approach and adding small-to-medium size 
improvements on top of each other.  That makes my life as a reviewer easier :) 
That's said, I don't want to commit to a big solution, where the author doesn't 
want to explain why there are two parts of the solution instead of one.

I want you to tell me why the code that's in `VisitSymbolCast` does what it 
does. And the same about `updateExistingConstraints`.  Also I want to hear a 
solid reason why it's split this way and why we need both of them.

You should understand that I'm not peaking on you personally.  The review 
process takes a lot of my time too.  I want to make it easier for both of us.  
When the reviewer understand what you are going for, it is much easier for them 
to help you in refining your solution.  This patch is very big, but the summary 
doesn't cover the main part: the approach.  And you leave me here dragging it 
out of you.


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

https://reviews.llvm.org/D103096

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

[PATCH] D103465: [OpaquePtr] Track pointee types in Clang

[James Y Knight via Phabricator via cfe-commits]

jyknight added inline comments.



Comment at: clang/lib/CodeGen/Address.h:26
   llvm::Value *Pointer;
+  llvm::Type *PointeeType;
   CharUnits Alignment;

erichkeane wrote:
> I think this will still end up being a problem when we try to look into the 
> type for multi-dimension pointers.  Should we have this just store the 
> `QualType` of the value instead, so we can unpack it later?  If we ever 
> needed the `llvm::Type` of the Pointee, a `convertTypeForMem` is all that is 
> standing in our way.
Do we do that? I didn't think Address was used that way.



Comment at: clang/lib/CodeGen/Address.h:29-30
 public:
   Address(llvm::Value *pointer, CharUnits alignment)
-  : Pointer(pointer), Alignment(alignment) {
+  : Address(pointer, nullptr, alignment) {}
+  Address(llvm::Value *pointer, llvm::Type *PointeeType, CharUnits alignment)

nikic wrote:
> dblaikie wrote:
> > At some point will this include an assertion that 'pointer' isn't a 
> > PointerType? I guess some uses of PointerTyped values won't need to know 
> > their pointee type?
> > 
> > (or are all values in Address PointerTyped? (owing to them being 
> > "addresses"))?
> Based on the unconditional cast in getType(), I'd assume that addresses are 
> always pointer typed.
This constructor seems odd. We shouldn't ever construct Address with a non-null 
pointer and a null PointeeType, should we?



Comment at: clang/lib/CodeGen/Address.h:31
+  : Address(pointer, nullptr, alignment) {}
+  Address(llvm::Value *pointer, llvm::Type *PointeeType, CharUnits alignment)
+  : Pointer(pointer), PointeeType(PointeeType), Alignment(alignment) {

craig.topper wrote:
> Is PointeeType expected to be non-null when pointer is non-null?
That would be my expectation.



Comment at: clang/lib/CodeGen/Address.h:58
   /// store it in Address instead for the convenience of writing code.
-  llvm::Type *getElementType() const {
-return getType()->getElementType();
-  }
+  llvm::Type *getElementType() const { return PointeeType; }
 

craig.topper wrote:
> Should this assert isValid() since it no longer goes through getType() and 
> getPointer() which would have asserted previously?
+1.



Comment at: clang/lib/CodeGen/CGValue.h:230-231
 private:
   void Initialize(QualType Type, Qualifiers Quals, CharUnits Alignment,
   LValueBaseInfo BaseInfo, TBAAAccessInfo TBAAInfo) {
 assert((!Alignment.isZero() || Type->isIncompleteType()) &&

dblaikie wrote:
> Maybe this could use some assertions added to check the PointeeType is 
> correct?
I think Initialize should be modified to take Addr instead of Alignment, and 
move
```
R.V = Addr.getPointer();
R.PointeeType = Addr.getElementType();
```
into this function, from all the callers.

If that's done, I don't think a separate assert is useful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D103465

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

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Jose Manuel Monsalve Diaz via Phabricator via cfe-commits]

josemonsalve2 added a comment.

In D102107#2867417 , @ABataev wrote:

> In D102107#2867382 , @jdoerfert 
> wrote:
>
>> In D102107#2832740 , @ABataev 
>> wrote:
>>
>>> In D102107#2832286 , @jdoerfert 
>>> wrote:
>>>
 In D102107#2824581 , @ABataev 
 wrote:

> In D102107#2823706 , @jdoerfert 
> wrote:
>
>> In D102107#2821976 , @ABataev 
>> wrote:
>>
>>> We used this kind of codegen initially but later found out that it 
>>> causes a large overhead when gathering pointers into a record. What 
>>> about hybrid scheme where the first args are passed as arguments and 
>>> others (if any) are gathered into a record?
>>
>> I'm confused, maybe I misunderstand the problem. The parallel function 
>> arguments need to go from the main thread to the workers somehow, I 
>> don't see how this is done w/o a record. This patch makes it explicit 
>> though.
>
> Pass it in a record for workers only? And use a hybrid scheme for all 
> other parallel regions.

 I still do not follow. What does it mean for workers only? What is a 
 hybrid scheme? And, probably most importantly, how would we not eventually 
 put everything into a record anyway?
>>>
>>> On the host you don’t need to put everything into a record, especially for 
>>> small parallel regions. Pass some first args in registers and only the 
>>> remaining args gather into the record. For workers just pass all args in 
>>> the record.
>>
>> Could you please respond to my question so we make progress here. We 
>> *always* have to pass things in a record, do you agree?
>
> On the GPU device, yes. And I'm absolutely fine with packing args for the GPU 
> device. But the patch packs the args not only for the GPU devices but also 
> for the host and other devices which may not require packing/unpacking. For 
> such devices/host better to avoid packing/unpacking as it introduces overhead 
> in many cases.

Hi Alexey,

Wouldn't you always need to pack to pass the arguments to the outlined 
function? What is the benefit of avoiding packing the arguments in the runtime 
call, if then you have to pack them for the outlined function?

I would really appreciate an example, since I am just getting an understanding 
of OpenMP in LLVM.

Thanks!

>> If we pack the things eventually to pass it to the workers, why would we not 
>> pack it right away and avoid complexity? Passing varargs, then packing them 
>> later (with the same thread) into a record to give it to the workers is 
>> arguably introducing cost. What is the benefit of a hybrid approach given 
>> that it is (theoretically) more costly and arguably more complex?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik updated this revision to Diff 357578.
shafik added reviewers: jingham, jasonmolenda.
shafik added a comment.

Changing approach based on Adrian's comments.


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

https://reviews.llvm.org/D105564

Files:
  lldb/source/Plugins/ExpressionParser/Clang/ASTResultSynthesizer.cpp
  lldb/source/Plugins/SymbolFile/DWARF/DWARFASTParserClang.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.cpp
  lldb/source/Plugins/SymbolFile/DWARF/SymbolFileDWARF.h
  lldb/test/Shell/SymbolFile/DWARF/x86/auto_return_symtab.s

Index: lldb/test/Shell/SymbolFile/DWARF/x86/auto_return_symtab.s
===
--- /dev/null
+++ lldb/test/Shell/SymbolFile/DWARF/x86/auto_return_symtab.s
@@ -0,0 +1,257 @@
+# This tests that lldb when using symbol table we are able to find the definition
+# for an auto return function.
+
+# RUN: llvm-mc -triple x86_64-apple-macosx10.15.0 %s -filetype=obj > %t.o
+# RUN: lldb-test symbols --dump-clang-ast %t.o | FileCheck %s
+
+# CHECK: CXXMethodDecl {{.*}} <>  f 'int ()'
+
+# This was compiled from the following code:
+#
+# struct A {
+# auto f();
+# };
+#
+# auto A::f() {
+# return 0;
+# }
+#
+# Compiled using:
+#
+#  -target x86_64-apple-macosx10.15.0
+#
+# and edited to remove some uneeded sections.
+
+	.section	__TEXT,__text,regular,pure_instructions
+	.globl	__ZN1A1fEv  ## -- Begin function _ZN1A1fEv
+	.p2align	4, 0x90
+__ZN1A1fEv: ## @_ZN1A1fEv
+Lfunc_begin0:
+	.cfi_startproc
+## %bb.0:   ## %entry
+	pushq	%rbp
+	.cfi_def_cfa_offset 16
+	.cfi_offset %rbp, -16
+	movq	%rsp, %rbp
+	.cfi_def_cfa_register %rbp
+	movq	%rdi, -8(%rbp)
+Ltmp0:
+	xorl	%eax, %eax
+	popq	%rbp
+	retq
+Ltmp1:
+Lfunc_end0:
+	.cfi_endproc
+## -- End function
+	.section	__DWARF,__debug_abbrev,regular,debug
+Lsection_abbrev:
+	.byte	1   ## Abbreviation Code
+	.byte	17  ## DW_TAG_compile_unit
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	37  ## DW_AT_producer
+	.byte	14  ## DW_FORM_strp
+	.byte	19  ## DW_AT_language
+	.byte	5   ## DW_FORM_data2
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\202|" ## DW_AT_LLVM_sysroot
+	.byte	14  ## DW_FORM_strp
+	.ascii	"\357\177"  ## DW_AT_APPLE_sdk
+	.byte	14  ## DW_FORM_strp
+	.byte	16  ## DW_AT_stmt_list
+	.byte	23  ## DW_FORM_sec_offset
+	.byte	27  ## DW_AT_comp_dir
+	.byte	14  ## DW_FORM_strp
+	.byte	17  ## DW_AT_low_pc
+	.byte	1   ## DW_FORM_addr
+	.byte	18  ## DW_AT_high_pc
+	.byte	6   ## DW_FORM_data4
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	2   ## Abbreviation Code
+	.byte	19  ## DW_TAG_structure_type
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	54  ## DW_AT_calling_convention
+	.byte	11  ## DW_FORM_data1
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	11  ## DW_AT_byte_size
+	.byte	11  ## DW_FORM_data1
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	0   ## EOM(1)
+	.byte	0   ## EOM(2)
+	.byte	3   ## Abbreviation Code
+	.byte	46  ## DW_TAG_subprogram
+	.byte	1   ## DW_CHILDREN_yes
+	.byte	110 ## DW_AT_linkage_name
+	.byte	14  ## DW_FORM_strp
+	.byte	3   ## DW_AT_name
+	.byte	14  ## DW_FORM_strp
+	.byte	58  ## DW_AT_decl_file
+	.byte	11  ## DW_FORM_data1
+	.byte	59  ## DW_AT_decl_line
+	.byte	11  ## DW_FORM_data1
+	.byte	73  ## DW_AT_type
+	.byte	19  ## DW_FORM_ref4
+	.byte	60  ## DW_AT_declaration
+	.byte	25  

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

@aprantl after your comments and discussion offline I changed my approach to do 
this lookup using the symbol table and it worked out.

The main issue with the first approach was that gcc would also have to be 
updated in order for them to change their approach to generating debug info as 
well and that felt a lot harder to justify.

This is also a simpler approach, it requires fewer changes.


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

https://reviews.llvm.org/D105564

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

[PATCH] D102107: [OpenMP] Codegen aggregate for outlined function captures

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D102107#2867670 , @josemonsalve2 
wrote:

> In D102107#2867417 , @ABataev wrote:
>
>> In D102107#2867382 , @jdoerfert 
>> wrote:
>>
>>> In D102107#2832740 , @ABataev 
>>> wrote:
>>>
 In D102107#2832286 , @jdoerfert 
 wrote:

> In D102107#2824581 , @ABataev 
> wrote:
>
>> In D102107#2823706 , 
>> @jdoerfert wrote:
>>
>>> In D102107#2821976 , @ABataev 
>>> wrote:
>>>
 We used this kind of codegen initially but later found out that it 
 causes a large overhead when gathering pointers into a record. What 
 about hybrid scheme where the first args are passed as arguments and 
 others (if any) are gathered into a record?
>>>
>>> I'm confused, maybe I misunderstand the problem. The parallel function 
>>> arguments need to go from the main thread to the workers somehow, I 
>>> don't see how this is done w/o a record. This patch makes it explicit 
>>> though.
>>
>> Pass it in a record for workers only? And use a hybrid scheme for all 
>> other parallel regions.
>
> I still do not follow. What does it mean for workers only? What is a 
> hybrid scheme? And, probably most importantly, how would we not 
> eventually put everything into a record anyway?

 On the host you don’t need to put everything into a record, especially for 
 small parallel regions. Pass some first args in registers and only the 
 remaining args gather into the record. For workers just pass all args in 
 the record.
>>>
>>> Could you please respond to my question so we make progress here. We 
>>> *always* have to pass things in a record, do you agree?
>>
>> On the GPU device, yes. And I'm absolutely fine with packing args for the 
>> GPU device. But the patch packs the args not only for the GPU devices but 
>> also for the host and other devices which may not require packing/unpacking. 
>> For such devices/host better to avoid packing/unpacking as it introduces 
>> overhead in many cases.
>
> Hi Alexey,
>
> Wouldn't you always need to pack to pass the arguments to the outlined 
> function? What is the benefit of avoiding packing the arguments in the 
> runtime call, if then you have to pack them for the outlined function?
>
> I would really appreciate an example, since I am just getting an 
> understanding of OpenMP in LLVM.
>
> Thanks!

Hi, generally speaking, no, you don't need to pack them. Initially, we 
packed/unpacked args, but then decided not to do it.
Here is an example:

  int a, b;
  #pragma omp parallel
   printf("%d %d\n", a, b);

What we generate currently is something like this:

  %a = alloca i32
  %b = alloca i32
  call __kmpc_fork_call(..., @outlined, %a, %b)
  ...
   internal @outlined(i32 *%a, i32 *%b) {
   printf();
   }

`__kmpc_fork_call` inside calls `@outlined` function with the passed args.

>>> If we pack the things eventually to pass it to the workers, why would we 
>>> not pack it right away and avoid complexity? Passing varargs, then packing 
>>> them later (with the same thread) into a record to give it to the workers 
>>> is arguably introducing cost. What is the benefit of a hybrid approach 
>>> given that it is (theoretically) more costly and arguably more complex?




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D102107

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

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

> Currently when we have a member function that has an auto return type and the 
> definition is out of line we generate two DWARF DIE.
> One for the declaration and a second one for the definition, the definition 
> holds the deduced type but does not contain a DW_AT_name nor
> a DW_AT_linkage_name so there was not way to look up the definition DIE.

Regarding the DWARF, the definition DIE should have a DW_AT_specification that 
points back to the declaration; is that missing here?
I'm not familiar with LLDB so it's likely I'm misunderstanding the problem.


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

https://reviews.llvm.org/D105564

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

[PATCH] D105564: Fix for DWARF parsing to better handle auto return type for member functions

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

In D105564#2867717 , @probinson wrote:

>> Currently when we have a member function that has an auto return type and 
>> the definition is out of line we generate two DWARF DIE.
>> One for the declaration and a second one for the definition, the definition 
>> holds the deduced type but does not contain a DW_AT_name nor
>> a DW_AT_linkage_name so there was not way to look up the definition DIE.
>
> Regarding the DWARF, the definition DIE should have a DW_AT_specification 
> that points back to the declaration; is that missing here?
> I'm not familiar with LLDB so it's likely I'm misunderstanding the problem.

Apologies, I should have updated the description as well, portions of that 
description are out of date with the new approach I just updated to.


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

https://reviews.llvm.org/D105564

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

[clang] 92dcb1d - [Clang] Introduce Swift async calling convention.

[Varun Gandhi via cfe-commits]

Author: Varun Gandhi
Date: 2021-07-09T11:50:10-07:00
New Revision: 92dcb1d2db8c4de48df0af806dca631523cd4169

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

LOG: [Clang] Introduce Swift async calling convention.

This change is intended as initial setup. The plan is to add
more semantic checks later. I plan to update the documentation
as more semantic checks are added (instead of documenting the
details up front). Most of the code closely mirrors that for
the Swift calling convention. Three places are marked as
[FIXME: swiftasynccc]; those will be addressed once the
corresponding convention is introduced in LLVM.

Reviewed By: rjmccall

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

Added: 
clang/test/CodeGen/swift-async-call-conv.c

Modified: 
clang/include/clang-c/Index.h
clang/include/clang/Basic/Attr.td
clang/include/clang/Basic/AttrDocs.td
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Basic/Features.def
clang/include/clang/Basic/Specifiers.h
clang/include/clang/CodeGen/SwiftCallingConv.h
clang/lib/AST/ExprCXX.cpp
clang/lib/AST/ItaniumMangle.cpp
clang/lib/AST/MicrosoftMangle.cpp
clang/lib/AST/Type.cpp
clang/lib/AST/TypePrinter.cpp
clang/lib/Basic/Targets/AArch64.cpp
clang/lib/Basic/Targets/ARM.cpp
clang/lib/Basic/Targets/PPC.h
clang/lib/Basic/Targets/SystemZ.h
clang/lib/Basic/Targets/WebAssembly.h
clang/lib/Basic/Targets/X86.h
clang/lib/CodeGen/CGCall.cpp
clang/lib/CodeGen/CGDebugInfo.cpp
clang/lib/CodeGen/CGStmt.cpp
clang/lib/Sema/SemaDeclAttr.cpp
clang/lib/Sema/SemaType.cpp
clang/test/CodeGen/64bit-swiftcall.c
clang/test/CodeGen/arm-swiftcall.c
clang/test/CodeGen/debug-info-cc.c
clang/test/CodeGen/swift-call-conv.c
clang/test/Sema/attr-c2x.c
clang/test/Sema/attr-swiftcall.c
clang/test/Sema/no_callconv.cpp
clang/test/SemaCXX/attr-swiftcall.cpp
clang/tools/libclang/CXType.cpp
llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
llvm/lib/Demangle/MicrosoftDemangle.cpp
llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
llvm/lib/Transforms/IPO/MergeFunctions.cpp
llvm/test/Demangle/ms-mangle.test

Removed: 




diff  --git a/clang/include/clang-c/Index.h b/clang/include/clang-c/Index.h
index c7d3b4e10622..26844d1c74f3 100644
--- a/clang/include/clang-c/Index.h
+++ b/clang/include/clang-c/Index.h
@@ -33,7 +33,7 @@
  * compatible, thus CINDEX_VERSION_MAJOR is expected to remain stable.
  */
 #define CINDEX_VERSION_MAJOR 0
-#define CINDEX_VERSION_MINOR 61
+#define CINDEX_VERSION_MINOR 62
 
 #define CINDEX_VERSION_ENCODE(major, minor) (((major)*1) + ((minor)*1))
 
@@ -3418,6 +3418,7 @@ enum CXCallingConv {
   CXCallingConv_PreserveMost = 14,
   CXCallingConv_PreserveAll = 15,
   CXCallingConv_AArch64VectorCall = 16,
+  CXCallingConv_SwiftAsync = 17,
 
   CXCallingConv_Invalid = 100,
   CXCallingConv_Unexposed = 200

diff  --git a/clang/include/clang/Basic/Attr.td 
b/clang/include/clang/Basic/Attr.td
index bfcf5f798d8f..e27efd404c21 100644
--- a/clang/include/clang/Basic/Attr.td
+++ b/clang/include/clang/Basic/Attr.td
@@ -2459,6 +2459,11 @@ def SwiftCall : DeclOrTypeAttr {
   let Documentation = [SwiftCallDocs];
 }
 
+def SwiftAsyncCall : DeclOrTypeAttr {
+  let Spellings = [Clang<"swiftasynccall">];
+  let Documentation = [SwiftAsyncCallDocs];
+}
+
 def SwiftContext : ParameterABIAttr {
   let Spellings = [Clang<"swift_context">];
   let Documentation = [SwiftContextDocs];

diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index bf51b05acaaa..0a665fee7686 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -4527,7 +4527,8 @@ def SwiftContextDocs : Documentation {
   let Category = DocCatVariable;
   let Content = [{
 The ``swift_context`` attribute marks a parameter of a ``swiftcall``
-function as having the special context-parameter ABI treatment.
+or ``swiftasynccall`` function as having the special context-parameter
+ABI treatment.
 
 This treatment generally passes the context value in a special register
 which is normally callee-preserved.
@@ -4540,14 +4541,39 @@ A context parameter must have pointer or reference type.
   }];
 }
 
+def SwiftAsyncCallDocs : Documentation {
+  let Category = DocCatVariable;
+  let Content = [{
+The ``swiftasynccall`` attribute indicates that a function is
+compatible with the low-level conventions of Swift async functions,
+provided it declares the right formal arguments.
+
+In most respects, this is similar to the ``swiftcall`` attribute, except for
+the following:
+- A parameter may be marked ``swift_async_context``, ``swift_context``
+  or ``swift_indirect_result`` (with the same rest

[PATCH] D95561: [Clang] Introduce Swift async calling convention.

[Varun Gandhi 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 rG92dcb1d2db8c: [Clang] Introduce Swift async calling 
convention. (authored by varungandhi-apple).

Changed prior to commit:
  https://reviews.llvm.org/D95561?vs=354073&id=357587#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D95561

Files:
  clang/include/clang-c/Index.h
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Basic/Features.def
  clang/include/clang/Basic/Specifiers.h
  clang/include/clang/CodeGen/SwiftCallingConv.h
  clang/lib/AST/ExprCXX.cpp
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/AST/MicrosoftMangle.cpp
  clang/lib/AST/Type.cpp
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Basic/Targets/AArch64.cpp
  clang/lib/Basic/Targets/ARM.cpp
  clang/lib/Basic/Targets/PPC.h
  clang/lib/Basic/Targets/SystemZ.h
  clang/lib/Basic/Targets/WebAssembly.h
  clang/lib/Basic/Targets/X86.h
  clang/lib/CodeGen/CGCall.cpp
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/64bit-swiftcall.c
  clang/test/CodeGen/arm-swiftcall.c
  clang/test/CodeGen/debug-info-cc.c
  clang/test/CodeGen/swift-async-call-conv.c
  clang/test/CodeGen/swift-call-conv.c
  clang/test/Sema/attr-c2x.c
  clang/test/Sema/attr-swiftcall.c
  clang/test/Sema/no_callconv.cpp
  clang/test/SemaCXX/attr-swiftcall.cpp
  clang/tools/libclang/CXType.cpp
  llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
  llvm/lib/Demangle/MicrosoftDemangle.cpp
  llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
  llvm/lib/Transforms/IPO/MergeFunctions.cpp
  llvm/test/Demangle/ms-mangle.test

Index: llvm/test/Demangle/ms-mangle.test
===
--- llvm/test/Demangle/ms-mangle.test
+++ llvm/test/Demangle/ms-mangle.test
@@ -341,6 +341,9 @@
 ?swift_func@@YSXXZ
 ; CHECK: void __attribute__((__swiftcall__)) swift_func(void)
 
+?swift_async_func@@YTXXZ
+; CHECK: void __attribute__((__swiftasynccall__)) swift_async_func(void)
+
 ??$fn_tmpl@$1?extern_c_func@@YAXXZ@@YAXXZ
 ; CHECK: void __cdecl fn_tmpl<&void __cdecl extern_c_func(void)>(void)
 
Index: llvm/lib/Transforms/IPO/MergeFunctions.cpp
===
--- llvm/lib/Transforms/IPO/MergeFunctions.cpp
+++ llvm/lib/Transforms/IPO/MergeFunctions.cpp
@@ -709,7 +709,10 @@
 
   CallInst *CI = Builder.CreateCall(F, Args);
   ReturnInst *RI = nullptr;
-  CI->setTailCall();
+  bool isSwiftTailCall = F->getCallingConv() == CallingConv::SwiftTail &&
+ G->getCallingConv() == CallingConv::SwiftTail;
+  CI->setTailCallKind(isSwiftTailCall ? llvm::CallInst::TCK_MustTail
+  : llvm::CallInst::TCK_Tail);
   CI->setCallingConv(F->getCallingConv());
   CI->setAttributes(F->getAttributes());
   if (H->getReturnType()->isVoidTy()) {
Index: llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
===
--- llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
+++ llvm/lib/Demangle/MicrosoftDemangleNodes.cpp
@@ -110,6 +110,9 @@
   case CallingConv::Swift:
 OS << "__attribute__((__swiftcall__)) ";
 break;
+  case CallingConv::SwiftAsync:
+OS << "__attribute__((__swiftasynccall__)) ";
+break;
   default:
 break;
   }
Index: llvm/lib/Demangle/MicrosoftDemangle.cpp
===
--- llvm/lib/Demangle/MicrosoftDemangle.cpp
+++ llvm/lib/Demangle/MicrosoftDemangle.cpp
@@ -1713,6 +1713,8 @@
 return CallingConv::Vectorcall;
   case 'S':
 return CallingConv::Swift;
+  case 'T':
+return CallingConv::SwiftAsync;
   }
 
   return CallingConv::None;
Index: llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
===
--- llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
+++ llvm/include/llvm/Demangle/MicrosoftDemangleNodes.h
@@ -67,7 +67,8 @@
   Eabi,
   Vectorcall,
   Regcall,
-  Swift, // Clang-only
+  Swift,  // Clang-only
+  SwiftAsync, // Clang-only
 };
 
 enum class ReferenceKind : uint8_t { None, LValueRef, RValueRef };
Index: clang/tools/libclang/CXType.cpp
===
--- clang/tools/libclang/CXType.cpp
+++ clang/tools/libclang/CXType.cpp
@@ -664,6 +664,7 @@
   TCALLINGCONV(AAPCS_VFP);
   TCALLINGCONV(IntelOclBicc);
   TCALLINGCONV(Swift);
+  TCALLINGCONV(SwiftAsync);
   TCALLINGCONV(PreserveMost);
   TCALLINGCONV(PreserveAll);
 case CC_SpirFunction: return CXCallingConv_Unexposed;
Index: clang/test/SemaCXX/attr-swiftcall.cpp
===

[clang] ff8b1b1 - Reapply [IR] Don't mark mustprogress as type attribute

[Nikita Popov via cfe-commits]

Author: Nikita Popov
Date: 2021-07-09T20:57:44+02:00
New Revision: ff8b1b1b9caef57046bda1ca36c95f0e03527c6e

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

LOG: Reapply [IR] Don't mark mustprogress as type attribute

Reapply with fixes for clang tests.

-

This is a simple enum attribute. Test changes are because enum
attributes are sorted before type attributes, so mustprogress is
now in a different position.

Added: 


Modified: 
clang/test/CodeGen/address-safety-attr-flavors.cpp
clang/test/CodeGen/address-safety-attr.cpp
clang/test/CodeGen/memtag-attr.cpp
clang/test/CodeGen/noduplicate-cxx11-test.cpp
clang/test/CodeGen/sanitize-thread-attr.cpp
clang/test/CodeGenCXX/attr.cpp
clang/test/CodeGenCXX/cxx11-exception-spec.cpp
clang/test/CodeGenCXX/cxx11-noreturn.cpp
clang/test/CodeGenCXX/derived-to-base.cpp
clang/test/CodeGenCXX/inline-hint.cpp
clang/test/CodeGenCXX/main-norecurse.cpp
clang/test/CodeGenCXX/microsoft-abi-array-cookies.cpp
clang/test/CodeGenCXX/no-exceptions.cpp
clang/test/CodeGenCXX/noinline-template.cpp
clang/test/CodeGenCXX/optnone-and-attributes.cpp
clang/test/CodeGenCXX/optnone-def-decl.cpp
clang/test/CodeGenCXX/reference-cast.cpp
clang/test/CodeGenCXX/threadsafe-statics.cpp
clang/test/CodeGenCXX/virtual-base-cast.cpp
clang/test/CodeGenObjC/objc-literal-tests.m
clang/test/CodeGenObjCXX/lambda-expressions.mm
clang/test/CodeGenOpenCL/convergent.cl

clang/test/utils/update_cc_test_checks/Inputs/check-attributes.cpp.funcattrs.expected
llvm/include/llvm/IR/Attributes.td
llvm/test/Analysis/TypeBasedAliasAnalysis/functionattrs.ll
llvm/test/CodeGen/AMDGPU/inline-attr.ll
llvm/test/Transforms/Attributor/willreturn.ll
llvm/test/Transforms/FunctionAttrs/atomic.ll
llvm/test/Transforms/FunctionAttrs/incompatible_fn_attrs.ll
llvm/test/Transforms/FunctionAttrs/nofree-attributor.ll
llvm/test/Transforms/FunctionAttrs/nofree.ll
llvm/test/Transforms/FunctionAttrs/nosync.ll
llvm/test/Transforms/FunctionAttrs/nounwind.ll
llvm/test/Transforms/FunctionAttrs/optnone.ll
llvm/test/Transforms/FunctionAttrs/willreturn-callsites.ll
llvm/test/Transforms/FunctionAttrs/willreturn.ll
llvm/test/Transforms/FunctionAttrs/writeonly.ll
llvm/test/Transforms/InferFunctionAttrs/annotate.ll
llvm/test/Transforms/InferFunctionAttrs/norecurse_debug.ll
llvm/test/Transforms/Inline/always-inline-attr.ll
llvm/test/Transforms/LICM/strlen.ll

Removed: 




diff  --git a/clang/test/CodeGen/address-safety-attr-flavors.cpp 
b/clang/test/CodeGen/address-safety-attr-flavors.cpp
index ac575ede0461..e6d17ed2da34 100644
--- a/clang/test/CodeGen/address-safety-attr-flavors.cpp
+++ b/clang/test/CodeGen/address-safety-attr-flavors.cpp
@@ -25,51 +25,51 @@
 // RUN:   FileCheck -check-prefix=CHECK-KHWASAN %s
 
 int HasSanitizeAddress() { return 1; }
-// CHECK-NOASAN: {{Function Attrs: noinline nounwind mustprogress$}}
-// CHECK-ASAN: Function Attrs: noinline nounwind sanitize_address mustprogress
-// CHECK-KASAN: Function Attrs: noinline nounwind sanitize_address mustprogress
-// CHECK-HWASAN: Function Attrs: noinline nounwind sanitize_hwaddress 
mustprogress
-// CHECK-KHWASAN: Function Attrs: noinline nounwind sanitize_hwaddress 
mustprogress
+// CHECK-NOASAN: {{Function Attrs: mustprogress noinline nounwind$}}
+// CHECK-ASAN: Function Attrs: mustprogress noinline nounwind sanitize_address
+// CHECK-KASAN: Function Attrs: mustprogress noinline nounwind sanitize_address
+// CHECK-HWASAN: Function Attrs: mustprogress noinline nounwind 
sanitize_hwaddress
+// CHECK-KHWASAN: Function Attrs: mustprogress noinline nounwind 
sanitize_hwaddress
 
 __attribute__((no_sanitize("address"))) int NoSanitizeQuoteAddress() {
   return 0;
 }
-// CHECK-NOASAN: {{Function Attrs: noinline nounwind mustprogress$}}
-// CHECK-ASAN: {{Function Attrs: noinline nounwind mustprogress$}}
-// CHECK-KASAN: {{Function Attrs: noinline nounwind mustprogress$}}
-// CHECK-HWASAN: {{Function Attrs: noinline nounwind sanitize_hwaddress 
mustprogress$}}
-// CHECK-KHWASAN: {{Function Attrs: noinline nounwind sanitize_hwaddress 
mustprogress$}}
+// CHECK-NOASAN: {{Function Attrs: mustprogress noinline nounwind$}}
+// CHECK-ASAN: {{Function Attrs: mustprogress noinline nounwind$}}
+// CHECK-KASAN: {{Function Attrs: mustprogress noinline nounwind$}}
+// CHECK-HWASAN: {{Function Attrs: mustprogress noinline nounwind 
sanitize_hwaddress$}}
+// CHECK-KHWASAN: {{Function Attrs: mustprogress noinline nounwind 
sanitize_hwaddress$}}
 
 __attribute__((no_sanitize_address)) int NoSanitizeAddress() { return 0; }
-// CHECK-NOASAN: {{Function Attrs: noinline nounwind mustprogress$}}
-// CHECK-ASAN: {{Function Attr

[PATCH] D105681: [clangd] Add platform triple (host & target) to version info

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

thanks! only printing the target when they differ makes a lot of sense! LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105681

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

[clang] 768e3af - PR51034: Debug Info: Remove 'prototyped' from K&R function declarations

[David Blaikie via cfe-commits]

Author: David Blaikie
Date: 2021-07-09T12:07:36-07:00
New Revision: 768e3af6345a532d383205049679aaaccca26628

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

LOG: PR51034: Debug Info: Remove 'prototyped' from K&R function declarations

Regression caused by 6c9559b67b91966bfeff9e17808a3e84a92e64a0.

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.cpp
clang/test/CodeGen/overloadable-debug.c

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.cpp 
b/clang/lib/CodeGen/CGDebugInfo.cpp
index adace59c9ee6..e4c3af07e664 100644
--- a/clang/lib/CodeGen/CGDebugInfo.cpp
+++ b/clang/lib/CodeGen/CGDebugInfo.cpp
@@ -3549,10 +3549,10 @@ void CGDebugInfo::collectFunctionDeclProps(GlobalDecl 
GD, llvm::DIFile *Unit,
   const auto *FD = cast(GD.getCanonicalDecl().getDecl());
   Name = getFunctionName(FD);
   // Use mangled name as linkage name for C/C++ functions.
-  if (FD->getType()->getAs()) {
+  if (FD->getType()->getAs())
 LinkageName = CGM.getMangledName(GD);
+  if (FD->hasPrototype())
 Flags |= llvm::DINode::FlagPrototyped;
-  }
   // No need to replicate the linkage name if it isn't 
diff erent from the
   // subprogram name, no need to have it at all unless coverage is enabled or
   // debug is set to more than just line tables or extra debug info is needed.

diff  --git a/clang/test/CodeGen/overloadable-debug.c 
b/clang/test/CodeGen/overloadable-debug.c
index 6eda31884496..c742f74f77ba 100644
--- a/clang/test/CodeGen/overloadable-debug.c
+++ b/clang/test/CodeGen/overloadable-debug.c
@@ -2,5 +2,8 @@
 
 __attribute__((overloadable)) void f1(a) int a; {
 }
+void f2(a) int a; {
+}
 
 // CHECK: !DISubprogram(name: "f1", linkageName: "_Z2f1i"
+// CHECK: !DISubprogram(name: "f2", scope: {{.*}}, spFlags: DISPFlagDefinition,



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

[PATCH] D104904: [OpenMP][AMDGCN] Initial math headers support

[Ethan Stewart via Phabricator via cfe-commits]

estewart08 added inline comments.



Comment at: clang/test/Headers/Inputs/include/cstdlib:29
 float fabs(float __x) { return __builtin_fabs(__x); }
+#endif
 

JonChesterfield wrote:
> jdoerfert wrote:
> > That seems to be fundamentally broken then, but let's see, maybe it will 
> > somehow work anyway.
> I thought fabs was in math, not stdlib. Not sure what this file is doing but 
> the functions above are inline and fabs isn't
I am afraid this is just a workaround to get openmp_device_math_isnan.cpp to 
pass for AMDGCN. This stems from not having an #ifndef OPENMP_AMDGCN in 
__clang_hip_cmath.h where 'using ::fabs' is present. Currently, OPENMP_AMDGCN 
uses all of the overloaded functions created by the HIP macros where NVPTX does 
not use the CUDA overloads. This may be a new topic of discussion.

https://github.com/llvm/llvm-project/blob/main/clang/lib/Headers/__clang_cuda_cmath.h#L191

By using this ifndef, it seems NVPTX looses quite a few overloaded functions. 
Are these meant to eventually be present in openmp_wrappers/cmath? Not sure 
what issues @jdoerfert ran into with D75788.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D104904

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

[PATCH] D105553: [analyzer][NFC] Split the main logic of NoStoreFuncVisitor to an abstract NoStateChangeVisitor class

[Artem Dergachev via Phabricator via cfe-commits]

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

This looks perfectly sensible to me.

In the mailing list I seem to have made a mistake about how this works: we 
don't explicitly scan the AST for potential statements that could cause a state 
change (eg., assignment operators in case of `NoStore`) but we only check if 
the region was explicitly made accessible. This makes things a lot easier (we 
don't have to build an auxiliary AST-based analysis) and I'm not sure if this 
other heuristic that I thought we have would actually make things significantly 
better. I guess it makes sense to keep in mind that we might want to ultimately 
plug it in but I don't immediately see what'd stop us.




Comment at: 
clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporterVisitors.h:629
+
+/// Put a diagnostic on return statement of all inlined functions for which 
some
+/// property remained unchanged.

Or on the `}` if there's no return statement.



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:399
+/// to get the correct parameter name.
+static ArrayRef getCallParameters(const CallEvent &Call) {
+  // Use runtime definition, if available.

While we're at it, can you take a look if the recently introduced 
`CallEvent::parameters()` is good enough for this?



Comment at: clang/lib/StaticAnalyzer/Core/BugReporterVisitors.cpp:532-533
 
-  /// Check and lazily calculate whether the region of interest is
-  /// modified in the stack frame to which \p N belongs.
-  /// The calculation is cached in FramesModifyingRegion.
-  bool isRegionOfInterestModifiedInFrame(const ExplodedNode *N) {
-const LocationContext *Ctx = N->getLocationContext();
-const StackFrameContext *SCtx = Ctx->getStackFrame();
-if (!FramesModifyingCalculated.count(SCtx))
-  findModifyingFrames(N);
-return FramesModifyingRegion.count(SCtx);
-  }
+  // Region of interest corresponds to an IVar, exiting a method
+  // which could have written into that IVar, but did not.
+  virtual PathDiagnosticPieceRef

I guess this comment could be copied to the other two virtual methods?


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

https://reviews.llvm.org/D105553

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

[PATCH] D105637: [clang][Analyzer] Add symbol uninterestingness to bug report.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D105637#2867602 , @NoQ wrote:

> You could define your own diagnostic consumer in the unittest and intercept 
> all the notes.

(you can find an example of this in D94476 ) 
(I can't believe I haven't landed these patches yet)

In D105637#2867602 , @NoQ wrote:

> But at this point i'd rather turn this into a LIT test by turning your 
> checker mock into a `debug.ExprInspection` item

(dunno how reusable it'll be, maybe unittest is indeed the way to go)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105637

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

[PATCH] D105726: Added fsanitize-address-instrument-via-callback, which controls if address sanitizer will always use a callback.

[Kirill Stoimenov via Phabricator via cfe-commits]

kstoimenov created this revision.
kstoimenov added reviewers: kda, vitalybuka.
Herald added a subscriber: dang.
kstoimenov requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Summary This option can be used to reduce the size of the binary. The trade-off 
in this case would be the run-time performance.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D105726

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/SanitizerArgs.h
  clang/lib/Driver/SanitizerArgs.cpp
  clang/test/Driver/fsanitize.c


Index: clang/test/Driver/fsanitize.c
===
--- clang/test/Driver/fsanitize.c
+++ clang/test/Driver/fsanitize.c
@@ -247,6 +247,13 @@
 // CHECK-ASAN-GLOBALS: -cc1{{.*}}-fsanitize-address-globals-dead-stripping
 // CHECK-NO-ASAN-GLOBALS-NOT: 
-cc1{{.*}}-fsanitize-address-globals-dead-stripping
 
+// RUN: %clang -target x86_64-linux-gnu 
-fsanitize-address-instrument-via-callback %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-CALLBACK-WARN
+// CHECK-ASAN-CALLBACK-WARN: warning: argument unused during compilation: 
'-fsanitize-address-instrument-via-callback'
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-address-instrument-via-callback %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-CALLBACK-OK
+// CHECK-ASAN-CALLBACK-OK: "-mllvm" 
"-asan-instrumentation-with-call-threshold=0"
+// RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fnosanitize-address-instrument-via-callback %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-NOCALLBACK
+// CHECK-ASAN-NOCALLBACK-NOT: "-mllvm" 
"-asan-instrumentation-with-call-threshold=0"
+
 // RUN: %clang -target x86_64-linux-gnu -fsanitize=address 
-fsanitize-address-use-odr-indicator %s -### 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-ODR-INDICATOR
 // RUN: %clang_cl --target=x86_64-windows -fsanitize=address 
-fsanitize-address-use-odr-indicator -### -- %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-ASAN-ODR-INDICATOR
 // CHECK-ASAN-ODR-INDICATOR: -cc1{{.*}}-fsanitize-address-use-odr-indicator
Index: clang/lib/Driver/SanitizerArgs.cpp
===
--- clang/lib/Driver/SanitizerArgs.cpp
+++ clang/lib/Driver/SanitizerArgs.cpp
@@ -805,6 +805,11 @@
 options::OPT_fno_sanitize_address_poison_custom_array_cookie,
 AsanPoisonCustomArrayCookie);
 
+AsanInstrumentViaCallback =
+Args.hasFlag(options::OPT_fsanitize_address_instrument_via_callback,
+ options::OPT_fnosanitize_address_instrument_via_callback,
+ AsanInstrumentViaCallback);
+
 // As a workaround for a bug in gold 2.26 and earlier, dead stripping of
 // globals in ASan is disabled by default on ELF targets.
 // See https://sourceware.org/bugzilla/show_bug.cgi?id=19002
@@ -1118,6 +1123,11 @@
 CmdArgs.push_back("-asan-detect-invalid-pointer-sub");
   }
 
+  if (AsanInstrumentViaCallback) {
+CmdArgs.push_back("-mllvm");
+CmdArgs.push_back("-asan-instrumentation-with-call-threshold=0");
+  }
+
   // Only pass the option to the frontend if the user requested,
   // otherwise the frontend will just use the codegen default.
   if (AsanDtorKind != llvm::AsanDtorKind::Invalid) {
Index: clang/include/clang/Driver/SanitizerArgs.h
===
--- clang/include/clang/Driver/SanitizerArgs.h
+++ clang/include/clang/Driver/SanitizerArgs.h
@@ -44,6 +44,7 @@
   bool AsanUseOdrIndicator = false;
   bool AsanInvalidPointerCmp = false;
   bool AsanInvalidPointerSub = false;
+  bool AsanInstrumentViaCallback = false;
   llvm::AsanDtorKind AsanDtorKind = llvm::AsanDtorKind::Invalid;
   std::string HwasanAbi;
   bool LinkRuntimes = true;
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1558,6 +1558,12 @@
 Group,
 Flags<[CoreOption, NoXarchOption]>,
 HelpText<"Disable origins tracking in 
MemorySanitizer">;
+def fsanitize_address_instrument_via_callback : Flag<["-"], 
"fsanitize-address-instrument-via-callback">,
+Group,
+HelpText<"Always use callback 
for the address sanitizer">;
+def fnosanitize_address_instrument_via_callback : Flag<["-"], 
"fnosanitize-address-instrument-via-callback">,
+  Group,
+  HelpText<"Use default logic 
for code inlining for the address sanitizer">;
 def fsanitize_hwaddress_experimental_aliasing
   : Flag<["-"], "fsanitize-hwaddress-experimental-aliasing">,
 Group,

[PATCH] D105562: [OPENMP]Fix overlapped mapping for dereferenced pointer members.

[Abhinav Gaba via Phabricator via cfe-commits]

abhinavgaba accepted this revision.
abhinavgaba added a comment.

Thanks for the fix, Alexey.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105562

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

[PATCH] D105501: [PowerPC] Power ISA features for Semachecking

[Quinn Pham via Phabricator via cfe-commits]

quinnp updated this revision to Diff 357604.
quinnp added a comment.

Feature is now working correctly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105501

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Basic/Targets/PPC.cpp
  clang/lib/Basic/Targets/PPC.h
  llvm/lib/Target/PowerPC/PPC.td
  llvm/lib/Target/PowerPC/PPCInstrInfo.td
  llvm/lib/Target/PowerPC/PPCSubtarget.cpp
  llvm/lib/Target/PowerPC/PPCSubtarget.h

Index: llvm/lib/Target/PowerPC/PPCSubtarget.h
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.h
+++ llvm/lib/Target/PowerPC/PPCSubtarget.h
@@ -146,6 +146,7 @@
   bool HasStoreFusion;
   bool HasAddiLoadFusion;
   bool HasAddisLoadFusion;
+  bool IsISA2_07;
   bool IsISA3_0;
   bool IsISA3_1;
   bool UseLongCalls;
@@ -319,6 +320,7 @@
 
   bool hasHTM() const { return HasHTM; }
   bool hasFloat128() const { return HasFloat128; }
+  bool isISA2_07() const { return IsISA2_07; }
   bool isISA3_0() const { return IsISA3_0; }
   bool isISA3_1() const { return IsISA3_1; }
   bool useLongCalls() const { return UseLongCalls; }
Index: llvm/lib/Target/PowerPC/PPCSubtarget.cpp
===
--- llvm/lib/Target/PowerPC/PPCSubtarget.cpp
+++ llvm/lib/Target/PowerPC/PPCSubtarget.cpp
@@ -126,6 +126,7 @@
   HasStoreFusion = false;
   HasAddiLoadFusion = false;
   HasAddisLoadFusion = false;
+  IsISA2_07 = false;
   IsISA3_0 = false;
   IsISA3_1 = false;
   UseLongCalls = false;
Index: llvm/lib/Target/PowerPC/PPCInstrInfo.td
===
--- llvm/lib/Target/PowerPC/PPCInstrInfo.td
+++ llvm/lib/Target/PowerPC/PPCInstrInfo.td
@@ -1176,6 +1176,7 @@
 : Predicate<"!Subtarget->getTargetMachine().Options.NoNaNsFPMath">;
 def HasBPERMD : Predicate<"Subtarget->hasBPERMD()">;
 def HasExtDiv : Predicate<"Subtarget->hasExtDiv()">;
+def IsISA2_07 : Predicate<"Subtarget->isISA2_07()">;
 def IsISA3_0 : Predicate<"Subtarget->isISA3_0()">;
 def HasFPU : Predicate<"Subtarget->hasFPU()">;
 def PCRelativeMemops : Predicate<"Subtarget->hasPCRelativeMemops()">;
Index: llvm/lib/Target/PowerPC/PPC.td
===
--- llvm/lib/Target/PowerPC/PPC.td
+++ llvm/lib/Target/PowerPC/PPC.td
@@ -210,9 +210,13 @@
 def DeprecatedDST: SubtargetFeature<"", "DeprecatedDST", "true",
   "Treat vector data stream cache control instructions as deprecated">;
 
+def FeatureISA2_07 : SubtargetFeature<"isa-v207-instructions", "IsISA2_07",
+  "true",
+  "Enable instructions in ISA 2.07.">;
 def FeatureISA3_0 : SubtargetFeature<"isa-v30-instructions", "IsISA3_0",
  "true",
- "Enable instructions in ISA 3.0.">;
+ "Enable instructions in ISA 3.0.",
+ [FeatureISA2_07]>;
 def FeatureISA3_1 : SubtargetFeature<"isa-v31-instructions", "IsISA3_1",
  "true",
  "Enable instructions in ISA 3.1.",
Index: clang/lib/Basic/Targets/PPC.h
===
--- clang/lib/Basic/Targets/PPC.h
+++ clang/lib/Basic/Targets/PPC.h
@@ -74,6 +74,9 @@
   bool HasP10Vector = false;
   bool HasPCRelativeMemops = false;
   bool HasPrefixInstrs = false;
+  bool IsISA2_07 = false;
+  bool IsISA3_0 = false;
+  bool IsISA3_1 = false;
 
 protected:
   std::string ABI;
Index: clang/lib/Basic/Targets/PPC.cpp
===
--- clang/lib/Basic/Targets/PPC.cpp
+++ clang/lib/Basic/Targets/PPC.cpp
@@ -73,6 +73,12 @@
   HasROPProtect = true;
 } else if (Feature == "+privileged") {
   HasPrivileged = true;
+} else if (Feature == "+isa-v207-instructions") {
+  IsISA2_07 = true;
+} else if (Feature == "+isa-v30-instructions") {
+  IsISA3_0 = true;
+} else if (Feature == "+isa-v31-instructions") {
+  IsISA3_1 = true;
 }
 // TODO: Finish this list and add an assert that we've handled them
 // all.
@@ -378,6 +384,15 @@
 .Case("e500", true)
 .Default(false);
 
+  Features["isa-v207-instructions"] = llvm::StringSwitch(CPU)
+  .Case("ppc64le", true)
+  .Case("pwr9", true)
+  .Case("pwr8", true)
+  .Default(false);
+
+  Features["isa-v30-instructions"] =
+  llvm::StringSwitch(CPU).Case("pwr9", true).Default(false);
+
   // Power10 includes all the same features as Power9 plus any features specific
   // to the Power10 core.
   if (CPU == "

[clang] ab8989a - [OPENMP]Fix overlapped mapping for dereferenced pointer members.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2021-07-09T12:51:26-07:00
New Revision: ab8989ab8710c693e83edbccf221746c897c835f

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

LOG: [OPENMP]Fix overlapped mapping for dereferenced pointer members.

If the base is used in a map clause and later we have a memberexpr with
this base, and the member is a pointer, and this pointer is dereferenced
anyhow (subscript, array section, dereference, etc.), such components
should be considered as overlapped, otherwise it may lead to incorrect
size computations, since we try to map a pointee as a part of the whole
struct, which is not true for the pointer members.

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

Added: 
openmp/libomptarget/test/mapping/target_pointers_members_map.cpp

Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/test/OpenMP/target_map_codegen_29.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index c2ef95cb1d28..8b0462988345 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -8997,11 +8997,17 @@ class MappableExprsHandler {
   // If one component is a pointer and another one is a kind of
   // dereference of this pointer (array subscript, section, 
dereference,
   // etc.), it is not an overlapping.
+  // Same, if one component is a base and another component is a
+  // dereferenced pointer memberexpr with the same base.
   if (!isa(It->getAssociatedExpression()) ||
-  std::prev(It)
-  ->getAssociatedExpression()
-  ->getType()
-  ->isPointerType())
+  (std::prev(It)->getAssociatedDeclaration() &&
+   std::prev(It)
+   ->getAssociatedDeclaration()
+   ->getType()
+   ->isPointerType()) ||
+  (It->getAssociatedDeclaration() &&
+   It->getAssociatedDeclaration()->getType()->isPointerType() &&
+   std::next(It) != CE && std::next(It) != SE))
 continue;
   const MapData &BaseData = CI == CE ? L : L1;
   OMPClauseMappableExprCommon::MappableExprComponentListRef SubData =
@@ -9061,7 +9067,7 @@ class MappableExprsHandler {
 const auto *FD2 = cast(SI->getAssociatedDeclaration());
 if (FD1->getParent() == FD2->getParent())
   return FD1->getFieldIndex() < FD2->getFieldIndex();
-const auto It =
+const auto *It =
 llvm::find_if(Layout, [FD1, FD2](const FieldDecl *FD) {
   return FD == FD1 || FD == FD2;
 });

diff  --git a/clang/test/OpenMP/target_map_codegen_29.cpp 
b/clang/test/OpenMP/target_map_codegen_29.cpp
index 360a44812197..2be0e2534d6d 100644
--- a/clang/test/OpenMP/target_map_codegen_29.cpp
+++ b/clang/test/OpenMP/target_map_codegen_29.cpp
@@ -38,9 +38,9 @@
 
 // CK30-LABEL: 
@.__omp_offloading_{{.*}}map_with_deep_copy{{.*}}_l{{[0-9]+}}.region_id = weak 
constant i8 0
 // The first element: 0x20 - OMP_MAP_TARGET_PARAM
-// 2-4: 0x10003 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_TO | OMP_MAP_FROM - 
copies all the data in structs excluding deep-copied elements (from &s to 
&s.ptrBase1, from &s.ptr to &s.ptr1, from &s.ptr1 to end of s).
-// 5-6: 0x10013 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_PTR_AND_OBJ | 
OMP_MAP_TO | OMP_MAP_FROM - deep copy of the pointers + pointee.
-// CK30: [[MTYPE00:@.+]] = private {{.*}}constant [6 x i64] [i64 32, i64 
281474976710659, i64 281474976710659, i64 281474976710659, i64 281474976710675, 
i64 281474976710675]
+// 2: 0x10003 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_TO | OMP_MAP_FROM - 
copies all the data in structs excluding deep-copied elements (from &s to end 
of s).
+// 3-4: 0x10013 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_PTR_AND_OBJ | 
OMP_MAP_TO | OMP_MAP_FROM - deep copy of the pointers + pointee.
+// CK30: [[MTYPE00:@.+]] = private {{.*}}constant [4 x i64] [i64 32, i64 
281474976710659, i64 281474976710675, i64 281474976710675]
 
 typedef struct {
   int *ptrBase;
@@ -55,18 +55,18 @@ typedef struct StructWithPtrTag : public Base {
   int *ptr1;
 } StructWithPtr;
 
-// CK30-DAG: call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}}, i64 -1, 
i8* @.__omp_offloading_{{.*}}map_with_deep_copy{{.*}}_l{{[0-9]+}}.region_id, 
i32 6, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], i64* [[GEPS:%.+]], i64* 
getelementptr inbounds ([6 x i64], [6 x i64]* [[MTYPE00]], i32 0, i32 0), i8** 
null, i8** null)
-// CK30-DAG: [[GEPS]] = getelementptr inbounds [6 x i{{64|32}}], [6 x i64]* 
[[SIZES:%.+]], i32 0, i32 0
-// CK30-DAG: [[GEPP]] = getelementptr inbounds [6 x i8*], [6 x i8*]* 
[[PTRS:%.+]], i32 0, i32

[PATCH] D105562: [OPENMP]Fix overlapped mapping for dereferenced pointer members.

[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 rGab8989ab8710: [OPENMP]Fix overlapped mapping for 
dereferenced pointer members. (authored by ABataev).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105562

Files:
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/test/OpenMP/target_map_codegen_29.cpp
  openmp/libomptarget/test/mapping/target_pointers_members_map.cpp

Index: openmp/libomptarget/test/mapping/target_pointers_members_map.cpp
===
--- /dev/null
+++ openmp/libomptarget/test/mapping/target_pointers_members_map.cpp
@@ -0,0 +1,55 @@
+// RUN: %libomptarget-compilexx-run-and-check-aarch64-unknown-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-powerpc64le-ibm-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-x86_64-pc-linux-gnu
+// RUN: %libomptarget-compilexx-run-and-check-nvptx64-nvidia-cuda
+
+#include 
+#include 
+
+typedef struct {
+  short *a;
+  long d1, d2;
+} DV_A;
+
+typedef struct {
+  DV_A b;
+  long d3;
+} C;
+
+typedef struct {
+  C *c;
+  long d4, d5;
+} DV_B;
+
+int main() {
+
+  short arr1[10] = {10, 11, 12, 13, 14, 15, 16, 17, 18, 19};
+  short arr2[10] = {20, 31, 22, 23, 24, 25, 26, 27, 28, 29};
+
+  C c1[2];
+  c1[0].b.a = (short *)arr1;
+  c1[1].b.a = (short *)arr2;
+  c1[0].b.d1 = 111;
+
+  DV_B dvb1;
+  dvb1.c = (C *)&c1;
+
+  // CHECK: 10 111
+  printf("%d %ld %p %p %p %p\n", dvb1.c[0].b.a[0], dvb1.c[0].b.d1, &dvb1,
+ &dvb1.c[0], &dvb1.c[0].b, &dvb1.c[0].b.a[0]);
+#pragma omp target map(to  \
+   : dvb1, dvb1.c [0:2])   \
+map(tofrom \
+: dvb1.c[0].b.a [0:10], dvb1.c[1].b.a [0:10])
+  {
+// CHECK: 10 111
+printf("%d %ld %p %p %p %p\n", dvb1.c[0].b.a[0], dvb1.c[0].b.d1, &dvb1,
+   &dvb1.c[0], &dvb1.c[0].b, &dvb1.c[0].b.a[0]);
+dvb1.c[0].b.a[0] = 333;
+dvb1.c[0].b.d1 = 444;
+  }
+  // CHECK: 333 111
+  printf("%d %ld %p %p %p %p\n", dvb1.c[0].b.a[0], dvb1.c[0].b.d1, &dvb1,
+ &dvb1.c[0], &dvb1.c[0].b, &dvb1.c[0].b.a[0]);
+}
Index: clang/test/OpenMP/target_map_codegen_29.cpp
===
--- clang/test/OpenMP/target_map_codegen_29.cpp
+++ clang/test/OpenMP/target_map_codegen_29.cpp
@@ -38,9 +38,9 @@
 
 // CK30-LABEL: @.__omp_offloading_{{.*}}map_with_deep_copy{{.*}}_l{{[0-9]+}}.region_id = weak constant i8 0
 // The first element: 0x20 - OMP_MAP_TARGET_PARAM
-// 2-4: 0x10003 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_TO | OMP_MAP_FROM - copies all the data in structs excluding deep-copied elements (from &s to &s.ptrBase1, from &s.ptr to &s.ptr1, from &s.ptr1 to end of s).
-// 5-6: 0x10013 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_PTR_AND_OBJ | OMP_MAP_TO | OMP_MAP_FROM - deep copy of the pointers + pointee.
-// CK30: [[MTYPE00:@.+]] = private {{.*}}constant [6 x i64] [i64 32, i64 281474976710659, i64 281474976710659, i64 281474976710659, i64 281474976710675, i64 281474976710675]
+// 2: 0x10003 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_TO | OMP_MAP_FROM - copies all the data in structs excluding deep-copied elements (from &s to end of s).
+// 3-4: 0x10013 - OMP_MAP_MEMBER_OF(0) | OMP_MAP_PTR_AND_OBJ | OMP_MAP_TO | OMP_MAP_FROM - deep copy of the pointers + pointee.
+// CK30: [[MTYPE00:@.+]] = private {{.*}}constant [4 x i64] [i64 32, i64 281474976710659, i64 281474976710675, i64 281474976710675]
 
 typedef struct {
   int *ptrBase;
@@ -55,18 +55,18 @@
   int *ptr1;
 } StructWithPtr;
 
-// CK30-DAG: call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}}, i64 -1, i8* @.__omp_offloading_{{.*}}map_with_deep_copy{{.*}}_l{{[0-9]+}}.region_id, i32 6, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], i64* [[GEPS:%.+]], i64* getelementptr inbounds ([6 x i64], [6 x i64]* [[MTYPE00]], i32 0, i32 0), i8** null, i8** null)
-// CK30-DAG: [[GEPS]] = getelementptr inbounds [6 x i{{64|32}}], [6 x i64]* [[SIZES:%.+]], i32 0, i32 0
-// CK30-DAG: [[GEPP]] = getelementptr inbounds [6 x i8*], [6 x i8*]* [[PTRS:%.+]], i32 0, i32 0
-// CK30-DAG: [[GEPBP]] = getelementptr inbounds [6 x i8*], [6 x i8*]* [[BASES:%.+]], i32 0, i32 0
+// CK30-DAG: call i32 @__tgt_target_mapper(%struct.ident_t* @{{.+}}, i64 -1, i8* @.__omp_offloading_{{.*}}map_with_deep_copy{{.*}}_l{{[0-9]+}}.region_id, i32 4, i8** [[GEPBP:%.+]], i8** [[GEPP:%.+]], i64* [[GEPS:%.+]], i64* getelementptr inbounds ([4 x i64], [4 x i64]* [[MTYPE00]], i32 0, i32 0), i8** null, i8** null)
+// CK30-DAG: [[GEPS]] = getelementptr inbounds [4 x i{{64|32}}], [4 x i64]* [[SIZES:%.+]], i32 0, i32 0
+// CK30-DAG: [[GEPP]] = getelementptr inbounds [4 x i8*], [4 x i8*]*

[PATCH] D69764: [clang-format] Add East/West Const fixer capability

[Stephen Kelly via Phabricator via cfe-commits]

steveire added a comment.

In D69764#2867109 , @MyDeveloperDay 
wrote:

> In D69764#2863648 , @owenpan wrote:
>
>> Has this been tested against a large code base? It also needs an unqualified 
>> LGTM before it can be merged.
>
> D105701: [clang-format] test revision (NOT FOR COMMIT) to demonstrate 
> east/west const fixer capability  
> demonstrates transforming clang-format itself to east const.
>
> Actually transformation of the whole of the clang subfolder is actually 
> holding up pretty well. I'm not seeing an violations (not sure if I 
> transformed all the files) but certainly so much so that creating a review 
> that covered all of it was way too big.
>
> Testing on a large code base can be hard especially one as large as LLVM 
> where its not currently fully clang-formatted in the first place.
>
> Of course the lit tests get mangled as the test code gets swapped but the 
> //CHECK-FIXES doesn't
>
> Like I mentioned before, by gut feeling is that this option is MOST useful in 
> preventing violation to your current style from creeping in than going to the 
> extreme of transforming a whole project from east to west or vice versa.

FYI - I also tested this on a large codebase at work. It is now used to keep 
the current style as you describe.

Should any known failure modes be documented?


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

https://reviews.llvm.org/D69764

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