[clang-tools-extra] 3219783 - [clang][clang-tools-extra] LLVM_NODISCARD => [[nodiscard]]. NFC

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2022-08-09T07:11:18Z
New Revision: 32197830ef5990d31b5582c9d497b2a96a1a381f

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

LOG: [clang][clang-tools-extra] LLVM_NODISCARD => [[nodiscard]]. NFC

Added: 


Modified: 
clang-tools-extra/clang-tidy/ClangTidyOptions.h
clang-tools-extra/clangd/ClangdServer.cpp
clang-tools-extra/clangd/ClangdServer.h
clang-tools-extra/clangd/index/Background.h
clang-tools-extra/clangd/index/Serialization.cpp
clang-tools-extra/clangd/support/Context.h
clang-tools-extra/clangd/support/Function.h
clang-tools-extra/clangd/support/Threading.h
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/ASTImporter.h
clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
clang/include/clang/StaticAnalyzer/Checkers/Taint.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/CallDescription.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ExprEngine.h
clang/include/clang/StaticAnalyzer/Core/PathSensitive/ProgramState.h
clang/include/clang/Tooling/Core/Replacement.h
clang/include/clang/Tooling/Syntax/Tokens.h
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/ExprConstant.cpp
clang/lib/CodeGen/CGCall.cpp
clang/lib/Format/FormatToken.h
clang/lib/Lex/DependencyDirectivesScanner.cpp
clang/lib/StaticAnalyzer/Checkers/MallocChecker.cpp
clang/lib/StaticAnalyzer/Core/ProgramState.cpp
clang/lib/StaticAnalyzer/Core/RangeConstraintManager.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/ClangTidyOptions.h 
b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
index 3f09d39df13d..20e54215ac8a 100644
--- a/clang-tools-extra/clang-tidy/ClangTidyOptions.h
+++ b/clang-tools-extra/clang-tidy/ClangTidyOptions.h
@@ -63,8 +63,8 @@ struct ClangTidyOptions {
   /// Creates a new \c ClangTidyOptions instance combined from all fields
   /// of this instance overridden by the fields of \p Other that have a value.
   /// \p Order specifies precedence of \p Other option.
-  LLVM_NODISCARD ClangTidyOptions merge(const ClangTidyOptions &Other,
-unsigned Order) const;
+  [[nodiscard]] ClangTidyOptions merge(const ClangTidyOptions &Other,
+   unsigned Order) const;
 
   /// Checks filter.
   llvm::Optional Checks;

diff  --git a/clang-tools-extra/clangd/ClangdServer.cpp 
b/clang-tools-extra/clangd/ClangdServer.cpp
index 5ab6cb7ef6af..eccb2cd56def 100644
--- a/clang-tools-extra/clangd/ClangdServer.cpp
+++ b/clang-tools-extra/clangd/ClangdServer.cpp
@@ -1000,7 +1000,7 @@ llvm::StringMap 
ClangdServer::fileStats() const {
   return WorkScheduler->fileStats();
 }
 
-LLVM_NODISCARD bool
+[[nodiscard]] bool
 ClangdServer::blockUntilIdleForTest(llvm::Optional TimeoutSeconds) {
   // Order is important here: we don't want to block on A and then B,
   // if B might schedule work on A.

diff  --git a/clang-tools-extra/clangd/ClangdServer.h 
b/clang-tools-extra/clangd/ClangdServer.h
index dacb7b74af99..99c56ebe3974 100644
--- a/clang-tools-extra/clangd/ClangdServer.h
+++ b/clang-tools-extra/clangd/ClangdServer.h
@@ -390,7 +390,7 @@ class ClangdServer {
   // Returns false if the timeout expires.
   // FIXME: various subcomponents each get the full timeout, so it's more of
   // an order of magnitude than a hard deadline.
-  LLVM_NODISCARD bool
+  [[nodiscard]] bool
   blockUntilIdleForTest(llvm::Optional TimeoutSeconds = 10);
 
   /// Builds a nested representation of memory used by components.

diff  --git a/clang-tools-extra/clangd/index/Background.h 
b/clang-tools-extra/clangd/index/Background.h
index 688c09814cd2..ac99a151e73b 100644
--- a/clang-tools-extra/clangd/index/Background.h
+++ b/clang-tools-extra/clangd/index/Background.h
@@ -110,7 +110,7 @@ class BackgroundQueue {
   // Disables thread priority lowering to ensure progress on loaded systems.
   // Only affects tasks that run after the call.
   static void preventThreadStarvationInTests();
-  LLVM_NODISCARD bool
+  [[nodiscard]] bool
   blockUntilIdleForTest(llvm::Optional TimeoutSeconds);
 
 private:
@@ -172,7 +172,7 @@ class BackgroundIndex : public SwapIndex {
   }
 
   // Wait until the queue is empty, to allow deterministic testing.
-  LLVM_NODISCARD bool
+  [[nodiscard]] bool
   blockUntilIdleForTest(llvm::Optional TimeoutSeconds = 10) {
 return Queue.blockUntilIdleForTest(TimeoutSeconds);
   }

diff  --git a/clang-tools-extra/clangd/index/Serialization.cpp 
b/clang-tools-extra/clangd/index/Serialization.cpp
index 9fc1567ad919..9270c897f88c 100644
--- a/clang-tools-extra/clangd/index/Serialization.cpp
+++ b/clang-tools-extra/clangd/index/Serialization.cpp
@@ -116,7 +116,7 @@ class Reader {
   // Read a vari

[PATCH] D131468: [WIP][BPF]: Force sign/zero extension for arguments in callee and return values in caller

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

In D131468#3708977 , @pengfei wrote:

> D124435  is going to change the assumption 
> :)

Thanks for the pointer. So looks like there is an effort to always do 
sign/zero-extension in callee. If the default is not to do extension in callee, 
we could change default value in BPF related codes, or we could implement bpf 
target specific code like this patch.

I guess I only need to focus on the func return value for now which is my focus 
any way.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131468

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

[PATCH] D130847: [clang] SourceManager: fix at SourceManager::getFileIDLoaded for the case of invalid SLockEntry

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko added a comment.

In D130847#3693668 , @aaron.ballman 
wrote:

> These changes look reasonable, but I verified that the precommit CI failures 
> are valid -- it looks like this change broke a test somehow; perhaps a caller 
> was relying on the old behavior and needs to be reworked?

I fixed the problem. It seems to be reasonable to abort the search procedure at 
the case of invalid `SLockEntrie`s. That also compatible with existent tests. 
I updated the patch title and summary as well.

@aaron.ballman , could you look at it?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130847

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

[clang] d4ff9ef - [clang][ASTImporter] Improve import of functions with auto return type.

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

Author: Balázs Kéri
Date: 2022-08-09T09:20:06+02:00
New Revision: d4ff9eff767c24716b5654780bc2e48c2017b4c5

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

LOG: [clang][ASTImporter] Improve import of functions with auto return type.

ASTImporter used to crash in some cases when a function is imported with
`auto` return type and the return type has references into the function.
The handling of such cases is improved and crash should not occur any more
but it is not fully verified, there are very many different types of
cases to care for.

Reviewed By: martong

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

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index 609d9f3a4ddf..fa2f07ad74fc 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3248,56 +3248,164 @@ static bool isAncestorDeclContextOf(const DeclContext 
*DC, const Stmt *S) {
   return false;
 }
 
-static bool hasTypeDeclaredInsideFunction(QualType T, const FunctionDecl *FD) {
-  if (T.isNull())
+namespace {
+/// Check if a type has any reference to a declaration that is inside the body
+/// of a function.
+/// The \c CheckType(QualType) function should be used to determine
+/// this property.
+///
+/// The type visitor visits one type object only (not recursive).
+/// To find all referenced declarations we must discover all type objects until
+/// the canonical type is reached (walk over typedef and similar objects). This
+/// is done by loop over all "sugar" type objects. For every such type we must
+/// check all declarations that are referenced from it. For this check the
+/// visitor is used. In the visit functions all referenced declarations except
+/// the one that follows in the sugar chain (if any) must be checked. For this
+/// check the same visitor is re-used (it has no state-dependent data).
+///
+/// The visit functions have 3 possible return values:
+///  - True, found a declaration inside \c ParentDC.
+///  - False, found declarations only outside \c ParentDC and it is not 
possible
+///to find more declarations (the "sugar" chain does not continue).
+///  - Empty optional value, found no declarations or only outside \c ParentDC,
+///but it is possible to find more declarations in the type "sugar" chain.
+/// The loop over the "sugar" types can be implemented by using type visit
+/// functions only (call \c CheckType with the desugared type). With the 
current
+/// solution no visit function is needed if the type has only a desugared type
+/// as data.
+class IsTypeDeclaredInsideVisitor
+: public TypeVisitor> {
+public:
+  IsTypeDeclaredInsideVisitor(const FunctionDecl *ParentDC)
+  : ParentDC(ParentDC) {}
+
+  bool CheckType(QualType T) {
+// Check the chain of "sugar" types.
+// The "sugar" types are typedef or similar types that have the same
+// canonical type.
+if (Optional Res = Visit(T.getTypePtr()))
+  return *Res;
+QualType DsT =
+T.getSingleStepDesugaredType(ParentDC->getParentASTContext());
+while (DsT != T) {
+  if (Optional Res = Visit(DsT.getTypePtr()))
+return *Res;
+  T = DsT;
+  DsT = T.getSingleStepDesugaredType(ParentDC->getParentASTContext());
+}
 return false;
+  }
+
+  Optional VisitTagType(const TagType *T) {
+if (auto *Spec = dyn_cast(T->getDecl()))
+  for (const auto &Arg : Spec->getTemplateArgs().asArray())
+if (checkTemplateArgument(Arg))
+  return true;
+return isAncestorDeclContextOf(ParentDC, T->getDecl());
+  }
+
+  Optional VisitPointerType(const PointerType *T) {
+return CheckType(T->getPointeeType());
+  }
+
+  Optional VisitReferenceType(const ReferenceType *T) {
+return CheckType(T->getPointeeTypeAsWritten());
+  }
+
+  Optional VisitTypedefType(const TypedefType *T) {
+const TypedefNameDecl *TD = T->getDecl();
+assert(TD);
+return isAncestorDeclContextOf(ParentDC, TD);
+  }
+
+  Optional VisitUsingType(const UsingType *T) {
+if (T->getFoundDecl() &&
+isAncestorDeclContextOf(ParentDC, T->getFoundDecl()))
+  return true;
+
+return {};
+  }
+
+  Optional
+  VisitTemplateSpecializationType(const TemplateSpecializationType *T) {
+for (const auto &Arg : T->template_arguments())
+  if (checkTemplateArgument(Arg))
+return true;
+// This type is a "sugar" to a record type, it can have a desugared type.
+return {};
+  }
+
+  Optional VisitConstantArrayType(const ConstantArrayType *T) {
+if (T->getSizeExpr() && isAncestorDeclContextOf(ParentDC, 
T->getSizeExpr()))
+  return true;
+
+return CheckType(T->getElementType());
+ 

[PATCH] D130705: [clang][ASTImporter] Improve import of functions with auto return type.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rGd4ff9eff767c: [clang][ASTImporter] Improve import of 
functions with auto return type. (authored by balazske).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130705

Files:
  clang/lib/AST/ASTImporter.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -6320,64 +6320,282 @@
   EXPECT_EQ(ToEPI.ExceptionSpec.SourceDecl, ToCtor);
 }
 
-struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {};
+struct ImportAutoFunctions : ASTImporterOptionSpecificTestBase {
+  void testImport(llvm::StringRef Code, clang::TestLanguage Lang = Lang_CXX14) {
+Decl *FromTU = getTuDecl(Code, Lang, "input0.cc");
+FunctionDecl *From = FirstDeclMatcher().match(
+FromTU, functionDecl(hasName("foo")));
+
+FunctionDecl *To = Import(From, Lang);
+EXPECT_TRUE(To);
+// We check here only that the type is auto type.
+// These tests are to verify that no crash happens.
+// The crash possibility is the presence of a reference to a declaration
+// in the function's body from the return type, if the function has auto
+// return type.
+EXPECT_TRUE(isa(To->getReturnType()));
+  }
+};
+
+TEST_P(ImportAutoFunctions, ReturnWithFunctionTemplate1) {
+  testImport(
+  R"(
+  template
+  C f1() { return C(); }
+  auto foo() {
+  struct B {};
+  return f1();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithFunctionTemplate2) {
+  testImport(
+  R"(
+  template
+  int f1(T t) { return 1; }
+  auto foo() {
+  struct B {};
+  return f1(B());
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithFunctionTemplate3) {
+  testImport(
+  R"(
+  template struct S1 {};
+  template struct S2 {};
+  template
+  S1 f1() { return S1(); }
+  auto foo() {
+  struct B {};
+  return f1>();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithFunctionTemplate4) {
+  testImport(
+  R"(
+  template struct S1 {};
+  template struct S2 {};
+  template
+  S1 f1() { return S1(); }
+  auto foo() {
+  struct B {};
+  return f1, bool>();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithVarTemplate1) {
+  testImport(
+  R"(
+  template T X;
+  auto foo() {
+  struct A {};
+  return X;
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithVarTemplate2) {
+  testImport(
+  R"(
+  template struct S1 {};
+  template S1 X;
+  auto foo() {
+  struct A {};
+  return X>;
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithVarTemplate3) {
+  testImport(
+  R"(
+  template struct S1 {};
+  template S1 X;
+  auto foo() {
+  struct A {};
+  return X>;
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithAutoUnresolvedArg) {
+  testImport(
+  R"(
+  template
+  auto foo() {
+return 22;
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithTemplateTemplateArg) {
+  // FIXME: Is it possible to have the template arg inside the function?
+  testImport(
+  R"(
+  template struct Tmpl {};
+  template class> struct TmplTmpl {};
+  auto foo() {
+return TmplTmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithDeclarationTemplateArg) {
+  // FIXME: Is it possible to have the template arg inside the function?
+  testImport(
+  R"(
+  template struct Tmpl {};
+  int A[10];
+  auto foo() {
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithNullPtrTemplateArg) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+constexpr int* A = nullptr;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithIntegralTemplateArg) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+using Int = int;
+constexpr Int A = 7;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithDecltypeTypeDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  auto foo() {
+struct X {};
+X x;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFunctions, ReturnWithTemplateWithUsingTypeDeclaredInside) {
+  testImport(
+  R"(
+  template struct Tmpl {};
+  namespace A { struct X {}; }
+  auto foo() {
+using A::X;
+return Tmpl();
+  }
+  )");
+}
+
+TEST_P(ImportAutoFuncti

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:138
   ///
+  ///  `Call` must be either a `CallExpr` or a `CXXConstructExpr`.
+  ///

How about we define overloads that take these types instead of taking an `Expr` 
here? This should remove the need for type-checking and guarding against bad 
input in the implementation. `transferInlineCall` can be a template if 
necessary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131438

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

[PATCH] D105255: [MLIR][OpenMP] Added target data, exit data, and enter data operation definition for MLIR.

[Valentin Clement via Phabricator via cfe-commits]

clementval added a comment.

In D105255#3708258 , 
@abidmalikwaterloo wrote:

> In D105255#3707533 , @clementval 
> wrote:
>
>> In D105255#3707495 , @raghavendhra 
>> wrote:
>>
>>> In D105255#3707088 , 
>>> @abidmalikwaterloo wrote:
>>>
 In D105255#3706602 , 
 @raghavendhra wrote:

> @abidmalikwaterloo Can you please check your last patch pushed for 
> review? https://buildkite.com/llvm-project/diff-checks/builds/117796 
> states it can not apply your patch something to do with the usage of diff 
> --update ? Can you please check?

 @raghavendhra  `I tried to correct it but could not figure out the 
 solution. Do you have any? I am not experienced with the phabricator 
 framework.  One solution is to submit another patch with all changes till 
 today and abandon this one.`
>>>
>>> Sorry, I am new to phabricator as well. I use "arc diff" to upload my 
>>> revisions for review after addressing review comments. If no one responds 
>>> probably I am fine with new review and abandoning this one.
>>
>> Did you try `arc diff --update D105255 ` from your branch?
>
> `yes, I tried this. Actually,  I updated the patch. through this`

The problem is likely that the commit before the commit of this patch is not in 
tree,

The easiest way is to create a new branch from main and cherry-pick the commit 
you need for this patch on the top of it. Then you just use arc diff --update 
and it should work.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D105255

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

[PATCH] D131088: [clang] Apply FixIts to members declared via `using` in derived classes

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

looks like you've uploaded the diff without context, can you upload it again 
with full context? also the changes to `MaybeAddResult` seem to be missing.


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

https://reviews.llvm.org/D131088

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[dongjunduo via Phabricator via cfe-commits]

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

Change the default storing path of `-ftime-trace`.

We can use `-ftime-trace` or `-ftime-trace=` to switch on the 
TimeProfiler.
But if we just use `-ftime-trace`, the default storing path is the directory 
which storing *.o.

The usual use case is that:

  
  $ clang -ftime-trace -o xxx.out xxx.cpp

But the json file will be stored in `/tmp` which stores the temp *.o, thus we 
CANNOT easily find the time tracing data.

This implementation change the default storing behavior of `-ftime-trace`:

1. If we specified "-ftime-trace=", the time tracing json file would be 
stored in that `path`;
2. If we just use "-ftime-trace" but compiling with linking action, the json 
would be stored in the same directory of linking result.
3. If we just use "-ftime-trace" and just compiling source to *.o, the json 
would be stored in the same directory of *.o.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131469

Files:
  clang/lib/Driver/Driver.cpp
  clang/test/Driver/check-time-trace.cpp
  clang/tools/driver/cc1_main.cpp

Index: clang/tools/driver/cc1_main.cpp
===
--- clang/tools/driver/cc1_main.cpp
+++ clang/tools/driver/cc1_main.cpp
@@ -256,17 +256,9 @@
   llvm::TimerGroup::clearAll();
 
   if (llvm::timeTraceProfilerEnabled()) {
-SmallString<128> Path(Clang->getFrontendOpts().OutputFile);
-llvm::sys::path::replace_extension(Path, "json");
-if (!Clang->getFrontendOpts().TimeTracePath.empty()) {
-  // replace the suffix to '.json' directly
-  SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
-  if (llvm::sys::fs::is_directory(TracePath))
-llvm::sys::path::append(TracePath, llvm::sys::path::filename(Path));
-  Path.assign(TracePath);
-}
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
 if (auto profilerOutput = Clang->createOutputFile(
-Path.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,
 /*useTemporary=*/false)) {
   llvm::timeTraceProfilerWrite(*profilerOutput);
   profilerOutput.reset();
Index: clang/test/Driver/check-time-trace.cpp
===
--- clang/test/Driver/check-time-trace.cpp
+++ clang/test/Driver/check-time-trace.cpp
@@ -1,3 +1,8 @@
+// RUN: rm -rf %T/exe && mkdir %T/exe
+// RUN: %clangxx -ftime-trace -ftime-trace-granularity=0 -o %T/exe/check-time-trace %s
+// RUN: cat %T/exe/check-time-trace*.json \
+// RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
+// RUN:   | FileCheck %s
 // RUN: %clangxx -S -ftime-trace -ftime-trace-granularity=0 -o %T/check-time-trace %s
 // RUN: cat %T/check-time-trace.json \
 // RUN:   | %python -c 'import json, sys; json.dump(json.loads(sys.stdin.read()), sys.stdout, sort_keys=True, indent=2)' \
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -4736,6 +4736,79 @@
/*TargetDeviceOffloadKind*/ Action::OFK_None);
   }
 
+  // set data storing path of the options `-ftime-trace`, `-ftime-trace=`
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;
+  bool TimeTraceFile = C.getArgs()
+   .getLastArg(options::OPT_ftime_trace_EQ) != nullptr;
+  if (TimeTrace || TimeTraceFile) {
+StringRef LinkingResParentPath;
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::LinkJobClass &&
+  !J.getOutputFilenames().empty()) {
+if (llvm::sys::path::has_parent_path(SmallString<128>(
+  J.getOutputFilenames()[0].c_str(
+  LinkingResParentPath = llvm::sys::path::parent_path(SmallString<128>(
+ J.getOutputFilenames()[0].c_str()));
+else
+  LinkingResParentPath = ".";
+  }
+}
+for (auto &J : C.getJobs()) {
+  if (J.getSource().getKind() == Action::AssembleJobClass ||
+  J.getSource().getKind() == Action::BackendJobClass) {
+SmallString<128> OutputPath(J.getOutputFilenames()[0].c_str());
+std::string arg = std::string("-ftime-trace=");
+if (!TimeTraceFile) {
+  if (LinkingResParentPath.empty()){
+// /xxx/yyy.o => /xxx/yyy.json
+llvm::sys::path::replace_extension(OutputPath, "json");
+arg += std::string(OutputPath.c_str());
+  } else {
+// /xxx/yyy.o => /executable_file_parent_path/yyy.json
+SmallString<128> TracePath(LinkingResParentPath);
+llvm::sys::path::append

[PATCH] D130894: [clang] Print more information about failed static assertions

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

tbaeder added a comment.

I don't really want to bike-shed about the presentation for too long... I'm 
fine with just removing the parens, since we present it like that in the error 
message as well anyway:

  ./assert.cpp:6:1: error: static assertion failed due to requirement ''c' == 
'a''
  static_assert('c' == 'a')



> Any tests with macros?

I can add some, but they should be handled transparently, with the usual 
"expanded from macro" note.


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

https://reviews.llvm.org/D130894

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

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

[Shawn Zhong via Phabricator via cfe-commits]

ShawnZhong marked an inline comment as done.
ShawnZhong added a comment.

In D131255#3706784 , @aaron.ballman 
wrote:

> LGTM! Do you need someone to commit on your behalf? If so, what name and 
> email address would you like used for patch attribution?

I'm new to Phabricator. I don't need someone else to commit on my behalf, but I 
guess I don't have permission to merge a commit on my own.

If the patch submitted misses author information for some reason, feel free to 
use `Shawn Zhong `.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

We have this error in OpenMP:
https://github.com/llvm/llvm-project/issues/57022


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Mike Hommey via Phabricator via cfe-commits]

glandium added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)

You didn't update llvm/cmake/platforms/WinMsvc.cmake accordingly


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[clang] af0052e - Fix MSVC "not all control paths return a value" warning. NFC.

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-08-09T09:55:57+01:00
New Revision: af0052ef741f9bcfa3c7f0038d49f0cb1eaa59a4

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

LOG: Fix MSVC "not all control paths return a value" warning. NFC.

Added: 


Modified: 
clang/lib/AST/ASTImporter.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTImporter.cpp b/clang/lib/AST/ASTImporter.cpp
index fa2f07ad74fc..c368a61577cb 100644
--- a/clang/lib/AST/ASTImporter.cpp
+++ b/clang/lib/AST/ASTImporter.cpp
@@ -3390,6 +3390,7 @@ class IsTypeDeclaredInsideVisitor
   // A template passed as argument can be not in ParentDC.
   return false;
 }
+llvm_unreachable("Unknown TemplateArgument::ArgKind enum");
   };
 };
 } // namespace



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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)

glandium wrote:
> You didn't update llvm/cmake/platforms/WinMsvc.cmake accordingly
AHA! That explains the fms-compatibility issue. Will you push a NFC commit or 
do you want me to do that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Mike Hommey via Phabricator via cfe-commits]

glandium added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)

thieta wrote:
> glandium wrote:
> > You didn't update llvm/cmake/platforms/WinMsvc.cmake accordingly
> AHA! That explains the fms-compatibility issue. Will you push a NFC commit or 
> do you want me to do that?
I don't have push access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added inline comments.



Comment at: llvm/cmake/modules/CheckCompilerVersion.cmake:16
 # _MSC_VER == 1927 MSVC++ 14.27 Visual Studio 2019 Version 16.7
-set(MSVC_MIN 19.20)
+set(MSVC_MIN 19.27)
 set(MSVC_SOFT_ERROR 19.27)

glandium wrote:
> thieta wrote:
> > glandium wrote:
> > > You didn't update llvm/cmake/platforms/WinMsvc.cmake accordingly
> > AHA! That explains the fms-compatibility issue. Will you push a NFC commit 
> > or do you want me to do that?
> I don't have push access.
fixed it here: 
https://github.com/llvm/llvm-project/commit/15eaefa5fe3608b03f1abefc31129efaf9eab88e


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[clang-tools-extra] d9e5462 - [clang-pseudo] Forest.h - don't inherit from std::iterator

[Simon Pilgrim via cfe-commits]

Author: Simon Pilgrim
Date: 2022-08-09T10:18:53+01:00
New Revision: d9e5462da61c3e2137a21a868a36f7022a39b59e

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

LOG: [clang-pseudo] Forest.h - don't inherit from std::iterator

Now that we've updated to C++17 MSVC gives very verbose warnings about not 
creating classes that inherit from std::iterator - use 
llvm::iterator_facade_base instead

Fixes #57005

Added: 


Modified: 
clang-tools-extra/pseudo/include/clang-pseudo/Forest.h

Removed: 




diff  --git a/clang-tools-extra/pseudo/include/clang-pseudo/Forest.h 
b/clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
index ef9a222faf6bf..130cf1ac7ef1a 100644
--- a/clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
+++ b/clang-tools-extra/pseudo/include/clang-pseudo/Forest.h
@@ -199,7 +199,9 @@ class ForestArena {
 };
 
 class ForestNode::RecursiveIterator
-: public std::iterator {
+: public llvm::iterator_facade_base {
   llvm::DenseSet Seen;
   struct StackFrame {
 const ForestNode *Parent;



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

[PATCH] D131067: [analyzer] Treat values passed as parameter as escaped

[Thomas Weißschuh via Phabricator via cfe-commits]

t-8ch updated this revision to Diff 451072.
t-8ch added a comment.

Rework tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131067

Files:
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/test/Analysis/dead-stores.cpp


Index: clang/test/Analysis/dead-stores.cpp
===
--- clang/test/Analysis/dead-stores.cpp
+++ clang/test/Analysis/dead-stores.cpp
@@ -217,3 +217,35 @@
   return i + j;
 }
 
+//===--===//
+// Dead store checking involving reference parameters.
+//===--===//
+
+struct ReferenceParameter {
+  int *ptr;
+
+  ReferenceParameter(int &i) : ptr(&i) {}
+  void function(int &i) {
+ptr = &i;
+  }
+
+  int value() {
+return *ptr;
+  }
+};
+
+void referenceParameters() {
+  int i = 7;
+  ReferenceParameter r(i);
+  i = 8;
+  if (r.value() == 8)
+;
+  i = 9; // FIXME this is a false-negative
+  
+  int j = 10;
+  r.function(j);
+  j = 11;
+  if (r.value() == 11)
+;
+  j = 12; // FIXME this is a false-negative
+}
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -483,12 +483,21 @@
   void operator()(const Stmt *S) {
 // Check for '&'. Any VarDecl whose address has been taken we treat as
 // escaped.
-// FIXME: What about references?
 if (auto *LE = dyn_cast(S)) {
   findLambdaReferenceCaptures(LE);
   return;
 }
 
+if (auto *CE = dyn_cast(S)) {
+  findCallReferenceParameters(CE);
+  return;
+}
+
+if (auto *CE = dyn_cast(S)) {
+  findConstructorReferenceParameters(CE);
+  return;
+}
+
 const UnaryOperator *U = dyn_cast(S);
 if (!U)
   return;
@@ -522,6 +531,31 @@
 Escaped.insert(VD);
 }
   }
+
+  void findReferenceParameter(const FunctionDecl *FD,
+  CallExpr::const_arg_range Args) {
+for (const auto It : llvm::zip(FD->parameters(), Args)) {
+  if (!std::get<0>(It)->getType()->isReferenceType())
+continue;
+
+  auto *DRE = dyn_cast(std::get<1>(It));
+  if (!DRE)
+continue;
+
+  if (auto *VD = dyn_cast(DRE->getDecl()))
+Escaped.insert(VD);
+}
+  }
+
+  void findCallReferenceParameters(const CallExpr *CE) {
+if (auto *FD = dyn_cast_or_null(CE->getCalleeDecl())) {
+  findReferenceParameter(FD, CE->arguments());
+}
+  }
+
+  void findConstructorReferenceParameters(const CXXConstructExpr *CE) {
+findReferenceParameter(CE->getConstructor(), CE->arguments());
+  }
 };
 } // end anonymous namespace
 


Index: clang/test/Analysis/dead-stores.cpp
===
--- clang/test/Analysis/dead-stores.cpp
+++ clang/test/Analysis/dead-stores.cpp
@@ -217,3 +217,35 @@
   return i + j;
 }
 
+//===--===//
+// Dead store checking involving reference parameters.
+//===--===//
+
+struct ReferenceParameter {
+  int *ptr;
+
+  ReferenceParameter(int &i) : ptr(&i) {}
+  void function(int &i) {
+ptr = &i;
+  }
+
+  int value() {
+return *ptr;
+  }
+};
+
+void referenceParameters() {
+  int i = 7;
+  ReferenceParameter r(i);
+  i = 8;
+  if (r.value() == 8)
+;
+  i = 9; // FIXME this is a false-negative
+  
+  int j = 10;
+  r.function(j);
+  j = 11;
+  if (r.value() == 11)
+;
+  j = 12; // FIXME this is a false-negative
+}
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -483,12 +483,21 @@
   void operator()(const Stmt *S) {
 // Check for '&'. Any VarDecl whose address has been taken we treat as
 // escaped.
-// FIXME: What about references?
 if (auto *LE = dyn_cast(S)) {
   findLambdaReferenceCaptures(LE);
   return;
 }
 
+if (auto *CE = dyn_cast(S)) {
+  findCallReferenceParameters(CE);
+  return;
+}
+
+if (auto *CE = dyn_cast(S)) {
+  findConstructorReferenceParameters(CE);
+  return;
+}
+
 const UnaryOperator *U = dyn_cast(S);
 if (!U)
   return;
@@ -522,6 +531,31 @@
 Escaped.insert(VD);
 }
   }
+
+  void findReferenceParameter(const FunctionDecl *FD,
+  CallExpr::const_arg_range Args) {
+for (const auto It : llvm::zip(FD->parameters(), Args)) {
+  if (!std::get<0>(It)->getType()->isReferenceType())
+continue;
+
+

[PATCH] D131479: Handle explicitly defaulted consteval special members.

[Utkarsh Saxena via Phabricator via cfe-commits]

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

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131479

Files:
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -766,3 +766,55 @@
   static_assert(c == 8);
 }
 }
+
+namespace DefaultedTemp {
+template 
+struct default_ctor {
+  T data;
+  consteval default_ctor() = default; // expected-note {{non-constexpr constructor 'foo' cannot be used in a constant expression}}
+};
+
+template 
+struct copy {
+  T data;
+
+  consteval copy(const copy &) = default;// expected-note {{non-constexpr constructor 'foo' cannot be used in a constant expression}}
+  consteval copy &operator=(const copy &) = default; // expected-note {{non-constexpr function 'operator=' cannot be used in a constant expression}}
+  copy() = default;
+};
+
+template 
+struct move {
+  T data;
+
+  consteval move(move &&) = default;// expected-note {{non-constexpr constructor 'foo' cannot be used in a constant expression}}
+  consteval move &operator=(move &&) = default; // expected-note {{non-constexpr function 'operator=' cannot be used in a constant expression}}
+  move() = default;
+};
+
+struct foo {
+  foo() {}// expected-note {{declared here}}
+  foo(const foo &) {} // expected-note {{declared here}}
+  foo(foo &&) {}  // expected-note {{declared here}}
+
+  foo& operator=(const foo &) { return *this; } // expected-note {{declared here}}
+  foo& operator=(foo &&) { return *this; }  // expected-note {{declared here}}
+};
+
+void func() {
+  default_ctor fail0; // expected-error {{call to consteval function 'DefaultedTemp::default_ctor::default_ctor' is not a constant expression}} \
+  expected-note {{in call to 'default_ctor()'}}
+
+  copy good0;
+  copy fail1{good0}; // expected-error {{call to consteval function 'DefaultedTemp::copy::copy' is not a constant expression}} \
+ expected-note {{in call to 'copy(good0)'}}
+  fail1 = good0;  // expected-error {{call to consteval function 'DefaultedTemp::copy::operator=' is not a constant expression}} \
+ expected-note {{in call to '&fail1->operator=(good0)'}}
+
+  move good1;
+  move fail2{static_cast&&>(good1)}; // expected-error {{call to consteval function 'DefaultedTemp::move::move' is not a constant expression}} \
+   expected-note {{in call to 'move(good1)'}}
+  fail2 = static_cast&&>(good1);  // expected-error {{call to consteval function 'DefaultedTemp::move::operator=' is not a constant expression}} \
+   expected-note {{in call to '&fail2->operator=(good1)'}}
+}
+} // namespace DefaultedTemp
Index: clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
===
--- clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
+++ clang/test/SemaCXX/cxx0x-defaulted-functions.cpp
@@ -44,18 +44,20 @@
 }
 
 template struct S : T {
-  constexpr S() = default;
-  constexpr S(const S&) = default;
-  constexpr S(S&&) = default;
+  constexpr S() = default; // expected-note {{previous declaration is here}}
+  constexpr S(const S&) = default; // expected-note {{previous declaration is here}}
+  constexpr S(S&&) = default;  // expected-note {{previous declaration is here}}
 };
 struct lit { constexpr lit() {} };
 S s_lit; // ok
 S s_bar; // ok
 
 struct Friends {
-  friend S::S();
-  friend S::S(const S&);
-  friend S::S(S&&);
+  // FIXME: these error may or may not be correct; there is an open question on
+  // the CWG reflectors about this.
+  friend S::S();  // expected-error {{non-constexpr declaration of 'S' follows constexpr declaration}}
+  friend S::S(const S&);  // expected-error {{non-constexpr declaration of 'S' follows constexpr declaration}}
+  friend S::S(S&&);   // expected-error {{non-constexpr declaration of 'S' follows constexpr declaration}}
 };
 
 namespace DefaultedFnExceptionSpec {
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -17,6 +17,7 @@
 #include "clang/AST/CXXInheritance.h"
 #include "clang/AST/CharUnits.h"
 #include "clang/AST/ComparisonCategories.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/AST/EvaluatedExprVisitor.h"
 #include "clang/AST/ExprCXX.h"
 #include "clang/AST/RecordLayout.h"
@@ -7202,10 +7203,24 @@
 static bool defaultedSpecialMemberIsConstexpr

[PATCH] D112049: [ObjC] avoid crashing when emitting synthesized getter/setter and ptrdiff_t is smaller than long

[Matt Jacobson via Phabricator via cfe-commits]

mhjacobson updated this revision to Diff 451085.
Herald added a project: All.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D112049

Files:
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/avr/objc-property.m


Index: clang/test/CodeGen/avr/objc-property.m
===
--- /dev/null
+++ clang/test/CodeGen/avr/objc-property.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm -fobjc-runtime=macosx %s -o /dev/null
+
+__attribute__((objc_root_class))
+@interface Foo
+
+@property(strong) Foo *f;
+
+@end
+
+@implementation Foo
+
+@synthesize f = _f;
+
+@end
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3967,6 +3967,8 @@
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
   const ObjCIvarDecl *Ivar);
+  llvm::Value *EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl *Interface,
+   const ObjCIvarDecl *Ivar);
   LValue EmitLValueForField(LValue Base, const FieldDecl* Field);
   LValue EmitLValueForLambdaField(const FieldDecl *Field);
 
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -1192,7 +1192,7 @@
   Builder.CreateLoad(GetAddrOfLocalVar(getterMethod->getCmdDecl()), "cmd");
 llvm::Value *self = Builder.CreateBitCast(LoadObjCSelf(), VoidPtrTy);
 llvm::Value *ivarOffset =
-  EmitIvarOffset(classImpl->getClassInterface(), ivar);
+EmitIvarOffsetAsPointerDiff(classImpl->getClassInterface(), ivar);
 
 CallArgList args;
 args.add(RValue::get(self), getContext().getObjCIdType());
@@ -1479,7 +1479,7 @@
 llvm::Value *self =
   Builder.CreateBitCast(LoadObjCSelf(), VoidPtrTy);
 llvm::Value *ivarOffset =
-  EmitIvarOffset(classImpl->getClassInterface(), ivar);
+EmitIvarOffsetAsPointerDiff(classImpl->getClassInterface(), ivar);
 Address argAddr = GetAddrOfLocalVar(*setterMethod->param_begin());
 llvm::Value *arg = Builder.CreateLoad(argAddr, "arg");
 arg = Builder.CreateBitCast(arg, VoidPtrTy);
Index: clang/lib/CodeGen/CGExpr.cpp
===
--- clang/lib/CodeGen/CGExpr.cpp
+++ clang/lib/CodeGen/CGExpr.cpp
@@ -5278,6 +5278,15 @@
   return CGM.getObjCRuntime().EmitIvarOffset(*this, Interface, Ivar);
 }
 
+llvm::Value *
+CodeGenFunction::EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl 
*Interface,
+ const ObjCIvarDecl *Ivar) {
+  llvm::Value *OffsetValue = EmitIvarOffset(Interface, Ivar);
+  QualType PointerDiffType = getContext().getPointerDiffType();
+  return Builder.CreateZExtOrTrunc(OffsetValue,
+   getTypes().ConvertType(PointerDiffType));
+}
+
 LValue CodeGenFunction::EmitLValueForIvar(QualType ObjectTy,
   llvm::Value *BaseValue,
   const ObjCIvarDecl *Ivar,


Index: clang/test/CodeGen/avr/objc-property.m
===
--- /dev/null
+++ clang/test/CodeGen/avr/objc-property.m
@@ -0,0 +1,14 @@
+// RUN: %clang_cc1 -triple avr -emit-llvm -fobjc-runtime=macosx %s -o /dev/null
+
+__attribute__((objc_root_class))
+@interface Foo
+
+@property(strong) Foo *f;
+
+@end
+
+@implementation Foo
+
+@synthesize f = _f;
+
+@end
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -3967,6 +3967,8 @@
 
   llvm::Value *EmitIvarOffset(const ObjCInterfaceDecl *Interface,
   const ObjCIvarDecl *Ivar);
+  llvm::Value *EmitIvarOffsetAsPointerDiff(const ObjCInterfaceDecl *Interface,
+   const ObjCIvarDecl *Ivar);
   LValue EmitLValueForField(LValue Base, const FieldDecl* Field);
   LValue EmitLValueForLambdaField(const FieldDecl *Field);
 
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -1192,7 +1192,7 @@
   Builder.CreateLoad(GetAddrOfLocalVar(getterMethod->getCmdDecl()), "cmd");
 llvm::Value *self = Builder.CreateBitCast(LoadObjCSelf(), VoidPtrTy);
 llvm::Value *ivarOffset =
-  EmitIvarOffset(classImpl->getClassInterface(), ivar);
+EmitIvarOffsetAsPointerDiff(classImpl->getClassInterface(), ivar);
 
 CallArgList args;
 args.add(RValue::get(self), getContext().getObjCIdType());
@

[clang] 0752999 - [Sema] Merge variable template specializations

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2022-08-09T12:17:41+02:00
New Revision: 07529996d92b9fc0cc760f4e98d88b607f66e747

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

LOG: [Sema] Merge variable template specializations

Clang used to produce redefinition errors, see tests for examples.

Reviewed By: ChuanqiXu

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

Added: 
clang/test/Modules/merge-var-template-spec-cxx-modules.cppm
clang/test/Modules/merge-var-template-spec.cpp

Modified: 
clang/lib/Sema/SemaDecl.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 04f01dd0de64d..605e4bf0abaeb 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4739,6 +4739,7 @@ bool Sema::checkVarDeclRedefinition(VarDecl *Old, VarDecl 
*New) {
   if (!hasVisibleDefinition(Old) &&
   (New->getFormalLinkage() == InternalLinkage ||
New->isInline() ||
+   isa(New) ||
New->getDescribedVarTemplate() ||
New->getNumTemplateParameterLists() ||
New->getDeclContext()->isDependentContext())) {

diff  --git a/clang/test/Modules/merge-var-template-spec-cxx-modules.cppm 
b/clang/test/Modules/merge-var-template-spec-cxx-modules.cppm
new file mode 100644
index 0..a451bfe7804d3
--- /dev/null
+++ b/clang/test/Modules/merge-var-template-spec-cxx-modules.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/var_def.cppm -o 
%t/var_def.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t 
%t/reexport1.cppm -o %t/reexport1.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t 
%t/reexport2.cppm -o %t/reexport2.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/use.cppm 
-fsyntax-only -verify
+
+//--- use.cppm
+import reexport1;
+import reexport2;
+
+auto foo = zero;
+auto bar = zero;
+auto baz = zero;
+
+template  constexpr T zero = 0; // expected-error-re 
{{declaration{{.*}}in the global module follows declaration in module var_def}}
+ // expected-note@* {{previous}}
+template <> constexpr Int zero = {0}; // expected-error-re 
{{declaration{{.*}}in the global module follows declaration in module var_def}}
+   // expected-note@* {{previous}}
+template  constexpr T* zero = nullptr; // expected-error-re 
{{declaration{{.*}}in the global module follows declaration in module var_def}}
+// expected-note@* 
{{previous}}
+
+template <> constexpr int** zero = nullptr; // ok, new specialization.
+template  constexpr T** zero = nullptr; // ok, new partial 
specilization.
+
+//--- var_def.cppm
+export module var_def;
+
+export template  constexpr T zero = 0;
+export struct Int {
+int value;
+};
+export template <> constexpr Int zero = {0};
+export template  constexpr T* zero = nullptr;
+
+//--- reexport1.cppm
+export module reexport1;
+export import var_def;
+
+//--- reexport2.cppm
+export module reexport2;
+export import var_def;

diff  --git a/clang/test/Modules/merge-var-template-spec.cpp 
b/clang/test/Modules/merge-var-template-spec.cpp
new file mode 100644
index 0..01448f34e6338
--- /dev/null
+++ b/clang/test/Modules/merge-var-template-spec.cpp
@@ -0,0 +1,70 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// We need '-fmodules-local-submodule-visibility' to properly test merging 
when building a module from multiple
+// headers inside the same TU. C++20 mode would imply this flag, but we need 
it to set it explicitly for C++14.
+//
+// RUN: %clang_cc1 -xc++ -std=c++14 -fmodules 
-fmodules-local-submodule-visibility -fmodule-name=library \
+// RUN: -emit-module %t/modules.map \
+// RUN: -o %t/module.pcm
+//
+//
+// RUN: %clang_cc1 -xc++ -std=c++14 -fmodules 
-fmodules-local-submodule-visibility -fmodule-file=%t/module.pcm  \
+// RUN: -fmodule-map-file=%t/modules.map \
+// RUN: -fsyntax-only -verify %t/use.cpp
+//
+//--- use.cpp
+
+#include "var1.h"
+#include "var2.h"
+
+auto foo = zero;
+auto bar = zero;
+auto baz = zero;
+
+template  constexpr T zero = 0; // expected-error {{redefinition}} 
expected-note@* {{previous}}
+template <> constexpr Int zero = {0}; // expected-error {{redefinition}} 
expected-note@* {{previous}}
+template  constexpr T* zero = nullptr; // expected-error 
{{redefinition}} expected-note@* {{previous}}
+
+template <> constexpr int** zero = nullptr; // ok, new specialization.
+template  constexpr T** zero = nullptr; // ok, new partial 
specilization.
+
+//--- modules.map
+module "library" {
+   export *
+   module "var1" {
+

[PATCH] D131258: [Sema] Merge variable template specializations

[Ilya Biryukov via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
ilya-biryukov marked 2 inline comments as done.
Closed by commit rG07529996d92b: [Sema] Merge variable template specializations 
(authored by ilya-biryukov).

Changed prior to commit:
  https://reviews.llvm.org/D131258?vs=450770&id=451087#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131258

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Modules/merge-var-template-spec-cxx-modules.cppm
  clang/test/Modules/merge-var-template-spec.cpp

Index: clang/test/Modules/merge-var-template-spec.cpp
===
--- /dev/null
+++ clang/test/Modules/merge-var-template-spec.cpp
@@ -0,0 +1,70 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// We need '-fmodules-local-submodule-visibility' to properly test merging when building a module from multiple
+// headers inside the same TU. C++20 mode would imply this flag, but we need it to set it explicitly for C++14.
+//
+// RUN: %clang_cc1 -xc++ -std=c++14 -fmodules -fmodules-local-submodule-visibility -fmodule-name=library \
+// RUN: -emit-module %t/modules.map \
+// RUN: -o %t/module.pcm
+//
+//
+// RUN: %clang_cc1 -xc++ -std=c++14 -fmodules -fmodules-local-submodule-visibility -fmodule-file=%t/module.pcm  \
+// RUN: -fmodule-map-file=%t/modules.map \
+// RUN: -fsyntax-only -verify %t/use.cpp
+//
+//--- use.cpp
+
+#include "var1.h"
+#include "var2.h"
+
+auto foo = zero;
+auto bar = zero;
+auto baz = zero;
+
+template  constexpr T zero = 0; // expected-error {{redefinition}} expected-note@* {{previous}}
+template <> constexpr Int zero = {0}; // expected-error {{redefinition}} expected-note@* {{previous}}
+template  constexpr T* zero = nullptr; // expected-error {{redefinition}} expected-note@* {{previous}}
+
+template <> constexpr int** zero = nullptr; // ok, new specialization.
+template  constexpr T** zero = nullptr; // ok, new partial specilization.
+
+//--- modules.map
+module "library" {
+	export *
+	module "var1" {
+		export *
+		header "var1.h"
+	}
+	module "var2" {
+		export *
+		header "var2.h"
+	}
+}
+
+//--- var1.h
+#ifndef VAR1_H
+#define VAR1_H
+
+template  constexpr T zero = 0;
+struct Int {
+int value;
+};
+template <> constexpr int zero = {0};
+template  constexpr T* zero = nullptr;
+
+#endif // VAR1_H
+
+//--- var2.h
+#ifndef VAR2_H
+#define VAR2_H
+
+template  constexpr T zero = 0;
+struct Int {
+int value;
+};
+template <> constexpr int zero = {0};
+template  constexpr T* zero = nullptr;
+
+#endif // VAR2_H
Index: clang/test/Modules/merge-var-template-spec-cxx-modules.cppm
===
--- /dev/null
+++ clang/test/Modules/merge-var-template-spec-cxx-modules.cppm
@@ -0,0 +1,44 @@
+// RUN: rm -rf %t
+// RUN: mkdir %t
+// RUN: split-file %s %t
+//
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface %t/var_def.cppm -o %t/var_def.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/reexport1.cppm -o %t/reexport1.pcm
+// RUN: %clang_cc1 -std=c++20 -emit-module-interface -fprebuilt-module-path=%t %t/reexport2.cppm -o %t/reexport2.pcm
+// RUN: %clang_cc1 -std=c++20 -fprebuilt-module-path=%t %t/use.cppm -fsyntax-only -verify
+
+//--- use.cppm
+import reexport1;
+import reexport2;
+
+auto foo = zero;
+auto bar = zero;
+auto baz = zero;
+
+template  constexpr T zero = 0; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
+ // expected-note@* {{previous}}
+template <> constexpr Int zero = {0}; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
+   // expected-note@* {{previous}}
+template  constexpr T* zero = nullptr; // expected-error-re {{declaration{{.*}}in the global module follows declaration in module var_def}}
+// expected-note@* {{previous}}
+
+template <> constexpr int** zero = nullptr; // ok, new specialization.
+template  constexpr T** zero = nullptr; // ok, new partial specilization.
+
+//--- var_def.cppm
+export module var_def;
+
+export template  constexpr T zero = 0;
+export struct Int {
+int value;
+};
+export template <> constexpr Int zero = {0};
+export template  constexpr T* zero = nullptr;
+
+//--- reexport1.cppm
+export module reexport1;
+export import var_def;
+
+//--- reexport2.cppm
+export module reexport2;
+export import var_def;
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4739,6 +4739,7 @@
   if (!hasVisibleDefinition(Old) &&
   (New->getFormalLinkage() == InternalLinkage ||

[PATCH] D131351: [C] Default implicit function pointer conversions diagnostic to be an error

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 451094.
aaron.ballman added a comment.
Herald added a project: clang-tools-extra.

Rebased and fixing up clang-tools-extra tests caught by the precommit CI.


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

https://reviews.llvm.org/D131351

Files:
  clang-tools-extra/test/clang-tidy/checkers/bugprone/signal-handler-minimal.c
  clang-tools-extra/test/clang-tidy/checkers/bugprone/signal-handler.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/test/CodeGen/attributes.c
  clang/test/CodeGen/overloadable.c
  clang/test/Sema/aarch64-svepcs.c
  clang/test/Sema/aarch64-vpcs.c
  clang/test/Sema/arm-cmse.c
  clang/test/Sema/attr-nocf_check.c
  clang/test/Sema/block-return.c
  clang/test/Sema/c2x-func-prototype.c
  clang/test/Sema/callingconv-ms_abi.c
  clang/test/Sema/callingconv-sysv_abi.c
  clang/test/Sema/callingconv.c
  clang/test/Sema/incompatible-function-pointer-types.c
  clang/test/Sema/initialize-noreturn.c
  clang/test/Sema/noescape.c
  clang/test/Sema/overloadable.c
  clang/test/Sema/pass-object-size.c
  clang/test/Sema/preserve-call-conv.c
  clang/test/SemaObjC/comptypes-legal.m

Index: clang/test/SemaObjC/comptypes-legal.m
===
--- clang/test/SemaObjC/comptypes-legal.m
+++ clang/test/SemaObjC/comptypes-legal.m
@@ -31,9 +31,9 @@
 
 void foo(void)
 {
-  // GCC currently allows this (it has some fiarly new support for covariant return types and contravariant argument types).
+  // GCC currently allows this (it has some fairly new support for covariant return types and contravariant argument types).
   // Since registerFunc: expects a Derived object as it's second argument, I don't know why this would be legal.
-  [Derived registerFunc: ExternFunc];  // expected-warning{{incompatible function pointer types sending 'NSObject *(NSObject *, NSObject *)' to parameter of type 'FuncSignature *' (aka 'id (*)(NSObject *, Derived *)')}}
+  [Derived registerFunc: ExternFunc];  // expected-error{{incompatible function pointer types sending 'NSObject *(NSObject *, NSObject *)' to parameter of type 'FuncSignature *' (aka 'id (*)(NSObject *, Derived *)')}}
 }
 
 // rdar://10751015
Index: clang/test/Sema/preserve-call-conv.c
===
--- clang/test/Sema/preserve-call-conv.c
+++ clang/test/Sema/preserve-call-conv.c
@@ -14,8 +14,8 @@
 
 void (__attribute__((preserve_most)) *pfoo1)(void *) = foo;
 
-void (__attribute__((cdecl)) *pfoo2)(void *) = foo; // expected-warning {{incompatible function pointer types initializing 'void (*)(void *) __attribute__((cdecl))' with an expression of type 'void (void *) __attribute__((preserve_most))'}}
-void (*pfoo3)(void *) = foo; // expected-warning {{incompatible function pointer types initializing 'void (*)(void *)' with an expression of type 'void (void *) __attribute__((preserve_most))'}}
+void (__attribute__((cdecl)) *pfoo2)(void *) = foo; // expected-error {{incompatible function pointer types initializing 'void (*)(void *) __attribute__((cdecl))' with an expression of type 'void (void *) __attribute__((preserve_most))'}}
+void (*pfoo3)(void *) = foo; // expected-error {{incompatible function pointer types initializing 'void (*)(void *)' with an expression of type 'void (void *) __attribute__((preserve_most))'}}
 
 typedef_fun_t typedef_fun_foo; // expected-note {{previous declaration is here}}
 void __attribute__((preserve_most)) typedef_fun_foo(int x) { } // expected-error {{function declared 'preserve_most' here was previously declared without calling convention}}
@@ -30,8 +30,8 @@
 
 void (__attribute__((preserve_all)) *pboo1)(void *) = boo;
 
-void (__attribute__((cdecl)) *pboo2)(void *) = boo; // expected-warning {{incompatible function pointer types initializing 'void (*)(void *) __attribute__((cdecl))' with an expression of type 'void (void *) __attribute__((preserve_all))'}}
-void (*pboo3)(void *) = boo; // expected-warning {{incompatible function pointer types initializing 'void (*)(void *)' with an expression of type 'void (void *) __attribute__((preserve_all))'}}
+void (__attribute__((cdecl)) *pboo2)(void *) = boo; // expected-error {{incompatible function pointer types initializing 'void (*)(void *) __attribute__((cdecl))' with an expression of type 'void (void *) __attribute__((preserve_all))'}}
+void (*pboo3)(void *) = boo; // expected-error {{incompatible function pointer types initializing 'void (*)(void *)' with an expression of type 'void (void *) __attribute__((preserve_all))'}}
 
 typedef_fun_t typedef_fun_boo; // expected-note {{previous declaration is here}}
 void __attribute__((preserve_all)) typedef_fun_boo(int x) { } // expected-error {{function declared 'preserve_all' here was previously declared without calling convention}}
Index: clang/test/Sema/pass-object-size.c
===
-

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131307#3709083 , @sylvestre.ledru 
wrote:

> We have this error in OpenMP:
> https://github.com/llvm/llvm-project/issues/57022

That bug looks to be a true positive from this change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default dialect

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I'm not opposed to this, but it should have a community RFC to give people 
notice that we'd like to change the default in case this will cause issues and 
people need a bit more time. It's also worth pointing out that GCC has already 
switched to gnu++17 as their default.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131465

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

[PATCH] D131067: [analyzer] Treat values passed as parameter as escaped

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

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

In D131067#3705783 , @t-8ch wrote:

> In D131067#3702044 , @steakhal 
> wrote:
>
>> Please, consider stating the motivation behind this change.
>
> This is now added.

I was about the empty revision summary.
Generally, we use that to highlight the motivation of some proposed change.
We also tend to keep it in sync which the commit message.
(I think arcanist auto updates the review version of the summary to match with 
the actual message once you commit your change.)

Sorry for blocking this change. It makes sense now.




Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:537
+  CallExpr::const_arg_range Args) {
+for (const auto It : llvm::zip(FD->parameters(), Args)) {
+  if (!std::get<0>(It)->getType()->isReferenceType())

I think as we are on C++17 now, we could use structured bindings. WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131067

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

[PATCH] D130894: [clang] Print more information about failed static assertions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130894#3709027 , @tbaeder wrote:

> I don't really want to bike-shed about the presentation for too long...

I understand the desire, but at the same time, the whole goal of this patch is 
to improve the presentation of the diagnostic. You can't invite us all to a 
bikeshed painting party and then ask us not to use our brushes, that's just 
cruel! :-D

> I'm fine with just removing the parens, since we present it like that in the 
> error message as well anyway:
>
>   ./assert.cpp:6:1: error: static assertion failed due to requirement ''c' == 
> 'a''
>   static_assert('c' == 'a')

That's still my preference as well. Splitting across multiple lines with hard 
line breaks has some issues for some IDEs that want to display diagnostic 
information in a listbox (IIRC), so I'd rather we avoid ad hoc use of line 
breaks in diagnostics for the moment (it'd be easier/more appropriate if we 
were making a diagnostic policy for using multiple lines, but that'd involve an 
RFC and is more effort than I think is needed for this patch).

>> Any tests with macros?
>
> I can add some, but they should be handled transparently, with the usual 
> "expanded from macro" note.

I'm fine either way.


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

https://reviews.llvm.org/D130894

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

[PATCH] D131067: [analyzer] Treat values passed as parameter as escaped

[Thomas Weißschuh via Phabricator via cfe-commits]

t-8ch added a comment.






Comment at: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp:537
+  CallExpr::const_arg_range Args) {
+for (const auto It : llvm::zip(FD->parameters(), Args)) {
+  if (!std::get<0>(It)->getType()->isReferenceType())

steakhal wrote:
> I think as we are on C++17 now, we could use structured bindings. WDYT?
Sounds good, will update.
Note that "now" means since three days ago :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131067

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

[PATCH] D130974: [analyzer] Fix for the crash in #56873

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

steakhal added a comment.

Some checker should have caught the uninitialized value earlier than the 
`defaultEvalCall()`.
I guess, the `MallocCkecher` could have checked for it in `PreStmt`.
Or alternatively, the `CallAndMessageChecker::preCall()` already does something 
like this in the `PreVisitProcessArg()`. I know that `CXXNewExpr` is not a 
//call//, but you get the idea.
WDYT, worth catching it?

Other than that, I think it's a good practice to not rely on some checkers to 
catch things to prevent crashes; so this 'fix' seems reasonable to me.




Comment at: clang/lib/StaticAnalyzer/Core/ExprEngineCallAndReturn.cpp:768
+  if (Size.isUndef())
+Size = UnknownVal();
+

I'm not a fan of mutating values like this.
Alternatively we could have used something like this at the point of use:
`Size.getAs().getValueOr(UnknownVal{})`
I'm not sure if it's more readable :D


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130974

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Jon Chesterfield via Phabricator via cfe-commits]

JonChesterfield added a comment.

Did some digging here. The function hsa_agent_get_info takes an argument of 
type hsa_agent_info_t, which has declared values in the range [0 24]. The 
implementation of that (in amd_gpu_agent fwiw) casts that argument to a size_t 
and then switches on it, checking those declared values and a bunch of 
extensions. This is used to provide vendor extensions through a vendor-agnostic 
interface.

This seems to be a case where C and C++ have diverged. As far as I can tell, C 
thinks an enum is an int, and anything that fits in an int can be stored in one 
and retrieved later. C23 lets one specify the underlying type. C++ evidently 
thinks the value stored must be within [min max] of the declaration, which is 
at least more flexible than requiring it be one in the declaration.

So I think the fix here is to change hsa_agent_info_t to include 
`HSA_AGENT_INFO_UNUSED_INCREASE_RANGE_OF_TYPE = INT32_MAX` so the vendor 
extensions remain accessible. It's a header that is usable from C and C++ so it 
needs to do something conforming to both. Does that sound right?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131307#3709494 , @JonChesterfield 
wrote:

> Did some digging here. The function hsa_agent_get_info takes an argument of 
> type hsa_agent_info_t, which has declared values in the range [0 24]. The 
> implementation of that (in amd_gpu_agent fwiw) casts that argument to a 
> size_t and then switches on it, checking those declared values and a bunch of 
> extensions. This is used to provide vendor extensions through a 
> vendor-agnostic interface.
>
> This seems to be a case where C and C++ have diverged.

Yes, they've always been divergent in this area, it's only thanks to the magic 
of constexpr that anyone really notices now.

> As far as I can tell, C thinks an enum is an int, and anything that fits in 
> an int can be stored in one and retrieved later.

It's a bit more complicated than that, unfortunately. But this is kind of the 
gist of it.

> C23 lets one specify the underlying type. C++ evidently thinks the value 
> stored must be within [min max] of the declaration, which is at least more 
> flexible than requiring it be one in the declaration.

Correct, C++ uses the minimum-sized bit-field that can hold all of the values 
of the enumeration.

> So I think the fix here is to change hsa_agent_info_t to include 
> `HSA_AGENT_INFO_UNUSED_INCREASE_RANGE_OF_TYPE = INT32_MAX` so the vendor 
> extensions remain accessible. It's a header that is usable from C and C++ so 
> it needs to do something conforming to both. Does that sound right?

Yes, that's how I'd solve the problem.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D130974: [analyzer] Fix for the crash in #56873

[Domján Dániel via Phabricator via cfe-commits]

isuckatcs added a comment.

> Some checker should have caught the uninitialized value earlier than the 
> defaultEvalCall().
> I guess, the MallocCkecher could have checked for it in PreStmt.
> Or alternatively, the CallAndMessageChecker::preCall() already does something 
> like this in the PreVisitProcessArg(). I know that CXXNewExpr is not a call, 
> but you get the idea.
> WDYT, worth catching it?

I definitely think it's worth catching it. I'm working on a checker which 
addresses this in D131299 . It was originally 
intended to be a part of MallocChecker but has been moved to a separate one.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130974

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

[PATCH] D131067: [analyzer] Treat values passed as parameter as escaped

[Thomas Weißschuh via Phabricator via cfe-commits]

t-8ch updated this revision to Diff 451117.
t-8ch added a comment.

- Inline tiny functions
- Use structured binding
- Rename findReferenceParameter -> findReferenceParameters


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131067

Files:
  clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
  clang/test/Analysis/dead-stores.cpp


Index: clang/test/Analysis/dead-stores.cpp
===
--- clang/test/Analysis/dead-stores.cpp
+++ clang/test/Analysis/dead-stores.cpp
@@ -217,3 +217,35 @@
   return i + j;
 }
 
+//===--===//
+// Dead store checking involving reference parameters.
+//===--===//
+
+struct ReferenceParameter {
+  int *ptr;
+
+  ReferenceParameter(int &i) : ptr(&i) {}
+  void function(int &i) {
+ptr = &i;
+  }
+
+  int value() {
+return *ptr;
+  }
+};
+
+void referenceParameters() {
+  int i = 7;
+  ReferenceParameter r(i);
+  i = 8;
+  if (r.value() == 8)
+;
+  i = 9; // FIXME this is a false-negative
+  
+  int j = 10;
+  r.function(j);
+  j = 11;
+  if (r.value() == 11)
+;
+  j = 12; // FIXME this is a false-negative
+}
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -483,12 +483,22 @@
   void operator()(const Stmt *S) {
 // Check for '&'. Any VarDecl whose address has been taken we treat as
 // escaped.
-// FIXME: What about references?
 if (auto *LE = dyn_cast(S)) {
   findLambdaReferenceCaptures(LE);
   return;
 }
 
+if (auto *CE = dyn_cast(S)) {
+  if (auto *FD = dyn_cast_or_null(CE->getCalleeDecl()))
+findReferenceParameters(FD, CE->arguments());
+  return;
+}
+
+if (auto *CE = dyn_cast(S)) {
+  findReferenceParameters(CE->getConstructor(), CE->arguments());
+  return;
+}
+
 const UnaryOperator *U = dyn_cast(S);
 if (!U)
   return;
@@ -522,6 +532,21 @@
 Escaped.insert(cast(VD));
 }
   }
+
+  void findReferenceParameters(const FunctionDecl *FD,
+   CallExpr::const_arg_range Args) {
+for (const auto &[Param, Arg] : llvm::zip(FD->parameters(), Args)) {
+  if (!Param->getType()->isReferenceType())
+continue;
+
+  auto *DRE = dyn_cast(Arg);
+  if (!DRE)
+continue;
+
+  if (auto *VD = dyn_cast(DRE->getDecl()))
+Escaped.insert(VD);
+}
+  }
 };
 } // end anonymous namespace
 


Index: clang/test/Analysis/dead-stores.cpp
===
--- clang/test/Analysis/dead-stores.cpp
+++ clang/test/Analysis/dead-stores.cpp
@@ -217,3 +217,35 @@
   return i + j;
 }
 
+//===--===//
+// Dead store checking involving reference parameters.
+//===--===//
+
+struct ReferenceParameter {
+  int *ptr;
+
+  ReferenceParameter(int &i) : ptr(&i) {}
+  void function(int &i) {
+ptr = &i;
+  }
+
+  int value() {
+return *ptr;
+  }
+};
+
+void referenceParameters() {
+  int i = 7;
+  ReferenceParameter r(i);
+  i = 8;
+  if (r.value() == 8)
+;
+  i = 9; // FIXME this is a false-negative
+  
+  int j = 10;
+  r.function(j);
+  j = 11;
+  if (r.value() == 11)
+;
+  j = 12; // FIXME this is a false-negative
+}
Index: clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/DeadStoresChecker.cpp
@@ -483,12 +483,22 @@
   void operator()(const Stmt *S) {
 // Check for '&'. Any VarDecl whose address has been taken we treat as
 // escaped.
-// FIXME: What about references?
 if (auto *LE = dyn_cast(S)) {
   findLambdaReferenceCaptures(LE);
   return;
 }
 
+if (auto *CE = dyn_cast(S)) {
+  if (auto *FD = dyn_cast_or_null(CE->getCalleeDecl()))
+findReferenceParameters(FD, CE->arguments());
+  return;
+}
+
+if (auto *CE = dyn_cast(S)) {
+  findReferenceParameters(CE->getConstructor(), CE->arguments());
+  return;
+}
+
 const UnaryOperator *U = dyn_cast(S);
 if (!U)
   return;
@@ -522,6 +532,21 @@
 Escaped.insert(cast(VD));
 }
   }
+
+  void findReferenceParameters(const FunctionDecl *FD,
+   CallExpr::const_arg_range Args) {
+for (const auto &[Param, Arg] : llvm::zip(FD->parameters(), Args)) {
+  if (!Param->getType()->isReferenceType())
+continue;
+
+  auto *DRE = dyn_cast(Arg);

[PATCH] D130847: [clang] SourceManager: fix at SourceManager::getFileIDLoaded for the case of invalid SLockEntry

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Basic/SourceManager.cpp:882-884
 const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I);
+if (E.getOffset() == 0)
+  return FileID(); // invalid entry.

This looks incorrect to me -- I don't think an offset of `0` is invalid. I 
think a better way to do this is:
```
bool Invalid;
const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I, &Invalid);
if (Invalid)
  return FileID(); // Invalid entry.
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130847

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

[clang] 1d1a569 - Clang: fix AST representation of expanded template arguments.

[Matheus Izvekov via cfe-commits]

Author: Matheus Izvekov
Date: 2022-08-09T14:26:16+02:00
New Revision: 1d1a56929b725f9a79d98877f12d0a14f8418b38

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

LOG: Clang: fix AST representation of expanded template arguments.

Extend clang's SubstTemplateTypeParm to represent the pack substitution index.

Fixes PR56099.

Signed-off-by: Matheus Izvekov 

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

Added: 


Modified: 
clang/include/clang/AST/ASTContext.h
clang/include/clang/AST/JSONNodeDumper.h
clang/include/clang/AST/TextNodeDumper.h
clang/include/clang/AST/Type.h
clang/include/clang/AST/TypeProperties.td
clang/lib/AST/ASTContext.cpp
clang/lib/AST/ASTImporter.cpp
clang/lib/AST/ASTStructuralEquivalence.cpp
clang/lib/AST/JSONNodeDumper.cpp
clang/lib/AST/TextNodeDumper.cpp
clang/lib/AST/Type.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/TreeTransform.h
clang/test/AST/ast-dump-template-decls.cpp
clang/unittests/AST/ASTImporterTest.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTContext.h 
b/clang/include/clang/AST/ASTContext.h
index d27b978998095..a01a6aa4bd7e4 100644
--- a/clang/include/clang/AST/ASTContext.h
+++ b/clang/include/clang/AST/ASTContext.h
@@ -1614,7 +1614,8 @@ class ASTContext : public RefCountedBase {
QualType Wrapped);
 
   QualType getSubstTemplateTypeParmType(const TemplateTypeParmType *Replaced,
-QualType Replacement) const;
+QualType Replacement,
+Optional PackIndex) const;
   QualType getSubstTemplateTypeParmPackType(
   const TemplateTypeParmType *Replaced,
 const TemplateArgument &ArgPack);

diff  --git a/clang/include/clang/AST/JSONNodeDumper.h 
b/clang/include/clang/AST/JSONNodeDumper.h
index a5575d7fd441e..3597903695797 100644
--- a/clang/include/clang/AST/JSONNodeDumper.h
+++ b/clang/include/clang/AST/JSONNodeDumper.h
@@ -220,6 +220,7 @@ class JSONNodeDumper
   void VisitUnaryTransformType(const UnaryTransformType *UTT);
   void VisitTagType(const TagType *TT);
   void VisitTemplateTypeParmType(const TemplateTypeParmType *TTPT);
+  void VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *STTPT);
   void VisitAutoType(const AutoType *AT);
   void VisitTemplateSpecializationType(const TemplateSpecializationType *TST);
   void VisitInjectedClassNameType(const InjectedClassNameType *ICNT);

diff  --git a/clang/include/clang/AST/TextNodeDumper.h 
b/clang/include/clang/AST/TextNodeDumper.h
index 2e4bcdd27a8ab..e6853b12ae7e5 100644
--- a/clang/include/clang/AST/TextNodeDumper.h
+++ b/clang/include/clang/AST/TextNodeDumper.h
@@ -317,6 +317,7 @@ class TextNodeDumper
   void VisitUnaryTransformType(const UnaryTransformType *T);
   void VisitTagType(const TagType *T);
   void VisitTemplateTypeParmType(const TemplateTypeParmType *T);
+  void VisitSubstTemplateTypeParmType(const SubstTemplateTypeParmType *T);
   void VisitAutoType(const AutoType *T);
   void VisitDeducedTemplateSpecializationType(
   const DeducedTemplateSpecializationType *T);

diff  --git a/clang/include/clang/AST/Type.h b/clang/include/clang/AST/Type.h
index 1a8cf27dab4bd..ad9835e839a65 100644
--- a/clang/include/clang/AST/Type.h
+++ b/clang/include/clang/AST/Type.h
@@ -1790,6 +1790,18 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
 unsigned NumArgs;
   };
 
+  class SubstTemplateTypeParmTypeBitfields {
+friend class SubstTemplateTypeParmType;
+
+unsigned : NumTypeBits;
+
+/// Represents the index within a pack if this represents a substitution
+/// from a pack expansion.
+/// Positive non-zero number represents the index + 1.
+/// Zero means this is not substituted from an expansion.
+unsigned PackIndex;
+  };
+
   class SubstTemplateTypeParmPackTypeBitfields {
 friend class SubstTemplateTypeParmPackType;
 
@@ -1872,6 +1884,7 @@ class alignas(8) Type : public ExtQualsTypeCommonBase {
 ElaboratedTypeBitfields ElaboratedTypeBits;
 VectorTypeBitfields VectorTypeBits;
 SubstTemplateTypeParmPackTypeBitfields SubstTemplateTypeParmPackTypeBits;
+SubstTemplateTypeParmTypeBitfields SubstTemplateTypeParmTypeBits;
 TemplateSpecializationTypeBitfields TemplateSpecializationTypeBits;
 DependentTemplateSpecializationTypeBitfields
   DependentTemplateSpecializationTypeBits;
@@ -4974,9 +4987,12 @@ class SubstTemplateTypeParmType : public Type, public 
llvm::FoldingSetNode {
   // The original type parameter.
   const TemplateTypeParmType *Replaced;
 
-  SubstTemplateTypeParmType(const Templ

[PATCH] D128113: Clang: fix AST representation of expanded template arguments.

[Matheus Izvekov 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 rG1d1a56929b72: Clang: fix AST representation of expanded 
template arguments. (authored by mizvekov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128113

Files:
  clang/include/clang/AST/ASTContext.h
  clang/include/clang/AST/JSONNodeDumper.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/AST/Type.h
  clang/include/clang/AST/TypeProperties.td
  clang/lib/AST/ASTContext.cpp
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/ASTStructuralEquivalence.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/TreeTransform.h
  clang/test/AST/ast-dump-template-decls.cpp
  clang/unittests/AST/ASTImporterTest.cpp

Index: clang/unittests/AST/ASTImporterTest.cpp
===
--- clang/unittests/AST/ASTImporterTest.cpp
+++ clang/unittests/AST/ASTImporterTest.cpp
@@ -4793,6 +4793,44 @@
   ToD2->getDeclContext(), ToD2->getTemplateParameters()->getParam(0)));
 }
 
+TEST_P(ASTImporterOptionSpecificTestBase, ImportSubstTemplateTypeParmType) {
+  constexpr auto Code = R"(
+template  struct A {
+  using B = A1(A2...);
+};
+template struct A;
+)";
+  Decl *FromTU = getTuDecl(Code, Lang_CXX11, "input.cpp");
+  auto *FromClass = FirstDeclMatcher().match(
+  FromTU, classTemplateSpecializationDecl());
+
+  auto testType = [&](ASTContext &Ctx, const char *Name,
+  llvm::Optional PackIndex) {
+const auto *Subst = selectFirst(
+"sttp", match(substTemplateTypeParmType(
+  hasReplacementType(hasCanonicalType(asString(Name
+  .bind("sttp"),
+  Ctx));
+const char *ExpectedTemplateParamName = PackIndex ? "A2" : "A1";
+ASSERT_TRUE(Subst);
+ASSERT_EQ(Subst->getReplacedParameter()->getIdentifier()->getName(),
+  ExpectedTemplateParamName);
+ASSERT_EQ(Subst->getPackIndex(), PackIndex);
+  };
+  auto tests = [&](ASTContext &Ctx) {
+testType(Ctx, "void", None);
+testType(Ctx, "char", 0);
+testType(Ctx, "float", 1);
+testType(Ctx, "int", 2);
+testType(Ctx, "short", 3);
+  };
+
+  tests(FromTU->getASTContext());
+
+  ClassTemplateSpecializationDecl *ToClass = Import(FromClass, Lang_CXX11);
+  tests(ToClass->getASTContext());
+}
+
 const AstTypeMatcher
 substTemplateTypeParmPackType;
 
Index: clang/test/AST/ast-dump-template-decls.cpp
===
--- clang/test/AST/ast-dump-template-decls.cpp
+++ clang/test/AST/ast-dump-template-decls.cpp
@@ -136,13 +136,13 @@
 };
 using t1 = foo::bind;
 // CHECK:  TemplateSpecializationType 0x{{[^ ]*}} 'Y' sugar Y
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'char' sugar
+// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'char' sugar pack_index 0
 // CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'Bs' dependent contains_unexpanded_pack depth 0 index 0 pack
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'float' sugar
+// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'float' sugar pack_index 1
 // CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'Bs' dependent contains_unexpanded_pack depth 0 index 0 pack
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar
+// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar pack_index 2
 // CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'Bs' dependent contains_unexpanded_pack depth 0 index 0 pack
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'short' sugar
+// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'short' sugar pack_index 3
 // CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'Bs' dependent contains_unexpanded_pack depth 0 index 0 pack
 
 template  struct D {
@@ -152,13 +152,13 @@
 // CHECK:  TemplateSpecializationType 0x{{[^ ]*}} 'B' sugar alias B
 // CHECK:  FunctionProtoType 0x{{[^ ]*}} 'int (int (*)(float, int), int (*)(char, short))' cdecl
 // CHECK:  FunctionProtoType 0x{{[^ ]*}} 'int (float, int)' cdecl
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'float' sugar
+// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'float' sugar pack_index 0
 // CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'T' dependent contains_unexpanded_pack depth 0 index 0 pack
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar
+// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'int' sugar pack_index 0
 // CHECK-NEXT: TemplateTypeParmType 0x{{[^ ]*}} 'U' dependent contains_unexpanded_pack depth 0 index 0 pack
 // CHECK:  FunctionProtoType 0x{{[^ ]*}} 'int (char, short)' cdecl
-// CHECK:  SubstTemplateTypeParmType 0x{{[^ ]*}} 'char' sugar
+// CHECK:  Subs

[PATCH] D131214: [clang][Driver] Pass correct reproduce flag to lld-link

[Nico Weber via Phabricator via cfe-commits]

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

Thanks! LG with comment.




Comment at: clang/lib/Driver/Driver.cpp:1638
 llvm::opt::ArgStringList ArgList = NewLLDInvocation.getArguments();
-ArgList.push_back(Saver.save(Twine{"--reproduce="} + TmpName).data());
+Twine ReproduceOption =
+C.getDefaultToolChain().getTriple().isWindowsMSVCEnvironment()

Twines shouldn't be on the stack. There's no reason for this to be a Twine 
either -- make it a StringRef (and then cast below if needed)


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

https://reviews.llvm.org/D131214

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

[PATCH] D131465: C++/ObjC++: switch to gnu++17 as the default dialect

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

What Aaron said.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131465

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

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

[Nemanja Ivanovic via Phabricator via cfe-commits]

nemanjai added inline comments.



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10702
+
+// If SafeStack or !StackProtector, kill_canary is not supported.
+if (MF.getFunction().hasFnAttribute(Attribute::SafeStack) ||

Again, this comment is nothing more than a re-reading of the code. As such, it 
is not useful. The comment should say what is happening and why. Something 
along the lines of:
```
// The kill_canary intrinsic only makes sense when the Stack Protector
// feature is on in the function. It can also not be used in conjunction
// with safe stack because the latter splits the stack and the canary
// value isn't used (i.e. safe stack supersedes stack protector).
// In situations where the kill_canary intrinsic is not supported,
// we simply replace uses of its chain with its input chain, causing
// the SDAG CSE to remove the node.
```



Comment at: llvm/lib/Target/PowerPC/PPCISelLowering.cpp:10720-10749
+if (useLoadStackGuardNode()) {
+  MachineSDNode *LSG =
+  DAG.getMachineNode(PPC::LOAD_STACK_GUARD, DL, VT, Op->getOperand(0));
+  Load = SDValue(LSG, 0);
+
+  // Frame index used to determine stack guard location if
+  // LOAD_STACK_GUARD is used.

The common code should just be common. Seems like the only thing that changes 
is how we load the value. Please refactor this to something like:
```
if (useLoadStackGuardNode()) {
  ...
  Load = ... // stack guard load
} else if (Value *GV = getSDagStackGuard(*M)) {
  ...
  Load = ... // canary word global load
} else
  llvm_unreachable("Unhandled stack guard case");

Store = ... // common store
return Store; // (or you can just create the store in-line and return it 
directly)
```



Comment at: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll:2
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix 
-ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s 
-check-prefix=CHECK-AIX

Please add one RUN line with `-O0` to ensure that this works as expected with 
Fast ISel.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129016

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

[PATCH] D128930: [pseudo] Add ForestNode descendants iterator, print ambiguous/opaque node stats.

[Haojian Wu via Phabricator via cfe-commits]

hokein added inline comments.



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Forest.h:202
+class ForestNode::RecursiveIterator
+: public std::iterator {
+  llvm::DenseSet Seen;

tschuett wrote:
> `std::iterator` is deprecated in C++17 and creates warnings.
This seems to be fixed in d9e5462da61c3e2137a21a868a36f7022a39b59e.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128930

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

[PATCH] D130847: [clang] SourceManager: fix at SourceManager::getFileIDLoaded for the case of invalid SLockEntry

[Ivan Murashko via Phabricator via cfe-commits]

ivanmurashko added inline comments.



Comment at: clang/lib/Basic/SourceManager.cpp:882-884
 const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I);
+if (E.getOffset() == 0)
+  return FileID(); // invalid entry.

aaron.ballman wrote:
> This looks incorrect to me -- I don't think an offset of `0` is invalid. I 
> think a better way to do this is:
> ```
> bool Invalid;
> const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I, &Invalid);
> if (Invalid)
>   return FileID(); // Invalid entry.
> ```
That's reasonable, do we want to update other places at the function as well 
(line 903 for instance)?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130847

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Adhemerval Zanella via Phabricator via cfe-commits]

zatrazz added a comment.

This commit seems to have broken test-suite on aarch64 [1]. The warning shows:

  FAILED: MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o
  /home/adhemerval.zanella/projects/llvm/test-suite-build/tools/timeit 
--summary MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o.time 
/home/adhemerval.zanella/projects/llvm/llvm-src-build-stage1-buildbot/bin/clang++
 -DNDEBUG  -O3 -DNDEBUG   -w -Werror=date-time -DNOASM -DLLVM -MD -MT 
MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o -MF 
MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o.d -o 
MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o -c 
/home/adhemerval.zanella/projects/llvm/test-suite/MultiSource/Benchmarks/PAQ8p/paq8p.cpp
  
/home/adhemerval.zanella/projects/llvm/test-suite/MultiSource/Benchmarks/PAQ8p/paq8p.cpp:3606:24:
 error: integer value -1 is outside the valid range of values [0, 15] for this 
enumeration type [-Wenum-constexpr-conversion]
  if (c==EOF) return (Filetype)(-1);
 ^
  1 error generated.

I am not sure if this is an existing issue with test-suite.

[1] https://lab.llvm.org/staging/#/builders/158/builds/344


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D130847: [clang] SourceManager: fix at SourceManager::getFileIDLoaded for the case of invalid SLockEntry

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Basic/SourceManager.cpp:882-884
 const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I);
+if (E.getOffset() == 0)
+  return FileID(); // invalid entry.

ivanmurashko wrote:
> aaron.ballman wrote:
> > This looks incorrect to me -- I don't think an offset of `0` is invalid. I 
> > think a better way to do this is:
> > ```
> > bool Invalid;
> > const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I, &Invalid);
> > if (Invalid)
> >   return FileID(); // Invalid entry.
> > ```
> That's reasonable, do we want to update other places at the function as well 
> (line 903 for instance)?
Assuming my idea works the way I think it will, yes, I think that's a good 
idea. :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130847

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4739
 
+  // set data storing path of the options `-ftime-trace`, `-ftime-trace=`
+  bool TimeTrace = C.getArgs().getLastArg(options::OPT_ftime_trace) != nullptr;

It would be cleaner if all these logic can be moved to a separate function with 
a description.



Comment at: clang/lib/Driver/Driver.cpp:4753
+else
+  LinkingResParentPath = ".";
+  }

Do you expect link only show up once? If so, you can add a break here.



Comment at: clang/tools/driver/cc1_main.cpp:260
+SmallString<128> TracePath(Clang->getFrontendOpts().TimeTracePath);
 if (auto profilerOutput = Clang->createOutputFile(
+TracePath.str(), /*Binary=*/false, /*RemoveFileOnSignal=*/false,

Should we assert that TimeTracePath is not empty?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

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



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:4371
 
+TEST(TransferTest, ContextSensitiveConstructorBody) {
+  std::string Code = R"(

What about a default constructor, including when there are field initializers 
like:
```
 class MyClass {
public:
  MyClass() = default;

  bool MyField = true;
};
```

Should we expect to handle that correctly? If so, can you add some tests?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131438

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

[PATCH] D131469: [Clang] change default storing path of `-ftime-trace`

[Whitney Tsang via Phabricator via cfe-commits]

Whitney added inline comments.



Comment at: clang/lib/Driver/Driver.cpp:4786
+const std::string::size_type size = arg.size();
+char *buffer = new char[size + 1];
+memcpy(buffer, arg.c_str(), size + 1);

Assert size not 0.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131469

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

[PATCH] D131307: [Clang] Allow downgrading to a warning the diagnostic for setting a non fixed enum to a value outside the range of the enumeration values

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D131307#3709595 , @zatrazz wrote:

> This commit seems to have broken test-suite on aarch64 [1]. The warning shows:
>
>   FAILED: MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o
>   /home/adhemerval.zanella/projects/llvm/test-suite-build/tools/timeit 
> --summary MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o.time 
> /home/adhemerval.zanella/projects/llvm/llvm-src-build-stage1-buildbot/bin/clang++
>  -DNDEBUG  -O3 -DNDEBUG   -w -Werror=date-time -DNOASM -DLLVM -MD -MT 
> MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o -MF 
> MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o.d -o 
> MultiSource/Benchmarks/PAQ8p/CMakeFiles/paq8p.dir/paq8p.cpp.o -c 
> /home/adhemerval.zanella/projects/llvm/test-suite/MultiSource/Benchmarks/PAQ8p/paq8p.cpp
>   
> /home/adhemerval.zanella/projects/llvm/test-suite/MultiSource/Benchmarks/PAQ8p/paq8p.cpp:3606:24:
>  error: integer value -1 is outside the valid range of values [0, 15] for 
> this enumeration type [-Wenum-constexpr-conversion]
>   if (c==EOF) return (Filetype)(-1);
>  ^
>   1 error generated.
>
> I am not sure if this is an existing issue with test-suite.
>
> [1] https://lab.llvm.org/staging/#/builders/158/builds/344

That IS Undefined Behavior, but I think was unintended by this patch.  The 
intent of this patch (and its parent) was to diagnose this UB during 
`constexpr` evaluation.  HOWEVER, this patch seems to have extended it to 
non-`constexpr` constant evaluation.  So while that _IS_ undefined behavior, I 
don't think it should be covered by this error (likely a normal 'warning' is 
fine here, but not this diagnostic).  Hopefully @shafik can correct this when 
he starts again today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131307

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

[PATCH] D131319: [clang-tidy] Update llvm-prefer-isa-or-dyn-cast-in-conditionals with new syntax

[Whisperity via Phabricator via cfe-commits]

whisperity added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp:12-13
 
 template 
 bool isa(Y *);
 template 

Will the tests pass properly once the fixes are applied, even though the 
replaced code refers to a symbol (`isa_and_present`) that is not declared in 
the TU?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131319

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

[PATCH] D131319: [clang-tidy] Update llvm-prefer-isa-or-dyn-cast-in-conditionals with new syntax

[Whisperity via Phabricator via cfe-commits]

whisperity removed a reviewer: bzcheeseman.
whisperity added a comment.
This revision now requires review to proceed.

In D131319#3708667 , @bzcheeseman 
wrote:

> This is great, thank you for doing this! I'm not a competent reviewer for the 
> actual clang-tidy code changes but the +1 for the idea :)

The problem with the approval here is that a single approval will turn the 
patch into a fully approved state, which breaks the dashboards for people added 
to the patch (i.e., other reviewers will think the patch is already approved, 
and thus perhaps will not consider putting effort into reviewing it!).

However, I think you should try the //Award Token// option from the menu on the 
right! Somewhere the awarded tokens should show up on the patch, tallying up 
support!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131319

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

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

[Paul Scoropan via Phabricator via cfe-commits]

pscoro updated this revision to Diff 451133.
pscoro added a comment.

Small revisions


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129016

Files:
  clang/include/clang/Basic/BuiltinsPPC.def
  clang/lib/Basic/Targets/PPC.cpp
  clang/test/CodeGen/PowerPC/builtins-ppc-stackprotect.c
  llvm/include/llvm/IR/IntrinsicsPowerPC.td
  llvm/lib/Target/PowerPC/PPCISelLowering.cpp
  llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll

Index: llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
===
--- /dev/null
+++ llvm/test/CodeGen/PowerPC/kill-canary-intrinsic.ll
@@ -0,0 +1,206 @@
+; NOTE: Assertions have been autogenerated by utils/update_llc_test_checks.py
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-LINUX-LE
+
+; RUN: llc -verify-machineinstrs -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX64
+; RUN: llc -verify-machineinstrs -mtriple=powerpc64le-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr8 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-LINUX-LE
+
+; RUN: llc -verify-machineinstrs -O0 -mtriple=powerpc-unknown-aix -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-AIX-FAST
+; RUN: llc -verify-machineinstrs -O0 -mtriple=powerpc64-unknown-linux-gnu -ppc-vsr-nums-as-vr \
+; RUN:   -mcpu=pwr7 --ppc-asm-full-reg-names < %s | FileCheck %s -check-prefix=CHECK-FAST
+
+attributes #1 = { nounwind }
+declare void @llvm.ppc.kill.canary()
+define dso_local void @test_kill_canary() #1 {
+; CHECK-AIX-LABEL: test_kill_canary:
+; CHECK-AIX:   # %bb.0: # %entry
+; CHECK-AIX-NEXT:blr
+;
+; CHECK-LABEL: test_kill_canary:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:blr
+;
+; CHECK-AIX64-LABEL: test_kill_canary:
+; CHECK-AIX64:   # %bb.0: # %entry
+; CHECK-AIX64-NEXT:blr
+;
+; CHECK-LINUX-LE-LABEL: test_kill_canary:
+; CHECK-LINUX-LE:   # %bb.0: # %entry
+; CHECK-LINUX-LE-NEXT:blr
+;
+; CHECK-AIX-FAST-LABEL: test_kill_canary:
+; CHECK-AIX-FAST:   # %bb.0: # %entry
+; CHECK-AIX-FAST-NEXT:blr
+;
+; CHECK-FAST-LABEL: test_kill_canary:
+; CHECK-FAST:   # %bb.0: # %entry
+; CHECK-FAST-NEXT:blr
+entry:
+  call void @llvm.ppc.kill.canary()
+  ret void
+}
+
+attributes #0 = { sspreq }
+; Function Attrs: sspreq
+define dso_local void @test_kill_canary_ssp() #0 #1 {
+; CHECK-AIX-LABEL: test_kill_canary_ssp:
+; CHECK-AIX:   # %bb.0: # %entry
+; CHECK-AIX-NEXT:mflr r0
+; CHECK-AIX-NEXT:stw r0, 8(r1)
+; CHECK-AIX-NEXT:stwu r1, -64(r1)
+; CHECK-AIX-NEXT:lwz r3, L..C0(r2) # @__ssp_canary_word
+; CHECK-AIX-NEXT:lwz r4, 0(r3)
+; CHECK-AIX-NEXT:stw r4, 60(r1)
+; CHECK-AIX-NEXT:lwz r4, 0(r3)
+; CHECK-AIX-NEXT:not r4, r4
+; CHECK-AIX-NEXT:stw r4, 60(r1)
+; CHECK-AIX-NEXT:lwz r3, 0(r3)
+; CHECK-AIX-NEXT:lwz r4, 60(r1)
+; CHECK-AIX-NEXT:cmplw r3, r4
+; CHECK-AIX-NEXT:bne cr0, L..BB1_2
+; CHECK-AIX-NEXT:  # %bb.1: # %entry
+; CHECK-AIX-NEXT:addi r1, r1, 64
+; CHECK-AIX-NEXT:lwz r0, 8(r1)
+; CHECK-AIX-NEXT:mtlr r0
+; CHECK-AIX-NEXT:blr
+; CHECK-AIX-NEXT:  L..BB1_2: # %entry
+; CHECK-AIX-NEXT:bl .__stack_chk_fail[PR]
+; CHECK-AIX-NEXT:nop
+;
+; CHECK-LABEL: test_kill_canary_ssp:
+; CHECK:   # %bb.0: # %entry
+; CHECK-NEXT:mflr r0
+; CHECK-NEXT:std r0, 16(r1)
+; CHECK-NEXT:stdu r1, -128(r1)
+; CHECK-NEXT:ld r3, -28688(r13)
+; CHECK-NEXT:std r3, 120(r1)
+; CHECK-NEXT:ld r3, -28688(r13)
+; CHECK-NEXT:not r3, r3
+; CHECK-NEXT:std r3, 120(r1)
+; CHECK-NEXT:ld r3, 120(r1)
+; CHECK-NEXT:ld r4, -28688(r13)
+; CHECK-NEXT:cmpld r4, r3
+; CHECK-NEXT:bne cr0, .LBB1_2
+; CHECK-NEXT:  # %bb.1: # %entry
+; CHECK-NEXT:addi r1, r1, 128
+; CHECK-NEXT:ld r0, 16(r1)
+; CHECK-NEXT:mtlr r0
+; CHECK-NE

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

[Sam Estep via Phabricator via cfe-commits]

samestep added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:138
   ///
+  ///  `Call` must be either a `CallExpr` or a `CXXConstructExpr`.
+  ///

sgatev wrote:
> How about we define overloads that take these types instead of taking an 
> `Expr` here? This should remove the need for type-checking and guarding 
> against bad input in the implementation. `transferInlineCall` can be a 
> template if necessary.
Hmm I guess we could; is there much of a benefit to doing this templated?



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:678-684
+if (const auto *NonConstructExpr = dyn_cast(S)) {
+  // Note that it is important for the storage location of `S` to be set
+  // before `pushCall`, because the latter uses it to set the storage
+  // location for `return`.
+  auto &ReturnLoc = Env.createStorageLocation(*S);
+  Env.setStorageLocation(*S, ReturnLoc);
+}

ymandel wrote:
> Why can't this stay in `VisitCallExpr`?
I had it there in an earlier version of this patch; it was causing tests to 
fail (the `SelfReferential*` ones, if I remember correctly).



Comment at: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp:4371
 
+TEST(TransferTest, ContextSensitiveConstructorBody) {
+  std::string Code = R"(

ymandel wrote:
> What about a default constructor, including when there are field initializers 
> like:
> ```
>  class MyClass {
> public:
>   MyClass() = default;
> 
>   bool MyField = true;
> };
> ```
> 
> Should we expect to handle that correctly? If so, can you add some tests?
Good idea! I'll add a test for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131438

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

[PATCH] D131438: [clang][dataflow] Analyze constructor bodies

[Sam Estep via Phabricator via cfe-commits]

samestep updated this revision to Diff 451134.
samestep added a comment.

Address Yitzie's comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131438

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -4368,4 +4368,106 @@
/*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
 }
 
+TEST(TransferTest, ContextSensitiveConstructorBody) {
+  std::string Code = R"(
+class MyClass {
+public:
+  MyClass() { MyField = true; }
+
+  bool MyField;
+};
+
+void target() {
+  MyClass MyObj;
+  bool Foo = MyObj.MyField;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+auto &FooVal =
+*cast(Env.getValue(*FooDecl, SkipPast::None));
+EXPECT_TRUE(Env.flowConditionImplies(FooVal));
+  },
+  {/*.ApplyBuiltinTransfer=*/true,
+   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
+TEST(TransferTest, ContextSensitiveConstructorInitializer) {
+  std::string Code = R"(
+class MyClass {
+public:
+  MyClass() : MyField(true) {}
+
+  bool MyField;
+};
+
+void target() {
+  MyClass MyObj;
+  bool Foo = MyObj.MyField;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+auto &FooVal =
+*cast(Env.getValue(*FooDecl, SkipPast::None));
+EXPECT_TRUE(Env.flowConditionImplies(FooVal));
+  },
+  {/*.ApplyBuiltinTransfer=*/true,
+   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
+TEST(TransferTest, ContextSensitiveConstructorDefault) {
+  std::string Code = R"(
+class MyClass {
+public:
+  MyClass() = default;
+
+  bool MyField = true;
+};
+
+void target() {
+  MyClass MyObj;
+  bool Foo = MyObj.MyField;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+auto &FooVal =
+*cast(Env.getValue(*FooDecl, SkipPast::None));
+EXPECT_TRUE(Env.flowConditionImplies(FooVal));
+  },
+  {/*.ApplyBuiltinTransfer=*/true,
+   /*.BuiltinTransferOptions=*/{/*.ContextSensitive=*/true}});
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -444,6 +444,8 @@
 Env.setStorageLocation(*S, Loc);
 if (Value *Val = Env.createValue(S->getType()))
   Env.setValue(Loc, *Val);
+
+transferInlineCall(S, ConstructorDecl);
   }
 
   void VisitCXXOperatorCallExpr(const CXXOperatorCallExpr *S) {
@@ -526,45 +528,7 @@
 return;
   Env.setStorageLocation(*S, *ArgLoc);
 } else if (const FunctionDecl *F = S->getDirectCallee()) {
-  // This case is for context-sensitive analysis.
-  if (!Options.ContextSensitive)
-return;
-
-  const ControlFlowContext *CFCtx = Env.getControlFlowContext(F);
-  if (!CFCtx)
-return;
-
-  // FIXME: We don't support context-sensitive analysis of recursion, so
-  // we should return early here if `F` is the same as the `FunctionDecl`
-  // holding `S` itself.
-
-  auto ExitBlock = CFCtx->getCF

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130689#3706424 , @aaron.ballman 
wrote:

> In D130689#3706377 , @thieta wrote:
>
>> In D130689#3706336 , 
>> @aaron.ballman wrote:
>>
>>> That's the only reason this hasn't been reverted already. Landing sweeping 
>>> changes on a weekend is a good way to reduce the pain, but we really need 
>>> to be sure someone watches the build lab and reacts when subsequent changes 
>>> break everything like this.
>>
>> Agreed, I think we need to update the protocol for changing the C++ standard 
>> in the future to account for more testing beforehand. I might push some 
>> changes to the policy document when all this has settled down to see if we 
>> can make sure it will be smoother the time we move to C++20. It's 
>> unfortunate that some stuff broke considering we where running some bots 
>> before it was merged and it didn't show any errors. And local windows builds 
>> for me have been clean as well.
>
> +1, thank you for thinking about how we can improve this process in the 
> future! Given that C++17 adoption across compilers has been far better than 
> C++20, I suspect the next time we bump the language version will be even more 
> of a challenge with these sort of weird issues.

One thing I think would be a definite improvement is to have done an RFC on 
Discourse for these changes so that downstreams have a chance to weigh in on 
the impact. The patch was put up on Jul 28 and landed about a week later 
without any notification to the rest of the community who might not be watching 
cfe-commits -- that's a very fast turnaround and very little notification for 
such a significant change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D131464: [test] Make tests pass regardless of gnu++14/gnu++17 default

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/AST/ast-dump-openmp-begin-declare-variant_11.c:2
 // RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=c_mode   
-ast-dump %s   | FileCheck %s --check-prefix=C
-// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=cxx_mode 
-ast-dump %s -x c++| FileCheck %s --check-prefix=CXX
+// RUN: %clang_cc1 -triple x86_64-unknown-unknown -fopenmp -verify=cxx_mode 
-ast-dump %s -x c++ -std=c++14 | FileCheck %s --check-prefix=CXX
 

I'm not really keen on this sort of change because it loses test coverage for 
other language standard versions. We usually try to write our tests to be 
standard version agnostic and only specify a specific language mode only when 
absolutely necessary, which doesn't seem to be the case for a lot of the tests 
in this patch (like this one).



Comment at: clang/test/CXX/basic/basic.stc/basic.stc.dynamic/p2.cpp:3
 // RUN: %clang_cc1 -fsyntax-only -fexceptions -fcxx-exceptions -verify 
-std=c++11 %s
-// RUN: %clang_cc1 -fsyntax-only -fexceptions -fcxx-exceptions -verify %s
+// RUN: %clang_cc1 -fsyntax-only -fexceptions -fcxx-exceptions -verify 
-std=c++14 %s
 int *use_new(int N) {

This is another example where we lose all testing in the future when we go to 
change the default language standard version. It's fine to have a 
C++14-specific test, but we should still have a test which has no mode 
specified.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131464

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

[PATCH] D130894: [clang] Print more information about failed static assertions

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

tbaeder updated this revision to Diff 451143.

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

https://reviews.llvm.org/D130894

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/CXX/dcl.decl/dcl.meaning/dcl.array/p3.cpp
  clang/test/CXX/drs/dr7xx.cpp
  clang/test/CXX/temp/temp.decls/temp.variadic/init-capture.cpp
  clang/test/Lexer/cxx1z-trigraphs.cpp
  clang/test/PCH/cxx-templates.cpp
  clang/test/Parser/objc-static-assert.mm
  clang/test/Sema/static-assert.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/recovery-expr-type.cpp
  clang/test/SemaCXX/static-assert-cxx17.cpp
  clang/test/SemaCXX/static-assert.cpp
  clang/test/SemaTemplate/instantiate-var-template.cpp
  clang/test/SemaTemplate/instantiation-dependence.cpp
  clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp

Index: clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
===
--- clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
+++ clang/test/SemaTemplate/temp_arg_nontype_cxx20.cpp
@@ -184,7 +184,8 @@
 
 namespace Diags {
   struct A { int n, m; };
-  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}}
+  template struct X { static_assert(a.n == a.m); }; // expected-error {{static assertion failed due to requirement 'Diags::A{1, 2}.n == Diags::A{1, 2}.m'}} \
+ // expected-note {{evaluates to '1 == 2'}}
   template struct X; // expected-note {{in instantiation of template class 'Diags::X<{1, 2}>' requested here}}
 }
 
Index: clang/test/SemaTemplate/instantiation-dependence.cpp
===
--- clang/test/SemaTemplate/instantiation-dependence.cpp
+++ clang/test/SemaTemplate/instantiation-dependence.cpp
@@ -68,8 +68,10 @@
   struct D : B, C {};
 
   static_assert(trait::specialization == 0);
-  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}}
-  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}}
+  static_assert(trait::specialization == 1); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to '0 == 1'}}
+  static_assert(trait::specialization == 2); // FIXME expected-error {{failed}} \
+// expected-note {{evaluates to '0 == 2'}}
   static_assert(trait::specialization == 0); // FIXME-error {{ambiguous partial specialization}}
 }
 
Index: clang/test/SemaTemplate/instantiate-var-template.cpp
===
--- clang/test/SemaTemplate/instantiate-var-template.cpp
+++ clang/test/SemaTemplate/instantiate-var-template.cpp
@@ -31,7 +31,8 @@
   static_assert(b == 1, ""); // expected-note {{in instantiation of}} expected-error {{not an integral constant}}
 
   template void f() {
-static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}}
+static_assert(a == 0, ""); // expected-error {{static assertion failed due to requirement 'a == 0'}} \
+   // expected-note {{evaluates to '1 == 0'}}
   }
 }
 
Index: clang/test/SemaCXX/static-assert.cpp
===
--- clang/test/SemaCXX/static-assert.cpp
+++ clang/test/SemaCXX/static-assert.cpp
@@ -22,7 +22,8 @@
 T<2> t2;
 
 template struct S {
-static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}}
+static_assert(sizeof(T) > sizeof(char), "Type not big enough!"); // expected-error {{static assertion failed due to requirement 'sizeof(char) > sizeof(char)': Type not big enough!}} \
+ // expected-note {{1 > 1}}
 };
 
 S s1; // expected-note {{in instantiation of template class 'S' requested here}}
@@ -215,3 +216,51 @@
 static_assert(constexprNotBool, "message"); // expected-error {{value of type 'const NotBool' is not contextually convertible to 'bool'}}
 
 static_assert(1 , "") // expected-error {{expected ';' after 'static_assert'}}
+
+
+namespace Diagnostics {
+  /// No notes for literals.
+  static_assert(false, ""); // expected-error {{failed}}
+  static_assert(1.0 > 2.0, ""); // expected-error {{failed}}
+  static_assert('c' == 'd', ""); // expected-error {{failed}}
+  static_assert(1 == 2, ""); // expected-error {{failed}}
+
+  /// Simple things are ignored.
+  static_assert(1 == (-(1)), ""); //expected-error {{failed}}
+
+  /// Chars are printed as chars.
+  constexpr char getChar() {
+return 'c';
+  }
+  stati

[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 451144.
ymandel marked an inline comment as done.
ymandel added a comment.

respond to comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131280

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -11,12 +11,15 @@
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/LangStandard.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Testing/Support/Error.h"
@@ -40,11 +43,39 @@
 template 
 void runDataflow(llvm::StringRef Code, Matcher Match,
  DataflowAnalysisOptions Options,
+ llvm::StringRef ModeledFunctionsCode = {},
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  llvm::StringRef TargetFun = "target") {
+  const std::vector Args = {
+  "-fsyntax-only", "-fno-delayed-template-parsing",
+  "-std=" +
+  std::string(LangStandard::getLangStandardForKind(Std).getName())};
+
+  auto MainASTUnit = tooling::buildASTFromCodeWithArgs(Code, Args);
+  ASSERT_NE(MainASTUnit, nullptr);
+
+  // The unique_ptr has to live outside the if-body since the lifetime of the
+  // function map is linked to the lifetime of the ASTUnit. The raw pointer
+  // allows us to select between the main TU and the (optional) model TU.
+  std::unique_ptr ModelASTUnit = nullptr;
+  ASTUnit *UnitPtr = nullptr;
+  if (ModeledFunctionsCode.empty()) {
+UnitPtr = MainASTUnit.get();
+  } else {
+ModelASTUnit =
+tooling::buildASTFromCodeWithArgs(ModeledFunctionsCode, Args);
+ASSERT_NE(ModelASTUnit, nullptr);
+UnitPtr = ModelASTUnit.get();
+  }
+
+  llvm::Expected> Result =
+  buildFunctionMapFromAST(*UnitPtr);
+  ASSERT_TRUE((bool)Result);
+  llvm::StringMap AnalyzableFunctions = std::move(*Result);
+
   ASSERT_THAT_ERROR(
   test::checkDataflow(
-  Code, TargetFun,
+  Code, std::move(MainASTUnit), ast_matchers::hasName(TargetFun),
   [Options](ASTContext &C, Environment &) {
 return NoopAnalysis(C, Options);
   },
@@ -53,18 +84,18 @@
   std::pair>>
   Results,
   ASTContext &ASTCtx) { Match(Results, ASTCtx); },
-  {"-fsyntax-only", "-fno-delayed-template-parsing",
-   "-std=" + std::string(
- LangStandard::getLangStandardForKind(Std).getName())}),
+  std::move(AnalyzableFunctions)),
   llvm::Succeeded());
 }
 
+// FIXME: inline this overload and remove it.
 template 
 void runDataflow(llvm::StringRef Code, Matcher Match,
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match, {ApplyBuiltinTransfer, {}}, Std, TargetFun);
+  runDataflow(Code, Match, /*Options=*/{ApplyBuiltinTransfer, {}},
+  /*AnalyzableFunctions=*/{}, Std, TargetFun);
 }
 
 TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) {
@@ -1320,9 +1351,19 @@
   }
 };
   )";
+
+  auto ASTUnit = tooling::buildASTFromCodeWithArgs(
+  Code,
+  /*Args=*/
+  {"-fsyntax-only", "-fno-delayed-template-parsing",
+   "-std=" + std::string(LangStandard::getLangStandardForKind(
+ LangStandard::lang_cxx17)
+ .getName())},
+  "input.cc", "clang-dataflow-test");
+
   ASSERT_THAT_ERROR(
   test::checkDataflow(
-  Code, cxxConstructorDecl(ofClass(hasName("B"))),
+  Code, std::move(ASTUnit), cxxConstructorDecl(ofClass(hasName("B"))),
   [](ASTContext &C, Environment &) {
 return No

[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 451145.
ymandel marked 9 inline comments as done.
ymandel added a comment.

fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131280

Files:
  clang/include/clang/Analysis/FlowSensitive/ControlFlowContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowAnalysisContext.h
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowAnalysisContext.cpp
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TestingSupport.h
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -11,12 +11,15 @@
 #include "clang/AST/Decl.h"
 #include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/ASTMatchers/ASTMatchers.h"
+#include "clang/Analysis/FlowSensitive/ControlFlowContext.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/NoopAnalysis.h"
 #include "clang/Analysis/FlowSensitive/StorageLocation.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
 #include "clang/Basic/LangStandard.h"
+#include "clang/Tooling/Tooling.h"
 #include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Casting.h"
 #include "llvm/Testing/Support/Error.h"
@@ -40,11 +43,39 @@
 template 
 void runDataflow(llvm::StringRef Code, Matcher Match,
  DataflowAnalysisOptions Options,
+ llvm::StringRef ModeledFunctionsCode = {},
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  llvm::StringRef TargetFun = "target") {
+  const std::vector Args = {
+  "-fsyntax-only", "-fno-delayed-template-parsing",
+  "-std=" +
+  std::string(LangStandard::getLangStandardForKind(Std).getName())};
+
+  auto MainASTUnit = tooling::buildASTFromCodeWithArgs(Code, Args);
+  ASSERT_NE(MainASTUnit, nullptr);
+
+  // The unique_ptr has to live outside the if-body since the lifetime of the
+  // function map is linked to the lifetime of the ASTUnit. The raw pointer
+  // allows us to select between the main TU and the (optional) model TU.
+  std::unique_ptr ModelASTUnit = nullptr;
+  ASTUnit *UnitPtr = nullptr;
+  if (ModeledFunctionsCode.empty()) {
+UnitPtr = MainASTUnit.get();
+  } else {
+ModelASTUnit =
+tooling::buildASTFromCodeWithArgs(ModeledFunctionsCode, Args);
+ASSERT_NE(ModelASTUnit, nullptr);
+UnitPtr = ModelASTUnit.get();
+  }
+
+  llvm::Expected> Result =
+  buildFunctionMapFromAST(*UnitPtr);
+  ASSERT_TRUE((bool)Result);
+  llvm::StringMap AnalyzableFunctions = std::move(*Result);
+
   ASSERT_THAT_ERROR(
   test::checkDataflow(
-  Code, TargetFun,
+  Code, std::move(MainASTUnit), ast_matchers::hasName(TargetFun),
   [Options](ASTContext &C, Environment &) {
 return NoopAnalysis(C, Options);
   },
@@ -53,18 +84,18 @@
   std::pair>>
   Results,
   ASTContext &ASTCtx) { Match(Results, ASTCtx); },
-  {"-fsyntax-only", "-fno-delayed-template-parsing",
-   "-std=" + std::string(
- LangStandard::getLangStandardForKind(Std).getName())}),
+  std::move(AnalyzableFunctions)),
   llvm::Succeeded());
 }
 
+// FIXME: inline this overload and remove it.
 template 
 void runDataflow(llvm::StringRef Code, Matcher Match,
  LangStandard::Kind Std = LangStandard::lang_cxx17,
  bool ApplyBuiltinTransfer = true,
  llvm::StringRef TargetFun = "target") {
-  runDataflow(Code, Match, {ApplyBuiltinTransfer, {}}, Std, TargetFun);
+  runDataflow(Code, Match, /*Options=*/{ApplyBuiltinTransfer, {}},
+  /*AnalyzableFunctions=*/{}, Std, TargetFun);
 }
 
 TEST(TransferTest, IntVarDeclNotTrackedWhenTransferDisabled) {
@@ -1320,9 +1351,19 @@
   }
 };
   )";
+
+  auto ASTUnit = tooling::buildASTFromCodeWithArgs(
+  Code,
+  /*Args=*/
+  {"-fsyntax-only", "-fno-delayed-template-parsing",
+   "-std=" + std::string(LangStandard::getLangStandardForKind(
+ LangStandard::lang_cxx17)
+ .getName())},
+  "input.cc", "clang-dataflow-test");
+
   ASSERT_THAT_ERROR(
   test::checkDataflow(
-  Code, cxxConstructorDecl(ofClass(hasName("B"))),
+  Code, std::move(ASTUnit), cxxConstructorDecl(ofClass(hasName("B"))),
   [](ASTContext &C, Environment &) {
 return NoopAnalysis(C

[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D131280#3706988 , @xazax.hun wrote:

> In D131280#3706915 , @ymandel wrote:
>
>> Sure. This is probably worth some discussion. Fully qualified names, however 
>> we define them, will not be enough, since they don't cover overload sets.
>
> I see. This is not a unique problem. I think there were multiple discussions 
> about API Notes and those need to solve the same problem. @gribozavr2 
> probably has more context on the current status of API Notes. An alternative 
> to fully qualified names is Clang's USR that is often used for 
> cross-referencing functions across translation units. Less user friendly, but 
> will support overloads.
>
>> I'd like some mechanism that matches how identifiers are used. So, for 
>> example, inline namespaces should *not* be necessary, since they are an 
>> implementation detail from this perspective.
>
> A similar matching is already implemented for the Clang Static Analyzer. See 
> https://clang.llvm.org/doxygen/classclang_1_1ento_1_1CallDescription.html and 
> https://clang.llvm.org/doxygen/classclang_1_1ento_1_1CallDescriptionMap.html
>
> One example use is in the CStringChecker: 
> https://github.com/llvm/llvm-project/blob/main/clang/lib/StaticAnalyzer/Checkers/CStringChecker.cpp#L136

Thanks. That looks good, but I'm concerned that it only counts the arguments 
and doesn't look at their types. I'd imagine this will be a limitation down the 
line when we want to deal with overload sets w/ the same number of arguments, 
but different types.

Aside: why the `const char *` interface? Do you think owners would be open to a 
`llvm::StringRef` overload for the constructor?




Comment at: clang/lib/Analysis/FlowSensitive/ControlFlowContext.cpp:96
+  for (auto It = Unit.top_level_begin(); It != Unit.top_level_end(); ++It) {
+if (auto *C = dyn_cast(*It)) {
+  for (auto *M : C->methods())

xazax.hun wrote:
> ymandel wrote:
> > xazax.hun wrote:
> > > Do we exclude non-toplevel declarations on purpuse? Or would this work 
> > > for methods of inline classes, methods of classes defined within a 
> > > function? 
> > > Do we exclude non-toplevel declarations on purpuse? Or would this work 
> > > for methods of inline classes, methods of classes defined within a 
> > > function? 
> > 
> > I think that for the current use case -- models of library types and their 
> > methods/functions -- we don't have a good usecase for this. But, I can see 
> > this becoming an issue if we want to expand to inlining other declarations. 
> > So, I'm inclined to hold off on this for the time being, since that's a 
> > larger design discussion.
> > 
> > Yet, I also think this should be generalized to take any decl and extract 
> > the functions/methods. I limited to `ASTUnit` for convenience, since that 
> > was the immediate need. I'm happy to either:
> > 1. Add a FIXME, and/or,
> > 2. Split this function into two: one that takes two decl iterators (begin, 
> > end) and does this work here and another which is just a convenience 
> > function for ASTUnit to apply the above to the top-level decls.
> > 
> > WDYT?
> I am fine with the current behavior, but I think the docstring "Builds a map 
> of all declared functions in the given AST" is misleading in this case. 
> Specifying in the docstring that this function will only map the top-level 
> functions and methods of top-level classes sounds good to me.
Thanks, done.



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:549
 
-  // Note that it is important for the storage location of `S` to be set
-  // before `pushCall`, because the latter uses it to set the storage
-  // location for `return`.
-  auto &ReturnLoc = Env.createStorageLocation(*S);
-  Env.setStorageLocation(*S, ReturnLoc);
-  auto CalleeEnv = Env.pushCall(S);
+  const FunctionDecl *FuncDecl = CFCtx->getDecl()->getAsFunction();
+  assert(FuncDecl != nullptr && "ControlFlowContexts in the environment "

sgatev wrote:
> How is that different from `F`? Why not let `Environment::pushCall` get this 
> from the `CallExpr` argument?
Added comment to explain.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131280

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

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

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

tbaeder added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5445
+  }
+}
+

This function copies the two conditions for the if-statements from 
`evaluateTypeTrait()`. Is just doing it in `evaluateTypeTrait()` too late? 
Duplicating that seems bad.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

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

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5445
+  }
+}
+

tbaeder wrote:
> This function copies the two conditions for the if-statements from 
> `evaluateTypeTrait()`. Is just doing it in `evaluateTypeTrait()` too late? 
> Duplicating that seems bad.
`evaluateTypeTrait()` should return evaluate result, i.e. bool value `true` / 
`false`, if we do this in `evaluateTypeTrait()` , how can we express that we 
encountered some error? I think it is reasonable to split these conditions here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

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

[PATCH] D91000: [clang-tidy] Add bugprone-unsafe-functions checker.

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

balazske added a comment.

Another functions that can be added to the list: `atoi()`, `atol()`, `atoll()`, 
`atof()`. These are unsafe according to 
https://wiki.sei.cmu.edu/confluence/display/c/ERR34-C.+Detect+errors+when+converting+a+string+to+a+number.


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

https://reviews.llvm.org/D91000

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Tobias Hieta via Phabricator via cfe-commits]

thieta added a comment.

In D130689#3709742 , @aaron.ballman 
wrote:

> One thing I think would be a definite improvement is to have done an RFC on 
> Discourse for these changes so that downstreams have a chance to weigh in on 
> the impact. The patch was put up on Jul 28 and landed about a week later 
> without any notification to the rest of the community who might not be 
> watching cfe-commits -- that's a very fast turnaround and very little 
> notification for such a significant change.

Yeah this is on me. Honestly I didn't expect it to be that much of a problem 
but rather the toolchain requirement we posted as part of it would be the big 
hurdle where bot owners would have to upgrade to get the right versions. But 
lesson learned  and we should add some more delays in the policy here: 
https://llvm.org/docs/DeveloperPolicy.html#id23 and cover the C++ standards 
upgrade.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D129016: [PowerPC] implemented @llvm.ppc.kill.canary to corrupt stack guard

[Nemanja Ivanovic via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129016

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

[PATCH] D131423: [clang] fix frontend crash when evaluating type trait

[YingChi Long via Phabricator via cfe-commits]

inclyc added inline comments.



Comment at: clang/lib/Sema/SemaExprCXX.cpp:5445
+  }
+}
+

inclyc wrote:
> tbaeder wrote:
> > This function copies the two conditions for the if-statements from 
> > `evaluateTypeTrait()`. Is just doing it in `evaluateTypeTrait()` too late? 
> > Duplicating that seems bad.
> `evaluateTypeTrait()` should return evaluate result, i.e. bool value `true` / 
> `false`, if we do this in `evaluateTypeTrait()` , how can we express that we 
> encountered some error? I think it is reasonable to split these conditions 
> here.
Do you think we should add a condition checks to the evaluate function 
`evaluateTypeTrait()` and return `false` if it is not satisfied?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131423

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

[PATCH] D131499: Change prototype merging error into a warning for builtins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: efriedma, jyknight, clang-language-wg.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

As was observed in https://reviews.llvm.org/D123627#3707635, it's confusing 
that a user can write:

  float rintf(void) {}

and get a warning, but writing:

  float rintf() {}

gives an error. This patch changes the behavior so that both are warnings, so 
that users who have functions which conflict with a builtin identifier can 
still use that identifier as they wish.

Note, if we accept this, I would like to backport it to the Clang 15.x branch 
because the original change was made in Clang 15.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D131499

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/prototype-redecls.c


Index: clang/test/Sema/prototype-redecls.c
===
--- clang/test/Sema/prototype-redecls.c
+++ clang/test/Sema/prototype-redecls.c
@@ -29,7 +29,7 @@
 
 // Ensure redeclarations that conflict with a builtin use a note which makes it
 // clear that the previous declaration was a builtin.
-float rintf() { // expected-error {{conflicting types for 'rintf'}} \
+float rintf() { // expected-warning {{incompatible redeclaration of library 
function 'rintf'}} \
expected-note {{'rintf' is a builtin with type 'float 
(float)'}}
   return 1.0f;
 }
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4042,7 +4042,7 @@
 // default argument promotion rules were already checked by
 // ASTContext::typesAreCompatible().
 if (Old->hasPrototype() && !New->hasWrittenPrototype() && NewDeclIsDefn &&
-Old->getNumParams() != New->getNumParams()) {
+Old->getNumParams() != New->getNumParams() && !Old->isImplicit()) {
   if (Old->hasInheritedPrototype())
 Old = Old->getCanonicalDecl();
   Diag(New->getLocation(), diag::err_conflicting_types) << New;


Index: clang/test/Sema/prototype-redecls.c
===
--- clang/test/Sema/prototype-redecls.c
+++ clang/test/Sema/prototype-redecls.c
@@ -29,7 +29,7 @@
 
 // Ensure redeclarations that conflict with a builtin use a note which makes it
 // clear that the previous declaration was a builtin.
-float rintf() { // expected-error {{conflicting types for 'rintf'}} \
+float rintf() { // expected-warning {{incompatible redeclaration of library function 'rintf'}} \
expected-note {{'rintf' is a builtin with type 'float (float)'}}
   return 1.0f;
 }
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4042,7 +4042,7 @@
 // default argument promotion rules were already checked by
 // ASTContext::typesAreCompatible().
 if (Old->hasPrototype() && !New->hasWrittenPrototype() && NewDeclIsDefn &&
-Old->getNumParams() != New->getNumParams()) {
+Old->getNumParams() != New->getNumParams() && !Old->isImplicit()) {
   if (Old->hasInheritedPrototype())
 Old = Old->getCanonicalDecl();
   Diag(New->getLocation(), diag::err_conflicting_types) << New;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123627: Correctly diagnose prototype redeclaration errors in C

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D123627#3707789 , @aaron.ballman 
wrote:

> In D123627#3707635 , @efriedma 
> wrote:
>
>> If the declaration we're redeclaring is a builtin, should the diagnostic be 
>> in the "-Wincompatible-library-redeclaration" warning group?  With this 
>> patch, we treat redefinitions of builtins without a prototype differently 
>> from redefinitions with a prototype, for example:
>>
>>   void acos() {} // error
>>   void acos(void) {} // warning
>>
>> Just ran into some code in Android which is using the first form.
>
> Er, I keep going back and forth on it. My initial inclination is that this is 
> closing a hole where we would incorrectly decide that these function 
> signatures are compatible enough to merge together when that's not the case, 
> so an error is appropriate. But the same can be said for redeclaring a 
> builtin with an incorrect prototype rather than declaring it without any 
> prototype. Given that these builtins are declared for the user automagically, 
> I think I come down on this being a case we'd rather warn instead of err. 
> It'd be weird to allow the user to define `void printf(void) {}` but not 
> `void printf();` (except in C2x mode).
>
> If we make a change here, I think it'd be good to get it done for Clang 15. 
> I'm not certain I've got the bandwidth to make this change in that timeframe 
> though (I can hopefully start on this sometime this week, but I have prior 
> commitments with deadlines that take priority).

I filed https://reviews.llvm.org/D131499 for this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123627

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

[PATCH] D131319: [clang-tidy] Update llvm-prefer-isa-or-dyn-cast-in-conditionals with new syntax

[Aman LaChapelle via Phabricator via cfe-commits]

bzcheeseman added a comment.

In D131319#3709671 , @whisperity 
wrote:

> In D131319#3708667 , @bzcheeseman 
> wrote:
>
>> This is great, thank you for doing this! I'm not a competent reviewer for 
>> the actual clang-tidy code changes but the +1 for the idea :)
>
> The problem with the approval here is that a single approval will turn the 
> patch into a fully approved state, which breaks the dashboards for people 
> added to the patch (i.e., other reviewers will think the patch is already 
> approved, and thus perhaps will not consider putting effort into reviewing 
> it!).
>
> However, I think you should try the //Award Token// option from the menu on 
> the right! Somewhere the awarded tokens should show up on the patch, tallying 
> up support!

Ah I had no idea, thanks for pointing that out. I was looking at the Code 
Review document (https://llvm.org/docs/CodeReview.html), I'll put up a patch to 
add a short section on this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131319

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

[clang] f53f2f2 - Extend ptr32 support to be applied on typedef

[Muiez Ahmed via cfe-commits]

Author: Ariel Burton
Date: 2022-08-09T11:08:52-04:00
New Revision: f53f2f232f794a257c270f4c273b9c9000421c81

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

LOG: Extend ptr32 support to be applied on typedef

Earlier, if the QualType was sugared, then we would error out
as it was not a pointer type, for example,

typedef int *int_star;

int_star __ptr32 p;

Now, if ptr32 is given we apply it if the raw Canonical Type
(i.e., the desugared type) is a PointerType, instead of only
checking whether the sugared type is a pointer type.

As before, we still disallow ptr32 usage if the pointer is used
as a pointer to a member.

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

Added: 


Modified: 
clang/lib/Sema/SemaType.cpp
clang/test/CodeGen/address-space-ptr32.c
clang/test/Sema/MicrosoftExtensions.c
clang/test/Sema/MicrosoftExtensions.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaType.cpp b/clang/lib/Sema/SemaType.cpp
index 11a6715cd9d21..14b2d2e74f3f2 100644
--- a/clang/lib/Sema/SemaType.cpp
+++ b/clang/lib/Sema/SemaType.cpp
@@ -7158,17 +7158,25 @@ static bool 
handleMSPointerTypeQualifierAttr(TypeProcessingState &State,
   }
 
   std::bitset Attrs;
-  attr::Kind NewAttrKind = A->getKind();
   QualType Desugared = Type;
-  const AttributedType *AT = dyn_cast(Type);
-  while (AT) {
+  for (;;) {
+if (const TypedefType *TT = dyn_cast(Desugared)) {
+  Desugared = TT->desugar();
+  continue;
+} else if (const ElaboratedType *ET = dyn_cast(Desugared)) 
{
+  Desugared = ET->desugar();
+  continue;
+}
+const AttributedType *AT = dyn_cast(Desugared);
+if (!AT)
+  break;
 Attrs[AT->getAttrKind()] = true;
 Desugared = AT->getModifiedType();
-AT = dyn_cast(Desugared);
   }
 
   // You cannot specify duplicate type attributes, so if the attribute has
   // already been applied, flag it.
+  attr::Kind NewAttrKind = A->getKind();
   if (Attrs[NewAttrKind]) {
 S.Diag(PAttr.getLoc(), diag::warn_duplicate_attribute_exact) << PAttr;
 return true;
@@ -7189,14 +7197,11 @@ static bool 
handleMSPointerTypeQualifierAttr(TypeProcessingState &State,
 return true;
   }
 
-  // Pointer type qualifiers can only operate on pointer types, but not
-  // pointer-to-member types.
-  //
-  // FIXME: Should we really be disallowing this attribute if there is any
-  // type sugar between it and the pointer (other than attributes)? Eg, this
-  // disallows the attribute on a parenthesized pointer.
-  // And if so, should we really allow *any* type attribute?
+  // Check the raw (i.e., desugared) Canonical type to see if it
+  // is a pointer type.
   if (!isa(Desugared)) {
+// Pointer type qualifiers can only operate on pointer types, but not
+// pointer-to-member types.
 if (Type->isMemberPointerType())
   S.Diag(PAttr.getLoc(), diag::err_attribute_no_member_pointers) << PAttr;
 else

diff  --git a/clang/test/CodeGen/address-space-ptr32.c 
b/clang/test/CodeGen/address-space-ptr32.c
index a2914e39bee4d..618db6906351d 100644
--- a/clang/test/CodeGen/address-space-ptr32.c
+++ b/clang/test/CodeGen/address-space-ptr32.c
@@ -1,10 +1,40 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-windows-msvc 
-fms-extensions -emit-llvm < %s | FileCheck %s
 
+_Static_assert(sizeof(void *) == 8, "sizeof(void *) has unexpected value.  
Expected 8.");
+
 int foo(void) {
+  // CHECK: define dso_local i32 @foo
+  // CHECK: %a = alloca i32 (i32) addrspace(270)*, align 4
+  // CHECK: ret i32 4
   int (*__ptr32 a)(int);
   return sizeof(a);
 }
 
-// CHECK: define dso_local i32 @foo
-// CHECK: %a = alloca i32 (i32) addrspace(270)*, align 4
-// CHECK: ret i32 4
+int bar(void) {
+  // CHECK: define dso_local i32 @bar
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  int *__ptr32 p;
+  return sizeof(p);
+}
+
+
+int baz(void) {
+  // CHECK: define dso_local i32 @baz
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  typedef int *__ptr32 IP32_PTR;
+
+  IP32_PTR p;
+  return sizeof(p);
+}
+
+int fugu(void) {
+  // CHECK: define dso_local i32 @fugu
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  typedef int *int_star;
+
+  int_star __ptr32 p;
+  return sizeof(p);
+}

diff  --git a/clang/test/Sema/MicrosoftExtensions.c 
b/clang/test/Sema/MicrosoftExtensions.c
index d0ca16269cceb..50077d9031488 100644
--- a/clang/test/Sema/MicrosoftExtensions.c
+++ b/clang/test/Sema/MicrosoftExtensions.c
@@ -173,8 +173,28 @@ int * __ptr32 __ptr32 wrong8;  // expected-warning 
{{attribute '__ptr32' is alrea
 
 int *(__ptr32 __sptr wrong9); // expected-error {{'__sptr' attribute only 
applies to pointer arguments}} // expected-error {{'__ptr32' attribute only 
ap

[PATCH] D130123: Extend ptr32 support to be applied on typedef

[Muiez Ahmed via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf53f2f232f79: Extend ptr32 support to be applied on typedef 
(authored by Ariel-Burton, committed by muiez).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130123

Files:
  clang/lib/Sema/SemaType.cpp
  clang/test/CodeGen/address-space-ptr32.c
  clang/test/Sema/MicrosoftExtensions.c
  clang/test/Sema/MicrosoftExtensions.cpp

Index: clang/test/Sema/MicrosoftExtensions.cpp
===
--- clang/test/Sema/MicrosoftExtensions.cpp
+++ clang/test/Sema/MicrosoftExtensions.cpp
@@ -1,6 +1,5 @@
 // RUN: %clang_cc1 -triple i686-windows %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
 // RUN: %clang_cc1 -triple x86_64-windows %s -fsyntax-only -Wmicrosoft -verify -fms-extensions
-// expected-no-diagnostics
 
 // Check that __ptr32/__ptr64 can be compared.
 int test_ptr_comparison(int *__ptr32 __uptr p32u, int *__ptr32 __sptr p32s,
@@ -9,3 +8,23 @@
  (p32u == p64) +
  (p32s == p64);
 }
+
+template
+void bad(T __ptr32 a) { // expected-error {{'__ptr32' attribute only applies to pointer arguments}}`
+  (*a) += 1;
+}
+
+template
+void f(T a) {
+  (*a) += sizeof(a);
+  static_assert(sizeof(a) == size_expected, "instantiated template argument has unexpected size");
+}
+void g(int *p) {
+  // instantiate for default sized pointer
+  f(p);
+}
+
+void h(int *__ptr32 p) {
+  // instantiate for 32-bit pointer
+  f<4>(p);
+}
Index: clang/test/Sema/MicrosoftExtensions.c
===
--- clang/test/Sema/MicrosoftExtensions.c
+++ clang/test/Sema/MicrosoftExtensions.c
@@ -173,8 +173,28 @@
 
 int *(__ptr32 __sptr wrong9); // expected-error {{'__sptr' attribute only applies to pointer arguments}} // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
 
+int *(__ptr32 wrong10); // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
+
+int *(__ptr64 wrong11); // expected-error {{'__ptr64' attribute only applies to pointer arguments}}
+
+int *(__ptr32 __ptr64 wrong12); // expected-error {{'__ptr32' attribute only applies to pointer arguments}} // expected-error {{'__ptr64' attribute only applies to pointer arguments}}
+
 typedef int *T;
-T __ptr32 wrong10; // expected-error {{'__ptr32' attribute only applies to pointer arguments}}
+T __ptr32 ok1;
+T __ptr64 ok2;
+T __ptr32 __ptr64 wrong13; // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr32 T1;
+T1 ok3;
+T1 __ptr32 wrong14;  // expected-warning {{attribute '__ptr32' is already applied}}
+T1 __ptr64 wrong15;  // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr64 T2;
+T2 ok4;
+T2 __ptr64 wrong16;  // expected-warning {{attribute '__ptr64' is already applied}}
+T2 __ptr32 wrong17;  // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
+
+typedef int *__ptr32 __ptr64 wrong18; // expected-error {{'__ptr32' and '__ptr64' attributes are not compatible}}
 
 typedef char *my_va_list;
 void __va_start(my_va_list *ap, ...); // expected-note {{passing argument to parameter 'ap' here}}
Index: clang/test/CodeGen/address-space-ptr32.c
===
--- clang/test/CodeGen/address-space-ptr32.c
+++ clang/test/CodeGen/address-space-ptr32.c
@@ -1,10 +1,40 @@
 // RUN: %clang_cc1 -no-opaque-pointers -triple x86_64-windows-msvc -fms-extensions -emit-llvm < %s | FileCheck %s
 
+_Static_assert(sizeof(void *) == 8, "sizeof(void *) has unexpected value.  Expected 8.");
+
 int foo(void) {
+  // CHECK: define dso_local i32 @foo
+  // CHECK: %a = alloca i32 (i32) addrspace(270)*, align 4
+  // CHECK: ret i32 4
   int (*__ptr32 a)(int);
   return sizeof(a);
 }
 
-// CHECK: define dso_local i32 @foo
-// CHECK: %a = alloca i32 (i32) addrspace(270)*, align 4
-// CHECK: ret i32 4
+int bar(void) {
+  // CHECK: define dso_local i32 @bar
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  int *__ptr32 p;
+  return sizeof(p);
+}
+
+
+int baz(void) {
+  // CHECK: define dso_local i32 @baz
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  typedef int *__ptr32 IP32_PTR;
+
+  IP32_PTR p;
+  return sizeof(p);
+}
+
+int fugu(void) {
+  // CHECK: define dso_local i32 @fugu
+  // CHECK: %p = alloca i32 addrspace(270)*, align 4
+  // CHECK: ret i32 4
+  typedef int *int_star;
+
+  int_star __ptr32 p;
+  return sizeof(p);
+}
Index: clang/lib/Sema/SemaType.cpp
===
--- clang/lib/Sema/SemaType.cpp
+++ clang/lib/Sema/SemaType.cpp
@@ -7158,17 +7158,25 @@
   }
 
   std::bitset Attrs;
-  attr::Kind NewAttrKind = A->getKind();
   QualType Desugared = Type;
-  const AttributedType *AT = dyn_cast(Type);
-  while (

[PATCH] D131499: Change prototype merging error into a warning for builtins

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/Sema/prototype-redecls.c:32
 // clear that the previous declaration was a builtin.
-float rintf() { // expected-error {{conflicting types for 'rintf'}} \
+float rintf() { // expected-warning {{incompatible redeclaration of library 
function 'rintf'}} \
expected-note {{'rintf' is a builtin with type 'float 
(float)'}}

Hmm... this is a definition of a builtin with a completely incompatible 
prototype.  Do we REALLY want this to not be an error?  

I guess I could see it being OK with declarations, but it is odd with a 
definition that is incompatible.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131499

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

[PATCH] D131499: Change prototype merging error into a warning for builtins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Sema/prototype-redecls.c:32
 // clear that the previous declaration was a builtin.
-float rintf() { // expected-error {{conflicting types for 'rintf'}} \
+float rintf() { // expected-warning {{incompatible redeclaration of library 
function 'rintf'}} \
expected-note {{'rintf' is a builtin with type 'float 
(float)'}}

erichkeane wrote:
> Hmm... this is a definition of a builtin with a completely incompatible 
> prototype.  Do we REALLY want this to not be an error?  
> 
> I guess I could see it being OK with declarations, but it is odd with a 
> definition that is incompatible.  
I think we do want it to not be an error. The user has no choice in whether 
these are predeclared or not, which steals identifiers from users. By giving 
the user a warning, they're alerted to the fact they're doing something that 
may be highly confusing, so I agree we definitely want a diagnostic. If we give 
the user an error, they're stuck. Also, it's even weirder that:
```
float rintf(void) {} // Always a warning, is fine
float rintf() {} // Error pre C2x, but fine in C2x
```


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131499

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

[PATCH] D131388: [docs] Add "C++20 Modules"

[Daniel Ruoso via Phabricator via cfe-commits]

ruoso added inline comments.



Comment at: clang/docs/CPlusPlus20Modules.rst:70-76
+* Primary module interface unit.
+
+* Module implementation unit.
+
+* Module partition interface unit.
+
+* Module partition implementation unit.

The terminology here is a bit different than what we've been building the 
consensus on. Please take a look at [[ 
https://github.com/cplusplus/modules-ecosystem-tr/blob/master/sourcefiles.tex#L19
 | sourcefiles.tex ]] (or section `[source.types]` in the [[ 
https://github.com/cplusplus/modules-ecosystem-tr/files/9237071/iso_cpp_modules_ecosystem_technical_report.pdf
 | rendered version ]] 




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

https://reviews.llvm.org/D131388

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

[PATCH] D131499: Change prototype merging error into a warning for builtins

[Erich Keane via Phabricator via cfe-commits]

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

Agree with backporting to Clang15, that'll break the fewest users.




Comment at: clang/test/Sema/prototype-redecls.c:32
 // clear that the previous declaration was a builtin.
-float rintf() { // expected-error {{conflicting types for 'rintf'}} \
+float rintf() { // expected-warning {{incompatible redeclaration of library 
function 'rintf'}} \
expected-note {{'rintf' is a builtin with type 'float 
(float)'}}

aaron.ballman wrote:
> erichkeane wrote:
> > Hmm... this is a definition of a builtin with a completely incompatible 
> > prototype.  Do we REALLY want this to not be an error?  
> > 
> > I guess I could see it being OK with declarations, but it is odd with a 
> > definition that is incompatible.  
> I think we do want it to not be an error. The user has no choice in whether 
> these are predeclared or not, which steals identifiers from users. By giving 
> the user a warning, they're alerted to the fact they're doing something that 
> may be highly confusing, so I agree we definitely want a diagnostic. If we 
> give the user an error, they're stuck. Also, it's even weirder that:
> ```
> float rintf(void) {} // Always a warning, is fine
> float rintf() {} // Error pre C2x, but fine in C2x
> ```
Aren't all those identifiers already stolen by the standard?  They are reserved 
already, right?  

But that behavior you post there is unfortunate... I guess leaving these as 
warnings is important so they get suppressed in headers (for cases where a 
C-library 'implements' it anyway, despite it being a builtin).

I think I can live with this, and I think backporting it is the least breaking 
way to do this. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131499

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

[PATCH] D128757: [Driver][test] Add -fuse-ld= option tests for NetBSD

[Frederic Cambus via Phabricator via cfe-commits]

fcambus added a comment.

Agreed, that makes sense. Sorry for the noise.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D128757

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

[PATCH] D131499: Change prototype merging error into a warning for builtins

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Sema/prototype-redecls.c:32
 // clear that the previous declaration was a builtin.
-float rintf() { // expected-error {{conflicting types for 'rintf'}} \
+float rintf() { // expected-warning {{incompatible redeclaration of library 
function 'rintf'}} \
expected-note {{'rintf' is a builtin with type 'float 
(float)'}}

erichkeane wrote:
> aaron.ballman wrote:
> > erichkeane wrote:
> > > Hmm... this is a definition of a builtin with a completely incompatible 
> > > prototype.  Do we REALLY want this to not be an error?  
> > > 
> > > I guess I could see it being OK with declarations, but it is odd with a 
> > > definition that is incompatible.  
> > I think we do want it to not be an error. The user has no choice in whether 
> > these are predeclared or not, which steals identifiers from users. By 
> > giving the user a warning, they're alerted to the fact they're doing 
> > something that may be highly confusing, so I agree we definitely want a 
> > diagnostic. If we give the user an error, they're stuck. Also, it's even 
> > weirder that:
> > ```
> > float rintf(void) {} // Always a warning, is fine
> > float rintf() {} // Error pre C2x, but fine in C2x
> > ```
> Aren't all those identifiers already stolen by the standard?  They are 
> reserved already, right?  
> 
> But that behavior you post there is unfortunate... I guess leaving these as 
> warnings is important so they get suppressed in headers (for cases where a 
> C-library 'implements' it anyway, despite it being a builtin).
> 
> I think I can live with this, and I think backporting it is the least 
> breaking way to do this. 
> Aren't all those identifiers already stolen by the standard? They are 
> reserved already, right?

Yes, they are. However, the situation we should have sympathy for is a user who 
is compiling in C17 mode and using an identifier like `ckd_add` suddenly having 
that identifier stolen out from under them because C2x added that as a library 
function and we decided we wanted it to be a builtin for some reason.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131499

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

[PATCH] D129973: [clang] Pass FoundDecl to DeclRefExpr creator for operator overloads

[Jonas Hahnfeld via Phabricator via cfe-commits]

Hahnfeld added a comment.

FWIW this also causes a `static_assert` failure while building ROOT 
:

  In file included from 
/home/jhahnfel/ROOT/src/tmva/tmva/src/DNN/Architectures/Cpu/CpuBuffer.cxx:17:
  In file included from 
/opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/memory:77:
  In file included from 
/opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/bits/shared_ptr.h:53:
  
/opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/bits/shared_ptr_base.h:1110:4:
 error: static assertion failed due to requirement 
'__is_invocable::TDestructor &, double 
**&>::value': deleter expression d(p) is well-formed
static_assert(__is_invocable<_Deleter&, _Yp*&>::value,
^ ~~~
  
/opt/rh/gcc-toolset-11/root/usr/lib/gcc/x86_64-redhat-linux/11/../../../../include/c++/11/bits/shared_ptr.h:178:11:
 note: in instantiation of function template specialization 
'std::__shared_ptr::__shared_ptr::TDestructor, void>' requested here
  : __shared_ptr<_Tp>(__p, std::move(__d)) { }
^
  
/home/jhahnfel/ROOT/src/tmva/tmva/src/DNN/Architectures/Cpu/CpuBuffer.cxx:42:14:
 note: in instantiation of function template specialization 
'std::shared_ptr::shared_ptr::TDestructor, void>' requested here
 fBuffer = std::shared_ptr(pointer, fDestructor);
   ^

After the revert, at least this issue is gone when building with current `main`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D129973

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

[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

[Gábor Horváth via Phabricator via cfe-commits]

xazax.hun added a comment.

In D131280#3709781 , @ymandel wrote:

> Thanks. That looks good, but I'm concerned that it only counts the arguments 
> and doesn't look at their types. I'd imagine this will be a limitation down 
> the line when we want to deal with overload sets w/ the same number of 
> arguments, but different types.

Yeah, it is not a fully baked solution at this point, but it does implement 
some of the features that you plan to add (like skipping inline namespaces), 
and some more (argument count, checking if a function is from a system header).

> Aside: why the `const char *` interface? Do you think owners would be open to 
> a `llvm::StringRef` overload for the constructor?

I am sure that the analyzer community is open to any improvements. The main 
reason I'd be glad if that facility could be shared across the two static 
analysis solution because improvements from one community could be benefited by 
the other, also the code would be more similar which could be great for cross 
pollination of ideas.

If you think it is feasible to reuse some of those facilities for your 
purposes, I am happy to review all of those patches. If it is not a good fit 
for some reason, I am ok with having a new custom solution here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131280

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

[PATCH] D131388: [docs] Add "C++20 Modules"

[Daniel Ruoso via Phabricator via cfe-commits]

ruoso added inline comments.



Comment at: clang/docs/CPlusPlus20Modules.rst:225
+
+It is possible to generate a module file for an importable module unit by 
specifying the ``--precompile`` option.
+

Likewise, here the term "Built Module Interface file", with the acronym "BMI" 
is what we're generally using when talking about the generated file.



Comment at: clang/docs/CPlusPlus20Modules.rst:238
+
+If the file names use different extensions, Clang may fail to build the module.
+

Is that actually true? Or does it require explicit arguments to explain how the 
compiler needs to translate the file?



Comment at: clang/docs/CPlusPlus20Modules.rst:243-244
+
+The option ``-fprebuilt-module-interface`` tells the compiler the path where 
to search for dependent module files.
+It may be used multiple times just like ``-I`` for specifying paths for header 
files.
+

Is that the case for C++20 named modules as well? I thought this was just for 
clang modules. How does the lookup work?



Comment at: clang/docs/CPlusPlus20Modules.rst:250-253
+When we compile a ``module implementation unit``, we must pass the module file 
of the corresponding
+``primary module interface unit`` by ``-fmodule-file``.
+Again, this option may occur multiple times. For example, the command line to 
compile ``M.cppm`` in
+the above example could be rewritten into:

I wonder if it's easier to explain that a module implementation unit implicitly 
imports the primary module interface unit, and therefore it needs the BMI for 
that interface to be provided, just like it's the case for every other import 
statement.


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

https://reviews.llvm.org/D131388

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

[PATCH] D130847: [clang] SourceManager: fix at SourceManager::getFileIDLoaded for the case of invalid SLockEntry

[Ivan Murashko via Phabricator via cfe-commits]

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

Use Invalid flag to detect invalid SLocEntry


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130847

Files:
  clang/lib/Basic/SourceManager.cpp


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -877,9 +877,12 @@
 I = (-LastID - 2) + 1;
 
   unsigned NumProbes;
+  bool Invalid = false;
   for (NumProbes = 0; NumProbes < 8; ++NumProbes, ++I) {
 // Make sure the entry is loaded!
-const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I);
+const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I, &Invalid);
+if (Invalid)
+  return FileID(); // invalid entry.
 if (E.getOffset() <= SLocOffset) {
   FileID Res = FileID::get(-int(I) - 2);
   LastFileIDLookup = Res;
@@ -897,8 +900,8 @@
   while (true) {
 ++NumProbes;
 unsigned MiddleIndex = (LessIndex - GreaterIndex) / 2 + GreaterIndex;
-const SrcMgr::SLocEntry &E = getLoadedSLocEntry(MiddleIndex);
-if (E.getOffset() == 0)
+const SrcMgr::SLocEntry &E = getLoadedSLocEntry(MiddleIndex, &Invalid);
+if (Invalid)
   return FileID(); // invalid entry.
 
 ++NumProbes;


Index: clang/lib/Basic/SourceManager.cpp
===
--- clang/lib/Basic/SourceManager.cpp
+++ clang/lib/Basic/SourceManager.cpp
@@ -877,9 +877,12 @@
 I = (-LastID - 2) + 1;
 
   unsigned NumProbes;
+  bool Invalid = false;
   for (NumProbes = 0; NumProbes < 8; ++NumProbes, ++I) {
 // Make sure the entry is loaded!
-const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I);
+const SrcMgr::SLocEntry &E = getLoadedSLocEntry(I, &Invalid);
+if (Invalid)
+  return FileID(); // invalid entry.
 if (E.getOffset() <= SLocOffset) {
   FileID Res = FileID::get(-int(I) - 2);
   LastFileIDLookup = Res;
@@ -897,8 +900,8 @@
   while (true) {
 ++NumProbes;
 unsigned MiddleIndex = (LessIndex - GreaterIndex) / 2 + GreaterIndex;
-const SrcMgr::SLocEntry &E = getLoadedSLocEntry(MiddleIndex);
-if (E.getOffset() == 0)
+const SrcMgr::SLocEntry &E = getLoadedSLocEntry(MiddleIndex, &Invalid);
+if (Invalid)
   return FileID(); // invalid entry.
 
 ++NumProbes;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 4c02ab8 - Change prototype merging error into a warning for builtins

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-08-09T11:36:48-04:00
New Revision: 4c02ab8c9742f6c32b17f49a306b3b072486f5c5

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

LOG: Change prototype merging error into a warning for builtins

As was observed in https://reviews.llvm.org/D123627#3707635, it's
confusing that a user can write:
```
float rintf(void) {}
```
and get a warning, but writing:
```
float rintf() {}
```
gives an error. This patch changes the behavior so that both are
warnings, so that users who have functions which conflict with a
builtin identifier can still use that identifier as they wish.

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

Added: 


Modified: 
clang/lib/Sema/SemaDecl.cpp
clang/test/Sema/prototype-redecls.c

Removed: 




diff  --git a/clang/lib/Sema/SemaDecl.cpp b/clang/lib/Sema/SemaDecl.cpp
index 605e4bf0abae..c3011c09a6dc 100644
--- a/clang/lib/Sema/SemaDecl.cpp
+++ b/clang/lib/Sema/SemaDecl.cpp
@@ -4042,7 +4042,7 @@ bool Sema::MergeFunctionDecl(FunctionDecl *New, NamedDecl 
*&OldD, Scope *S,
 // default argument promotion rules were already checked by
 // ASTContext::typesAreCompatible().
 if (Old->hasPrototype() && !New->hasWrittenPrototype() && NewDeclIsDefn &&
-Old->getNumParams() != New->getNumParams()) {
+Old->getNumParams() != New->getNumParams() && !Old->isImplicit()) {
   if (Old->hasInheritedPrototype())
 Old = Old->getCanonicalDecl();
   Diag(New->getLocation(), diag::err_conflicting_types) << New;

diff  --git a/clang/test/Sema/prototype-redecls.c 
b/clang/test/Sema/prototype-redecls.c
index ed569b5223ce..49305db10d24 100644
--- a/clang/test/Sema/prototype-redecls.c
+++ b/clang/test/Sema/prototype-redecls.c
@@ -29,7 +29,7 @@ void garp(x) int x; {}
 
 // Ensure redeclarations that conflict with a builtin use a note which makes it
 // clear that the previous declaration was a builtin.
-float rintf() { // expected-error {{conflicting types for 'rintf'}} \
+float rintf() { // expected-warning {{incompatible redeclaration of library 
function 'rintf'}} \
expected-note {{'rintf' is a builtin with type 'float 
(float)'}}
   return 1.0f;
 }



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

[PATCH] D131499: Change prototype merging error into a warning for builtins

[Aaron Ballman 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 rG4c02ab8c9742: Change prototype merging error into a warning 
for builtins (authored by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131499

Files:
  clang/lib/Sema/SemaDecl.cpp
  clang/test/Sema/prototype-redecls.c


Index: clang/test/Sema/prototype-redecls.c
===
--- clang/test/Sema/prototype-redecls.c
+++ clang/test/Sema/prototype-redecls.c
@@ -29,7 +29,7 @@
 
 // Ensure redeclarations that conflict with a builtin use a note which makes it
 // clear that the previous declaration was a builtin.
-float rintf() { // expected-error {{conflicting types for 'rintf'}} \
+float rintf() { // expected-warning {{incompatible redeclaration of library 
function 'rintf'}} \
expected-note {{'rintf' is a builtin with type 'float 
(float)'}}
   return 1.0f;
 }
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4042,7 +4042,7 @@
 // default argument promotion rules were already checked by
 // ASTContext::typesAreCompatible().
 if (Old->hasPrototype() && !New->hasWrittenPrototype() && NewDeclIsDefn &&
-Old->getNumParams() != New->getNumParams()) {
+Old->getNumParams() != New->getNumParams() && !Old->isImplicit()) {
   if (Old->hasInheritedPrototype())
 Old = Old->getCanonicalDecl();
   Diag(New->getLocation(), diag::err_conflicting_types) << New;


Index: clang/test/Sema/prototype-redecls.c
===
--- clang/test/Sema/prototype-redecls.c
+++ clang/test/Sema/prototype-redecls.c
@@ -29,7 +29,7 @@
 
 // Ensure redeclarations that conflict with a builtin use a note which makes it
 // clear that the previous declaration was a builtin.
-float rintf() { // expected-error {{conflicting types for 'rintf'}} \
+float rintf() { // expected-warning {{incompatible redeclaration of library function 'rintf'}} \
expected-note {{'rintf' is a builtin with type 'float (float)'}}
   return 1.0f;
 }
Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -4042,7 +4042,7 @@
 // default argument promotion rules were already checked by
 // ASTContext::typesAreCompatible().
 if (Old->hasPrototype() && !New->hasWrittenPrototype() && NewDeclIsDefn &&
-Old->getNumParams() != New->getNumParams()) {
+Old->getNumParams() != New->getNumParams() && !Old->isImplicit()) {
   if (Old->hasInheritedPrototype())
 Old = Old->getCanonicalDecl();
   Diag(New->getLocation(), diag::err_conflicting_types) << New;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131388: [docs] Add "C++20 Modules"

[Chuanqi Xu via Phabricator via cfe-commits]

ChuanqiXu added inline comments.



Comment at: clang/docs/CPlusPlus20Modules.rst:70-76
+* Primary module interface unit.
+
+* Module implementation unit.
+
+* Module partition interface unit.
+
+* Module partition implementation unit.

ruoso wrote:
> The terminology here is a bit different than what we've been building the 
> consensus on. Please take a look at [[ 
> https://github.com/cplusplus/modules-ecosystem-tr/blob/master/sourcefiles.tex#L19
>  | sourcefiles.tex ]] (or section `[source.types]` in the [[ 
> https://github.com/cplusplus/modules-ecosystem-tr/files/9237071/iso_cpp_modules_ecosystem_technical_report.pdf
>  | rendered version ]] 
> 
> 
If there is a consensus already, we should follow. 

(BTW, I thought we'll discuss module things in SG2 but it looks like we're 
discussing them in SG15... my bad)



Comment at: clang/docs/CPlusPlus20Modules.rst:225
+
+It is possible to generate a module file for an importable module unit by 
specifying the ``--precompile`` option.
+

ruoso wrote:
> Likewise, here the term "Built Module Interface file", with the acronym "BMI" 
> is what we're generally using when talking about the generated file.
Yeah, this is what I was confused in the chat. In my mind, "BMI" describes a 
compatible interface format like ABI (like Itanium ABI). In another words, a 
BMI could be compiled by compiler which follows the BMI standard (like clang 
and gcc both accepts Itanium ABI). But currrently, a module file couldn't be 
compiled by clang in different versions.

So from my point of view, the term `module file` is more appropriate than `BMI` 
now.



Comment at: clang/docs/CPlusPlus20Modules.rst:243-244
+
+The option ``-fprebuilt-module-interface`` tells the compiler the path where 
to search for dependent module files.
+It may be used multiple times just like ``-I`` for specifying paths for header 
files.
+

ruoso wrote:
> Is that the case for C++20 named modules as well? I thought this was just for 
> clang modules. How does the lookup work?
In this document, modules are referring to `C++20 Named Modules` by default. 
The option is borrowed from clang modules. The look up rule here is:

(1) When we import module `M`. The compiler would look up `M.pcm` in the 
directories specified by ``-fprebuilt-module-interface``.
(2) When we import partition module unit `M:P`. The compiler would look up 
`M-P.pcm` in the directories specified by ``-fprebuilt-module-interface``.

I thought this is clear enough. Do you have suggestion to improve? I am a 
little worried to be wordy.



Comment at: clang/docs/CPlusPlus20Modules.rst:250-253
+When we compile a ``module implementation unit``, we must pass the module file 
of the corresponding
+``primary module interface unit`` by ``-fmodule-file``.
+Again, this option may occur multiple times. For example, the command line to 
compile ``M.cppm`` in
+the above example could be rewritten into:

ruoso wrote:
> I wonder if it's easier to explain that a module implementation unit 
> implicitly imports the primary module interface unit, and therefore it needs 
> the BMI for that interface to be provided, just like it's the case for every 
> other import statement.
OK, I was afraid to cite too many standard paragraphs to scare readers. But 
this point looks necessary 


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

https://reviews.llvm.org/D131388

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

[clang] e486e48 - [clang] fix deprecation

[Thorsten Schütt via cfe-commits]

Author: Thorsten Schütt
Date: 2022-08-09T17:42:55+02:00
New Revision: e486e48c3d9e99e4c17d365bbc4b429c8e5b5999

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

LOG: [clang] fix deprecation

Added: 


Modified: 
clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp

Removed: 




diff  --git a/clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp 
b/clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
index bdc3895162898..b8788bae8171c 100644
--- a/clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
+++ b/clang/lib/DirectoryWatcher/mac/DirectoryWatcher-mac.cpp
@@ -136,7 +136,7 @@ static void eventStreamCallback(ConstFSEventStreamRef 
Stream,
   llvm::sys::path::filename(Path));
   continue;
 } else if (Flags & ModifyingFileEvents) {
-  if (!getFileStatus(Path).hasValue()) {
+  if (!getFileStatus(Path).has_value()) {
 Events.emplace_back(DirectoryWatcher::Event::EventKind::Removed,
 llvm::sys::path::filename(Path));
   } else {



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

[PATCH] D131255: Fix Wbitfield-constant-conversion on 1-bit signed bitfield

[Aaron Ballman via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG82afc9b169a6: Fix -Wbitfield-constant-conversion on 1-bit 
signed bitfield (authored by ShawnZhong, committed by aaron.ballman).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131255

Files:
  clang/docs/ReleaseNotes.rst
  clang/lib/Sema/SemaChecking.cpp
  clang/test/CXX/class/class.bit/p1.cpp
  clang/test/CXX/drs/dr6xx.cpp
  clang/test/CodeGen/bitfield.c
  clang/test/CodeGenCXX/bitfield-layout.cpp
  clang/test/Rewriter/rewrite-modern-struct-ivar-1.mm
  clang/test/Rewriter/rewrite-modern-struct-ivar.mm
  clang/test/Sema/constant-conversion.c
  clang/test/SemaCXX/constant-expression-cxx11.cpp
  clang/test/SemaCXX/cxx1z-decomposition.cpp
  clang/test/SemaCXX/decomposition-blocks.cpp

Index: clang/test/SemaCXX/decomposition-blocks.cpp
===
--- clang/test/SemaCXX/decomposition-blocks.cpp
+++ clang/test/SemaCXX/decomposition-blocks.cpp
@@ -7,7 +7,7 @@
 
 void run(void (^)());
 void test() {
-  auto [i, j] = S{1, 42}; // expected-note {{'i' declared here}}
+  auto [i, j] = S{-1, 42}; // expected-note {{'i' declared here}}
   run(^{
 (void)i; // expected-error {{reference to local binding 'i' declared in enclosing function 'test'}}
   });
Index: clang/test/SemaCXX/cxx1z-decomposition.cpp
===
--- clang/test/SemaCXX/cxx1z-decomposition.cpp
+++ clang/test/SemaCXX/cxx1z-decomposition.cpp
@@ -84,7 +84,7 @@
 
   (void)[outerbit1]{}; // expected-error {{'outerbit1' cannot be captured because it does not have automatic storage duration}}
 
-  auto [bit, var] = S2{1, 1}; // expected-note 2{{'bit' declared here}}
+  auto [bit, var] = S2{-1, 1}; // expected-note 2{{'bit' declared here}}
 
   (void)[&bit] { // expected-error {{non-const reference cannot bind to bit-field 'a'}} \
 // expected-warning {{C++20}}
Index: clang/test/SemaCXX/constant-expression-cxx11.cpp
===
--- clang/test/SemaCXX/constant-expression-cxx11.cpp
+++ clang/test/SemaCXX/constant-expression-cxx11.cpp
@@ -2035,6 +2035,7 @@
   }
 };
 static_assert(X::f(3) == -1, "3 should truncate to -1");
+static_assert(X::f(1) == -1, "1 should truncate to -1");
   }
 
   struct HasUnnamedBitfield {
Index: clang/test/Sema/constant-conversion.c
===
--- clang/test/Sema/constant-conversion.c
+++ clang/test/Sema/constant-conversion.c
@@ -15,8 +15,16 @@
 }
 
 void test(void) {
-  struct { int bit : 1; } a;
-  a.bit = 1; // shouldn't warn
+  struct S {
+int b : 1;  // The only valid values are 0 and -1.
+  } s;
+
+  s.b = -3; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -3 to -1}}
+  s.b = -2; // expected-warning {{implicit truncation from 'int' to bit-field changes value from -2 to 0}}
+  s.b = -1; // no-warning
+  s.b = 0;  // no-warning
+  s.b = 1;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 1 to -1}}
+  s.b = 2;  // expected-warning {{implicit truncation from 'int' to bit-field changes value from 2 to 0}}
 }
 
 enum Test2 { K_zero, K_one };
Index: clang/test/Rewriter/rewrite-modern-struct-ivar.mm
===
--- clang/test/Rewriter/rewrite-modern-struct-ivar.mm
+++ clang/test/Rewriter/rewrite-modern-struct-ivar.mm
@@ -41,10 +41,10 @@
 @implementation Foo
 - (void)x {
   bar.x = 0;
-  bar.y = 1;
+  bar.y = -1;
 
   s.x = 0;
-  s.y = 1;
+  s.y = -1;
 }
 @end
 
Index: clang/test/Rewriter/rewrite-modern-struct-ivar-1.mm
===
--- clang/test/Rewriter/rewrite-modern-struct-ivar-1.mm
+++ clang/test/Rewriter/rewrite-modern-struct-ivar-1.mm
@@ -34,7 +34,7 @@
 @implementation Foo
 - (void)x:(Foo *)other {
   bar.x = 0;
-  bar.y = 1;
+  bar.y = -1;
   self->_internal._singleRange._range = (( other ->bar.x) ? &( other ->_internal._singleRange._range) : ((NSRange *)(&(((_NSRangeInfo *)( other ->_internal._multipleRanges._data))->_ranges[0];
 }
 @end
Index: clang/test/CodeGenCXX/bitfield-layout.cpp
===
--- clang/test/CodeGenCXX/bitfield-layout.cpp
+++ clang/test/CodeGenCXX/bitfield-layout.cpp
@@ -105,6 +105,15 @@
 // CHECK: define{{.*}} i32 @test_trunc_three_bits()
 // CHECK: ret i32 -1
 
+int test_trunc_one_bit() {
+  union {
+int i : 1; // truncated to 0b1 == -1
+  } const U = {1};  // 0b0001
+  return U.i;
+}
+// CHECK: define{{.*}} i32 @test_trunc_one_bit()
+// CHECK: ret i32 -1
+
 int test_trunc_1() {
   union {
 int i : 1; // truncated to 0b1 == -1
Index: clang/test/CodeGen/bitfield.c
=

[clang] 82afc9b - Fix -Wbitfield-constant-conversion on 1-bit signed bitfield

[Aaron Ballman via cfe-commits]

Author: Shawn Zhong
Date: 2022-08-09T11:43:50-04:00
New Revision: 82afc9b169a67e8b8a1862fb9c41a2cd974d6691

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

LOG: Fix -Wbitfield-constant-conversion on 1-bit signed bitfield

A one-bit signed bit-field can only hold the values 0 and -1; this
corrects the diagnostic behavior accordingly.

Fixes #53253
Differential Revision: https://reviews.llvm.org/D131255

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaChecking.cpp
clang/test/CXX/class/class.bit/p1.cpp
clang/test/CXX/drs/dr6xx.cpp
clang/test/CodeGen/bitfield.c
clang/test/CodeGenCXX/bitfield-layout.cpp
clang/test/Rewriter/rewrite-modern-struct-ivar-1.mm
clang/test/Rewriter/rewrite-modern-struct-ivar.mm
clang/test/Sema/constant-conversion.c
clang/test/SemaCXX/constant-expression-cxx11.cpp
clang/test/SemaCXX/cxx1z-decomposition.cpp
clang/test/SemaCXX/decomposition-blocks.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index d830928a13d4d..4b01f018005ed 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -82,6 +82,9 @@ Improvements to Clang's diagnostics
 - ``-Wformat`` now recognizes ``%b`` for the ``printf``/``scanf`` family of
   functions and ``%B`` for the ``printf`` family of functions. Fixes
   `Issue 56885: `_.
+- ``-Wbitfield-constant-conversion`` now diagnoses implicit truncation when 1 
is
+  assigned to a 1-bit signed integer bitfield. This fixes
+  `Issue 53253 `_.
 
 Non-comprehensive list of changes in this release
 -

diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index 5a1c21d3a367b..a0ea35e1234f6 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -13064,11 +13064,6 @@ static bool AnalyzeBitFieldAssignment(Sema &S, 
FieldDecl *Bitfield, Expr *Init,
   if (llvm::APSInt::isSameValue(Value, TruncatedValue))
 return false;
 
-  // Special-case bitfields of width 1: booleans are naturally 0/1, and
-  // therefore don't strictly fit into a signed bitfield of width 1.
-  if (FieldWidth == 1 && Value == 1)
-return false;
-
   std::string PrettyValue = toString(Value, 10);
   std::string PrettyTrunc = toString(TruncatedValue, 10);
 

diff  --git a/clang/test/CXX/class/class.bit/p1.cpp 
b/clang/test/CXX/class/class.bit/p1.cpp
index ab15e3a356cfc..5df1ab52cdd8d 100644
--- a/clang/test/CXX/class/class.bit/p1.cpp
+++ b/clang/test/CXX/class/class.bit/p1.cpp
@@ -9,11 +9,11 @@ struct A {
   int [[]] c : 1; // OK, attribute applies to the type.
   int : 2 = 1; // expected-error {{anonymous bit-field cannot have a default 
member initializer}}
   int : 0 { 1 }; // expected-error {{anonymous bit-field cannot have a default 
member initializer}}
-  int : 0, d : 1 = 1;
+  unsigned int : 0, d : 1 = 1;
   int : 1 = 12, e : 1; // expected-error {{anonymous bit-field cannot have a 
default member initializer}}
-  int : 0, f : 1 = 1;
-  int g [[]] : 1 = 1;
-  int h [[]] : 1 {1};
-  int i : foo() = foo();
+  unsigned int : 0, f : 1 = 1;
+  unsigned int g [[]] : 1 = 1;
+  unsigned int h [[]] : 1 {1};
+  unsigned int i : foo() = foo();
   int j, [[]] k; // expected-error {{an attribute list cannot appear here}}
 };

diff  --git a/clang/test/CXX/drs/dr6xx.cpp b/clang/test/CXX/drs/dr6xx.cpp
index ad87c7295cfe8..432fd6e6c5efa 100644
--- a/clang/test/CXX/drs/dr6xx.cpp
+++ b/clang/test/CXX/drs/dr6xx.cpp
@@ -911,9 +911,9 @@ namespace dr674 { // dr674: 8
 namespace dr675 { // dr675: dup 739
   template struct A { T n : 1; };
 #if __cplusplus >= 201103L
-  static_assert(A{1}.n < 0, "");
-  static_assert(A{1}.n < 0, "");
-  static_assert(A{1}.n < 0, "");
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit 
truncation from 'int' to bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit 
truncation from 'int' to bit-field changes value from 1 to -1}}
+  static_assert(A{1}.n < 0, ""); // expected-warning {{implicit 
truncation from 'int' to bit-field changes value from 1 to -1}}
 #endif
 }
 

diff  --git a/clang/test/CodeGen/bitfield.c b/clang/test/CodeGen/bitfield.c
index c624d0045d330..4845d7c478429 100644
--- a/clang/test/CodeGen/bitfield.c
+++ b/clang/test/CodeGen/bitfield.c
@@ -87,3 +87,19 @@ int g3(void) {
 // PATH: ret i32 1
   return f3(20) + 130725747;
 }
+
+static int f4(void) {
+  struct s5 {
+int b:1;
+  } x;
+  x.b = 1;
+  return x.b;
+}
+
+int g4(void) {
+// CHECK-LABEL: @g4()
+// CHECK: ret i32 1
+// PATH-LABEL: @g4()
+// PATH: ret i32 1
+  return f4() + 2;
+}

diff  --git a/

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a subscriber: aaron.ballman.
dblaikie added a comment.

@aaron.ballman is this something you're comfortable reviewing or could 
recommend anyone else who might be suitable?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051

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

[PATCH] D131280: [clang][dataflow] Parameterize analysis by explicit map of analyzable functions.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel added a comment.

In D131280#3709964 , @xazax.hun wrote:

> In D131280#3709781 , @ymandel wrote:
>
>> Thanks. That looks good, but I'm concerned that it only counts the arguments 
>> and doesn't look at their types. I'd imagine this will be a limitation down 
>> the line when we want to deal with overload sets w/ the same number of 
>> arguments, but different types.
>
> Yeah, it is not a fully baked solution at this point, but it does implement 
> some of the features that you plan to add (like skipping inline namespaces), 
> and some more (argument count, checking if a function is from a system 
> header).
>
>> Aside: why the `const char *` interface? Do you think owners would be open 
>> to a `llvm::StringRef` overload for the constructor?
>
> I am sure that the analyzer community is open to any improvements. The main 
> reason I'd be glad if that facility could be shared across the two static 
> analysis solution because improvements from one community could be benefited 
> by the other, also the code would be more similar which could be great for 
> cross pollination of ideas.
>
> If you think it is feasible to reuse some of those facilities for your 
> purposes, I am happy to review all of those patches. If it is not a good fit 
> for some reason, I am ok with having a new custom solution here.

I think we should try to reuse for the reasons you mention. We can evolve it 
with more features as needed. I'm kind of allergic to `const char *` so I 
propose that I first send a patch for that and can then follow up here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131280

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

[PATCH] D122139: [pseudo] Introduce parse forest.

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: clang-tools-extra/pseudo/include/clang-pseudo/Forest.h:158
+  size_t nodeCount() const { return NodeCount; }
+  size_t bytes() const { return Arena.getBytesAllocated() + sizeof(this); }
+

@hokein Static analysis is warning about sizeof(this) - should it be 
sizeof(*this) ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122139

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

[PATCH] D131388: [docs] Add "C++20 Modules"

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/docs/CPlusPlus20Modules.rst:31
+
+This document was intended to be a manual first and foremost, however, we 
consider it helpful to
+introduce some language background here for readers who are not familiar with





Comment at: clang/docs/CPlusPlus20Modules.rst:122
+
+Let's see a "hello world" example to use modules.
+





Comment at: clang/docs/CPlusPlus20Modules.rst:262
+
+Remember to link module files
+~





Comment at: clang/docs/CPlusPlus20Modules.rst:312-321
+Note that **currently** the compiler doesn't consider inconsistent macro 
definition a problem. For example:
+
+.. code-block:: console
+
+  $ clang++ -std=c++20 M.cppm --precompile -o M.pcm
+  # Inconsistent optimization level.
+  $ clang++ -std=c++20 -O3 -DNDEBUG Use.cpp -fprebuilt-module-path=.

this sort of aside might be best left for a separate part of the document - an 
FAQ/side-notes (a footnote, perhaps?), etc to keep the rest of the document 
more focussed?



Comment at: clang/docs/CPlusPlus20Modules.rst:347
+  $ clang++ -std=c++20 M.cppm --precompile -o M.pcm
+  $ rm -f M.cppm
+  $ clang++ -std=c++20 Use.cpp -fmodule-file=M.pcm

Could probably skip the `-f`?



Comment at: clang/docs/CPlusPlus20Modules.rst:395-396
+
+Roughly, this theory is correct. But the problem is that it is too rough. 
Let's see what actually happens.
+For example, the behavior also depends on the optimization level, as we will 
illustrate below.
+

I'm not sure I'm able to follow the example and how it justifies the rough 
theory as inadequate to explain the motivation for modules - could you clarify 
more directly (in comments, and then we can discuss how to word it) what the 
motivation for this section is/what you're trying to convey?



Comment at: clang/docs/CPlusPlus20Modules.rst:610
+
+Another difference with modules is that we can't compile the module file.
+It makes sense due to the semantics of header units, which are just like 
headers.

Might need some more words here - I guess this means "there is no .o for a .pcm 
from a header unit" basically?


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

https://reviews.llvm.org/D131388

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

[PATCH] D131319: [clang-tidy] Update llvm-prefer-isa-or-dyn-cast-in-conditionals with new syntax

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/llvm/prefer-isa-or-dyn-cast-in-conditionals.cpp:12-13
 
 template 
 bool isa(Y *);
 template 

whisperity wrote:
> Will the tests pass properly once the fixes are applied, even though the 
> replaced code refers to a symbol (`isa_and_present`) that is not declared in 
> the TU?
The current tests would fail if ran once the fixes are applied as 
`isa_and_nonnull` doesn't exist in this TU. Most tidy checks (for better or 
worse) don't check for existence of an identifier that is expected to be there 
when making replacements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131319

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

[PATCH] D119051: Extend the C++03 definition of POD to include defaulted functions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Oh wow that's an awful lot of pings without any response; I'm very sorry you 
had that experience, so thank you for tagging me to try to get this unstuck!

The precommit CI test failures definitely look relevant and should be fixed up.




Comment at: clang/include/clang/Basic/LangOptions.def:219
 BENIGN_LANGOPT(AccessControl , 1, 1, "C++ access control")
+LANGOPT(DefaultedSMFArePOD, 1, 0, "Defaulted Special Members are allowed on 
POD types")
 LANGOPT(CharIsSigned  , 1, 1, "signed char")





Comment at: clang/include/clang/Driver/Options.td:1116
   PosFlag>;
+defm defaulted_smf_are_pod : BoolFOption<"defaulted-smf-are-pod",
+  LangOpts<"DefaultedSMFArePOD">, DefaultFalse,

Lol, well that's going to be fun when said out loud -- I come up with 
"defaulted smurfs are pod". :-D



Comment at: clang/include/clang/Driver/Options.td:1118-1119
+  LangOpts<"DefaultedSMFArePOD">, DefaultFalse,
+  NegFlag,
+  PosFlag>;
 def falign_functions : Flag<["-"], "falign-functions">, Group;





Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5592
+StringRef Ver = A->getValue();
+CmdArgs.push_back(Args.MakeArgString("-fclang-abi-compat=" + Ver));
+unsigned Num;

Are we going to get duplicates passed to -cc1, as we also do: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/Clang.cpp#L5645



Comment at: clang/lib/Driver/ToolChains/Clang.cpp:5594
+unsigned Num;
+if (!Ver.consumeInteger(10, Num) && Num <= 13)
+  DefaultedSMFArePOD = false;

Is Clang 13 still the correct thing to test for here, or should this be 16 
these days?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119051

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

[PATCH] D130894: [clang] Print more information about failed static assertions

[Christopher Di Bella via Phabricator via cfe-commits]

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

In D130894#3709431 , @aaron.ballman 
wrote:

> In D130894#3709027 , @tbaeder wrote:
>
>> I don't really want to bike-shed about the presentation for too long...
>
> I understand the desire, but at the same time, the whole goal of this patch 
> is to improve the presentation of the diagnostic. You can't invite us all to 
> a bikeshed painting party and then ask us not to use our brushes, that's just 
> cruel! :-D

Agreed. Getting the presentation of diagnostics right is critical.

>> I'm fine with just removing the parens, since we present it like that in the 
>> error message as well anyway:
>>
>>   ./assert.cpp:6:1: error: static assertion failed due to requirement ''c' 
>> == 'a''
>>   static_assert('c' == 'a')
>
> That's still my preference as well. Splitting across multiple lines with hard 
> line breaks has some issues for some IDEs that want to display diagnostic 
> information in a listbox (IIRC), so I'd rather we avoid ad hoc use of line 
> breaks in diagnostics for the moment (it'd be easier/more appropriate if we 
> were making a diagnostic policy for using multiple lines, but that'd involve 
> an RFC and is more effort than I think is needed for this patch).

Fair enough, no further objections on my part.

>>> Any tests with macros?
>>
>> I can add some, but they should be handled transparently, with the usual 
>> "expanded from macro" note.
>
> I'm fine either way.




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

https://reviews.llvm.org/D130894

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

[PATCH] D130131: [HLSL] CodeGen hlsl cbuffer/tbuffer.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/CodeGen/CGHLSLRuntime.cpp:96
+  }
+  Buf.LayoutStruct = llvm::StructType::get(EltTys[0]->getContext(), EltTys);
+}

Why are you manually inserting padding?

IR level accesses don't require explicit layout, and we don't do this in DXC 
either.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130131

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

[PATCH] D131466: [clang] add APValue type check in `TryPrintAsStringLiteral`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Can you add test coverage for these changes?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131466

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Roy Jacobson via Phabricator via cfe-commits]

royjacobson added a comment.

In D130689#3709834 , @thieta wrote:

> In D130689#3709742 , @aaron.ballman 
> wrote:
>
>> One thing I think would be a definite improvement is to have done an RFC on 
>> Discourse for these changes so that downstreams have a chance to weigh in on 
>> the impact. The patch was put up on Jul 28 and landed about a week later 
>> without any notification to the rest of the community who might not be 
>> watching cfe-commits -- that's a very fast turnaround and very little 
>> notification for such a significant change.
>
> Yeah this is on me. Honestly I didn't expect it to be that much of a problem 
> but rather the toolchain requirement we posted as part of it would be the big 
> hurdle where bot owners would have to upgrade to get the right versions. But 
> lesson learned  and we should add some more delays in the policy here: 
> https://llvm.org/docs/DeveloperPolicy.html#id23 and cover the C++ standards 
> upgrade.

Two points I want to add that I think would've been useful as well -

1. In addition to the toolchain soft errors, add a version check + #warning to 
some llvm header. This would be useful as it is more visible than the CMake 
warning and it could show up for cases where LLVM is used as a library+headers 
and not built from sources.
2. Delay actual usage of new language features until after the next release. 
Currently I see people pushing lots of cleanup commits that could hurt bug 
backports. It also has the benefit of making the transition more gradual.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130689: [LLVM] Update C++ standard to 17

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D130689#3710281 , @royjacobson 
wrote:

> In D130689#3709834 , @thieta wrote:
>
>> In D130689#3709742 , 
>> @aaron.ballman wrote:
>>
>>> One thing I think would be a definite improvement is to have done an RFC on 
>>> Discourse for these changes so that downstreams have a chance to weigh in 
>>> on the impact. The patch was put up on Jul 28 and landed about a week later 
>>> without any notification to the rest of the community who might not be 
>>> watching cfe-commits -- that's a very fast turnaround and very little 
>>> notification for such a significant change.
>>
>> Yeah this is on me. Honestly I didn't expect it to be that much of a problem 
>> but rather the toolchain requirement we posted as part of it would be the 
>> big hurdle where bot owners would have to upgrade to get the right versions. 
>> But lesson learned  and we should add some more delays in the policy here: 
>> https://llvm.org/docs/DeveloperPolicy.html#id23 and cover the C++ standards 
>> upgrade.
>
> Two points I want to add that I think would've been useful as well -
>
> 1. In addition to the toolchain soft errors, add a version check + #warning 
> to some llvm header. This would be useful as it is more visible than the 
> CMake warning and it could show up for cases where LLVM is used as a 
> library+headers and not built from sources.
> 2. Delay actual usage of new language features until after the next release. 
> Currently I see people pushing lots of cleanup commits that could hurt bug 
> backports. It also has the benefit of making the transition more gradual.

Strong +1 to point #2 especially. This is something we could have plausibly 
reverted to work through the kinks rather than doing that work live and while 
under duress, but it became implausible pretty quickly because everyone started 
landing their C++17 NFC changes. Those kinds of changes almost always can wait 
until after we've validated that the switch has gone smoothly.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D130689

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

[PATCH] D130894: [clang] Print more information about failed static assertions

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman accepted this revision.
aaron.ballman added a comment.

LGTM, thank you for working on this, it's a great usability enhancement!


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

https://reviews.llvm.org/D130894

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

[PATCH] D131466: [clang] add APValue type check in `TryPrintAsStringLiteral`

[YingChi Long via Phabricator via cfe-commits]

inclyc updated this revision to Diff 451195.
inclyc added a comment.

address comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131466

Files:
  clang/lib/AST/APValue.cpp
  clang/test/SemaCXX/try-print-as-string-literal-type-check.cpp


Index: clang/test/SemaCXX/try-print-as-string-literal-type-check.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/try-print-as-string-literal-type-check.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -verify
+// expected-no-diagnostics
+
+// Reported by: https://github.com/llvm/llvm-project/issues/57013
+// The following code should not crash clang
+struct X {
+  char arr[2];
+  constexpr X() {}
+  constexpr void modify() {
+arr[0] = 0;
+  }
+};
+constexpr X f(X t) {
+t.modify();
+return t;
+}
+auto x = f(X());
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -637,10 +637,10 @@
 return false;
 
   // Nothing we can do about a sequence that is not null-terminated
-  if (!Inits.back().getInt().isZero())
+  if (!Inits.back().isInt() || !Inits.back().getInt().isZero())
 return false;
-  else
-Inits = Inits.drop_back();
+
+  Inits = Inits.drop_back();
 
   llvm::SmallString<40> Buf;
   Buf.push_back('"');
@@ -655,6 +655,8 @@
   }
 
   for (auto &Val : Inits) {
+if (!Val.isInt())
+  return false;
 int64_t Char64 = Val.getInt().getExtValue();
 if (!isASCII(Char64))
   return false; // Bye bye, see you in integers.


Index: clang/test/SemaCXX/try-print-as-string-literal-type-check.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/try-print-as-string-literal-type-check.cpp
@@ -0,0 +1,17 @@
+// RUN: %clang_cc1 %s -std=c++20 -fsyntax-only -verify
+// expected-no-diagnostics
+
+// Reported by: https://github.com/llvm/llvm-project/issues/57013
+// The following code should not crash clang
+struct X {
+  char arr[2];
+  constexpr X() {}
+  constexpr void modify() {
+arr[0] = 0;
+  }
+};
+constexpr X f(X t) {
+t.modify();
+return t;
+}
+auto x = f(X());
Index: clang/lib/AST/APValue.cpp
===
--- clang/lib/AST/APValue.cpp
+++ clang/lib/AST/APValue.cpp
@@ -637,10 +637,10 @@
 return false;
 
   // Nothing we can do about a sequence that is not null-terminated
-  if (!Inits.back().getInt().isZero())
+  if (!Inits.back().isInt() || !Inits.back().getInt().isZero())
 return false;
-  else
-Inits = Inits.drop_back();
+
+  Inits = Inits.drop_back();
 
   llvm::SmallString<40> Buf;
   Buf.push_back('"');
@@ -655,6 +655,8 @@
   }
 
   for (auto &Val : Inits) {
+if (!Val.isInt())
+  return false;
 int64_t Char64 = Val.getInt().getExtValue();
 if (!isASCII(Char64))
   return false; // Bye bye, see you in integers.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D131314: [clang] format string checks for `InitListExpr`

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D131314#3707131 , @inclyc wrote:

> ping

FWIW, we usually only ping a review that hasn't had any activity in a week or 
more (it's not uncommon for reviews to sit for a few days while people think 
about them or reviewers are busy on other stuff).

I've not had the chance to do an in-depth review yet, but I have two thoughts I 
can share early on. I think you can simplify the patch a little bit by treating 
a string literal and an initializer list the same way (using an iterator to 
walk over the elements) instead of ginning up a fake `StringLiteral` AST node 
(that's a very heavy handed way to implement this). However, even with that 
simplification, I'm not certain the use cases for the diagnostic happen enough 
to warrant this large of a change in how we process format strings. I had 
encouraged such a diagnostic given the equivalence of the construct with string 
literals, but I was imagining that the support would be a few lines of code 
rather than anything substantial like this. Perhaps you can find a way to make 
the changes less invasive, but if not, I think we may want to hold off on this 
change until a user files an issue pointing out some real world code that would 
be easier to fix if they had such a diagnostic.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D131314

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