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

[Stefan Haller via Phabricator via cfe-commits]

stefanhaller added a comment.

In D124715#3488447 , @sammccall wrote:

> (How) does this interact with battery vs mains power on laptops?
> It seems like there's a common scenario where:
>
> - the user is on a relatively slow laptop, running off battery
> - the codebase is large, and indexing is unlikely to finish within an editing 
> session
>
> In this case, it seems like only using efficiency cores is what you'd want, 
> and that people are likely to be upset if clangd 15 keeps their performance 
> cores running at all times.

I can only speak for myself here: I'm a laptop user myself, and I work on 
battery the majority of the day; but I still wouldn't trade indexing speed for 
saving battery. The clangd index is so essential for my work that I always want 
it to be available as quickly as possible.

> Reading the docs 
> 
>  it seems like background is the intended QoS for this type of work ("such as 
> indexing"..."minutes or hours").

How long it takes is only one aspect of it. Other clues from the documentation:

Utility: "typically have a progress bar that is visible to the user. Focuses on 
providing a balance between responsiveness, performance, and energy efficiency."
Background: "isn't visible to the user. Focuses on energy efficiency."

The way I read it, Background is intended for tasks that don't make a 
significant difference to the functionality of the software. An example might 
be face detection in the Photos app; as long as it is not finished, it only 
affects a small part of the functionality, the rest of the app is perfectly 
usable. That's not the case for clangd, the software is pretty much not usable 
without an index.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124715

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

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

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/InlayHints.cpp:474
+// the parameter names from the wrapped function
+if (Args.size() > FixedParamCount && Args.size() == Params.size()) {
+  auto ForwardedParams = matchForwardedParams(

upsj wrote:
> nridge wrote:
> > What is the reason for the `Args.size() == Params.size()` condition?
> > 
> > Doesn't this break cases where there is more than one argument matching the 
> > variadic parameter? For example:
> > 
> > ```
> > void foo(int a, int b);
> > template 
> > void bar(Args&&... args) { foo(args...); }
> > int main() {
> >   bar(1, 2);
> > }
> > ```
> > 
> > Here, I expect we'd have `Args.size() == 2` and `Params.size() == 1`.
> > 
> > (We should probably have test coverage for such multiple-arguments cases as 
> > well. We probably don't need it for all combinations, but we should have at 
> > least one test case exercising it.)
> The function we are looking at is an already instantiated template. The check 
> `Args.size() == Params.size()` is only necessary because of va_args
Ah, thanks, I overlooked that. A lot of things in this patch that initially 
confused me make a lot more sense now :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124690

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

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header and rename it .

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

balazske added inline comments.



Comment at: clang/include/clang/AST/ASTImportError.h:17
+
+#include "clang/AST/APValue.h"
+#include "llvm/Support/Error.h"

This header is probably not needed.



Comment at: clang/include/clang/AST/ASTImportError.h:22
+
+class ImportError : public llvm::ErrorInfo {
+public:

balazske wrote:
> Rename to `ASTImportError`.
The rename to `ASTImportError` is better in a separate patch. The rename would 
touch code in much more places and there is a rule to make independent changes 
in separate patches. Still I think this class should not be called just 
`ImportError` if it has an own header (it is not an internal-like class of 
`ASTImporter` as is used to be).



Comment at: clang/include/clang/AST/ASTImporterLookupTable.h:17
 
+#include "clang/AST/ASTImportError.h"
 #include "clang/AST/DeclBase.h" // lookup_result

phyBrackets wrote:
> balazske wrote:
> > This include is not needed here.
> Hey thanks for reviewing , I have a doubt if I remove this include it from 
> here , then I need to include this header in the ASTImporterSharedState.h , 
> now the ASTImporterSharedState.h and ASTImporter.h both have ASTImportError.h 
> included , isn't this affect on ASTImporter.cpp file on conflicting over 
> ASTImportError ? Strangely it builds fine on my system tho .
> isn't this affect on ASTImporter.cpp file on conflicting over ASTImportError ?
This is no problem, this is why the include guard `#ifndef 
LLVM_CLANG_AST_ASTIMPORTERROR_H` is there.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

[PATCH] D124807: [clang][dataflow] Only skip ExprWithCleanups when visiting terminators

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:230
+  ///
+  /// Requirements:
+  ///

Why add these as requirements instead of skipping past `ExprWithCleanups` 
internally, as we currently do for parens? Should we add similar requirements 
to `createStorageLocation`, `setStorageLocation`, and `getStorageLocation`? 
Would that result in lots of client code sprinkled with `IgnoreParens` and some 
form of `ignoreExprWithCleanups`?



Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:38
 ///
-///  The type of `S` must not be `ParenExpr`.
+///  The type of `S` must not be `ParenExpr` or `ExprWithCleanups`.
 void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env);

For consistency with the other comment.



Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:41
 
+/// Skip past any `ExprWithCleanups` which might surround `E`. Returns a null
+/// pointer if `E` is null.

I find the use of "any" confusing here. Let's clarify that it skips past at 
most one instance of `ExprWithCleanups`.



Comment at: clang/include/clang/Analysis/FlowSensitive/Transfer.h:41-42
 
+/// Skip past any `ExprWithCleanups` which might surround `E`. Returns a null
+/// pointer if `E` is null.
+///

sgatev wrote:
> I find the use of "any" confusing here. Let's clarify that it skips past at 
> most one instance of `ExprWithCleanups`.




Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:429-430
 Value *Environment::getValue(const Expr &E, SkipPast SP) const {
+  assert(!isa(&E));
+  assert(!isa(&E));
   auto *Loc = getStorageLocation(E, SP);





Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:224
+  if (auto *OptionalVal = cast_or_null(State.Env.getValue(
+  *ObjectExpr->IgnoreParens(), SkipPast::ReferenceThenPointer))) {
 auto *HasValueVal = getHasValue(OptionalVal);

Do we have a test that fails if `IgnoreParens` is missing? If not, let's add a 
test or extend one of the existing. Perhaps 
https://github.com/llvm/llvm-project/blob/main/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp#L1286



Comment at: clang/lib/Analysis/FlowSensitive/Transfer.cpp:607-608
 void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env) {
   assert(!isa(&S));
+  assert(!isa(&S));
   TransferVisitor(StmtToEnv, Env).Visit(&S);





Comment at: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp:78
+/// and integers in the framework.
+static const Expr *ignoreParenImpCastsExceptToBool(const Expr *E) {
+  const Expr *LastE = nullptr;

xazax.hun wrote:
> li.zhe.hua wrote:
> > sgatev wrote:
> > > xazax.hun wrote:
> > > > li.zhe.hua wrote:
> > > > > sgatev wrote:
> > > > > > I don't recall why we need to ignore implicit casts here. Can't we 
> > > > > > ignore parens and rely on the built-in transfer function, possibly 
> > > > > > adding handling of some missing casts there? 
> > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Analysis/FlowSensitive/Transfer.cpp#L192
> > > > > If we only ignore parens, a test in the optional checker tests begins 
> > > > > to fail, specifically 
> > > > > `UncheckedOptionalAccessTest.ValueOrComparison`. The missing "cast" 
> > > > > is an `ExprWithCleanups`. I didn't know how to deal with that, so 
> > > > > this patch just working around the assert.
> > > > In general, I prefer to handle as much as possible with transfer 
> > > > functions and skip as little as possible in the AST. We might skip 
> > > > `ExprWithCleanups` nodes today, but we will need them tomorrow to 
> > > > properly model where certain destructors are being invoked. 
> > > We already have `skipExprWithCleanups` [1]. I wonder if it makes sense to 
> > > replace that with a simple transfer function like the one for the 
> > > `CK_NoOp` implicit cast. Would that solve the problem and remove the need 
> > > for `ignoreParenImpCastsExceptToBool`? In the future we might replace 
> > > that transfer function with a proper one, as Gábor suggested.
> > > 
> > > [1] 
> > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Analysis/FlowSensitive/Transfer.cpp#L36
> > I sat down with @ymandel to get a better understanding of what was 
> > happening here, which was really helpful in understanding why my random 
> > flailing was making tests pass, and what I believe to be a better way 
> > forward.
> > 
> > So, the CFG does not emit `ParenExpr` nodes. As such, the transfer function 
> > never sees them, and so when we do any kind of manual traversal (e.g. to 
> > look at a sub-expression), we use `ignoreParens()` because they effectively 
> > don't exist. The 

[PATCH] D124638: [clang] Track how headers get included generally during lookup time

[Jan Svoboda via Phabricator via cfe-commits]

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

LGTM, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124638

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

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header and rename it .

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added inline comments.



Comment at: clang/include/clang/AST/ASTImportError.h:22
+
+class ImportError : public llvm::ErrorInfo {
+public:

balazske wrote:
> balazske wrote:
> > Rename to `ASTImportError`.
> The rename to `ASTImportError` is better in a separate patch. The rename 
> would touch code in much more places and there is a rule to make independent 
> changes in separate patches. Still I think this class should not be called 
> just `ImportError` if it has an own header (it is not an internal-like class 
> of `ASTImporter` as is used to be).
So, what do you suggest , for this patch should I go with name //ImportError// 
only ? And rename it as //ASTImportError// in a separate patch ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header and rename it .

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets added inline comments.



Comment at: clang/include/clang/AST/ASTImporterLookupTable.h:17
 
+#include "clang/AST/ASTImportError.h"
 #include "clang/AST/DeclBase.h" // lookup_result

balazske wrote:
> phyBrackets wrote:
> > balazske wrote:
> > > This include is not needed here.
> > Hey thanks for reviewing , I have a doubt if I remove this include it from 
> > here , then I need to include this header in the ASTImporterSharedState.h , 
> > now the ASTImporterSharedState.h and ASTImporter.h both have 
> > ASTImportError.h included , isn't this affect on ASTImporter.cpp file on 
> > conflicting over ASTImportError ? Strangely it builds fine on my system tho 
> > .
> > isn't this affect on ASTImporter.cpp file on conflicting over 
> > ASTImportError ?
> This is no problem, this is why the include guard `#ifndef 
> LLVM_CLANG_AST_ASTIMPORTERROR_H` is there.
> 
> 
ahh yes! Thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header and rename it .

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

balazske added inline comments.



Comment at: clang/include/clang/AST/ASTImportError.h:1
+//===- ASTImporter.h - Import Error while Importing AST ---*- C++ -*-===//
+//

balazske wrote:
> 




Comment at: clang/include/clang/AST/ASTImportError.h:22
+
+class ImportError : public llvm::ErrorInfo {
+public:

phyBrackets wrote:
> balazske wrote:
> > balazske wrote:
> > > Rename to `ASTImportError`.
> > The rename to `ASTImportError` is better in a separate patch. The rename 
> > would touch code in much more places and there is a rule to make 
> > independent changes in separate patches. Still I think this class should 
> > not be called just `ImportError` if it has an own header (it is not an 
> > internal-like class of `ASTImporter` as is used to be).
> So, what do you suggest , for this patch should I go with name 
> //ImportError// only ? And rename it as //ASTImportError// in a separate 
> patch ?
Yes: Do not change the class name here (but header should be called 
ASTImportError.h). In a next patch make only a rename.



Comment at: clang/include/clang/AST/ASTImportError.h:52
+#endif // LLVM_CLANG_AST_ASTIMPORTERROR_H
\ No newline at end of file


Please add newline. I do not know if this can cause buildbot failures.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

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

[PATCH] D124669: [flang][driver] Add support for -save-temps

[Diana Picus via Phabricator via cfe-commits]

rovka added inline comments.



Comment at: flang/test/Driver/fno-integrated-as.f90:6
+!--
+! Verify that there _is_ a seperate line with an assembler invocation
+! RUN: %flang -c -fno-integrated-as %s -### 2>&1 | FileCheck %s





Comment at: flang/test/Driver/fno-integrated-as.f90:10
+! CHECK-SAME: "-o" "[[assembly_file:.*]].s"
+! CHECK-NEXT: "-o" "fno-integrated-as.o" "[[assembly_file:.*]].s"
+





Comment at: flang/test/Driver/fno-integrated-as.f90:18
+! DEFAULT-LABEL: "-fc1"
+! DEFAULT-SAME: "-o" "fno-integrated-as.o" "{{.*}}fno-integrated-as.f90"

Nit (here and above): Is it a rule that `-o blah` comes right before the input 
file, or should we also add a {{.*}} in between in case other flags sneak in?



Comment at: flang/test/Driver/save-temps.f90:1
+! Tests for the `-save-temps` flag. As `flang` does not implement `-fc1as` 
(i.e. a driver for the intergrated assembly), we need to
+! use `-fno-integrated-as` here.




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124669

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

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

[Peter Waller via Phabricator via cfe-commits]

peterwaller-arm added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64FrameLowering.cpp:752
+#undef CASE
+  }
+}

void wrote:
> kristof.beyls wrote:
> > Just a drive-by comment: I'm wondering if SVE registers should also be 
> > listed here?
> I'm not familiar with the SVE registers (I assume you mean the `Z#` and `P#` 
> ones). Could you give an example program?
SVE is slightly tricker here because the set of registers the caller must 
preserve depends on the signature of the function.

This is described here: 
https://github.com/ARM-software/abi-aa/blob/8a7b266879c60ca1c76e94ebb279b2dac60ed6a5/aapcs64/aapcs64.rst#613scalable-vector-registers

The callee-preserved registers are z8-z23 and p4-p15 if the function is using 
the VARIANT_PCS, the code for that condition in the asm printer is here:

https://github.com/llvm/llvm-project/blob/78fd413cf736953ac623cabf3d5f84c8219e31f8/llvm/lib/Target/AArch64/AArch64AsmPrinter.cpp#L864-L875


```
 if (MF->getFunction().getCallingConv() == CallingConv::AArch64_VectorCall ||
  MF->getFunction().getCallingConv() ==
  CallingConv::AArch64_SVE_VectorCall ||
  STI->getRegisterInfo()->hasSVEArgsOrReturn(MF)) {
```

Hope that helps a little.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124836

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

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

Can we have a test for this, got idea from here 
(https://stackoverflow.com/questions/4129961/how-is-the-size-of-a-struct-with-bit-fields-determined-measured)

  typedef struct
  {
  unsigned int a:1;
  unsigned int x:31;
  unsigned int c:1;
  int b[2];
  } mystruct;
  ...
  ff.b[0] = 3;
  clang_analyzer_eval(*((int *)pff + 2) == 3); // expected-warning{{TRUE}}  // 
Or should this be `pff + 3` ???




Comment at: clang/test/Analysis/array-struct-region.c:1
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config 
eagerly-assume=false %s
 

Should we pin the target, shouldn't we?



Comment at: clang/test/Analysis/array-struct-region.c:365
+  BITFIELD_CAST *pff = &ff;
+  clang_analyzer_eval(*((int *)pff + 1) == 0); // expected-warning{{TRUE}}
+  ff.b[0] = 3;

My gut feeling is that we should pin the target  for these arithmetics.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124349

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

[PATCH] D124666: In MSVC compatibility mode, handle unqualified templated base class initialization

[Fred Tingaud via Phabricator via cfe-commits]

frederic-tingaud-sonarsource updated this revision to Diff 426944.
frederic-tingaud-sonarsource added a comment.

Sorry about the confusion, it makes sense. Fixed.


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

https://reviews.llvm.org/D124666

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/test/SemaTemplate/ms-unqualified-base-class.cpp

Index: clang/test/SemaTemplate/ms-unqualified-base-class.cpp
===
--- /dev/null
+++ clang/test/SemaTemplate/ms-unqualified-base-class.cpp
@@ -0,0 +1,85 @@
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility -fsyntax-only -verify=before,expected %s
+// RUN: %clang_cc1 -std=c++17 -fms-compatibility -fdelayed-template-parsing -fsyntax-only -verify=before,expected %s
+// RUN: %clang_cc1 -std=c++20 -fms-compatibility -fsyntax-only -verify=after,expected %s
+// RUN: %clang_cc1 -std=c++20 -fms-compatibility -fdelayed-template-parsing -fsyntax-only -verify=after,expected %s
+
+template 
+class Base {
+};
+
+template 
+class Based {}; // Trying to trick the typo detection
+
+template 
+class Derived : public Base {
+public:
+  // after-error@+1 {{member initializer 'Base' does not name a non-static data member or base class}}
+  Derived() : Base() {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+private:
+  int Baze; // Trying to trick the typo detection
+};
+
+template  struct AggregateBase {
+  T i;
+};
+
+template 
+struct AggregateDerived : public AggregateBase {
+  int i;
+
+  // after-error@+1 {{member initializer 'AggregateBase' does not name a non-static data member or base class}}
+  AggregateDerived(T j) : AggregateBase{4}, i{j} {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+  int f() {
+return i + AggregateBase::i; // expected-warning {{use of undeclared identifier 'AggregateBase'; unqualified lookup into dependent bases of class template 'AggregateDerived' is a Microsoft extension}}
+  }
+};
+
+template  struct MultiTypesBase {
+};
+
+template 
+struct MultiTypesDerived : public MultiTypesBase {
+  // after-error@+1 {{member initializer 'MultiTypesBase' does not name a non-static data member or base class}}
+  MultiTypesDerived() : MultiTypesBase{} {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template  struct IntegerBase {
+};
+
+template 
+struct IntegerDerived : public IntegerBase {
+  // after-error@+1 {{member initializer 'IntegerBase' does not name a non-static data member or base class}}
+  IntegerDerived() : IntegerBase{} {} // before-warning {{unqualified base initializer of class templates is a Microsoft extension}}
+};
+
+template  struct ConformingBase {
+  T i;
+};
+
+template 
+struct ConformingDerived : public ConformingBase {
+  int i;
+
+  ConformingDerived(T j) : ConformingBase{4}, i{j} {}
+  int f() {
+return i + ConformingBase::i;
+  }
+};
+
+int main() {
+  int I;
+  Derived t;
+
+  AggregateDerived AD{2};
+  AD.AggregateBase::i = 3;
+  I = AD.f();
+
+  MultiTypesDerived MTD;
+
+  IntegerDerived<4> ID;
+
+  ConformingDerived CD{2};
+  I = CD.f();
+
+  return I;
+}
Index: clang/lib/Sema/SemaDeclCXX.cpp
===
--- clang/lib/Sema/SemaDeclCXX.cpp
+++ clang/lib/Sema/SemaDeclCXX.cpp
@@ -4307,6 +4307,15 @@
 }
   }
 
+  if (getLangOpts().MSVCCompat && !getLangOpts().CPlusPlus20) {
+auto UnqualifiedBase = R.getAsSingle();
+if (UnqualifiedBase) {
+  Diag(IdLoc, diag::ext_unqualified_base_class)
+  << SourceRange(IdLoc, Init->getSourceRange().getEnd());
+  BaseType = UnqualifiedBase->getInjectedClassNameSpecialization();
+}
+  }
+
   // If no results were found, try to correct typos.
   TypoCorrection Corr;
   MemInitializerValidatorCCC CCC(ClassDecl);
Index: clang/include/clang/Basic/DiagnosticSemaKinds.td
===
--- clang/include/clang/Basic/DiagnosticSemaKinds.td
+++ clang/include/clang/Basic/DiagnosticSemaKinds.td
@@ -5508,6 +5508,9 @@
 def ext_found_later_in_class : ExtWarn<
   "use of member %0 before its declaration is a Microsoft extension">,
   InGroup;
+def ext_unqualified_base_class : ExtWarn<
+  "unqualified base initializer of class templates is a Microsoft extension">,
+  InGroup;
 def note_dependent_member_use : Note<
   "must qualify identifier to find this declaration in dependent base class">;
 def err_not_found_by_two_phase_lookup : Error<"call to function %0 that is neither "
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libunwind] 7167237 - [libunwind][SystemZ] Unwind out of signal handlers

[Ulrich Weigand via cfe-commits]

Author: Ulrich Weigand
Date: 2022-05-04T10:43:11+02:00
New Revision: 71672375fe91d602699ae2a6d6a88e910ff91b5c

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

LOG: [libunwind][SystemZ] Unwind out of signal handlers

Unwinding out of signal handlers currently does not work since
the sigreturn trampoline is not annotated with CFI data.

Fix this by detecting the sigreturn trampoline during unwinding
and providing appropriate unwind data manually. This follows
closely the approach used by existing code for the AArch64 target.

Reviewed by: MaskRay

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

Added: 


Modified: 
libunwind/src/UnwindCursor.hpp
libunwind/test/signal_unwind.pass.cpp
libunwind/test/unwind_leaffunction.pass.cpp

Removed: 




diff  --git a/libunwind/src/UnwindCursor.hpp b/libunwind/src/UnwindCursor.hpp
index 655e41d81d5eb..7e56eff471596 100644
--- a/libunwind/src/UnwindCursor.hpp
+++ b/libunwind/src/UnwindCursor.hpp
@@ -953,7 +953,7 @@ class UnwindCursor : public AbstractUnwindCursor{
   }
 #endif
 
-#if defined(_LIBUNWIND_TARGET_LINUX) && defined(_LIBUNWIND_TARGET_AARCH64)
+#if defined(_LIBUNWIND_TARGET_LINUX) && (defined(_LIBUNWIND_TARGET_AARCH64) || 
defined(_LIBUNWIND_TARGET_S390X))
   bool setInfoForSigReturn() {
 R dummy;
 return setInfoForSigReturn(dummy);
@@ -962,8 +962,14 @@ class UnwindCursor : public AbstractUnwindCursor{
 R dummy;
 return stepThroughSigReturn(dummy);
   }
+  #if defined(_LIBUNWIND_TARGET_AARCH64)
   bool setInfoForSigReturn(Registers_arm64 &);
   int stepThroughSigReturn(Registers_arm64 &);
+  #endif
+  #if defined(_LIBUNWIND_TARGET_S390X)
+  bool setInfoForSigReturn(Registers_s390x &);
+  int stepThroughSigReturn(Registers_s390x &);
+  #endif
   template  bool setInfoForSigReturn(Registers &) {
 return false;
   }
@@ -1258,7 +1264,7 @@ class UnwindCursor : public AbstractUnwindCursor{
   unw_proc_info_t  _info;
   bool _unwindInfoMissing;
   bool _isSignalFrame;
-#if defined(_LIBUNWIND_TARGET_LINUX) && defined(_LIBUNWIND_TARGET_AARCH64)
+#if defined(_LIBUNWIND_TARGET_LINUX) && (defined(_LIBUNWIND_TARGET_AARCH64) || 
defined(_LIBUNWIND_TARGET_S390X))
   bool _isSigReturn = false;
 #endif
 };
@@ -2465,7 +2471,7 @@ int UnwindCursor::stepWithTBTable(pint_t pc, 
tbtable *TBTable,
 
 template 
 void UnwindCursor::setInfoBasedOnIPRegister(bool isReturnAddress) {
-#if defined(_LIBUNWIND_TARGET_LINUX) && defined(_LIBUNWIND_TARGET_AARCH64)
+#if defined(_LIBUNWIND_TARGET_LINUX) && (defined(_LIBUNWIND_TARGET_AARCH64) || 
defined(_LIBUNWIND_TARGET_S390X))
   _isSigReturn = false;
 #endif
 
@@ -2580,7 +2586,7 @@ void UnwindCursor::setInfoBasedOnIPRegister(bool 
isReturnAddress) {
   }
 #endif // #if defined(_LIBUNWIND_SUPPORT_DWARF_UNWIND)
 
-#if defined(_LIBUNWIND_TARGET_LINUX) && defined(_LIBUNWIND_TARGET_AARCH64)
+#if defined(_LIBUNWIND_TARGET_LINUX) && (defined(_LIBUNWIND_TARGET_AARCH64) || 
defined(_LIBUNWIND_TARGET_S390X))
   if (setInfoForSigReturn())
 return;
 #endif
@@ -2653,6 +2659,104 @@ int UnwindCursor::stepThroughSigReturn(Registers_arm64 &) {
 }
 #endif // defined(_LIBUNWIND_TARGET_LINUX) && 
defined(_LIBUNWIND_TARGET_AARCH64)
 
+#if defined(_LIBUNWIND_TARGET_LINUX) && defined(_LIBUNWIND_TARGET_S390X)
+template 
+bool UnwindCursor::setInfoForSigReturn(Registers_s390x &) {
+  // Look for the sigreturn trampoline. The trampoline's body is a
+  // specific instruction (see below). Typically the trampoline comes from the
+  // vDSO (i.e. the __kernel_[rt_]sigreturn function). A libc might provide its
+  // own restorer function, though, or user-mode QEMU might write a trampoline
+  // onto the stack.
+  const pint_t pc = static_cast(this->getReg(UNW_REG_IP));
+  const uint16_t inst = _addressSpace.get16(pc);
+  if (inst == 0x0a77 || inst == 0x0aad) {
+_info = {};
+_info.start_ip = pc;
+_info.end_ip = pc + 2;
+_isSigReturn = true;
+return true;
+  }
+  return false;
+}
+
+template 
+int UnwindCursor::stepThroughSigReturn(Registers_s390x &) {
+  // Determine current SP.
+  const pint_t sp = static_cast(this->getReg(UNW_REG_SP));
+  // According to the s390x ABI, the CFA is at (incoming) SP + 160.
+  const pint_t cfa = sp + 160;
+
+  // Determine current PC and instruction there (this must be either
+  // a "svc __NR_sigreturn" or "svc __NR_rt_sigreturn").
+  const pint_t pc = static_cast(this->getReg(UNW_REG_IP));
+  const uint16_t inst = _addressSpace.get16(pc);
+
+  // Find the addresses of the signo and sigcontext in the frame.
+  pint_t pSigctx = 0;
+  pint_t pSigno = 0;
+
+  // "svc __NR_sigreturn" uses a non-RT signal trampoline frame.
+  if (inst == 0x0a77) {
+// Layout of a non-RT signal trampoline frame, starting at the CFA:
+/

[PATCH] D123831: [clang][extract-api] Use relative includes

[Daniel Grumberg via Phabricator via cfe-commits]

dang accepted this revision.
dang added a comment.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123831

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

[PATCH] D124909: Fixes following incompatibility issues with c++ modules

[Kugan Vivekanandarajah via Phabricator via cfe-commits]

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

1.

Preamble generated by clangd is generated with WriteCommentListToPCH = false;
However, if we are using precompiled modules that have comments in the
serialised AST, following sanity check in the clangd fails.

// Sanity check that the comment does not come from the PCH. We choose to not
// write them into PCH, because they are racy and slow to load.
assert(!Ctx.getSourceManager().isLoadedSourceLocation(RC->getBeginLoc()));

In this patch when we are generating Preamble with
WriteCommentListToPCH, we do not load the comments from AST.

1.

If we have modules that are built with RetainCommentsFromSystemHeaders
difference, that wouldn prevent modules from getting loaded. Since this
difference is not critical that should be stopping modules from being loaded,
this patch changes it to be a COMPATIBLE_LANGOPT.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124909

Files:
  clang-tools-extra/clangd/test/modules-options-compatablity-test/A.h
  clang-tools-extra/clangd/test/modules-options-compatablity-test/B.h
  
clang-tools-extra/clangd/test/modules-options-compatablity-test/compile_commands.json
  
clang-tools-extra/clangd/test/modules-options-compatablity-test/definition.jsonrpc
  
clang-tools-extra/clangd/test/modules-options-compatablity-test/module.modulemap
  clang-tools-extra/clangd/test/modules-options-compatablity-test/use.c
  clang-tools-extra/clangd/test/modules-options-compatablity.test
  clang/include/clang/Basic/LangOptions.def
  clang/lib/Serialization/ASTReader.cpp

Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3079,6 +3079,10 @@
   return Err;
 if (llvm::Error Err = ReadBlockAbbrevs(C, COMMENTS_BLOCK_ID))
   return Err;
+if (PP.getPreprocessorOpts().GeneratePreamble &&
+!PP.getPreprocessorOpts().WriteCommentListToPCH) {
+  break;
+}
 CommentsCursors.push_back(std::make_pair(C, &F));
 break;
   }
Index: clang/include/clang/Basic/LangOptions.def
===
--- clang/include/clang/Basic/LangOptions.def
+++ clang/include/clang/Basic/LangOptions.def
@@ -387,7 +387,7 @@
 
 LANGOPT(XLPragmaPack, 1, 0, "IBM XL #pragma pack handling")
 
-LANGOPT(RetainCommentsFromSystemHeaders, 1, 0, "retain documentation comments from system headers in the AST")
+COMPATIBLE_LANGOPT(RetainCommentsFromSystemHeaders, 1, 0, "retain documentation comments from system headers in the AST")
 
 LANGOPT(SanitizeAddressFieldPadding, 2, 0, "controls how aggressive is ASan "
"field padding (0: none, 1:least "
Index: clang-tools-extra/clangd/test/modules-options-compatablity.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity.test
@@ -0,0 +1,10 @@
+
+# RUN: rm -rf %t && mkdir -p %t
+# RUN: cp -r %S/modules-options-compatablity-test/* %t
+# RUN: mkdir -p %t/prebuilt
+# RUN: sed -i -e "s|DIRECTORY|%t|g" %t/definition.jsonrpc
+# RUN: sed -i -e "s|DIRECTORY|%t|g" %t/compile_commands.json
+# RUN: %clang -cc1 -emit-module -o %t/prebuilt/A.pcm -fmodules %t/module.modulemap -fmodule-name=A
+# RUN: %clang -cc1 -fretain-comments-from-system-headers -emit-module -o %t/prebuilt/B.pcm -fmodules %t/module.modulemap -fmodule-name=B -fprebuilt-module-path=%t/prebuilt
+# RUN: rm %t/*.h
+# RUN: clangd -background-index -lit-test < %t/definition.jsonrpc
Index: clang-tools-extra/clangd/test/modules-options-compatablity-test/use.c
===
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity-test/use.c
@@ -0,0 +1,5 @@
+/* use */
+#include 
+#include 
+
+void use() { a(); }
Index: clang-tools-extra/clangd/test/modules-options-compatablity-test/module.modulemap
===
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity-test/module.modulemap
@@ -0,0 +1,8 @@
+/* module.modulemap */
+module A {
+  header "A.h"
+}
+module B {
+  header "B.h"
+  export *
+}
Index: clang-tools-extra/clangd/test/modules-options-compatablity-test/definition.jsonrpc
===
--- /dev/null
+++ clang-tools-extra/clangd/test/modules-options-compatablity-test/definition.jsonrpc
@@ -0,0 +1,28 @@
+{
+  "jsonrpc": "2.0",
+"id": 0,
+"method": "initialize",
+"params": {
+  "processId": 123,
+  "rootPath": "clangd",
+  "capabilities": { "w

[PATCH] D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice.

[Nathan James via Phabricator via cfe-commits]

njames93 marked 4 inline comments as done.
njames93 added inline comments.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:51
 ClangTidyContext *Context)
-: NamePrefix(CheckName.str() + "."), CheckOptions(CheckOptions),
+: NamePrefix((CheckName + ".").str()), CheckOptions(CheckOptions),
   Context(Context) {}

LegalizeAdulthood wrote:
> Why is `StringRef::operator+` considered "better" than 
> `std::string::operator+`?
Because 2 strings are created in the second instance, whereas only one is 
created in the first.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:56
 ClangTidyCheck::OptionsView::get(StringRef LocalName) const {
-  const auto &Iter = CheckOptions.find(NamePrefix + LocalName.str());
+  const auto &Iter = CheckOptions.find((NamePrefix + LocalName).str());
   if (Iter != CheckOptions.end())

LegalizeAdulthood wrote:
> `find` takes a `StringRef` so why convert to `std::string` here?
Because a concatenation of StringRef results in a Twine, which cannot be passed 
to find.



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.cpp:123
 StringRef Value) const {
-  Options[NamePrefix + LocalName.str()] = Value;
+  Options[(NamePrefix + LocalName).str()] = Value;
 }

LegalizeAdulthood wrote:
> `operator[]` takes a `StringRef` so why convert to `std::string` here?
Again can't use a Twine for operator[].



Comment at: clang-tools-extra/clang-tidy/ClangTidyCheck.h:158
 /// ``None``.
-llvm::Optional get(StringRef LocalName) const;
+llvm::Optional get(StringRef LocalName) const;
 

LegalizeAdulthood wrote:
> I think I would feel safer if the changes to the infrastructure were pushed 
> separately from the changes to the checks, with some "bake time" in between 
> so we have more confidence in the changes.
> 
> While debugging my own checks, I've seen that there are surprises with the 
> lifetimes of `StringRef`.
These changes on their own don't make any sense without the corresponding 
changes to the checks.
Also the lifetime of all the checks options is tied to the ClangTidyOptions 
which outlives the checks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124341

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

[PATCH] D124850: [Sema][SVE2] Move/simplify Sema testing for SVE2 ACLE builtins

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added a comment.

Thanks for working on this @RosieSumpter!




Comment at: clang/test/Sema/aarch64-sve2-intrinsics/acle_sve2.cpp:1
+// RUN: %clang_cc1 -triple aarch64-none-linux-gnu -target-feature +sve 
-target-feature +bf16 -fallow-half-arguments-and-returns -fsyntax-only -verify 
-verify-ignore-unexpected=error,note %s
+// RUN: %clang_cc1 -DSVE_OVERLOADED_FORMS -triple aarch64-none-linux-gnu 
-target-feature +sve -target-feature +bf16 -fallow-half-arguments-and-returns 
-fsyntax-only -verify=overload -verify-ignore-unexpected=error,note %s

(see my commen later on as well) I'd remove this target feature and move those 
BF16 tests to a different file.



Comment at: clang/test/Sema/aarch64-sve2-intrinsics/acle_sve2.cpp:13
+
+void test_s8(svbool_t pg, int8_t op1, uint64_t op2, const int8_t *op3, uint8_t 
op4)
+{

It would be nice if these operands have slightly more descriptive names. 
Additionally, you may be able to allocate these as globals so that they don't 
need to be passed as operands to the test function, e.g.

 svbool_t pg;
 float f32;
 double f64;
 int32_t i32;
 int64_t i64;
 float *f32_ptr;
 double *f64_ptr;
 




Comment at: clang/test/Sema/aarch64-sve2-intrinsics/acle_sve2.cpp:4992
+
+void test_bf16(const bfloat16_t *op1)
+{

I think you'll need to move these to a separate test file where the RUN line 
has only `+sve2` (and not `+bf16`)
We'll probably want to do the same for the SVE bf16 tests


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124850

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

[PATCH] D124836: [AArch64] Add support for -fzero-call-used-regs

[Sander de Smalen via Phabricator via cfe-commits]

sdesmalen added inline comments.



Comment at: llvm/lib/Target/AArch64/AArch64RegisterInfo.td:1398
+ (sequence "W%u", 0, 7))>;
+def FPR8_ARG : RegisterClass<"AArch64", [untyped], 8, (trunc FPR8, 7)> {
+  let Size = 8;

Should this feature/attribute work with other calling conventions? If so, then 
it's probably best not to hard-code these values here, but rather to get them 
from the chosen calling convention for that particular function. The supported 
calling conventions are defined in AArch64CallingConvention.td.

For example, you could iterate all registers in GPR64/FPR128/ZPR/PPR register 
classes and zero their values if they are not marked as callee saved. You can 
query this information from the call by looking at it's callee-saved regmask 
(see for example `CSR_AArch64_AAPCS_RegMask` to see how those are defined 
defined).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124836

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

[PATCH] D124349: [analyzer] Get direct binding for specific punned case

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

steakhal added a comment.

In D124349#3490524 , @martong wrote:

> Can we have a test for this, got idea from here 
> (https://stackoverflow.com/questions/4129961/how-is-the-size-of-a-struct-with-bit-fields-determined-measured)
>
>   typedef struct
>   {
>   unsigned int a:1;
>   unsigned int x:31;
>   unsigned int c:1;
>   int b[2];
>   } mystruct;
>   ...
>   ff.b[0] = 3;
>   clang_analyzer_eval(*((int *)pff + 2) == 3); // expected-warning{{TRUE}}  
> // Or should this be `pff + 3` ???

Generally, you are right. But in this case, we are talking about a *single bit* 
bitfield.
That bitfield cannot span across multiple `unsigned` objects. And `int` is 
supposed to be at least one byte large, hence there is plenty of room for an 
additional `CHAR_BIT - 1` bits along with this one and we would be still 
portable.




Comment at: clang/test/Analysis/array-struct-region.c:1
 // RUN: %clang_analyze_cc1 
-analyzer-checker=core,alpha.core,debug.ExprInspection -verify -analyzer-config 
eagerly-assume=false %s
 

martong wrote:
> Should we pin the target, shouldn't we?
There is no need for that.
The `sizeof(int)` might change, but the `operator+` will accommodate for that 
in the pointer arithmetic. And the field after bitfields is by default aligned 
to its preferred alignment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124349

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

[PATCH] D123831: [clang][extract-api] Use relative includes

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

Since this is a new test can we use the approach in 
https://reviews.llvm.org/D124634 to check for diagnostics output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123831

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

[PATCH] D124341: [clang-tidy][NFC] Replace many instances of std::string where a StringRef would suffice.

[Aaron Ballman via Phabricator via cfe-commits]

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

In D124341#3476533 , @njames93 wrote:

> In D124341#3475083 , @aaron.ballman 
> wrote:
>
>> I went through the changes and they look correct to me... yet I'm still 
>> mildly terrified. :-D Have you tried running clang-tidy through its paces 
>> with ASan/MSan enabled to see if the change introduces any use-after-free 
>> issues in practice?
>
> I can't seem to build LLVM with msan at all. But ASan, which checks things 
> like use after free, ran all clang tools tests without a hitch.

Thanks for checking! I've convinced myself that I don't see any lifetime issues 
here (in each case, what the `StringRef` refers to should outlive the 
`StringRef`), so this LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124341

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

[PATCH] D124534: [clang] Add a diagnostic for line directive of a gnu extension

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D124534#3490098 , @ken-matsui 
wrote:

> @aaron.ballman
>
>> If so, I think putting Diag after the call of this function would be better.
>
> With the above change, I tried to add comments to failed tests, but there 
> were over 300 files.
>
> During my investigation, I found most tests printed warnings without file 
> information strangely.
>
>   error: 'warning' diagnostics seen but not expected: 
> Line 0: this style of line directive is a GNU extension
> Line 0: this style of line directive is a GNU extension
>   2 errors generated.

That is rather unexpected.

> If warnings have file information, it should be like:
>
>   error: 'warning' diagnostics seen but not expected: 
> File /tmp/llvm-project/clang/test/Preprocessor/line-directive.c Line 9: 
> this style of line directive is a GNU extension
>   1 error generated.
>
> Even tests that completely do not use preprocessor directives, such as 
> `clang/test/SemaCXX/matrix-type.cpp`, failed with the above strange warnings.
>
> Thus, I suspect that the warnings without file paths have come from 
> internally included SDK (I'm using macOS that includes 
> `/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX12.3.sdk`)
>  or something that is not related to target test files.
>
> What do you think?

The tests run in -cc1 mode and don't #include anything, so I don't think the 
issue is an internally included SDK. I think the issue could be from this: 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/InitPreprocessor.cpp#L1355
 and 
https://github.com/llvm/llvm-project/blob/main/clang/lib/Frontend/InitPreprocessor.cpp#L1368.
 You may have to hook up to a debugger to see why we're issuing those 
surprising warnings. If it turns out that it's these inserted directives, you 
may have to look at the source location of the digit token to see if 
`isWrittenInBuiltinFile()` is true or not (and we may need to also check 
`isWrittenInScratchSpace()` as well, perhaps).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124534

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

[PATCH] D124221: Reimplement `__builtin_dump_struct` in Sema.

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:433
+
+  llvm::StringRef getFormatSpecifier(QualType T) {
+if (auto *BT = T->getAs()) {

rsmith wrote:
> rsmith wrote:
> > aaron.ballman wrote:
> > > yihanaa wrote:
> > > > I think this is better maintained in "clang/AST/FormatString.h". For 
> > > > example analyze_printf::PrintfSpecifier can get format specifier, what 
> > > > do you all think about?
> > > +1 to this suggestion -- my hope is that we could generalize it more then 
> > > as I notice there are missing specifiers for things like intmax_t, 
> > > size_t, ptrdiff_t, _Decimal types, etc. Plus, that will hopefully make it 
> > > easier for __builtin_dump_struct to benefit when new format specifiers 
> > > are added, such as ones for printing a _BitInt.
> > I am somewhat uncertain: every one of these is making arbitrary choices 
> > about how to format the value, so it's not clear to me that this is general 
> > logic rather than something specific to `__builtin_dump_struct`. For 
> > example, using `%f` rather than one of the myriad other ways of formatting 
> > `double` is somewhat arbitrary. Using `%s` for any `const char*` is 
> > *extremely* arbitrary and will be wrong and will cause crashes in some 
> > cases, but it may be the pragmatically correct choice for a dumping 
> > utility. A general-purpose mechanism would use `%p` for all kinds of 
> > pointer.
> > 
> > We could perhaps factor out the formatting for cases where there is a clear 
> > canonical default formatting, such as for integer types and probably `%p` 
> > for pointers, then call that from here with some special-casing, but 
> > without a second consumer for that functionality it's really not clear to 
> > me what form it should take.
> I went ahead and did this, mostly to match concurrent changes to the old 
> implementation. There are a few cases where our existing "guess a format 
> specifier" logic does the wrong thing for dumping purposes, which I've 
> explicitly handled -- things like wanting to dump a `char` / `signed char` / 
> `unsigned char` member as a number rather than as a (potentially 
> non-printable or whitespace) character.
 When I was patching that old implementation, I found that for uint8_t, int8_t, 
Clang's existing "guess a format specifier" logic would treat the value as an 
integer, but for unsigned char, signed char, char types, it would Treat it as a 
character, please look at this example ( https://godbolt.org/z/ooqn4468T ), I 
guess this existing logic may have made some special judgment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124221

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

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman created this revision.
aaron.ballman added reviewers: erichkeane, rsmith, rjmccall.
Herald added a project: All.
aaron.ballman requested review of this revision.
Herald added a project: clang.

With sufficiently tortured code, it's possible to cause a stack overflow when 
parsing declarators. Thus, we now check for resource exhaustion when 
recursively parsing declarators so that we can at least warn the user we're 
about to crash before we actually crash.

This fixes https://github.com/llvm/llvm-project/issues/51642.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124915

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Parse/Parser.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/Parser.cpp
  clang/test/Parser/declarators-recursion-crash.c

Index: clang/test/Parser/declarators-recursion-crash.c
===
--- /dev/null
+++ clang/test/Parser/declarators-recursion-crash.c
@@ -0,0 +1,10 @@
+// RUN: not %clang_cc1 %s -fsyntax-only -verify
+
+#define PTR1 * * * * * * * * * *
+#define PTR2 PTR1 PTR1 PTR1 PTR1 PTR1 PTR1 PTR1 PTR1 PTR1 PTR1
+#define PTR3 PTR2 PTR2 PTR2 PTR2 PTR2 PTR2 PTR2 PTR2 PTR2 PTR2
+#define PTR4 PTR3 PTR3 PTR3 PTR3 PTR3 PTR3 PTR3 PTR3 PTR3 PTR3
+#define PTR5 PTR4 PTR4 PTR4 PTR4 PTR4 PTR4 PTR4 PTR4 PTR4 PTR4
+#define PTR6 PTR5 PTR5 PTR5 PTR5 PTR5 PTR5 PTR5 PTR5 PTR5 PTR5
+
+int PTR6 q3_var = 0; // expected-warning {{stack nearly exhausted; compilation time may suffer, and crashes due to stack overflow are likely}}
Index: clang/lib/Parse/Parser.cpp
===
--- clang/lib/Parse/Parser.cpp
+++ clang/lib/Parse/Parser.cpp
@@ -15,6 +15,7 @@
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/DeclTemplate.h"
 #include "clang/Basic/FileManager.h"
+#include "clang/Basic/Stack.h"
 #include "clang/Parse/ParseDiagnostic.h"
 #include "clang/Parse/RAIIObjectsForParser.h"
 #include "clang/Sema/DeclSpec.h"
@@ -2610,6 +2611,19 @@
   return false;
 }
 
+void Parser::warnStackExhausted(SourceLocation Loc) {
+  // Only warn about this once.
+  if (!Actions.WarnedStackExhausted) {
+Diag(Loc, diag::warn_stack_exhausted);
+Actions.WarnedStackExhausted = true;
+  }
+}
+
+void Parser::runWithSufficientStackSpace(SourceLocation Loc,
+ llvm::function_ref Fn) {
+  clang::runWithSufficientStackSpace([&] { warnStackExhausted(Loc); }, Fn);
+}
+
 bool BalancedDelimiterTracker::diagnoseOverflow() {
   P.Diag(P.Tok, diag::err_bracket_depth_exceeded)
 << P.getLangOpts().BracketDepth;
Index: clang/lib/Parse/ParseDecl.cpp
===
--- clang/lib/Parse/ParseDecl.cpp
+++ clang/lib/Parse/ParseDecl.cpp
@@ -5751,11 +5751,12 @@
 }
 
 /// ParseDeclarator - Parse and verify a newly-initialized declarator.
-///
 void Parser::ParseDeclarator(Declarator &D) {
   /// This implements the 'declarator' production in the C grammar, then checks
   /// for well-formedness and issues diagnostics.
-  ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
+  runWithSufficientStackSpace(D.getBeginLoc(), [&] {
+ParseDeclaratorInternal(D, &Parser::ParseDirectDeclarator);
+  });
 }
 
 static bool isPtrOperatorToken(tok::TokenKind Kind, const LangOptions &Lang,
@@ -5866,7 +5867,9 @@
   D.ExtendWithDeclSpec(DS);
 
   // Recurse to parse whatever is left.
-  ParseDeclaratorInternal(D, DirectDeclParser);
+  runWithSufficientStackSpace(D.getBeginLoc(), [&] {
+ParseDeclaratorInternal(D, DirectDeclParser);
+  });
 
   // Sema will have to catch (syntactically invalid) pointers into global
   // scope. It has to catch pointers into namespace scope anyway.
@@ -5915,7 +5918,8 @@
 D.ExtendWithDeclSpec(DS);
 
 // Recursively parse the declarator.
-ParseDeclaratorInternal(D, DirectDeclParser);
+runWithSufficientStackSpace(
+D.getBeginLoc(), [&] { ParseDeclaratorInternal(D, DirectDeclParser); });
 if (Kind == tok::star)
   // Remember that we parsed a pointer type, and remember the type-quals.
   D.AddTypeInfo(DeclaratorChunk::getPointer(
@@ -5960,7 +5964,8 @@
 }
 
 // Recursively parse the declarator.
-ParseDeclaratorInternal(D, DirectDeclParser);
+runWithSufficientStackSpace(
+D.getBeginLoc(), [&] { ParseDeclaratorInternal(D, DirectDeclParser); });
 
 if (D.getNumTypeObjects() > 0) {
   // C++ [dcl.ref]p4: There shall be no references to references.
Index: clang/include/clang/Parse/Parser.h
===
--- clang/include/clang/Parse/Parser.h
+++ clang/include/clang/Parse/Parser.h
@@ -791,6 +791,16 @@
 return PP.LookAhead(N-1);
   }
 
+  /// Warn that the stack is nearly exhausted.
+  void warnStackExhausted(SourceLocation Loc);
+
+  /// Run some code with "sufficient" stack space. (Currently, at least 256K is
+  /// guarante

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

[Matt Arsenault via Phabricator via cfe-commits]

arsenm added a comment.

I think I've seen this one before


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124915

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

[PATCH] D124915: Check for resource exhaustion when recursively parsing declarators

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

Two things to note:

1. I'm not convinced that adding a test case is the right thing to do here. On 
the one hand, we want to verify that we now emit the diagnostic before 
crashing. But on the other hand, this test is extremely expensive to run 
(because it exhausts the stack) and its behavior is highly specific to the test 
platform its being run on. If someone asked me to remove the test coverage 
here, I would not be sad.
2. The `runWithSufficientStackSpace()` implementation is a copy of what's 
already in Sema. I investigated lifting this interface to something more 
common, but the issue is with generating a diagnostic (there's no common 
diagnostic engine between `Sema` and `Parser` that I could find). Given the 
simplicity of the implementation, I figured it's reasonably defensible to 
duplicate the code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124915

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

[PATCH] D124909: Fix issues with using clangd with C++ modules

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

To back up a bit...

Until now I don't think we've had a serious effort/plan for supporting modules 
in clangd, or AFAIK in clang tools in general.
To the extent that modules work, it's a side-effect of how we use clang's 
driver + compilation database, and inherit behavior by default.
So we probably need a bit of discussion of what configurations should be 
supported, how they should be configured & interact with the build system etc.

If I understand right (not a modules expert) in a build system:

1. modules can be produced by the build system as a separate step, and passed 
to build actions (e.g. -fprebuilt-module-path or -fmodule-file)
2. or they can be built on demand by compile steps that use them in a 
clang-private cache dir (-fimplicit-modules)
3. or we can have a build with no modules at all

If I understand right, you want to support a build system that does 1.
This makes sense (I suspect we should support 2 as well one day, but we can 
call it out of scope for now)

In clangd we'll see the compile commands used by the build system, via e.g. 
compile_commands.json, and again we have several strategies:

- we can consume modules from the build system
- we can build the needed modules in some clangd-private storage
- we can disable modular compilation, and parse headers textually

I think this patch intends to support strategy 1 (for build systems that 
produce modules).

My concern here is that this is fragile:

- the PCM format is not stable, clangd can only consume module files if the 
build system is using clang and that clang is built from the exact same 
revision. This is hard to achieve for most users. (And compile_commands.json 
having module-related flags in it a signal that it is true)
- clangd may now/later parse headers in different modes, skip function bodies 
etc. The COMPATIBLE_LANGOPT changes hint at the problems we can run into trying 
to reuse modules - it seems likely we'll run into issues later (compatibility, 
but maybe also performance?)
- if the modules are produced by the build system, clangd doesn't get a chance 
to examine the source code or AST directly. How does the dynamic index work on 
modular code? How do we know its include structure?

There may well be good reasons to handle modules this way, but let's work out 
the implications before this ends up in production! :-)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124909

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

[PATCH] D124916: [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

[Phoebe Wang via Phabricator via cfe-commits]

pengfei created this revision.
pengfei added reviewers: FreddyYe, RKSimon, LuoYuanke, craig.topper.
Herald added a subscriber: StephenFan.
Herald added a project: All.
pengfei requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Fix uninitialized variables introduced by D116325 
.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124916

Files:
  clang/lib/Headers/cetintrin.h
  clang/test/CodeGen/X86/sse-builtins-constrained.c


Index: clang/test/CodeGen/X86/sse-builtins-constrained.c
===
--- clang/test/CodeGen/X86/sse-builtins-constrained.c
+++ clang/test/CodeGen/X86/sse-builtins-constrained.c
@@ -1,8 +1,8 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s 
--check-prefix=UNCONSTRAINED --check-prefix=COMMON --check-prefix=COMMONIR
 // RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -emit-llvm -o - 
-Wall -Werror | FileCheck %s --check-prefix=CONSTRAINED --check-prefix=COMMON 
--check-prefix=COMMONIR
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -S %s -o - -Wall -Werror | FileCheck %s 
--check-prefix=CHECK-ASM --check-prefix=COMMON
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -S %s -o - 
-Wall -Werror | FileCheck %s --check-prefix=CHECK-ASM --check-prefix=COMMON
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -S -o - -Wall -Werror | FileCheck %s 
--check-prefix=CHECK-ASM --check-prefix=COMMON
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -S -o - -Wall 
-Werror | FileCheck %s --check-prefix=CHECK-ASM --check-prefix=COMMON
 
 #ifdef STRICT
 // Test that the constrained intrinsics are picking up the exception
Index: clang/lib/Headers/cetintrin.h
===
--- clang/lib/Headers/cetintrin.h
+++ clang/lib/Headers/cetintrin.h
@@ -43,8 +43,11 @@
 }
 
 static __inline__ unsigned int __DEFAULT_FN_ATTRS _rdsspd_i32() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned int t;
   return __builtin_ia32_rdsspd(t);
+#pragma clang diagnostic pop
 }
 
 #ifdef __x86_64__
@@ -53,8 +56,11 @@
 }
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS _rdsspq_i64() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned long long t;
   return __builtin_ia32_rdsspq(t);
+#pragma clang diagnostic pop
 }
 #endif /* __x86_64__ */
 


Index: clang/test/CodeGen/X86/sse-builtins-constrained.c
===
--- clang/test/CodeGen/X86/sse-builtins-constrained.c
+++ clang/test/CodeGen/X86/sse-builtins-constrained.c
@@ -1,8 +1,8 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefix=UNCONSTRAINED --check-prefix=COMMON --check-prefix=COMMONIR
 // RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -emit-llvm -o - -Wall -Werror | FileCheck %s --check-prefix=CONSTRAINED --check-prefix=COMMON --check-prefix=COMMONIR
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +sse -S %s -o - -Wall -Werror | FileCheck %s --check-prefix=CHECK-ASM --check-prefix=COMMON
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -S %s -o - -Wall -Werror | FileCheck %s --check-prefix=CHECK-ASM --check-prefix=COMMON
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +sse -S -o - -Wall -Werror | FileCheck %s --check-prefix=CHECK-ASM --check-prefix=COMMON
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu -target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -S -o - -Wall -Werror | FileCheck %s --check-prefix=CHECK-ASM --check-prefix=COMMON
 
 #ifdef STRICT
 // Test that the constrained intrinsics are picking up the exception
Index: clang/lib/Headers/cetintrin.h
===
--- clang/lib/Headers/cetintrin.h
+++ clang/lib/Headers/cetintrin.h
@@ -43,8 +43,11 @@
 }
 
 static __inline__ unsigned int __DEFAULT_FN_ATTRS _rdsspd_i32() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned int t;
   return __builtin_ia32_rdsspd(t);
+#pragma clang diagno

[PATCH] D124790: [HLSL] Enable half type for hlsl.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D124790#3489690 , @python3kgae 
wrote:

> Add option -fcgl which output clang codeGen result to avoid test dependent on 
> build DirectX backend.

Thanks -- I think this should actually be a separate patch though, because it's 
not really related to the `half` datatype. (You can always wait to land this 
patch until after the `-fcgl` one has landed.)

Aside from the `-fcgl` flag, this looks ready to go though.




Comment at: clang/lib/Basic/LangOptions.cpp:197
 
-  // OpenCL has half keyword
-  Opts.Half = Opts.OpenCL;
+  // OpenCL and HLSL have half keyword
+  Opts.Half = Opts.OpenCL || Opts.HLSL;

python3kgae wrote:
> python3kgae wrote:
> > aaron.ballman wrote:
> > > python3kgae wrote:
> > > > aaron.ballman wrote:
> > > > > Shouldn't this be looking for HLSL 2018? Or shader model 6.2?
> > > > half keyword is always available.
> > > > Without enable_16bit_types, half will be like using half=float.
> > > > With enable_16bit_types, half will be real half.
> > > > 
> > > > The check for HLSL 2018 and shader model 6.2 will be in another PR, 
> > > > still WIP. I'll add FIXME about it.
> > > > half keyword is always available.
> > > > Without enable_16bit_types, half will be like using half=float.
> > > > With enable_16bit_types, half will be real half.
> > > 
> > > Is there room for change here, or is this strictly required by HLSL? This 
> > > strikes me as just begging to confuse users into creating ODR violations. 
> > > CC @beanz 
> > Here's the doc about half for dxc.
> > https://github.com/microsoft/DirectXShaderCompiler/wiki/16-Bit-Scalar-Types
> > 
> > Old doc for fxc (the old shader compiler for shader model <= 5.1) is here
> > https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-scalar
> > 
> > Change the behavior might affect a lot of existing shaders.
> More detail about half from https://github.com/tex3d.
> 
> half originally mapped to a fuzzy "partial precision" float, where some 
> operations were designated as _pp, meaning the implementation was free to use 
> lower-precision math for those operations (like 24-bit, but not specified for 
> the language). All storage in host-visible memory would still be float. 
> "partial precision" went away with DX9, eventually to be replaced with 
> min-precision with a specific minimum precision an implementation was allowed 
> to use. When "partial precision" went away, it simply mapped to float for 
> DX10+.
> People could have tried to use it liberally when they thought 32-bit 
> precision wasn't necessary, without explicitly targeting/testing API/hardware 
> that actually supported lower precision.
Thank you for the extra information, it sounds like we're stuck supporting this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124790

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

[PATCH] D124916: [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: clang/test/CodeGen/X86/sse-builtins-constrained.c:5
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -S -o - -Wall -Werror | FileCheck %s 
--check-prefix=CHECK-ASM --check-prefix=COMMON
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -S -o - -Wall 
-Werror | FileCheck %s --check-prefix=CHECK-ASM --check-prefix=COMMON
 

Any reason you can't commit this separately and immediately?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124916

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

[PATCH] D124748: [clang-format] Fix whitespace counting stuff

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

Is there any way you could add a unit test for this?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124748

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

[PATCH] D124916: [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added inline comments.



Comment at: clang/test/CodeGen/X86/sse-builtins-constrained.c:5
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -S -o - -Wall -Werror | FileCheck %s 
--check-prefix=CHECK-ASM --check-prefix=COMMON
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -S -o - -Wall 
-Werror | FileCheck %s --check-prefix=CHECK-ASM --check-prefix=COMMON
 

RKSimon wrote:
> Any reason you can't commit this separately and immediately?
Good point! I'll do it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124916

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

[PATCH] D124749: [clang-format] Handle Verilog preprocessor directives

[MyDeveloperDay via Phabricator via cfe-commits]

MyDeveloperDay added a comment.

You add significant number of keywords here but I don't see any of them being 
tested? can you add a unit tests to cover what you are adding


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124749

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

[clang] b540ee5 - [X86] Fix redundant `%s` in RUN command. NFC

[Phoebe Wang via cfe-commits]

Author: Phoebe Wang
Date: 2022-05-04T20:29:50+08:00
New Revision: b540ee540266f42b238e683c775c32a10c184ab5

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

LOG: [X86] Fix redundant `%s` in RUN command. NFC

Added: 


Modified: 
clang/test/CodeGen/X86/sse-builtins-constrained.c

Removed: 




diff  --git a/clang/test/CodeGen/X86/sse-builtins-constrained.c 
b/clang/test/CodeGen/X86/sse-builtins-constrained.c
index 47556ad2412e3..c67e230c2ff0f 100644
--- a/clang/test/CodeGen/X86/sse-builtins-constrained.c
+++ b/clang/test/CodeGen/X86/sse-builtins-constrained.c
@@ -1,8 +1,8 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -emit-llvm -o - -Wall -Werror | FileCheck %s 
--check-prefix=UNCONSTRAINED --check-prefix=COMMON --check-prefix=COMMONIR
 // RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -emit-llvm -o - 
-Wall -Werror | FileCheck %s --check-prefix=CONSTRAINED --check-prefix=COMMON 
--check-prefix=COMMONIR
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -S %s -o - -Wall -Werror | FileCheck %s 
--check-prefix=CHECK-ASM --check-prefix=COMMON
-// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -S %s -o - 
-Wall -Werror | FileCheck %s --check-prefix=CHECK-ASM --check-prefix=COMMON
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -S -o - -Wall -Werror | FileCheck %s 
--check-prefix=CHECK-ASM --check-prefix=COMMON
+// RUN: %clang_cc1 -ffreestanding %s -triple=x86_64-unknown-linux-gnu 
-target-feature +sse -ffp-exception-behavior=maytrap -DSTRICT=1 -S -o - -Wall 
-Werror | FileCheck %s --check-prefix=CHECK-ASM --check-prefix=COMMON
 
 #ifdef STRICT
 // Test that the constrained intrinsics are picking up the exception



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

[PATCH] D124916: [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

[Phoebe Wang via Phabricator via cfe-commits]

pengfei updated this revision to Diff 426971.
pengfei added a comment.

Seperated unrelated change and rebase.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124916

Files:
  clang/lib/Headers/cetintrin.h


Index: clang/lib/Headers/cetintrin.h
===
--- clang/lib/Headers/cetintrin.h
+++ clang/lib/Headers/cetintrin.h
@@ -43,8 +43,11 @@
 }
 
 static __inline__ unsigned int __DEFAULT_FN_ATTRS _rdsspd_i32() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned int t;
   return __builtin_ia32_rdsspd(t);
+#pragma clang diagnostic pop
 }
 
 #ifdef __x86_64__
@@ -53,8 +56,11 @@
 }
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS _rdsspq_i64() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned long long t;
   return __builtin_ia32_rdsspq(t);
+#pragma clang diagnostic pop
 }
 #endif /* __x86_64__ */
 


Index: clang/lib/Headers/cetintrin.h
===
--- clang/lib/Headers/cetintrin.h
+++ clang/lib/Headers/cetintrin.h
@@ -43,8 +43,11 @@
 }
 
 static __inline__ unsigned int __DEFAULT_FN_ATTRS _rdsspd_i32() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned int t;
   return __builtin_ia32_rdsspd(t);
+#pragma clang diagnostic pop
 }
 
 #ifdef __x86_64__
@@ -53,8 +56,11 @@
 }
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS _rdsspq_i64() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned long long t;
   return __builtin_ia32_rdsspq(t);
+#pragma clang diagnostic pop
 }
 #endif /* __x86_64__ */
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124258: [C89/C2x] Change the behavior of implicit int diagnostics

[Aaron Ballman via Phabricator via cfe-commits]

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

I've landed in 2cb2cd242ca08d0bbd2a51a41f1317442e5414fc 
 and will 
keep my eyes on the bots for long tail issues from the diagnostic change.


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

https://reviews.llvm.org/D124258

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

[PATCH] D124918: Add a new clang-tidy for non-trivial unused variables.

[Luca Versari via Phabricator via cfe-commits]

veluca93 created this revision.
Herald added subscribers: carlosgalvezp, mgorny.
Herald added a project: All.
veluca93 requested review of this revision.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

This check is meant to detect case that -Wunused-variable does not,
by integrating some extra knowledge of types. For example, a
std::vector that only ever gets push_back()-ed is similar to a
written-but-never-read variable.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124918

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s performance-unused-no-side-effect %t -- \
+// RUN:  -config="{CheckOptions: [{key: \"performance-unused-no-side-effect.TypesByBase\", value: '{\"Marker\":[\"swap\"]}'}]}"
+
+namespace std {
+
+template 
+struct char_traits {};
+
+template 
+struct allocator {};
+
+template <
+class CharT,
+class Traits = std::char_traits,
+class Allocator = std::allocator>
+class basic_string {
+public:
+  basic_string &operator+=(const char *);
+  ~basic_string();
+};
+
+using string = basic_string;
+
+class thread {
+public:
+  ~thread();
+};
+
+template >
+class vector {
+public:
+  void push_back(T);
+};
+
+} // namespace std
+
+struct Marker {
+};
+
+struct SideEffectFree : Marker {
+  SideEffectFree();
+  ~SideEffectFree();
+  void swap(SideEffectFree *);
+};
+
+struct POD {
+  int A;
+};
+
+void f(const std::string &);
+
+void unusedString() {
+  std::string SomeString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+}
+
+void unusedStringWithMethodCall() {
+  std::string SomeString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+  SomeString += "hi";
+}
+
+void unusedStdThread() {
+  std::thread T;
+}
+
+void usedString() {
+  std::string SomeString;
+  SomeString += "hi";
+  f(SomeString);
+}
+
+void vectorString() {
+  std::vector VecString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: variable 'VecString' is never read [performance-unused-no-side-effect]
+  VecString.push_back(std::string{});
+}
+
+void vectorThread() {
+  std::vector VecThread;
+  VecThread.push_back(std::thread{});
+}
+
+void withMarker() {
+  SideEffectFree M;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: variable 'M' is never read [performance-unused-no-side-effect]
+}
+
+void withMarkerAndExcludedMethod(SideEffectFree *S) {
+  SideEffectFree M;
+  M.swap(S);
+}
+
+void withPOD() {
+  POD P;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'P' is never read [performance-unused-no-side-effect]
+  P.A = 1;
+}
+
+int withPODReturn() {
+  POD P;
+  P.A = 1;
+  return P.A;
+}
+
+int main() {
+  std::vector VecLoops;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: variable 'VecLoops' is never read [performance-unused-no-side-effect]
+  for (int i = 0; i < 15; i++) {
+VecLoops.push_back(1);
+  }
+  int j = 0;
+  while (j < 1) {
+VecLoops.push_back(2);
+j++;
+  }
+  (VecLoops).push_back(3);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
@@ -0,0 +1,192 @@
+.. title:: clang-tidy - performance-unused-no-side-effect
+
+performance-unused-no-side-effect
+=
+
+Finds variables that are not used and do not have user-visible side effects.
+
+A variable is defined to be side-effect-free and unused if
+
+- It has a side-effect-free type (defined below)
+- It is not passed to a function that reads its content and whose result is used
+
+A type is side-effect-free if it is:
+
+- A type with a base class specified in TypesByBase
+- A type listed in RawTypes
+- A template type listed in TemplateTypes whose template arguments are
+  themselves side-effect-free types
+- A smart pointer (listed in SmartPointerTypes) whose pointee is of side-effect-free type.
+- A primitive type.
+- A pointer.
+- A struct/class with no user-defined destructor and members of trivial type.
+
+A function is considered to "not

[PATCH] D124916: [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

[Simon Pilgrim via Phabricator via cfe-commits]

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

LGTM - I'm intending to add -Wsystem-headers to the clang x86 builtins tests 
once everything is clean.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124916

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

[PATCH] D124666: In MSVC compatibility mode, handle unqualified templated base class initialization

[Fred Tingaud via Phabricator via cfe-commits]

frederic-tingaud-sonarsource added a comment.

By the way, in the current state of the code, there is no risk of such conflict 
between typo correction and UnqualifiedBase, because when an unqualified base 
exists the lookup result will not be empty.
That's why the test was not failing even without moving the compatibility first.


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

https://reviews.llvm.org/D124666

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

[PATCH] D124916: [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

[Phoebe Wang via Phabricator via cfe-commits]

pengfei added a comment.

In D124916#3490868 , @RKSimon wrote:

> LGTM - I'm intending to add -Wsystem-headers to the clang x86 builtins tests 
> once everything is clean.

Thanks @RKSimon! That sounds great! I was thinking the headers will do 
diagnosis when `-Wall`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124916

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

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 426977.
mboehme added a comment.

Rebased to a more recent base change.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111548

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/attr-annotate-type.c
  clang/test/CodeGenCXX/annotate-type.cpp
  clang/test/Sema/annotate-type.c
  clang/test/SemaCXX/annotate-type.cpp
  clang/unittests/AST/AttrTest.cpp

Index: clang/unittests/AST/AttrTest.cpp
===
--- clang/unittests/AST/AttrTest.cpp
+++ clang/unittests/AST/AttrTest.cpp
@@ -7,7 +7,10 @@
 //===--===//
 
 #include "clang/AST/Attr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/AttrKinds.h"
+#include "clang/Tooling/Tooling.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -15,10 +18,154 @@
 
 namespace {
 
+using clang::ast_matchers::constantExpr;
+using clang::ast_matchers::equals;
+using clang::ast_matchers::functionDecl;
+using clang::ast_matchers::has;
+using clang::ast_matchers::hasDescendant;
+using clang::ast_matchers::hasName;
+using clang::ast_matchers::integerLiteral;
+using clang::ast_matchers::match;
+using clang::ast_matchers::selectFirst;
+using clang::ast_matchers::stringLiteral;
+using clang::ast_matchers::varDecl;
+using clang::tooling::buildASTFromCode;
+using clang::tooling::buildASTFromCodeWithArgs;
+
 TEST(Attr, Doc) {
   EXPECT_THAT(Attr::getDocumentation(attr::Used).str(),
   testing::HasSubstr("The compiler must emit the definition even "
  "if it appears to be unused"));
 }
 
+const FunctionDecl *getFunctionNode(ASTUnit *AST, const std::string &Name) {
+  auto Result =
+  match(functionDecl(hasName(Name)).bind("fn"), AST->getASTContext());
+  EXPECT_EQ(Result.size(), 1u);
+  return Result[0].getNodeAs("fn");
+}
+
+const VarDecl *getVariableNode(ASTUnit *AST, const std::string &Name) {
+  auto Result = match(varDecl(hasName(Name)).bind("var"), AST->getASTContext());
+  EXPECT_EQ(Result.size(), 1u);
+  return Result[0].getNodeAs("var");
+}
+
+template 
+void AssertAnnotatedAs(TypeLoc TL, llvm::StringRef annotation,
+   ModifiedTypeLoc &ModifiedTL,
+   const AnnotateTypeAttr **AnnotateOut = nullptr) {
+  const auto AttributedTL = TL.getAs();
+  ASSERT_FALSE(AttributedTL.isNull());
+  ModifiedTL = AttributedTL.getModifiedLoc().getAs();
+  ASSERT_TRUE(ModifiedTL);
+
+  ASSERT_NE(AttributedTL.getAttr(), nullptr);
+  const auto *Annotate = dyn_cast(AttributedTL.getAttr());
+  ASSERT_NE(Annotate, nullptr);
+  EXPECT_EQ(Annotate->getAnnotation(), annotation);
+  if (AnnotateOut) {
+*AnnotateOut = Annotate;
+  }
+}
+
+TEST(Attr, AnnotateType) {
+
+  // Test that the AnnotateType attribute shows up in the right places and that
+  // it stores its arguments correctly.
+
+  auto AST = buildASTFromCode(R"cpp(
+void f(int* [[clang::annotate_type("foo", "arg1", 2)]] *,
+   int [[clang::annotate_type("bar")]]);
+
+int [[clang::annotate_type("int")]] * [[clang::annotate_type("ptr")]]
+  array[10] [[clang::annotate_type("arr")]];
+
+void (* [[clang::annotate_type("funcptr")]] fp)(void);
+
+struct S { int mem; };
+int [[clang::annotate_type("int")]]
+S::* [[clang::annotate_type("ptr_to_mem")]] ptr_to_member = &S::mem;
+)cpp");
+
+  {
+const FunctionDecl *Func = getFunctionNode(AST.get(), "f");
+
+// First parameter.
+const auto PointerTL = Func->getParamDecl(0)
+   ->getTypeSourceInfo()
+   ->getTypeLoc()
+   .getAs();
+ASSERT_FALSE(PointerTL.isNull());
+PointerTypeLoc PointerPointerTL;
+const AnnotateTypeAttr *Annotate;
+AssertAnnotatedAs(PointerTL.getPointeeLoc(), "foo", PointerPointerTL,
+  &Annotate);
+
+EXPECT_EQ(Annotate->args_size(), 2);
+const auto *StringLit = selectFirst(
+"str", match(constantExpr(hasDescendant(stringLiteral().bind("str"))),
+ *Annotate->args_begin()[0], AST->getASTContext()));
+ASSERT_NE(StringLit, nullptr);
+EXPECT_EQ(StringLit->getString(), "arg1");
+EXPECT_EQ(match(constantExpr(has(integerLiteral(equals(2)).bind("int"))),
+*Annotate->args_begin()[1], AST->getASTContext())
+  .size(),
+  1);
+
+// Second parameter.
+BuiltinTypeLoc IntTL;
+AssertAnnotatedAs(Func->getParamDecl

[PATCH] D124919: [clang] [WIP] Reject non-declaration C++11 attributes on declarations.

[Martin Böhme via Phabricator via cfe-commits]

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

For backwards compatiblity, we only emit a warning if the attribute is one of
the existing type attributes that we have historically allowed to "slide" to the
`DeclSpec` just as if it had been specified in GNU syntax. (We will call these
"legacy type attributes" below.)

The two high-level changes that achieve this are:

- We move any C++11 attributes (except legacy type attributes) that occur in 
the attribute-specifier-seq at the beginning of a simple-declaration (and other 
similar declarations) to `Declarator::Attrs`. This means that we treat them the 
same as if they had been placed after the declarator-id. (Justification below.)

- We make `ProcessDeclAttribute` emit a warning if it sees any non-declaration 
attributes in C++11 syntax, except in the following cases:
  - If it is being called for attributes on a `DeclSpec` or `DeclaratorChunk`
  - If the attribute is a legacy type attribute

The standard justifies treating attributes at the beginning of a
simple-declaration and attributes after a declarator-id the same. Here are some
relevant parts of the standard:

- The attribute-specifier-seq at the beginning of a simple-declaration 
"appertains to each of the entities declared by the declarators of the 
init-declarator-list" (https://eel.is/c++draft/dcl.dcl#dcl.pre-3)

- "In the declaration for an entity, attributes appertaining to that entity can 
appear at the start of the declaration and after the declarator-id for that 
declaration." (https://eel.is/c++draft/dcl.dcl#dcl.pre-note-2)

- "The optional attribute-specifier-seq following a declarator-id appertains to 
the entity that is declared." 
(https://eel.is/c++draft/dcl.dcl#dcl.meaning.general-1)

The standard contains similar wording to that for a simple-declaration in other
similar types of declarations, for example:

- "The optional attribute-specifier-seq in a parameter-declaration appertains 
to the parameter." (https://eel.is/c++draft/dcl.fct#3)

- "The optional attribute-specifier-seq in an exception-declaration appertains 
to the parameter of the catch clause" (https://eel.is/c++draft/except.pre#1)

Things that remain to be done:

- Emit warnings when a legacy type attribute occurs on a declaration. (This is 
not hard to do but will likely require updating a large number of tests, so I'd 
first like to get confirmation that the general direction of this change makes 
sense.)

- Move additional tests added to annotate-type.cpp to 
https://reviews.llvm.org/D111548 instead.

- Various other TODOs in the code

Depends On D111548 


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124919

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Parse/RAIIObjectsForParser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/ParsedAttr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/annotate-type.c
  clang/test/SemaCXX/annotate-type.cpp

Index: clang/test/SemaCXX/annotate-type.cpp
===
--- clang/test/SemaCXX/annotate-type.cpp
+++ clang/test/SemaCXX/annotate-type.cpp
@@ -1,11 +1,10 @@
-// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -fcxx-exceptions -verify
 
 struct S1 {
   void f() [[clang::annotate_type("foo")]];
-  // FIXME: We would want to prohibit the attribute in the following location.
-  // However, Clang currently generally doesn't prohibit type-only C++11
-  // attributes on declarations. This should be fixed more generally.
-  [[clang::annotate_type("foo")]] void g();
+  [[clang::annotate_type("foo")]] void
+  g(); // expected-error {{'annotate_type' attribute cannot be applied to a
+   // declaration}}
 };
 
 template  struct is_same {
@@ -53,11 +52,31 @@
 
 // More error cases: Prohibit adding the attribute to declarations.
 // Different declarations hit different code paths, so they need separate tests.
-// FIXME: Clang currently generally doesn't prohibit type-only C++11
-// attributes on declarations.
-namespace [[clang::annotate_type("foo")]] my_namespace {}
-struct [[clang::annotate_type("foo")]] S3{};
+namespace [[clang::annotate_type("foo")]] my_namespace {
+} // namespace my_namespace
+struct [[clang::annotate_type(
+"foo")]] S3{}; // expected-error {{'annotate_type' attribute cannot be
+   // applied to a declaration}}
 void f4() {
   for ([[clang::annotate_type("foo")]] int i = 0; i < 42; ++i) {
+  } // expected-error {{'annotate_type' at

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
Herald added subscribers: carlosgalvezp, xazax.hun.
Herald added a project: All.
njames93 updated this revision to Diff 426524.
njames93 added a comment.
njames93 updated this revision to Diff 426976.
njames93 edited the summary of this revision.
njames93 added reviewers: aaron.ballman, LegalizeAdulthood.
njames93 published this revision for review.
Herald added a project: clang-tools-extra.
Herald added a subscriber: cfe-commits.

Small update


njames93 added a comment.

Add tracking info to prevented nested cases from emitting conflicting fixes, 
disabled for use cases like clangd.
Prevent emitting fixes in macro expansions.


Adds support for recognising and converting boolean expressions that can be 
simplified using De Morgans Law.

This is a different implementation to D124650 
.

Fixes https://github.com/llvm/llvm-project/issues/55092


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124806

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
@@ -0,0 +1,47 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+void foo(bool A1, bool A2, bool A3) {
+  bool X;
+  X = !(!A1 || A2);
+  X = !(A1 || !A2);
+  X = !(!A1 || !A2);
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 && !A2);
+  // CHECK-FIXES-NEXT: X = (!A1 && A2);
+  // CHECK-FIXES-NEXT: X = (A1 && A2);
+
+  X = !(!A1 && A2);
+  X = !(A1 && !A2);
+  X = !(!A1 && !A2);
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 || !A2);
+  // CHECK-FIXES-NEXT: X = (!A1 || A2);
+  // CHECK-FIXES-NEXT: X = (A1 || A2);
+
+  X = !(!A1 && !A2 && !A3);
+  X = !(!A1 && (!A2 && !A3));
+  X = !(!A1 && (A2 && A3));
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 || A2 || A3);
+  // CHECK-FIXES-NEXT: X = (A1 || (A2 || A3));
+  // CHECK-FIXES-NEXT: X = (A1 || (!A2 || !A3));
+
+  X = !(A1 && A2 == A3);
+  X = !(!A1 && A2 > A3);
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (!A1 || A2 != A3);
+  // CHECK-FIXES-NEXT: X = (A1 || A2 <= A3);
+
+  // Ensure the check doesn't try to combine fixes for the inner and outer demorgan simplification.
+  X = !(!A1 && !(!A2 && !A3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 || (!A2 && !A3));
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
@@ -4,7 +4,8 @@
 =
 
 Looks for boolean expressions involving boolean constants and simplifies
-them to use the appropriate boolean expression directly.
+them to use the appropriate boolean expression directly.  Simplifies
+boolean expressions by application of DeMorgan's Theorem.
 
 Examples:
 
@@ -27,6 +28,12 @@
 ``if (e) b = false; else b = true;``   ``b = !e;``
 ``if (e) return true; return false;``  ``return e;``
 ``if (e) return false; return true;``  ``return !e;``
+``!(!a || b)`` ``(a && !b)``
+``!(a || !b)`` ``(!a && b)``
+``!

[clang] 1587f6b - Bump the serialization major version number

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-05-04T09:07:57-04:00
New Revision: 1587f6bb3ca2cf83591f392f932007dd0fdb7b54

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

LOG: Bump the serialization major version number

This is a speculative fix for a build bot which does not put the LLVM
revision information into the PCH hash.

http://45.33.8.238/linux/75290/step_7.txt

Added: 


Modified: 
clang/include/clang/Serialization/ASTBitCodes.h

Removed: 




diff  --git a/clang/include/clang/Serialization/ASTBitCodes.h 
b/clang/include/clang/Serialization/ASTBitCodes.h
index d11c91368ee3f..1ed234e644e79 100644
--- a/clang/include/clang/Serialization/ASTBitCodes.h
+++ b/clang/include/clang/Serialization/ASTBitCodes.h
@@ -41,7 +41,7 @@ namespace serialization {
 /// Version 4 of AST files also requires that the version control branch and
 /// revision match exactly, since there is no backward compatibility of
 /// AST files at this time.
-const unsigned VERSION_MAJOR = 18;
+const unsigned VERSION_MAJOR = 19;
 
 /// AST file minor version number supported by this version of
 /// Clang.



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

[clang] 2d18a86 - [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

[Phoebe Wang via cfe-commits]

Author: Phoebe Wang
Date: 2022-05-04T21:12:12+08:00
New Revision: 2d18a86d14a936798a34b10d4b951465b7f0527f

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

LOG: [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

Fix uninitialized variables introduced by D116325.

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

Added: 


Modified: 
clang/lib/Headers/cetintrin.h

Removed: 




diff  --git a/clang/lib/Headers/cetintrin.h b/clang/lib/Headers/cetintrin.h
index 019cab0261e7f..55670a0a9dba6 100644
--- a/clang/lib/Headers/cetintrin.h
+++ b/clang/lib/Headers/cetintrin.h
@@ -43,8 +43,11 @@ static __inline__ unsigned int __DEFAULT_FN_ATTRS 
_rdsspd(unsigned int __a) {
 }
 
 static __inline__ unsigned int __DEFAULT_FN_ATTRS _rdsspd_i32() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned int t;
   return __builtin_ia32_rdsspd(t);
+#pragma clang diagnostic pop
 }
 
 #ifdef __x86_64__
@@ -53,8 +56,11 @@ static __inline__ unsigned long long __DEFAULT_FN_ATTRS 
_rdsspq(unsigned long lo
 }
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS _rdsspq_i64() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned long long t;
   return __builtin_ia32_rdsspq(t);
+#pragma clang diagnostic pop
 }
 #endif /* __x86_64__ */
 



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

[PATCH] D124916: [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

[Phoebe Wang via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG2d18a86d14a9: [X86] Fix uninitialized variable warnings in 
cetintrin.h reported by #55224 (authored by pengfei).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124916

Files:
  clang/lib/Headers/cetintrin.h


Index: clang/lib/Headers/cetintrin.h
===
--- clang/lib/Headers/cetintrin.h
+++ clang/lib/Headers/cetintrin.h
@@ -43,8 +43,11 @@
 }
 
 static __inline__ unsigned int __DEFAULT_FN_ATTRS _rdsspd_i32() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned int t;
   return __builtin_ia32_rdsspd(t);
+#pragma clang diagnostic pop
 }
 
 #ifdef __x86_64__
@@ -53,8 +56,11 @@
 }
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS _rdsspq_i64() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned long long t;
   return __builtin_ia32_rdsspq(t);
+#pragma clang diagnostic pop
 }
 #endif /* __x86_64__ */
 


Index: clang/lib/Headers/cetintrin.h
===
--- clang/lib/Headers/cetintrin.h
+++ clang/lib/Headers/cetintrin.h
@@ -43,8 +43,11 @@
 }
 
 static __inline__ unsigned int __DEFAULT_FN_ATTRS _rdsspd_i32() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned int t;
   return __builtin_ia32_rdsspd(t);
+#pragma clang diagnostic pop
 }
 
 #ifdef __x86_64__
@@ -53,8 +56,11 @@
 }
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS _rdsspq_i64() {
+#pragma clang diagnostic push
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned long long t;
   return __builtin_ia32_rdsspq(t);
+#pragma clang diagnostic pop
 }
 #endif /* __x86_64__ */
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

[Martin Böhme via Phabricator via cfe-commits]

mboehme added inline comments.



Comment at: clang/test/SemaCXX/annotate-type.cpp:2
+// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -verify
+
+struct S1 {

aaron.ballman wrote:
> mboehme wrote:
> > aaron.ballman wrote:
> > > mboehme wrote:
> > > > aaron.ballman wrote:
> > > > > mboehme wrote:
> > > > > > aaron.ballman wrote:
> > > > > > > mboehme wrote:
> > > > > > > > aaron.ballman wrote:
> > > > > > > > > Can you also add some test coverage for applying the 
> > > > > > > > > attribute to a declaration instead of a type or not giving it 
> > > > > > > > > any arguments? Also, should test arguments which are not a 
> > > > > > > > > constant expression.
> > > > > > > > I've added tests as you suggested, though I put most of them in 
> > > > > > > > Sema/annotate-type.c, as they're not specific to C++.
> > > > > > > > 
> > > > > > > > Thanks for prompting me to do this -- the tests caused me to 
> > > > > > > > notice and fix a number of bugs.
> > > > > > > > 
> > > > > > > > I haven't managed to diagnose the case when the attribute 
> > > > > > > > appears at the beginning of the declaration. It seems to me 
> > > > > > > > that, at the point where I've added the check, this case is 
> > > > > > > > indistinguishable from an attribute that appears on the type. 
> > > > > > > > In both cases, the `TAL` is `TAL_DeclSpec`, and the attribute 
> > > > > > > > is contained in `DeclSpec::getAttributes()`. This is because 
> > > > > > > > `Parser::ParseSimpleDeclaration` forwards the declaration 
> > > > > > > > attributes to the `DeclSpec`:
> > > > > > > > 
> > > > > > > > https://github.com/llvm/llvm-project/blob/main/clang/lib/Parse/ParseDecl.cpp#L1851
> > > > > > > > 
> > > > > > > > I believe if I wanted to prohibit this case, I would need to 
> > > > > > > > add a check to `Parser::ParseStatementOrDeclaration` and 
> > > > > > > > prohibit any occurrences of `annotate_type` there. However, 
> > > > > > > > this seems the wrong place to do this because it doesn't 
> > > > > > > > contain any specific processing for other attributes.
> > > > > > > > 
> > > > > > > > I've noticed that Clang also doesn't prohibit other type 
> > > > > > > > attributes (even ones with C++ 11 syntax) from being applied to 
> > > > > > > > declarations. For example: https://godbolt.org/z/Yj1zWY7nn
> > > > > > > > 
> > > > > > > > How do you think I should proceed here? I think the underlying 
> > > > > > > > issue is that Clang doesn't always distinguish cleanly between 
> > > > > > > > declaration attributes and type attributes, and fixing this 
> > > > > > > > might be nontrivial.
> > > > > > > > How do you think I should proceed here? I think the underlying 
> > > > > > > > issue is that Clang doesn't always distinguish cleanly between 
> > > > > > > > declaration attributes and type attributes, and fixing this 
> > > > > > > > might be nontrivial.
> > > > > > > 
> > > > > > > This is a general issue with attribute processing. I would 
> > > > > > > imagine that SemaDeclAttr.cpp should be able to diagnose that 
> > > > > > > case when the attribute only applies to types and not 
> > > > > > > declarations, but it'd take some investigation for me to be sure.
> > > > > > > 
> > > > > > > Because this issue isn't new to your situation, I'd recommend 
> > > > > > > filing an issue about the general problem and then we can solve 
> > > > > > > that later.
> > > > > > I've done some more investigation myself, and I think I've come up 
> > > > > > with a solution; actually, two potential solutions. I have draft 
> > > > > > patches for both of these; I wanted to run these by you here first, 
> > > > > > so I haven't opened a bug yet.
> > > > > > 
> > > > > > I'd appreciate your input on how you'd prefer me to proceed with 
> > > > > > this. I do think it makes sense to do this work now because 
> > > > > > otherwise, people will start putting `annotate_type` in places 
> > > > > > where it doesn't belong, and then we'll have to keep supporting 
> > > > > > that in the future.
> > > > > > 
> > > > > > **My preferred solution**
> > > > > > 
> > > > > > https://reviews.llvm.org/D124081
> > > > > > 
> > > > > > This adds a function `DiagnoseCXX11NonDeclAttrs()` and calls it in 
> > > > > > all places where we should reject attributes that can't be put on 
> > > > > > declarations. (As part of this work, I noticed that `gsl::suppress` 
> > > > > > should be a `DeclOrStmtAttr`, not just a `StmtAttr`.)
> > > > > > 
> > > > > > Advantages:
> > > > > > - Not very invasive.
> > > > > > Disadvantages:
> > > > > > - Need to make sure we call `DiagnoseCXX11NonDeclAttrs()` 
> > > > > > everywhere that non-declaration attributes should be rejected. (Not 
> > > > > > sure if I've identified all of those places yet?)
> > > > > > 
> > > > > > **Alternative solution**
> > > > > > 
> > > > > > https://reviews.llvm.org/D124083
> > > > > > 
> > > > > > This adds an `appertainsTo` parameter to `ParseCXX11Attributes()` 
> > >

[PATCH] D124807: [clang][dataflow] Only skip ExprWithCleanups when visiting terminators

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua updated this revision to Diff 426985.
li.zhe.hua marked 7 inline comments as done.
li.zhe.hua added a comment.

Add `ExprWithCleanups` requirement to `createStorageLocation`, 
`setStorageLocation`, and `getStorageLocation`.
Remove `ParenExpr` requirement from `getValue`.
Remove unnecessary calls to `IgnoreParens`.
Address misc. comments; apply suggestions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124807

Files:
  clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
  clang/include/clang/Analysis/FlowSensitive/Transfer.h
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
  clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1152,4 +1152,39 @@
   });
 }
 
+// FIXME: Remove this test once it provides no additional test coverage.
+TEST_F(FlowConditionTest, DoesNotAssertForImplicitCastToBool) {
+  std::string Code = R"(
+void target(int *Ptr) {
+  bool Foo = false;
+  if (Ptr) {
+Foo = true;
+/*[[p1]]*/
+  }
+
+  (void)0;
+  /*[[p2]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const Environment &Env1 = Results[1].second.Env;
+auto &FooVal1 =
+*cast(Env1.getValue(*FooDecl, SkipPast::Reference));
+EXPECT_TRUE(Env1.flowConditionImplies(FooVal1));
+
+const Environment &Env2 = Results[0].second.Env;
+auto &FooVal2 =
+*cast(Env2.getValue(*FooDecl, SkipPast::Reference));
+EXPECT_FALSE(Env2.flowConditionImplies(FooVal2));
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -77,26 +77,26 @@
   : StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx) {}
 
   void VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond()->IgnoreParenImpCasts();
+auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
 assert(Cond != nullptr);
 extendFlowCondition(*Cond);
   }
 
   void VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond()->IgnoreParenImpCasts();
+auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
 assert(Cond != nullptr);
 extendFlowCondition(*Cond);
   }
 
   void VisitBinaryOperator(const BinaryOperator *S) {
 assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS()->IgnoreParenImpCasts();
+auto *LHS = ignoreExprWithCleanups(S->getLHS())->IgnoreParens();
 assert(LHS != nullptr);
 extendFlowCondition(*LHS);
   }
 
   void VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond()->IgnoreParenImpCasts();
+auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
 assert(Cond != nullptr);
 extendFlowCondition(*Cond);
   }
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -33,7 +33,7 @@
 namespace clang {
 namespace dataflow {
 
-static const Expr *skipExprWithCleanups(const Expr *E) {
+const Expr *ignoreExprWithCleanups(const Expr *E) {
   if (auto *C = dyn_cast_or_null(E))
 return C->getSubExpr();
   return E;
@@ -155,9 +155,7 @@
   return;
 }
 
-// The CFG does not contain `ParenExpr` as top-level statements in basic
-// blocks, however sub-expressions can still be of that type.
-InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens());
+InitExpr = ignoreExprWithCleanups(D.getInit());
 assert(InitExpr != nullptr);
 
 if (D.getType()->isReferenceType()) {
@@ -190,10 +188,7 @@
   }
 
   void VisitImplicitCastExpr(const ImplicitCastExpr *S) {
-// The CFG does not contain `ParenExpr` as top-level statements in basic
-// blocks, however sub-expressions can still be of that type.
-assert(S->getSubExpr() != nullptr);
-const Expr *

[PATCH] D124807: [clang][dataflow] Only skip ExprWithCleanups when visiting terminators

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua added inline comments.



Comment at: clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h:230
+  ///
+  /// Requirements:
+  ///

sgatev wrote:
> Why add these as requirements instead of skipping past `ExprWithCleanups` 
> internally, as we currently do for parens? Should we add similar requirements 
> to `createStorageLocation`, `setStorageLocation`, and `getStorageLocation`? 
> Would that result in lots of client code sprinkled with `IgnoreParens` and 
> some form of `ignoreExprWithCleanups`?
Ah, I missed that we do this automatically for parens, in part because we 
//do// unnecessarily sprinkle `IgnoreParens` in a lot of places in 
`Transfer.cpp`.

That said, we can't avoid sprinkling them at least in some places for the time 
being. Fixing this greatly expands beyond the scope of fixing a `cast` assert 
firing. (That is still, aspirationally, the goal of this patch.)

Let me remove the `ParenExpr` requirement (that's incorrect), leave the 
`ExprWithCleanups` requirement (with `FIXME`), and leave the holistic cleanup 
for a separate patch.



Comment at: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp:429-430
 Value *Environment::getValue(const Expr &E, SkipPast SP) const {
+  assert(!isa(&E));
+  assert(!isa(&E));
   auto *Loc = getStorageLocation(E, SP);

sgatev wrote:
> 
Acknowledged. Obviated by removing the `ParenExpr` assert.



Comment at: 
clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp:224
+  if (auto *OptionalVal = cast_or_null(State.Env.getValue(
+  *ObjectExpr->IgnoreParens(), SkipPast::ReferenceThenPointer))) {
 auto *HasValueVal = getHasValue(OptionalVal);

sgatev wrote:
> Do we have a test that fails if `IgnoreParens` is missing? If not, let's add 
> a test or extend one of the existing. Perhaps 
> https://github.com/llvm/llvm-project/blob/main/clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp#L1286
I missed that we ignore parens automatically in `getValue`. Removing this.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124807

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

[PATCH] D124807: [clang][dataflow] Only skip ExprWithCleanups when visiting terminators

[Eric Li via Phabricator via cfe-commits]

li.zhe.hua updated this revision to Diff 426987.
li.zhe.hua added a comment.

Forgot to add a file... let's try this again...


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124807

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

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1152,4 +1152,39 @@
   });
 }
 
+// FIXME: Remove this test once it provides no additional test coverage.
+TEST_F(FlowConditionTest, DoesNotAssertForImplicitCastToBool) {
+  std::string Code = R"(
+void target(int *Ptr) {
+  bool Foo = false;
+  if (Ptr) {
+Foo = true;
+/*[[p1]]*/
+  }
+
+  (void)0;
+  /*[[p2]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const Environment &Env1 = Results[1].second.Env;
+auto &FooVal1 =
+*cast(Env1.getValue(*FooDecl, SkipPast::Reference));
+EXPECT_TRUE(Env1.flowConditionImplies(FooVal1));
+
+const Environment &Env2 = Results[0].second.Env;
+auto &FooVal2 =
+*cast(Env2.getValue(*FooDecl, SkipPast::Reference));
+EXPECT_FALSE(Env2.flowConditionImplies(FooVal2));
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -77,26 +77,26 @@
   : StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx) {}
 
   void VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond()->IgnoreParenImpCasts();
+auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
 assert(Cond != nullptr);
 extendFlowCondition(*Cond);
   }
 
   void VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond()->IgnoreParenImpCasts();
+auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
 assert(Cond != nullptr);
 extendFlowCondition(*Cond);
   }
 
   void VisitBinaryOperator(const BinaryOperator *S) {
 assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS()->IgnoreParenImpCasts();
+auto *LHS = ignoreExprWithCleanups(S->getLHS())->IgnoreParens();
 assert(LHS != nullptr);
 extendFlowCondition(*LHS);
   }
 
   void VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond()->IgnoreParenImpCasts();
+auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
 assert(Cond != nullptr);
 extendFlowCondition(*Cond);
   }
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -33,7 +33,7 @@
 namespace clang {
 namespace dataflow {
 
-static const Expr *skipExprWithCleanups(const Expr *E) {
+const Expr *ignoreExprWithCleanups(const Expr *E) {
   if (auto *C = dyn_cast_or_null(E))
 return C->getSubExpr();
   return E;
@@ -155,9 +155,7 @@
   return;
 }
 
-// The CFG does not contain `ParenExpr` as top-level statements in basic
-// blocks, however sub-expressions can still be of that type.
-InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens());
+InitExpr = ignoreExprWithCleanups(D.getInit());
 assert(InitExpr != nullptr);
 
 if (D.getType()->isReferenceType()) {
@@ -190,10 +188,7 @@
   }
 
   void VisitImplicitCastExpr(const ImplicitCastExpr *S) {
-// The CFG does not contain `ParenExpr` as top-level statements in basic
-// blocks, however sub-expressions can still be of that type.
-assert(S->getSubExpr() != nullptr);
-const Expr *SubExpr = S->getSubExpr()->IgnoreParens();
+const Expr *SubExpr = S->getSubExpr();
 assert(SubExpr != nullptr);
 
 switch (S->getCastKind()) {
@@ -252,10 +247,7 @@
   }
 
   void VisitUnaryOperator(const UnaryOperator *S) {
-// The CFG does not contain `ParenExpr` as top-level statements in basic
-

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 426990.
mboehme added a comment.

Changes in response to review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111548

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/Attr.td
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/TypePrinter.cpp
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/AST/attr-annotate-type.c
  clang/test/CodeGenCXX/annotate-type.cpp
  clang/test/Sema/annotate-type.c
  clang/test/SemaCXX/annotate-type.cpp
  clang/unittests/AST/AttrTest.cpp

Index: clang/unittests/AST/AttrTest.cpp
===
--- clang/unittests/AST/AttrTest.cpp
+++ clang/unittests/AST/AttrTest.cpp
@@ -7,7 +7,10 @@
 //===--===//
 
 #include "clang/AST/Attr.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
+#include "clang/ASTMatchers/ASTMatchers.h"
 #include "clang/Basic/AttrKinds.h"
+#include "clang/Tooling/Tooling.h"
 #include "gmock/gmock.h"
 #include "gtest/gtest.h"
 
@@ -15,10 +18,154 @@
 
 namespace {
 
+using clang::ast_matchers::constantExpr;
+using clang::ast_matchers::equals;
+using clang::ast_matchers::functionDecl;
+using clang::ast_matchers::has;
+using clang::ast_matchers::hasDescendant;
+using clang::ast_matchers::hasName;
+using clang::ast_matchers::integerLiteral;
+using clang::ast_matchers::match;
+using clang::ast_matchers::selectFirst;
+using clang::ast_matchers::stringLiteral;
+using clang::ast_matchers::varDecl;
+using clang::tooling::buildASTFromCode;
+using clang::tooling::buildASTFromCodeWithArgs;
+
 TEST(Attr, Doc) {
   EXPECT_THAT(Attr::getDocumentation(attr::Used).str(),
   testing::HasSubstr("The compiler must emit the definition even "
  "if it appears to be unused"));
 }
 
+const FunctionDecl *getFunctionNode(ASTUnit *AST, const std::string &Name) {
+  auto Result =
+  match(functionDecl(hasName(Name)).bind("fn"), AST->getASTContext());
+  EXPECT_EQ(Result.size(), 1u);
+  return Result[0].getNodeAs("fn");
+}
+
+const VarDecl *getVariableNode(ASTUnit *AST, const std::string &Name) {
+  auto Result = match(varDecl(hasName(Name)).bind("var"), AST->getASTContext());
+  EXPECT_EQ(Result.size(), 1u);
+  return Result[0].getNodeAs("var");
+}
+
+template 
+void AssertAnnotatedAs(TypeLoc TL, llvm::StringRef annotation,
+   ModifiedTypeLoc &ModifiedTL,
+   const AnnotateTypeAttr **AnnotateOut = nullptr) {
+  const auto AttributedTL = TL.getAs();
+  ASSERT_FALSE(AttributedTL.isNull());
+  ModifiedTL = AttributedTL.getModifiedLoc().getAs();
+  ASSERT_TRUE(ModifiedTL);
+
+  ASSERT_NE(AttributedTL.getAttr(), nullptr);
+  const auto *Annotate = dyn_cast(AttributedTL.getAttr());
+  ASSERT_NE(Annotate, nullptr);
+  EXPECT_EQ(Annotate->getAnnotation(), annotation);
+  if (AnnotateOut) {
+*AnnotateOut = Annotate;
+  }
+}
+
+TEST(Attr, AnnotateType) {
+
+  // Test that the AnnotateType attribute shows up in the right places and that
+  // it stores its arguments correctly.
+
+  auto AST = buildASTFromCode(R"cpp(
+void f(int* [[clang::annotate_type("foo", "arg1", 2)]] *,
+   int [[clang::annotate_type("bar")]]);
+
+int [[clang::annotate_type("int")]] * [[clang::annotate_type("ptr")]]
+  array[10] [[clang::annotate_type("arr")]];
+
+void (* [[clang::annotate_type("funcptr")]] fp)(void);
+
+struct S { int mem; };
+int [[clang::annotate_type("int")]]
+S::* [[clang::annotate_type("ptr_to_mem")]] ptr_to_member = &S::mem;
+  )cpp");
+
+  {
+const FunctionDecl *Func = getFunctionNode(AST.get(), "f");
+
+// First parameter.
+const auto PointerTL = Func->getParamDecl(0)
+   ->getTypeSourceInfo()
+   ->getTypeLoc()
+   .getAs();
+ASSERT_FALSE(PointerTL.isNull());
+PointerTypeLoc PointerPointerTL;
+const AnnotateTypeAttr *Annotate;
+AssertAnnotatedAs(PointerTL.getPointeeLoc(), "foo", PointerPointerTL,
+  &Annotate);
+
+EXPECT_EQ(Annotate->args_size(), 2);
+const auto *StringLit = selectFirst(
+"str", match(constantExpr(hasDescendant(stringLiteral().bind("str"))),
+ *Annotate->args_begin()[0], AST->getASTContext()));
+ASSERT_NE(StringLit, nullptr);
+EXPECT_EQ(StringLit->getString(), "arg1");
+EXPECT_EQ(match(constantExpr(has(integerLiteral(equals(2)).bind("int"))),
+*Annotate->args_begin()[1], AST->getASTContext())
+  .size(),
+  1);
+
+// Second parameter.
+BuiltinTypeLoc IntTL;
+AssertAnnotatedAs(Func->getParam

[PATCH] D124919: [clang] [WIP] Reject non-declaration C++11 attributes on declarations.

[Martin Böhme via Phabricator via cfe-commits]

mboehme updated this revision to Diff 426991.
mboehme added a comment.

Reuploaded because base revision changed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124919

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Parse/RAIIObjectsForParser.h
  clang/include/clang/Sema/ParsedAttr.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseDeclCXX.cpp
  clang/lib/Parse/ParseExprCXX.cpp
  clang/lib/Parse/ParseStmt.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/lib/Parse/Parser.cpp
  clang/lib/Sema/ParsedAttr.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/lib/Sema/SemaType.cpp
  clang/test/Sema/annotate-type.c
  clang/test/SemaCXX/annotate-type.cpp

Index: clang/test/SemaCXX/annotate-type.cpp
===
--- clang/test/SemaCXX/annotate-type.cpp
+++ clang/test/SemaCXX/annotate-type.cpp
@@ -1,11 +1,10 @@
-// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -verify
+// RUN: %clang_cc1 %s -std=c++17 -fsyntax-only -fcxx-exceptions -verify
 
 struct S1 {
   void f() [[clang::annotate_type("foo")]];
-  // FIXME: We would want to prohibit the attribute in the following location.
-  // However, Clang currently generally doesn't prohibit type-only C++11
-  // attributes on declarations. This should be fixed more generally.
-  [[clang::annotate_type("foo")]] void g();
+  [[clang::annotate_type("foo")]] void
+  g(); // expected-error {{'annotate_type' attribute cannot be applied to a
+   // declaration}}
 };
 
 template  struct is_same {
@@ -48,11 +47,31 @@
 
 // More error cases: Prohibit adding the attribute to declarations.
 // Different declarations hit different code paths, so they need separate tests.
-// FIXME: Clang currently generally doesn't prohibit type-only C++11
-// attributes on declarations.
-namespace [[clang::annotate_type("foo")]] my_namespace {}
-struct [[clang::annotate_type("foo")]] S3{};
+namespace [[clang::annotate_type("foo")]] my_namespace {
+} // namespace my_namespace
+struct [[clang::annotate_type(
+"foo")]] S3{}; // expected-error {{'annotate_type' attribute cannot be
+   // applied to a declaration}}
 void f4() {
   for ([[clang::annotate_type("foo")]] int i = 0; i < 42; ++i) {
+  } // expected-error {{'annotate_type' attribute cannot be applied to a
+// declaration}}
+  for (; [[clang::annotate_type("foo")]] bool b = false;) {
+  } // expected-error {{'annotate_type' attribute cannot be applied to a
+// declaration}}
+  while ([[clang::annotate_type("foo")]] bool b = false) {
+  } // expected-error {{'annotate_type' attribute cannot be applied to a
+// declaration}}
+  if ([[clang::annotate_type("foo")]] bool b = false) {
+  } // expected-error {{'annotate_type' attribute cannot be applied to a
+// declaration}}
+  try {
+  } catch ([[clang::annotate_type(
+  "foo")]] int i) { // expected-error {{'annotate_type' attribute cannot be
+// applied to a declaration}}
   }
 }
+template 
+[[clang::annotate_type(
+"foo")]] T var_template; // expected-error {{'annotate_type' attribute
+ // cannot be applied to a declaration}}
Index: clang/test/Sema/annotate-type.c
===
--- clang/test/Sema/annotate-type.c
+++ clang/test/Sema/annotate-type.c
@@ -17,10 +17,7 @@
   int *__attribute__((annotate_type("bar"))) y2; // expected-warning {{unknown attribute 'annotate_type' ignored}}
 
   // Various error cases
-  // FIXME: We would want to prohibit the attribute in the following location.
-  // However, Clang currently generally doesn't prohibit type-only C++11
-  // attributes on declarations. This should be fixed more generally.
-  [[clang::annotate_type("bar")]] int *z1;
+  [[clang::annotate_type("bar")]] int *z1; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
   int *z2 [[clang::annotate_type("bar")]]; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
   [[clang::annotate_type("bar")]]; // expected-error {{'annotate_type' attribute cannot be applied to a statement}}
   int *[[clang::annotate_type(1)]] z3; // expected-error {{'annotate_type' attribute requires a string}}
@@ -33,7 +30,6 @@
 }
 // More error cases: Prohibit adding the attribute to declarations.
 // Different declarations hit different code paths, so they need separate tests.
-// FIXME: Clang currently generally doesn't prohibit type-only C++11
-// attributes on declarations.
-[[clang::annotate_type("bar")]] int *global;
-void g([[clang::annotate_type("bar")]] int);
+[[clang::annotate_type("bar")]] int *global; // expected-error {{'annotate_type' attribute cannot be applied to a declaration}}
+[[clang::annotate_type("bar")]] void annotated_function(); // expected-error {{'annotate_type' attribute cannot be applied to a declarati

[PATCH] D124774: [clang][ASTImporter][NFC]: Move clang::ImportError into own header and rename it .

[Shivam Rajput via Phabricator via cfe-commits]

phyBrackets updated this revision to Diff 426992.
phyBrackets added a comment.

Address inline comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124774

Files:
  clang/include/clang/AST/ASTImportError.h
  clang/include/clang/AST/ASTImporter.h
  clang/include/clang/AST/ASTImporterLookupTable.h
  clang/include/clang/AST/ASTImporterSharedState.h


Index: clang/include/clang/AST/ASTImporterSharedState.h
===
--- clang/include/clang/AST/ASTImporterSharedState.h
+++ clang/include/clang/AST/ASTImporterSharedState.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_AST_ASTIMPORTERSHAREDSTATE_H
 #define LLVM_CLANG_AST_ASTIMPORTERSHAREDSTATE_H
 
+#include "clang/AST/ASTImportError.h"
 #include "clang/AST/ASTImporterLookupTable.h"
 #include "clang/AST/Decl.h"
 #include "llvm/ADT/DenseMap.h"
Index: clang/include/clang/AST/ASTImporterLookupTable.h
===
--- clang/include/clang/AST/ASTImporterLookupTable.h
+++ clang/include/clang/AST/ASTImporterLookupTable.h
@@ -14,7 +14,6 @@
 #ifndef LLVM_CLANG_AST_ASTIMPORTERLOOKUPTABLE_H
 #define LLVM_CLANG_AST_ASTIMPORTERLOOKUPTABLE_H
 
-#include "clang/AST/ASTImportError.h"
 #include "clang/AST/DeclBase.h" // lookup_result
 #include "clang/AST/DeclarationName.h"
 #include "llvm/ADT/DenseMap.h"
Index: clang/include/clang/AST/ASTImporter.h
===
--- clang/include/clang/AST/ASTImporter.h
+++ clang/include/clang/AST/ASTImporter.h
@@ -14,7 +14,7 @@
 #ifndef LLVM_CLANG_AST_ASTIMPORTER_H
 #define LLVM_CLANG_AST_ASTIMPORTER_H
 
-#include "clang/AST/ASTImporterLookupTable.h"
+#include "clang/AST/ASTImportError.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -1,4 +1,5 @@
-//===- ASTImporter.h - Import Error while Importing AST ---*- C++ -*-===//
+//===- ASTImportError.h -  Define errors while importing AST  ---*- C++
+//-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
 // See https://llvm.org/LICENSE.txt for license information.
@@ -14,7 +15,6 @@
 #ifndef LLVM_CLANG_AST_ASTIMPORTERROR_H
 #define LLVM_CLANG_AST_ASTIMPORTERROR_H
 
-#include "clang/AST/APValue.h"
 #include "llvm/Support/Error.h"
 
 namespace clang {
@@ -42,10 +42,10 @@
 
   std::string toString() const;
 
-  void log(raw_ostream &OS) const override;
+  void log(llvm::raw_ostream &OS) const override;
   std::error_code convertToErrorCode() const override;
 };
 
 } // namespace clang
 
-#endif // LLVM_CLANG_AST_ASTIMPORTERROR_H
\ No newline at end of file
+#endif // LLVM_CLANG_AST_ASTIMPORTERROR_H


Index: clang/include/clang/AST/ASTImporterSharedState.h
===
--- clang/include/clang/AST/ASTImporterSharedState.h
+++ clang/include/clang/AST/ASTImporterSharedState.h
@@ -14,6 +14,7 @@
 #ifndef LLVM_CLANG_AST_ASTIMPORTERSHAREDSTATE_H
 #define LLVM_CLANG_AST_ASTIMPORTERSHAREDSTATE_H
 
+#include "clang/AST/ASTImportError.h"
 #include "clang/AST/ASTImporterLookupTable.h"
 #include "clang/AST/Decl.h"
 #include "llvm/ADT/DenseMap.h"
Index: clang/include/clang/AST/ASTImporterLookupTable.h
===
--- clang/include/clang/AST/ASTImporterLookupTable.h
+++ clang/include/clang/AST/ASTImporterLookupTable.h
@@ -14,7 +14,6 @@
 #ifndef LLVM_CLANG_AST_ASTIMPORTERLOOKUPTABLE_H
 #define LLVM_CLANG_AST_ASTIMPORTERLOOKUPTABLE_H
 
-#include "clang/AST/ASTImportError.h"
 #include "clang/AST/DeclBase.h" // lookup_result
 #include "clang/AST/DeclarationName.h"
 #include "llvm/ADT/DenseMap.h"
Index: clang/include/clang/AST/ASTImporter.h
===
--- clang/include/clang/AST/ASTImporter.h
+++ clang/include/clang/AST/ASTImporter.h
@@ -14,7 +14,7 @@
 #ifndef LLVM_CLANG_AST_ASTIMPORTER_H
 #define LLVM_CLANG_AST_ASTIMPORTER_H
 
-#include "clang/AST/ASTImporterLookupTable.h"
+#include "clang/AST/ASTImportError.h"
 #include "clang/AST/DeclBase.h"
 #include "clang/AST/DeclarationName.h"
 #include "clang/AST/ExprCXX.h"
Index: clang/include/clang/AST/ASTImportError.h
===
--- clang/include/clang/AST/ASTImportError.h
+++ clang/include/clang/AST/ASTImportError.h
@@ -1,4 +1,5 @@
-//===- ASTImporter.h - Import Error while Importing AST ---*- C++ -*-===//
+//===- ASTImportError.h -  Define errors while importing AST  ---*- C++
+//-*-===//
 //
 // Part of the LLVM Project, under the Apache License v2.0 with LLVM Exceptions.
 // See https://llvm.or

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

[Martin Böhme via Phabricator via cfe-commits]

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



Comment at: clang/unittests/AST/AttrTest.cpp:89
+S::* [[clang::annotate_type("ptr_to_mem")]] ptr_to_member = &S::mem;
+)cpp");
+

aaron.ballman wrote:
> The formatting looks a bit off for this in terms of indentation.
Fixed.



Comment at: clang/unittests/AST/AttrTest.cpp:159
+__auto_type [[clang::annotate_type("auto")]] auto_var = 1;
+)c",
+ {"-fdouble-square-bracket-attributes"},

aaron.ballman wrote:
> Formatting looks a bit off here in terms of the indentation for this.
Fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111548

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

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov created this revision.
ilya-biryukov added reviewers: sammccall, saar.raz.
Herald added a project: All.
ilya-biryukov requested review of this revision.
Herald added a project: clang.

- Exit early when constraint caching is disabled.
- Use unique_ptr to manage temporary lifetime.
- Fix a typo in a comment (InsertPos instead of InsertNode).

The new code duplicates the forwarding call to CheckConstraintSatisfaction,
but reduces the number of interconnected if statements and simplifies lifetime
management.

This increases the overall readability.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124923

Files:
  clang/lib/Sema/SemaConcept.cpp


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -318,35 +318,30 @@
 return false;
   }
 
+  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
+  if (!ShouldCache) {
+return ::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
+ TemplateArgs, TemplateIDRange,
+ OutSatisfaction);
+  }
   llvm::FoldingSetNodeID ID;
+  ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
   void *InsertPos;
-  ConstraintSatisfaction *Satisfaction = nullptr;
-  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
-  if (ShouldCache) {
-ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
-Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos);
-if (Satisfaction) {
-  OutSatisfaction = *Satisfaction;
-  return false;
-}
-Satisfaction = new ConstraintSatisfaction(Template, TemplateArgs);
-  } else {
-Satisfaction = &OutSatisfaction;
+  if (auto *Cached = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos)) {
+OutSatisfaction = *Cached;
+return false;
   }
+  auto Satisfaction =
+  std::make_unique(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
 TemplateArgs, TemplateIDRange,
 *Satisfaction)) {
-if (ShouldCache)
-  delete Satisfaction;
 return true;
   }
-
-  if (ShouldCache) {
-// We cannot use InsertNode here because CheckConstraintSatisfaction might
-// have invalidated it.
-SatisfactionCache.InsertNode(Satisfaction);
-OutSatisfaction = *Satisfaction;
-  }
+  OutSatisfaction = *Satisfaction;
+  // We cannot use InsertPos here because CheckConstraintSatisfaction might 
have
+  // invalidated it.
+  SatisfactionCache.InsertNode(Satisfaction.release());
   return false;
 }
 


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -318,35 +318,30 @@
 return false;
   }
 
+  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
+  if (!ShouldCache) {
+return ::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
+ TemplateArgs, TemplateIDRange,
+ OutSatisfaction);
+  }
   llvm::FoldingSetNodeID ID;
+  ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
   void *InsertPos;
-  ConstraintSatisfaction *Satisfaction = nullptr;
-  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
-  if (ShouldCache) {
-ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
-Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos);
-if (Satisfaction) {
-  OutSatisfaction = *Satisfaction;
-  return false;
-}
-Satisfaction = new ConstraintSatisfaction(Template, TemplateArgs);
-  } else {
-Satisfaction = &OutSatisfaction;
+  if (auto *Cached = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos)) {
+OutSatisfaction = *Cached;
+return false;
   }
+  auto Satisfaction =
+  std::make_unique(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
 TemplateArgs, TemplateIDRange,
 *Satisfaction)) {
-if (ShouldCache)
-  delete Satisfaction;
 return true;
   }
-
-  if (ShouldCache) {
-// We cannot use InsertNode here because CheckConstraintSatisfaction might
-// have invalidated it.
-SatisfactionCache.InsertNode(Satisfaction);
-OutSatisfaction = *Satisfaction;
-  }
+  OutSatisfaction = *Satisfaction;
+  // We cannot use InsertPos here because CheckConstraintSatisfaction might have
+  // invalidated it.
+  SatisfactionCache.InsertNode(Satisfaction.release());
   return false;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-c

[PATCH] D124807: [clang][dataflow] Only skip ExprWithCleanups when visiting terminators

[Stanislav Gatev via Phabricator via cfe-commits]

sgatev accepted this revision.
sgatev added a comment.

Looks great! Thank you!




Comment at: 
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp:1156
+// FIXME: Remove this test once it provides no additional test coverage.
+TEST_F(FlowConditionTest, DoesNotAssertForImplicitCastToBool) {
+  std::string Code = R"(

Maybe call this `PointerToBoolImplicitCast` and remove the FIXME?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124807

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

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:335
+  auto Satisfaction =
+  std::make_unique(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,

I also wonder if this could be allocated in the AST arena, maybe worth 
exploring.
Definitely outside this change, though, would like to keep this one an NFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124923

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

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

[Martin Böhme via Phabricator via cfe-commits]

mboehme marked 3 inline comments as done.
mboehme added a comment.

In D111548#3483275 , @erichkeane 
wrote:

> I don't really know how useful this ends up being, these attributes (since 
> they are part of `AttributedType` end up disappearing pretty quickly/easily.  
> Anything that causes canonicalization will cause these to go away,

This is true, but I also think we don't have much choice in the matter if we 
don't want these attributes to affect the C++ type system. We discuss this at 
length in the RFC:

https://discourse.llvm.org/t/rfc-new-attribute-annotate-type-iteration-2/61378#rationale-5

I go into more depth in this comment:

https://discourse.llvm.org/t/rfc-new-attribute-annotate-type-iteration-2/61378/4?u=martinboehme

However, it is possible for a static analysis to propagate the annotations 
through typedefs, template arguments and such, and we do so in the lifetime 
static analysis (see also below).

> they don't apply to references to these types, etc.

Not sure what you mean here -- do you mean that the annotations don't propagate 
through typedefs? As noted, it's possible for a static analysis tool to perform 
this propagation, and the discussion here is again relevant:

https://discourse.llvm.org/t/rfc-new-attribute-annotate-type-iteration-2/61378/4?u=martinboehme

> But I otherwise don't have concerns.
>
> HOWEVER, have you implemented the lifetime static analysis that you're 
> wanting to do on top of this already

Yes, we have. We are able to propagate lifetime annotations through non-trivial 
C++ constructs, including template arguments. Based on what our current 
prototype can do, we're confident these annotations will do everything we need 
them to. We also believe they are general enough to be useful for other types 
of static analysis.




Comment at: clang/lib/AST/TypePrinter.cpp:1689
+// the type.
+OS << " [[clang::annotate_type]]";
+return;

erichkeane wrote:
> My opinion (though not attached to it) would be `clang::annotate_type(...)` 
> or, `clang::annotate_type()` or something like that.
Good idea -- this makes it more obvious that the arguments were omitted. 
Changed!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111548

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

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:204
+// subtraction/addition of the negated value.
+if (!RHS.isNegative()) {
+  ConvertedRHS = &BasicVals.Convert(resultTy, RHS);

steakhal wrote:
> tomasz-kaminski-sonarsource wrote:
> > steakhal wrote:
> > > I would rather swap these branches though, to leave the default case 
> > > (aka. this) to the end.
> > I folded the `RHS.isNegative()` into the if for the 
> > `BinaryOperator::isAssociative(op)`, as same conversion is performed in 
> > final else branch.
> I think what confused me is that a different API is used for doing the 
> conversion.
>  - `resultIntTy.convert(RHS)`
>  - `&BasicVals.Convert(resultTy, RHS)`
> 
> Anyway, leave it as-is.
As a note, the use of different APIs was intentional. The `BasicVals` one is 
persisting the value, so it is safe to use ptr to it, as a consequence it is 
more costfull. So, I am delaying its use until I know I will need to persist 
the value.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:212-219
+  llvm::APSInt ConvertedRHSValue = resultIntTy.convert(RHS);
+  // Check if the negation of the RHS is representable,
+  // i.e., the resultTy is signed, and it is not the lowest
+  // representable negative value.
+  if (ConvertedRHSValue > resultIntTy.getMinValue()) {
+ConvertedRHS = &BasicVals.getValue(-ConvertedRHSValue);
+op = (op == BO_Add) ? BO_Sub : BO_Add;

steakhal wrote:
> tomasz-kaminski-sonarsource wrote:
> > tomasz-kaminski-sonarsource wrote:
> > > steakhal wrote:
> > > > Somehow I miss a check for signedness here.
> > > > Why do you think it would be only triggered for signed types?
> > > > 
> > > > I have a guess, that since we already handled `x +-0`, SymIntExprs like 
> > > > `x - (-0)` cannot exist here, thus cannot trigger this condition 
> > > > spuriously. I cannot think of any ther example that could cause this 
> > > > misbehaving. So in that sense `ConvertedRHSValue > 
> > > > resultIntTy.getMinValue()` implies *at this place* that 
> > > > `ConvertedRHSValue.isSigned()`.
> > > > I would rather see this redundant check here to make the correctness 
> > > > reasoning local though.
> > > The integer representation does not have negative zeros (the standard and 
> > > clang assume two's complement). However, this condition does need to 
> > > check for the signedness of the types. What I mean is that if the `RHS` 
> > > is negative, but `ConvertedRHSValue` the branch will trigger and we will 
> > > change `x - INT_MIN` to `x + (INT_MAX + 1)U` which is ok, as a negation 
> > > of `INT_MIN` is representable as an unsigned type of same or lager bit 
> > > with.
> > > 
> > However, I was not able to reach this point with `RHS` being signed, and 
> > `resultTy` being unsigned. Any hints how this could be done?
> I'm not saying that I can follow this thought process. But the 
> `clang/test/Analysis/PR49642.c` would trigger an assertion like this:
> 
> ```lang=diff
> diff --git a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp 
> b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
> index 088c33c8e612..7e59309228e1 100644
> --- a/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
> +++ b/clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
> @@ -207,6 +207,16 @@ SVal SimpleSValBuilder::MakeSymIntVal(const SymExpr *LHS,
> "number of bits as its operands.");
>  
>  llvm::APSInt ConvertedRHSValue = resultIntTy.convert(RHS);
> +if (RHS.isSigned() && resultTy->isUnsignedIntegerOrEnumerationType()) {
> +  llvm::errs() << "LHS sym:\n";
> +  LHS->dump();
> +  llvm::errs() << "RHS integral:\n";
> +  RHS.dump();
> +  llvm::errs() << "OP: " << BinaryOperator::getOpcodeStr(op) << "\n";
> +  llvm::errs() << "result type:\n";
> +  resultTy->dump();
> +  llvm_unreachable("how is it possible??");
> +}
>  // Check if the negation of the RHS is representable,
>  // i.e., the resultTy is signed, and it is not the lowest
>  // representable negative value.
> ```
> 
> Which can be reduced into this one:
> 
> ```lang=c
> // RUN: %clang_analyze_cc1 -Wno-implicit-function-declaration -w -verify %s \
> // RUN:   -analyzer-checker=core \
> // RUN:   -analyzer-checker=apiModeling.StdCLibraryFunctions
> 
> // expected-no-diagnostics
> 
> typedef int ssize_t;
> int write(int, const void *, unsigned long);
> unsigned c;
> void a() {
>   int b = write(0, 0, c);
>   b != 0;
>   c -= b;
>   b < 1;
>   ++c; // crash simplifySValOnce: derived_$4{conj_$1{int, LC1, S700, #1},c}  
> op(-)  APInt(32b, 4294967295u -1s)  :: unsigned int
> }
> ```
What I mean, is that performing normalization (op and sign switch) is always 
correct for the unsigned `resultInTy`, even if `RHS` is the lowest 
representable negative number. The code is already behaving correctly in that 
case (I

[PATCH] D124702: [MSVC] Add support for pragma function

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 426998.
steplong added a comment.

Removed unnecessary braces (mentioned in the LLVM coding standard)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp

Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10218,10 +10218,12 @@
   // marking the function.
   AddCFAuditedAttribute(NewFD);
 
-  // If this is a function definition, check if we have to apply optnone due to
-  // a pragma.
-  if(D.isFunctionDefinition())
+  // If this is a function definition, check if we have to apply any
+  // attributes (i.e. optnone and no_builtin) due to a pragma.
+  if(D.isFunctionDefinition()) {
 AddRangeBasedOptnone(NewFD);
+AddRangeBasedNoBuiltin(NewFD);
+  }
 
   // If this is the first declaration of an extern C variable, update
   // the map of such variables.
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -1065,6 +1065,19 @@
 OptimizeOffPragmaLocation = PragmaLoc;
 }
 
+void Sema::ActOnPragmaMSIntrinsic(const std::vector &Intrinsics) {
+  for (auto &Intrinsic : Intrinsics)
+MSFunctionNoBuiltins.erase(std::remove(MSFunctionNoBuiltins.begin(),
+   MSFunctionNoBuiltins.end(),
+   Intrinsic),
+   MSFunctionNoBuiltins.end());
+}
+
+void Sema::ActOnPragmaMSFunction(const std::vector &NoBuiltins) {
+  MSFunctionNoBuiltins.insert(MSFunctionNoBuiltins.end(),
+  NoBuiltins.begin(), NoBuiltins.end());
+}
+
 void Sema::AddRangeBasedOptnone(FunctionDecl *FD) {
   // In the future, check other pragmas if they're implemented (e.g. pragma
   // optimize 0 will probably map to this functionality too).
@@ -1086,6 +1099,12 @@
 FD->addAttr(NoInlineAttr::CreateImplicit(Context, Loc));
 }
 
+void Sema::AddRangeBasedNoBuiltin(FunctionDecl *FD) {
+  if (!MSFunctionNoBuiltins.empty())
+FD->addAttr(NoBuiltinAttr::CreateImplicit(Context,
+MSFunctionNoBuiltins.data(), MSFunctionNoBuiltins.size()));
+}
+
 typedef std::vector > VisStack;
 enum : unsigned { NoVisibility = ~0U };
 
Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -250,9 +250,21 @@
 };
 
 struct PragmaMSIntrinsicHandler : public PragmaHandler {
-  PragmaMSIntrinsicHandler() : PragmaHandler("intrinsic") {}
+  PragmaMSIntrinsicHandler(Sema &S) : PragmaHandler("intrinsic"), Actions(S) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
+
+private:
+  Sema &Actions;
+};
+
+struct PragmaMSFunctionHandler : public PragmaHandler {
+  PragmaMSFunctionHandler(Sema &S) : PragmaHandler("function"), Actions(S) {}
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
+Token &FirstToken) override;
+
+private:
+  Sema &Actions;
 };
 
 struct PragmaMSOptimizeHandler : public PragmaHandler {
@@ -447,8 +459,10 @@
 PP.AddPragmaHandler(MSSection.get());
 MSRuntimeChecks = std::make_unique();
 PP.AddPragmaHandler(MSRuntimeChecks.get());
-MSIntrinsic = std::make_unique();
+MSIntrinsic = std::make_unique(Actions);
 PP.AddPragmaHandler(MSIntrinsic.get());
+MSFunction = std::make_unique(Actions);
+PP.AddPragmaHandler(MSFunction.get());
 MSOptimize = std::make_unique();
 PP.AddPragmaHandler(MSOptimize.get());
 MSFenvAccess = std::make_unique();
@@ -558,6 +572,8 @@
 MSRuntimeChecks.reset();
 PP.RemovePragmaHandler(MSIntrinsic.get());
 MSIntrinsic.reset();
+PP.RemovePragmaHandler(MSFunction.get());
+MSFunction.reset();
 PP.RemovePragmaHandler(MSOptimize.get());
 MSOptimize.reset();
 PP.RemovePragmaHandler(MSFenvAccess.get());
@@ -3523,11 +3539,14 @@
 
   bool SuggestIntrinH = !PP.isMacroDefined("__INTRIN_H");
 
+  std::vector Intrinsics;
   while (Tok.is(tok::identifier)) {
 IdentifierInfo *II = Tok.getIdentifierInfo();
 if (!II->getBuiltinID())
   PP.Diag(Tok.getLocation(), diag::warn_pragma_intrinsic_builtin)
   << II << SuggestIntrinH;
+else
+  Intrinsics.push_back(II->getName());
 
 PP.Lex(Tok);
 if (Tok.isNot(tok::comma))
@@ -3545,6 +3564,51 @@
   if (Tok.isNot(tok::eod))
 PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
 << "intrinsic";
+
+  Actions.ActOnPragmaMSIntrinsic(Intrinsics);
+}
+
+void PragmaMSFunctionHandler::Ha

[PATCH] D111548: [Clang] Add the `annotate_type` attribute

[Martin Böhme via Phabricator via cfe-commits]

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

In D111548#3483326 , @xbolva00 wrote:

> This patch should not land until we see some real use cases to justify new 
> hundreds of lines of code. We should more careful and take maintenance cost 
> really into account and not merge every experimental cool research extensions 
> automatically.

I assume you've seen the RFC 
 for the 
primary use case (Rust-style lifetime annotations for C++) that motivates this 
change?

There's a bit of a chicken-and-egg problem here: We obviously can't submit code 
for the lifetime analysis tooling before the type annotations it needs are 
supported. We're conscious that it's not desirable to add features merely for 
the purpose of an experimental tool that may never be stabilized. This is why 
we've consciously steered away from adding attributes that are specific to our 
use case and are instead proposing a general-purpose type annotation attribute.

> Please answer points in “ Contributing Extensions to Clang “
> https://clang.llvm.org/get_involved.html

This makes sense. I'll add an appendix answering these points to the RFC for 
this change 
.
 I probably won't get round to doing this today, but I did want to let you know 
that I've seen your comment. I'll ping this comment again when I've added the 
appendix to the RFC.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D111548

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

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource updated this revision to Diff 426999.

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

https://reviews.llvm.org/D124658

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/additive-op-on-sym-int-expr.c
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -11,7 +11,7 @@
 
 void foo(int x) {
   clang_analyzer_dump(x); // expected-warning{{reg_$0}}
-  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) + -1}}
+  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) - 1}}
   int y = 1;
   for (; y < 3; ++y) {
 clang_analyzer_numTimesReached(); // expected-warning{{2}}
Index: clang/test/Analysis/additive-op-on-sym-int-expr.c
===
--- /dev/null
+++ clang/test/Analysis/additive-op-on-sym-int-expr.c
@@ -0,0 +1,58 @@
+// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+
+void clang_analyzer_dump(int);
+void clang_analyzer_dumpL(long);
+void clang_analyzer_warnIfReached();
+
+void testInspect(int x) {
+  if ((x < 10) || (x > 100)) {
+return;
+  }
+  // x: [10, 100]
+
+  int i = x + 1;
+  long l = i - 10U;
+  clang_analyzer_dump(i);   // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}
+  clang_analyzer_dumpL(l);  // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
+  clang_analyzer_dumpL(l + 0L); // expected-warning-re {{(reg_${{[0-9]+}}) - 9 }}  instead of + 4294967287
+
+  if ((l - 1000) > 0) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if (l > 1000) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if (l > 1000L) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if ((l + 0L) > 1000) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+
+  i = x - 1;
+  l = i + 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) + 9U }} instead of - 4294967287U
+
+  i = x + (-1);
+  l = i - 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 11U }} instead of + 4294967285U
+
+  i = x + 1U;
+  l = i - 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
+}
+
+void testMin(int i, long l) {
+  clang_analyzer_dump(i + (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
+  clang_analyzer_dump(i - (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
+  clang_analyzer_dumpL(l + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
+  clang_analyzer_dumpL(l - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
+
+  int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negative value is not representable
+  // Do not normalize representation if negation would not be representable
+  clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
+  clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }}
+  // Produced value has higher bit with (long) so negation if representable
+  clang_analyzer_dumpL(l + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }} instead of + -2147483648
+  clang_analyzer_dumpL(l - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }} instead of - -2147483648
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -197,8 +197,27 @@
   if (RHS.isSigned() && !SymbolType->isSignedIntegerOrEnumerationType())
 ConvertedRHS = &BasicVals.Convert(SymbolType, RHS);
 }
-  } else
+  } else if (BinaryOperator::isAdditiveOp(op) && RHS.isNegative()) {
+// Change a+(-N) into a-N, and a-(-N) into a+N
+// Adjust addition/subtraction of negative value, to
+// subtraction/addition of the negated value.
+APSIntType resultIntTy = BasicVals.getAPSIntType(resultTy);
+assert(resultIntTy.getBitWidth() >= RHS.getBitWidth() &&
+   "The result operation type must have at least the same "
+   "number of bits as its operands.");
+
+llvm::APSInt ConvertedRHSValue = resultIntTy.convert(RHS);
+// Check if the negation of the RHS is representable,
+// i.e., the resultTy is signed, and it is not the lowest
+// representable negative value.
+if (ConvertedRHSValue > resultIntTy.getMinValue()) {
+  ConvertedRHS = &BasicVals.getValue(-ConvertedRHSValue);
+  op = (op == BO_Add) ? BO_Sub : BO_Add;
+}
+
+  } else {
 ConvertedRHS = &BasicVals.Convert(resultTy, RHS);
+  }
 
   return makeN

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

[Tomasz Kamiński via Phabricator via cfe-commits]

tomasz-kaminski-sonarsource updated this revision to Diff 427001.

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

https://reviews.llvm.org/D124658

Files:
  clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
  clang/test/Analysis/additive-op-on-sym-int-expr.c
  clang/test/Analysis/expr-inspection.c

Index: clang/test/Analysis/expr-inspection.c
===
--- clang/test/Analysis/expr-inspection.c
+++ clang/test/Analysis/expr-inspection.c
@@ -11,7 +11,7 @@
 
 void foo(int x) {
   clang_analyzer_dump(x); // expected-warning{{reg_$0}}
-  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) + -1}}
+  clang_analyzer_dump(x + (-1)); // expected-warning{{(reg_$0) - 1}}
   int y = 1;
   for (; y < 3; ++y) {
 clang_analyzer_numTimesReached(); // expected-warning{{2}}
Index: clang/test/Analysis/additive-op-on-sym-int-expr.c
===
--- /dev/null
+++ clang/test/Analysis/additive-op-on-sym-int-expr.c
@@ -0,0 +1,58 @@
+// RUN: %clang_analyze_cc1 -triple=x86_64-unknown-linux -analyzer-checker=core,debug.ExprInspection -analyzer-config eagerly-assume=false -verify %s
+
+void clang_analyzer_dump(int);
+void clang_analyzer_dumpL(long);
+void clang_analyzer_warnIfReached();
+
+void testInspect(int x) {
+  if ((x < 10) || (x > 100)) {
+return;
+  }
+  // x: [10, 100]
+
+  int i = x + 1;
+  long l = i - 10U;
+  clang_analyzer_dump(i);   // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }}
+  clang_analyzer_dumpL(l);  // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
+  clang_analyzer_dumpL(l + 0L); // expected-warning-re {{(reg_${{[0-9]+}}) - 9 }}  instead of + 4294967287
+
+  if ((l - 1000) > 0) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if (l > 1000) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if (l > 1000L) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+  if ((l + 0L) > 1000) {
+clang_analyzer_warnIfReached(); // no-warning
+  }
+
+  i = x - 1;
+  l = i + 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) + 9U }} instead of - 4294967287U
+
+  i = x + (-1);
+  l = i - 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 11U }} instead of + 4294967285U
+
+  i = x + 1U;
+  l = i - 10U;
+  clang_analyzer_dumpL(l); // expected-warning-re {{(reg_${{[0-9]+}}) - 9U }} instead of + 4294967287U
+}
+
+void testMin(int i, long l) {
+  clang_analyzer_dump(i + (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
+  clang_analyzer_dump(i - (-1));  // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
+  clang_analyzer_dumpL(l + (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) - 1 }} instead of + -1
+  clang_analyzer_dumpL(l - (-1)); // expected-warning-re {{(reg_${{[0-9]+}}) + 1 }} instead of - -1
+
+  int intMin = 1 << (sizeof(int) * 8 - 1); // INT_MIN, negative value is not representable
+  // Do not normalize representation if negation would not be representable
+  clang_analyzer_dump(i + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + -2147483648 }}
+  clang_analyzer_dump(i - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - -2147483648 }}
+  // Produced value has higher bit with (long) so negation if representable
+  clang_analyzer_dumpL(l + intMin); // expected-warning-re {{(reg_${{[0-9]+}}) - 2147483648 }} instead of + -2147483648
+  clang_analyzer_dumpL(l - intMin); // expected-warning-re {{(reg_${{[0-9]+}}) + 2147483648 }} instead of - -2147483648
+}
Index: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
===
--- clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
+++ clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp
@@ -197,8 +197,28 @@
   if (RHS.isSigned() && !SymbolType->isSignedIntegerOrEnumerationType())
 ConvertedRHS = &BasicVals.Convert(SymbolType, RHS);
 }
-  } else
+  } else if (BinaryOperator::isAdditiveOp(op) && RHS.isNegative()) {
+// Change a+(-N) into a-N, and a-(-N) into a+N
+// Adjust addition/subtraction of negative value, to
+// subtraction/addition of the negated value.
+APSIntType resultIntTy = BasicVals.getAPSIntType(resultTy);
+assert(resultIntTy.getBitWidth() >= RHS.getBitWidth() &&
+   "The result operation type must have at least the same "
+   "number of bits as its operands.");
+
+llvm::APSInt ConvertedRHSValue = resultIntTy.convert(RHS);
+// Check if the negation of the RHS is representable:
+// * if resultTy is unsigned, then negation is always representable
+// * if resultTy is signed, and RHS is not the lowest representable
+//   negative value for resultTy.
+if (resultIntTy.isUnsigned() ||
+(ConvertedRHSValue > resultIntTy.getMinValue())) {
+  ConvertedRHS = &BasicVals.getValue(-ConvertedRHSValue);
+  op = (op == BO

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp:11
+
+#include 
+#include 

C++ headers should be after application's ones.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp:116
+CalleesForStmts[CurrentExprOrDeclStmt].insert(Callee);
+auto *CXXMethod = dyn_cast(Callee);
+if (!CXXMethod || CXXMethod->isStatic()) {

`const auto *`. Same in other places.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h:12
+
+#include 
+

C++ headers should be after application's ones.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:8
+
+A variable is defined to be side-effect-free and unused if
+

Missing colon?



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:10
+
+- It has a side-effect-free type (defined below)
+- It is not passed to a function that reads its content and whose result is 
used

Missing period. Same in other places.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:15
+
+- A type with a base class specified in TypesByBase
+- A type listed in RawTypes

Please highlight `TypesByBase` with single back-ticks. Same for other elements 
of JSON.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:27
+
+- It is a method of the explicitly listed types and the argument is `this`,
+  except:

Please use double back-ticks for language constructs like `this`.



Comment at: 
clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst:30
+
+  - The exceptions associated with the type (eg. std::vector::swap).
+  - A constructor of a smart pointer type that takes a non-newly-allocated

Please highlight `std::vector::swap` with double back-ticks.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D124674: [analyzer] Indicate if a parent state is infeasible

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@NoQ Could you please take a look? This change effects the very core of the 
analyzer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124674

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

[PATCH] D124702: [MSVC] Add support for pragma function

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.



>> And should we error/warn if the pragma occurs not in namespace scope?
>
> Oh I wasn't sure how to check if the function pragma was outside of a 
> function.

In the ActOn.. methods I think you can check what the CurContext is.




Comment at: clang/lib/Parse/ParsePragma.cpp:3542
 
+  std::vector Intrinsics;
   while (Tok.is(tok::identifier)) {

SmallVector would be more common LLVM style, I think. The ActOn.. methods could 
take the argument as a const-ref to SmallVectorImpl. This applies to 
the function pragma below as well.



Comment at: clang/lib/Parse/ParsePragma.cpp:3561
 << "intrinsic";
 return;
   }

since the above is just a warning, we should probably still call the ActOn.. 
method?



Comment at: clang/lib/Parse/ParsePragma.cpp:3605
+<< "function";
+return;
+  }

same as above: since it's just a warning, maybe we still want to call the 
ActOn.. method?



Comment at: clang/lib/Sema/SemaAttr.cpp:1079
+  MSFunctionNoBuiltins.insert(MSFunctionNoBuiltins.end(),
+  NoBuiltins.begin(), NoBuiltins.end());
+}

Do we want to avoid duplicates in MSFunctionNoBuiltins? Or maybe it doesn't 
matter?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Luca Versari via Phabricator via cfe-commits]

veluca93 updated this revision to Diff 427007.
veluca93 marked 7 inline comments as done.
veluca93 added a comment.

Address first round of review comments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s performance-unused-no-side-effect %t -- \
+// RUN:  -config="{CheckOptions: [{key: \"performance-unused-no-side-effect.TypesByBase\", value: '{\"Marker\":[\"swap\"]}'}]}"
+
+namespace std {
+
+template 
+struct char_traits {};
+
+template 
+struct allocator {};
+
+template <
+class CharT,
+class Traits = std::char_traits,
+class Allocator = std::allocator>
+class basic_string {
+public:
+  basic_string &operator+=(const char *);
+  ~basic_string();
+};
+
+using string = basic_string;
+
+class thread {
+public:
+  ~thread();
+};
+
+template >
+class vector {
+public:
+  void push_back(T);
+};
+
+} // namespace std
+
+struct Marker {
+};
+
+struct SideEffectFree : Marker {
+  SideEffectFree();
+  ~SideEffectFree();
+  void swap(SideEffectFree *);
+};
+
+struct POD {
+  int A;
+};
+
+void f(const std::string &);
+
+void unusedString() {
+  std::string SomeString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+}
+
+void unusedStringWithMethodCall() {
+  std::string SomeString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+  SomeString += "hi";
+}
+
+void unusedStdThread() {
+  std::thread T;
+}
+
+void usedString() {
+  std::string SomeString;
+  SomeString += "hi";
+  f(SomeString);
+}
+
+void vectorString() {
+  std::vector VecString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: variable 'VecString' is never read [performance-unused-no-side-effect]
+  VecString.push_back(std::string{});
+}
+
+void vectorThread() {
+  std::vector VecThread;
+  VecThread.push_back(std::thread{});
+}
+
+void withMarker() {
+  SideEffectFree M;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: variable 'M' is never read [performance-unused-no-side-effect]
+}
+
+void withMarkerAndExcludedMethod(SideEffectFree *S) {
+  SideEffectFree M;
+  M.swap(S);
+}
+
+void withPOD() {
+  POD P;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'P' is never read [performance-unused-no-side-effect]
+  P.A = 1;
+}
+
+int withPODReturn() {
+  POD P;
+  P.A = 1;
+  return P.A;
+}
+
+int main() {
+  std::vector VecLoops;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: variable 'VecLoops' is never read [performance-unused-no-side-effect]
+  for (int i = 0; i < 15; i++) {
+VecLoops.push_back(1);
+  }
+  int j = 0;
+  while (j < 1) {
+VecLoops.push_back(2);
+j++;
+  }
+  (VecLoops).push_back(3);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
@@ -0,0 +1,193 @@
+.. title:: clang-tidy - performance-unused-no-side-effect
+
+performance-unused-no-side-effect
+=
+
+Finds variables that are not used and do not have user-visible side effects.
+
+A variable is defined to be side-effect-free and unused if:
+
+- It has a side-effect-free type (defined below).
+- It is not passed to a function that reads its content and whose result is used.
+
+A type is side-effect-free if it is any of the following:
+
+- A type with a base class specified in `TypesByBase`.
+- A type listed in `RawTypes`.
+- A template type listed in `TemplateTypes` whose template arguments are
+  themselves side-effect-free types.
+- A smart pointer (listed in `SmartPointerTypes`) whose pointee is of
+  side-effect-free type.
+- A primitive type.
+- A pointer.
+- A struct/class with no user-defined destructor and members of trivial type.
+
+A function is considered to "not read an argument unless the return value is
+used" in the following cases:
+
+- It is a method of the explicitly listed types and the argument is ``this``,
+  except:
+
+  - The exceptions associated 

[PATCH] D124525: [OpenMP][ClangLinkerWrapper] Extending linker wrapper to embed metadata for multi-arch fat binaries

[Joseph Huber via Phabricator via cfe-commits]

jhuber6 added a comment.

I'm suggesting instead we define a new `__tgt_device_image` and a new 
`__tgt_register_lib` function to support this. This new `__tgt_device_image` 
will simply contain a pointer to an optional information struct.

  struct __tgt_device_image_v2 {
void*   ImageStart;
void*   ImageEnd;
__tgt_offload_entry*EntriesBegin;
__tgt_offload_entry*EntriesEnd;
__tgt_image_into*  ImageInfo;
  };

This new struct breaks the ABI with the old `__tgt_device_image` because these 
are put into an array and we change the size, but we should be able to provide 
backwards compatibility by copying from the old format to the new format and 
creating a new array. We can detect the new vs. old ABI by expecting that 
existing applications will call the `__tgt_register_image` function. We will 
create a new `__tgt_register_image_v2` function for example that all new 
programs will call. In `libomptarget` we then change `__tgt_register_image` to 
do the necessary translation.

  struct __tgt_bin_desc {   

   
int32_t NumDeviceImages;   // Number of device types supported  

 
__tgt_device_image *DeviceImages;  // Array of device images (1 per dev. 
type)
__tgt_offload_entry *HostEntriesBegin; // Begin of table with all host 
entries   
__tgt_offload_entry *HostEntriesEnd;   // End of table (non inclusive)  

 
  };
  
  EXTERN void __tgt_register_lib(__tgt_bin_desc *desc) {
__tgt_device_image_v2 *new_image = alloc_new_version(desc);
desc->DeviceImages = new_Image;
__tgt_register_lib_v2(desc);
  }
  
  EXTERN void __tgt_unregister_lib(__tgt_bin_desc *desc) { // Probably called 
by __tgt_unregister_lib_v2
dealloc(desc->DeviceImages);
  }

Now the rest of `libomptarget` solely uses the new format, and we check if 
information is available by seeing that the `ImageInfo` field is non-null.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124525

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

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

[Sam McCall via Phabricator via cfe-commits]

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

Thanks for cleaning up the scary new/deletes.




Comment at: clang/lib/Sema/SemaConcept.cpp:321
 
+  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
+  if (!ShouldCache) {

Another loose end that could be cleaned up sometime!

`ConceptSatisfactionCaching` is on by default and controlled by an -f flag.
It was added in https://reviews.llvm.org/D72552 - essential for performance, 
unclear whether the caching scheme was allowed.

Probably this option should be removed (and if needed, the caching scheme 
semantics made to match what was standardized)



Comment at: clang/lib/Sema/SemaConcept.cpp:335
+  auto Satisfaction =
+  std::make_unique(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,

ilya-biryukov wrote:
> I also wonder if this could be allocated in the AST arena, maybe worth 
> exploring.
> Definitely outside this change, though, would like to keep this one an NFC.
not just could, but must - FoldingSet doesn't own its objects, so this object 
is leaked once the FoldingSet is destroyed! Allocating it on the ASTContext 
would fix this because they have the same lifetime.

Agree with not mixing it into this patch, but please add a FIXME and do follow 
up on it if you don't mind.
(and at that point, should we really be passing it back by value?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124923

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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp:135
+// or class, and that there are no side effects on `this`.
+if (auto *Callee = dyn_cast(Call->getCallee())) {
+  markSideEffectFree(Callee);

`const auto *`. Same in other similar places.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Luca Versari via Phabricator via cfe-commits]

veluca93 updated this revision to Diff 427013.
veluca93 added a comment.

Missing `const` from auto *


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

Files:
  clang-tools-extra/clang-tidy/performance/CMakeLists.txt
  clang-tools-extra/clang-tidy/performance/PerformanceTidyModule.cpp
  clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp
  clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.h
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
  
clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/performance-unused-no-side-effect.cpp
@@ -0,0 +1,118 @@
+// RUN: %check_clang_tidy %s performance-unused-no-side-effect %t -- \
+// RUN:  -config="{CheckOptions: [{key: \"performance-unused-no-side-effect.TypesByBase\", value: '{\"Marker\":[\"swap\"]}'}]}"
+
+namespace std {
+
+template 
+struct char_traits {};
+
+template 
+struct allocator {};
+
+template <
+class CharT,
+class Traits = std::char_traits,
+class Allocator = std::allocator>
+class basic_string {
+public:
+  basic_string &operator+=(const char *);
+  ~basic_string();
+};
+
+using string = basic_string;
+
+class thread {
+public:
+  ~thread();
+};
+
+template >
+class vector {
+public:
+  void push_back(T);
+};
+
+} // namespace std
+
+struct Marker {
+};
+
+struct SideEffectFree : Marker {
+  SideEffectFree();
+  ~SideEffectFree();
+  void swap(SideEffectFree *);
+};
+
+struct POD {
+  int A;
+};
+
+void f(const std::string &);
+
+void unusedString() {
+  std::string SomeString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+}
+
+void unusedStringWithMethodCall() {
+  std::string SomeString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:15: warning: variable 'SomeString' is never read [performance-unused-no-side-effect]
+  SomeString += "hi";
+}
+
+void unusedStdThread() {
+  std::thread T;
+}
+
+void usedString() {
+  std::string SomeString;
+  SomeString += "hi";
+  f(SomeString);
+}
+
+void vectorString() {
+  std::vector VecString;
+  // CHECK-MESSAGES: :[[@LINE-1]]:28: warning: variable 'VecString' is never read [performance-unused-no-side-effect]
+  VecString.push_back(std::string{});
+}
+
+void vectorThread() {
+  std::vector VecThread;
+  VecThread.push_back(std::thread{});
+}
+
+void withMarker() {
+  SideEffectFree M;
+  // CHECK-MESSAGES: :[[@LINE-1]]:18: warning: variable 'M' is never read [performance-unused-no-side-effect]
+}
+
+void withMarkerAndExcludedMethod(SideEffectFree *S) {
+  SideEffectFree M;
+  M.swap(S);
+}
+
+void withPOD() {
+  POD P;
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: variable 'P' is never read [performance-unused-no-side-effect]
+  P.A = 1;
+}
+
+int withPODReturn() {
+  POD P;
+  P.A = 1;
+  return P.A;
+}
+
+int main() {
+  std::vector VecLoops;
+  // CHECK-MESSAGES: :[[@LINE-1]]:20: warning: variable 'VecLoops' is never read [performance-unused-no-side-effect]
+  for (int i = 0; i < 15; i++) {
+VecLoops.push_back(1);
+  }
+  int j = 0;
+  while (j < 1) {
+VecLoops.push_back(2);
+j++;
+  }
+  (VecLoops).push_back(3);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/performance-unused-no-side-effect.rst
@@ -0,0 +1,193 @@
+.. title:: clang-tidy - performance-unused-no-side-effect
+
+performance-unused-no-side-effect
+=
+
+Finds variables that are not used and do not have user-visible side effects.
+
+A variable is defined to be side-effect-free and unused if:
+
+- It has a side-effect-free type (defined below).
+- It is not passed to a function that reads its content and whose result is used.
+
+A type is side-effect-free if it is any of the following:
+
+- A type with a base class specified in `TypesByBase`.
+- A type listed in `RawTypes`.
+- A template type listed in `TemplateTypes` whose template arguments are
+  themselves side-effect-free types.
+- A smart pointer (listed in `SmartPointerTypes`) whose pointee is of
+  side-effect-free type.
+- A primitive type.
+- A pointer.
+- A struct/class with no user-defined destructor and members of trivial type.
+
+A function is considered to "not read an argument unless the return value is
+used" in the following cases:
+
+- It is a method of the explicitly listed types and the argument is ``this``,
+  except:
+
+  - The exceptions associated with the type (eg. ``std::vector::swap``).
+  - A const

[PATCH] D124918: [clang-tidy] Add a new check for non-trivial unused variables.

[Luca Versari via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/performance/UnusedNoSideEffectCheck.cpp:135
+// or class, and that there are no side effects on `this`.
+if (auto *Callee = dyn_cast(Call->getCallee())) {
+  markSideEffectFree(Callee);

Eugene.Zelenko wrote:
> `const auto *`. Same in other similar places.
Somehow I missed a couple of those... should be fixed now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124918

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

[PATCH] D119147: [AIX][clang][driver] Check the command string to the linker for exportlist opts and

[Digger Lin via Phabricator via cfe-commits]

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



Comment at: clang/test/Driver/aix-ld.c:675-676
+// CHECK-LD32-SHARED-EXPORTS-NOT: "{{.*}}llvm-nm"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "-X"
+// CHECK-LD32-SHARED-EXPORTS-NOT: "32"
+// CHECK-LD32-SHARED-EXPORTS: "{{.*}}ld{{(.exe)?}}"

stevewan wrote:
> And please keep this consistent across the tests.
using:
// CHECK-LD32-SHARED-EXPORTS-NOT: "-X"
// CHECK-LD32-SHARED-EXPORTS-NOT: "32"

will be fail on the -X 64 too.

using  // CHECK-LD32-SHARED-EXPORTS-NOT: "-X" "32"
 will be success on the -X 64. 
So I am prefer use the
 CHECK-LD32-SHARED-EXPORTS-NOT: "-X"
 CHECK-LD32-SHARED-EXPORTS-NOT: "32" 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119147

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

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov updated this revision to Diff 427018.
ilya-biryukov added a comment.

- add FIXME for a memory leak


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124923

Files:
  clang/lib/Sema/SemaConcept.cpp


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -318,35 +318,31 @@
 return false;
   }
 
+  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
+  if (!ShouldCache) {
+return ::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
+ TemplateArgs, TemplateIDRange,
+ OutSatisfaction);
+  }
   llvm::FoldingSetNodeID ID;
+  ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
   void *InsertPos;
-  ConstraintSatisfaction *Satisfaction = nullptr;
-  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
-  if (ShouldCache) {
-ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
-Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos);
-if (Satisfaction) {
-  OutSatisfaction = *Satisfaction;
-  return false;
-}
-Satisfaction = new ConstraintSatisfaction(Template, TemplateArgs);
-  } else {
-Satisfaction = &OutSatisfaction;
+  if (auto *Cached = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos)) {
+OutSatisfaction = *Cached;
+return false;
   }
+  auto Satisfaction =
+  std::make_unique(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
 TemplateArgs, TemplateIDRange,
 *Satisfaction)) {
-if (ShouldCache)
-  delete Satisfaction;
 return true;
   }
-
-  if (ShouldCache) {
-// We cannot use InsertNode here because CheckConstraintSatisfaction might
-// have invalidated it.
-SatisfactionCache.InsertNode(Satisfaction);
-OutSatisfaction = *Satisfaction;
-  }
+  OutSatisfaction = *Satisfaction;
+  // We cannot use InsertPos here because CheckConstraintSatisfaction might 
have
+  // invalidated it.
+  // FIXME: this leaks memory, we should allocate in the arena instead.
+  SatisfactionCache.InsertNode(Satisfaction.release());
   return false;
 }
 


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -318,35 +318,31 @@
 return false;
   }
 
+  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
+  if (!ShouldCache) {
+return ::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
+ TemplateArgs, TemplateIDRange,
+ OutSatisfaction);
+  }
   llvm::FoldingSetNodeID ID;
+  ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
   void *InsertPos;
-  ConstraintSatisfaction *Satisfaction = nullptr;
-  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
-  if (ShouldCache) {
-ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
-Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos);
-if (Satisfaction) {
-  OutSatisfaction = *Satisfaction;
-  return false;
-}
-Satisfaction = new ConstraintSatisfaction(Template, TemplateArgs);
-  } else {
-Satisfaction = &OutSatisfaction;
+  if (auto *Cached = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos)) {
+OutSatisfaction = *Cached;
+return false;
   }
+  auto Satisfaction =
+  std::make_unique(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
 TemplateArgs, TemplateIDRange,
 *Satisfaction)) {
-if (ShouldCache)
-  delete Satisfaction;
 return true;
   }
-
-  if (ShouldCache) {
-// We cannot use InsertNode here because CheckConstraintSatisfaction might
-// have invalidated it.
-SatisfactionCache.InsertNode(Satisfaction);
-OutSatisfaction = *Satisfaction;
-  }
+  OutSatisfaction = *Satisfaction;
+  // We cannot use InsertPos here because CheckConstraintSatisfaction might have
+  // invalidated it.
+  // FIXME: this leaks memory, we should allocate in the arena instead.
+  SatisfactionCache.InsertNode(Satisfaction.release());
   return false;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

[Ilya Biryukov via Phabricator via cfe-commits]

ilya-biryukov added inline comments.



Comment at: clang/lib/Sema/SemaConcept.cpp:321
 
+  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
+  if (!ShouldCache) {

sammccall wrote:
> Another loose end that could be cleaned up sometime!
> 
> `ConceptSatisfactionCaching` is on by default and controlled by an -f flag.
> It was added in https://reviews.llvm.org/D72552 - essential for performance, 
> unclear whether the caching scheme was allowed.
> 
> Probably this option should be removed (and if needed, the caching scheme 
> semantics made to match what was standardized)
Good point, thanks. I'll ask around to see whether the committee discussions 
led anywhere.



Comment at: clang/lib/Sema/SemaConcept.cpp:335
+  auto Satisfaction =
+  std::make_unique(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,

sammccall wrote:
> ilya-biryukov wrote:
> > I also wonder if this could be allocated in the AST arena, maybe worth 
> > exploring.
> > Definitely outside this change, though, would like to keep this one an NFC.
> not just could, but must - FoldingSet doesn't own its objects, so this object 
> is leaked once the FoldingSet is destroyed! Allocating it on the ASTContext 
> would fix this because they have the same lifetime.
> 
> Agree with not mixing it into this patch, but please add a FIXME and do 
> follow up on it if you don't mind.
> (and at that point, should we really be passing it back by value?)
Oh, wow, good catch, thanks. I'll send a separate fix for the memory leak.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124923

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

[PATCH] D124842: [NFC][CUDA][HIP] rework mangling number for aux target

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: clang/lib/AST/ASTContext.cpp:11770-11778
+  if (!LangOpts.CUDA || LangOpts.CUDAIsDevice) {
+assert(!ForAuxTarget && "Only CUDA/HIP host compilation supports mangling "
+"number for aux target");
 return Res;
+  }
 
   // CUDA/HIP host compilation encodes host and device mangling numbers

tra wrote:
> Nit: I'd rephrase it as :
> ```
> if (LangOpts.CUDA && !LangOpts.CUDAIsDevice) {
> Res = ForAuxTarget ? Res >> 16 : Res & 0x; 
> } else {
> assert(!ForAuxTarget && "Only CUDA/HIP host compilation supports mangling 
> number for aux target");
> }
> return Res > 1 ? Res : 1;
> ```
will do when committing.


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

https://reviews.llvm.org/D124842

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

[PATCH] D124807: [clang][dataflow] Only skip ExprWithCleanups when visiting terminators

[Eric Li via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG62b2a47a9f15: [clang][dataflow] Only skip ExprWithCleanups 
when visiting terminators (authored by li.zhe.hua).

Changed prior to commit:
  https://reviews.llvm.org/D124807?vs=426987&id=427019#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124807

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

Index: clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp
@@ -1152,4 +1152,38 @@
   });
 }
 
+TEST_F(FlowConditionTest, PointerToBoolImplicitCast) {
+  std::string Code = R"(
+void target(int *Ptr) {
+  bool Foo = false;
+  if (Ptr) {
+Foo = true;
+/*[[p1]]*/
+  }
+
+  (void)0;
+  /*[[p2]]*/
+}
+  )";
+  runDataflow(
+  Code, [](llvm::ArrayRef<
+   std::pair>>
+   Results,
+   ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p2", _), Pair("p1", _)));
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const Environment &Env1 = Results[1].second.Env;
+auto &FooVal1 =
+*cast(Env1.getValue(*FooDecl, SkipPast::Reference));
+EXPECT_TRUE(Env1.flowConditionImplies(FooVal1));
+
+const Environment &Env2 = Results[0].second.Env;
+auto &FooVal2 =
+*cast(Env2.getValue(*FooDecl, SkipPast::Reference));
+EXPECT_FALSE(Env2.flowConditionImplies(FooVal2));
+  });
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
===
--- clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
+++ clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
@@ -77,26 +77,26 @@
   : StmtToEnv(StmtToEnv), Env(Env), BlockSuccIdx(BlockSuccIdx) {}
 
   void VisitIfStmt(const IfStmt *S) {
-auto *Cond = S->getCond()->IgnoreParenImpCasts();
+auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
 assert(Cond != nullptr);
 extendFlowCondition(*Cond);
   }
 
   void VisitWhileStmt(const WhileStmt *S) {
-auto *Cond = S->getCond()->IgnoreParenImpCasts();
+auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
 assert(Cond != nullptr);
 extendFlowCondition(*Cond);
   }
 
   void VisitBinaryOperator(const BinaryOperator *S) {
 assert(S->getOpcode() == BO_LAnd || S->getOpcode() == BO_LOr);
-auto *LHS = S->getLHS()->IgnoreParenImpCasts();
+auto *LHS = ignoreExprWithCleanups(S->getLHS())->IgnoreParens();
 assert(LHS != nullptr);
 extendFlowCondition(*LHS);
   }
 
   void VisitConditionalOperator(const ConditionalOperator *S) {
-auto *Cond = S->getCond()->IgnoreParenImpCasts();
+auto *Cond = ignoreExprWithCleanups(S->getCond())->IgnoreParens();
 assert(Cond != nullptr);
 extendFlowCondition(*Cond);
   }
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
===
--- clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -33,7 +33,7 @@
 namespace clang {
 namespace dataflow {
 
-static const Expr *skipExprWithCleanups(const Expr *E) {
+const Expr *ignoreExprWithCleanups(const Expr *E) {
   if (auto *C = dyn_cast_or_null(E))
 return C->getSubExpr();
   return E;
@@ -155,9 +155,7 @@
   return;
 }
 
-// The CFG does not contain `ParenExpr` as top-level statements in basic
-// blocks, however sub-expressions can still be of that type.
-InitExpr = skipExprWithCleanups(D.getInit()->IgnoreParens());
+InitExpr = ignoreExprWithCleanups(D.getInit());
 assert(InitExpr != nullptr);
 
 if (D.getType()->isReferenceType()) {
@@ -190,10 +188,7 @@
   }
 
   void VisitImplicitCastExpr(const ImplicitCastExpr *S) {
-// The CFG does not contain `ParenExpr` as top-level statements in basic
-// blocks, however sub-expressions can still be of that type.
-assert(S->getSubExpr() != nullptr);
-const Expr *SubExpr = S->getSubExpr()->IgnoreParens();
+const Expr *SubExpr = S->getSubExpr();
 assert(SubExpr != nullptr);
 
 switch (S->getCastKind()) {
@@ -252,10 +247,7 @@
   }
 
   void VisitUnaryOperator(const UnaryOperator *S) {
-  

[clang] 62b2a47 - [clang][dataflow] Only skip ExprWithCleanups when visiting terminators

[Eric Li via cfe-commits]

Author: Eric Li
Date: 2022-05-04T15:31:49Z
New Revision: 62b2a47a9f15ed2f1dc4b39c924341c7b9bd7cf8

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

LOG: [clang][dataflow] Only skip ExprWithCleanups when visiting terminators

`IgnoreParenImpCasts` will remove implicit casts to bool
(e.g. `PointerToBoolean`), such that the resulting expression may not
be of the `bool` type. The `cast_or_null` in
`extendFlowCondition` will then trigger an assert, as the pointer
expression will not have a `BoolValue`.

Instead, we only skip `ExprWithCleanups` and `ParenExpr` nodes, as the
CFG does not emit them.

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

Added: 


Modified: 
clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
clang/include/clang/Analysis/FlowSensitive/Transfer.h
clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/lib/Analysis/FlowSensitive/TypeErasedDataflowAnalysis.cpp
clang/unittests/Analysis/FlowSensitive/TypeErasedDataflowAnalysisTest.cpp

Removed: 




diff  --git a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h 
b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
index 0a2c75f804c2a..019d07c9b7f72 100644
--- a/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
+++ b/clang/include/clang/Analysis/FlowSensitive/DataflowEnvironment.h
@@ -172,6 +172,10 @@ class Environment {
   /// Creates a storage location for `E`. Does not assign the returned storage
   /// location to `E` in the environment. Does not assign a value to the
   /// returned storage location in the environment.
+  ///
+  /// Requirements:
+  ///
+  ///  `E` must not be a `ExprWithCleanups`.
   StorageLocation &createStorageLocation(const Expr &E);
 
   /// Assigns `Loc` as the storage location of `D` in the environment.
@@ -191,11 +195,16 @@ class Environment {
   /// Requirements:
   ///
   ///  `E` must not be assigned a storage location in the environment.
+  ///  `E` must not be a `ExprWithCleanups`.
   void setStorageLocation(const Expr &E, StorageLocation &Loc);
 
   /// Returns the storage location assigned to `E` in the environment, applying
   /// the `SP` policy for skipping past indirections, or null if `E` isn't
   /// assigned a storage location in the environment.
+  ///
+  /// Requirements:
+  ///
+  ///  `E` must not be a `ExprWithCleanups`.
   StorageLocation *getStorageLocation(const Expr &E, SkipPast SP) const;
 
   /// Returns the storage location assigned to the `this` pointee in the
@@ -226,6 +235,12 @@ class Environment {
 
   /// Equivalent to `getValue(getStorageLocation(E, SP), SkipPast::None)` if 
`E`
   /// is assigned a storage location in the environment, otherwise returns 
null.
+  ///
+  /// Requirements:
+  ///
+  ///  `E` must not be a `ExprWithCleanups`.
+  ///
+  /// FIXME: `Environment` should ignore any `ExprWithCleanups` it sees.
   Value *getValue(const Expr &E, SkipPast SP) const;
 
   /// Transfers ownership of `Loc` to the analysis context and returns a

diff  --git a/clang/include/clang/Analysis/FlowSensitive/Transfer.h 
b/clang/include/clang/Analysis/FlowSensitive/Transfer.h
index a6b663b997fd6..c5b1086c451fd 100644
--- a/clang/include/clang/Analysis/FlowSensitive/Transfer.h
+++ b/clang/include/clang/Analysis/FlowSensitive/Transfer.h
@@ -35,9 +35,19 @@ class StmtToEnvMap {
 ///
 /// Requirements:
 ///
-///  The type of `S` must not be `ParenExpr`.
+///  `S` must not be `ParenExpr` or `ExprWithCleanups`.
 void transfer(const StmtToEnvMap &StmtToEnv, const Stmt &S, Environment &Env);
 
+/// Skip past a `ExprWithCleanups` which might surround `E`. Returns null if 
`E`
+/// is null.
+///
+/// The CFG omits `ExprWithCleanups` nodes (as it does with `ParenExpr`), and 
so
+/// the transfer function doesn't accept them as valid input. Manual traversal
+/// of the AST should skip and unwrap any `ExprWithCleanups` it might expect to
+/// see. They are safe to skip, as the CFG will emit calls to destructors as
+/// appropriate.
+const Expr *ignoreExprWithCleanups(const Expr *E);
+
 } // namespace dataflow
 } // namespace clang
 

diff  --git a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp 
b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
index 469696643d319..ad917f18b2570 100644
--- a/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ b/clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -15,6 +15,7 @@
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/AST/Decl.h"
 #include "clang/AST/DeclCXX.h"
+#include "clang/AST/ExprCXX.h"
 #include "clang/AST/Type.h"
 #include "clang/Analysis/FlowSensitive/DataflowLattice.h"
 #include "clang/Analysis/FlowSensitive/StorageLocat

[PATCH] D120290: [Clang][OpenMP] Add the codegen support for `atomic compare capture`

[Shilei Tian via Phabricator via cfe-commits]

tianshilei1992 added a comment.

ping




Comment at: clang/lib/Sema/SemaOpenMP.cpp:11663
   X = BO->getLHS();
-  D = BO->getRHS();
+  D = BO->getRHS()->IgnoreImpCasts();
 

tianshilei1992 wrote:
> ABataev wrote:
> > Why do we need to use `IgnoreImpCasts()` here and in other places?
> Clang usually inserts implicit casts. For example, if we have:
> ```
> char a, b, c;
> #pragma omp atomic compare capture
>   { r = a; if (a > c) { a = b; } }
> ```
> Clang inserts an implicit cast from `char` to `int` for all statements except 
> `r = a`. In this case, what should be the right solution? I'm not quite sure 
> actually.
https://godbolt.org/z/a6WWdx581
This shows how the AST looks like.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120290

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

[PATCH] D124790: [HLSL] Enable half type for hlsl.

[Chris Bieneman via Phabricator via cfe-commits]

beanz added inline comments.



Comment at: clang/lib/Basic/LangOptions.cpp:197
 
-  // OpenCL has half keyword
-  Opts.Half = Opts.OpenCL;
+  // OpenCL and HLSL have half keyword
+  Opts.Half = Opts.OpenCL || Opts.HLSL;

aaron.ballman wrote:
> python3kgae wrote:
> > python3kgae wrote:
> > > aaron.ballman wrote:
> > > > python3kgae wrote:
> > > > > aaron.ballman wrote:
> > > > > > Shouldn't this be looking for HLSL 2018? Or shader model 6.2?
> > > > > half keyword is always available.
> > > > > Without enable_16bit_types, half will be like using half=float.
> > > > > With enable_16bit_types, half will be real half.
> > > > > 
> > > > > The check for HLSL 2018 and shader model 6.2 will be in another PR, 
> > > > > still WIP. I'll add FIXME about it.
> > > > > half keyword is always available.
> > > > > Without enable_16bit_types, half will be like using half=float.
> > > > > With enable_16bit_types, half will be real half.
> > > > 
> > > > Is there room for change here, or is this strictly required by HLSL? 
> > > > This strikes me as just begging to confuse users into creating ODR 
> > > > violations. CC @beanz 
> > > Here's the doc about half for dxc.
> > > https://github.com/microsoft/DirectXShaderCompiler/wiki/16-Bit-Scalar-Types
> > > 
> > > Old doc for fxc (the old shader compiler for shader model <= 5.1) is here
> > > https://docs.microsoft.com/en-us/windows/win32/direct3dhlsl/dx-graphics-hlsl-scalar
> > > 
> > > Change the behavior might affect a lot of existing shaders.
> > More detail about half from https://github.com/tex3d.
> > 
> > half originally mapped to a fuzzy "partial precision" float, where some 
> > operations were designated as _pp, meaning the implementation was free to 
> > use lower-precision math for those operations (like 24-bit, but not 
> > specified for the language). All storage in host-visible memory would still 
> > be float. "partial precision" went away with DX9, eventually to be replaced 
> > with min-precision with a specific minimum precision an implementation was 
> > allowed to use. When "partial precision" went away, it simply mapped to 
> > float for DX10+.
> > People could have tried to use it liberally when they thought 32-bit 
> > precision wasn't necessary, without explicitly targeting/testing 
> > API/hardware that actually supported lower precision.
> Thank you for the extra information, it sounds like we're stuck supporting 
> this.
Because HLSL’s library linking mode is pretty constrained, in practice this 
hasn’t hurt us yet. That said, I think we probably should consider some 
retroactive changes to how we handle float-16, especially in the presence of 
library shaders…

I’ll try and float this topic in some team meetings this week and see if we can 
come up with a set of tweaks that may have limited impact on existing code. We 
might be able to change the default behavior in clang with a switch to toggle 
back to the old mode for legacy code.



Comment at: clang/lib/Driver/ToolChains/HLSL.cpp:177
+  A->claim();
+  continue;
+}

`-fcgl` also should imply `-disable-llvm-options`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124790

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

[PATCH] D120290: [Clang][OpenMP] Add the codegen support for `atomic compare capture`

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/lib/Sema/SemaOpenMP.cpp:11663
   X = BO->getLHS();
-  D = BO->getRHS();
+  D = BO->getRHS()->IgnoreImpCasts();
 

tianshilei1992 wrote:
> tianshilei1992 wrote:
> > ABataev wrote:
> > > Why do we need to use `IgnoreImpCasts()` here and in other places?
> > Clang usually inserts implicit casts. For example, if we have:
> > ```
> > char a, b, c;
> > #pragma omp atomic compare capture
> >   { r = a; if (a > c) { a = b; } }
> > ```
> > Clang inserts an implicit cast from `char` to `int` for all statements 
> > except `r = a`. In this case, what should be the right solution? I'm not 
> > quite sure actually.
> https://godbolt.org/z/a6WWdx581
> This shows how the AST looks like.
I don't see casts to int in `a=b` (BO points to this expression,right?). The 
only cast, that maybe should be removed here, is LValueToRValue


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120290

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

[PATCH] D124909: Fix issues with using clangd with C++ modules

[Kugan Vivekanandarajah via Phabricator via cfe-commits]

kuganv added a comment.

Thanks a lot for the detailed. comment. Yes, you are right in that we are using 
build system (buck) to build the modules and provide as part of the CDB. This 
is fairly cheap when we hit the buck cache that is also shared by others.

You are right in that the PCM format is not stable and clangd can only consume 
modules if the build system uses the exact same version of the clang. We do 
have checks in ASTReader that validates this. I don’t see anyway we can relax 
this unless we formalise AST and guarantee full compatibility. For our use, we 
expect to use the exact same version of clang as clangd for building the PCM.

As for different modes, unless I am missing something, we should only care 
about modes that we explicitly set in clangd. We should expect the modules in 
CDB should match the options provided by CDB. We could  relax some these if 
they difference does not matter.

My main goat of this patch is to enable middles so that we can test is more 
widely.  I understand that we would find more issues that may need fixing,


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124909

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

[PATCH] D124932: [clang][dataflow] Track `optional`s' values (i.e. contents) in `optional` model.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: xazax.hun, sgatev.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This patch adds partial support for tracking (i.e. modeling) the contents of an
optional value. Specifically, it supports tracking the value after it is
accessed. We leave tracking constructed/assigned contents to a future patch.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124932

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2150,8 +2150,195 @@
   UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInOptional) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using Foo = $ns::$optional;
+
+void target($ns::$optional foo) {
+  if (foo && *foo) {
+foo->value();
+/*[[access]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("access", "safe")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInOptionalStruct) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target(const $ns::$optional& foo) {
+  if (foo && foo->opt) {
+foo->opt.value();
+/*[[access]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("access", "safe")));
+
+  // `reset` changes the state.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (foo && foo->opt) {
+foo->opt.reset();
+foo->opt.value();
+/*[[reset]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("reset", "unsafe: input.cc:11:9")));
+
+  // `has_value` and `operator*`.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (foo && foo->opt.has_value()) {
+*foo->opt;
+/*[[other-ops]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("other-ops", "safe")));
+
+  // `operator->`.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (foo && foo->opt.has_value()) {
+foo->opt->empty();
+/*[[arrow]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("arrow", "safe")));
+
+  // Accessing the nested optional in the wrong branch.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (!foo.has_value()) return;
+  if (!foo->opt.has_value()) {
+foo->opt.value();
+/*[[not-set]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("not-set", "unsafe: input.cc:11:9")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInOptionalStructField) {
+  // Non-standard assignment.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+struct Bar {
+  $ns::$optional member;
+};
+
+Bar createBar();
+
+void target() {
+  Bar bar = createBar();
+  bar.member->opt = "a";
+  /*[[ns-assign]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("ns-assign", "unsafe: input.cc:16:7")));
+
+  // Resetting the nested optional.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Member {
+  $ns::$optional opt;
+};
+
+struct Foo {
+  $ns::$optional member;
+};
+
+Foo createFoo();
+
+void target() {
+  Foo foo = createFoo();
+  foo.member->opt.reset();
+  /*[[nested-reset]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("nested-reset", "unsafe: input.cc:16:7")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInitialization) {
+  // FIXME: Fix when to initialize `value`. All unwrapping should be safe in
+  // this example, but `value` initialization is done multiple times during the
+  // fixpoint iterations and joining the environment won't correctly merge them.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_

[clang] 726d7b0 - [Sema] Simplify CheckConstraintSatisfaction. NFC

[Ilya Biryukov via cfe-commits]

Author: Ilya Biryukov
Date: 2022-05-04T15:53:07Z
New Revision: 726d7b07fcde58c61743c45723c9b614911fe084

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

LOG: [Sema] Simplify CheckConstraintSatisfaction. NFC

- Exit early when constraint caching is disabled.
- Use unique_ptr to manage temporary lifetime.
- Fix a typo in a comment (InsertPos instead of InsertNode).

The new code duplicates the forwarding call to CheckConstraintSatisfaction,
but reduces the number of interconnected if statements and simplifies lifetime
management.

This increases the overall readability.

Reviewed By: sammccall

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

Added: 


Modified: 
clang/lib/Sema/SemaConcept.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaConcept.cpp b/clang/lib/Sema/SemaConcept.cpp
index 449f9eb33f47b..4ec37a94089b0 100644
--- a/clang/lib/Sema/SemaConcept.cpp
+++ b/clang/lib/Sema/SemaConcept.cpp
@@ -318,35 +318,31 @@ bool Sema::CheckConstraintSatisfaction(
 return false;
   }
 
+  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
+  if (!ShouldCache) {
+return ::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
+ TemplateArgs, TemplateIDRange,
+ OutSatisfaction);
+  }
   llvm::FoldingSetNodeID ID;
+  ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
   void *InsertPos;
-  ConstraintSatisfaction *Satisfaction = nullptr;
-  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
-  if (ShouldCache) {
-ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
-Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos);
-if (Satisfaction) {
-  OutSatisfaction = *Satisfaction;
-  return false;
-}
-Satisfaction = new ConstraintSatisfaction(Template, TemplateArgs);
-  } else {
-Satisfaction = &OutSatisfaction;
+  if (auto *Cached = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos)) {
+OutSatisfaction = *Cached;
+return false;
   }
+  auto Satisfaction =
+  std::make_unique(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
 TemplateArgs, TemplateIDRange,
 *Satisfaction)) {
-if (ShouldCache)
-  delete Satisfaction;
 return true;
   }
-
-  if (ShouldCache) {
-// We cannot use InsertNode here because CheckConstraintSatisfaction might
-// have invalidated it.
-SatisfactionCache.InsertNode(Satisfaction);
-OutSatisfaction = *Satisfaction;
-  }
+  OutSatisfaction = *Satisfaction;
+  // We cannot use InsertPos here because CheckConstraintSatisfaction might 
have
+  // invalidated it.
+  // FIXME: this leaks memory, we should allocate in the arena instead.
+  SatisfactionCache.InsertNode(Satisfaction.release());
   return false;
 }
 



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

[PATCH] D124923: [Sema] Simplify CheckConstraintSatisfaction. NFC

[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.
Closed by commit rG726d7b07fcde: [Sema] Simplify CheckConstraintSatisfaction. 
NFC (authored by ilya-biryukov).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124923

Files:
  clang/lib/Sema/SemaConcept.cpp


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -318,35 +318,31 @@
 return false;
   }
 
+  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
+  if (!ShouldCache) {
+return ::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
+ TemplateArgs, TemplateIDRange,
+ OutSatisfaction);
+  }
   llvm::FoldingSetNodeID ID;
+  ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
   void *InsertPos;
-  ConstraintSatisfaction *Satisfaction = nullptr;
-  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
-  if (ShouldCache) {
-ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
-Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos);
-if (Satisfaction) {
-  OutSatisfaction = *Satisfaction;
-  return false;
-}
-Satisfaction = new ConstraintSatisfaction(Template, TemplateArgs);
-  } else {
-Satisfaction = &OutSatisfaction;
+  if (auto *Cached = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos)) {
+OutSatisfaction = *Cached;
+return false;
   }
+  auto Satisfaction =
+  std::make_unique(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
 TemplateArgs, TemplateIDRange,
 *Satisfaction)) {
-if (ShouldCache)
-  delete Satisfaction;
 return true;
   }
-
-  if (ShouldCache) {
-// We cannot use InsertNode here because CheckConstraintSatisfaction might
-// have invalidated it.
-SatisfactionCache.InsertNode(Satisfaction);
-OutSatisfaction = *Satisfaction;
-  }
+  OutSatisfaction = *Satisfaction;
+  // We cannot use InsertPos here because CheckConstraintSatisfaction might 
have
+  // invalidated it.
+  // FIXME: this leaks memory, we should allocate in the arena instead.
+  SatisfactionCache.InsertNode(Satisfaction.release());
   return false;
 }
 


Index: clang/lib/Sema/SemaConcept.cpp
===
--- clang/lib/Sema/SemaConcept.cpp
+++ clang/lib/Sema/SemaConcept.cpp
@@ -318,35 +318,31 @@
 return false;
   }
 
+  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
+  if (!ShouldCache) {
+return ::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
+ TemplateArgs, TemplateIDRange,
+ OutSatisfaction);
+  }
   llvm::FoldingSetNodeID ID;
+  ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
   void *InsertPos;
-  ConstraintSatisfaction *Satisfaction = nullptr;
-  bool ShouldCache = LangOpts.ConceptSatisfactionCaching && Template;
-  if (ShouldCache) {
-ConstraintSatisfaction::Profile(ID, Context, Template, TemplateArgs);
-Satisfaction = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos);
-if (Satisfaction) {
-  OutSatisfaction = *Satisfaction;
-  return false;
-}
-Satisfaction = new ConstraintSatisfaction(Template, TemplateArgs);
-  } else {
-Satisfaction = &OutSatisfaction;
+  if (auto *Cached = SatisfactionCache.FindNodeOrInsertPos(ID, InsertPos)) {
+OutSatisfaction = *Cached;
+return false;
   }
+  auto Satisfaction =
+  std::make_unique(Template, TemplateArgs);
   if (::CheckConstraintSatisfaction(*this, Template, ConstraintExprs,
 TemplateArgs, TemplateIDRange,
 *Satisfaction)) {
-if (ShouldCache)
-  delete Satisfaction;
 return true;
   }
-
-  if (ShouldCache) {
-// We cannot use InsertNode here because CheckConstraintSatisfaction might
-// have invalidated it.
-SatisfactionCache.InsertNode(Satisfaction);
-OutSatisfaction = *Satisfaction;
-  }
+  OutSatisfaction = *Satisfaction;
+  // We cannot use InsertPos here because CheckConstraintSatisfaction might have
+  // invalidated it.
+  // FIXME: this leaks memory, we should allocate in the arena instead.
+  SatisfactionCache.InsertNode(Satisfaction.release());
   return false;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

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

[Gabor Marton via Phabricator via cfe-commits]

martong accepted this revision.
martong added a comment.
Herald added a subscriber: rnkovacs.

LGTM


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124708

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

[PATCH] D124932: [clang][dataflow] Track `optional` contents in `optional` model.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 427029.
ymandel added a comment.

remove stray newline


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124932

Files:
  clang/lib/Analysis/FlowSensitive/Models/UncheckedOptionalAccessModel.cpp
  clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/UncheckedOptionalAccessModelTest.cpp
@@ -2150,8 +2150,195 @@
   UnorderedElementsAre(Pair("check-1", "safe"), Pair("check-2", "safe")));
 }
 
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInOptional) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using Foo = $ns::$optional;
+
+void target($ns::$optional foo) {
+  if (foo && *foo) {
+foo->value();
+/*[[access]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("access", "safe")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInOptionalStruct) {
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target(const $ns::$optional& foo) {
+  if (foo && foo->opt) {
+foo->opt.value();
+/*[[access]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("access", "safe")));
+
+  // `reset` changes the state.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (foo && foo->opt) {
+foo->opt.reset();
+foo->opt.value();
+/*[[reset]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("reset", "unsafe: input.cc:11:9")));
+
+  // `has_value` and `operator*`.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (foo && foo->opt.has_value()) {
+*foo->opt;
+/*[[other-ops]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("other-ops", "safe")));
+
+  // `operator->`.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (foo && foo->opt.has_value()) {
+foo->opt->empty();
+/*[[arrow]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("arrow", "safe")));
+
+  // Accessing the nested optional in the wrong branch.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+void target($ns::$optional& foo) {
+  if (!foo.has_value()) return;
+  if (!foo->opt.has_value()) {
+foo->opt.value();
+/*[[not-set]]*/
+  }
+}
+  )",
+  UnorderedElementsAre(Pair("not-set", "unsafe: input.cc:11:9")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInOptionalStructField) {
+  // Non-standard assignment.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Foo {
+  $ns::$optional opt;
+};
+
+struct Bar {
+  $ns::$optional member;
+};
+
+Bar createBar();
+
+void target() {
+  Bar bar = createBar();
+  bar.member->opt = "a";
+  /*[[ns-assign]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("ns-assign", "unsafe: input.cc:16:7")));
+
+  // Resetting the nested optional.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+struct Member {
+  $ns::$optional opt;
+};
+
+struct Foo {
+  $ns::$optional member;
+};
+
+Foo createFoo();
+
+void target() {
+  Foo foo = createFoo();
+  foo.member->opt.reset();
+  /*[[nested-reset]]*/
+}
+  )",
+  UnorderedElementsAre(Pair("nested-reset", "unsafe: input.cc:16:7")));
+}
+
+TEST_P(UncheckedOptionalAccessTest, NestedOptionalInitialization) {
+  // FIXME: Fix when to initialize `value`. All unwrapping should be safe in
+  // this example, but `value` initialization is done multiple times during the
+  // fixpoint iterations and joining the environment won't correctly merge them.
+  ExpectLatticeChecksFor(
+  R"(
+#include "unchecked_optional_access_test.h"
+
+using Foo = $ns::$optional;
+
+void target($ns::$optional foo, bool b) {
+  if (!foo.has_value()) return;
+  if (b) {
+if (!foo->has_value()) return;
+// We have created `foo.value()`.
+foo->value();
+  } else {
+if (!foo->has_value()) return;
+// We have created `fo

[PATCH] D124667: [flang][driver] Add support for consuming LLVM IR/BC files

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 427032.
awarzynski added a comment.

Rebase on top of main


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124667

Files:
  clang/include/clang/Driver/Types.h
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/Types.cpp
  flang/lib/Frontend/CMakeLists.txt
  flang/lib/Frontend/FrontendActions.cpp
  flang/lib/Frontend/FrontendOptions.cpp
  flang/test/Driver/emit-asm-from-llvm-bc.ll
  flang/test/Driver/emit-asm-from-llvm.ll
  flang/test/Driver/missing-triple.ll
  flang/test/Driver/override-triple.ll
  flang/test/lit.cfg.py

Index: flang/test/lit.cfg.py
===
--- flang/test/lit.cfg.py
+++ flang/test/lit.cfg.py
@@ -27,7 +27,8 @@
 # suffixes: A list of file extensions to treat as test files.
 config.suffixes = ['.c', '.cpp', '.f', '.F', '.ff', '.FOR', '.for', '.f77', '.f90', '.F90',
'.ff90', '.f95', '.F95', '.ff95', '.fpp', '.FPP', '.cuf'
-   '.CUF', '.f18', '.F18', '.fir', '.f03', '.F03', '.f08', '.F08']
+   '.CUF', '.f18', '.F18', '.fir', '.f03', '.F03', '.f08',
+   '.F08', '.ll']
 
 config.substitutions.append(('%PATH%', config.environment['PATH']))
 config.substitutions.append(('%llvmshlibdir', config.llvm_shlib_dir))
Index: flang/test/Driver/override-triple.ll
===
--- /dev/null
+++ flang/test/Driver/override-triple.ll
@@ -0,0 +1,25 @@
+; Verify that the module triple is overridden by the driver - even in the presence
+; of a module triple.
+; NOTE: At the time of writing, the tested behaviour was consistent with Clang
+
+;-
+; RUN COMMANDS
+;-
+; RUN: %flang_fc1 -S %s -o - 2>&1 | FileCheck %s
+; RUN: %flang -S  %s -o - 2>&1 | FileCheck %s
+
+;
+; EXPECTED OUTPUT
+;
+; CHECK: warning: overriding the module target triple with {{.*}}
+
+;--
+; INPUT
+;--
+; For the triple to be overridden by the driver, it needs to be different to the host triple.
+; Use a random string to guarantee that.
+target triple = "invalid-triple"
+
+define void @foo() {
+  ret void
+}
Index: flang/test/Driver/missing-triple.ll
===
--- /dev/null
+++ flang/test/Driver/missing-triple.ll
@@ -0,0 +1,21 @@
+; Verify that the module triple is overridden by the driver - even when the
+; module triple is missing.
+; NOTE: At the time of writing, the tested behaviour was consistent with Clang
+
+;-
+; RUN COMMANDS
+;-
+; RUN: %flang_fc1 -S %s -o - 2>&1 | FileCheck %s
+; RUN: %flang -S  %s -o - 2>&1 | FileCheck %s
+
+;
+; EXPECTED OUTPUT
+;
+; CHECK: warning: overriding the module target triple with {{.*}}
+
+;--
+; INPUT
+;--
+define void @foo() {
+  ret void
+}
Index: flang/test/Driver/emit-asm-from-llvm.ll
===
--- /dev/null
+++ flang/test/Driver/emit-asm-from-llvm.ll
@@ -0,0 +1,23 @@
+; Verify that the driver can consume LLVM IR files. The expected assembly is
+; fairly generic (verified on AArch64 and X86_64), but we may need to tweak when
+; testing on other platforms. Note that the actual output doesn't matter
+; as long as it's in Assembly format.
+
+;-
+; RUN COMMANDS
+;-
+; RUN: %flang_fc1 -S %s -o - | FileCheck %s
+; RUN: %flang -S  %s -o - | FileCheck %s
+
+;
+; EXPECTED OUTPUT
+;
+; CHECK-LABEL: foo:
+; CHECK: ret
+
+;--
+; INPUT
+;--
+define void @foo() {
+  ret void
+}
Index: flang/test/Driver/emit-asm-from-llvm-bc.ll
===
--- /dev/null
+++ flang/test/Driver/emit-asm-from-llvm-bc.ll
@@ -0,0 +1,30 @@
+; Verify that the driver can consume LLVM BC files. The expected assembly is
+; fairly generic (tested on AArch64 and X86_64), but we may need to tweak when
+; testing on other platforms. Note that the actual output doesn't matter as
+; long as it's in Assembly format.
+
+;-
+; RUN COMMANDS
+;-
+; RUN: rm -f %t.bc
+; RUN: %flang_fc1 -emit-llvm-bc %s -o %t.bc
+; RUN: %flang_fc1 -S -o - %t.bc | FileCheck %s
+; RUN: rm -f %t.bc
+
+; RUN: rm -f %t.bc
+; RUN: %flang -c -emit-llvm %s -o %t.bc
+; RUN: %flang -S -o - %t.bc | FileCheck %s
+; RUN: rm -f %t.bc
+
+;
+; EXPECTED OUTPUT
+;
+; CHECK-LABEL: foo:
+; CHECK: ret
+
+;--
+; INPUT
+;--
+define void @foo() {
+  ret void
+}
Index: flang/lib/Frontend/FrontendOptions.cpp
===
--- flang/lib/Frontend/FrontendOptions.cpp
+++ flang/lib/Frontend/FrontendOptions.cpp
@@ -35,5 +35,9 @@
   if (isFixedFormSuffix(extension) || isFreeFormSuffix(extension)) {
 return Language::Fortran;
   }

[PATCH] D124874: [clang] add -fmodule-file-home-is-cwd

[Richard Howell via Phabricator via cfe-commits]

rmaz updated this revision to Diff 427034.
rmaz added a comment.

Use regex for path separators in test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124874

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Lex/HeaderSearchOptions.h
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/module-file-home-is-cwd.m


Index: clang/test/Modules/module-file-home-is-cwd.m
===
--- /dev/null
+++ clang/test/Modules/module-file-home-is-cwd.m
@@ -0,0 +1,8 @@
+// RUN: cd %S
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-file-home-is-cwd 
-fmodule-name=libA -emit-module Inputs/normal-module-map/module.map -o 
%t/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 
'Inputs{{/|}}normal-module-map{{/|}}a1.h'
+// CHECK:  blob data = 
'Inputs{{/|}}normal-module-map{{/|}}a2.h'
+// CHECK:  blob data = 
'Inputs{{/|}}normal-module-map{{/|}}module.map'
+// CHECK-NOT: MODULE_DIRECTORY
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -1223,7 +1223,15 @@
 Stream.EmitRecordWithBlob(AbbrevCode, Record, WritingModule->Name);
   }
 
-  if (WritingModule && WritingModule->Directory) {
+  if (WritingModule &&
+  PP.getHeaderSearchInfo().getHeaderSearchOpts().ModuleFileHomeIsCwd) {
+// Use the current working directory as the base path for all inputs.
+auto *CWD =
+Context.getSourceManager().getFileManager().getDirectory(".").get();
+SmallString<128> BaseDir(CWD->getName());
+cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
+BaseDirectory.assign(BaseDir.begin(), BaseDir.end());
+  } else if (WritingModule && WritingModule->Directory) {
 SmallString<128> BaseDir(WritingModule->Directory->getName());
 cleanPathForOutput(Context.getSourceManager().getFileManager(), BaseDir);
 
Index: clang/include/clang/Lex/HeaderSearchOptions.h
===
--- clang/include/clang/Lex/HeaderSearchOptions.h
+++ clang/include/clang/Lex/HeaderSearchOptions.h
@@ -143,6 +143,12 @@
   /// file.
   unsigned ModuleMapFileHomeIsCwd : 1;
 
+  /// Set the base path of a built module file to be the current working
+  /// directory. This is useful for sharing module files across machines
+  /// that build with different paths without having to rewrite all
+  /// modulemap files to have working directory relative paths.
+  unsigned ModuleFileHomeIsCwd : 1;
+
   /// Also search for prebuilt implicit modules in the prebuilt module cache
   /// path.
   unsigned EnablePrebuiltImplicitModules : 1;
@@ -222,9 +228,9 @@
   HeaderSearchOptions(StringRef _Sysroot = "/")
   : Sysroot(_Sysroot), ModuleFormat("raw"), DisableModuleHash(false),
 ImplicitModuleMaps(false), ModuleMapFileHomeIsCwd(false),
-EnablePrebuiltImplicitModules(false), UseBuiltinIncludes(true),
-UseStandardSystemIncludes(true), UseStandardCXXIncludes(true),
-UseLibcxx(false), Verbose(false),
+ModuleFileHomeIsCwd(false), EnablePrebuiltImplicitModules(false),
+UseBuiltinIncludes(true), UseStandardSystemIncludes(true),
+UseStandardCXXIncludes(true), UseLibcxx(false), Verbose(false),
 ModulesValidateOncePerBuildSession(false),
 ModulesValidateSystemHeaders(false),
 ValidateASTInputFilesContent(false), UseDebugInfo(false),
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -5609,6 +5609,10 @@
   HelpText<"Use the current working directory as the home directory of "
"module maps specified by -fmodule-map-file=">,
   MarshallingInfoFlag>;
+def fmodule_file_home_is_cwd : Flag<["-"], "fmodule-file-home-is-cwd">,
+  HelpText<"Use the current working directory as the base directory of "
+   "compiled module files.">,
+  MarshallingInfoFlag>;
 def fmodule_feature : Separate<["-"], "fmodule-feature">,
   MetaVarName<"">,
   HelpText<"Enable  in module map requires declarations">,


Index: clang/test/Modules/module-file-home-is-cwd.m
===
--- /dev/null
+++ clang/test/Modules/module-file-home-is-cwd.m
@@ -0,0 +1,8 @@
+// RUN: cd %S
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-file-home-is-cwd -fmodule-name=libA -emit-module Inputs/normal-module-map/module.map -o %t/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'Inputs{{/|}}normal-module-map{{/|}}a1.h'
+// CHECK:  blob data = 'Inputs{{/|

[PATCH] D124669: [flang][driver] Add support for -save-temps

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski updated this revision to Diff 427036.
awarzynski added a comment.

Fix typos as per comments from @rovka, thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124669

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Driver/Types.def
  flang/test/Driver/driver-help-hidden.f90
  flang/test/Driver/driver-help.f90
  flang/test/Driver/fno-integrated-as.f90
  flang/test/Driver/save-temps.f90

Index: flang/test/Driver/save-temps.f90
===
--- /dev/null
+++ flang/test/Driver/save-temps.f90
@@ -0,0 +1,53 @@
+! Tests for the `-save-temps` flag. As `flang` does not implement `-fc1as` (i.e. a driver for the integrated assembler), we need to
+! use `-fno-integrated-as` here.
+
+!--
+! Basic case: `-save-temps`
+!--
+! RUN: %flang -save-temps -fno-integrated-as %s -### 2>&1 | FileCheck %s
+! CHECK: "-o" "save-temps.i"
+! CHECK-NEXT: "-o" "save-temps.bc"
+! CHECK-NEXT: "-o" "save-temps.s"
+! CHECK-NEXT: "-o" "save-temps.o"
+! CHECK-NEXT: "-o" "a.out"
+
+!--
+! `-save-temps=cwd`
+!--
+! This should work the same as -save-temps above
+
+! RUN: %flang -save-temps=cwd -fno-integrated-as  %s -### 2>&1 | FileCheck %s -check-prefix=CWD
+! CWD: "-o" "save-temps.i"
+! CWD-NEXT: "-o" "save-temps.bc"
+! CWD-NEXT: "-o" "save-temps.s"
+! CWD-NEXT: "-o" "save-temps.o"
+! CWD-NEXT: "-o" "a.out"
+
+!--
+! `-save-temps=obj`
+!--
+! Check that temp files are saved in the same directory as the output file
+! regardless of whether -o is specified.
+
+! RUN: %flang -save-temps=obj -fno-integrated-as -o obj/dir/a.out %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-OBJ
+! CHECK-OBJ: "-o" "obj/dir/save-temps.i"
+! CHECK-OBJ-NEXT: "-o" "obj/dir/save-temps.bc"
+! CHECK-OBJ-NEXT: "-o" "obj/dir/save-temps.s"
+! CHECK-OBJ-NEXT: "-o" "obj/dir/save-temps.o"
+! CHECK-OBJ-NEXT: "-o" "obj/dir/a.out"
+
+! RUN: %flang -save-temps=obj -fno-integrated-as %s -### 2>&1 | FileCheck %s -check-prefix=CHECK-OBJ-NOO
+! CHECK-OBJ-NOO: "-o" "save-temps.i"
+! CHECK-OBJ-NOO-NEXT: "-o" "save-temps.bc"
+! CHECK-OBJ-NOO-NEXT: "-o" "save-temps.s"
+! CHECK-OBJ-NOO-NEXT: "-o" "save-temps.o"
+! CHECK-OBJ-NOO-NEXT: "-o" "a.out"
+
+!--
+! `-S` without `-save-temps`
+!--
+! Check for a single `flang -fc1` invocation when NOT using -save-temps.
+! RUN: %flang -S %s -### 2>&1 | FileCheck %s -check-prefix=NO-TEMPS
+! NO-TEMPS: "-fc1"
+! NO-TEMPS-SAME: "-S"
+! NO-TEMPS-SAME: "-o" "save-temps.s"
Index: flang/test/Driver/fno-integrated-as.f90
===
--- /dev/null
+++ flang/test/Driver/fno-integrated-as.f90
@@ -0,0 +1,18 @@
+! Tests for the `-fno-integrated-as` flag.
+
+!--
+! With `-fno-integrated-as`
+!--
+! Verify that there _is_ a separate line with an assembler invocation
+! RUN: %flang -c -fno-integrated-as %s -### 2>&1 | FileCheck %s
+! CHECK-LABEL: "-fc1"
+! CHECK-SAME: "-o" "[[assembly_file:.*]].s"
+! CHECK-NEXT: "-o" "{{.*}}.o" "[[assembly_file:.*]].s"
+
+!-
+! Without `-fno-integrated-as`
+!-
+! Verify that there _is no_ separate line with an assembler invocation
+! RUN: %flang -c %s -### 2>&1 | FileCheck %s -check-prefix=DEFAULT
+! DEFAULT-LABEL: "-fc1"
+! DEFAULT-SAME: "-o" "{{.*}}.o" "{{.*}}fno-integrated-as.f90"
Index: flang/test/Driver/driver-help.f90
===
--- flang/test/Driver/driver-help.f90
+++ flang/test/Driver/driver-help.f90
@@ -42,6 +42,7 @@
 ! HELP-NEXT: -flogical-abbreviations Enable logical abbreviations
 ! HELP-NEXT: -fno-automatic Implies the SAVE attribute for non-automatic local objects in subprograms unless RECURSIVE
 ! HELP-NEXT: -fno-color-diagnostics  Disable colors in diagnostics
+! HELP-NEXT: -fno-integrated-as  Disable the integrated assembler
 ! HELP-NEXT: -fopenacc  Enable OpenACC
 ! HELP-NEXT: -fopenmp   Parse OpenMP pragmas and generate parallel code.
 ! HELP-NEXT: -fxor-operator Enable .XOR. as a synonym of .NEQV.
@@ -56,6 +57,8 @@
 ! HELP-NEXT: -print-effective-triple Print the effective target triple
 ! HELP-NEXT: -print-target-triplePrint the normalized target triple
 ! HELP-NEXT: -P Disable linemarker output in -E mode
+! HELP-NEXT: -save-temps=Save intermediate compilation results.
+! HELP-NEXT: -save-tempsSave intermediate compilation results
 ! HELP-NEXT: -std=   Language standard to compile for
 ! HELP-NEXT: -S Only run preprocess and compilation steps
 ! HELP-NEXT: --target=   Generate code for the given target
Index: flang/test/Dr

[PATCH] D124669: [flang][driver] Add support for -save-temps

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski marked 4 inline comments as done.
awarzynski added inline comments.



Comment at: flang/test/Driver/fno-integrated-as.f90:18
+! DEFAULT-LABEL: "-fc1"
+! DEFAULT-SAME: "-o" "fno-integrated-as.o" "{{.*}}fno-integrated-as.f90"

rovka wrote:
> Nit (here and above): Is it a rule that `-o blah` comes right before the 
> input file, or should we also add a {{.*}} in between in case other flags 
> sneak in?
> Nit (here and above): Is it a rule that -o blah comes right before the input 
> file, or should we also add a {{.*}} in between in case other flags sneak in?

TBH, it's not something that I checked, but it's been the case so far :) We may 
need to insert `{{.*}}` there at some point, but perhaps lets delay until it's 
really needed? This way we will know exactly what violates that rule and 
perhaps we could tweak then?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124669

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

[PATCH] D124806: [clang-tidy] add support for Demorgan conversions to readability-simplify-bool-expr

[Nathan James via Phabricator via cfe-commits]

njames93 updated this revision to Diff 427038.
njames93 added a comment.

Enhanced support for parens, both adding them in when needed as well as 
removing some un-needed parens.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124806

Files:
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.h
  clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
  
clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp

Index: clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/readability-simplify-bool-expr-demorgan.cpp
@@ -0,0 +1,64 @@
+// RUN: %check_clang_tidy %s readability-simplify-boolean-expr %t
+
+void foo(bool A1, bool A2, bool A3) {
+  bool X;
+  X = !(!A1 || A2);
+  X = !(A1 || !A2);
+  X = !(!A1 || !A2);
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 && !A2);
+  // CHECK-FIXES-NEXT: X = (!A1 && A2);
+  // CHECK-FIXES-NEXT: X = (A1 && A2);
+
+  X = !(!A1 && A2);
+  X = !(A1 && !A2);
+  X = !(!A1 && !A2);
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 || !A2);
+  // CHECK-FIXES-NEXT: X = (!A1 || A2);
+  // CHECK-FIXES-NEXT: X = (A1 || A2);
+
+  X = !(!A1 && !A2 && !A3);
+  X = !(!A1 && (!A2 && !A3));
+  X = !(!A1 && (A2 && A3));
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-3]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 || A2 || A3);
+  // CHECK-FIXES-NEXT: X = (A1 || A2 || A3);
+  // CHECK-FIXES-NEXT: X = (A1 || !A2 || !A3);
+
+  X = !(A1 && A2 == A3);
+  X = !(!A1 && A2 > A3);
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-2]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (!A1 || A2 != A3);
+  // CHECK-FIXES-NEXT: X = (A1 || A2 <= A3);
+
+  // Ensure the check doesn't try to combine fixes for the inner and outer demorgan simplification.
+  X = !(!A1 && !(!A2 && !A3));
+  // CHECK-MESSAGES: :[[@LINE-1]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-2]]:16: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (A1 || (!A2 && !A3));
+
+  // Testing to see how it handles parens
+  X = !(A1 && !A2 && !A3);
+  X = !(A1 && !A2 || !A3);
+  X = !(!A1 || A2 && !A3);
+  X = !((A1 || !A2) && !A3);
+  X = !((A1 || !A2) || !A3);
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-MESSAGES: :[[@LINE-5]]:7: warning: boolean expression can be simplified by DeMorgan's theorem
+  // CHECK-FIXES: X = (!A1 || A2 || A3);
+  // CHECK-FIXES-NEXT: X = ((!A1 || A2) && A3);
+  // CHECK-FIXES-NEXT: X = (A1 && (!A2 || A3));
+  // CHECK-FIXES-NEXT: X = (!A1 && A2 || A3);
+  // CHECK-FIXES-NEXT: X = (!A1 && A2 && A3);
+}
Index: clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
+++ clang-tools-extra/docs/clang-tidy/checks/readability-simplify-boolean-expr.rst
@@ -4,7 +4,8 @@
 =
 
 Looks for boolean expressions involving boolean constants and simplifies
-them to use the appropriate boolean expression directly.
+them to use the appropriate boolean expression directly.  Simplifies
+boolean expressions by application of DeMorgan's Theorem.
 
 Examples:
 
@@ -27,6 +28,12 @@
 ``if (e) b = false; else b = true;``  

[PATCH] D124658: [analyzer] Canonicalize SymIntExpr so the RHS is positive when possible

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

steakhal added a comment.

Just a few nits left.
Consider marking 'done' the corresponding boxes.




Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:211
+// Check if the negation of the RHS is representable:
+// * if resultTy is unsigned, then negation is always representable
+// * if resultTy is signed, and RHS is not the lowest representable

It feels odd that the comments refer to `resultTy`, but you then use 
`resultIntTy` instead, and both of these entities are alive at this scope.



Comment at: clang/lib/StaticAnalyzer/Core/SimpleSValBuilder.cpp:215
+if (resultIntTy.isUnsigned() ||
+(ConvertedRHSValue > resultIntTy.getMinValue())) {
+  ConvertedRHS = &BasicVals.getValue(-ConvertedRHSValue);

These should be the same.


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

https://reviews.llvm.org/D124658

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

[PATCH] D124702: [MSVC] Add support for pragma function

[Stephen Long via Phabricator via cfe-commits]

steplong updated this revision to Diff 427040.
steplong added a comment.

Changed std::vector to llvm::SmallVector


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

Files:
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/Sema.h
  clang/lib/Parse/ParsePragma.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/lib/Sema/SemaDecl.cpp

Index: clang/lib/Sema/SemaDecl.cpp
===
--- clang/lib/Sema/SemaDecl.cpp
+++ clang/lib/Sema/SemaDecl.cpp
@@ -10218,10 +10218,12 @@
   // marking the function.
   AddCFAuditedAttribute(NewFD);
 
-  // If this is a function definition, check if we have to apply optnone due to
-  // a pragma.
-  if(D.isFunctionDefinition())
+  // If this is a function definition, check if we have to apply any
+  // attributes (i.e. optnone and no_builtin) due to a pragma.
+  if(D.isFunctionDefinition()) {
 AddRangeBasedOptnone(NewFD);
+AddRangeBasedNoBuiltin(NewFD);
+  }
 
   // If this is the first declaration of an extern C variable, update
   // the map of such variables.
Index: clang/lib/Sema/SemaAttr.cpp
===
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -1065,6 +1065,20 @@
 OptimizeOffPragmaLocation = PragmaLoc;
 }
 
+void Sema::ActOnPragmaMSIntrinsic(
+const SmallVectorImpl &Intrinsics) {
+  for (auto &Intrinsic : Intrinsics)
+MSFunctionNoBuiltins.erase(std::remove(MSFunctionNoBuiltins.begin(),
+   MSFunctionNoBuiltins.end(),
+   Intrinsic),
+   MSFunctionNoBuiltins.end());
+}
+
+void Sema::ActOnPragmaMSFunction(const SmallVectorImpl &NoBuiltins) {
+  MSFunctionNoBuiltins.insert(MSFunctionNoBuiltins.end(),
+  NoBuiltins.begin(), NoBuiltins.end());
+}
+
 void Sema::AddRangeBasedOptnone(FunctionDecl *FD) {
   // In the future, check other pragmas if they're implemented (e.g. pragma
   // optimize 0 will probably map to this functionality too).
@@ -1086,6 +1100,12 @@
 FD->addAttr(NoInlineAttr::CreateImplicit(Context, Loc));
 }
 
+void Sema::AddRangeBasedNoBuiltin(FunctionDecl *FD) {
+  if (!MSFunctionNoBuiltins.empty())
+FD->addAttr(NoBuiltinAttr::CreateImplicit(Context,
+MSFunctionNoBuiltins.data(), MSFunctionNoBuiltins.size()));
+}
+
 typedef std::vector > VisStack;
 enum : unsigned { NoVisibility = ~0U };
 
Index: clang/lib/Parse/ParsePragma.cpp
===
--- clang/lib/Parse/ParsePragma.cpp
+++ clang/lib/Parse/ParsePragma.cpp
@@ -250,9 +250,21 @@
 };
 
 struct PragmaMSIntrinsicHandler : public PragmaHandler {
-  PragmaMSIntrinsicHandler() : PragmaHandler("intrinsic") {}
+  PragmaMSIntrinsicHandler(Sema &S) : PragmaHandler("intrinsic"), Actions(S) {}
   void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
 Token &FirstToken) override;
+
+private:
+  Sema &Actions;
+};
+
+struct PragmaMSFunctionHandler : public PragmaHandler {
+  PragmaMSFunctionHandler(Sema &S) : PragmaHandler("function"), Actions(S) {}
+  void HandlePragma(Preprocessor &PP, PragmaIntroducer Introducer,
+Token &FirstToken) override;
+
+private:
+  Sema &Actions;
 };
 
 struct PragmaMSOptimizeHandler : public PragmaHandler {
@@ -447,8 +459,10 @@
 PP.AddPragmaHandler(MSSection.get());
 MSRuntimeChecks = std::make_unique();
 PP.AddPragmaHandler(MSRuntimeChecks.get());
-MSIntrinsic = std::make_unique();
+MSIntrinsic = std::make_unique(Actions);
 PP.AddPragmaHandler(MSIntrinsic.get());
+MSFunction = std::make_unique(Actions);
+PP.AddPragmaHandler(MSFunction.get());
 MSOptimize = std::make_unique();
 PP.AddPragmaHandler(MSOptimize.get());
 MSFenvAccess = std::make_unique();
@@ -558,6 +572,8 @@
 MSRuntimeChecks.reset();
 PP.RemovePragmaHandler(MSIntrinsic.get());
 MSIntrinsic.reset();
+PP.RemovePragmaHandler(MSFunction.get());
+MSFunction.reset();
 PP.RemovePragmaHandler(MSOptimize.get());
 MSOptimize.reset();
 PP.RemovePragmaHandler(MSFenvAccess.get());
@@ -3523,11 +3539,14 @@
 
   bool SuggestIntrinH = !PP.isMacroDefined("__INTRIN_H");
 
+  SmallVector Intrinsics;
   while (Tok.is(tok::identifier)) {
 IdentifierInfo *II = Tok.getIdentifierInfo();
 if (!II->getBuiltinID())
   PP.Diag(Tok.getLocation(), diag::warn_pragma_intrinsic_builtin)
   << II << SuggestIntrinH;
+else
+  Intrinsics.emplace_back(II->getName());
 
 PP.Lex(Tok);
 if (Tok.isNot(tok::comma))
@@ -3545,6 +3564,51 @@
   if (Tok.isNot(tok::eod))
 PP.Diag(Tok.getLocation(), diag::warn_pragma_extra_tokens_at_eol)
 << "intrinsic";
+
+  Actions.ActOnPragmaMSIntrinsic(Intrinsics);
+}
+
+void PragmaMSFunctionHandler::HandlePragm

[PATCH] D124702: [MSVC] Add support for pragma function

[Stephen Long via Phabricator via cfe-commits]

steplong added inline comments.



Comment at: clang/lib/Parse/ParsePragma.cpp:3561
 << "intrinsic";
 return;
   }

hans wrote:
> since the above is just a warning, we should probably still call the ActOn.. 
> method?
Hmm, I'm not sure because all the msgs above are also warnings (diag::warn_)



Comment at: clang/lib/Sema/SemaAttr.cpp:1079
+  MSFunctionNoBuiltins.insert(MSFunctionNoBuiltins.end(),
+  NoBuiltins.begin(), NoBuiltins.end());
+}

hans wrote:
> Do we want to avoid duplicates in MSFunctionNoBuiltins? Or maybe it doesn't 
> matter?
Yea, I didn't think it really mattered. I originally wanted to use a set, but I 
needed the strings to be stored in contiguous memory for 
NoBuitinAttr::CreateImplicit() in Sema::AddRangeBasedNoBuiltin()


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124702

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

[PATCH] D124916: [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/Headers/cetintrin.h:45
 
 static __inline__ unsigned int __DEFAULT_FN_ATTRS _rdsspd_i32() {
+#pragma clang diagnostic push

The argument should also be `(void)`.



Comment at: clang/lib/Headers/cetintrin.h:58
 
 static __inline__ unsigned long long __DEFAULT_FN_ATTRS _rdsspq_i64() {
+#pragma clang diagnostic push

Same here


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124916

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

[PATCH] D124916: [X86] Fix uninitialized variable warnings in cetintrin.h reported by #55224

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/lib/Headers/cetintrin.h:48
+#pragma clang diagnostic ignored "-Wuninitialized"
   unsigned int t;
   return __builtin_ia32_rdsspd(t);

So if CET isn't enabled this intrinsic returns a random value instead of 0 like 
_get_ssp?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D124916

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

[clang] 94d36fd - Fix a crash on invalid with _Generic expressions

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2022-05-04T12:41:56-04:00
New Revision: 94d36fdbd7d2c6eab250f15f65fd20a6447b92eb

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

LOG: Fix a crash on invalid with _Generic expressions

We were failing to check if the controlling expression is dependent or
not when testing whether it has side effects. This would trigger an
assertion. Instead, if the controlling expression is dependent, we
suppress the check and diagnostic.

This fixes Issue 50227.

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/lib/Sema/SemaExpr.cpp
clang/test/Sema/generic-selection.c

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index 74d180053ee81..079523a07361e 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -128,6 +128,10 @@ Bug Fixes
   the functions were 
diff erent. It now diagnoses this case correctly as an
   ambiguous call and an error. Fixes
   `Issue 53640 `_.
+- No longer crash when trying to determine whether the controlling expression
+  argument to a generic selection expression has side effects in the case where
+  the expression is result dependent. This fixes
+  `Issue 50227 `_.
 
 Improvements to Clang's diagnostics
 ^^^

diff  --git a/clang/lib/Sema/SemaExpr.cpp b/clang/lib/Sema/SemaExpr.cpp
index 67012527888d3..058967b656bc9 100644
--- a/clang/lib/Sema/SemaExpr.cpp
+++ b/clang/lib/Sema/SemaExpr.cpp
@@ -1653,18 +1653,18 @@ Sema::CreateGenericSelectionExpr(SourceLocation KeyLoc,
 ControllingExpr = R.get();
   }
 
+  bool TypeErrorFound = false,
+   IsResultDependent = ControllingExpr->isTypeDependent(),
+   ContainsUnexpandedParameterPack
+ = ControllingExpr->containsUnexpandedParameterPack();
+
   // The controlling expression is an unevaluated operand, so side effects are
   // likely unintended.
-  if (!inTemplateInstantiation() &&
+  if (!inTemplateInstantiation() && !IsResultDependent &&
   ControllingExpr->HasSideEffects(Context, false))
 Diag(ControllingExpr->getExprLoc(),
  diag::warn_side_effects_unevaluated_context);
 
-  bool TypeErrorFound = false,
-   IsResultDependent = ControllingExpr->isTypeDependent(),
-   ContainsUnexpandedParameterPack
- = ControllingExpr->containsUnexpandedParameterPack();
-
   for (unsigned i = 0; i < NumAssocs; ++i) {
 if (Exprs[i]->containsUnexpandedParameterPack())
   ContainsUnexpandedParameterPack = true;

diff  --git a/clang/test/Sema/generic-selection.c 
b/clang/test/Sema/generic-selection.c
index 82d40afec8a5d..82ba75e1ffc73 100644
--- a/clang/test/Sema/generic-selection.c
+++ b/clang/test/Sema/generic-selection.c
@@ -1,5 +1,5 @@
-// RUN: %clang_cc1 -std=c11 -fsyntax-only -Wno-strict-prototypes -verify %s
-// RUN: %clang_cc1 -std=c99 -pedantic -fsyntax-only -Wno-strict-prototypes 
-verify=expected,ext %s
+// RUN: %clang_cc1 -std=c11 -fsyntax-only -Wno-strict-prototypes 
-Wno-implicit-function-declaration -verify %s
+// RUN: %clang_cc1 -std=c99 -pedantic -fsyntax-only -Wno-strict-prototypes 
-Wno-implicit-function-declaration -verify=expected,ext %s
 
 void g(void);
 
@@ -47,3 +47,13 @@ char testc(char);
 void PR30201(void) {
   _Generic(4, char:testc, default:test)(4); // ext-warning {{'_Generic' is a 
C11 extension}}
 }
+
+void GH50227(void) {
+  // Previously, the controlling expression for the outer _Generic makes it
+  // result dependent, and testing whether that controlling expression has side
+  // effects would cause a crash.
+  _Generic( // ext-warning {{'_Generic' is a C11 extension}}
+n(
+  _Generic(n++, int : 0) // expected-error {{cannot increment value of 
type 'int ()'}} ext-warning {{'_Generic' is a C11 extension}}
+), int : 0);
+}



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

[PATCH] D124938: [clang] serialize SUBMODULE_TOPHEADER relative to BaseDirectory

[Richard Howell via Phabricator via cfe-commits]

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

This diff changes the serialization of the `SUBMODULE_TOPHEADER`
entry in module files to be serialized relative to the module's
`BaseDirectory`. This matches the behavior of the
`SUBMODULE_HEADER` entry and will allow for the module to be
relocatable across machines.

The path is restored relative to the module's `BaseDirectory` on
deserialization.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D124938

Files:
  clang/lib/Serialization/ASTReader.cpp
  clang/lib/Serialization/ASTWriter.cpp
  clang/test/Modules/relative-submodule-topheader.m


Index: clang/test/Modules/relative-submodule-topheader.m
===
--- /dev/null
+++ clang/test/Modules/relative-submodule-topheader.m
@@ -0,0 +1,10 @@
+// RUN: cd %S
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x objective-c++ 
-fmodule-name=std -emit-module Inputs/submodules/module.map -o %t/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'vector.h'
+// CHECK:  blob data = 'vector.h'
+// CHECK:  blob data = 'type_traits.h'
+// CHECK:  blob data = 'type_traits.h'
+// CHECK:  blob data = 'hash_map.h'
+// CHECK:  blob data = 'hash_map.h'
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2848,8 +2848,11 @@
 {
   auto TopHeaders = Mod->getTopHeaders(PP->getFileManager());
   RecordData::value_type Record[] = {SUBMODULE_TOPHEADER};
-  for (auto *H : TopHeaders)
-Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, H->getName());
+  for (auto *H : TopHeaders) {
+SmallString<128> HeaderName(H->getName());
+PreparePathForOutput(HeaderName);
+Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, HeaderName);
+  }
 }
 
 // Emit the imports.
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -5637,9 +5637,12 @@
   // them here.
   break;
 
-case SUBMODULE_TOPHEADER:
-  CurrentModule->addTopHeaderFilename(Blob);
+case SUBMODULE_TOPHEADER: {
+  std::string HeaderName(Blob);
+  ResolveImportedPath(F, HeaderName);
+  CurrentModule->addTopHeaderFilename(HeaderName);
   break;
+}
 
 case SUBMODULE_UMBRELLA_DIR: {
   // See comments in SUBMODULE_UMBRELLA_HEADER


Index: clang/test/Modules/relative-submodule-topheader.m
===
--- /dev/null
+++ clang/test/Modules/relative-submodule-topheader.m
@@ -0,0 +1,10 @@
+// RUN: cd %S
+// RUN: %clang_cc1 -fmodules -fno-implicit-modules -x objective-c++ -fmodule-name=std -emit-module Inputs/submodules/module.map -o %t/mod.pcm
+// RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
+
+// CHECK:  blob data = 'vector.h'
+// CHECK:  blob data = 'vector.h'
+// CHECK:  blob data = 'type_traits.h'
+// CHECK:  blob data = 'type_traits.h'
+// CHECK:  blob data = 'hash_map.h'
+// CHECK:  blob data = 'hash_map.h'
Index: clang/lib/Serialization/ASTWriter.cpp
===
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -2848,8 +2848,11 @@
 {
   auto TopHeaders = Mod->getTopHeaders(PP->getFileManager());
   RecordData::value_type Record[] = {SUBMODULE_TOPHEADER};
-  for (auto *H : TopHeaders)
-Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, H->getName());
+  for (auto *H : TopHeaders) {
+SmallString<128> HeaderName(H->getName());
+PreparePathForOutput(HeaderName);
+Stream.EmitRecordWithBlob(TopHeaderAbbrev, Record, HeaderName);
+  }
 }
 
 // Emit the imports.
Index: clang/lib/Serialization/ASTReader.cpp
===
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -5637,9 +5637,12 @@
   // them here.
   break;
 
-case SUBMODULE_TOPHEADER:
-  CurrentModule->addTopHeaderFilename(Blob);
+case SUBMODULE_TOPHEADER: {
+  std::string HeaderName(Blob);
+  ResolveImportedPath(F, HeaderName);
+  CurrentModule->addTopHeaderFilename(HeaderName);
   break;
+}
 
 case SUBMODULE_UMBRELLA_DIR: {
   // See comments in SUBMODULE_UMBRELLA_HEADER
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] b6c67c3 - [clang] Track how headers get included generally during lookup time

[Cyndy Ishida via cfe-commits]

Author: Cyndy Ishida
Date: 2022-05-04T09:52:31-07:00
New Revision: b6c67c3c67893d532fe741c508dfa2ac40fae1ad

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

LOG: [clang] Track how headers get included generally during lookup time

tapi & clang-extractapi both attempt to construct then check against
how a header was included to determine api information when working
against multiple search paths, headermap, and vfsoverlay mechanisms.
Validating this against what the preprocessor sees during lookup time
makes this check more reliable.

Reviewed By: zixuw, jansvoboda11

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

Added: 


Modified: 
clang/include/clang/Lex/HeaderSearch.h
clang/lib/Lex/HeaderSearch.cpp
clang/unittests/Lex/HeaderSearchTest.cpp

Removed: 




diff  --git a/clang/include/clang/Lex/HeaderSearch.h 
b/clang/include/clang/Lex/HeaderSearch.h
index e88e600ba2b974..b523ad5ef9ff23 100644
--- a/clang/include/clang/Lex/HeaderSearch.h
+++ b/clang/include/clang/Lex/HeaderSearch.h
@@ -20,6 +20,7 @@
 #include "clang/Lex/ModuleMap.h"
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
+#include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
@@ -314,6 +315,9 @@ class HeaderSearch {
   /// whether they were valid or not.
   llvm::DenseMap LoadedModuleMaps;
 
+  // A map of discovered headers with their associated include file name.
+  llvm::DenseMap> IncludeNames;
+
   /// Uniqued set of framework names, which is used to track which
   /// headers were included as framework headers.
   llvm::StringSet FrameworkNames;
@@ -823,6 +827,13 @@ class HeaderSearch {
   /// Retrieve a uniqued framework name.
   StringRef getUniqueFrameworkName(StringRef Framework);
 
+  /// Retrieve the include name for the header.
+  ///
+  /// \param File The entry for a given header.
+  /// \returns The name of how the file was included when the header's location
+  /// was resolved.
+  StringRef getIncludeNameForHeader(const FileEntry *File) const;
+
   /// Suggest a path by which the specified file could be found, for use in
   /// diagnostics to suggest a #include. Returned path will only contain 
forward
   /// slashes as separators. MainFile is the absolute path of the file that we

diff  --git a/clang/lib/Lex/HeaderSearch.cpp b/clang/lib/Lex/HeaderSearch.cpp
index 14b9a1d7f68be1..5fd853541b6a21 100644
--- a/clang/lib/Lex/HeaderSearch.cpp
+++ b/clang/lib/Lex/HeaderSearch.cpp
@@ -1030,8 +1030,11 @@ Optional HeaderSearch::LookupFile(
 
 CurDir = It;
 
+const auto FE = &File->getFileEntry();
+IncludeNames[FE] = Filename;
+
 // This file is a system header or C++ unfriendly if the dir is.
-HeaderFileInfo &HFI = getFileInfo(&File->getFileEntry());
+HeaderFileInfo &HFI = getFileInfo(FE);
 HFI.DirInfo = CurDir->getDirCharacteristic();
 
 // If the directory characteristic is User but this framework was
@@ -1460,6 +1463,13 @@ StringRef HeaderSearch::getUniqueFrameworkName(StringRef 
Framework) {
   return FrameworkNames.insert(Framework).first->first();
 }
 
+StringRef HeaderSearch::getIncludeNameForHeader(const FileEntry *File) const {
+  auto It = IncludeNames.find(File);
+  if (It == IncludeNames.end())
+return {};
+  return It->second;
+}
+
 bool HeaderSearch::hasModuleMap(StringRef FileName,
 const DirectoryEntry *Root,
 bool IsSystem) {

diff  --git a/clang/unittests/Lex/HeaderSearchTest.cpp 
b/clang/unittests/Lex/HeaderSearchTest.cpp
index fbb5cb30754a79..87d1c01650987b 100644
--- a/clang/unittests/Lex/HeaderSearchTest.cpp
+++ b/clang/unittests/Lex/HeaderSearchTest.cpp
@@ -207,6 +207,7 @@ TEST_F(HeaderSearchTest, HeaderFrameworkLookup) {
   EXPECT_TRUE(FI);
   EXPECT_TRUE(FI->IsValid);
   EXPECT_EQ(FI->Framework.str(), "Foo");
+  EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
 }
 
 // Helper struct with null terminator character to make MemoryBuffer happy.
@@ -275,6 +276,7 @@ TEST_F(HeaderSearchTest, HeaderMapFrameworkLookup) {
   EXPECT_TRUE(FI);
   EXPECT_TRUE(FI->IsValid);
   EXPECT_EQ(FI->Framework.str(), "Foo");
+  EXPECT_EQ(Search.getIncludeNameForHeader(FE), "Foo/Foo.h");
 }
 
 } // namespace



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