[PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added a comment.

I am open to different approaches to this problem, this is opening attempt at 
fixing it but I may be missing other interactions.

AFAICT starting the definition of `ToRecord` the way  `CompleteDecl(…)`  does 
when we know `FromRecord` is not defined is just broken for the `RecordDecl` 
case if we have bases.

It took me a lot of time to come up with this reproducer from a real life issue 
debugging `llc`.


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

https://reviews.llvm.org/D78000



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

[PATCH] D78000: [ASTImporter] Fix handling of not defined FromRecord in ImportContext(...)

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik created this revision.
shafik added reviewers: martong, balazske, teemperor, a_sidorin.
Herald added a subscriber: rnkovacs.
Herald added a reviewer: a.sidorin.
shafik added a comment.

I am open to different approaches to this problem, this is opening attempt at 
fixing it but I may be missing other interactions.

AFAICT starting the definition of `ToRecord` the way  `CompleteDecl(…)`  does 
when we know `FromRecord` is not defined is just broken for the `RecordDecl` 
case if we have bases.

It took me a lot of time to come up with this reproducer from a real life issue 
debugging `llc`.


In `ImportContext(…)` we may call into `CompleteDecl(…)` which if `FromRecrord` 
is not defined will start the definition of a `ToRecord` but from what I can 
tell at least one of the paths though here don't ensure we complete the 
definition. 
For a `RecordDecl` this can be problematic since this means we won’t import 
base classes and we won’t have any of the methods or types we inherit from 
these bases.

One such path is through `VisitTypedefNameDecl(…)` which is exercised by the 
reproducer.

For LLDB expression parsing this results in expressions failing but sometimes 
succeeding in subsequent attempts e.g.:

  (lldb) p BB->dump()
  error: no member named 'dump' in 'B'
  (lldb) p BB->dump()
  (lldb) 

This happens because the first time through `FromRecord` is not defined but in 
subsequent attempts through it may be.


https://reviews.llvm.org/D78000

Files:
  clang/lib/AST/ASTImporter.cpp
  
lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py
  
lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp


Index: 
lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp
===
--- /dev/null
+++ 
lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp
@@ -0,0 +1,36 @@
+struct B {
+  int dump() const;
+};
+
+int B::dump() const { return 42; }
+
+// Derived class D obtains dump() method from base class B
+struct D : public B {
+  // Introduce a TypedefNameDecl
+  using Iterator = D *;
+};
+
+struct C {
+  // This will cause use to invoke VisitTypedefNameDecl(...) when Importing
+  // the DeclContext of C.
+  // We will invoke ImportContext(...) which should force the From Decl to
+  // be defined if it not already defined. We will then Import the definition
+  // to the To Decl.
+  // This will force processing of the base class of D which allows us to see
+  // base class methods such as dump().
+  D::Iterator iter;
+
+  bool f(D *DD) {
+return true; //%self.expect_expr("DD->dump()", result_type="int",
+ // result_value="42")
+  }
+};
+
+int main() {
+  C c;
+  D d;
+
+  c.f();
+
+  return 0;
+}
Index: 
lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py
===
--- /dev/null
+++ 
lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/TestImportBaseClassWhenClassHasDerivedMember.py
@@ -0,0 +1,4 @@
+from lldbsuite.test import lldbinline
+from lldbsuite.test import decorators
+
+lldbinline.MakeInlineTest(__file__, globals())
Index: clang/lib/AST/ASTImporter.cpp
===
--- clang/lib/AST/ASTImporter.cpp
+++ clang/lib/AST/ASTImporter.cpp
@@ -8166,7 +8166,19 @@
   FromRecord, ToRecord, ASTNodeImporter::IDK_Basic))
 return std::move(Err);
 } else {
-  CompleteDecl(ToRecord);
+  // If FromRecord is not defined we need to force it to be.
+  // Simply calling CompleteDecl(...) for a RecordDecl will break some 
cases
+  // it will start the definition but we never finish it.
+  // If a RecordDecl has base classes they won't be imported and we will
+  // be missing anything that we inherit from those bases.
+  if (FromRecord->hasExternalLexicalStorage() &&
+  !FromRecord->isCompleteDefinition())
+FromRecord->getASTContext().getExternalSource()->CompleteType(
+FromRecord);
+
+  if (Error Err = ASTNodeImporter(*this).ImportDefinition(
+  FromRecord, ToRecord, ASTNodeImporter::IDK_Basic))
+return std::move(Err);
 }
   } else if (auto *ToEnum = dyn_cast(ToDC)) {
 auto *FromEnum = cast(FromDC);


Index: lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp
===
--- /dev/null
+++ lldb/test/API/commands/expression/import_base_class_when_class_has_derived_member/main.cpp
@@ -0,0 +1,36 @@
+struct B {
+  int dump() const;
+};
+
+int B::dump() const { return 42; }
+
+// Derived class D obtains dump() method from base class B
+struct D : public B {
+ 

[clang] 8dda0f9 - Remove dependency between test files.

[Alexander Kornienko via cfe-commits]

Author: Alexander Kornienko
Date: 2020-04-13T06:19:09+02:00
New Revision: 8dda0f91995955bb8f484f8d41374aff59118355

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

LOG: Remove dependency between test files.

There seems to be no good reason for rocm-device-libs.cl to depend on opencl.cl.
Removed this dependency to unbreak the tests in our setup.

Added: 


Modified: 
clang/test/Driver/rocm-device-libs.cl

Removed: 




diff  --git a/clang/test/Driver/rocm-device-libs.cl 
b/clang/test/Driver/rocm-device-libs.cl
index 77e9782f2594..a5e9e6496c45 100644
--- a/clang/test/Driver/rocm-device-libs.cl
+++ b/clang/test/Driver/rocm-device-libs.cl
@@ -7,7 +7,7 @@
 // RUN: %clang -### -target amdgcn-amd-amdhsa \
 // RUN:   -x cl -mcpu=gfx900 \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMON,COMMON-DEFAULT,GFX900-DEFAULT,GFX900,WAVE64 %s
 
 
@@ -16,7 +16,7 @@
 // RUN: %clang -### -target amdgcn-amd-amdhsa \
 // RUN:   -x cl -mcpu=gfx803 \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMON,COMMON-DEFAULT,GFX803-DEFAULT,GFX803,WAVE64 %s
 
 
@@ -25,7 +25,7 @@
 // RUN: %clang -### -target amdgcn-amd-amdhsa \
 // RUN:   -x cl -mcpu=fiji \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMON,COMMON-DEFAULT,GFX803-DEFAULT,GFX803,WAVE64 %s
 
 
@@ -34,7 +34,7 @@
 // RUN:   -x cl -mcpu=gfx900 \
 // RUN:   -cl-denorms-are-zero \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMON,COMMON-DAZ,GFX900,WAVE64 %s
 
 
@@ -42,7 +42,7 @@
 // RUN:   -x cl -mcpu=gfx803 \
 // RUN:   -cl-denorms-are-zero \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMON,COMMON-DAZ,GFX803,WAVE64 %s
 
 
@@ -51,7 +51,7 @@
 // RUN:   -x cl -mcpu=gfx803 \
 // RUN:   -cl-finite-math-only \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMON,COMMON-FINITE-ONLY,GFX803,WAVE64 %s
 
 
@@ -60,7 +60,7 @@
 // RUN:   -x cl -mcpu=gfx803 \
 // RUN:   -cl-fp32-correctly-rounded-divide-sqrt \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMON,COMMON-CORRECT-SQRT,GFX803,WAVE64 %s
 
 
@@ -69,7 +69,7 @@
 // RUN:   -x cl -mcpu=gfx803 \
 // RUN:   -cl-fast-relaxed-math \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMON,COMMON-FAST-RELAXED,GFX803,WAVE64 %s
 
 
@@ -78,45 +78,45 @@
 // RUN:   -x cl -mcpu=gfx803 \
 // RUN:   -cl-unsafe-math-optimizations \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMON,COMMON-UNSAFE,GFX803,WAVE64 %s
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa\
 // RUN:   -x cl -mcpu=gfx1010\
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMMON,GFX1010,WAVE32 %s
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa\
 // RUN:   -x cl -mcpu=gfx1011\
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMMON,GFX1011,WAVE32 %s
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa\
 // RUN:   -x cl -mcpu=gfx1012\
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMMON,GFX1012,WAVE32 %s
 
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa\
 // RUN:   -x cl -mcpu=gfx1010 -mwavefrontsize64  \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs \
-// RUN:   %S/opencl.cl \
+// RUN:   %s \
 // RUN: 2>&1 | FileCheck -dump-input-on-failure 
--check-prefixes=COMMMON,GFX1010,WAVE64 %s
 
 // RUN: %clang -### -target amdgcn-amd-amdhsa\
 // RUN:   -x cl -mcpu=gfx1010 -mwavefrontsize64 -mno-wavefrontsize64  \
 // RUN:   --rocm-path=%S/Inputs/rocm-device-libs 

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/Sema/SemaOverload.cpp:9481
+  // emitted, Cand1 is not better than Cand2. This rule should have precedence
+  // over other rules.
+  //

Please add `[CUDA]` or something similar to the top of this comment so that 
readers can immediately know that it's dialect-specific.

At a high level, this part of the rule is essentially saying that CUDA 
non-emittability is a kind of non-viability.  Should we just make non-emittable 
functions get flagged as non-viable (which will avoid a lot of relatively 
expensive conversion checking), or is it important to be able to select 
non-emittable candidates over candidates that are non-viable for other reasons?



Comment at: clang/lib/Sema/SemaOverload.cpp:9749
+  if (isBetterMultiversionCandidate(Cand1, Cand2))
+return true;
+

If we move anything below this check, it needs to figure out a tri-state so 
that it can return false if `Cand2` is a better candidate than `Cand1`.  Now, 
that only matters if multiversion functions are supported under CUDA, but if 
you're relying on them not being supported, that should at least be commented 
on.



Comment at: clang/lib/Sema/SemaOverload.cpp:9752
+  // If other rules cannot determine which is better, CUDA preference is used
+  // to determine which is better.
+  if (S.getLangOpts().CUDA && Cand1.Function && Cand2.Function) {

Okay, let's think about the right place to put this check in the ordering; we 
don't want different extensions to get into a who-comes-last competition.

- Certainly this should have lower priority than the standard-defined 
preferences like argument conversion ranks or `enable_if` partial-ordering.
- The preference for pass-object-size parameters is probably most similar to a 
type-based-overloading decision and so should take priority.
- I would say that this should take priority over function multi-versioning.  
Function multi-versioning is all about making specialized versions of the "same 
function", whereas I think host/device overloading is meant to be semantically 
broader than that.

What do you think?

Regardless, the rationale for the order should be explained in comments.


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

https://reviews.llvm.org/D77954



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

[PATCH] D77743: [HIP] Emit symbols with kernel name in host binary

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Ah, too bad.  Is there any way to suppress that in debug info?  I'm not sure 
there's any other way to satisfy the competing requirements here, and if it's 
not going to be consistent, it'd be better to avoid the complexity of mangling 
the thunk differently.


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

https://reviews.llvm.org/D77743



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

[PATCH] D77233: [NFC] Refactoring PropertyAttributeKind for ObjCPropertyDecl and ObjCDeclSpec.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/DeclObjCCommon.h:21
+/// Keep this list in sync with LLVM's Dwarf.h ApplePropertyAttributes.
+enum ObjCPropertyAttributeKind {
+  OBJC_PR_noattr = 0x00,

erik.pilkington wrote:
> plotfi wrote:
> > plotfi wrote:
> > > compnerd wrote:
> > > > It seems that you are touching all the sites that use this, so perhaps 
> > > > this is the time to change this to `enum class` and drop the `OBJC_PR_` 
> > > > prefixes and explicitly inherit from `uint16_t`.  This should be named 
> > > > `ObjCPropertyAttribute` I think.
> > > Ah yeah, that sounds pretty good. Will do. 
> > Talked offline: update is that its not so easy to change these enums to 
> > enum classes because of how they are constantly used with unsigned ints. I 
> > could try and implement lots of operator overloads but I think that could 
> > be potentially buggy and not so NFC-like. @compnerd you wanted to leave the 
> > OBJC_PR_  and dropped the DQ prefixes right? Other than that, ping (for 
> > others)? I think this could be a nice cleanup. 
> If you just want the scoping benefits of the enum class you can simulate it 
> with a `namespace`:
> 
> ```
> namespace ObjCPropertyAttribute {
> enum {
>   noattr,
>   readonly,
>   ...
> };
> }
> ```
> 
> Which would make the users of this enum look a bit nicer. I agree that adding 
> overloading operators is over-engineering this. 
Ah yeah, nice. so the prefixes can be dropped. Another reason I don't want to 
change the enum to an enun class is, from the comments it seems a lot of the 
use of unsigned is to avoid the differences in how Win32 vs *nix handle enums 
(signed vs unsigned I think). I'll post a follow up with the scoping. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77233



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

[PATCH] D77743: [HIP] Emit symbols with kernel name in host binary

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D77743#1975822 , @rjmccall wrote:

> Is the renaming just being done to avoid breakpoints from triggering in the 
> stub?  Can you not disable debugging the stub using whatever mechanism 
> `__attribute__((nodebug))` uses?


I tried it. The source info and line number is gone, but gdb will still break 
on the function since symbol is still there.


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

https://reviews.llvm.org/D77743



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

[PATCH] D77989: Allow disabling of vectorization using internal options

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson created this revision.
tejohnson added reviewers: fhahn, wmi.
Herald added subscribers: cfe-commits, dexonsmith, steven_wu, hiraditya.
Herald added a project: clang.

Currently, the internal options -vectorize-loops, -vectorize-slp, and
-interleave-loops do not have much practical effect. This is because
they are used to initialize the corresponding flags in the pass
managers, and those flags are then unconditionally overwritten when
compiling via clang or via LTO from the linkers. The only exception was
-vectorize-loops via opt because of some special hackery there.

While vectorization could still be disabled when compiling via clang,
using -fno-[slp-]vectorize, this meant that there was no way to disable
it when compiling in LTO mode via the linkers. This only affected
ThinLTO, since for regular LTO vectorization is done during the compile
step for scalability reasons. For ThinLTO it is invoked in the LTO
backends.

This patch makes it so the internal options can actually be used to
disable these optimizations. Ultimately, the best long term solution is
to mark the loops with metadata (similar to the approach used to fix
-fno-unroll-loops in D77058 ), but this 
enables a shorter term
workaround, and actually makes these internal options useful.

I constant propagated the initial values of these internal flags into
the pass manager flags (for some reasons vectorize-loops and
interleave-loops were initialized to true, while vectorize-slp was
initialized to false). As mentioned above, they are overwritten
unconditionally so this doesn't have any real impact, and these initial
values aren't particularly meaningful.

I then changed the passes to check the internl values and return without
performing the associated optimization when false (I changed the default
of -vectorize-slp to true so the options behave similarly). I was able
to remove the hackery in opt used to get -vectorize-loops=false to work,
as well as a special option there used to disable SLP vectorization.

Finally, I changed thinlto-slp-vectorize-pm.c to:
a) Only test SLP (moved the loop vectorization checking to a new test).
b) Use code that is slp vectorized when it is enabled, and check that
instead of whether the pass is enabled.
c) Test the new behavior of -vectorize-slp.
d) Test both pass managers.

The loop vectorization (and associated interleaving) testing I moved to
a new thinlto-loop-vectorize-pm.c test, with several changes:
a) Changed the flags on the interleaving testing so that it will
actually interleave, and check that.
b) Test the new behavior of -vectorize-loops and -interleave-loops.
c) Test both pass managers.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77989

Files:
  clang/test/CodeGen/thinlto-loop-vectorize-pm.c
  clang/test/CodeGen/thinlto-slp-vectorize-pm.c
  llvm/include/llvm/Passes/PassBuilder.h
  llvm/include/llvm/Transforms/Vectorize/LoopVectorize.h
  llvm/include/llvm/Transforms/Vectorize/SLPVectorizer.h
  llvm/lib/Passes/PassBuilder.cpp
  llvm/lib/Transforms/IPO/PassManagerBuilder.cpp
  llvm/lib/Transforms/Vectorize/LoopVectorize.cpp
  llvm/lib/Transforms/Vectorize/SLPVectorizer.cpp
  llvm/test/Transforms/SLPVectorizer/X86/opt.ll
  llvm/tools/opt/opt.cpp

Index: llvm/tools/opt/opt.cpp
===
--- llvm/tools/opt/opt.cpp
+++ llvm/tools/opt/opt.cpp
@@ -180,11 +180,6 @@
  cl::desc("Disable loop unrolling in all relevant passes"),
  cl::init(false));
 
-static cl::opt
-DisableSLPVectorization("disable-slp-vectorization",
-cl::desc("Disable the slp vectorization pass"),
-cl::init(false));
-
 static cl::opt EmitSummaryIndex("module-summary",
   cl::desc("Emit module summary index"),
   cl::init(false));
@@ -406,18 +401,9 @@
   Builder.DisableUnrollLoops = (DisableLoopUnrolling.getNumOccurrences() > 0) ?
DisableLoopUnrolling : OptLevel == 0;
 
-  // Check if vectorization is explicitly disabled via -vectorize-loops=false.
-  // The flag enables vectorization in the LoopVectorize pass, it is on by
-  // default, and if it was disabled, leave it disabled here.
-  // Another flag that exists: -loop-vectorize, controls adding the pass to the
-  // pass manager. If set, the pass is added, and there is no additional check
-  // here for it.
-  if (Builder.LoopVectorize)
-Builder.LoopVectorize = OptLevel > 1 && SizeLevel < 2;
-
-  // When #pragma vectorize is on for SLP, do the same as above
-  Builder.SLPVectorize =
-  DisableSLPVectorization ? false : OptLevel > 1 && SizeLevel < 2;
+  Builder.LoopVectorize = OptLevel > 1 && SizeLevel < 2;
+
+  Builder.SLPVectorize = OptLevel > 1 && SizeLevel < 2;
 
   if (TM)
 TM->adjustPassManager(Builder);
Index: llvm/test/Transforms/SLPVectorizer/X86/opt.ll

[PATCH] D77233: [NFC] Refactoring PropertyAttributeKind for ObjCPropertyDecl and ObjCDeclSpec.

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington added inline comments.



Comment at: clang/include/clang/AST/DeclObjCCommon.h:21
+/// Keep this list in sync with LLVM's Dwarf.h ApplePropertyAttributes.
+enum ObjCPropertyAttributeKind {
+  OBJC_PR_noattr = 0x00,

plotfi wrote:
> plotfi wrote:
> > compnerd wrote:
> > > It seems that you are touching all the sites that use this, so perhaps 
> > > this is the time to change this to `enum class` and drop the `OBJC_PR_` 
> > > prefixes and explicitly inherit from `uint16_t`.  This should be named 
> > > `ObjCPropertyAttribute` I think.
> > Ah yeah, that sounds pretty good. Will do. 
> Talked offline: update is that its not so easy to change these enums to enum 
> classes because of how they are constantly used with unsigned ints. I could 
> try and implement lots of operator overloads but I think that could be 
> potentially buggy and not so NFC-like. @compnerd you wanted to leave the 
> OBJC_PR_  and dropped the DQ prefixes right? Other than that, ping (for 
> others)? I think this could be a nice cleanup. 
If you just want the scoping benefits of the enum class you can simulate it 
with a `namespace`:

```
namespace ObjCPropertyAttribute {
enum {
  noattr,
  readonly,
  ...
};
}
```

Which would make the users of this enum look a bit nicer. I agree that adding 
overloading operators is over-engineering this. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77233



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

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Ten Tzen via Phabricator via cfe-commits]

tentzen added a comment.

In D77936#1976984 , @efriedma wrote:

> I'm not concerned about the performance implications of whatever approach we 
> take here.  In the worst case, an "icmp+zext" corresponds to two extra 
> arithmetic instructions; that's not enough to matter.  And I expect usually 
> it'll get optimized away.
>
> I'd prefer to avoid exposing our cleanup numbering to user code, if possible. 
>  The Microsoft documentation says it returns 0 or 1.


OK,  will do..
thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77936



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

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Ten Tzen via Phabricator via cfe-commits]

tentzen updated this revision to Diff 256906.
tentzen added a comment.

Per Eli's suggestion,
Use icmp (and zext) to check the JumpDest index, instead of directly passing 
the Index to _finally().


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77936

Files:
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/EHScopeStack.h
  clang/test/CodeGen/windows-seh-abnormal-exits.c

Index: clang/test/CodeGen/windows-seh-abnormal-exits.c
===
--- /dev/null
+++ clang/test/CodeGen/windows-seh-abnormal-exits.c
@@ -0,0 +1,30 @@
+// RUN: %clang_cc1 -triple x86_64-windows -fms-extensions -Wno-implicit-function-declaration -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: %[[src:[0-9-]+]] = call i8* @llvm.localaddress()
+// CHECK-NEXT: %cleanup.dest = load i32, i32* %cleanup.dest.slot, align 4
+// CHECK-NEXT: %[[src2:[0-9-]+]] = icmp ne i32 %cleanup.dest, 0
+// CHECK-NEXT: %[[src3:[0-9-]+]] = zext i1 %[[src2]] to i8
+// CHECK-NEXT: call void @"?fin$0@0@seh_abnormal_exits@@"(i8 %[[src3]], i8* %[[src]])
+
+void seh_abnormal_exits(int *Counter) {
+  for (int i = 0; i < 5; i++) {
+__try {
+  if (i == 0)
+continue;   // abnormal termination
+  else if (i == 1)
+goto t10;   // abnormal termination
+  else if (i == 2)
+__leave;  // normal execution
+  else if (i == 4)
+return;  // abnormal termination
+}
+__finally {
+  if (AbnormalTermination()) {
+*Counter += 1;
+  }
+}
+  t10:;
+  }
+  return; // *Counter == 3
+}
+
Index: clang/lib/CodeGen/EHScopeStack.h
===
--- clang/lib/CodeGen/EHScopeStack.h
+++ clang/lib/CodeGen/EHScopeStack.h
@@ -158,9 +158,10 @@
 /// Generation flags.
 class Flags {
   enum {
-F_IsForEH = 0x1,
+F_IsForEH = 0x1,
 F_IsNormalCleanupKind = 0x2,
-F_IsEHCleanupKind = 0x4
+F_IsEHCleanupKind = 0x4,
+F_HasSehAbnormalExits = 0x8,
   };
   unsigned flags;
 
@@ -179,8 +180,10 @@
   /// cleanup.
   bool isEHCleanupKind() const { return flags & F_IsEHCleanupKind; }
   void setIsEHCleanupKind() { flags |= F_IsEHCleanupKind; }
-};
 
+  bool hasSehAbnormalExits() const { return flags & F_HasSehAbnormalExits; }
+  void setHasSehAbnormalExits() { flags |= F_HasSehAbnormalExits; }
+};
 
 /// Emit the cleanup.  For normal cleanups, this is run in the
 /// same EH context as when the cleanup was pushed, i.e. the
Index: clang/lib/CodeGen/CGException.cpp
===
--- clang/lib/CodeGen/CGException.cpp
+++ clang/lib/CodeGen/CGException.cpp
@@ -1639,6 +1639,19 @@
 
 llvm::Value *IsForEH =
 llvm::ConstantInt::get(CGF.ConvertType(ArgTys[0]), F.isForEHCleanup());
+
+// Except _leave and fall-through at the end, all other exits in a _try
+//   (return/goto/continue/break) are considered as abnormal terminations
+//   since _leave/fall-through is always Indexed 0,
+//   just use NormalCleanupDestSlot (>= 1 for goto/return/..),
+//   as 1st Arg to indicate abnormal termination
+if (!F.isForEHCleanup() && F.hasSehAbnormalExits()) {
+  Address Addr = CGF.getNormalCleanupDestSlot();
+  llvm::Value *Load = CGF.Builder.CreateLoad(Addr, "cleanup.dest");
+  llvm::Value *Zero = llvm::Constant::getNullValue(CGM.Int32Ty);
+  IsForEH = CGF.Builder.CreateICmpNE(Load, Zero);
+}
+
 Args.add(RValue::get(IsForEH), ArgTys[0]);
 Args.add(RValue::get(FP), ArgTys[1]);
 
Index: clang/lib/CodeGen/CGCleanup.cpp
===
--- clang/lib/CodeGen/CGCleanup.cpp
+++ clang/lib/CodeGen/CGCleanup.cpp
@@ -860,6 +860,9 @@
 // TODO: base this on the number of branch-afters and fixups
 const unsigned SwitchCapacity = 10;
 
+// pass the abnormal exit flag to Fn (SEH cleanup)
+cleanupFlags.setHasSehAbnormalExits();
+
 llvm::LoadInst *Load =
   createLoadInstBefore(getNormalCleanupDestSlot(), "cleanup.dest",
nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77954: [CUDA][HIP] Fix host/device based overload resolution

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 256903.
yaxunl retitled this revision from "[CUDA][HIP] Fix overload resolution issue 
for device host functions" to "[CUDA][HIP] Fix host/device based overload 
resolution".
yaxunl added a comment.

Revised by John's comments.


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

https://reviews.llvm.org/D77954

Files:
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCUDA/function-overload.cu

Index: clang/test/SemaCUDA/function-overload.cu
===
--- clang/test/SemaCUDA/function-overload.cu
+++ clang/test/SemaCUDA/function-overload.cu
@@ -331,9 +331,6 @@
 // If we have a mix of HD and H-only or D-only candidates in the overload set,
 // normal C++ overload resolution rules apply first.
 template  TemplateReturnTy template_vs_hd_function(T arg)
-#ifdef __CUDA_ARCH__
-//expected-note@-2 {{declared here}}
-#endif
 {
   return TemplateReturnTy();
 }
@@ -342,11 +339,13 @@
 }
 
 __host__ __device__ void test_host_device_calls_hd_template() {
-  HostDeviceReturnTy ret1 = template_vs_hd_function(1.0f);
-  TemplateReturnTy ret2 = template_vs_hd_function(1);
 #ifdef __CUDA_ARCH__
-  // expected-error@-2 {{reference to __host__ function 'template_vs_hd_function' in __host__ __device__ function}}
+  typedef HostDeviceReturnTy ExpectedReturnTy;
+#else
+  typedef TemplateReturnTy ExpectedReturnTy;
 #endif
+  HostDeviceReturnTy ret1 = template_vs_hd_function(1.0f);
+  ExpectedReturnTy ret2 = template_vs_hd_function(1);
 }
 
 __host__ void test_host_calls_hd_template() {
Index: clang/lib/Sema/SemaOverload.cpp
===
--- clang/lib/Sema/SemaOverload.cpp
+++ clang/lib/Sema/SemaOverload.cpp
@@ -9475,6 +9475,35 @@
   else if (!Cand1.Viable)
 return false;
 
+  // If Cand1 can be emitted and Cand2 cannot be emitted in the current context,
+  // Cand1 is better than Cand2. If Cand1 can not be emitted and Cand2 can be
+  // emitted, Cand1 is not better than Cand2. This rule should have precedence
+  // over other rules.
+  //
+  // If both Cand1 and Cand2 can be emitted, or neither can be emitted, then
+  // other rules should be used to determine which is better.
+  //
+  // If other rules cannot determine which is better, CUDA preference will be
+  // used again to determine which is better.
+  //
+  // TODO: Currently IdentifyCUDAPreference does not return correct values
+  // for functions called in global variable initializers due to missing
+  // correct context about device/host. Therefore we can only enforce this
+  // rule when there is a caller. We should enforce this rule for functions
+  // in global variable initializers once proper context is added.
+  if (S.getLangOpts().CUDA && Cand1.Function && Cand2.Function) {
+if (FunctionDecl *Caller = dyn_cast(S.CurContext)) {
+  auto Cand1Emittable = S.IdentifyCUDAPreference(Caller, Cand1.Function) >
+Sema::CFP_WrongSide;
+  auto Cand2Emittable = S.IdentifyCUDAPreference(Caller, Cand2.Function) >
+Sema::CFP_WrongSide;
+  if (Cand1Emittable && !Cand2Emittable)
+return true;
+  if (!Cand1Emittable && Cand2Emittable)
+return false;
+}
+  }
+
   // C++ [over.match.best]p1:
   //
   //   -- if F is a static member function, ICS1(F) is defined such
@@ -9709,12 +9738,6 @@
   return Cmp == Comparison::Better;
   }
 
-  if (S.getLangOpts().CUDA && Cand1.Function && Cand2.Function) {
-FunctionDecl *Caller = dyn_cast(S.CurContext);
-return S.IdentifyCUDAPreference(Caller, Cand1.Function) >
-   S.IdentifyCUDAPreference(Caller, Cand2.Function);
-  }
-
   bool HasPS1 = Cand1.Function != nullptr &&
 functionHasPassObjectSizeParams(Cand1.Function);
   bool HasPS2 = Cand2.Function != nullptr &&
@@ -9722,7 +9745,19 @@
   if (HasPS1 != HasPS2 && HasPS1)
 return true;
 
-  return isBetterMultiversionCandidate(Cand1, Cand2);
+  if (isBetterMultiversionCandidate(Cand1, Cand2))
+return true;
+
+  // If other rules cannot determine which is better, CUDA preference is used
+  // to determine which is better.
+  if (S.getLangOpts().CUDA && Cand1.Function && Cand2.Function) {
+if (FunctionDecl *Caller = dyn_cast(S.CurContext)) {
+  return S.IdentifyCUDAPreference(Caller, Cand1.Function) >
+ S.IdentifyCUDAPreference(Caller, Cand2.Function);
+}
+  }
+
+  return false;
 }
 
 /// Determine whether two declarations are "equivalent" for the purposes of
@@ -9808,33 +9843,6 @@
   std::transform(begin(), end(), std::back_inserter(Candidates),
  [](OverloadCandidate ) { return  });
 
-  // [CUDA] HD->H or HD->D calls are technically not allowed by CUDA but
-  // are accepted by both clang and NVCC. However, during a particular
-  // compilation mode only one call variant is viable. We need to
-  // exclude non-viable overload candidates from consideration 

[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Please upload patches with full context (-U100).




Comment at: clang/lib/CodeGen/CGException.cpp:1798
+
+// if the parent is a _finally, need to retrive Establisher's FP,
+//  2nd paramenter, saved & named frame_pointer in parent's frame

Maybe worth expanding this comment a little, to explain that a "finally" should 
have at most one localescape'ed variable.  (At least, that's my understanding.)



Comment at: clang/lib/CodeGen/CGException.cpp:1800
+//  2nd paramenter, saved & named frame_pointer in parent's frame
+if (ParentCGF.ParentCGF != NULL) {
+  // Locate and escape Parent's frame_pointer.addr alloca

nullptr

Do you actually use ParentCGF.ParentCGF anywhere?  If not, do you really need 
to save it?



Comment at: clang/lib/CodeGen/CGException.cpp:1805
+llvm::AllocaInst *II = dyn_cast();
+if (II && II->getName().startswith("frame_pointer")) {
+  FramePtrAddrAlloca = II;

Using the name isn't reliable. You should be using data stored somewhere in the 
CodeGenFunction or something like that.



Comment at: clang/lib/CodeGen/CGException.cpp:1822
+  llvm::Function *FrameRecoverFn = llvm::Intrinsic::getDeclaration(
+  (), llvm::Intrinsic::localrecover);
+  llvm::Constant *ParentI8Fn =

Is there some reason you can't reuse recoverAddrOfEscapedLocal here?



Comment at: clang/lib/CodeGen/CodeGenFunction.h:370
 
+  // For outliner helper only
+  CodeGenFunction *ParentCGF = nullptr;

This comment isn't really helpful.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77982



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

[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

[Ten Tzen via Phabricator via cfe-commits]

tentzen updated this revision to Diff 256896.
tentzen added a comment.

Fixed the format at comment lines


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77982

Files:
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/windows-seh-filter-inFinally.c

Index: clang/test/CodeGen/windows-seh-filter-inFinally.c
===
--- /dev/null
+++ clang/test/CodeGen/windows-seh-filter-inFinally.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple x86_64-windows -fms-extensions -Wno-implicit-function-declaration -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: %[[dst:[0-9-]+]] = call i8* @llvm.eh.recoverfp(i8* bitcast (void (i8, i8*)* @"?fin$0@0@main@@" to i8*), i8* %frame_pointer)
+// CHECK-NEXT: %[[dst1:[0-9-]+]] = call i8* @llvm.localrecover(i8* bitcast (void (i8, i8*)* @"?fin$0@0@main@@" to i8*), i8* %[[dst]], i32 0)
+// CHECK-NEXT: %[[dst2:[0-9-]+]] = bitcast i8* %[[dst1]] to i8**
+// CHECK-NEXT: = load i8*, i8** %[[dst2]], align 8
+
+int
+main(int argc, char *argv[])
+{
+int Counter = 0;
+//
+// Try/except within the finally clause of a try/finally.
+//
+__try {
+  Counter -= 1;
+}
+__finally {
+  __try {
+Counter += 2;
+// RtlRaiseStatus(STATUS_INTEGER_OVERFLOW);
+  } __except(Counter) {
+__try {
+  Counter += 3;
+}
+__finally {
+  if (abnormal_termination() == 1) {
+Counter += 5;
+  }
+}
+  }
+}
+// expect Counter == 9
+return 1;
+}
+
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -367,6 +367,9 @@
   CodeGenModule   // Per-module state.
   const TargetInfo 
 
+  // For outliner helper only
+  CodeGenFunction *ParentCGF = nullptr;
+
   typedef std::pair ComplexPairTy;
   LoopInfoStack LoopStack;
   CGBuilderTy Builder;
Index: clang/lib/CodeGen/CGException.cpp
===
--- clang/lib/CodeGen/CGException.cpp
+++ clang/lib/CodeGen/CGException.cpp
@@ -1794,6 +1794,40 @@
 llvm::Constant *ParentI8Fn =
 llvm::ConstantExpr::getBitCast(ParentCGF.CurFn, Int8PtrTy);
 ParentFP = Builder.CreateCall(RecoverFPIntrin, {ParentI8Fn, EntryFP});
+
+// if the parent is a _finally, need to retrive Establisher's FP,
+//  2nd paramenter, saved & named frame_pointer in parent's frame
+if (ParentCGF.ParentCGF != NULL) {
+  // Locate and escape Parent's frame_pointer.addr alloca
+  llvm::AllocaInst *FramePtrAddrAlloca = nullptr;
+  for (llvm::Instruction  : ParentCGF.CurFn->getEntryBlock()) {
+llvm::AllocaInst *II = dyn_cast();
+if (II && II->getName().startswith("frame_pointer")) {
+  FramePtrAddrAlloca = II;
+  break;
+}
+  }
+  assert(FramePtrAddrAlloca);
+  auto InsertPair = ParentCGF.EscapedLocals.insert(
+  std::make_pair(FramePtrAddrAlloca, ParentCGF.EscapedLocals.size()));
+  int FrameEscapeIdx = InsertPair.first->second;
+
+  // an example of a filter's prolog::
+  // %0 = call i8* @llvm.eh.recoverfp(bitcast(@"?fin$0@0@main@@"),..)
+  // %1 = call i8* @llvm.localrecover(bitcast(@"?fin$0@0@main@@"),..)
+  // %2 = bitcast i8* %1 to i8**
+  // %3 = load i8*, i8* *%2, align 8
+  //   ==> %3 is the frame-pointer of outermost host function
+  llvm::Function *FrameRecoverFn = llvm::Intrinsic::getDeclaration(
+  (), llvm::Intrinsic::localrecover);
+  llvm::Constant *ParentI8Fn =
+  llvm::ConstantExpr::getBitCast(ParentCGF.CurFn, Int8PtrTy);
+  ParentFP = Builder.CreateCall(
+  FrameRecoverFn, {ParentI8Fn, ParentFP,
+   llvm::ConstantInt::get(Int32Ty, FrameEscapeIdx)});
+  ParentFP = Builder.CreateBitCast(ParentFP, CGM.VoidPtrPtrTy);
+  ParentFP = Builder.CreateLoad(Address(ParentFP, getPointerAlign()));
+}
   }
 
   // Create llvm.localrecover calls for all captures.
@@ -1992,6 +2026,7 @@
 
 void CodeGenFunction::EnterSEHTryStmt(const SEHTryStmt ) {
   CodeGenFunction HelperCGF(CGM, /*suppressNewContext=*/true);
+  HelperCGF.ParentCGF = this;
   if (const SEHFinallyStmt *Finally = S.getFinallyHandler()) {
 // Outline the finally block.
 llvm::Function *FinallyFunc =
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77984: Make IRBuilder automatically set alignment on load/store/alloca.

[Eli Friedman via Phabricator via cfe-commits]

efriedma created this revision.
efriedma added reviewers: jdoerfert, lebedev.ri, spatel.
Herald added subscribers: cfe-commits, kerbowa, nhaehnle, jvesely.
Herald added a reviewer: bollu.
Herald added a project: clang.

This is equivalent in terms of LLVM IR semantics, but we want to transition 
away from using MaybeAlign to represent the alignment of these instructions.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77984

Files:
  clang/test/CodeGen/arm_neon_intrinsics.c
  llvm/include/llvm/IR/IRBuilder.h
  llvm/test/Bindings/llvm-c/atomics.ll
  llvm/test/Bindings/llvm-c/echo.ll
  llvm/test/Bindings/llvm-c/memops.ll
  llvm/test/CodeGen/AMDGPU/widen_extending_scalar_loads.ll
  llvm/test/Instrumentation/AddressSanitizer/debug-info-alloca.ll
  llvm/test/Instrumentation/SanitizerCoverage/inline-8bit-counters.ll
  llvm/test/Instrumentation/SanitizerCoverage/inline-bool-flag.ll
  llvm/test/Transforms/ArgumentPromotion/dbg.ll
  llvm/test/Transforms/SROA/alignment.ll
  llvm/test/Transforms/SROA/basictest.ll
  llvm/test/Transforms/SROA/preserve-nonnull.ll
  polly/test/Isl/CodeGen/invariant_load_alias_metadata.ll
  polly/test/Isl/CodeGen/non-affine-phi-node-expansion-2.ll
  polly/test/Isl/CodeGen/partial_write_array.ll
  polly/test/Isl/CodeGen/partial_write_impossible_restriction.ll

Index: polly/test/Isl/CodeGen/partial_write_impossible_restriction.ll
===
--- polly/test/Isl/CodeGen/partial_write_impossible_restriction.ll
+++ polly/test/Isl/CodeGen/partial_write_impossible_restriction.ll
@@ -50,10 +50,10 @@
 
 ; CHECK-LABEL: polly.stmt.cond.false:
 ; CHECK: %polly.access..pn2 = getelementptr i32, i32* %.pn, i64 %polly.indvar
-; CHECK: store i32 %cond.in.sroa.speculate.load.cond.false_p_scalar_, i32* %polly.access..pn2, !alias.scope !0, !noalias !2
+; CHECK: store i32 %cond.in.sroa.speculate.load.cond.false_p_scalar_, i32* %polly.access..pn2, align 4, !alias.scope !0, !noalias !2
 ; CHECK: br label %polly.merge
 
 ; CHECK-LABEL: polly.stmt.cond.false11:
 ; CHECK: %polly.access..pn14 = getelementptr i32, i32* %.pn, i64 0
-; CHECK: store i32 %cond.in.sroa.speculate.load.cond.false_p_scalar_13, i32* %polly.access..pn14, !alias.scope !0, !noalias !2
+; CHECK: store i32 %cond.in.sroa.speculate.load.cond.false_p_scalar_13, i32* %polly.access..pn14, align 4, !alias.scope !0, !noalias !2
 ; CHECK: br label %polly.stmt.cond.end15
Index: polly/test/Isl/CodeGen/partial_write_array.ll
===
--- polly/test/Isl/CodeGen/partial_write_array.ll
+++ polly/test/Isl/CodeGen/partial_write_array.ll
@@ -38,7 +38,7 @@
 
 ; CHECK:  polly.stmt.body.Stmt_body_Write0.partial:
 ; CHECK-NEXT:   %polly.access.A = getelementptr double, double* %A, i64 0
-; CHECK-NEXT:   store double 4.20e+01, double* %polly.access.A, !alias.scope !0, !noalias !2
+; CHECK-NEXT:   store double 4.20e+01, double* %polly.access.A, align 8, !alias.scope !0, !noalias !2
 ; CHECK-NEXT:   br label %polly.stmt.body.cont
 
 ; CHECK:  polly.stmt.body.cont:
Index: polly/test/Isl/CodeGen/non-affine-phi-node-expansion-2.ll
===
--- polly/test/Isl/CodeGen/non-affine-phi-node-expansion-2.ll
+++ polly/test/Isl/CodeGen/non-affine-phi-node-expansion-2.ll
@@ -4,7 +4,7 @@
 
 
 ; CHECK: polly.stmt.bb3:   ; preds = %polly.stmt.bb3.entry
-; CHECK:   %tmp6_p_scalar_ = load double, double* %arg1{{[0-9]*}}, !alias.scope !0, !noalias !2
+; CHECK:   %tmp6_p_scalar_ = load double, double* %arg1{{[0-9]*}}, align 8, !alias.scope !0, !noalias !2
 ; CHECK:   %p_tmp7 = fadd double 1.00e+00, %tmp6_p_scalar_
 ; CHECK:   %p_tmp8 = fcmp olt double 1.40e+01, %p_tmp7
 ; CHECK:   br i1 %p_tmp8, label %polly.stmt.bb9, label %polly.stmt.bb10
Index: polly/test/Isl/CodeGen/invariant_load_alias_metadata.ll
===
--- polly/test/Isl/CodeGen/invariant_load_alias_metadata.ll
+++ polly/test/Isl/CodeGen/invariant_load_alias_metadata.ll
@@ -4,7 +4,7 @@
 ; This test case checks whether Polly generates alias metadata in case of
 ; the ublas gemm kernel and polly-invariant-load-hoisting.
 ;
-; CHECK: store float 4.20e+01, float* %polly.access.A.load, !alias.scope !3, !noalias !4
+; CHECK: store float 4.20e+01, float* %polly.access.A.load, align 4, !alias.scope !3, !noalias !4
 ;
 ; CHECK: !0 = distinct !{!0, !1, !"polly.alias.scope.MemRef_A"}
 ; CHECK-NEXT: !1 = distinct !{!1, !"polly.alias.scope.domain"}
Index: llvm/test/Transforms/SROA/preserve-nonnull.ll
===
--- llvm/test/Transforms/SROA/preserve-nonnull.ll
+++ llvm/test/Transforms/SROA/preserve-nonnull.ll
@@ -12,7 +12,7 @@
 ; CHECK-NEXT:%[[A:.*]] = alloca i8*
 ; CHECK-NEXT:%[[V_CAST:.*]] = bitcast i32* 

[PATCH] D77983: clang-tidy doc: add a note for every checker with an autofix

[Eugene Zelenko via Phabricator via cfe-commits]

Eugene.Zelenko added a comment.
Herald added a subscriber: wuzish.

It'll be reasonable to use two spaces indent. At least this is what mostly used 
for code blocks. Options are exceptions, but will be good idea to reformat them 
eventually.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77983



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

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

[Sam Clegg via Phabricator via cfe-commits]

sbc100 added a comment.

I don't have documented plan anywhere.  I simply observed that we have CPUs 
called `mvp` and `generic` and it made sense to start including more things in 
the generic CPU, leaving the `mvp` option open to those who want to be 
conservative.

I don't think its a huge issue that emscripten should have different default 
cpu to llvm itself.   Emscripten already passes flags to llvm and has different 
defaults.   It even has its own target triple.

I confess my main goal here was to be able to encode a configuration which 
represents the "current spec", but its not "bleeding edge". This also 
happens to the CPU that the wasi SDK would want to use as I believe almost all 
the users of wasi-sdk are targeting VMs which include the current spec features.

As a less controversial version of this change I could instead create a new CPU 
called `current` and leave `generic` as is (basically leave it at mvp) until we 
can agree that a features is widespread enough to warrant being part of generic?

Having said that, any*non*-emscripten toolchains that are targeting the web 
already, like emscripten, have to decide which feature flags to pass by 
default.  Its not something they can avoid.   They need to have some opinion.  
I don't think that llvm and emscripten having different opinions on the default 
set of features to target is terrible outcome.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77908



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

[PATCH] D77983: clang-tidy doc: add a note for every checker with an autofix

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru created this revision.
sylvestre.ledru added reviewers: alexfh, aaron.ballman.
Herald added subscribers: Charusso, arphaman, kbarton, nemanjai.
Herald added a project: clang.

Currently, when looking at a checker documentation, we have to go back
to the whole list or look at the sources to figure out if an autofix
is available or not.

Feel free to propose a better wording

Here is the ugly way I used:
LIST=$(grep Yes list.rst|awk '{print $1}'|cut -d\` -f2) 
   
or f in $LIST; do cat /tmp/a.txt >> $f.rst; done

with /tmp/a.txt containing the text


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77983

Files:
  clang-tools-extra/docs/clang-tidy/checks/abseil-duration-addition.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-duration-comparison.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-duration-conversion-cast.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-duration-division.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-duration-factory-float.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-duration-factory-scale.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-duration-subtraction.rst
  
clang-tools-extra/docs/clang-tidy/checks/abseil-duration-unnecessary-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-faster-strsplit-delimiter.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-redundant-strcat-calls.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-str-cat-append.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-string-find-startswith.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-time-comparison.rst
  clang-tools-extra/docs/clang-tidy/checks/abseil-time-subtraction.rst
  
clang-tools-extra/docs/clang-tidy/checks/abseil-upgrade-duration-conversions.rst
  clang-tools-extra/docs/clang-tidy/checks/android-cloexec-accept.rst
  clang-tools-extra/docs/clang-tidy/checks/android-cloexec-creat.rst
  clang-tools-extra/docs/clang-tidy/checks/android-cloexec-dup.rst
  clang-tools-extra/docs/clang-tidy/checks/android-cloexec-pipe.rst
  clang-tools-extra/docs/clang-tidy/checks/boost-use-to-string.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-argument-comment.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-bool-pointer-implicit-conversion.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-copy-constructor-init.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-inaccurate-erase.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-macro-parentheses.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-operator-in-strlen-in-alloc.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-misplaced-pointer-arithmetic-in-alloc.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-move-forwarding-reference.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-not-null-terminated-result.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-parent-virtual-call.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-posix-return.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-reserved-identifier.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-string-constructor.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-string-integer-assignment.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-memset-usage.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-semicolon.rst
  
clang-tools-extra/docs/clang-tidy/checks/bugprone-suspicious-string-compare.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-swapped-arguments.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-terminating-continue.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-unused-raii.rst
  clang-tools-extra/docs/clang-tidy/checks/bugprone-virtual-near-miss.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-dcl03-c.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-dcl16-c.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-dcl37-c.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-dcl51-cpp.rst
  clang-tools-extra/docs/clang-tidy/checks/cert-oop11-cpp.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-explicit-virtual-functions.rst
  clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-init-variables.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-bounds-constant-array-index.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-type-cstyle-cast.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-type-member-init.rst
  
clang-tools-extra/docs/clang-tidy/checks/cppcoreguidelines-pro-type-static-cast-downcast.rst
  clang-tools-extra/docs/clang-tidy/checks/darwin-dispatch-once-nonstatic.rst
  
clang-tools-extra/docs/clang-tidy/checks/fuchsia-default-arguments-declarations.rst
  clang-tools-extra/docs/clang-tidy/checks/google-explicit-constructor.rst
  

[PATCH] D77233: [NFC] Refactoring PropertyAttributeKind for ObjCPropertyDecl and ObjCDeclSpec.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/DeclObjCCommon.h:21
+/// Keep this list in sync with LLVM's Dwarf.h ApplePropertyAttributes.
+enum ObjCPropertyAttributeKind {
+  OBJC_PR_noattr = 0x00,

plotfi wrote:
> compnerd wrote:
> > It seems that you are touching all the sites that use this, so perhaps this 
> > is the time to change this to `enum class` and drop the `OBJC_PR_` prefixes 
> > and explicitly inherit from `uint16_t`.  This should be named 
> > `ObjCPropertyAttribute` I think.
> Ah yeah, that sounds pretty good. Will do. 
Talked offline: update is that its not so easy to change these enums to enum 
classes because of how they are constantly used with unsigned ints. I could try 
and implement lots of operator overloads but I think that could be potentially 
buggy and not so NFC-like. @compnerd you wanted to leave the OBJC_PR_  and 
dropped the DQ prefixes right? Other than that, ping (for others)? I think this 
could be a nice cleanup. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77233



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

[PATCH] D77982: [Windows SEH] Fix the frame-ptr of a nested-filter within a _finally

[Ten Tzen via Phabricator via cfe-commits]

tentzen created this revision.
tentzen added reviewers: rnk, eli.friedman, JosephTremoulet, asmith.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This change fixed a SEH bug (exposed by test58 & test61 in MSVC test xcpt4u.c);
when an Except-filter is located inside a finally, the frame-pointer generated 
today via intrinsic @llvm.eh.recoverfp is the frame-pointer of the immediate 
parent  _finally, not the frame-ptr of outermost host function.

The fix is to retrieve the Establisher's frame-pointer that was previously 
saved in parent's frame.  
The prolog of a filter inside a _finally should be like:

  %0 = call i8* @llvm.eh.recoverfp(i8* bitcast (@"?fin$0@0@main@@"), i8* 
%frame_pointer)
  %1 = call i8* @llvm.localrecover(i8* bitcast (@"?fin$0@0@main@@"), i8* %0, 
i32 0)
  %2 = bitcast i8* %1 to i8**
  %3 = load i8*, i8** %2, align 8


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77982

Files:
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/test/CodeGen/windows-seh-filter-inFinally.c

Index: clang/test/CodeGen/windows-seh-filter-inFinally.c
===
--- /dev/null
+++ clang/test/CodeGen/windows-seh-filter-inFinally.c
@@ -0,0 +1,36 @@
+// RUN: %clang_cc1 -triple x86_64-windows -fms-extensions -Wno-implicit-function-declaration -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: %[[dst:[0-9-]+]] = call i8* @llvm.eh.recoverfp(i8* bitcast (void (i8, i8*)* @"?fin$0@0@main@@" to i8*), i8* %frame_pointer)
+// CHECK-NEXT: %[[dst1:[0-9-]+]] = call i8* @llvm.localrecover(i8* bitcast (void (i8, i8*)* @"?fin$0@0@main@@" to i8*), i8* %[[dst]], i32 0)
+// CHECK-NEXT: %[[dst2:[0-9-]+]] = bitcast i8* %[[dst1]] to i8**
+// CHECK-NEXT: = load i8*, i8** %[[dst2]], align 8
+
+int
+main(int argc, char *argv[])
+{
+int Counter = 0;
+//
+// Try/except within the finally clause of a try/finally.
+//
+__try {
+  Counter -= 1;
+}
+__finally {
+  __try {
+Counter += 2;
+// RtlRaiseStatus(STATUS_INTEGER_OVERFLOW);
+  } __except(Counter) {
+__try {
+  Counter += 3;
+}
+__finally {
+  if (abnormal_termination() == 1) {
+Counter += 5;
+  }
+}
+  }
+}
+// expect Counter == 9
+return 1;
+}
+
Index: clang/lib/CodeGen/CodeGenFunction.h
===
--- clang/lib/CodeGen/CodeGenFunction.h
+++ clang/lib/CodeGen/CodeGenFunction.h
@@ -367,6 +367,9 @@
   CodeGenModule   // Per-module state.
   const TargetInfo 
 
+  // For outliner helper only
+  CodeGenFunction *ParentCGF = nullptr;
+
   typedef std::pair ComplexPairTy;
   LoopInfoStack LoopStack;
   CGBuilderTy Builder;
Index: clang/lib/CodeGen/CGException.cpp
===
--- clang/lib/CodeGen/CGException.cpp
+++ clang/lib/CodeGen/CGException.cpp
@@ -1794,6 +1794,40 @@
 llvm::Constant *ParentI8Fn =
 llvm::ConstantExpr::getBitCast(ParentCGF.CurFn, Int8PtrTy);
 ParentFP = Builder.CreateCall(RecoverFPIntrin, {ParentI8Fn, EntryFP});
+
+// if the parent is a _finally, need to retrive Establisher's FP,
+//  2nd paramenter, saved & named frame_pointer in parent's frame
+if (ParentCGF.ParentCGF != NULL) {
+  // Locate and escape Parent's frame_pointer.addr alloca
+  llvm::AllocaInst *FramePtrAddrAlloca = nullptr;
+  for (llvm::Instruction  : ParentCGF.CurFn->getEntryBlock()) {
+llvm::AllocaInst *II = dyn_cast();
+if (II && II->getName().startswith("frame_pointer")) {
+  FramePtrAddrAlloca = II;
+  break;
+}
+  }
+  assert(FramePtrAddrAlloca);
+  auto InsertPair = ParentCGF.EscapedLocals.insert(
+  std::make_pair(FramePtrAddrAlloca, ParentCGF.EscapedLocals.size()));
+  int FrameEscapeIdx = InsertPair.first->second;
+
+  // an example of a filter's prolog::
+  // %0 = call i8 * @llvm.eh.recoverfp(i8 * bitcast(@"?fin$0@0@main@@"), i8 * %frame_pointer)
+  // %1 = call i8 * @llvm.localrecover(i8 * bitcast(@"?fin$0@0@main@@"), i8 * %0, i32 0)
+  // %2 = bitcast i8 * %1 to i8 **
+  // %3 = load i8*, i8 * *%2, align 8
+  //   ==> %3 is the frame-pointer of outermost host function
+  llvm::Function *FrameRecoverFn = llvm::Intrinsic::getDeclaration(
+  (), llvm::Intrinsic::localrecover);
+  llvm::Constant *ParentI8Fn =
+  llvm::ConstantExpr::getBitCast(ParentCGF.CurFn, Int8PtrTy);
+  ParentFP = Builder.CreateCall(
+  FrameRecoverFn, {ParentI8Fn, ParentFP,
+   llvm::ConstantInt::get(Int32Ty, FrameEscapeIdx)});
+  ParentFP = Builder.CreateBitCast(ParentFP, CGM.VoidPtrPtrTy);
+  ParentFP = Builder.CreateLoad(Address(ParentFP, getPointerAlign()));
+}
   }
 
   // Create llvm.localrecover calls for all captures.
@@ -1992,6 +2026,7 @@
 
 

Re: [clang] 246398e - [clang][Parse] properly parse asm-qualifiers, asm inline

[Joerg Sonnenberger via cfe-commits]

On Thu, Mar 12, 2020 at 03:25:49PM -0700, Nick Desaulniers via cfe-commits 
wrote:
> 
> Author: Nick Desaulniers
> Date: 2020-03-12T15:13:59-07:00
> New Revision: 246398ece7115b57a02dbe7876d86ae8e72406ef
> 
> URL: 
> https://github.com/llvm/llvm-project/commit/246398ece7115b57a02dbe7876d86ae8e72406ef
> DIFF: 
> https://github.com/llvm/llvm-project/commit/246398ece7115b57a02dbe7876d86ae8e72406ef.diff
> 
> LOG: [clang][Parse] properly parse asm-qualifiers, asm inline
> 
> Summary:
> The parsing of GNU C extended asm statements was a little brittle and
> had a few issues:

I find it very questionable that this change drops a warning flag making
IMO benign code differences like marking a global asm as volatile an
unconditional error. This strongly seems to be something that we
shouldn't blindly follow GCC on.

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

[PATCH] D77831: [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

[Nathan James via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG672207c319a0: [clang-tidy] Convert config options that are 
bools to use the bool overload of… (authored by njames93).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77831

Files:
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousStringCompareCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h
  clang-tools-extra/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp
  clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
  
clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
  clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp

Index: clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
@@ -183,7 +183,7 @@
 : ClangTidyCheck(Name, Context),
   NewSuffixes(
   utils::options::parseStringList(Options.get("NewSuffixes", ""))),
-  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
 void UppercaseLiteralSuffixCheck::storeOptions(
 ClangTidyOptions::OptionMap ) {
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -333,13 +333,12 @@
   const MatchFinder::MatchResult 
 };
 
-
 SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  ChainedConditionalReturn(Options.get("ChainedConditionalReturn", 0U)),
+  ChainedConditionalReturn(Options.get("ChainedConditionalReturn", false)),
   ChainedConditionalAssignment(
-  Options.get("ChainedConditionalAssignment", 0U)) {}
+  Options.get("ChainedConditionalAssignment", false)) {}
 
 bool containsBoolLiteral(const Expr *E) {
   if (!E)
Index: clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
@@ -28,7 +28,7 @@
 public:
   RedundantSmartptrGetCheck(StringRef Name, ClangTidyContext *Context)
   : ClangTidyCheck(Name, Context),
-IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
   bool isLanguageVersionSupported(const LangOptions ) const override {
 return LangOpts.CPlusPlus;
   }
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
@@ -25,7 +25,7 @@
   

[clang-tools-extra] 672207c - [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-04-12T23:06:09+01:00
New Revision: 672207c319a06f20dc634bcd21678d5dbbe7a6b9

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

LOG: [clang-tidy] Convert config options that are bools to use the bool 
overload of get(GlobalOrLocal)?

Summary: This was done with a script that looks for calls to 
Options.get(GlobalOrLocal) that take an integer for the second argument and the 
result is either compared not equal to 0 or implicitly converted to bool. There 
may be other occurances

Reviewers: aaron.ballman, alexfh, gribozavr2

Reviewed By: aaron.ballman

Subscribers: wuzish, nemanjai, xazax.hun, kbarton, cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.h
clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
clang-tools-extra/clang-tidy/bugprone/SuspiciousStringCompareCheck.cpp
clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.h
clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp

clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
clang-tools-extra/clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h
clang-tools-extra/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp
clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp

clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp

clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp 
b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
index 4da91d1162ac..5ec4c779d59a 100644
--- a/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
+++ b/clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
@@ -23,16 +23,16 @@ namespace bugprone {
 ArgumentCommentCheck::ArgumentCommentCheck(StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  StrictMode(Options.getLocalOrGlobal("StrictMode", 0) != 0),
-  IgnoreSingleArgument(Options.get("IgnoreSingleArgument", 0) != 0),
-  CommentBoolLiterals(Options.get("CommentBoolLiterals", 0) != 0),
-  CommentIntegerLiterals(Options.get("CommentIntegerLiterals", 0) != 0),
-  CommentFloatLiterals(Options.get("CommentFloatLiterals", 0) != 0),
-  CommentStringLiterals(Options.get("CommentStringLiterals", 0) != 0),
-  CommentUserDefinedLiterals(Options.get("CommentUserDefinedLiterals", 0) 
!=
- 0),
-  CommentCharacterLiterals(Options.get("CommentCharacterLiterals", 0) != 
0),
-  CommentNullPtrs(Options.get("CommentNullPtrs", 0) != 0),
+  StrictMode(Options.getLocalOrGlobal("StrictMode", false)),
+  IgnoreSingleArgument(Options.get("IgnoreSingleArgument", false)),
+  CommentBoolLiterals(Options.get("CommentBoolLiterals", false)),
+  CommentIntegerLiterals(Options.get("CommentIntegerLiterals", false)),
+  CommentFloatLiterals(Options.get("CommentFloatLiterals", false)),
+  CommentStringLiterals(Options.get("CommentStringLiterals", false)),
+  CommentUserDefinedLiterals(
+  Options.get("CommentUserDefinedLiterals", false)),
+  CommentCharacterLiterals(Options.get("CommentCharacterLiterals", false)),
+  CommentNullPtrs(Options.get("CommentNullPtrs", false)),
   IdentRE("^(/\\* 

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

I'm not concerned about the performance implications of whatever approach we 
take here.  In the worst case, an "icmp+zext" corresponds to two extra 
arithmetic instructions; that's not enough to matter.  And I expect usually 
it'll get optimized away.

I'd prefer to avoid exposing our cleanup numbering to user code, if possible.  
The Microsoft documentation says it returns 0 or 1.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77936



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

[PATCH] D33029: [clang-format] add option for dangling parenthesis

[Bhopesh Bassi via Phabricator via cfe-commits]

bbassi added a comment.

@MyDeveloperDay  hey, I am currently working on this, and adding a new option 
called BreakBeforeClosingBracket. I have some questions to understand the 
existing code, they might not be directly linked to this change so I am not 
sure if this the best place to ask those questions. What do you think? e.g. I 
don't understand what FakeLParens is and have some questions about that.


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

https://reviews.llvm.org/D33029



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

[PATCH] D77952: [TLII] Reduce copies of TLII for TLA

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

lgtm with the VecLibDesc references removed.




Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:596
   memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray));
   return *this;
 }

wenlei wrote:
> tejohnson wrote:
> > This is missing copying of the VecLibDescs (looks like I missed this as 
> > well originally).
> Now I remembered why this was missed from my side, before my patch, 
> `VecLibDescs` isn't copied here either, which seems like a bug? Same for the 
> move one below. 
I assume you mean the VectorDescs/ScalarDescs vectors. You are right, even 
before your D77632 patch they were not being copied here or below. Looks like 
there is an existing bug there that no one noticed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77952



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

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Ten Tzen via Phabricator via cfe-commits]

tentzen added a comment.

In D77936#1976938 , @efriedma wrote:

> Instead of asserting there are less than 256 cleanup destinations, can you 
> emit an icmp against zero, or something like that?


I did consider that. but it splits the block and induces one compare and one 
branch in _try exit sequence for something that will not happen in RWC.  And 
it's hard for Optimizer to deal with it.  Optimizer can probably can 
tail-duplicate each path at the expense of code size which I don't think LLVM 
would do today.  Do you really think it's worth to split the block here? thanks




Comment at: clang/lib/CodeGen/CGException.cpp:1651
+  llvm::Value* Load = CGF.Builder.CreateLoad(Addr, "cleanup.dest");
+  IsForEH = CGF.Builder.CreateTrunc(Load, CGM.Int8Ty);
+}

efriedma wrote:
> Is just truncating the value really correct?  What's the possible range of 
> values stored in getNormalCleanupDestSlot()?
Good question! 
Usually the number in NormalCleanupDestSlot is single digit (or at most 
double-digit) , the totoal number of Jump Destinations 
(return/goto/break/continue/..)  in the function.  
I thought about widening 1st arg from CHAR to unsigned, but dropped the idea as 
it seems unnecessary.  Yes, someone can write a code to make more than 255 
Jumps in a function,  but it’s unlikely to happen in real world code.  What do 
you think?
I can go either way..
For sure, we can add an assert to catch it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77936



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

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

Instead of asserting there are less than 256 cleanup destinations, can you emit 
an icmp against zero, or something like that?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77936



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

[PATCH] D77669: [clangd] Update TUStatus test to handle async PreambleThread

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 256875.
kadircet marked 6 inline comments as done.
kadircet added a comment.

- Record actions for each thread separately.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77669

Files:
  clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp

Index: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
===
--- clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
+++ clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp
@@ -8,6 +8,7 @@
 
 #include "Annotations.h"
 #include "Cancellation.h"
+#include "ClangdServer.h"
 #include "Context.h"
 #include "Diagnostics.h"
 #include "Matchers.h"
@@ -829,18 +830,35 @@
   class CaptureTUStatus : public ClangdServer::Callbacks {
   public:
 void onFileUpdated(PathRef File, const TUStatus ) override {
+  auto ASTAction = Status.ASTActivity.K;
+  auto PreambleAction = Status.PreambleActivity;
   std::lock_guard Lock(Mutex);
-  AllStatus.push_back(Status);
+  // Only push the action if it has changed. Since TUStatus can be published
+  // from either Preamble or AST thread and when one changes the other stays
+  // the same.
+  // Note that this can result in missing some updates when something other
+  // than action kind changes, e.g. when AST is built/reused the action kind
+  // stays as Building.
+  if (ASTActions.empty() || ASTActions.back() != ASTAction)
+ASTActions.push_back(ASTAction);
+  if (PreambleActions.empty() || PreambleActions.back() != PreambleAction)
+PreambleActions.push_back(PreambleAction);
 }
 
-std::vector allStatus() {
+std::vector preambleStatuses() {
   std::lock_guard Lock(Mutex);
-  return AllStatus;
+  return PreambleActions;
+}
+
+std::vector astStatuses() {
+  std::lock_guard Lock(Mutex);
+  return ASTActions;
 }
 
   private:
 std::mutex Mutex;
-std::vector AllStatus;
+std::vector ASTActions;
+std::vector PreambleActions;
   } CaptureTUStatus;
   MockFSProvider FS;
   MockCompilationDatabase CDB;
@@ -850,35 +868,39 @@
   // We schedule the following tasks in the queue:
   //   [Update] [GoToDefinition]
   Server.addDocument(testPath("foo.cpp"), Code.code(), "1",
- WantDiagnostics::Yes);
+ WantDiagnostics::Auto);
+  ASSERT_TRUE(Server.blockUntilIdleForTest());
   Server.locateSymbolAt(testPath("foo.cpp"), Code.point(),
 [](Expected> Result) {
   ASSERT_TRUE((bool)Result);
 });
-
   ASSERT_TRUE(Server.blockUntilIdleForTest());
 
-  EXPECT_THAT(CaptureTUStatus.allStatus(),
+  EXPECT_THAT(CaptureTUStatus.preambleStatuses(),
+  ElementsAre(
+  // PreambleThread starts idle, as the update is first handled
+  // by ASTWorker.
+  PreambleAction::Idle,
+  // Then it starts building first preamble and releases that to
+  // ASTWorker.
+  PreambleAction::Building,
+  // Then goes idle and stays that way as we don't receive any
+  // more update requests.
+  PreambleAction::Idle));
+  EXPECT_THAT(CaptureTUStatus.astStatuses(),
   ElementsAre(
-  // Everything starts with ASTWorker starting to execute an
-  // update
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // We build the preamble
-  TUState(PreambleAction::Building, ASTAction::RunningAction),
-  // We built the preamble, and issued ast built on ASTWorker
-  // thread. Preambleworker goes idle afterwards.
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // Start task for building the ast, as a result of building
-  // preamble, on astworker thread.
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // AST build starts.
-  TUState(PreambleAction::Idle, ASTAction::Building),
-  // AST built finished successfully
-  TUState(PreambleAction::Idle, ASTAction::Building),
-  // Running go to def
-  TUState(PreambleAction::Idle, ASTAction::RunningAction),
-  // ASTWorker goes idle.
-  TUState(PreambleAction::Idle, ASTAction::Idle)));
+  // Starts handling the update action and blocks until the
+  // first preamble is built.
+  ASTAction::RunningAction,
+  // Afterwqards it builds an AST for that preamble to publish
+  // diagnostics.
+  ASTAction::Building,
+  // 

[PATCH] D77669: [clangd] Update TUStatus test to handle async PreambleThread

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

going with separately recording each thread. that way we can also test building 
of diagnostics.




Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:849
+  auto Opts = ClangdServer::optsForTest();
+  Opts.AsyncThreadsCount = 0;
+  ClangdServer Server(CDB, FS, Opts, );

sammccall wrote:
> kadircet wrote:
> > sammccall wrote:
> > > This seems unfortunate because it doesn't test the production 
> > > configuration.
> > > 
> > > How many possible sequences are there? Are there fewer if we 
> > > blockuntilidle between adddoc + locatesymbolat?
> > > 
> > > Can we use testing::AnyOf to fudge around the nondeterminism?
> > I didn't want to do that as it was rather asserting on action ordering of 
> > preamble and ast worker threads rather than checking for whether we emit 
> > TUStatuses, but I suppose this test was always trying to assert both.
> > 
> > Introducing some more simplications by turning of diagnostics and also 
> > making use of the assumption that ASTWorker will block for first preamble 
> > build.
> This makes sense, at the risk of being a massive pain do you think we should 
> have a separate test that (only) creates diagnostics?
changed the test to record each thread separately while deduplicating, so we 
can also build the diags now.



Comment at: clang-tools-extra/clangd/unittests/TUSchedulerTests.cpp:891
+  // action.
+  TUState(PreambleAction::Idle, ASTAction::RunningAction),
+  // Builds AST and serves the request, then goes idle.

sammccall wrote:
> Hmm... are you sure the preamble can't still be running at this point? What 
> stops it from pausing forever before exiting?
it has to go idle before stopping, since we block after issuing the update.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77669



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

[PATCH] D77955: [clangd] Add target_info test

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet abandoned this revision.
kadircet added a comment.

landed as 101a69d71b93f901561621508ed36b187e594d91 



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77955



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

[PATCH] D77979: [clang-tidy] modernize-use-using: Fix broken fixit with InjectedClassName

[Matthias Gehre via Phabricator via cfe-commits]

mgehre created this revision.
mgehre added reviewers: aaron.ballman, alexfh, hokein, njames93.
Herald added a subscriber: xazax.hun.
Herald added a project: clang.

Before this PR, `modernize-use-using` would transform the typedef in

  template 
  struct InjectedClassName {
typedef InjectedClassName b;
  };

into `using b = InjectedClassName;` and

  template 
  struct InjectedClassNameWithUnnamedArgument {
typedef InjectedClassNameWithUnnamedArgument b;
  };

into `using b = InjectedClassNameWithUnnamedArgument<>;`.
The first fixit is surprising because its different than the code
before, but the second fixit doesn't even compile.

This PR adds an option to the TypePrinter to print InjectedClassNameType without
template parameters (i.e. as written).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D77979

Files:
  clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
  clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
  clang/include/clang/AST/PrettyPrinter.h
  clang/lib/AST/TypePrinter.cpp


Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1305,7 +1305,12 @@
 
 void TypePrinter::printInjectedClassNameBefore(const InjectedClassNameType *T,
raw_ostream ) {
-  printTemplateSpecializationBefore(T->getInjectedTST(), OS);
+  if (Policy.PrintInjectedClassNameWithArguments)
+return printTemplateSpecializationBefore(T->getInjectedTST(), OS);
+
+  IncludeStrongLifetimeRAII Strong(Policy);
+  T->getTemplateName().print(OS, Policy);
+  spaceBeforePlaceHolder(OS);
 }
 
 void TypePrinter::printInjectedClassNameAfter(const InjectedClassNameType *T,
Index: clang/include/clang/AST/PrettyPrinter.h
===
--- clang/include/clang/AST/PrettyPrinter.h
+++ clang/include/clang/AST/PrettyPrinter.h
@@ -63,7 +63,7 @@
 MSWChar(LO.MicrosoftExt && !LO.WChar), IncludeNewlines(true),
 MSVCFormatting(false), ConstantsAsWritten(false),
 SuppressImplicitBase(false), FullyQualifiedName(false),
-PrintCanonicalTypes(false) {}
+PrintCanonicalTypes(false), PrintInjectedClassNameWithArguments(true) 
{}
 
   /// Adjust this printing policy for cases where it's known that we're
   /// printing C++ code (for instance, if AST dumping reaches a C++-only
@@ -244,6 +244,11 @@
   /// Whether to print types as written or canonically.
   unsigned PrintCanonicalTypes : 1;
 
+  /// Whether to print an InjectedClassNameType with template arguments or as
+  /// written. When a template arguments are unnamed, printing them results in
+  /// invalid C++ code.
+  unsigned PrintInjectedClassNameWithArguments : 1;
+
   /// Callbacks to use to allow the behavior of printing to be customized.
   const PrintingCallbacks *Callbacks = nullptr;
 };
Index: clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
===
--- clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
+++ clang-tools-extra/test/clang-tidy/checkers/modernize-use-using.cpp
@@ -278,3 +278,16 @@
 // CHECK-MESSAGES: :[[@LINE-1]]:1: warning: use 'using' instead of 'typedef'
 // CHECK-FIXES: using EnumT2_CheckTypedefImpactFromAnotherFile = enum { ea2, 
eb2 };
 
+template 
+struct InjectedClassName {
+  typedef InjectedClassName b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
+  // CHECK-FIXES: using b = InjectedClassName;
+};
+
+template 
+struct InjectedClassNameWithUnnamedArgument {
+  typedef InjectedClassNameWithUnnamedArgument b;
+  // CHECK-MESSAGES: :[[@LINE-1]]:3: warning: use 'using' instead of 'typedef'
+  // CHECK-FIXES: using b = InjectedClassNameWithUnnamedArgument;
+};
Index: clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
===
--- clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
+++ clang-tools-extra/clang-tidy/modernize/UseUsingCheck.cpp
@@ -58,6 +58,7 @@
   printPolicy.SuppressScope = true;
   printPolicy.ConstantArraySizeAsWritten = true;
   printPolicy.UseVoidForZeroParams = false;
+  printPolicy.PrintInjectedClassNameWithArguments = false;
 
   std::string Type = MatchedDecl->getUnderlyingType().getAsString(printPolicy);
   std::string Name = MatchedDecl->getNameAsString();


Index: clang/lib/AST/TypePrinter.cpp
===
--- clang/lib/AST/TypePrinter.cpp
+++ clang/lib/AST/TypePrinter.cpp
@@ -1305,7 +1305,12 @@
 
 void TypePrinter::printInjectedClassNameBefore(const InjectedClassNameType *T,
raw_ostream ) {
-  printTemplateSpecializationBefore(T->getInjectedTST(), OS);
+  if (Policy.PrintInjectedClassNameWithArguments)
+return 

[PATCH] D77831: [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM, thanks for the cleanup!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77831



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

[PATCH] D77836: [Attribute] Fix noderef attribute false-negatives

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/test/Frontend/noderef.c:211
+
+int *implicit_cast(NODEREF int *x) {
+  return (int *)x; // expected-warning{{casting to dereferenceable pointer 
removes 'noderef' attribute}}

The name of the function is a bit odd given that there's an explicit cast.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77836



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

[PATCH] D69171: [clang-fuzzer] Add new fuzzer target for Objective-C

[Simon Pilgrim via Phabricator via cfe-commits]

RKSimon added inline comments.



Comment at: clang/tools/clang-fuzzer/ClangObjectiveCFuzzer.cpp:19
 
-extern "C" int LLVMFuzzerInitialize(int *argc, char ***argv) { return 0; }
-

@dgoldman @morehouse Removing this has been causing link failures ever since it 
was committed on a number of targets including msvc (PR44414) and I've just 
noticed it on a linux release+asserts build. If it is superfluous why did you 
leave it in ClangFuzzer.cpp ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69171



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

[PATCH] D77908: [WebAssembly] Enable nontrapping-fptoint for `default` cpu

[Alon Zakai via Phabricator via cfe-commits]

kripken added a comment.

Is the general plan for LLVM documented somewhere?

It's not obvious to me why something being in the wasm spec means it should be 
enabled by default in LLVM. (It's also not obvious to me that is wrong! I'm 
just not sure what the reasoning is here.)

In particular, something being in the wasm spec doesn't mean it is widespread 
yet. I see https://github.com/emscripten-core/emscripten/pull/10885 proposes to 
diverge the emscripten defaults from LLVMs. That seems odd to me - why 
shouldn't those be the same? If they aren't the same it's a potential source of 
confusion in multiple ways. For example, if the reasoning is "LLVM moves with 
the spec, emscripten moves with the web" then that means *non*-emscripten 
toolchains targeting the web have more work to do. Also comparisons between 
toolchains will get harder to get apples-to-apples.

Again, not opposed to this, just not sure what the big picture is.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77908



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

[PATCH] D77952: [TLII] Reduce copies of TLII for TLA

[Wenlei He via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/BackendUtil.cpp:689
   // Set up the per-function pass manager.
-  FPM.add(new TargetLibraryInfoWrapperPass(*TLII));
+  FPM.add(new TargetLibraryInfoWrapperPass(TargetTriple));
   if (CodeGenOpts.VerifyModule)

tejohnson wrote:
> wenlei wrote:
> > tejohnson wrote:
> > > These changes mean we now construct a new TLII multiple times (e.g. both 
> > > when we add the TargetLibraryInfoWrapperPass to the MPM earlier and to 
> > > the FPM here, rather that just copying. Is this actually faster? It seems 
> > > like it would be slower overall.
> > Oops, this one isn't intentional... changed it back. Though for other 
> > instances where TLII isn't reused, similar change turns extra copy into 
> > move.
> I suppose you could std::move the TLII here to avoid this second copy.
> 
> Do you know how much difference this patch makes on the compile time 
> instruction count regressions seen with the original patch? It seems like it 
> shouldn't be huge given that this is just during the pipeline setup for the 
> module. But if it does explain the instruction count increases that's 
> probably why it didn't regress the actual wall time measurements.
Yeah, the 2nd use can be a move, though I'm inclined to not do that, as TLII 
isn't immediately out of scope yet.

I didn't setup measurement for this. I was basically playing with some tests 
for sanity check just to make sure we're not doing things unexpectedly, e.g. we 
don't do per-function copies unexpectedly in `TargetLibraryAnalysis::run`. But 
other than a few extra copies on setup path, I didn't see anything unusual. 
Since the original patch can make any copies more expensive, I thought it's 
good to reduce that as I see it.



Comment at: llvm/lib/Analysis/TargetLibraryInfo.cpp:596
   memcpy(AvailableArray, TLI.AvailableArray, sizeof(AvailableArray));
   return *this;
 }

tejohnson wrote:
> This is missing copying of the VecLibDescs (looks like I missed this as well 
> originally).
Now I remembered why this was missed from my side, before my patch, 
`VecLibDescs` isn't copied here either, which seems like a bug? Same for the 
move one below. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77952



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

[PATCH] D77952: [TLII] Reduce copies of TLII for TLA

[Wenlei He via Phabricator via cfe-commits]

wenlei added a comment.

In D77952#1976803 , @tejohnson wrote:

> Also, just a nit, because TLI is sometimes used to refer to the 
> TargetLibraryInfo and occasionally to the TargetLibraryInfoImpl, and the 
> latter is frequently referred to as TLII, could you change the description to 
> say TLII or TLI Impl? Since this doesn't affect TargetLibraryInfo/TLI object 
> copies.


Sure, done.. changed the title, and made the description more explicit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77952



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

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added a comment.

Also, just a nit, because TLI is sometimes used to refer to the 
TargetLibraryInfo and occasionally to the TargetLibraryInfoImpl, and the latter 
is frequently referred to as TLII, could you change the description to say TLII 
or TLI Impl? Since this doesn't affect TargetLibraryInfo/TLI object copies.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77952



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

[PATCH] D77952: [TLI] Reduce copies for TLI and TLA

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:689
   // Set up the per-function pass manager.
-  FPM.add(new TargetLibraryInfoWrapperPass(*TLII));
+  FPM.add(new TargetLibraryInfoWrapperPass(TargetTriple));
   if (CodeGenOpts.VerifyModule)

wenlei wrote:
> tejohnson wrote:
> > These changes mean we now construct a new TLII multiple times (e.g. both 
> > when we add the TargetLibraryInfoWrapperPass to the MPM earlier and to the 
> > FPM here, rather that just copying. Is this actually faster? It seems like 
> > it would be slower overall.
> Oops, this one isn't intentional... changed it back. Though for other 
> instances where TLII isn't reused, similar change turns extra copy into move.
I suppose you could std::move the TLII here to avoid this second copy.

Do you know how much difference this patch makes on the compile time 
instruction count regressions seen with the original patch? It seems like it 
shouldn't be huge given that this is just during the pipeline setup for the 
module. But if it does explain the instruction count increases that's probably 
why it didn't regress the actual wall time measurements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77952



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

[clang-tools-extra] 101a69d - [clangd] Reland target_info_test

[Kadir Cetinkaya via cfe-commits]

Author: Kadir Cetinkaya
Date: 2020-04-12T16:10:04+02:00
New Revision: 101a69d71b93f901561621508ed36b187e594d91

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

LOG: [clangd] Reland target_info_test

Added: 
clang-tools-extra/clangd/test/target_info.test

Modified: 


Removed: 




diff  --git a/clang-tools-extra/clangd/test/target_info.test 
b/clang-tools-extra/clangd/test/target_info.test
new file mode 100644
index ..6017dbd321fa
--- /dev/null
+++ b/clang-tools-extra/clangd/test/target_info.test
@@ -0,0 +1,35 @@
+# REQUIRES: arm-registered-target
+# Mock 'compile_commands.json' to contain a driver name targeting armv7.
+# Afterwards check that correct target is passed into clang.
+
+# RUN: rm -rf %t.dir && mkdir -p %t.dir
+
+# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/armv7-clang -x c++ 
the-file.cpp -v", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+
+# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
+
+# RUN: clangd -lit-test < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{
+  "jsonrpc":"2.0",
+  "method":"textDocument/didOpen",
+  "params": {
+"textDocument": {
+  "uri": "file://INPUT_DIR/the-file.cpp",
+  "languageId":"cpp",
+  "version":1,
+  "text":""
+}
+  }
+}
+# Make sure we have target passed into cc1 driver, which is printed due to -v 
in
+# the compile_commands.json
+# CHECK: Target: armv7
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}



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

[PATCH] D76496: [clang-tidy] StringFindStartswith should ignore 3-arg find()

[Nathan James via Phabricator via cfe-commits]

njames93 added a comment.

In D76496#1949851 , @niko wrote:

> Correct me if I'm wrong, but that seems to be in violation of IWYU? Maybe I'm 
> misreading this, or is the idea that higher-lever tooling (e.g. IWYU fixer) 
> is supposed to address that?


It is but it sort of gets a pass as the absl/strings/match.h has to include it. 
Similar to how if you `#include `, you wouldn't then `#include 
` so you could construct a `std::vector` using a 
`std::initializer_list`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D76496



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

[PATCH] D77732: [clangd] Shard preamble symbols in dynamic index

[Kadir Cetinkaya via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/index/FileIndex.cpp:97
+/// paths used by \p FileSyms.
+void shardSlabToFiles(const SymbolSlab , const RelationSlab ,
+  FileSymbols , llvm::StringRef HintPath) {

sammccall wrote:
> This screams out to be shared with BackgroundIndex::update(). Is it very hard 
> to do?
> 
> Things that are different:
>  - no duplication of symbol into defining file (why?)
>  - refs not gathered here (but I guess empty input -> empty output?)
>  - include graph not gathered here (same)
>  - this holds 3 copies of all the data at peak, other impl holds 2
> 
> I imagine factoring out a function that returns a StringMap like in 
> BackgroundIndex::Update, but File has a build() function that returns an 
> IndexFileIn or so.
> (Ooh, is depending on IndexFileIn a layering violation? Maybe we should move 
> that into another header. Duplicating the struct is probably OK for now)
In backgroundindex we "prefilter" symbols and only duplicate the ones coming 
from modified files, whereas in this one we duplicate every symbol no matter 
what. I didn't want to change backgroundindex into post filtering mode (I 
suppose that's what you mean by the last item).

as for the first bullet point, duplicating in case of preamble symbols wouldn't 
matter since we don't merge(rather pick one) while building the index for 
preamblesymbols. I suppose storing them twice shouldn't be a huge issue.

Depending to `IndexFileIn` in "Index.h" would be a violation, but depending on 
it in "FileIndex.h" is fine.

I am happy to refactor a `StringMap shardIndexToFiles(IndexFileIn, 
HintPath)` into "FileIndex.h" that can be used by both backgroundindex and 
this, if you are OK with above mentioned regressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77732



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

[PATCH] D77882: [clang-tidy] Add option to use alpha checkers from clang-analyzer when using `run-clang-tidy.py`

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

LGTM! thanks


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77882



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

[PATCH] D77831: [clang-tidy] Convert config options that are bools to use the bool overload of get(GlobalOrLocal)?

[Nathan James via Phabricator via cfe-commits]

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

Added a few more cases by hand


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77831

Files:
  clang-tools-extra/clang-tidy/bugprone/ArgumentCommentCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/NotNullTerminatedResultCheck.h
  clang-tools-extra/clang-tidy/bugprone/SizeofExpressionCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/StringConstructorCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousEnumUsageCheck.cpp
  clang-tools-extra/clang-tidy/bugprone/SuspiciousStringCompareCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/MacroUsageCheck.h
  clang-tools-extra/clang-tidy/cppcoreguidelines/NarrowingConversionsCheck.cpp
  clang-tools-extra/clang-tidy/cppcoreguidelines/SpecialMemberFunctionsCheck.cpp
  clang-tools-extra/clang-tidy/hicpp/MultiwayPathsCoveredCheck.h
  clang-tools-extra/clang-tidy/misc/UnusedParametersCheck.cpp
  clang-tools-extra/clang-tidy/modernize/AvoidBindCheck.cpp
  clang-tools-extra/clang-tidy/modernize/PassByValueCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseAutoCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseDefaultMemberInitCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEmplaceCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEqualsDefaultCheck.cpp
  clang-tools-extra/clang-tidy/modernize/UseEqualsDeleteCheck.h
  clang-tools-extra/clang-tidy/modernize/UseTransparentFunctorsCheck.cpp
  clang-tools-extra/clang-tidy/performance/ForRangeCopyCheck.cpp
  
clang-tools-extra/clang-tidy/performance/InefficientStringConcatenationCheck.cpp
  clang-tools-extra/clang-tidy/portability/SIMDIntrinsicsCheck.cpp
  clang-tools-extra/clang-tidy/readability/IdentifierNamingCheck.cpp
  
clang-tools-extra/clang-tidy/readability/InconsistentDeclarationParameterNameCheck.h
  clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
  clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
  clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
  clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp

Index: clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/UppercaseLiteralSuffixCheck.cpp
@@ -183,7 +183,7 @@
 : ClangTidyCheck(Name, Context),
   NewSuffixes(
   utils::options::parseStringList(Options.get("NewSuffixes", ""))),
-  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+  IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
 void UppercaseLiteralSuffixCheck::storeOptions(
 ClangTidyOptions::OptionMap ) {
Index: clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
===
--- clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
+++ clang-tools-extra/clang-tidy/readability/SimplifyBooleanExprCheck.cpp
@@ -333,13 +333,12 @@
   const MatchFinder::MatchResult 
 };
 
-
 SimplifyBooleanExprCheck::SimplifyBooleanExprCheck(StringRef Name,
ClangTidyContext *Context)
 : ClangTidyCheck(Name, Context),
-  ChainedConditionalReturn(Options.get("ChainedConditionalReturn", 0U)),
+  ChainedConditionalReturn(Options.get("ChainedConditionalReturn", false)),
   ChainedConditionalAssignment(
-  Options.get("ChainedConditionalAssignment", 0U)) {}
+  Options.get("ChainedConditionalAssignment", false)) {}
 
 bool containsBoolLiteral(const Expr *E) {
   if (!E)
Index: clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantSmartptrGetCheck.h
@@ -28,7 +28,7 @@
 public:
   RedundantSmartptrGetCheck(StringRef Name, ClangTidyContext *Context)
   : ClangTidyCheck(Name, Context),
-IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", 1) != 0) {}
+IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
   bool isLanguageVersionSupported(const LangOptions ) const override {
 return LangOpts.CPlusPlus;
   }
Index: clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
===
--- clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
+++ clang-tools-extra/clang-tidy/readability/RedundantMemberInitCheck.h
@@ -25,7 +25,7 @@
   RedundantMemberInitCheck(StringRef Name, ClangTidyContext *Context)
   : ClangTidyCheck(Name, Context),
 

[PATCH] D77947: [clangd] Send the correct error code when cancelling requests.

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

Thanks for remembering this :D LGTM!




Comment at: clang-tools-extra/clangd/JSONTransport.cpp:28
+  [&](const CancelledError ) -> llvm::Error {
+switch (C.Reason) {
+  case static_cast(ErrorCode::ContentModified):

Maybe `static_cast(C.Reason)` instead of casting cases.

I know the reason is not necessarily LSP specific, but we seem to be defaulting 
to `ErrorCode::RequestCancelled` anyways.



Comment at: clang-tools-extra/clangd/JSONTransport.cpp:29
+switch (C.Reason) {
+  case static_cast(ErrorCode::ContentModified):
+Code = ErrorCode::ContentModified;

please fix


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77947



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

[PATCH] D71524: [analyzer] Support tainted objects in GenericTaintChecker

[Borsik Gábor via Phabricator via cfe-commits]

boga95 updated this revision to Diff 256839.

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

https://reviews.llvm.org/D71524

Files:
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  clang/lib/StaticAnalyzer/Checkers/Taint.cpp
  clang/test/Analysis/Inputs/system-header-simulator-cxx.h
  clang/test/Analysis/diagnostics/explicit-suppression.cpp
  clang/test/Analysis/taint-generic.cpp

Index: clang/test/Analysis/taint-generic.cpp
===
--- clang/test/Analysis/taint-generic.cpp
+++ clang/test/Analysis/taint-generic.cpp
@@ -1,5 +1,7 @@
 // RUN: %clang_analyze_cc1  -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -analyzer-config alpha.security.taint.TaintPropagation:Config=%S/Inputs/taint-generic-config.yaml -Wno-format-security -verify -std=c++11 %s
 
+#include "Inputs/system-header-simulator-cxx.h"
+
 #define BUFSIZE 10
 int Buffer[BUFSIZE];
 
@@ -124,3 +126,64 @@
   foo.myMemberScanf("%d", );
   Buffer[x] = 1; // expected-warning {{Out of bound memory access }}
 }
+
+// Test I/O
+void testCin() {
+  int x, y;
+  std::cin >> x >> y;
+  Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void testWcin() {
+  int x, y;
+  std::wcin >> x >> y;
+  Buffer[y] = 1; // expected-warning {{Out of bound memory access }}
+}
+
+void mySink(const std::string &, int, const char *);
+
+void testFstream() {
+  std::string str;
+  std::ifstream file("example.txt");
+  file >> str;
+  mySink(str, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testIfstream() {
+  std::string str;
+  std::fstream file("example.txt");
+  file >> str;
+  mySink(str, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testStdstring() {
+  std::string str1;
+  std::cin >> str1;
+
+  std::string  = str1;
+  mySink(str2, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+
+  const std::string  = str1;
+  mySink(str3, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testTaintedThis() {
+  std::string str;
+  mySink(std::string(), 0, str.c_str()); // no-warning
+
+  std::cin >> str;
+  mySink(std::string(), 0, str.c_str()); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testOverloadedAssignmentOp() {
+  std::string str1, str2;
+  std::cin >> str1;
+  str2 = str1;
+  mySink(str2, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
+
+void testGetline() {
+  std::string str;
+  std::getline(std::cin, str);
+  mySink(str, 0, ""); // expected-warning {{Untrusted data is passed to a user-defined sink}}
+}
Index: clang/test/Analysis/diagnostics/explicit-suppression.cpp
===
--- clang/test/Analysis/diagnostics/explicit-suppression.cpp
+++ clang/test/Analysis/diagnostics/explicit-suppression.cpp
@@ -19,6 +19,6 @@
 void testCopyNull(C *I, C *E) {
   std::copy(I, E, (C *)0);
 #ifndef SUPPRESSED
-  // expected-warning@../Inputs/system-header-simulator-cxx.h:699 {{Called C++ object pointer is null}}
+  // expected-warning@../Inputs/system-header-simulator-cxx.h:742 {{Called C++ object pointer is null}}
 #endif
 }
Index: clang/test/Analysis/Inputs/system-header-simulator-cxx.h
===
--- clang/test/Analysis/Inputs/system-header-simulator-cxx.h
+++ clang/test/Analysis/Inputs/system-header-simulator-cxx.h
@@ -579,6 +579,9 @@
 void resize(size_type count);
 void shrink_to_fit();
 void swap(basic_string );
+  private:
+unsigned size;
+char *ptr;
   };
 
   typedef basic_string string;
@@ -588,6 +591,46 @@
   typedef basic_string u32string;
 #endif
 
+  template 
+  class char_traits {};
+
+  class ios_base {};
+
+  template  >
+  class basic_ios : public ios_base {};
+
+  template  >
+  class basic_istream : virtual public basic_ios {};
+  template  >
+  class basic_ostream : virtual public basic_ios {};
+
+  using istream = basic_istream;
+  using ostream = basic_ostream;
+  using wistream = basic_istream;
+
+  istream >>(istream , int );
+  wistream >>(wistream , int );
+
+  extern istream cin;
+  extern istream wcin;
+
+  template  >
+  class basic_fstream : public basic_istream, public basic_ostream {
+  public:
+basic_fstream(const char *) {}
+  };
+  template  >
+  class basic_ifstream : public basic_istream {
+  public:
+basic_ifstream(const char *) {}
+  };
+
+  using ifstream = basic_ifstream;
+  using fstream = basic_ifstream;
+
+  istream >>(istream , string );
+  istream (istream , string );
+
   class exception {
   public:
 exception() throw();
Index: clang/lib/StaticAnalyzer/Checkers/Taint.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/Taint.cpp
+++ 

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Ten Tzen via Phabricator via cfe-commits]

tentzen updated this revision to Diff 256840.
tentzen marked an inline comment as done.
tentzen added a comment.

Add option  -fms-extensions in test case


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77936

Files:
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/EHScopeStack.h
  clang/test/CodeGen/windows-seh-abnormal-exits.c

Index: clang/test/CodeGen/windows-seh-abnormal-exits.c
===
--- /dev/null
+++ clang/test/CodeGen/windows-seh-abnormal-exits.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-windows -fms-extensions -Wno-implicit-function-declaration -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: %[[src:[0-9-]+]] = call i8* @llvm.localaddress()
+// CHECK-NEXT: %cleanup.dest = load i32, i32* %cleanup.dest.slot, align 4
+// CHECK-NEXT: %[[src2:[0-9-]+]] = trunc i32 %cleanup.dest to i8
+// CHECK-NEXT: call void @"?fin$0@0@seh_abnormal_exits@@"(i8 %[[src2]], i8* %[[src]])
+
+void seh_abnormal_exits(int *Counter) {
+  for (int i = 0; i < 5; i++) {
+__try {
+  if (i == 0)
+continue;   // abnormal termination
+  else if (i == 1)
+goto t10;   // abnormal termination
+  else if (i == 2)
+__leave;  // normal execution
+  else if (i == 4)
+return;  // abnormal termination
+}
+__finally {
+  if (AbnormalTermination()) {
+*Counter += 1;
+  }
+}
+  t10:;
+  }
+  return; // *Counter == 3
+}
+
Index: clang/lib/CodeGen/EHScopeStack.h
===
--- clang/lib/CodeGen/EHScopeStack.h
+++ clang/lib/CodeGen/EHScopeStack.h
@@ -158,9 +158,10 @@
 /// Generation flags.
 class Flags {
   enum {
-F_IsForEH = 0x1,
+F_IsForEH = 0x1,
 F_IsNormalCleanupKind = 0x2,
-F_IsEHCleanupKind = 0x4
+F_IsEHCleanupKind = 0x4,
+F_HasSehAbnormalExits = 0x8,
   };
   unsigned flags;
 
@@ -179,8 +180,10 @@
   /// cleanup.
   bool isEHCleanupKind() const { return flags & F_IsEHCleanupKind; }
   void setIsEHCleanupKind() { flags |= F_IsEHCleanupKind; }
-};
 
+  bool hasSehAbnormalExits() const { return flags & F_HasSehAbnormalExits; }
+  void setHasSehAbnormalExits() { flags |= F_HasSehAbnormalExits; }
+};
 
 /// Emit the cleanup.  For normal cleanups, this is run in the
 /// same EH context as when the cleanup was pushed, i.e. the
Index: clang/lib/CodeGen/CGException.cpp
===
--- clang/lib/CodeGen/CGException.cpp
+++ clang/lib/CodeGen/CGException.cpp
@@ -1639,6 +1639,19 @@
 
 llvm::Value *IsForEH =
 llvm::ConstantInt::get(CGF.ConvertType(ArgTys[0]), F.isForEHCleanup());
+
+// Except _leave and fall-through at the end, all other exits in a _try
+//   (return/goto/continue/break) are considered as abnormal terminations
+//   since _leave/fall-through is always Indexed 0,
+//   just use NormalCleanupDestSlot (>= 1 for goto/return/..),
+//   as 1st Arg to indicate abnormal termination
+if (!F.isForEHCleanup() && F.hasSehAbnormalExits()) {
+  assert(CGF.NextCleanupDestIndex < 256 && " JumpDest Index exceeds 255");
+  Address Addr = CGF.getNormalCleanupDestSlot();
+  llvm::Value* Load = CGF.Builder.CreateLoad(Addr, "cleanup.dest");
+  IsForEH = CGF.Builder.CreateTrunc(Load, CGM.Int8Ty);
+}
+
 Args.add(RValue::get(IsForEH), ArgTys[0]);
 Args.add(RValue::get(FP), ArgTys[1]);
 
Index: clang/lib/CodeGen/CGCleanup.cpp
===
--- clang/lib/CodeGen/CGCleanup.cpp
+++ clang/lib/CodeGen/CGCleanup.cpp
@@ -860,6 +860,9 @@
 // TODO: base this on the number of branch-afters and fixups
 const unsigned SwitchCapacity = 10;
 
+// pass the abnormal exit flag to Fn (SEH cleanup)
+cleanupFlags.setHasSehAbnormalExits();
+
 llvm::LoadInst *Load =
   createLoadInstBefore(getNormalCleanupDestSlot(), "cleanup.dest",
nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77955: [clangd] Add target_info test

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet updated this revision to Diff 256841.
kadircet added a comment.

- Only test for arm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77955

Files:
  clang-tools-extra/clangd/test/target_info.test


Index: clang-tools-extra/clangd/test/target_info.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/target_info.test
@@ -0,0 +1,35 @@
+# REQUIRES: arm-registered-target
+# Mock 'compile_commands.json' to contain a driver name targeting armv7.
+# Afterwards check that correct target is passed into clang.
+
+# RUN: rm -rf %t.dir && mkdir -p %t.dir
+
+# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/armv7-clang -x c++ 
the-file.cpp -v", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+
+# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
+
+# RUN: clangd -lit-test < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{
+  "jsonrpc":"2.0",
+  "method":"textDocument/didOpen",
+  "params": {
+"textDocument": {
+  "uri": "file://INPUT_DIR/the-file.cpp",
+  "languageId":"cpp",
+  "version":1,
+  "text":""
+}
+  }
+}
+# Make sure we have target passed into cc1 driver, which is printed due to -v 
in
+# the compile_commands.json
+# CHECK: Target: armv7
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}


Index: clang-tools-extra/clangd/test/target_info.test
===
--- /dev/null
+++ clang-tools-extra/clangd/test/target_info.test
@@ -0,0 +1,35 @@
+# REQUIRES: arm-registered-target
+# Mock 'compile_commands.json' to contain a driver name targeting armv7.
+# Afterwards check that correct target is passed into clang.
+
+# RUN: rm -rf %t.dir && mkdir -p %t.dir
+
+# RUN: echo '[{"directory": "%/t.dir", "command": "%/t.dir/armv7-clang -x c++ the-file.cpp -v", "file": "the-file.cpp"}]' > %t.dir/compile_commands.json
+
+# RUN: sed -e "s|INPUT_DIR|%/t.dir|g" %s > %t.test.1
+# On Windows, we need the URI in didOpen to look like "uri":"file:///C:/..."
+# (with the extra slash in the front), so we add it here.
+# RUN: sed -E -e "s|file://([A-Z]):/|file:///\1:/|g" %t.test.1 > %t.test
+
+# RUN: clangd -lit-test < %t.test 2>&1 | FileCheck -strict-whitespace %t.test
+{"jsonrpc":"2.0","id":0,"method":"initialize","params":{}}
+---
+{
+  "jsonrpc":"2.0",
+  "method":"textDocument/didOpen",
+  "params": {
+"textDocument": {
+  "uri": "file://INPUT_DIR/the-file.cpp",
+  "languageId":"cpp",
+  "version":1,
+  "text":""
+}
+  }
+}
+# Make sure we have target passed into cc1 driver, which is printed due to -v in
+# the compile_commands.json
+# CHECK: Target: armv7
+---
+{"jsonrpc":"2.0","id":1,"method":"shutdown"}
+---
+{"jsonrpc":"2.0","method":"exit"}
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D77936: [Windows SEH] Fix abnormal-exits in _try

[Ten Tzen via Phabricator via cfe-commits]

tentzen updated this revision to Diff 256836.
tentzen marked an inline comment as done.
tentzen added a comment.

Per Eli's feedbacks:
(1) a test case (windows-seh-abnormal-exit.c) is added under clang\test\CodeGen 
directory.  
(2) an assert is added to catch the extremely rare case that the number of 
JumpDests in a function is greater than 255.  The max. number of JumpDests is 1 
(return) + 2*M + N, where M is the number of for/while-loops and N is the 
number of Goto targets.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D77936

Files:
  clang/lib/CodeGen/CGCleanup.cpp
  clang/lib/CodeGen/CGException.cpp
  clang/lib/CodeGen/EHScopeStack.h
  clang/test/CodeGen/windows-seh-abnormal-exits.c

Index: clang/test/CodeGen/windows-seh-abnormal-exits.c
===
--- /dev/null
+++ clang/test/CodeGen/windows-seh-abnormal-exits.c
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-windows -Wno-implicit-function-declaraton -S -emit-llvm %s -o - | FileCheck %s
+
+// CHECK: %[[src:[0-9-]+]] = call i8* @llvm.localaddress()
+// CHECK-NEXT: %cleanup.dest = load i32, i32* %cleanup.dest.slot, align 4
+// CHECK-NEXT: %[[src2:[0-9-]+]] = trunc i32 %cleanup.dest to i8
+// CHECK-NEXT: call void @"?fin$0@0@seh_abnormal_exits@@"(i8 %[[src2]], i8* %[[src]])
+
+void seh_abnormal_exits(int *Counter) {
+  for (int i = 0; i < 5; i++) {
+__try {
+  if (i == 0)
+continue;   // abnormal termination
+  else if (i == 1)
+goto t10;   // abnormal termination
+  else if (i == 2)
+__leave;  // normal execution
+  else if (i == 4)
+return;  // abnormal termination
+}
+__finally {
+  if (AbnormalTermination()) {
+*Counter += 1;
+  }
+}
+  t10:;
+  }
+  return; // *Counter == 3
+}
+
Index: clang/lib/CodeGen/EHScopeStack.h
===
--- clang/lib/CodeGen/EHScopeStack.h
+++ clang/lib/CodeGen/EHScopeStack.h
@@ -158,9 +158,10 @@
 /// Generation flags.
 class Flags {
   enum {
-F_IsForEH = 0x1,
+F_IsForEH = 0x1,
 F_IsNormalCleanupKind = 0x2,
-F_IsEHCleanupKind = 0x4
+F_IsEHCleanupKind = 0x4,
+F_HasSehAbnormalExits = 0x8,
   };
   unsigned flags;
 
@@ -179,8 +180,10 @@
   /// cleanup.
   bool isEHCleanupKind() const { return flags & F_IsEHCleanupKind; }
   void setIsEHCleanupKind() { flags |= F_IsEHCleanupKind; }
-};
 
+  bool hasSehAbnormalExits() const { return flags & F_HasSehAbnormalExits; }
+  void setHasSehAbnormalExits() { flags |= F_HasSehAbnormalExits; }
+};
 
 /// Emit the cleanup.  For normal cleanups, this is run in the
 /// same EH context as when the cleanup was pushed, i.e. the
Index: clang/lib/CodeGen/CGException.cpp
===
--- clang/lib/CodeGen/CGException.cpp
+++ clang/lib/CodeGen/CGException.cpp
@@ -1639,6 +1639,19 @@
 
 llvm::Value *IsForEH =
 llvm::ConstantInt::get(CGF.ConvertType(ArgTys[0]), F.isForEHCleanup());
+
+// Except _leave and fall-through at the end, all other exits in a _try
+//   (return/goto/continue/break) are considered as abnormal terminations
+//   since _leave/fall-through is always Indexed 0,
+//   just use NormalCleanupDestSlot (>= 1 for goto/return/..),
+//   as 1st Arg to indicate abnormal termination
+if (!F.isForEHCleanup() && F.hasSehAbnormalExits()) {
+  assert(CGF.NextCleanupDestIndex < 256 && " JumpDest Index exceeds 255");
+  Address Addr = CGF.getNormalCleanupDestSlot();
+  llvm::Value* Load = CGF.Builder.CreateLoad(Addr, "cleanup.dest");
+  IsForEH = CGF.Builder.CreateTrunc(Load, CGM.Int8Ty);
+}
+
 Args.add(RValue::get(IsForEH), ArgTys[0]);
 Args.add(RValue::get(FP), ArgTys[1]);
 
Index: clang/lib/CodeGen/CGCleanup.cpp
===
--- clang/lib/CodeGen/CGCleanup.cpp
+++ clang/lib/CodeGen/CGCleanup.cpp
@@ -860,6 +860,9 @@
 // TODO: base this on the number of branch-afters and fixups
 const unsigned SwitchCapacity = 10;
 
+// pass the abnormal exit flag to Fn (SEH cleanup)
+cleanupFlags.setHasSehAbnormalExits();
+
 llvm::LoadInst *Load =
   createLoadInstBefore(getNormalCleanupDestSlot(), "cleanup.dest",
nullptr);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits