[PATCH] D70157: Align branches within 32-Byte boundary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a comment.

In D70157#1772019 , @reames wrote:

> In D70157#1771841 , @jyknight wrote:
>
> > In D70157#1771832 , @reames wrote:
> >
> > > I've been digging through the code for this for the last day or so.  This 
> > > is a new area for me, so it's possible I'm off base, but I have some 
> > > concerns about the current design.
> > >
> > > First, there appears to already be support for instruction bundling and 
> > > alignment in the assembler today.  I stumbled across the 
> > > .bundle_align_mode, .bundle_start, and .bundle_end mechanism 
> > > (https://lists.llvm.org/pipermail/llvm-dev/2012-December/056723.html) 
> > > which seems to *heavily* overlap with this proposal.  I suspect that the 
> > > compiler support suggested by James and myself earlier in this thread 
> > > could be implemented on to this existing mechanism.
> > >
> > > Second, the new callbacks and infrastructure added appear to overlap 
> > > heavily w/the MCCodePadding infrastructure.  (Which, admitted, appears 
> > > unused and untested.)
> >
> >
> > My conclusion after looking at all of that was actually that I plan to 
> > propose removing both the MCCodePadding and all the bundle-padding 
> > infrastructure, not add new stuff on top of it -- the former is unused, and 
> > I believe the latter is only for Chrome's NaCL, which is deprecated, and 
> > fairly close to being removed. If we need something similar in the future, 
> > we should certainly look to both of those for inspiration, but I don't 
> > think we need to be constrained by them.
>
>
> I can definitely see removing the code padding stuff, since it's unused and 
> untested.
>
> As for the bundle mechanisms, why?  It seems like exactly what we're going to 
> want here.  Regardless of the auto-detect feature, we're going to need a 
> representation of a bundle which needs to be properly placed to avoid 
> splitting, and the current code does that.  Why not reuse the, presumable 
> reasonable well tested, existing infrastructure?   The only extra thing we 
> seem to need is the ability to toggle off bundle formation for instruction 
> types we don't care about.  Since we're going to need an assembler spelling 
> of that regardless, it seems like the current code is a decent baseline?


I created D71106  to delete MCCodePadder and 
accompanying classes.


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

https://reviews.llvm.org/D70157



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

[PATCH] D70958: [compiler-rt] [test] Disable ASLR on ASAN/MSAN/TSAN tests on NetBSD

[Michał Górny via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG6c2b2b9e20ab: [compiler-rt] [test] Disable ASLR on 
ASAN/MSAN/TSAN tests on NetBSD (authored by mgorny).
Herald added a project: Sanitizers.
Herald added a subscriber: Sanitizers.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70958

Files:
  compiler-rt/test/asan/lit.cfg.py
  compiler-rt/test/lit.common.cfg.py
  compiler-rt/test/msan/lit.cfg.py
  compiler-rt/test/sanitizer_common/lit.common.cfg.py
  compiler-rt/test/sanitizer_common/netbsd_commands/run_noaslr.sh
  compiler-rt/test/tsan/lit.cfg.py


Index: compiler-rt/test/tsan/lit.cfg.py
===
--- compiler-rt/test/tsan/lit.cfg.py
+++ compiler-rt/test/tsan/lit.cfg.py
@@ -88,3 +88,6 @@
 
 if not config.parallelism_group:
   config.parallelism_group = 'shadow-memory'
+
+if config.host_os == 'NetBSD':
+  config.substitutions.insert(0, ('%run', config.netbsd_noaslr_prefix))
Index: compiler-rt/test/sanitizer_common/netbsd_commands/run_noaslr.sh
===
--- /dev/null
+++ compiler-rt/test/sanitizer_common/netbsd_commands/run_noaslr.sh
@@ -0,0 +1,3 @@
+#!/bin/sh
+/usr/sbin/paxctl +a "${1}"
+exec "${@}"
Index: compiler-rt/test/sanitizer_common/lit.common.cfg.py
===
--- compiler-rt/test/sanitizer_common/lit.common.cfg.py
+++ compiler-rt/test/sanitizer_common/lit.common.cfg.py
@@ -73,3 +73,6 @@
 
 if not config.parallelism_group:
   config.parallelism_group = 'shadow-memory'
+
+if config.host_os == 'NetBSD':
+  config.substitutions.insert(0, ('%run', config.netbsd_noaslr_prefix))
Index: compiler-rt/test/msan/lit.cfg.py
===
--- compiler-rt/test/msan/lit.cfg.py
+++ compiler-rt/test/msan/lit.cfg.py
@@ -45,3 +45,6 @@
   config.substitutions.append( ('CHECK-%short-stack', 'CHECK-SHORT-STACK'))
 else:
   config.substitutions.append( ('CHECK-%short-stack', 'CHECK-FULL-STACK'))
+
+if config.host_os == 'NetBSD':
+  config.substitutions.insert(0, ('%run', config.netbsd_noaslr_prefix))
Index: compiler-rt/test/lit.common.cfg.py
===
--- compiler-rt/test/lit.common.cfg.py
+++ compiler-rt/test/lit.common.cfg.py
@@ -498,3 +498,9 @@
 
 config.clang = " " + " ".join(run_wrapper + [config.compile_wrapper, 
config.clang]) + " "
 config.target_cflags = " " + " ".join(target_cflags + extra_cflags) + " "
+
+if config.host_os == 'NetBSD':
+  nb_commands_dir = os.path.join(config.compiler_rt_src_root,
+ "test", "sanitizer_common", "netbsd_commands")
+  config.netbsd_noaslr_prefix = ('sh ' +
+ os.path.join(nb_commands_dir, 
'run_noaslr.sh'))
Index: compiler-rt/test/asan/lit.cfg.py
===
--- compiler-rt/test/asan/lit.cfg.py
+++ compiler-rt/test/asan/lit.cfg.py
@@ -237,3 +237,6 @@
 
 if not config.parallelism_group:
   config.parallelism_group = 'shadow-memory'
+
+if config.host_os == 'NetBSD':
+  config.substitutions.insert(0, ('%run', config.netbsd_noaslr_prefix))


Index: compiler-rt/test/tsan/lit.cfg.py
===
--- compiler-rt/test/tsan/lit.cfg.py
+++ compiler-rt/test/tsan/lit.cfg.py
@@ -88,3 +88,6 @@
 
 if not config.parallelism_group:
   config.parallelism_group = 'shadow-memory'
+
+if config.host_os == 'NetBSD':
+  config.substitutions.insert(0, ('%run', config.netbsd_noaslr_prefix))
Index: compiler-rt/test/sanitizer_common/netbsd_commands/run_noaslr.sh
===
--- /dev/null
+++ compiler-rt/test/sanitizer_common/netbsd_commands/run_noaslr.sh
@@ -0,0 +1,3 @@
+#!/bin/sh
+/usr/sbin/paxctl +a "${1}"
+exec "${@}"
Index: compiler-rt/test/sanitizer_common/lit.common.cfg.py
===
--- compiler-rt/test/sanitizer_common/lit.common.cfg.py
+++ compiler-rt/test/sanitizer_common/lit.common.cfg.py
@@ -73,3 +73,6 @@
 
 if not config.parallelism_group:
   config.parallelism_group = 'shadow-memory'
+
+if config.host_os == 'NetBSD':
+  config.substitutions.insert(0, ('%run', config.netbsd_noaslr_prefix))
Index: compiler-rt/test/msan/lit.cfg.py
===
--- compiler-rt/test/msan/lit.cfg.py
+++ compiler-rt/test/msan/lit.cfg.py
@@ -45,3 +45,6 @@
   config.substitutions.append( ('CHECK-%short-stack', 'CHECK-SHORT-STACK'))
 else:
   config.substitutions.append( ('CHECK-%short-stack', 'CHECK-FULL-STACK'))
+
+if config.host_os == 'NetBSD':
+  config.substitutions.insert(0, ('%run', config.netbsd_noaslr_prefix))
Index: compiler-rt/test/lit.common.cfg.py

[PATCH] D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools

[Csaba Dabis via Phabricator via cfe-commits]

Charusso added a comment.

I would change the order of CCh and scan-build because we usually list stuff in 
alphabetical order. Also the chronological order is that, the newest is the 
first.




Comment at: clang/www/analyzer/command-line.html:54
+Can run clang-tidy checkers too.
+Out-of-tree, not part of the LLVM project.
+  

LLVM peoples seems to care to use open source projects, so I would mention it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70439



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

[PATCH] D69088: [Lex] #pragma clang transform

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

This is a major new language feature, and code review is probably not the right 
venue for reviewing it; there should be a thread on cfe-dev.  My apologies if 
that's already been discussed and I missed it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69088



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

[PATCH] D70172: [CUDA][HIP] Fix assertion due to dtor check on windows

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Richard is definitely our main expert in the implicit synthesis of special 
members.  It seems to me that if we need the destructor declaration at some 
point, we should be forcing it to exist at that point.


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

https://reviews.llvm.org/D70172



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 232491.
MadCoder edited the summary of this revision.

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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,6 +11,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
 @end
 
 @implementation Root
@@ -173,3 +174,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,8 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), 
Method));
 
   return Method;
 }


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,6 +11,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
 @end
 
 @implementation Root
@@ -173,3 +174,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,8 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -4027,7 +4027,7 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
+  auto I = DirectMethodDefinitions.find(OMD->getCanonicalDecl());
   if (I != DirectMethodDefinitions.end())
 return I->second;
 
@@ -4040,7 +4040,7 @@
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(OMD->getCanonicalDecl(), Method));
 
   return Method;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

oh hah, thanks :)


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

https://reviews.llvm.org/D71091



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

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

[Bryan Chan via Phabricator via cfe-commits]

bryanpkc added inline comments.



Comment at: clang/test/OpenMP/target_messages.cpp:4
+
+// RUN: not %clang_cc1 -fopenmp -std=c++11 -fopenmp-targets=aaa-bbb-ccc-ddd -o 
- %s 2>&1 | FileCheck %s
 // CHECK: error: OpenMP target is invalid: 'aaa-bbb-ccc-ddd'

ABataev wrote:
> Why do you need the changes in this file?
The clean-up is not strictly necessary. I came across this file when I was 
looking for the right file to add the tests, and thought it would be better to 
group similar tests (and their corresponding CHECK patterns) together.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70804



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

@rsmith - thanks for looking at this. I'd looked at it a bit and I was/am still 
wondering whether there's a way to coalesce these codepaths between PCH and the 
existing modular code generation support? But if you're good with it, that's 
certainly good enough for me.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D70524#1771709 , @probinson wrote:

> In D70524#1771522 , @shafik wrote:
>
> > @probinson I was reading the C++ "auto" return type 
> >  issue and I can't come 
> > up with a case where we don't have class descriptions across compilation 
> > units that are not consistent wrt to auto return type. Do you have a 
> > specific example?
>
>
> The actual return type is known in a compile_unit where the method is 
> defined, and not known in other compile_units.  If the non-defining 
> compile_units omit the return type, that means "void" not "auto".  That is, 
> one compile unit says it returns "void" and another compile unit says it 
> returns something else.  That is the inconsistency I meant.
>
> If we use unspecified_type instead of omitting the return type, then a 
> consumer knows that the method returns *something*, but it will have to look 
> elsewhere to determine what that is.


Yeah, my argument was to omit the declarations of auto-returning functions 
entirely except in cases where their definition is available (either only when 
the definition is emitted, or whenever the definition's available so the 
deduced return type can be provided), same as a template instantiation.

So the class would be missing the declaration of the function in cases where 
the definition isn't available - same as templates and implicit special members.

(yeah, you could take this to its logical extreme and say we should treat all 
member functions this way - never produce a whole class definition enumerating 
all members - Eric and I have bandied that idea about as "classes as 
namespaces" more or less (the members in a class definition in one CU aren't 
necessarily the same as those in another, and a consumer would have to consider 
all of them together like it would have to for namespaces))

& honestly it's going to be pretty uncommon that any of this comes up - people 
aren't likely to separate their auto-returning function declaration from its 
definition. It'd be awkward to read code like that.


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

https://reviews.llvm.org/D70524



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

[PATCH] D60499: [ASTImporter] Various source location and range import fixes.

[Douglas Yung via Phabricator via cfe-commits]

dyung added a comment.

This change caused the Import/namespace/struct-and-var/test.cpp test to fail on 
ARM due to an extra line with `-CXXRecordDecl being emitted by the compiler 
that was being matched instead of the intended line. I checked in a fix to 
tighten up the check a little more so that it gets the correct line in 757bc55 
. I don't 
think it should negatively affect the test, but please do review the change.

Sample test failure from build bot:
http://lab.llvm.org:8011/builders/llvm-clang-win-x-armv7l/builds/1131/steps/test-check-clang/logs/FAIL%3A%20Clang%3A%3Atest.cpp

   TEST 'Clang :: Import/struct-and-var/test.cpp' FAILED 

  Script:
  --
  : 'RUN: at line 1';   clang-import-test -dump-ast --import 
C:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\llvm-project\clang\test\Import\struct-and-var/Inputs/S1.cpp
 --import 
C:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\llvm-project\clang\test\Import\struct-and-var/Inputs/S2.cpp
 -expression 
C:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\llvm-project\clang\test\Import\struct-and-var\test.cpp
 | c:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\build\bin\filecheck.exe 
C:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\llvm-project\clang\test\Import\struct-and-var\test.cpp
  --
  Exit Code: 1
  
  Command Output (stdout):
  --
  $ ":" "RUN: at line 1"
  $ "clang-import-test" "-dump-ast" "--import" 
"C:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\llvm-project\clang\test\Import\struct-and-var/Inputs/S1.cpp"
 "--import" 
"C:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\llvm-project\clang\test\Import\struct-and-var/Inputs/S2.cpp"
 "-expression" 
"C:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\llvm-project\clang\test\Import\struct-and-var\test.cpp"
  $ "c:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\build\bin\filecheck.exe" 
"C:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\llvm-project\clang\test\Import\struct-and-var\test.cpp"
  # command stderr:
  
C:\buildbot\as-builder-1\llvm-clang-win-x-armv7l\llvm-project\clang\test\Import\struct-and-var\test.cpp:4:16:
 error: CHECK-SAME: is not on the same line as the previous match
  
  // CHECK-SAME: Inputs/S2.cpp:1:1, line:3:1> line:1:8 struct F
  
 ^
  
  :49:126: note: 'next' match was here
  
  `-CXXRecordDecl 0x63d9992750 
 line:1:8 struct F definition
  

   ^
  
  :25:18: note: previous match ended here
  
  | `-CXXRecordDecl 0x63d9992378 <>  implicit 
 struct __va_list definition
  
   ^


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D60499



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

[clang] 7599095 - Fix crash if a user-defined conversion is applied in the middle of a

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-05T18:44:15-08:00
New Revision: 759909506c2b557b9bc5cd3883970dea48e0b86b

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

LOG: Fix crash if a user-defined conversion is applied in the middle of a
rewrite of an operator in terms of operator<=>.

Added: 


Modified: 
clang/include/clang/AST/Expr.h
clang/lib/AST/Expr.cpp
clang/lib/AST/ExprCXX.cpp
clang/test/SemaCXX/compare-cxx2a.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 867615ded162..afb710efe792 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -792,6 +792,16 @@ class Expr : public ValueStmt {
 return const_cast(this)->IgnoreImplicit();
   }
 
+  /// Skip past any implicit AST nodes which might surround this expression
+  /// until reaching a fixed point. Same as IgnoreImplicit, except that it
+  /// also skips over implicit calls to constructors and conversion functions.
+  ///
+  /// FIXME: Should IgnoreImplicit do this?
+  Expr *IgnoreImplicitAsWritten() LLVM_READONLY;
+  const Expr *IgnoreImplicitAsWritten() const {
+return const_cast(this)->IgnoreImplicitAsWritten();
+  }
+
   /// Skip past any parentheses which might surround this expression until
   /// reaching a fixed point. Skips:
   /// * ParenExpr

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 3bc2ea60aa14..c4c8fcf25495 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -2893,6 +2893,13 @@ static Expr *IgnoreImplicitSingleStep(Expr *E) {
   return E;
 }
 
+static Expr *IgnoreImplicitAsWrittenSingleStep(Expr *E) {
+  if (auto *ICE = dyn_cast(E))
+return ICE->getSubExprAsWritten();
+
+  return IgnoreImplicitSingleStep(E);
+}
+
 static Expr *IgnoreParensSingleStep(Expr *E) {
   if (auto *PE = dyn_cast(E))
 return PE->getSubExpr();
@@ -2972,6 +2979,10 @@ Expr *Expr::IgnoreImplicit() {
   return IgnoreExprNodes(this, IgnoreImplicitSingleStep);
 }
 
+Expr *Expr::IgnoreImplicitAsWritten() {
+  return IgnoreExprNodes(this, IgnoreImplicitAsWrittenSingleStep);
+}
+
 Expr *Expr::IgnoreParens() {
   return IgnoreExprNodes(this, IgnoreParensSingleStep);
 }

diff  --git a/clang/lib/AST/ExprCXX.cpp b/clang/lib/AST/ExprCXX.cpp
index bef181ed4cf7..deb76820afe5 100644
--- a/clang/lib/AST/ExprCXX.cpp
+++ b/clang/lib/AST/ExprCXX.cpp
@@ -107,7 +107,7 @@ CXXRewrittenBinaryOperator::getDecomposedForm() const {
 return Result;
 
   // Otherwise, we expect a <=> to now be on the LHS.
-  E = Result.LHS->IgnoreImplicit();
+  E = Result.LHS->IgnoreImplicitAsWritten();
   if (auto *BO = dyn_cast(E)) {
 assert(BO->getOpcode() == BO_Cmp);
 Result.LHS = BO->getLHS();

diff  --git a/clang/test/SemaCXX/compare-cxx2a.cpp 
b/clang/test/SemaCXX/compare-cxx2a.cpp
index 28854a77fb24..1aca2dd86c58 100644
--- a/clang/test/SemaCXX/compare-cxx2a.cpp
+++ b/clang/test/SemaCXX/compare-cxx2a.cpp
@@ -361,6 +361,11 @@ void test_user_conv() {
   }
 }
 
+struct X {
+  constexpr const Conv operator<=>(X) { return {}; }
+};
+static_assert(X() < X());
+
 } // namespace TestUserDefinedConvSeq
 
 void test_array_conv() {



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

[clang] 757bc55 - Tighten up CHECK lines added in a9f10ebffa to work on ARM.

[Douglas Yung via cfe-commits]

Author: Douglas Yung
Date: 2019-12-05T18:35:08-08:00
New Revision: 757bc55f8314c8c789f81b7f0b3397a8c6148b68

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

LOG: Tighten up CHECK lines added in a9f10ebffa to work on ARM.

On ARM platforms, the compiler generates an additional line containing 
`-CXXRecordDecl which is not the intended line, but preceeds the intended match 
causing the test to fail.

Added: 


Modified: 
clang/test/Import/struct-and-var/test.cpp

Removed: 




diff  --git a/clang/test/Import/struct-and-var/test.cpp 
b/clang/test/Import/struct-and-var/test.cpp
index 8732cb937582..8a499bfa797e 100644
--- a/clang/test/Import/struct-and-var/test.cpp
+++ b/clang/test/Import/struct-and-var/test.cpp
@@ -1,6 +1,6 @@
 // RUN: clang-import-test -dump-ast --import %S/Inputs/S1.cpp --import 
%S/Inputs/S2.cpp -expression %s | FileCheck %s
 
-// CHECK: `-CXXRecordDecl
+// CHECK: {{^`-CXXRecordDecl}}
 // CHECK-SAME: Inputs/S2.cpp:1:1, line:3:1> line:1:8 struct F
 
 void expr() {



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

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: clang/test/OpenMP/target_messages.cpp:4
+
+// RUN: not %clang_cc1 -fopenmp -std=c++11 -fopenmp-targets=aaa-bbb-ccc-ddd -o 
- %s 2>&1 | FileCheck %s
 // CHECK: error: OpenMP target is invalid: 'aaa-bbb-ccc-ddd'

Why do you need the changes in this file?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70804



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

[PATCH] D66437: Sema: Create a no-op implicit cast for lvalue function conversions.

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added inline comments.



Comment at: clang/lib/Sema/SemaInit.cpp:4745
+  else if (FunctionConversion)
+Sequence.AddQualificationConversionStep(cv1T1, VK_LValue);
 

Is this a qualification conversion though? The standard lists it "function 
pointer conversion".



Comment at: clang/test/CodeGenCXX/implicit-function-conversion.cpp:6-7
+
+// CHECK: call i32 @_Z1bRFddE(double (double)* @_Z1ad)
+int c = b(a);

Wouldn't it make slightly more sense to check the AST for a specific 
`ImplicitCastExpr`? This test would have passed before your change when 
assertions are turned off, if I'm not mistaken.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D66437



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

[PATCH] D70804: [Frontend] Allow OpenMP offloading to aarch64

[Bryan Chan via Phabricator via cfe-commits]

bryanpkc added a comment.

Pinging reviewers.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70804



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

[PATCH] D67112: [Sema] Add implicit cast for conversion of function references

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert abandoned this revision.
aaronpuchert added a comment.

Already fixed via D66437 .


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67112



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

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

[dmajor via Phabricator via cfe-commits]

dmajor added a comment.

In D65761#1772031 , @rnk wrote:

> I think `-fcf-protection` and `__attribute__((nocf_check))` have to do with 
> CET and Intel's endbranch instruction or what have you. Similar goals, 
> different implementation. I think at this point it's "patches welcome". =S


Well, this patch specifically has code (even a test) that `nocf_check` prevents 
CFG, so it looks like the intent was there, it's just that there isn't a way to 
get that attribute onto functions from cpp in a CFG/non-CET world.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761



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

[PATCH] D68627: [Sema][X86] Consider target attribute into the checks in validateOutputSize and validateInputSize.

[Reid Kleckner via Phabricator via cfe-commits]

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

lgtm

Hoisting this to ASTContext and using it where needed seems like the logical 
thing to do.


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

https://reviews.llvm.org/D68627



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

[PATCH] D70351: [clang][WIP][clang-scan-deps] Add an experimental C API.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese updated this revision to Diff 232484.
Bigcheese marked an inline comment as done.
Bigcheese added a comment.
Herald added a subscriber: mgrang.

- Remove duplicate decl
- Add test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70351

Files:
  clang/include/clang-c/Dependencies.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningWorker.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb.json
  clang/test/ClangScanDeps/modules-full.cpp
  clang/test/Index/Core/scan-deps.m
  clang/tools/c-index-test/CMakeLists.txt
  clang/tools/c-index-test/core_main.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp
  clang/tools/libclang/CDependencies.cpp
  clang/tools/libclang/CMakeLists.txt
  clang/tools/libclang/CXString.cpp
  clang/tools/libclang/CXString.h
  clang/tools/libclang/libclang.exports
  llvm/include/llvm/ADT/FunctionExtras.h

Index: llvm/include/llvm/ADT/FunctionExtras.h
===
--- llvm/include/llvm/ADT/FunctionExtras.h
+++ llvm/include/llvm/ADT/FunctionExtras.h
@@ -287,6 +287,37 @@
   }
 };
 
+template  struct FunctionObjectCallback {
+  void *Context;
+  CallTy *Callback;
+};
+
+namespace detail {
+template 
+struct functionObjectToCCallbackRefImpl;
+
+template 
+struct functionObjectToCCallbackRefImpl {
+  static FunctionObjectCallback impl(FuncTy ) {
+auto Func = +[](void *C, Args... V) -> Ret {
+  return (*reinterpret_cast *>(C))(
+  std::forward(V)...);
+};
+
+return {, Func};
+  }
+};
+} // namespace detail
+
+/// Returns a function pointer and context pair suitable for use as a C
+/// callback.
+///
+/// \param F the function object to turn into a C callback. The returned
+///   callback has the same lifetime as F.
+template 
+auto functionObjectToCCallbackRef(FuncTy ) {
+  return detail::functionObjectToCCallbackRefImpl::impl(F);
+}
 } // end namespace llvm
 
 #endif // LLVM_ADT_FUNCTION_H
Index: clang/tools/libclang/libclang.exports
===
--- clang/tools/libclang/libclang.exports
+++ clang/tools/libclang/libclang.exports
@@ -157,6 +157,13 @@
 clang_equalRanges
 clang_equalTypes
 clang_executeOnThread
+clang_experimental_DependencyScannerService_create_v0
+clang_experimental_DependencyScannerService_dispose_v0
+clang_experimental_DependencyScannerWorker_create_v0
+clang_experimental_DependencyScannerWorker_dispose_v0
+clang_experimental_DependencyScannerWorker_getFileDependencies_v0
+clang_experimental_FileDependencies_dispose
+clang_experimental_ModuleDependencySet_dispose
 clang_findIncludesInFile
 clang_findIncludesInFileWithBlock
 clang_findReferencesInFile
Index: clang/tools/libclang/CXString.h
===
--- clang/tools/libclang/CXString.h
+++ clang/tools/libclang/CXString.h
@@ -17,6 +17,7 @@
 #include "clang/Basic/LLVM.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringRef.h"
+#include "llvm/ADT/StringSet.h"
 #include "llvm/Support/Compiler.h"
 #include 
 #include 
@@ -69,6 +70,8 @@
 
 CXStringSet *createSet(const std::vector );
 
+CXStringSet *createSet(const llvm::StringSet<> );
+
 /// A string pool used for fast allocation/deallocation of strings.
 class CXStringPool {
 public:
Index: clang/tools/libclang/CXString.cpp
===
--- clang/tools/libclang/CXString.cpp
+++ clang/tools/libclang/CXString.cpp
@@ -119,6 +119,15 @@
   return Set;
 }
 
+CXStringSet *createSet(const llvm::StringSet<> ) {
+  CXStringSet *Set = new CXStringSet;
+  Set->Count = Strings.size();
+  Set->Strings = new CXString[Set->Count];
+  int I = 0;
+  for (auto SI = Strings.begin(), SE = Strings.end(); SI != SE; ++SI)
+Set->Strings[I++] = createDup(SI->getKey());
+  return Set;
+}
 
 //===--===//
 // String pools.
Index: clang/tools/libclang/CMakeLists.txt
===
--- clang/tools/libclang/CMakeLists.txt
+++ clang/tools/libclang/CMakeLists.txt
@@ -1,6 +1,7 @@
 set(SOURCES
   ARCMigrate.cpp
   BuildSystem.cpp
+  CDependencies.cpp
   CIndex.cpp
   CIndexCXX.cpp
   CIndexCodeCompletion.cpp
@@ -37,6 +38,7 @@
 set(LIBS
   clangAST
   clangBasic
+  clangDependencyScanning
   clangDriver
   clangFrontend
   clangIndex
Index: clang/tools/libclang/CDependencies.cpp
===
--- /dev/null
+++ clang/tools/libclang/CDependencies.cpp

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese added inline comments.



Comment at: clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp:97
+  if (OutputPaths.empty())
+OutputPaths = Opts.Targets;
   Dependencies.push_back(File);

arphaman wrote:
> Bigcheese wrote:
> > arphaman wrote:
> > > What if `Opts.Targets` is empty?
> > If I recall correctly, `Opts.Targets` can never be empty. I gets set to 
> > `.o` if it would be empty.
> What I mean is, what if the client didn't request a dependency file in the 
> original compilation command? ScanDeps worker currently has a fake 
> `"clang-scan-deps dependency"` target that it adds if the target is empty, 
> but I do't think that should be reported as an output file.
Ah, I see now. This is only needed for the compilation database case (as that's 
the only way to map back to a compile command), it's not needed if you're 
calling `getFullDependencies` directly. I can remove it for now, but I'm not 
really sure what to replace it with.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70268



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

[PATCH] D70268: [clang][clang-scan-deps] Aggregate the full dependency information.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese updated this revision to Diff 232483.
Bigcheese marked 2 inline comments as done.
Bigcheese added a comment.

- Removed `OutputPaths`.
- Add documentation for `AlreadySeen`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70268

Files:
  clang/include/clang/Tooling/DependencyScanning/DependencyScanningTool.h
  clang/include/clang/Tooling/DependencyScanning/ModuleDepCollector.h
  clang/lib/Tooling/DependencyScanning/DependencyScanningTool.cpp
  clang/lib/Tooling/DependencyScanning/DependencyScanningWorker.cpp
  clang/lib/Tooling/DependencyScanning/ModuleDepCollector.cpp
  clang/test/ClangScanDeps/Inputs/modules_cdb.json
  clang/test/ClangScanDeps/modules-full.cpp
  clang/tools/clang-scan-deps/ClangScanDeps.cpp

Index: clang/tools/clang-scan-deps/ClangScanDeps.cpp
===
--- clang/tools/clang-scan-deps/ClangScanDeps.cpp
+++ clang/tools/clang-scan-deps/ClangScanDeps.cpp
@@ -15,6 +15,7 @@
 #include "llvm/Support/CommandLine.h"
 #include "llvm/Support/FileUtilities.h"
 #include "llvm/Support/InitLLVM.h"
+#include "llvm/Support/JSON.h"
 #include "llvm/Support/Program.h"
 #include "llvm/Support/Signals.h"
 #include "llvm/Support/Threading.h"
@@ -129,6 +130,11 @@
 llvm::cl::init(ScanningOutputFormat::Make),
 llvm::cl::cat(DependencyScannerCategory));
 
+static llvm::cl::opt FullCommandLine(
+"full-command-line",
+llvm::cl::desc("Include the full command lines to use to build modules"),
+llvm::cl::init(false), llvm::cl::cat(DependencyScannerCategory));
+
 llvm::cl::opt
 NumThreads("j", llvm::cl::Optional,
llvm::cl::desc("Number of worker threads to use (default: use "
@@ -189,9 +195,10 @@
 /// based on the result.
 ///
 /// \returns True on error.
-static bool handleDependencyToolResult(const std::string ,
-   llvm::Expected ,
-   SharedStream , SharedStream ) {
+static bool
+handleMakeDependencyToolResult(const std::string ,
+   llvm::Expected ,
+   SharedStream , SharedStream ) {
   if (!MaybeFile) {
 llvm::handleAllErrors(
 MaybeFile.takeError(), [, ](llvm::StringError ) {
@@ -206,6 +213,184 @@
   return false;
 }
 
+static llvm::json::Array toJSONSorted(const llvm::StringSet<> ) {
+  std::vector Strings;
+  for (auto & : Set)
+Strings.push_back(I.getKey());
+  std::sort(Strings.begin(), Strings.end());
+  return llvm::json::Array(Strings);
+}
+
+static llvm::json::Array toJSONSorted(std::vector V) {
+  std::sort(V.begin(), V.end(),
+[](const ClangModuleDep , const ClangModuleDep ) {
+  return std::tie(A.ModuleName, A.ContextHash) <
+ std::tie(B.ModuleName, B.ContextHash);
+});
+
+  llvm::json::Array Ret;
+  for (const ClangModuleDep  : V)
+Ret.push_back(llvm::json::Object(
+{{"module-name", CMD.ModuleName}, {"context-hash", CMD.ContextHash}}));
+  return Ret;
+}
+
+// Thread safe.
+class FullDeps {
+public:
+  void mergeDeps(StringRef Input, FullDependenciesResult FDR,
+ size_t InputIndex) {
+const FullDependencies  = FDR.FullDeps;
+
+InputDeps ID;
+ID.FileName = Input;
+ID.ContextHash = std::move(FD.ContextHash);
+ID.FileDeps = std::move(FD.FileDeps);
+ID.ModuleDeps = std::move(FD.ClangModuleDeps);
+
+std::unique_lock ul(Lock);
+for (const ModuleDeps  : FDR.DiscoveredModules) {
+  auto I = Modules.find({MD.ContextHash, MD.ModuleName, 0});
+  if (I != Modules.end()) {
+I->first.InputIndex = std::min(I->first.InputIndex, InputIndex);
+continue;
+  }
+  Modules.insert(
+  I, {{MD.ContextHash, MD.ModuleName, InputIndex}, std::move(MD)});
+}
+
+if (FullCommandLine)
+  ID.AdditonalCommandLine = FD.getAdditionalCommandLine(
+  [&](ClangModuleDep CMD) { return lookupPCMPath(CMD); },
+  [&](ClangModuleDep CMD) -> const ModuleDeps & {
+return lookupModuleDeps(CMD);
+  });
+
+Inputs.push_back(std::move(ID));
+  }
+
+  void printFullOutput(raw_ostream ) {
+// Sort the modules by name to get a deterministic order.
+std::vector ModuleNames;
+for (auto & : Modules)
+  ModuleNames.push_back(M.first);
+std::sort(ModuleNames.begin(), ModuleNames.end(),
+  [](const ContextModulePair , const ContextModulePair ) {
+return std::tie(A.ModuleName, A.InputIndex) <
+   std::tie(B.ModuleName, B.InputIndex);
+  });
+
+std::sort(Inputs.begin(), Inputs.end(),
+  [](const InputDeps , const InputDeps ) {
+return A.FileName < B.FileName;
+  });
+
+using namespace llvm::json;
+
+Array OutModules;
+for (auto & : ModuleNames) {
+  auto  = Modules[ModName];
+  Object 

[PATCH] D65761: Add Windows Control Flow Guard checks (/guard:cf).

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

In D65761#1766993 , @dmajor wrote:

> Are there any plans to implement `__declspec(guard(nocf))` or an equivalent 
> mechanism? `__attribute__((nocf_check))` doesn't do anything without the 
> -fcf-protection flag. (https://bugs.llvm.org/show_bug.cgi?id=44096)


I think `-fcf-protection` and `__attribute__((nocf_check))` have to do with CET 
and Intel's endbranch instruction or what have you. Similar goals, different 
implementation. I think at this point it's "patches welcome". =S


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D65761



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

[PATCH] D70439: [Analyzer][Docs][NFC] Add CodeChecker to the command line tools

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/www/analyzer/codechecker.html:13
+
+
+

martong wrote:
> NoQ wrote:
> > Note related to your patch, but SSI seem to be completely broken these 
> > days; previously the dropdown menus header kept working on the front page 
> > but now even that's missing. Patches are very welcome >.<
> Ok, actually, this line is a result of copy pasting from scan-build.html. I 
> don't think I'd be competence enough to solve the drop-down menu issue, so, I 
> just removed this line.
No-no, i'd rather keep it. Otherwise how do we remember to fix it? :D



Comment at: clang/www/analyzer/codechecker.html:48
+CodeChecker parse ./reports -e html -o ./reports_html
+firefox ./reports_html/index.html
+

Maybe `xdg-open`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70439



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

[PATCH] D69948: [Checkers] Added support for freopen to StreamChecker.

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:201
+std::tie(StateRetNotNull, StateRetNull) =
+CM.assumeDual(StateStreamNull, RetVal);
+if (StateRetNull) {

baloghadamsoftware wrote:
> balazske wrote:
> > NoQ wrote:
> > > Mmm, wait, this doesn't look right to me. You cannot deduce from the 
> > > presence of `freopen()` in the code that the argument may be null, so the 
> > > split over the argument is not well-justified.
> > > 
> > > The right thing to do here will be to produce a "Schrödinger file 
> > > descriptor" that's both `Opened` and `OpenFailed` until we observe the 
> > > return value to be constrained to null or non-null later during analysis 
> > > (cf. D32449), otherwise conservatively assume that the open succeeded 
> > > when queried for the first time (because that's how non-paranoid code 
> > > under analysis will behave).
> > > 
> > > Or you could drop the failed path immediately if the argument isn't known 
> > > to be zero. That'll drop the coverage slightly but won't cause anything 
> > > bad to happen.
> > > 
> > > 
> > Where does this comment belong? I got this in the email:
> > ```
> > Comment at: clang/lib/StaticAnalyzer/Checkers/StreamChecker.cpp:201
> > +std::tie(StateRetNotNull, StateRetNull) =
> > +CM.assumeDual(StateStreamNull, RetVal);
> > +if (StateRetNull) {
> > ```
> > Is the problem with the null-check of argument of freopen? Why is this 
> > different than the other calls where `checkNullStream` is used? 
> > Or is the problem with the case of NULL return value from `freopen` (in 
> > this case the argument was non-null, for the null case we assume program 
> > crash)? In this case we really do not know what happened with the file 
> > descriptor argument (but the value of it is not changed), so the code 
> > assumes now that is will be OpenFailed. This is not accurate always (it may 
> > remain open or become closed). We could have another state split here (for 
> > these cases)?
> The argument is checked against `NULL`, becuase it must not be `NULL`. That 
> is the "checker" part. The return value is split like in the existing code 
> for modeling `fopen()`. I do not see anything wrong here.
The final code looks correct to me, thanks!~


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69948



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

In D70157#1771841 , @jyknight wrote:

> In D70157#1771832 , @reames wrote:
>
> > I've been digging through the code for this for the last day or so.  This 
> > is a new area for me, so it's possible I'm off base, but I have some 
> > concerns about the current design.
> >
> > First, there appears to already be support for instruction bundling and 
> > alignment in the assembler today.  I stumbled across the 
> > .bundle_align_mode, .bundle_start, and .bundle_end mechanism 
> > (https://lists.llvm.org/pipermail/llvm-dev/2012-December/056723.html) which 
> > seems to *heavily* overlap with this proposal.  I suspect that the compiler 
> > support suggested by James and myself earlier in this thread could be 
> > implemented on to this existing mechanism.
> >
> > Second, the new callbacks and infrastructure added appear to overlap 
> > heavily w/the MCCodePadding infrastructure.  (Which, admitted, appears 
> > unused and untested.)
>
>
> My conclusion after looking at all of that was actually that I plan to 
> propose removing both the MCCodePadding and all the bundle-padding 
> infrastructure, not add new stuff on top of it -- the former is unused, and I 
> believe the latter is only for Chrome's NaCL, which is deprecated, and fairly 
> close to being removed. If we need something similar in the future, we should 
> certainly look to both of those for inspiration, but I don't think we need to 
> be constrained by them.


I can definitely see removing the code padding stuff, since it's unused and 
untested.

As for the bundle mechanisms, why?  It seems like exactly what we're going to 
want here.  Regardless of the auto-detect feature, we're going to need a 
representation of a bundle which needs to be properly placed to avoid 
splitting, and the current code does that.  Why not reuse the, presumable 
reasonable well tested, existing infrastructure?   The only extra thing we seem 
to need is the ability to toggle off bundle formation for instruction types we 
don't care about.  Since we're going to need an assembler spelling of that 
regardless, it seems like the current code is a decent baseline?


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

https://reviews.llvm.org/D70157



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

[PATCH] D70926: [clang-format] Add option for not breaking line before ObjC params

[Kanglei Fang via Phabricator via cfe-commits]

ghvg1313 added a comment.

@MyDeveloperDay Anything I'll need to do on my end to start the landing process?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70926



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

[clang] 6470497 - Revert "[AST] Traverse the class type loc inside the member type loc."

[Sterling Augustine via cfe-commits]

Author: Sterling Augustine
Date: 2019-12-05T16:48:18-08:00
New Revision: 6470497817eafe3fe2d15e11ade78fd99753d7ca

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

LOG: Revert "[AST] Traverse the class type loc inside the member type loc."

This reverts commit 7f93cb62280a73e3e899d49c45be8bfbac634b7d.

The assertion at RecursiveASTVisitor.h:1169 fails when passed a TypeLocNode.
Not sure if the correct fix is to use getTypeLocClass or something else.

Added: 


Modified: 
clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
clang/include/clang/AST/RecursiveASTVisitor.h
clang/unittests/Tooling/CMakeLists.txt

Removed: 
clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp



diff  --git a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp 
b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
index b353c0bdb4ec..7b880faa554c 100644
--- a/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
+++ b/clang-tools-extra/clangd/unittests/SemanticHighlightingTests.cpp
@@ -407,8 +407,8 @@ TEST(SemanticHighlighting, GetsCorrectTokens) {
   }
 )cpp",
   R"cpp(
-  template
+  template
   struct $Class[[G]] {
 void $Method[[foo]](
 $TemplateParameter[[T]] *$Parameter[[O]]) {

diff  --git a/clang/include/clang/AST/RecursiveASTVisitor.h 
b/clang/include/clang/AST/RecursiveASTVisitor.h
index d2144efb58e4..312f8bdf6bc8 100644
--- a/clang/include/clang/AST/RecursiveASTVisitor.h
+++ b/clang/include/clang/AST/RecursiveASTVisitor.h
@@ -1162,12 +1162,11 @@ DEF_TRAVERSE_TYPELOC(LValueReferenceType,
 DEF_TRAVERSE_TYPELOC(RValueReferenceType,
  { TRY_TO(TraverseTypeLoc(TL.getPointeeLoc())); })
 
+// FIXME: location of base class?
 // We traverse this in the type case as well, but how is it not reached through
 // the pointee type?
 DEF_TRAVERSE_TYPELOC(MemberPointerType, {
-  auto *TSI = TL.getClassTInfo();
-  assert(TSI);
-  TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));
+  TRY_TO(TraverseType(QualType(TL.getTypePtr()->getClass(), 0)));
   TRY_TO(TraverseTypeLoc(TL.getPointeeLoc()));
 })
 

diff  --git a/clang/unittests/Tooling/CMakeLists.txt 
b/clang/unittests/Tooling/CMakeLists.txt
index be641845558b..5cef154926ae 100644
--- a/clang/unittests/Tooling/CMakeLists.txt
+++ b/clang/unittests/Tooling/CMakeLists.txt
@@ -42,7 +42,6 @@ add_clang_unittest(ToolingTests
   RecursiveASTVisitorTests/LambdaDefaultCapture.cpp
   RecursiveASTVisitorTests/LambdaExpr.cpp
   RecursiveASTVisitorTests/LambdaTemplateParams.cpp
-  RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp
   RecursiveASTVisitorTests/NestedNameSpecifiers.cpp
   RecursiveASTVisitorTests/ParenExpr.cpp
   RecursiveASTVisitorTests/TemplateArgumentLocTraverser.cpp

diff  --git 
a/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp 
b/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp
deleted file mode 100644
index 851c33dcf5a3..
--- a/clang/unittests/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp
+++ /dev/null
@@ -1,47 +0,0 @@
-//===- unittest/Tooling/RecursiveASTVisitorTests/MemberPointerTypeLoc.cpp 
-===//
-//
-// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
-// See https://llvm.org/LICENSE.txt for license information.
-// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
-//
-//===--===//
-
-#include "TestVisitor.h"
-
-using namespace clang;
-
-namespace {
-
-class MemberPointerTypeLocVisitor
-: public ExpectedLocationVisitor {
-public:
-  bool VisitTemplateTypeParmTypeLoc(TemplateTypeParmTypeLoc TL) {
-if (!TL)
-  return true;
-Match(TL.getDecl()->getName(), TL.getNameLoc());
-return true;
-  }
-  bool VisitRecordTypeLoc(RecordTypeLoc RTL) {
-if (!RTL)
-  return true;
-Match(RTL.getDecl()->getName(), RTL.getNameLoc());
-return true;
-  }
-};
-
-TEST(RecursiveASTVisitor, VisitTypeLocInMemberPointerTypeLoc) {
-  MemberPointerTypeLocVisitor Visitor;
-  Visitor.ExpectMatch("Bar", 4, 36);
-  Visitor.ExpectMatch("T", 7, 23);
-  EXPECT_TRUE(Visitor.runOver(R"cpp(
- class Bar { void func(int); };
- class Foo {
-   void bind(const char*, void(Bar::*Foo)(int)) {}
-
-   template
-   void test(void(T::*Foo)());
- };
-  )cpp"));
-}
-
-} // end anonymous namespace



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

[PATCH] D69732: [WIP][LTO] Apply SamplePGO pipeline tunes for ThinLTO pre-link to full LTO

[Matthew Voss via Phabricator via cfe-commits]

ormris added a comment.

I've done testing with the following global parameters.

- The base for the branch is llvmorg-10-init-8655-g94a4a2c97f8
- Used llvm, clang, lld, and llvm-ar from this branch.
- The sqlite kvtest program was the test payload.

This test compared an unmodified compiler from the base of the branch with a 
modified compiler with this patch applied and the loop optimisation passes 
mentioned above moved to the backend. The results were as follows. All numbers 
in seconds.

| Run  | Modified LTO | Modified SPGO+LTO | Unmodified SPGO+LTO |
|  |  | - | --- |
| 1| 42.00| 41.73 | 42.08   |
| 2| 42.30| 39.49 | 42.45   |
| 3| 41.21| 42.46 | 42.49   |
| AVG: | 41.84| 41.23 | 42.34   |
|

TL;DR the average run using a compiler built with the modified SPGO pipeline is 
about a second faster. Definitely a positive initial result.

In D69732#1730732 , @tejohnson wrote:

> This probably needs to be taken over by someone who cares about full LTO 
> performance (@wristow or @ormris ?). This patch was some cleanup of the full 
> LTO sample PGO pipeline, but has a number of issues I enumerate in the 
> summary.


Given the performance improvements here, I'd like to develop this patch further.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69732



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

[PATCH] D71098: Handle two corner cases in creduce-clang-crash.py

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG1f822f212cde: Handle two corner cases in 
creduce-clang-crash.py (authored by rnk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71098

Files:
  clang/utils/creduce-clang-crash.py


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -30,6 +30,7 @@
 print(*args, **kwargs)
 
 def check_file(fname):
+  fname = os.path.normpath(fname)
   if not os.path.isfile(fname):
 sys.exit("ERROR: %s does not exist" % (fname))
   return fname
@@ -40,6 +41,8 @@
   or absolute path to cmd_dir/cmd_name.
   """
   if cmd_path:
+# Make the path absolute so the creduce test can be run from any directory.
+cmd_path = os.path.abspath(cmd_path)
 cmd = find_executable(cmd_path)
 if cmd:
   return cmd


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -30,6 +30,7 @@
 print(*args, **kwargs)
 
 def check_file(fname):
+  fname = os.path.normpath(fname)
   if not os.path.isfile(fname):
 sys.exit("ERROR: %s does not exist" % (fname))
   return fname
@@ -40,6 +41,8 @@
   or absolute path to cmd_dir/cmd_name.
   """
   if cmd_path:
+# Make the path absolute so the creduce test can be run from any directory.
+cmd_path = os.path.abspath(cmd_path)
 cmd = find_executable(cmd_path)
 if cmd:
   return cmd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] 1f822f2 - Handle two corner cases in creduce-clang-crash.py

[Reid Kleckner via cfe-commits]

Author: Reid Kleckner
Date: 2019-12-05T16:24:24-08:00
New Revision: 1f822f212cde1ad9099cf45af0652a83380de772

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

LOG: Handle two corner cases in creduce-clang-crash.py

Summary:
First, call os.path.normpath on the filename argument. I passed in
./foo-asdf.cpp, and this meant that the script failed to find the
filename, and bad things happened.

Second, call os.path.abspath on binaries. CReduce runs the
interestingness test in a temp dir, so relative paths will not work.

Reviewers: akhuang

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/utils/creduce-clang-crash.py

Removed: 




diff  --git a/clang/utils/creduce-clang-crash.py 
b/clang/utils/creduce-clang-crash.py
index e886bf72e5a7..b69d6efc2481 100755
--- a/clang/utils/creduce-clang-crash.py
+++ b/clang/utils/creduce-clang-crash.py
@@ -30,6 +30,7 @@ def verbose_print(*args, **kwargs):
 print(*args, **kwargs)
 
 def check_file(fname):
+  fname = os.path.normpath(fname)
   if not os.path.isfile(fname):
 sys.exit("ERROR: %s does not exist" % (fname))
   return fname
@@ -40,6 +41,8 @@ def check_cmd(cmd_name, cmd_dir, cmd_path=None):
   or absolute path to cmd_dir/cmd_name.
   """
   if cmd_path:
+# Make the path absolute so the creduce test can be run from any directory.
+cmd_path = os.path.abspath(cmd_path)
 cmd = find_executable(cmd_path)
 if cmd:
   return cmd



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

[PATCH] D70849: [AST] Traverse the class type loc inside the member pointer type loc.

[Sterling Augustine via Phabricator via cfe-commits]

saugustine added inline comments.



Comment at: clang/include/clang/AST/RecursiveASTVisitor.h:1169
 DEF_TRAVERSE_TYPELOC(MemberPointerType, {
-  TRY_TO(TraverseType(QualType(TL.getTypePtr()->getClass(), 0)));
+  if (auto *TSI = TL.getClassTInfo())
+TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()));

ilya-biryukov wrote:
> Can this actually happen in practice?
> Did we try doing **only** `TRY_TO(TraverseTypeLoc(TSI->getTypeLoc()))` and 
> seeing whether this fails?
> 
> The code in `MemberPointerTypeLoc` suggests this can happen, but would be 
> interesting to find examples when it happens.
This can happen when traversing a TypeNodeLoc, and the assertion will fail. I'm 
reverting this change for now.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70849



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

[PATCH] D71098: Handle two corner cases in creduce-clang-crash.py

[Amy Huang via Phabricator via cfe-commits]

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

lgtm


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71098



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

[PATCH] D71098: Handle two corner cases in creduce-clang-crash.py

[Reid Kleckner via Phabricator via cfe-commits]

rnk created this revision.
rnk added a reviewer: akhuang.
Herald added a project: clang.

First, call os.path.normpath on the filename argument. I passed in
./foo-asdf.cpp, and this meant that the script failed to find the
filename, and bad things happened.

Second, call os.path.abspath on binaries. CReduce runs the
interestingness test in a temp dir, so relative paths will not work.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71098

Files:
  clang/utils/creduce-clang-crash.py


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -30,6 +30,7 @@
 print(*args, **kwargs)
 
 def check_file(fname):
+  fname = os.path.normpath(fname)
   if not os.path.isfile(fname):
 sys.exit("ERROR: %s does not exist" % (fname))
   return fname
@@ -40,6 +41,8 @@
   or absolute path to cmd_dir/cmd_name.
   """
   if cmd_path:
+# Make the path absolute so the creduce test can be run from any directory.
+cmd_path = os.path.abspath(cmd_path)
 cmd = find_executable(cmd_path)
 if cmd:
   return cmd


Index: clang/utils/creduce-clang-crash.py
===
--- clang/utils/creduce-clang-crash.py
+++ clang/utils/creduce-clang-crash.py
@@ -30,6 +30,7 @@
 print(*args, **kwargs)
 
 def check_file(fname):
+  fname = os.path.normpath(fname)
   if not os.path.isfile(fname):
 sys.exit("ERROR: %s does not exist" % (fname))
   return fname
@@ -40,6 +41,8 @@
   or absolute path to cmd_dir/cmd_name.
   """
   if cmd_path:
+# Make the path absolute so the creduce test can be run from any directory.
+cmd_path = os.path.abspath(cmd_path)
 cmd = find_executable(cmd_path)
 if cmd:
   return cmd
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

@MadCoder Indexing by names doesn't seem like the right solution. You should be 
able to get the canonical declaration (`getCanonicalDecl`) from the declaration 
that represents the method in @implementation, which should avoid the problem 
you're describing.


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

https://reviews.llvm.org/D71091



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

[clang] e7c2466 - [Concepts] Fix build failures in D41569

[Saar Raz via cfe-commits]

Author: Saar Raz
Date: 2019-12-06T01:53:18+02:00
New Revision: e7c24667816edc1a3754b46a49f9eac011dc1fee

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

LOG: [Concepts] Fix build failures in D41569

Fix build failures in previous commit.

Added: 


Modified: 
clang/lib/AST/ASTConcept.cpp

Removed: 




diff  --git a/clang/lib/AST/ASTConcept.cpp b/clang/lib/AST/ASTConcept.cpp
index b38b0952145f..fc32e768d92f 100644
--- a/clang/lib/AST/ASTConcept.cpp
+++ b/clang/lib/AST/ASTConcept.cpp
@@ -29,10 +29,10 @@ ASTConstraintSatisfaction::ASTConstraintSatisfaction(const 
ASTContext ,
  Detail.second.get())};
 else {
   auto  =
-  *Detail.second.get *>();
+  *Detail.second.get *>();
   unsigned MessageSize = SubstitutionDiagnostic.second.size();
   char *Mem = new (C) char[MessageSize];
-  memcpy(Mem, SubstitutionDiagnostic.second.c_str(), MessageSize);
+  memcpy(Mem, SubstitutionDiagnostic.second.data(), MessageSize);
   auto *NewSubstDiag = new (C) std::pair(
   SubstitutionDiagnostic.first, StringRef(Mem, MessageSize));
   new (getTrailingObjects() + I)



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Fangrui Song via Phabricator via cfe-commits]

MaskRay added a subscriber: opaparo.
MaskRay added a comment.

In D70157#1771841 , @jyknight wrote:

> In D70157#1771832 , @reames wrote:
>
> > I've been digging through the code for this for the last day or so.  This 
> > is a new area for me, so it's possible I'm off base, but I have some 
> > concerns about the current design.
> >
> > First, there appears to already be support for instruction bundling and 
> > alignment in the assembler today.  I stumbled across the 
> > .bundle_align_mode, .bundle_start, and .bundle_end mechanism 
> > (https://lists.llvm.org/pipermail/llvm-dev/2012-December/056723.html) which 
> > seems to *heavily* overlap with this proposal.  I suspect that the compiler 
> > support suggested by James and myself earlier in this thread could be 
> > implemented on to this existing mechanism.
> >
> > Second, the new callbacks and infrastructure added appear to overlap 
> > heavily w/the MCCodePadding infrastructure.  (Which, admitted, appears 
> > unused and untested.)
>
>
> My conclusion after looking at all of that was actually that I plan to 
> propose removing both the MCCodePadding and all the bundle-padding 
> infrastructure, not add new stuff on top of it -- the former is unused, and I 
> believe the latter is only for Chrome's NaCL, which is deprecated, and fairly 
> close to being removed. If we need something similar in the future, we should 
> certainly look to both of those for inspiration, but I don't think we need to 
> be constrained by them.


CC the author of D34393  - @opaparo for 
MCCodePadding. Intel folks may know how to contact @opaparo?

I also noticed that MCCodePadder.cpp is never updated (except a license change) 
after the initial check-in.

>> Third, I have not see a justification for why complexity for instruction 
>> prefix padding is necessary.  All the effected CPUs support multi-byte nops, 
>> so we're talking about a *single micro op* difference between the nop form 
>> and prefix form.  Can anyone point to a performance delta due to this?  If 
>> not, I'd suggest we should start with the nop form, and then build the 
>> prefix form in a generic manner for all alignment varieties.
> 
> +1.

+1. Starting from just NOP padding sounds a simple and good first step. We can 
explore segment override prefixes in the future.


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

https://reviews.llvm.org/D70157



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

[PATCH] D71041: [analyzer][discussion] Talk about escapes

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/fuchsia_handle.cpp:210
+  // Because of arrays, structs, the suggestion is to escape when whe no longer
+  // have any pointer to that symbolic region.
+  if (zx_channel_create(0, get_handle_address(), ))

NoQ wrote:
> xazax.hun wrote:
> > xazax.hun wrote:
> > > NoQ wrote:
> > > > xazax.hun wrote:
> > > > > xazax.hun wrote:
> > > > > > xazax.hun wrote:
> > > > > > > NoQ wrote:
> > > > > > > > xazax.hun wrote:
> > > > > > > > > NoQ wrote:
> > > > > > > > > > NoQ wrote:
> > > > > > > > > > > This has nothing to do with symbolic regions. We can run 
> > > > > > > > > > > into this problem even if it's a local variable in the 
> > > > > > > > > > > current stack frame:
> > > > > > > > > > > ```lang=c++
> > > > > > > > > > > void foo() {
> > > > > > > > > > >   zx_handle_t sa, sb;
> > > > > > > > > > >   escape(); // Escape *before* create!!
> > > > > > > > > > > 
> > > > > > > > > > >   zx_channel_create(0, , );
> > > > > > > > > > >   zx_handle_close(sa);
> > > > > > > > > > >   close_escaped();
> > > > > > > > > > > }
> > > > > > > > > > > ```
> > > > > > > > > > > 
> > > > > > > > > > > The solution that'll obviously work would be to keep 
> > > > > > > > > > > track of all regions that escaped at least once, and then 
> > > > > > > > > > > not even start tracking the handle if it's getting placed 
> > > > > > > > > > > into a region that causes an escape when written into or 
> > > > > > > > > > > has itself escaped before, but that sounds like a huge 
> > > > > > > > > > > overkill.
> > > > > > > > > > > 
> > > > > > > > > > > Lemme think. This sounds vaguely familiar but i can't 
> > > > > > > > > > > immediately recall what my thoughts were last time i 
> > > > > > > > > > > thought about it.
> > > > > > > > > > `$ cat test.c`
> > > > > > > > > > ```lang=c++
> > > > > > > > > > void manage(void **x);
> > > > > > > > > > void free_managed();
> > > > > > > > > > 
> > > > > > > > > > void foo() {
> > > > > > > > > >   void *x;
> > > > > > > > > >   manage();
> > > > > > > > > >   x = malloc(1);
> > > > > > > > > >   free_managed();
> > > > > > > > > > }
> > > > > > > > > > ```
> > > > > > > > > > `$ clang --analyze test.c`
> > > > > > > > > > ```lang=c++
> > > > > > > > > > test.c:8:3: warning: Potential leak of memory pointed to by 
> > > > > > > > > > 'x'
> > > > > > > > > >   free_managed();
> > > > > > > > > >   ^~
> > > > > > > > > > 1 warning generated.
> > > > > > > > > > ```
> > > > > > > > > > Sigh.
> > > > > > > > > Oh, I see. Yeah this one will be fun to deal with 
> > > > > > > > The rules are pretty easy though, right?
> > > > > > > > ```lang=c++
> > > > > > > >   2680 // A value escapes in four possible cases:
> > > > > > > >   2681 // (1) We are binding to something that is not a memory 
> > > > > > > > region.
> > > > > > > >   2682 // (2) We are binding to a MemRegion that does not have 
> > > > > > > > stack storage.
> > > > > > > >   2683 // (3) We are binding to a top-level parameter region 
> > > > > > > > with a non-trivial
> > > > > > > >   2684 // destructor. We won't see the destructor during 
> > > > > > > > analysis, but it's there.
> > > > > > > >   2685 // (4) We are binding to a MemRegion with stack storage 
> > > > > > > > that the store
> > > > > > > >   2686 // does not understand.
> > > > > > > >   2687 ProgramStateRef
> > > > > > > >   2688 ExprEngine::processPointerEscapedOnBind(ProgramStateRef 
> > > > > > > > State, SVal Loc,
> > > > > > > >   2689 SVal Val, const 
> > > > > > > > LocationContext *LCtx) {
> > > > > > > > ```
> > > > > > > > Basically, locals are the only special case; writing into 
> > > > > > > > anything else should be an immediate escape.
> > > > > > > > 
> > > > > > > > We could easily update this procedure to additionally keep 
> > > > > > > > track of all escaped locals in the program state, and then 
> > > > > > > > escape all binds to previously escaped locals as well.
> > > > > > > > 
> > > > > > > > The checker would then have to follow the same rules.
> > > > > > > > 
> > > > > > > > In the worst case, manually.
> > > > > > > > 
> > > > > > > > But i think we should instead update the `checkPointerEscape()` 
> > > > > > > > callback to escape the values of out-parameters upon evaluating 
> > > > > > > > the call conservatively (if it doesn't already) and then teach 
> > > > > > > > the checker to mark escaped regions explicitly as escaped (as 
> > > > > > > > opposed to removing them from the program state), and then 
> > > > > > > > teach it to never transition from escaped to opened. That would 
> > > > > > > > be cleaner because that'll only require us to hardcode the 
> > > > > > > > escaping procedure once.
> > > > > > > > 
> > > > > > > > Or we could just make the "bool 
> > > > > > > > doesWriteIntoThisRegionCauseAnEscape?(Region, State)" function 
> > > > > > > > re-usable.
> > > > 

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder updated this revision to Diff 232460.
MadCoder added a comment.

Fix the fact that the hashmap of direct method was indexed by Declarations 
instead of names (and depending on code ordering, the declaration used at 
codegen time may be the one from the @interface or from the @implementation 
leading to name collisions and llvm "helpfully" adding `.1`'s everywhere


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

https://reviews.llvm.org/D71091

Files:
  clang/lib/CodeGen/CGObjCMac.cpp
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,6 +11,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
 @end
 
 @implementation Root
@@ -173,3 +174,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,8 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -876,7 +876,7 @@
 
   /// DirectMethodDefinitions - map of direct methods which have been defined 
in
   /// this translation unit.
-  llvm::DenseMap 
DirectMethodDefinitions;
+  llvm::DenseMap DirectMethodDefinitions;
 
   /// PropertyNames - uniqued method variable names.
   llvm::DenseMap PropertyNames;
@@ -4027,20 +4027,20 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) {
-  auto I = DirectMethodDefinitions.find(OMD);
-  if (I != DirectMethodDefinitions.end())
-return I->second;
-
   SmallString<256> Name;
   GetNameForMethod(OMD, CD, Name);
 
+  auto I = DirectMethodDefinitions.find(Name);
+  if (I != DirectMethodDefinitions.end())
+return I->second;
+
   CodeGenTypes  = CGM.getTypes();
   llvm::FunctionType *MethodTy =
 Types.GetFunctionType(Types.arrangeObjCMethodDeclaration(OMD));
   llvm::Function *Method =
   llvm::Function::Create(MethodTy, llvm::GlobalValue::ExternalLinkage,
  Name.str(), ());
-  DirectMethodDefinitions.insert(std::make_pair(OMD, Method));
+  DirectMethodDefinitions.insert(std::make_pair(Method->getName(), Method));
 
   return Method;
 }


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -11,6 +11,7 @@
 
 __attribute__((objc_root_class))
 @interface Root
+- (int)getInt __attribute__((objc_direct));
 @end
 
 @implementation Root
@@ -173,3 +174,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,8 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
Index: clang/lib/CodeGen/CGObjCMac.cpp
===
--- clang/lib/CodeGen/CGObjCMac.cpp
+++ clang/lib/CodeGen/CGObjCMac.cpp
@@ -876,7 +876,7 @@
 
   /// DirectMethodDefinitions - map of direct methods which have been defined in
   /// this translation unit.
-  llvm::DenseMap DirectMethodDefinitions;
+  llvm::DenseMap DirectMethodDefinitions;
 
   /// PropertyNames - uniqued method variable names.
   llvm::DenseMap PropertyNames;
@@ -4027,20 +4027,20 @@
 llvm::Function *
 CGObjCCommonMac::GenerateDirectMethod(const ObjCMethodDecl *OMD,
   const ObjCContainerDecl *CD) 

[clang] fdf80e8 - [Concepts] Constraint Enforcement & Diagnostics

[Saar Raz via cfe-commits]

Author: Saar Raz
Date: 2019-12-06T01:34:20+02:00
New Revision: fdf80e86a52849813d05da4b6c25884c06ba9e98

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

LOG: [Concepts] Constraint Enforcement & Diagnostics

Part of the C++20 concepts implementation effort.
- Associated constraints (requires clauses, currently) are now enforced when 
instantiating/specializing templates and when considering partial 
specializations and function overloads.
- Elaborated diagnostics give helpful insight as to why the constraints were 
not satisfied.
Phabricator: D41569

Re-commit, after fixing some memory bugs.

Added: 
clang/include/clang/AST/ASTConcept.h
clang/lib/AST/ASTConcept.cpp
clang/test/CXX/temp/temp.constr/temp.constr.constr/function-templates.cpp

clang/test/CXX/temp/temp.constr/temp.constr.constr/non-function-templates.cpp

clang/test/CXX/temp/temp.constr/temp.constr.constr/partial-specializations.cpp

Modified: 
clang/include/clang/AST/ExprCXX.h
clang/include/clang/Basic/DiagnosticSemaKinds.td
clang/include/clang/Sema/Sema.h
clang/include/clang/Sema/TemplateDeduction.h
clang/lib/AST/ASTContext.cpp
clang/lib/AST/CMakeLists.txt
clang/lib/AST/Decl.cpp
clang/lib/AST/ExprCXX.cpp
clang/lib/Sema/SemaConcept.cpp
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaOverload.cpp
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateDeduction.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Serialization/ASTReaderStmt.cpp
clang/lib/Serialization/ASTWriterStmt.cpp
clang/test/CXX/expr/expr.prim/expr.prim.id/p3.cpp

Removed: 




diff  --git a/clang/include/clang/AST/ASTConcept.h 
b/clang/include/clang/AST/ASTConcept.h
new file mode 100644
index ..937a8a9b345e
--- /dev/null
+++ b/clang/include/clang/AST/ASTConcept.h
@@ -0,0 +1,80 @@
+//===--- ASTConcept.h - Concepts Related AST Data Structures *- C++ 
-*-===//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+///
+/// \file
+/// \brief This file provides AST data structures related to concepts.
+///
+//===--===//
+
+#ifndef LLVM_CLANG_AST_ASTCONCEPT_H
+#define LLVM_CLANG_AST_ASTCONCEPT_H
+#include "clang/AST/Expr.h"
+#include "clang/Basic/SourceLocation.h"
+#include "llvm/ADT/PointerUnion.h"
+#include "llvm/ADT/SmallVector.h"
+#include 
+#include 
+namespace clang {
+
+/// \brief The result of a constraint satisfaction check, containing the
+/// necessary information to diagnose an unsatisfied constraint.
+struct ConstraintSatisfaction {
+  using SubstitutionDiagnostic = std::pair;
+  using Detail = llvm::PointerUnion;
+
+  bool IsSatisfied = false;
+
+  /// \brief Pairs of unsatisfied atomic constraint expressions along with the
+  /// substituted constraint expr, if the template arguments could be
+  /// substituted into them, or a diagnostic if substitution resulted in an
+  /// invalid expression.
+  llvm::SmallVector, 4> Details;
+
+  // This can leak if used in an AST node, use ASTConstraintSatisfaction
+  // instead.
+  void *operator new(size_t bytes, ASTContext ) = delete;
+};
+
+/// Pairs of unsatisfied atomic constraint expressions along with the
+/// substituted constraint expr, if the template arguments could be
+/// substituted into them, or a diagnostic if substitution resulted in
+/// an invalid expression.
+using UnsatisfiedConstraintRecord =
+std::pair *>>;
+
+/// \brief The result of a constraint satisfaction check, containing the
+/// necessary information to diagnose an unsatisfied constraint.
+///
+/// This is safe to store in an AST node, as opposed to ConstraintSatisfaction.
+struct ASTConstraintSatisfaction final :
+llvm::TrailingObjects {
+  std::size_t NumRecords;
+  bool IsSatisfied : 1;
+
+  const UnsatisfiedConstraintRecord *begin() const {
+return getTrailingObjects();
+  }
+
+  const UnsatisfiedConstraintRecord *end() const {
+return getTrailingObjects() + NumRecords;
+  }
+
+  ASTConstraintSatisfaction(const ASTContext ,
+const ConstraintSatisfaction );
+
+  static ASTConstraintSatisfaction *
+  Create(const ASTContext , const ConstraintSatisfaction );
+};
+
+} // clang
+
+#endif // LLVM_CLANG_AST_ASTCONCEPT_H
\ No newline at end of file

diff  --git a/clang/include/clang/AST/ExprCXX.h 
b/clang/include/clang/AST/ExprCXX.h
index 1eac1ce842d8..1a16aa7aacec 100644
--- a/clang/include/clang/AST/ExprCXX.h
+++ 

[PATCH] D70157: Align branches within 32-Byte boundary

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

In D70157#1771832 , @reames wrote:

> I've been digging through the code for this for the last day or so.  This is 
> a new area for me, so it's possible I'm off base, but I have some concerns 
> about the current design.
>
> First, there appears to already be support for instruction bundling and 
> alignment in the assembler today.  I stumbled across the .bundle_align_mode, 
> .bundle_start, and .bundle_end mechanism 
> (https://lists.llvm.org/pipermail/llvm-dev/2012-December/056723.html) which 
> seems to *heavily* overlap with this proposal.  I suspect that the compiler 
> support suggested by James and myself earlier in this thread could be 
> implemented on to this existing mechanism.
>
> Second, the new callbacks and infrastructure added appear to overlap heavily 
> w/the MCCodePadding infrastructure.  (Which, admitted, appears unused and 
> untested.)


My conclusion after looking at all of that was actually that I plan to propose 
removing both the MCCodePadding and all the bundle-padding infrastructure, not 
add new stuff on top of it -- the former is unused, and I believe the latter is 
only for Chrome's NaCL, which is deprecated, and fairly close to being removed. 
If we need something similar in the future, we should certainly look to both of 
those for inspiration, but I don't think we need to be constrained by them.

> Third, I have not see a justification for why complexity for instruction 
> prefix padding is necessary.  All the effected CPUs support multi-byte nops, 
> so we're talking about a *single micro op* difference between the nop form 
> and prefix form.  Can anyone point to a performance delta due to this?  If 
> not, I'd suggest we should start with the nop form, and then build the prefix 
> form in a generic manner for all alignment varieties.

+1.


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

https://reviews.llvm.org/D70157



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

[PATCH] D70701: Fix more VFS tests on Windows

[Reid Kleckner via Phabricator via cfe-commits]

rnk added inline comments.



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1077-1078
+std::error_code RedirectingFileSystem::makeAbsolute(SmallVectorImpl 
) const {
+  if (llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::posix) ||
+  llvm::sys::path::is_absolute(Path, llvm::sys::path::Style::windows))
+return {};

amccarth wrote:
> amccarth wrote:
> > rnk wrote:
> > > I think Posix-style paths are considered absolute, even on Windows. The 
> > > opposite isn't true, a path with a drive letter is considered to be 
> > > relative if the default path style is Posix. But, I don't think that 
> > > matters. We only end up in this mixed path style situation on Windows.
> > > 
> > > Does this change end up being necessary? I would expect the existing 
> > > implementation of makeAbsolute to do the right thing on Windows as is. I 
> > > think the other change seems to be what matters.
> > Yes, it's necessary.  The Posix-style path `\tests` is not considered 
> > absolute on Windows.  Thus the original makeAboslute would merge it with 
> > the current working directory, which gives it a drive letter, like 
> > `D:\tests\`.  The drive letter component then causes comparisons to fail.
> To make sure I wasn't misremembering or hallucinating, I double-checked the 
> behavior here:  Posix-style paths like `\tests` are not considered absolute 
> paths in Windows-style.
I see, I agree, the platforms diverge here:
  bool rootDir = has_root_directory(p, style);
  bool rootName =
  (real_style(style) != Style::windows) || has_root_name(p, style);

  return rootDir && rootName;

So, on Windows rootDir is true and rootName is false.

I still wonder if this behavior shouldn't be pushed down into the base class. 
If I pass the path `\test` to the real `FileSystem::makeAbsolute` on Windows, 
should that prepend the CWD, or should it leave the path alone?



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1431
 
-if (IsRootEntry && !sys::path::is_absolute(Name)) {
-  assert(NameValueNode && "Name presence should be checked earlier");

amccarth wrote:
> rnk wrote:
> > Is there a way to unit test this? I see some existing tests in 
> > llvm/unittests/Support/VirtualFileSystemTest.cpp.
> > 
> > I looked at the yaml files in the VFS tests this fixes, and I see entries 
> > like this:
> > { 'name': '/tests', 'type': 'directory', ... },
> > { 'name': '/indirect-vfs-root-files', 'type': 'directory', ... },
> > { 'name': 'C:/src/llvm-project/clang/test/VFS/Inputs/Broken.framework', 
> > 'type': 'directory', ... },
> > { 'name': 
> > 'C:/src/llvm-project/build/tools/clang/test/VFS/Output/vfsroot-with-overlay.c.tmp/cache',
> >  'type': 'directory', ... },
> > 
> > So it makes sense to me that we need to handle both kinds of absolute path.
> > Is there a way to unit test this?
> 
> What do you mean by "this"?  The `/tests` and `/indirect-vfs-root-files` 
> entries in that yaml are the ones causing several tests to fail without this 
> fix, so I take it that this is already being tested.  But perhaps you meant 
> testing something more specific?
> What do you mean by "this"? 
I guess what I meant was, can you unit test the whole change in case there are 
behavior differences here not covered by the clang tests?



Comment at: llvm/lib/Support/VirtualFileSystem.cpp:1448-1449
+  // which style we have, and use it consistently.
+  if (sys::path::is_absolute(Name, sys::path::Style::posix)) {
+path_style = sys::path::Style::posix;
+  } else if (sys::path::is_absolute(Name, sys::path::Style::windows)) {

amccarth wrote:
> amccarth wrote:
> > rnk wrote:
> > > I wonder if there's a simpler fix here. If the working directory is an 
> > > absolute Windows-style path, we could prepend the drive letter of the 
> > > working directory onto any absolute Posix style paths read from YAML 
> > > files. That's somewhat consistent with what native Windows tools do. In 
> > > cmd, you can run `cd \Windows`, and that will mean `C:\Windows` if the 
> > > active driver letter is C. I think on Windows every drive has an active 
> > > directory, but that's not part of the file system model.
> > I'm not seeing how this would be simpler.
> > 
> > I don't understand the analogy of your proposal to what the native Windows 
> > tools do.  When you say, `cd \Windows`, the `\Windows` is _not_ an absolute 
> > path.  It's relative to the current drive.
> > 
> > I could be wrong, but I don't think prepending the drive onto absolution 
> > Posix style paths read from YAML would work.  That would give us something 
> > like `D:/tests` (which is what was happening in makeAbsolute) and that 
> > won't match paths generated from non-YAML sources, which will still come 
> > out as `/tests/foo.h`.
> > 
> > > I think on Windows every drive has an active directory 
> > 
> > Yep.  I think of it as "every drive 

[PATCH] D71092: [VFS] Use path canonicalization on all paths

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I would add, take a look at the existing unit tests 
(llvm/unittests/Support/VirtualFileSystemTests.cpp) and audit for _WIN32 ifdefs 
that we can remove after this change.


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

https://reviews.llvm.org/D71092



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

I've been digging through the code for this for the last day or so.  This is a 
new area for me, so it's possible I'm off base, but I have some concerns about 
the current design.

First, there appears to already be support for instruction bundling and 
alignment in the assembler today.  I stumbled across the .bundle_align_mode, 
.bundle_start, and .bundle_end mechanism 
(https://lists.llvm.org/pipermail/llvm-dev/2012-December/056723.html) which 
seems to *heavily* overlap with this proposal.  I suspect that the compiler 
support suggested by James and myself earlier in this thread could be 
implemented on to this existing mechanism.

Second, the new callbacks and infrastructure added appear to overlap heavily 
w/the MCCodePadding infrastructure.  (Which, admitted, appears unused and 
untested.)

Third, I have not see a justification for why complexity for instruction prefix 
padding is necessary.  All the effected CPUs support multi-byte nops, so we're 
talking about a *single micro op* difference between the nop form and prefix 
form.  Can anyone point to a performance delta due to this?  If not, I'd 
suggest we should start with the nop form, and then build the prefix form in a 
generic manner for all alignment varieties.


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

https://reviews.llvm.org/D70157



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

[PATCH] D69950: Reapply "Fix crash on switch conditions of non-integer types in templates"

[Martin Storsjö via Phabricator via cfe-commits]

mstorsjo added a comment.

In D69950#1771515 , @rnk wrote:

> I fixed this particular code upstream:
>  https://github.com/KhronosGroup/glslang/pull/2010
>  I am not enough an expert to be sure, but I suspect this is in the area of 
> "invalid, no diagnostic required", where this code is invalid, but a 
> conforming C++ implementation could either reject or accept it. Now we reject 
> it, and that seems better in the long term, even though it creates a fire 
> drill in the short term. =(


Ok, thanks for the input! I'll go cherrypick that fix then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69950



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

[PATCH] D70958: [compiler-rt] [test] Disable ASLR on ASAN/MSAN/TSAN tests on NetBSD

[Kamil Rytarowski via Phabricator via cfe-commits]

krytarowski added inline comments.



Comment at: compiler-rt/test/sanitizer_common/netbsd_commands/run_noaslr.sh:4
+PATH=${PATH}:/usr/sbin
+paxctl +a "${1}"
+exec "${@}"

eugenis wrote:
> krytarowski wrote:
> > I propose to use `/usr/bin/paxctl` as it will be PATH and environment 
> > independent. We already use this direct path approach in pkgsrc.
> It this path correct? /usr/sbin in the code vs /usr/bin in the comment.
It is correct. I made a typo.


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

https://reviews.llvm.org/D70958



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

[PATCH] D71092: [VFS] Use path canonicalization on all paths

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth planned changes to this revision.
amccarth added a comment.

Hold off on this one.  It needs an explicit test for canonicalization.


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

https://reviews.llvm.org/D71092



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

[PATCH] D41910: [Concepts] Constrained partial specializations and function overloads.

[Saar Raz via Phabricator via cfe-commits]

saar.raz added a comment.

Addressed in latest diff


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D41910



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

[PATCH] D63960: [C++20] Add consteval-specific semantic for functions

[Tyker via Phabricator via cfe-commits]

Tyker updated this revision to Diff 232450.
Tyker marked 14 inline comments as done.
Tyker added a comment.

comments were fixed.


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

https://reviews.llvm.org/D63960

Files:
  clang/include/clang/AST/Expr.h
  clang/include/clang/AST/Stmt.h
  clang/include/clang/Basic/DiagnosticASTKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaDeclCXX.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Sema/SemaLambda.cpp
  clang/lib/Sema/SemaOverload.cpp
  clang/test/SemaCXX/cxx2a-consteval.cpp

Index: clang/test/SemaCXX/cxx2a-consteval.cpp
===
--- clang/test/SemaCXX/cxx2a-consteval.cpp
+++ clang/test/SemaCXX/cxx2a-consteval.cpp
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -std=c++2a -fsyntax-only %s -verify
+// RUN: %clang_cc1 -std=c++2a -fsyntax-only -Wno-unused-value %s -verify
 
 namespace basic_sema {
 
@@ -12,6 +12,7 @@
 }
 
 constexpr auto l_eval = [](int i) consteval {
+// expected-note@-1+ {{declared here}}
 
   return i;
 };
@@ -23,11 +24,12 @@
 
 struct A {
   consteval int f1(int i) const {
+// expected-note@-1 {{declared here}}
 return i;
   }
   consteval A(int i);
   consteval A() = default;
-  consteval ~A() = default;
+  consteval ~A() = default; // expected-error {{destructor cannot be declared consteval}}
 };
 
 consteval struct B {}; // expected-error {{struct cannot be marked consteval}}
@@ -51,14 +53,329 @@
 struct D {
   C c;
   consteval D() = default; // expected-error {{cannot be consteval}}
-  consteval ~D() = default; // expected-error {{cannot be consteval}}
+  consteval ~D() = default; // expected-error {{destructor cannot be declared consteval}}
 };
 
-struct E : C { // expected-note {{here}}
-  consteval ~E() {} // expected-error {{cannot be declared consteval because base class 'basic_sema::C' does not have a constexpr destructor}}
+struct E : C {
+  consteval ~E() {} // expected-error {{cannot be declared consteval}}
 };
 }
 
 consteval int main() { // expected-error {{'main' is not allowed to be declared consteval}}
   return 0;
 }
+
+consteval int f_eval(int i) {
+// expected-note@-1+ {{declared here}}
+  return i;
+}
+
+namespace taking_address {
+
+using func_type = int(int);
+
+func_type* p1 = (_eval);
+// expected-error@-1 {{take address}}
+func_type* p7 = __builtin_addressof(f_eval);
+// expected-error@-1 {{take address}}
+
+auto p = f_eval;
+// expected-error@-1 {{take address}}
+
+auto m1 = _sema::A::f1;
+// expected-error@-1 {{take address}}
+auto l1 = (basic_sema::l_eval)::operator();
+// expected-error@-1 {{take address}}
+
+consteval int f(int i) {
+// expected-note@-1+ {{declared here}}
+  return i;
+}
+
+auto ptr = 
+// expected-error@-1 {{take address}}
+
+auto f1() {
+  return 
+// expected-error@-1 {{take address}}
+}
+
+}
+
+namespace invalid_function {
+using size_t = unsigned long;
+struct A {
+  consteval void *operator new(size_t count);
+  // expected-error@-1 {{'operator new' cannot be declared consteval}}
+  consteval void *operator new[](size_t count);
+  // expected-error@-1 {{'operator new[]' cannot be declared consteval}}
+  consteval void operator delete(void* ptr);
+  // expected-error@-1 {{'operator delete' cannot be declared consteval}}
+  consteval void operator delete[](void* ptr);
+  // expected-error@-1 {{'operator delete[]' cannot be declared consteval}}
+  consteval ~A() {}
+  // expected-error@-1 {{destructor cannot be declared consteval}}
+};
+
+}
+
+namespace nested {
+consteval int f() {
+  return 0;
+}
+
+consteval int f1(...) {
+  return 1;
+}
+
+enum E {};
+
+using T = int(&)();
+
+consteval auto operator+ (E, int(*a)()) {
+  return 0;
+}
+
+void d() {
+  auto i = f1(E() + );
+}
+
+auto l0 = [](auto) consteval {
+  return 0;
+};
+
+int i0 = l0();
+
+int i1 = f1(l0(4));
+
+int i2 = f1(, , , , , , );
+
+int i3 = f1(f1(f1(, ), f1(, ), f1(f1(, ), )));
+
+}
+
+namespace user_defined_literal {
+
+consteval int operator"" _test(unsigned long long i) {
+// expected-note@-1+ {{declared here}}
+  return 0;
+}
+
+int i = 0_test;
+
+auto ptr = "" _test;
+// expected-error@-1 {{take address}}
+
+consteval auto operator"" _test1(unsigned long long i) {
+  return _eval;
+}
+
+auto i1 = 0_test1; // expected-error {{could not be evaluated}}
+// expected-note@-1 {{is not a constant expression}}
+
+}
+
+namespace return_address {
+
+consteval int f() {
+// expected-note@-1 {{declared here}}
+  return 0;
+}
+
+consteval int(*ret1(int i))() {
+  return 
+}
+
+auto ptr = ret1(0);
+// expected-error@-1 {{could not be evaluated}}
+// expected-note@-2 {{pointer to a consteval}}
+
+struct A {
+  consteval int f(int) {
+// expected-note@-1+ {{declared here}}
+return 0;
+  }
+};
+
+using mem_ptr_type = int (A::*)(int);
+
+template
+struct C {};
+
+C<::f> c;
+// expected-error@-1 {{is not a 

[PATCH] D41910: [Concepts] Constrained partial specializations and function overloads.

[Saar Raz via Phabricator via cfe-commits]

saar.raz updated this revision to Diff 232457.
saar.raz marked 7 inline comments as done.
saar.raz added a comment.

Address all CR comments by rsmith (including rewrite of normalization).
Decided to not support things like:

template
concept C = ...;

template requires C
struct S { };

For now, as wording is not clear what the normal form of these should be.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D41910

Files:
  clang/include/clang/AST/DeclTemplate.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/DeclTemplate.cpp
  clang/lib/Sema/SemaConcept.cpp
  clang/lib/Sema/SemaTemplate.cpp
  clang/lib/Sema/SemaTemplateDeduction.cpp
  clang/lib/Sema/SemaTemplateInstantiate.cpp
  clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
  clang/lib/Serialization/ASTReaderDecl.cpp
  clang/lib/Serialization/ASTWriterDecl.cpp
  clang/test/CXX/concepts-ts/expr/expr.prim/expr.prim.id/p3.cpp
  clang/test/CXX/concepts-ts/temp/temp.constr/temp.constr.normal/p1.cpp
  
clang/test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/class-template-partial-specializations.cpp
  
clang/test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/function-templates.cpp
  
clang/test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/var-template-partial-specializations.cpp

Index: clang/test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/var-template-partial-specializations.cpp
===
--- /dev/null
+++ clang/test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/var-template-partial-specializations.cpp
@@ -0,0 +1,53 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+template requires sizeof(T) >= 4
+bool a = false; // expected-note{{template is declared here}}
+
+template requires sizeof(T) >= 4 && sizeof(T) <= 10
+bool a = true; // expected-error{{variable template partial specialization is not more specialized than the primary template}}
+
+template
+concept C1 = sizeof(T) >= 4;
+
+template requires C1
+bool b = false;
+
+template requires C1 && sizeof(T) <= 10
+bool b = true;
+
+template
+concept C2 = sizeof(T) > 1 && sizeof(T) <= 8;
+
+template
+bool c = false;
+
+template requires C1
+bool c = true;
+
+template
+bool d = false;
+
+template
+bool d = true; // expected-error{{variable template partial specialization does not specialize any template argument; to define the primary template, remove the template argument list}}
+
+template requires C1
+bool e = false;
+
+template
+bool e = true; // expected-error{{variable template partial specialization does not specialize any template argument; to define the primary template, remove the template argument list}}
+
+template
+constexpr int f = 1;
+
+template requires C1 && C2
+constexpr int f = 2;
+
+template requires C1 || C2
+constexpr int f = 3;
+
+static_assert(f == 2);
+static_assert(f == 3);
+static_assert(f == 1);
+
+
+
Index: clang/test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/function-templates.cpp
===
--- /dev/null
+++ clang/test/CXX/concepts-ts/temp/temp.constr/temp.constr.order/function-templates.cpp
@@ -0,0 +1,82 @@
+// RUN: %clang_cc1 -std=c++2a -fconcepts-ts -x c++ -verify %s
+
+template requires sizeof(T) >= 4
+bool a() { return false; } // expected-note {{candidate function [with T = unsigned int]}}
+
+template requires sizeof(T) >= 4 && sizeof(T) <= 10
+bool a() { return true; } // expected-note {{candidate function [with T = unsigned int]}}
+
+bool av = a(); // expected-error {{call to 'a' is ambiguous}}
+
+template
+concept C1 = sizeof(T) >= 4;
+
+template requires C1
+constexpr bool b() { return false; }
+
+template requires C1 && sizeof(T) <= 10
+constexpr bool b() { return true; }
+
+static_assert(b());
+static_assert(!b());
+
+template
+concept C2 = sizeof(T) > 1 && sizeof(T) <= 8;
+
+template
+bool c() { return false; }
+
+template requires C1
+bool c() { return true; }
+
+template requires C1
+constexpr bool d() { return false; }
+
+template
+constexpr bool d() { return true; }
+
+static_assert(!d());
+
+template
+constexpr int e() { return 1; }
+
+template requires C1 && C2
+constexpr int e() { return 2; }
+
+template requires C1 || C2
+constexpr int e() { return 3; }
+
+static_assert(e() == 2);
+static_assert(e() == 3);
+static_assert(e() == 1);
+
+template
+concept BiggerThan = sizeof(T) > sizeof(U);
+
+template
+concept BiggerThanInt = BiggerThan;
+
+template requires BiggerThan
+void f() { }
+// expected-note@-1 {{candidate function [with T = long long, U = int]}}
+
+template requires BiggerThanInt
+void f() { }
+// expected-note@-1 {{candidate function [with T = long long, U = int]}}
+
+static_assert(sizeof(f()));
+// expected-error@-1 {{call to 'f' is ambiguous}}
+
+template
+concept C3 = true;
+
+template
+concept C4 = true && C3;
+

[PATCH] D63960: [C++20] Add consteval-specific semantic for functions

[Tyker via Phabricator via cfe-commits]

Tyker added inline comments.



Comment at: clang/lib/AST/ExprConstant.cpp:13618
+  if (InPlace) {
+LValue LVal;
+if (!::EvaluateInPlace(Result.Val, Info, LVal, this) ||

rsmith wrote:
> This isn't sufficient: the evaluation process can refer back to the object 
> under construction (eg, via `this`), and we need an lvalue that actually 
> names the result in order for this to work.
> 
> I think you'll need to do something like creating a suitable object (perhaps 
> a `LifetimeExtendedTemporaryDecl`) in the caller to represent the result of 
> the immediate invocation, and passing that into here.
i believe this solution should work. without LifetimeExtendedTemporaryDecl 
because reference/pointer on temporaries are not valid results of constant 
evaluation. so the AST should never store an APValue whose LValue is a 
ConstantExpr.


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

https://reviews.llvm.org/D63960



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Liu Liu via Phabricator via cfe-commits]

liuliu added a comment.

Did you mean the linkage issue?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71091



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Richard Smith - zygoloid via Phabricator via cfe-commits]

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

It's a bit weird for this to be controlled by a `-fmodules` flag, but it's only 
a `-cc1` flag, so I'm OK with that; we can rename it if/when we expose it from 
the driver.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D71041: [analyzer][discussion] Talk about escapes

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/fuchsia_handle.cpp:210
+  // Because of arrays, structs, the suggestion is to escape when whe no longer
+  // have any pointer to that symbolic region.
+  if (zx_channel_create(0, get_handle_address(), ))

xazax.hun wrote:
> xazax.hun wrote:
> > NoQ wrote:
> > > xazax.hun wrote:
> > > > xazax.hun wrote:
> > > > > xazax.hun wrote:
> > > > > > NoQ wrote:
> > > > > > > xazax.hun wrote:
> > > > > > > > NoQ wrote:
> > > > > > > > > NoQ wrote:
> > > > > > > > > > This has nothing to do with symbolic regions. We can run 
> > > > > > > > > > into this problem even if it's a local variable in the 
> > > > > > > > > > current stack frame:
> > > > > > > > > > ```lang=c++
> > > > > > > > > > void foo() {
> > > > > > > > > >   zx_handle_t sa, sb;
> > > > > > > > > >   escape(); // Escape *before* create!!
> > > > > > > > > > 
> > > > > > > > > >   zx_channel_create(0, , );
> > > > > > > > > >   zx_handle_close(sa);
> > > > > > > > > >   close_escaped();
> > > > > > > > > > }
> > > > > > > > > > ```
> > > > > > > > > > 
> > > > > > > > > > The solution that'll obviously work would be to keep track 
> > > > > > > > > > of all regions that escaped at least once, and then not 
> > > > > > > > > > even start tracking the handle if it's getting placed into 
> > > > > > > > > > a region that causes an escape when written into or has 
> > > > > > > > > > itself escaped before, but that sounds like a huge overkill.
> > > > > > > > > > 
> > > > > > > > > > Lemme think. This sounds vaguely familiar but i can't 
> > > > > > > > > > immediately recall what my thoughts were last time i 
> > > > > > > > > > thought about it.
> > > > > > > > > `$ cat test.c`
> > > > > > > > > ```lang=c++
> > > > > > > > > void manage(void **x);
> > > > > > > > > void free_managed();
> > > > > > > > > 
> > > > > > > > > void foo() {
> > > > > > > > >   void *x;
> > > > > > > > >   manage();
> > > > > > > > >   x = malloc(1);
> > > > > > > > >   free_managed();
> > > > > > > > > }
> > > > > > > > > ```
> > > > > > > > > `$ clang --analyze test.c`
> > > > > > > > > ```lang=c++
> > > > > > > > > test.c:8:3: warning: Potential leak of memory pointed to by 
> > > > > > > > > 'x'
> > > > > > > > >   free_managed();
> > > > > > > > >   ^~
> > > > > > > > > 1 warning generated.
> > > > > > > > > ```
> > > > > > > > > Sigh.
> > > > > > > > Oh, I see. Yeah this one will be fun to deal with 
> > > > > > > The rules are pretty easy though, right?
> > > > > > > ```lang=c++
> > > > > > >   2680 // A value escapes in four possible cases:
> > > > > > >   2681 // (1) We are binding to something that is not a memory 
> > > > > > > region.
> > > > > > >   2682 // (2) We are binding to a MemRegion that does not have 
> > > > > > > stack storage.
> > > > > > >   2683 // (3) We are binding to a top-level parameter region with 
> > > > > > > a non-trivial
> > > > > > >   2684 // destructor. We won't see the destructor during 
> > > > > > > analysis, but it's there.
> > > > > > >   2685 // (4) We are binding to a MemRegion with stack storage 
> > > > > > > that the store
> > > > > > >   2686 // does not understand.
> > > > > > >   2687 ProgramStateRef
> > > > > > >   2688 ExprEngine::processPointerEscapedOnBind(ProgramStateRef 
> > > > > > > State, SVal Loc,
> > > > > > >   2689 SVal Val, const 
> > > > > > > LocationContext *LCtx) {
> > > > > > > ```
> > > > > > > Basically, locals are the only special case; writing into 
> > > > > > > anything else should be an immediate escape.
> > > > > > > 
> > > > > > > We could easily update this procedure to additionally keep track 
> > > > > > > of all escaped locals in the program state, and then escape all 
> > > > > > > binds to previously escaped locals as well.
> > > > > > > 
> > > > > > > The checker would then have to follow the same rules.
> > > > > > > 
> > > > > > > In the worst case, manually.
> > > > > > > 
> > > > > > > But i think we should instead update the `checkPointerEscape()` 
> > > > > > > callback to escape the values of out-parameters upon evaluating 
> > > > > > > the call conservatively (if it doesn't already) and then teach 
> > > > > > > the checker to mark escaped regions explicitly as escaped (as 
> > > > > > > opposed to removing them from the program state), and then teach 
> > > > > > > it to never transition from escaped to opened. That would be 
> > > > > > > cleaner because that'll only require us to hardcode the escaping 
> > > > > > > procedure once.
> > > > > > > 
> > > > > > > Or we could just make the "bool 
> > > > > > > doesWriteIntoThisRegionCauseAnEscape?(Region, State)" function 
> > > > > > > re-usable.
> > > > > > Yeah.  I wonder if this procedure is the right place though. We do 
> > > > > > not actually see a bind in the code above.
> > > > > Hmm, in the stack example we do see the point of invalidation (which 
> > > > > results in 

[PATCH] D70157: Align branches within 32-Byte boundary

[Philip Reames via Phabricator via cfe-commits]

reames added a comment.

We uncovered another functional issue with this patch, or at least, the 
interaction of this patch and other parts of LLVM.  In our support for 
STATEPOINT, PATCHPOINT, and STACKMAP we use N-byte nop sequences for regions of 
code which might be patched out.  It's important that these regions are exactly 
N bytes as concurrent patching which doesn't replace an integral number of 
instructions is ill-defined on X86-64.  This patch causes the N-byte nop 
sequence to sometimes become (N+M) bytes which breaks the patching.  I believe 
that the XRAY support may have a similar issue.

More generally, I'm worried about the legality of arbitrarily prefixing 
instructions from unknown sources.  In the particular example we saw, we had 
something along the following:

.Ltmp0:
.p2align3, 0x90
(16 byte nop sequence)
.Ltmp3:
jmp *%rax

In addition to the patching legality issue above, padding the nop sequence does 
something else interesting in this example.  It changes the alignment of Ltmp3. 
 Before, Ltmp3 was always 8 byte aligned, after prefixes are added, it's not.  
It's not clear to me exactly what the required semantics here are, but we at 
least had been assuming the alignment of Ltmp3 was guaranteed in this case.  
(That's actually how we found the patching issue.)


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

https://reviews.llvm.org/D70157



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

[PATCH] D71092: [VFS] Use path canonicalization on all paths

[Adrian McCarthy via Phabricator via cfe-commits]

amccarth created this revision.
amccarth added reviewers: rnk, vsapsai, arphaman.
Herald added subscribers: llvm-commits, dexonsmith, hiraditya.
Herald added a project: LLVM.

The virtual file system had an option to eliminate dots from paths (e.g., 
`/foo/./bar` -> `/foo/bar`).  Because of differences in Windows-style paths, 
this was disabled by default on Windows.

Once the earlier VFS portability fixes land we were just one away from enable 
canonicalization on Windows.  (See https://reviews.llvm.org/D70701)

Note that the old UsePathCanonicalization member could never be changed from 
the default, so now that we canonicalize on all systems, there was no need to 
keep the option nor the alternate code paths, so this patch deletes all of that.

Tested with ninja check-clang.


https://reviews.llvm.org/D71092

Files:
  clang/test/VFS/external-names.c
  llvm/include/llvm/Support/VirtualFileSystem.h
  llvm/lib/Support/VirtualFileSystem.cpp

Index: llvm/lib/Support/VirtualFileSystem.cpp
===
--- llvm/lib/Support/VirtualFileSystem.cpp
+++ llvm/lib/Support/VirtualFileSystem.cpp
@@ -1341,16 +1341,12 @@
   return nullptr;
 
 NameValueNode = I.getValue();
-if (FS->UseCanonicalizedPaths) {
-  SmallString<256> Path(Value);
-  // Guarantee that old YAML files containing paths with ".." and "."
-  // are properly canonicalized before read into the VFS.
-  Path = sys::path::remove_leading_dotslash(Path);
-  sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
-  Name = Path.str();
-} else {
-  Name = Value;
-}
+// Guarantee that old YAML files containing paths with ".." and "."
+// are properly canonicalized before read into the VFS.
+SmallString<256> Path(Value);
+Path = sys::path::remove_leading_dotslash(Path);
+sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
+Name = Path.str();
   } else if (Key == "type") {
 if (!parseScalarString(I.getValue(), Value, Buffer))
   return nullptr;
@@ -1403,12 +1399,11 @@
   FullPath = Value;
 }
 
-if (FS->UseCanonicalizedPaths) {
-  // Guarantee that old YAML files containing paths with ".." and "."
-  // are properly canonicalized before read into the VFS.
-  FullPath = sys::path::remove_leading_dotslash(FullPath);
-  sys::path::remove_dots(FullPath, /*remove_dot_dot=*/true);
-}
+// Guarantee that old YAML files containing paths with ".." and "."
+// are properly canonicalized before read into the VFS.
+FullPath = sys::path::remove_leading_dotslash(FullPath);
+sys::path::remove_dots(FullPath, /*remove_dot_dot=*/true);
+
 ExternalContentsPath = FullPath.str();
   } else if (Key == "use-external-name") {
 bool Val;
@@ -1653,13 +1648,11 @@
   if (std::error_code EC = makeAbsolute(Path))
 return EC;
 
-  // Canonicalize path by removing ".", "..", "./", etc components. This is
-  // a VFS request, do bot bother about symlinks in the path components
+  // Canonicalize path by removing ".", "..", "./", components. This is
+  // a VFS request, do not bother about symlinks in the path components
   // but canonicalize in order to perform the correct entry search.
-  if (UseCanonicalizedPaths) {
-Path = sys::path::remove_leading_dotslash(Path);
-sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
-  }
+  Path = sys::path::remove_leading_dotslash(Path);
+  sys::path::remove_dots(Path, /*remove_dot_dot=*/true);
 
   if (Path.empty())
 return make_error_code(llvm::errc::invalid_argument);
@@ -1679,16 +1672,9 @@
 RedirectingFileSystem::lookupPath(sys::path::const_iterator Start,
   sys::path::const_iterator End,
   RedirectingFileSystem::Entry *From) const {
-#ifndef _WIN32
   assert(!isTraversalComponent(*Start) &&
  !isTraversalComponent(From->getName()) &&
  "Paths should not contain traversal components");
-#else
-  // FIXME: this is here to support windows, remove it once canonicalized
-  // paths become globally default.
-  if (Start->equals("."))
-++Start;
-#endif
 
   StringRef FromName = From->getName();
 
Index: llvm/include/llvm/Support/VirtualFileSystem.h
===
--- llvm/include/llvm/Support/VirtualFileSystem.h
+++ llvm/include/llvm/Support/VirtualFileSystem.h
@@ -705,16 +705,6 @@
   bool IsFallthrough = true;
   /// @}
 
-  /// Virtual file paths and external files could be canonicalized without "..",
-  /// "." and "./" in their paths. FIXME: some unittests currently fail on
-  /// win32 when using remove_dots and remove_leading_dotslash on paths.
-  bool UseCanonicalizedPaths =
-#ifdef _WIN32
-  false;
-#else
-  true;
-#endif
-
   

[PATCH] D70763: [clang][IFS] Allow 2 output files when using -o and -c with clang IFS stubs.

[Puyan Lotfi via Phabricator via cfe-commits]

plotfi updated this revision to Diff 232455.
plotfi added a comment.

Updating comments and altering test as per @cishida and @compnerd's feedback


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70763

Files:
  clang/lib/Driver/Driver.cpp
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/test/InterfaceStubs/driver-test3.c


Index: clang/test/InterfaceStubs/driver-test3.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/driver-test3.c
@@ -0,0 +1,19 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: shell
+
+// RUN: mkdir -p %t; cd %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs %s -o 
%t/driver-test3.o
+// RUN: llvm-nm %t/driver-test3.o | FileCheck --check-prefix=CHECK-OBJ %s
+// RUN: cat %t/driver-test3.ifs | FileCheck --check-prefix=CHECK-IFS %s
+
+// CHECK-OBJ: bar
+
+// CHECK-IFS: --- !experimental-ifs-v1
+// CHECK-IFS-NEXT: IfsVersion:
+// CHECK-IFS-NEXT: Triple:
+// CHECK-IFS-NEXT: ObjectFileFormat:
+// CHECK-IFS-NEXT: Symbols:
+// CHECK-IFS-NEXT:   "bar" : { Type: Func }
+// CHECK-IFS-NEXT: ...
+
+int bar(int a) { return a; }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -5826,8 +5826,16 @@
   if (Output.getType() == types::TY_Dependencies) {
 // Handled with other dependency code.
   } else if (Output.isFilename()) {
-CmdArgs.push_back("-o");
-CmdArgs.push_back(Output.getFilename());
+if (Output.getType() == clang::driver::types::TY_IFS_CPP ||
+Output.getType() == clang::driver::types::TY_IFS) {
+  SmallString<128> OutputFilename(Output.getFilename());
+  llvm::sys::path::replace_extension(OutputFilename, "ifs");
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Args.MakeArgString(OutputFilename));
+} else {
+  CmdArgs.push_back("-o");
+  CmdArgs.push_back(Output.getFilename());
+}
   } else {
 assert(Output.isNothing() && "Invalid output.");
   }
Index: clang/lib/Driver/Driver.cpp
===
--- clang/lib/Driver/Driver.cpp
+++ clang/lib/Driver/Driver.cpp
@@ -3689,16 +3689,27 @@
   Arg *FinalOutput = C.getArgs().getLastArg(options::OPT_o);
 
   // It is an error to provide a -o option if we are making multiple output
-  // files. There is one exception, IfsMergeJob: when generating interface 
stubs
-  // enabled we want to be able to generate the stub file at the same time that
-  // we generate the real library/a.out. So when a .o, .so, etc are the output,
-  // with clang interface stubs there will also be a .ifs and .ifso at the same
-  // location.
+  // files. There are exceptions:
+  //
+  // IfsMergeJob: when generating interface stubs enabled we want to be able to
+  // generate the stub file at the same time that we generate the real
+  // library/a.out. So when a .o, .so, etc are the output, with clang interface
+  // stubs there will also be a .ifs and .ifso at the same location.
+  //
+  // CompileJob of type TY_IFS_CPP: when generating interface stubs is enabled
+  // and -c is passed, we still want to be able to generate a .ifs file while
+  // we are also generating .o files. So we allow more than one output file in
+  // this case as well.
+  //
   if (FinalOutput) {
 unsigned NumOutputs = 0;
+unsigned NumIfsOutputs = 0;
 for (const Action *A : C.getActions())
   if (A->getType() != types::TY_Nothing &&
   !(A->getKind() == Action::IfsMergeJobClass ||
+(A->getType() == clang::driver::types::TY_IFS_CPP &&
+ A->getKind() == clang::driver::Action::CompileJobClass &&
+ 0 == NumIfsOutputs++) ||
 (A->getKind() == Action::BindArchClass && A->getInputs().size() &&
  A->getInputs().front()->getKind() == Action::IfsMergeJobClass)))
 ++NumOutputs;


Index: clang/test/InterfaceStubs/driver-test3.c
===
--- /dev/null
+++ clang/test/InterfaceStubs/driver-test3.c
@@ -0,0 +1,19 @@
+// REQUIRES: x86-registered-target
+// REQUIRES: shell
+
+// RUN: mkdir -p %t; cd %t
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs %s -o %t/driver-test3.o
+// RUN: llvm-nm %t/driver-test3.o | FileCheck --check-prefix=CHECK-OBJ %s
+// RUN: cat %t/driver-test3.ifs | FileCheck --check-prefix=CHECK-IFS %s
+
+// CHECK-OBJ: bar
+
+// CHECK-IFS: --- !experimental-ifs-v1
+// CHECK-IFS-NEXT: IfsVersion:
+// CHECK-IFS-NEXT: Triple:
+// CHECK-IFS-NEXT: ObjectFileFormat:
+// CHECK-IFS-NEXT: Symbols:
+// CHECK-IFS-NEXT:   "bar" : { Type: Func }
+// CHECK-IFS-NEXT: ...
+
+int bar(int a) { return a; }
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- 

[PATCH] D70763: [clang][IFS] Allow 2 output files when using -o and -c with clang IFS stubs.

[Puyan Lotfi via Phabricator via cfe-commits]

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



Comment at: clang/test/InterfaceStubs/driver-test3.c:6
+// RUN: %clang -target x86_64-unknown-linux-gnu -c -emit-interface-stubs %s -o 
%t/driver-test3.o
+// RUN: cat %t/driver-test3.ifs | FileCheck %s
+

cishida wrote:
> wouldn't hurt to validate that the object file was generated
ditto. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70763



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

hmm wait I have an old problem I had fixed creep up again :'(

need to fix it again sigh.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71091



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

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

[Sam Clegg via Phabricator via cfe-commits]

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



Comment at: llvm/test/MC/WebAssembly/import-module.s:13
+  .import_module  foo, bar
+  .import_name  foo, qux
+

sbc100 wrote:
> dschuff wrote:
> > What should happen if there's a directive that refers to a symbol that 
> > doesn't exist in the asm file?
> I seems that it does the same thing we do for `.functype` above, basically 
> nothing.
To be a little more specific it seems that for these kind of attributes we call 
`Ctx.getOrCreateSymbol` then set a propoerty of the symbol, but if that symbol 
isn't used elsewhere it isn't included in the output.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70877



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

[clang] b220662 - Properly convert all declaration non-type template arguments when

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2019-12-05T14:32:36-08:00
New Revision: b220662a45c8067a2ae485ae34c1138d93506df9

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

LOG: Properly convert all declaration non-type template arguments when
forming non-type template parameter values.

This reverts commit 93cc982f9e971f382ade6acf6634c5914966,
which reverted commit 11d10527852b4d3ed738aa90d8bec0f398160593.

We now always form `` when forming a pointer to a function rather than
trying to use function-to-pointer decay. This matches the behavior of
the old code in this case, but not the intent as described by the
comments.

Added: 


Modified: 
clang/lib/Sema/SemaTemplate.cpp
clang/test/SemaCXX/exceptions-seh.cpp
clang/test/SemaCXX/warn-bool-conversion.cpp
clang/test/SemaTemplate/temp_arg_nontype_cxx1z.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index e800f7fe7424..7dd1e9075c10 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -6968,100 +6968,73 @@ Sema::BuildExpressionFromDeclTemplateArgument(const 
TemplateArgument ,
 
   ValueDecl *VD = Arg.getAsDecl();
 
-  if (VD->getDeclContext()->isRecord() &&
-  (isa(VD) || isa(VD) ||
-   isa(VD))) {
-// If the value is a class member, we might have a pointer-to-member.
-// Determine whether the non-type template template parameter is of
-// pointer-to-member type. If so, we need to build an appropriate
-// expression for a pointer-to-member, since a "normal" DeclRefExpr
-// would refer to the member itself.
-if (ParamType->isMemberPointerType()) {
-  QualType ClassType
-= Context.getTypeDeclType(cast(VD->getDeclContext()));
-  NestedNameSpecifier *Qualifier
-= NestedNameSpecifier::Create(Context, nullptr, false,
-  ClassType.getTypePtr());
-  CXXScopeSpec SS;
-  SS.MakeTrivial(Context, Qualifier, Loc);
-
-  // The actual value-ness of this is unimportant, but for
-  // internal consistency's sake, references to instance methods
-  // are r-values.
-  ExprValueKind VK = VK_LValue;
-  if (isa(VD) && cast(VD)->isInstance())
-VK = VK_RValue;
-
-  ExprResult RefExpr = BuildDeclRefExpr(VD,
-
VD->getType().getNonReferenceType(),
-VK,
-Loc,
-);
-  if (RefExpr.isInvalid())
-return ExprError();
-
-  RefExpr = CreateBuiltinUnaryOp(Loc, UO_AddrOf, RefExpr.get());
-
-  // We might need to perform a trailing qualification conversion, since
-  // the element type on the parameter could be more qualified than the
-  // element type in the expression we constructed, and likewise for a
-  // function conversion.
-  bool ObjCLifetimeConversion;
-  QualType Ignored;
-  if (IsFunctionConversion(RefExpr.get()->getType(), ParamType, Ignored) ||
-  IsQualificationConversion(RefExpr.get()->getType(),
-ParamType.getUnqualifiedType(), false,
-ObjCLifetimeConversion))
-RefExpr = ImpCastExprToType(RefExpr.get(),
-ParamType.getUnqualifiedType(), CK_NoOp);
-
-  // FIXME: We need to perform derived-to-base or base-to-derived
-  // pointer-to-member conversions here too.
-  assert(!RefExpr.isInvalid() &&
- Context.hasSameType(RefExpr.get()->getType(),
- ParamType.getUnqualifiedType()));
-  return RefExpr;
-}
-  }
-
-  QualType T = VD->getType().getNonReferenceType();
+  CXXScopeSpec SS;
+  if (ParamType->isMemberPointerType()) {
+// If this is a pointer to member, we need to use a qualified name to
+// form a suitable pointer-to-member constant.
+assert(VD->getDeclContext()->isRecord() &&
+   (isa(VD) || isa(VD) ||
+isa(VD)));
+QualType ClassType
+  = Context.getTypeDeclType(cast(VD->getDeclContext()));
+NestedNameSpecifier *Qualifier
+  = NestedNameSpecifier::Create(Context, nullptr, false,
+ClassType.getTypePtr());
+SS.MakeTrivial(Context, Qualifier, Loc);
+  }
+
+  ExprResult RefExpr = BuildDeclarationNameExpr(
+  SS, DeclarationNameInfo(VD->getDeclName(), Loc), VD);
+  if (RefExpr.isInvalid())
+return ExprError();
 
-  if (ParamType->isPointerType()) {
-// When the non-type template parameter is a pointer, take the
-// address of the declaration.
-ExprResult RefExpr = BuildDeclRefExpr(VD, T, VK_LValue, Loc);
+  // For a pointer, the 

[PATCH] D71041: [analyzer][discussion] Talk about escapes

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

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/test/Analysis/fuchsia_handle.cpp:210
+  // Because of arrays, structs, the suggestion is to escape when whe no longer
+  // have any pointer to that symbolic region.
+  if (zx_channel_create(0, get_handle_address(), ))

xazax.hun wrote:
> NoQ wrote:
> > xazax.hun wrote:
> > > xazax.hun wrote:
> > > > xazax.hun wrote:
> > > > > NoQ wrote:
> > > > > > xazax.hun wrote:
> > > > > > > NoQ wrote:
> > > > > > > > NoQ wrote:
> > > > > > > > > This has nothing to do with symbolic regions. We can run into 
> > > > > > > > > this problem even if it's a local variable in the current 
> > > > > > > > > stack frame:
> > > > > > > > > ```lang=c++
> > > > > > > > > void foo() {
> > > > > > > > >   zx_handle_t sa, sb;
> > > > > > > > >   escape(); // Escape *before* create!!
> > > > > > > > > 
> > > > > > > > >   zx_channel_create(0, , );
> > > > > > > > >   zx_handle_close(sa);
> > > > > > > > >   close_escaped();
> > > > > > > > > }
> > > > > > > > > ```
> > > > > > > > > 
> > > > > > > > > The solution that'll obviously work would be to keep track of 
> > > > > > > > > all regions that escaped at least once, and then not even 
> > > > > > > > > start tracking the handle if it's getting placed into a 
> > > > > > > > > region that causes an escape when written into or has itself 
> > > > > > > > > escaped before, but that sounds like a huge overkill.
> > > > > > > > > 
> > > > > > > > > Lemme think. This sounds vaguely familiar but i can't 
> > > > > > > > > immediately recall what my thoughts were last time i thought 
> > > > > > > > > about it.
> > > > > > > > `$ cat test.c`
> > > > > > > > ```lang=c++
> > > > > > > > void manage(void **x);
> > > > > > > > void free_managed();
> > > > > > > > 
> > > > > > > > void foo() {
> > > > > > > >   void *x;
> > > > > > > >   manage();
> > > > > > > >   x = malloc(1);
> > > > > > > >   free_managed();
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > `$ clang --analyze test.c`
> > > > > > > > ```lang=c++
> > > > > > > > test.c:8:3: warning: Potential leak of memory pointed to by 'x'
> > > > > > > >   free_managed();
> > > > > > > >   ^~
> > > > > > > > 1 warning generated.
> > > > > > > > ```
> > > > > > > > Sigh.
> > > > > > > Oh, I see. Yeah this one will be fun to deal with 
> > > > > > The rules are pretty easy though, right?
> > > > > > ```lang=c++
> > > > > >   2680 // A value escapes in four possible cases:
> > > > > >   2681 // (1) We are binding to something that is not a memory 
> > > > > > region.
> > > > > >   2682 // (2) We are binding to a MemRegion that does not have 
> > > > > > stack storage.
> > > > > >   2683 // (3) We are binding to a top-level parameter region with a 
> > > > > > non-trivial
> > > > > >   2684 // destructor. We won't see the destructor during 
> > > > > > analysis, but it's there.
> > > > > >   2685 // (4) We are binding to a MemRegion with stack storage that 
> > > > > > the store
> > > > > >   2686 // does not understand.
> > > > > >   2687 ProgramStateRef
> > > > > >   2688 ExprEngine::processPointerEscapedOnBind(ProgramStateRef 
> > > > > > State, SVal Loc,
> > > > > >   2689 SVal Val, const 
> > > > > > LocationContext *LCtx) {
> > > > > > ```
> > > > > > Basically, locals are the only special case; writing into anything 
> > > > > > else should be an immediate escape.
> > > > > > 
> > > > > > We could easily update this procedure to additionally keep track of 
> > > > > > all escaped locals in the program state, and then escape all binds 
> > > > > > to previously escaped locals as well.
> > > > > > 
> > > > > > The checker would then have to follow the same rules.
> > > > > > 
> > > > > > In the worst case, manually.
> > > > > > 
> > > > > > But i think we should instead update the `checkPointerEscape()` 
> > > > > > callback to escape the values of out-parameters upon evaluating the 
> > > > > > call conservatively (if it doesn't already) and then teach the 
> > > > > > checker to mark escaped regions explicitly as escaped (as opposed 
> > > > > > to removing them from the program state), and then teach it to 
> > > > > > never transition from escaped to opened. That would be cleaner 
> > > > > > because that'll only require us to hardcode the escaping procedure 
> > > > > > once.
> > > > > > 
> > > > > > Or we could just make the "bool 
> > > > > > doesWriteIntoThisRegionCauseAnEscape?(Region, State)" function 
> > > > > > re-usable.
> > > > > Yeah.  I wonder if this procedure is the right place though. We do 
> > > > > not actually see a bind in the code above.
> > > > Hmm, in the stack example we do see the point of invalidation (which 
> > > > results in an escape). That make things easier, checkers could even 
> > > > work that problem around if they wanted to. In the original example, 
> > > > however, there is no point of invalidation, 

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder added a comment.

turns out that I had no codegen check for the call site and that one of the 
last iteration broke it trivially :'(


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71091



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

[PATCH] D70157: Align branches within 32-Byte boundary

[Fedor Sergeev via Phabricator via cfe-commits]

fedor.sergeev added a comment.

In D70157#1768310 , @skan wrote:

> fix the bug
>
> https://bugs.llvm.org/show_bug.cgi?id=44215


FYI: I did close the bug as fixed after verifying that the fix works for me.


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

https://reviews.llvm.org/D70157



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

[PATCH] D71091: Make sure that the implicit arguments for direct methods have been setup

[Pierre Habouzit via Phabricator via cfe-commits]

MadCoder created this revision.
MadCoder added reviewers: rjmccall, arphaman, dexonsmith.
MadCoder added a project: clang.
Herald added a subscriber: cfe-commits.
MadCoder added a comment.

turns out that I had no codegen check for the call site and that one of the 
last iteration broke it trivially :'(


Radar-Id: rdar://problem/57661767


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71091

Files:
  clang/lib/Sema/SemaDeclObjC.cpp
  clang/test/CodeGenObjC/direct-method.m


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -173,3 +173,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,10 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  if (ObjCMethod->isDirectMethod()) {
+ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+  }
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)


Index: clang/test/CodeGenObjC/direct-method.m
===
--- clang/test/CodeGenObjC/direct-method.m
+++ clang/test/CodeGenObjC/direct-method.m
@@ -173,3 +173,9 @@
 // CHECK-LABEL: define hidden void @"\01-[Foo setGetDynamic_setDirect:]"(
 // CHECK-LABEL: define internal void @"\01-[Foo .cxx_destruct]"(
 @end
+
+int useRoot(Root *r) {
+  // CHEC-LABEL: define i32 @useRoot
+  // CHECK: %call = call i32 bitcast {{.*}} @"\01-[Root getInt]"
+  return [r getInt];
+}
Index: clang/lib/Sema/SemaDeclObjC.cpp
===
--- clang/lib/Sema/SemaDeclObjC.cpp
+++ clang/lib/Sema/SemaDeclObjC.cpp
@@ -4836,6 +4836,10 @@
 cast(ClassDecl)->addDecl(ObjCMethod);
   }
 
+  if (ObjCMethod->isDirectMethod()) {
+ObjCMethod->createImplicitParams(Context, ObjCMethod->getClassInterface());
+  }
+
   if (PrevMethod) {
 // You can never have two method definitions with the same name.
 Diag(ObjCMethod->getLocation(), diag::err_duplicate_method_decl)
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D70877: [WebAssebmly][MC] Support .import_name/.import_field asm directives

[Sam Clegg via Phabricator via cfe-commits]

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



Comment at: llvm/test/MC/WebAssembly/import-module.s:13
+  .import_module  foo, bar
+  .import_name  foo, qux
+

dschuff wrote:
> What should happen if there's a directive that refers to a symbol that 
> doesn't exist in the asm file?
I seems that it does the same thing we do for `.functype` above, basically 
nothing.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70877



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

[PATCH] D70615: Add an -fno-temp-file flag for compilation

[Zachary Henkel via Phabricator via cfe-commits]

zahen added a comment.

In D70615#1770550 , @hans wrote:

> Seems fine to me. I'm not sure how to test the actual "don't write to temp 
> file" functionality, but at least there could be a test to check that the 
> flag gets forwarded to cc1.


Added a best guess on the flag forwarding test case


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

https://reviews.llvm.org/D70615



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

[PATCH] D71041: [analyzer][discussion] Talk about escapes

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/fuchsia_handle.cpp:301
+// So the value in some sense escaped our analysis.
+zx_handle_close(sa);
+  } else

xazax.hun wrote:
> This is also nasty.
Cf.:
```lang=c++
int *x = malloc(sizeof(int));
if (!x) {
  // leak???
}
```
Basically, you can suppress the leak warning if the symbol is constrained to a 
constant.

On the other hand, in your case you might instead want to track concrete 
handles. I.e., you could change your map from "SymbolRef => HandleState" to 
"SVal => HandleState", update it whenever a symbol collapses to a constant and 
as such becomes dead, and use linear lookup with `evalEq()` instead of map 
lookup. Or you could add a separate map for concrete ints if you're sure that 
other kinds of concrete values will never appear (you could insist to have some 
fun with statements like `x == y` where `x` is a handle symbol and `y` is a 
`LocAsInteger`, but loc-as-integers shouldn't have been introduced to begin 
with).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71041



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

[PATCH] D70958: [compiler-rt] [test] Disable ASLR on ASAN/MSAN/TSAN tests on NetBSD

[Evgenii Stepanov via Phabricator via cfe-commits]

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

LGTM




Comment at: compiler-rt/test/sanitizer_common/netbsd_commands/run_noaslr.sh:4
+PATH=${PATH}:/usr/sbin
+paxctl +a "${1}"
+exec "${@}"

krytarowski wrote:
> I propose to use `/usr/bin/paxctl` as it will be PATH and environment 
> independent. We already use this direct path approach in pkgsrc.
It this path correct? /usr/sbin in the code vs /usr/bin in the comment.


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

https://reviews.llvm.org/D70958



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

[PATCH] D70836: [analysis] Fix value tracking for pointers to qualified types

[Artem Dergachev via Phabricator via cfe-commits]

NoQ accepted this revision.
NoQ edited reviewers, added: NoQ; removed: dergachev.a.
NoQ added a comment.

Thanks!! This definitely doesn't sort out all the problems of this kind, but 
that's a strict improvement.

Do you have commit access? Or i could commit for you.




Comment at: clang/test/Analysis/uninit-val-const-likeness.c:21
+  for (int i = 0; i < params->noOfSymbols; i++)
+sum += fooList[i];
+  return sum;

I suggest adding `// no-warning` markers on the lines on which warnings were 
previously emitted. It doesn't have any physical meaning, just makes it easier 
to understand what the test is about when you accidentally break it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70836



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

[PATCH] D71090: [clangd] Navigation from definition of template specialization to primary templateFixes https://github.com/clangd/clangd/issues/212.

[Nathan Ridge via Phabricator via cfe-commits]

nridge created this revision.
nridge added a reviewer: sammccall.
Herald added subscribers: cfe-commits, usaxena95, kadircet, arphaman, jkorous, 
MaskRay, ilya-biryukov.
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D71090

Files:
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/unittests/XRefsTests.cpp


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -450,7 +450,22 @@
   +^+x;
 }
   )cpp",
-  };
+
+  R"cpp(// Declaration of explicit template specialization
+template 
+struct $decl[[Foo]] {};
+
+template <>
+struct Fo^o {};
+  )cpp",
+
+  R"cpp(// Declaration of partial template specialization
+template 
+struct $decl[[Foo]] {};
+
+template 
+struct Fo^o {};
+  )cpp"};
   for (const char *Test : Tests) {
 Annotations T(Test);
 llvm::Optional WantDecl;
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -191,10 +191,10 @@
 
   // Macros are simple: there's no declaration/definition distinction.
   // As a consequence, there's no need to look them up in the index either.
-  SourceLocation MaybeMacroLocation = SM.getMacroArgExpandedLocation(
+  SourceLocation IdentStartLoc = SM.getMacroArgExpandedLocation(
   getBeginningOfIdentifier(Pos, AST.getSourceManager(), 
AST.getLangOpts()));
   std::vector Result;
-  if (auto M = locateMacroAt(MaybeMacroLocation, AST.getPreprocessor())) {
+  if (auto M = locateMacroAt(IdentStartLoc, AST.getPreprocessor())) {
 if (auto Loc = makeLocation(AST.getASTContext(),
 M->Info->getDefinitionLoc(), *MainFilePath)) {
   LocatedSymbol Macro;
@@ -234,6 +234,17 @@
   for (const Decl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
 const Decl *Def = getDefinition(D);
 const Decl *Preferred = Def ? Def : D;
+
+// If we're at the point of declaration of a template specialization,
+// it's more useful to navigate to the template declaration.
+if (Preferred->getLocation() == IdentStartLoc) {
+  if (auto *CTSD = dyn_cast(Preferred)) {
+D = CTSD->getSpecializedTemplate();
+Def = getDefinition(D);
+Preferred = Def ? Def : D;
+  }
+}
+
 auto Loc = makeLocation(AST.getASTContext(),
 spellingLocIfSpelled(findName(Preferred), SM),
 *MainFilePath);


Index: clang-tools-extra/clangd/unittests/XRefsTests.cpp
===
--- clang-tools-extra/clangd/unittests/XRefsTests.cpp
+++ clang-tools-extra/clangd/unittests/XRefsTests.cpp
@@ -450,7 +450,22 @@
   +^+x;
 }
   )cpp",
-  };
+
+  R"cpp(// Declaration of explicit template specialization
+template 
+struct $decl[[Foo]] {};
+
+template <>
+struct Fo^o {};
+  )cpp",
+
+  R"cpp(// Declaration of partial template specialization
+template 
+struct $decl[[Foo]] {};
+
+template 
+struct Fo^o {};
+  )cpp"};
   for (const char *Test : Tests) {
 Annotations T(Test);
 llvm::Optional WantDecl;
Index: clang-tools-extra/clangd/XRefs.cpp
===
--- clang-tools-extra/clangd/XRefs.cpp
+++ clang-tools-extra/clangd/XRefs.cpp
@@ -191,10 +191,10 @@
 
   // Macros are simple: there's no declaration/definition distinction.
   // As a consequence, there's no need to look them up in the index either.
-  SourceLocation MaybeMacroLocation = SM.getMacroArgExpandedLocation(
+  SourceLocation IdentStartLoc = SM.getMacroArgExpandedLocation(
   getBeginningOfIdentifier(Pos, AST.getSourceManager(), AST.getLangOpts()));
   std::vector Result;
-  if (auto M = locateMacroAt(MaybeMacroLocation, AST.getPreprocessor())) {
+  if (auto M = locateMacroAt(IdentStartLoc, AST.getPreprocessor())) {
 if (auto Loc = makeLocation(AST.getASTContext(),
 M->Info->getDefinitionLoc(), *MainFilePath)) {
   LocatedSymbol Macro;
@@ -234,6 +234,17 @@
   for (const Decl *D : getDeclAtPosition(AST, SourceLoc, Relations)) {
 const Decl *Def = getDefinition(D);
 const Decl *Preferred = Def ? Def : D;
+
+// If we're at the point of declaration of a template specialization,
+// it's more useful to navigate to the template declaration.
+if (Preferred->getLocation() == IdentStartLoc) {
+  if (auto *CTSD = dyn_cast(Preferred)) {
+D = CTSD->getSpecializedTemplate();
+Def = getDefinition(D);
+Preferred = Def ? Def : D;
+  }
+}
+
 auto Loc = 

[PATCH] D70524: Support DebugInfo generation for auto return type for C++ functions.

[Paul Robinson via Phabricator via cfe-commits]

probinson added a comment.

In D70524#1771522 , @shafik wrote:

> @probinson I was reading the C++ "auto" return type 
>  issue and I can't come up 
> with a case where we don't have class descriptions across compilation units 
> that are not consistent wrt to auto return type. Do you have a specific 
> example?


The actual return type is known in a compile_unit where the method is defined, 
and not known in other compile_units.  If the non-defining compile_units omit 
the return type, that means "void" not "auto".  That is, one compile unit says 
it returns "void" and another compile unit says it returns something else.  
That is the inconsistency I meant.

If we use unspecified_type instead of omitting the return type, then a consumer 
knows that the method returns *something*, but it will have to look elsewhere 
to determine what that is.


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

https://reviews.llvm.org/D70524



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

[PATCH] D71041: [analyzer][discussion] Talk about escapes

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

xazax.hun marked an inline comment as done.
xazax.hun added inline comments.



Comment at: clang/test/Analysis/fuchsia_handle.cpp:210
+  // Because of arrays, structs, the suggestion is to escape when whe no longer
+  // have any pointer to that symbolic region.
+  if (zx_channel_create(0, get_handle_address(), ))

NoQ wrote:
> xazax.hun wrote:
> > xazax.hun wrote:
> > > xazax.hun wrote:
> > > > NoQ wrote:
> > > > > xazax.hun wrote:
> > > > > > NoQ wrote:
> > > > > > > NoQ wrote:
> > > > > > > > This has nothing to do with symbolic regions. We can run into 
> > > > > > > > this problem even if it's a local variable in the current stack 
> > > > > > > > frame:
> > > > > > > > ```lang=c++
> > > > > > > > void foo() {
> > > > > > > >   zx_handle_t sa, sb;
> > > > > > > >   escape(); // Escape *before* create!!
> > > > > > > > 
> > > > > > > >   zx_channel_create(0, , );
> > > > > > > >   zx_handle_close(sa);
> > > > > > > >   close_escaped();
> > > > > > > > }
> > > > > > > > ```
> > > > > > > > 
> > > > > > > > The solution that'll obviously work would be to keep track of 
> > > > > > > > all regions that escaped at least once, and then not even start 
> > > > > > > > tracking the handle if it's getting placed into a region that 
> > > > > > > > causes an escape when written into or has itself escaped 
> > > > > > > > before, but that sounds like a huge overkill.
> > > > > > > > 
> > > > > > > > Lemme think. This sounds vaguely familiar but i can't 
> > > > > > > > immediately recall what my thoughts were last time i thought 
> > > > > > > > about it.
> > > > > > > `$ cat test.c`
> > > > > > > ```lang=c++
> > > > > > > void manage(void **x);
> > > > > > > void free_managed();
> > > > > > > 
> > > > > > > void foo() {
> > > > > > >   void *x;
> > > > > > >   manage();
> > > > > > >   x = malloc(1);
> > > > > > >   free_managed();
> > > > > > > }
> > > > > > > ```
> > > > > > > `$ clang --analyze test.c`
> > > > > > > ```lang=c++
> > > > > > > test.c:8:3: warning: Potential leak of memory pointed to by 'x'
> > > > > > >   free_managed();
> > > > > > >   ^~
> > > > > > > 1 warning generated.
> > > > > > > ```
> > > > > > > Sigh.
> > > > > > Oh, I see. Yeah this one will be fun to deal with 
> > > > > The rules are pretty easy though, right?
> > > > > ```lang=c++
> > > > >   2680 // A value escapes in four possible cases:
> > > > >   2681 // (1) We are binding to something that is not a memory region.
> > > > >   2682 // (2) We are binding to a MemRegion that does not have stack 
> > > > > storage.
> > > > >   2683 // (3) We are binding to a top-level parameter region with a 
> > > > > non-trivial
> > > > >   2684 // destructor. We won't see the destructor during 
> > > > > analysis, but it's there.
> > > > >   2685 // (4) We are binding to a MemRegion with stack storage that 
> > > > > the store
> > > > >   2686 // does not understand.
> > > > >   2687 ProgramStateRef
> > > > >   2688 ExprEngine::processPointerEscapedOnBind(ProgramStateRef State, 
> > > > > SVal Loc,
> > > > >   2689 SVal Val, const 
> > > > > LocationContext *LCtx) {
> > > > > ```
> > > > > Basically, locals are the only special case; writing into anything 
> > > > > else should be an immediate escape.
> > > > > 
> > > > > We could easily update this procedure to additionally keep track of 
> > > > > all escaped locals in the program state, and then escape all binds to 
> > > > > previously escaped locals as well.
> > > > > 
> > > > > The checker would then have to follow the same rules.
> > > > > 
> > > > > In the worst case, manually.
> > > > > 
> > > > > But i think we should instead update the `checkPointerEscape()` 
> > > > > callback to escape the values of out-parameters upon evaluating the 
> > > > > call conservatively (if it doesn't already) and then teach the 
> > > > > checker to mark escaped regions explicitly as escaped (as opposed to 
> > > > > removing them from the program state), and then teach it to never 
> > > > > transition from escaped to opened. That would be cleaner because 
> > > > > that'll only require us to hardcode the escaping procedure once.
> > > > > 
> > > > > Or we could just make the "bool 
> > > > > doesWriteIntoThisRegionCauseAnEscape?(Region, State)" function 
> > > > > re-usable.
> > > > Yeah.  I wonder if this procedure is the right place though. We do not 
> > > > actually see a bind in the code above.
> > > Hmm, in the stack example we do see the point of invalidation (which 
> > > results in an escape). That make things easier, checkers could even work 
> > > that problem around if they wanted to. In the original example, however, 
> > > there is no point of invalidation, the region we get is already escaped 
> > > without seeing any bind later on. And there is no store into the escaped 
> > > region, since `zx_channel_create` is evaluated conservatively, we just 
> > > attach the state to the conjured 

[PATCH] D70615: Add an -fno-temp-file flag for compilation

[Zachary Henkel via Phabricator via cfe-commits]

zahen updated this revision to Diff 232442.
zahen added a comment.

Fixed patch


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

https://reviews.llvm.org/D70615

Files:
  clang/include/clang/Driver/Options.td
  clang/include/clang/Frontend/FrontendOptions.h
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Frontend/FrontendActions.cpp
  clang/test/Driver/clang_f_opts.c


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -571,3 +571,6 @@
 // CHECK-TRIVIAL-PATTERN-NOT: hasn't been enabled
 // CHECK-TRIVIAL-ZERO-GOOD-NOT: hasn't been enabled
 // CHECK-TRIVIAL-ZERO-BAD: hasn't been enabled
+
+// RUN: %clang -### -S -fno-temp-file %s 2>&1 | FileCheck 
-check-prefix=CHECK-NO-TEMP-FILE %s
+// CHECK-NO-TEMP-FILE: "-fno-temp-file"
\ No newline at end of file
Index: clang/lib/Frontend/FrontendActions.cpp
===
--- clang/lib/Frontend/FrontendActions.cpp
+++ clang/lib/Frontend/FrontendActions.cpp
@@ -140,7 +140,7 @@
   std::unique_ptr OS =
   CI.createOutputFile(CI.getFrontendOpts().OutputFile, /*Binary=*/true,
   /*RemoveFileOnSignal=*/false, InFile,
-  /*Extension=*/"", /*UseTemporary=*/true);
+  /*Extension=*/"", CI.getFrontendOpts().UseTemporary);
   if (!OS)
 return nullptr;
 
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1886,6 +1886,7 @@
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
   Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
+  Opts.UseTemporary = !Args.hasArg(OPT_fno_temp_file);
 
   Opts.CodeCompleteOpts.IncludeMacros
 = Args.hasArg(OPT_code_completion_macros);
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -672,7 +672,7 @@
   StringRef Extension) {
   return createOutputFile(getFrontendOpts().OutputFile, Binary,
   /*RemoveFileOnSignal=*/true, InFile, Extension,
-  /*UseTemporary=*/true);
+  getFrontendOpts().UseTemporary);
 }
 
 std::unique_ptr CompilerInstance::createNullOutputFile() {
Index: clang/lib/Driver/ToolChains/Clang.cpp
===
--- clang/lib/Driver/ToolChains/Clang.cpp
+++ clang/lib/Driver/ToolChains/Clang.cpp
@@ -4688,6 +4688,7 @@
   Args.AddLastArg(CmdArgs, options::OPT_ftime_trace_granularity_EQ);
   Args.AddLastArg(CmdArgs, options::OPT_ftrapv);
   Args.AddLastArg(CmdArgs, options::OPT_malign_double);
+  Args.AddLastArg(CmdArgs, options::OPT_fno_temp_file);
 
   if (Arg *A = Args.getLastArg(options::OPT_ftrapv_handler_EQ)) {
 CmdArgs.push_back("-ftrapv-handler");
Index: clang/include/clang/Frontend/FrontendOptions.h
===
--- clang/include/clang/Frontend/FrontendOptions.h
+++ clang/include/clang/Frontend/FrontendOptions.h
@@ -294,6 +294,9 @@
   /// Whether timestamps should be written to the produced PCH file.
   unsigned IncludeTimestamps : 1;
 
+  /// Should a temporary file be used during compilation.
+  unsigned UseTemporary : 1;
+
   CodeCompleteOptions CodeCompleteOpts;
 
   /// Specifies the output format of the AST.
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1559,6 +1559,9 @@
 def fno_strict_vtable_pointers: Flag<["-"], "fno-strict-vtable-pointers">,
   Group;
 def fno_strict_overflow : Flag<["-"], "fno-strict-overflow">, Group;
+def fno_temp_file : Flag<["-"], "fno-temp-file">, Group,
+  Flags<[CC1Option, CoreOption]>, HelpText<
+  "Directly create compilation output files. This may lead to incorrect 
incremental builds if the compiler crashes">;
 def fno_threadsafe_statics : Flag<["-"], "fno-threadsafe-statics">, 
Group,
   Flags<[CC1Option]>, HelpText<"Do not emit code to make initialization of 
local statics thread safe">;
 def fno_use_cxa_atexit : Flag<["-"], "fno-use-cxa-atexit">, Group, 
Flags<[CC1Option]>,


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -571,3 +571,6 @@
 // CHECK-TRIVIAL-PATTERN-NOT: hasn't been enabled
 // 

[clang] 338588d - Debug Info: Apply a default location for cleanups if none is available.

[Adrian Prantl via cfe-commits]

Author: Adrian Prantl
Date: 2019-12-05T13:30:23-08:00
New Revision: 338588d7cf1865f2095f5961b73cfb533bc535c4

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

LOG: Debug Info: Apply a default location for cleanups if none is available.

This unbreaks the debuginfo-tests testsuite by replacing the assertion
with a default location. There are cleanups in helper functions that
don't have a valid source location such as block copy helpers and it's
not worth tracking each of them down.

rdar://57630879

Added: 


Modified: 
clang/lib/CodeGen/CGDebugInfo.h
clang/lib/CodeGen/CodeGenFunction.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGDebugInfo.h b/clang/lib/CodeGen/CGDebugInfo.h
index 8e74f7e01965..fed79f0095b5 100644
--- a/clang/lib/CodeGen/CGDebugInfo.h
+++ b/clang/lib/CodeGen/CGDebugInfo.h
@@ -748,6 +748,7 @@ class ApplyDebugLocation {
   ApplyDebugLocation(ApplyDebugLocation &) : CGF(Other.CGF) {
 Other.CGF = nullptr;
   }
+  ApplyDebugLocation =(ApplyDebugLocation &&) = default;
 
   ~ApplyDebugLocation();
 

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index a25383f6e158..ffccbe2289d9 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -377,11 +377,14 @@ void CodeGenFunction::FinishFunction(SourceLocation 
EndLoc) {
   if (HasCleanups) {
 // Make sure the line table doesn't jump back into the body for
 // the ret after it's been at EndLoc.
+Optional AL;
 if (CGDebugInfo *DI = getDebugInfo()) {
   if (OnlySimpleReturnStmts)
 DI->EmitLocation(Builder, EndLoc);
   else
-assert(EndLoc.isValid() && "no location for inlineable cleanup calls");
+// We may not have a valid end location. Try to apply it anyway, and
+// fall back to an artificial location if needed.
+AL = ApplyDebugLocation::CreateDefaultArtificial(*this, EndLoc);
 }
 
 PopCleanupBlocks(PrologueCleanupDepth);



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

[PATCH] D70615: Add an -fno-temp-file flag for compilation

[Zachary Henkel via Phabricator via cfe-commits]

zahen updated this revision to Diff 232441.
zahen added a comment.

Updated with review feedback and formatting fixes


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

https://reviews.llvm.org/D70615

Files:
  clang/include/clang/Driver/Options.td
  clang/lib/Frontend/CompilerInstance.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/Driver/clang_f_opts.c


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -571,3 +571,6 @@
 // CHECK-TRIVIAL-PATTERN-NOT: hasn't been enabled
 // CHECK-TRIVIAL-ZERO-GOOD-NOT: hasn't been enabled
 // CHECK-TRIVIAL-ZERO-BAD: hasn't been enabled
+
+// RUN: %clang -### -S -fno-temp-file %s 2>&1 | FileCheck 
-check-prefix=CHECK-NO-TEMP-FILE %s
+// CHECK-NO-TEMP-FILE: "-fno-temp-file"
\ No newline at end of file
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1886,7 +1886,7 @@
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
   Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
-  Opts.UseTemporary = !Args.hasArg(OPT_fno_temp_file); 
+  Opts.UseTemporary = !Args.hasArg(OPT_fno_temp_file);
 
   Opts.CodeCompleteOpts.IncludeMacros
 = Args.hasArg(OPT_code_completion_macros);
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -672,7 +672,7 @@
   StringRef Extension) {
   return createOutputFile(getFrontendOpts().OutputFile, Binary,
   /*RemoveFileOnSignal=*/true, InFile, Extension,
-  getFrontendOpts().UseTemporary );
+  getFrontendOpts().UseTemporary);
 }
 
 std::unique_ptr CompilerInstance::createNullOutputFile() {
Index: clang/include/clang/Driver/Options.td
===
--- clang/include/clang/Driver/Options.td
+++ clang/include/clang/Driver/Options.td
@@ -1560,7 +1560,8 @@
   Group;
 def fno_strict_overflow : Flag<["-"], "fno-strict-overflow">, Group;
 def fno_temp_file : Flag<["-"], "fno-temp-file">, Group,
-  Flags<[CC1Option, CoreOption]>, HelpText<"Do not create a temp file during 
compilation">;
+  Flags<[CC1Option, CoreOption]>, HelpText<
+  "Directly create compilation output files. This may lead to incorrect 
incremental builds if the compiler crashes">;
 def fno_threadsafe_statics : Flag<["-"], "fno-threadsafe-statics">, 
Group,
   Flags<[CC1Option]>, HelpText<"Do not emit code to make initialization of 
local statics thread safe">;
 def fno_use_cxa_atexit : Flag<["-"], "fno-use-cxa-atexit">, Group, 
Flags<[CC1Option]>,


Index: clang/test/Driver/clang_f_opts.c
===
--- clang/test/Driver/clang_f_opts.c
+++ clang/test/Driver/clang_f_opts.c
@@ -571,3 +571,6 @@
 // CHECK-TRIVIAL-PATTERN-NOT: hasn't been enabled
 // CHECK-TRIVIAL-ZERO-GOOD-NOT: hasn't been enabled
 // CHECK-TRIVIAL-ZERO-BAD: hasn't been enabled
+
+// RUN: %clang -### -S -fno-temp-file %s 2>&1 | FileCheck -check-prefix=CHECK-NO-TEMP-FILE %s
+// CHECK-NO-TEMP-FILE: "-fno-temp-file"
\ No newline at end of file
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -1886,7 +1886,7 @@
   Opts.ModulesEmbedFiles = Args.getAllArgValues(OPT_fmodules_embed_file_EQ);
   Opts.ModulesEmbedAllFiles = Args.hasArg(OPT_fmodules_embed_all_files);
   Opts.IncludeTimestamps = !Args.hasArg(OPT_fno_pch_timestamp);
-  Opts.UseTemporary = !Args.hasArg(OPT_fno_temp_file); 
+  Opts.UseTemporary = !Args.hasArg(OPT_fno_temp_file);
 
   Opts.CodeCompleteOpts.IncludeMacros
 = Args.hasArg(OPT_code_completion_macros);
Index: clang/lib/Frontend/CompilerInstance.cpp
===
--- clang/lib/Frontend/CompilerInstance.cpp
+++ clang/lib/Frontend/CompilerInstance.cpp
@@ -672,7 +672,7 @@
   StringRef Extension) {
   return createOutputFile(getFrontendOpts().OutputFile, Binary,
   /*RemoveFileOnSignal=*/true, InFile, Extension,
-  getFrontendOpts().UseTemporary );
+  getFrontendOpts().UseTemporary);
 }
 
 std::unique_ptr CompilerInstance::createNullOutputFile() {
Index: clang/include/clang/Driver/Options.td
===
--- 

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[George Burgess IV via Phabricator via cfe-commits]

george.burgess.iv added a comment.

Thanks for looking into this!

I dunno what implications this has elsewhere, but I'd like to note a few things 
about this general approach:

- AIUI, this'll only work for FORTIFY functions which are literally calling 
`__builtin___foo_chk`; an equivalent implementation without such a builtin, 
like below, should be blocked by other code in clang 
,
 since it results in infinite recursion . glibc 
implements quite a few (most?) FORTIFY'ed functions in this way.



  void *memcpy(void *a, void *b, size_t c) {
if (__builtin_object_size(a, 0) == -1)
  return memcpy(a, b, c);
return __memcpy_chk(a, b, c, __builtin_object_size(a, 0));
  }



- This change only gives us `-D_FORTIFY_SOURCE=1`-level checking here. If we 
want to offer better support for `-D_FORTIFY_SOURCE=2` (including potentially 
FORTIFY's compile-time diagnostics), which is AIUI what most of the world uses, 
we'll need to add clang-specific support into glibc's headers. At that point, 
this patch becomes a nop, since the FORTIFY'ed memcpy becomes an overload for 
the built-in one.

In other words, I think this patch will improve our support for the pieces of 
glibc's FORTIFY that're implemented with `__builtin___*_chk`, but I don't think 
there's much more room to incrementally improve beyond that before we need to 
start hacking on glibc directly. If we get to that point, the changes we need 
to make in glibc will likely obviate the need for this patch.

So if your overarching goal here is to make clang support glibc's FORTIFY as-is 
a bit better, that's great and I'm personally cool with this patch (again, not 
being deeply aware of what interactions this may have in a non-FORTIFY'ed 
context). If the intent is to build off of this to try and make glibc's FORTIFY 
story with clang a great one, I'm unsure how useful this patch will be in the 
long run.




Comment at: clang/lib/AST/Decl.cpp:3007
+bool FunctionDecl::isReplaceableSystemFunction() const {
+  FunctionDecl const *Definition;
+  if (hasBody(Definition)) {

style nit: `const FunctionDecl *` is the preferred ordering in most of LLVM



Comment at: clang/lib/AST/Decl.cpp:3179
+  // C library do that to implement fortified version.
+  if (isReplaceableSystemFunction()) {
+return 0;

style nit: please don't use braces here or below, for consistency with nearby 
code.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082



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

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

I'd be fine with the cmake var and the env var and no driver flag.

I think clang developers on Darwin probably want the single-process `clang -c` 
behavior (that's how I interpreted what Duncan said), even though Apple as a 
distributor of clang wants the multi-process behavior. I would skip the 
platform check and in upstream clang and suggest that Apple customize the CMake 
variable when producing the XCode tools package. But, that's just my opinion, 
please disagree if you don't like that idea.

Lastly, we could make the environment variable more cross-platform by calling 
it `CLANG_SPAWN_CC1` to be inclusive of OSs where fork doesn't exist. Besides, 
doesn't clang use `posix_spawn` on Mac anyway?


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

https://reviews.llvm.org/D69825



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

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

[Vedant Kumar via Phabricator via cfe-commits]

vsk added a comment.

This is causing a failure in debuginfo-tests:

http://lab.llvm.org:8080/green/view/LLDB/job/lldb-cmake/4450/testReport/junit/debuginfo-tests/llgdb-tests/block_var_m/

Assertion failed: (EndLoc.isValid() && "no location for inlineable cleanup 
calls")


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71042



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

[PATCH] D71041: [analyzer][discussion] Talk about escapes

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added inline comments.



Comment at: clang/test/Analysis/fuchsia_handle.cpp:210
+  // Because of arrays, structs, the suggestion is to escape when whe no longer
+  // have any pointer to that symbolic region.
+  if (zx_channel_create(0, get_handle_address(), ))

xazax.hun wrote:
> xazax.hun wrote:
> > xazax.hun wrote:
> > > NoQ wrote:
> > > > xazax.hun wrote:
> > > > > NoQ wrote:
> > > > > > NoQ wrote:
> > > > > > > This has nothing to do with symbolic regions. We can run into 
> > > > > > > this problem even if it's a local variable in the current stack 
> > > > > > > frame:
> > > > > > > ```lang=c++
> > > > > > > void foo() {
> > > > > > >   zx_handle_t sa, sb;
> > > > > > >   escape(); // Escape *before* create!!
> > > > > > > 
> > > > > > >   zx_channel_create(0, , );
> > > > > > >   zx_handle_close(sa);
> > > > > > >   close_escaped();
> > > > > > > }
> > > > > > > ```
> > > > > > > 
> > > > > > > The solution that'll obviously work would be to keep track of all 
> > > > > > > regions that escaped at least once, and then not even start 
> > > > > > > tracking the handle if it's getting placed into a region that 
> > > > > > > causes an escape when written into or has itself escaped before, 
> > > > > > > but that sounds like a huge overkill.
> > > > > > > 
> > > > > > > Lemme think. This sounds vaguely familiar but i can't immediately 
> > > > > > > recall what my thoughts were last time i thought about it.
> > > > > > `$ cat test.c`
> > > > > > ```lang=c++
> > > > > > void manage(void **x);
> > > > > > void free_managed();
> > > > > > 
> > > > > > void foo() {
> > > > > >   void *x;
> > > > > >   manage();
> > > > > >   x = malloc(1);
> > > > > >   free_managed();
> > > > > > }
> > > > > > ```
> > > > > > `$ clang --analyze test.c`
> > > > > > ```lang=c++
> > > > > > test.c:8:3: warning: Potential leak of memory pointed to by 'x'
> > > > > >   free_managed();
> > > > > >   ^~
> > > > > > 1 warning generated.
> > > > > > ```
> > > > > > Sigh.
> > > > > Oh, I see. Yeah this one will be fun to deal with 
> > > > The rules are pretty easy though, right?
> > > > ```lang=c++
> > > >   2680 // A value escapes in four possible cases:
> > > >   2681 // (1) We are binding to something that is not a memory region.
> > > >   2682 // (2) We are binding to a MemRegion that does not have stack 
> > > > storage.
> > > >   2683 // (3) We are binding to a top-level parameter region with a 
> > > > non-trivial
> > > >   2684 // destructor. We won't see the destructor during analysis, 
> > > > but it's there.
> > > >   2685 // (4) We are binding to a MemRegion with stack storage that the 
> > > > store
> > > >   2686 // does not understand.
> > > >   2687 ProgramStateRef
> > > >   2688 ExprEngine::processPointerEscapedOnBind(ProgramStateRef State, 
> > > > SVal Loc,
> > > >   2689 SVal Val, const 
> > > > LocationContext *LCtx) {
> > > > ```
> > > > Basically, locals are the only special case; writing into anything else 
> > > > should be an immediate escape.
> > > > 
> > > > We could easily update this procedure to additionally keep track of all 
> > > > escaped locals in the program state, and then escape all binds to 
> > > > previously escaped locals as well.
> > > > 
> > > > The checker would then have to follow the same rules.
> > > > 
> > > > In the worst case, manually.
> > > > 
> > > > But i think we should instead update the `checkPointerEscape()` 
> > > > callback to escape the values of out-parameters upon evaluating the 
> > > > call conservatively (if it doesn't already) and then teach the checker 
> > > > to mark escaped regions explicitly as escaped (as opposed to removing 
> > > > them from the program state), and then teach it to never transition 
> > > > from escaped to opened. That would be cleaner because that'll only 
> > > > require us to hardcode the escaping procedure once.
> > > > 
> > > > Or we could just make the "bool 
> > > > doesWriteIntoThisRegionCauseAnEscape?(Region, State)" function 
> > > > re-usable.
> > > Yeah.  I wonder if this procedure is the right place though. We do not 
> > > actually see a bind in the code above.
> > Hmm, in the stack example we do see the point of invalidation (which 
> > results in an escape). That make things easier, checkers could even work 
> > that problem around if they wanted to. In the original example, however, 
> > there is no point of invalidation, the region we get is already escaped 
> > without seeing any bind later on. And there is no store into the escaped 
> > region, since `zx_channel_create` is evaluated conservatively, we just 
> > attach the state to the conjured symbol retroactively.
> > 
> > So at this point I wonder if the checker should ever track any symbols that 
> > are is bound to a non-stack region (even if we do not see the bind itself). 
> > This might circumvent most of our problems?
> > 
> > And for the stack example we can just 

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

[Adrian Prantl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGce7d35988d1c: Debug Info: Assert that location is available 
for cleanups (authored by aprantl).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D71042?vs=232431=232435#toc

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71042

Files:
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CGOpenMPRuntime.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm


Index: clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
===
--- /dev/null
+++ clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple arm64e-apple-ios13.0 -debug-info-kind=standalone 
-fobjc-arc \
+// RUN:   %s -emit-llvm -o - | FileCheck %s
+
+@interface NSObject
++ (id)alloc;
+@end
+
+@interface NSString : NSObject
+@end
+
+// CHECK: define {{.*}}@"\01-[MyData setData:]"
+// CHECK: [[DATA:%.*]] = alloca %struct.Data
+// CHECK: call %struct.Data* @_ZN4DataD1Ev(%struct.Data* [[DATA]]){{.*}}, !dbg 
[[LOC:![0-9]+]]
+// CHECK-NEXT: ret void
+
+// [[LOC]] = !DILocation(line: 0
+
+@interface MyData : NSObject
+struct Data {
+NSString *name;
+};
+@property struct Data data;
+@end
+@implementation MyData
+@end
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -377,9 +377,12 @@
   if (HasCleanups) {
 // Make sure the line table doesn't jump back into the body for
 // the ret after it's been at EndLoc.
-if (CGDebugInfo *DI = getDebugInfo())
+if (CGDebugInfo *DI = getDebugInfo()) {
   if (OnlySimpleReturnStmts)
 DI->EmitLocation(Builder, EndLoc);
+  else
+assert(EndLoc.isValid() && "no location for inlineable cleanup calls");
+}
 
 PopCleanupBlocks(PrologueCleanupDepth);
   }
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6189,7 +6189,7 @@
   // Emit the finalizer body:
   // (* %0)
   RCG.emitCleanups(CGF, N, PrivateAddr);
-  CGF.FinishFunction();
+  CGF.FinishFunction(Loc);
   return Fn;
 }
 
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -981,7 +981,7 @@
 
   generateObjCGetterBody(IMP, PID, OMD, AtomicHelperFn);
 
-  FinishFunction();
+  FinishFunction(OMD->getEndLoc());
 }
 
 static bool hasTrivialGetExpr(const ObjCPropertyImplDecl *propImpl) {
@@ -1515,7 +1515,7 @@
 
   generateObjCSetterBody(IMP, PID, AtomicHelperFn);
 
-  FinishFunction();
+  FinishFunction(OMD->getEndLoc());
 }
 
 namespace {


Index: clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
===
--- /dev/null
+++ clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple arm64e-apple-ios13.0 -debug-info-kind=standalone -fobjc-arc \
+// RUN:   %s -emit-llvm -o - | FileCheck %s
+
+@interface NSObject
++ (id)alloc;
+@end
+
+@interface NSString : NSObject
+@end
+
+// CHECK: define {{.*}}@"\01-[MyData setData:]"
+// CHECK: [[DATA:%.*]] = alloca %struct.Data
+// CHECK: call %struct.Data* @_ZN4DataD1Ev(%struct.Data* [[DATA]]){{.*}}, !dbg [[LOC:![0-9]+]]
+// CHECK-NEXT: ret void
+
+// [[LOC]] = !DILocation(line: 0
+
+@interface MyData : NSObject
+struct Data {
+NSString *name;
+};
+@property struct Data data;
+@end
+@implementation MyData
+@end
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -377,9 +377,12 @@
   if (HasCleanups) {
 // Make sure the line table doesn't jump back into the body for
 // the ret after it's been at EndLoc.
-if (CGDebugInfo *DI = getDebugInfo())
+if (CGDebugInfo *DI = getDebugInfo()) {
   if (OnlySimpleReturnStmts)
 DI->EmitLocation(Builder, EndLoc);
+  else
+assert(EndLoc.isValid() && "no location for inlineable cleanup calls");
+}
 
 PopCleanupBlocks(PrologueCleanupDepth);
   }
Index: clang/lib/CodeGen/CGOpenMPRuntime.cpp
===
--- clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6189,7 +6189,7 @@
   // Emit the finalizer body:
   // (* %0)
   RCG.emitCleanups(CGF, N, PrivateAddr);
-  CGF.FinishFunction();
+  CGF.FinishFunction(Loc);
   return Fn;
 }
 
Index: clang/lib/CodeGen/CGObjC.cpp

[PATCH] D71084: Set a source location for Objective-C accessor stubs

[Adrian Prantl via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa1a9aa17b4db: Set a source location for Objective-C accessor 
stubs even when there is no… (authored by aprantl).
Herald added a project: clang.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71084

Files:
  clang/lib/Sema/SemaObjCProperty.cpp
  clang/test/SemaObjC/default-synthesize-sourceloc.m


Index: clang/test/SemaObjC/default-synthesize-sourceloc.m
===
--- /dev/null
+++ clang/test/SemaObjC/default-synthesize-sourceloc.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -ast-dump %s | FileCheck %s
+
+// Test that accessor stubs for default-synthesized ObjC accessors
+// have a valid source location.
+
+__attribute__((objc_root_class))
+@interface NSObject
++ (id)alloc;
+@end
+
+@interface NSString : NSObject
+@end
+
+@interface MyData : NSObject
+struct Data {
+NSString *name;
+};
+@property struct Data data;
+@end
+// CHECK: ObjCImplementationDecl {{.*}}line:[[@LINE+2]]{{.*}} MyData
+// CHECK: ObjCMethodDecl {{.*}}col:23 implicit - setData: 'void'
+@implementation MyData
+@end
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1058,11 +1058,13 @@
   SourceLocation PropertyLoc) {
   ObjCMethodDecl *Decl = AccessorDecl;
   ObjCMethodDecl *ImplDecl = ObjCMethodDecl::Create(
-  Context, AtLoc, PropertyLoc, Decl->getSelector(), Decl->getReturnType(),
+  Context, AtLoc.isValid() ? AtLoc : Decl->getBeginLoc(),
+  PropertyLoc.isValid() ? PropertyLoc : Decl->getEndLoc(),
+  Decl->getSelector(), Decl->getReturnType(),
   Decl->getReturnTypeSourceInfo(), Impl, Decl->isInstanceMethod(),
-  Decl->isVariadic(), Decl->isPropertyAccessor(), /* isSynthesized*/ true,
-  Decl->isImplicit(), Decl->isDefined(), Decl->getImplementationControl(),
-  Decl->hasRelatedResultType());
+  Decl->isVariadic(), Decl->isPropertyAccessor(),
+  /* isSynthesized*/ true, Decl->isImplicit(), Decl->isDefined(),
+  Decl->getImplementationControl(), Decl->hasRelatedResultType());
   ImplDecl->getMethodFamily();
   if (Decl->hasAttrs())
 ImplDecl->setAttrs(Decl->getAttrs());


Index: clang/test/SemaObjC/default-synthesize-sourceloc.m
===
--- /dev/null
+++ clang/test/SemaObjC/default-synthesize-sourceloc.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -ast-dump %s | FileCheck %s
+
+// Test that accessor stubs for default-synthesized ObjC accessors
+// have a valid source location.
+
+__attribute__((objc_root_class))
+@interface NSObject
++ (id)alloc;
+@end
+
+@interface NSString : NSObject
+@end
+
+@interface MyData : NSObject
+struct Data {
+NSString *name;
+};
+@property struct Data data;
+@end
+// CHECK: ObjCImplementationDecl {{.*}}line:[[@LINE+2]]{{.*}} MyData
+// CHECK: ObjCMethodDecl {{.*}}col:23 implicit - setData: 'void'
+@implementation MyData
+@end
Index: clang/lib/Sema/SemaObjCProperty.cpp
===
--- clang/lib/Sema/SemaObjCProperty.cpp
+++ clang/lib/Sema/SemaObjCProperty.cpp
@@ -1058,11 +1058,13 @@
   SourceLocation PropertyLoc) {
   ObjCMethodDecl *Decl = AccessorDecl;
   ObjCMethodDecl *ImplDecl = ObjCMethodDecl::Create(
-  Context, AtLoc, PropertyLoc, Decl->getSelector(), Decl->getReturnType(),
+  Context, AtLoc.isValid() ? AtLoc : Decl->getBeginLoc(),
+  PropertyLoc.isValid() ? PropertyLoc : Decl->getEndLoc(),
+  Decl->getSelector(), Decl->getReturnType(),
   Decl->getReturnTypeSourceInfo(), Impl, Decl->isInstanceMethod(),
-  Decl->isVariadic(), Decl->isPropertyAccessor(), /* isSynthesized*/ true,
-  Decl->isImplicit(), Decl->isDefined(), Decl->getImplementationControl(),
-  Decl->hasRelatedResultType());
+  Decl->isVariadic(), Decl->isPropertyAccessor(),
+  /* isSynthesized*/ true, Decl->isImplicit(), Decl->isDefined(),
+  Decl->getImplementationControl(), Decl->hasRelatedResultType());
   ImplDecl->getMethodFamily();
   if (Decl->hasAttrs())
 ImplDecl->setAttrs(Decl->getAttrs());
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] ce7d359 - Debug Info: Assert that location is available for cleanups

[Adrian Prantl via cfe-commits]

Author: Adrian Prantl
Date: 2019-12-05T12:45:10-08:00
New Revision: ce7d35988d1ca5e17758283804ea1f76389dc1f0

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

LOG: Debug Info: Assert that location is available for cleanups

rdar://57630879

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

Added: 
clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm

Modified: 
clang/lib/CodeGen/CGObjC.cpp
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CodeGenFunction.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGObjC.cpp b/clang/lib/CodeGen/CGObjC.cpp
index 14391f3b129a..6db05391722d 100644
--- a/clang/lib/CodeGen/CGObjC.cpp
+++ b/clang/lib/CodeGen/CGObjC.cpp
@@ -981,7 +981,7 @@ void 
CodeGenFunction::GenerateObjCGetter(ObjCImplementationDecl *IMP,
 
   generateObjCGetterBody(IMP, PID, OMD, AtomicHelperFn);
 
-  FinishFunction();
+  FinishFunction(OMD->getEndLoc());
 }
 
 static bool hasTrivialGetExpr(const ObjCPropertyImplDecl *propImpl) {
@@ -1515,7 +1515,7 @@ void 
CodeGenFunction::GenerateObjCSetter(ObjCImplementationDecl *IMP,
 
   generateObjCSetterBody(IMP, PID, AtomicHelperFn);
 
-  FinishFunction();
+  FinishFunction(OMD->getEndLoc());
 }
 
 namespace {

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 8ce403c8dab0..9f1b907e89db 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -6189,7 +6189,7 @@ static llvm::Value *emitReduceFiniFunction(CodeGenModule 
,
   // Emit the finalizer body:
   // (* %0)
   RCG.emitCleanups(CGF, N, PrivateAddr);
-  CGF.FinishFunction();
+  CGF.FinishFunction(Loc);
   return Fn;
 }
 

diff  --git a/clang/lib/CodeGen/CodeGenFunction.cpp 
b/clang/lib/CodeGen/CodeGenFunction.cpp
index fd3020835a4d..a25383f6e158 100644
--- a/clang/lib/CodeGen/CodeGenFunction.cpp
+++ b/clang/lib/CodeGen/CodeGenFunction.cpp
@@ -377,9 +377,12 @@ void CodeGenFunction::FinishFunction(SourceLocation 
EndLoc) {
   if (HasCleanups) {
 // Make sure the line table doesn't jump back into the body for
 // the ret after it's been at EndLoc.
-if (CGDebugInfo *DI = getDebugInfo())
+if (CGDebugInfo *DI = getDebugInfo()) {
   if (OnlySimpleReturnStmts)
 DI->EmitLocation(Builder, EndLoc);
+  else
+assert(EndLoc.isValid() && "no location for inlineable cleanup calls");
+}
 
 PopCleanupBlocks(PrologueCleanupDepth);
   }

diff  --git a/clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm 
b/clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
new file mode 100644
index ..805766b23bdb
--- /dev/null
+++ b/clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple arm64e-apple-ios13.0 -debug-info-kind=standalone 
-fobjc-arc \
+// RUN:   %s -emit-llvm -o - | FileCheck %s
+
+@interface NSObject
++ (id)alloc;
+@end
+
+@interface NSString : NSObject
+@end
+
+// CHECK: define {{.*}}@"\01-[MyData setData:]"
+// CHECK: [[DATA:%.*]] = alloca %struct.Data
+// CHECK: call %struct.Data* @_ZN4DataD1Ev(%struct.Data* [[DATA]]){{.*}}, !dbg 
[[LOC:![0-9]+]]
+// CHECK-NEXT: ret void
+
+// [[LOC]] = !DILocation(line: 0
+
+@interface MyData : NSObject
+struct Data {
+NSString *name;
+};
+@property struct Data data;
+@end
+@implementation MyData
+@end



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

[clang] a1a9aa1 - Set a source location for Objective-C accessor stubs

[Adrian Prantl via cfe-commits]

Author: Adrian Prantl
Date: 2019-12-05T12:45:10-08:00
New Revision: a1a9aa17b4db08937e458cdda85327b9eff307df

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

LOG: Set a source location for Objective-C accessor stubs
even when there is no explicit synthesize statement.

This fixes a regression introduced in https://reviews.llvm.org/D68108
that could lead to missing debug locations in cleanup code in
synthesized Objective-C++ properties.

rdar://57630879

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

Added: 
clang/test/SemaObjC/default-synthesize-sourceloc.m

Modified: 
clang/lib/Sema/SemaObjCProperty.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaObjCProperty.cpp 
b/clang/lib/Sema/SemaObjCProperty.cpp
index 87f7baaf568f..2d91ea1f5fd0 100644
--- a/clang/lib/Sema/SemaObjCProperty.cpp
+++ b/clang/lib/Sema/SemaObjCProperty.cpp
@@ -1058,11 +1058,13 @@ RedeclarePropertyAccessor(ASTContext , 
ObjCImplementationDecl *Impl,
   SourceLocation PropertyLoc) {
   ObjCMethodDecl *Decl = AccessorDecl;
   ObjCMethodDecl *ImplDecl = ObjCMethodDecl::Create(
-  Context, AtLoc, PropertyLoc, Decl->getSelector(), Decl->getReturnType(),
+  Context, AtLoc.isValid() ? AtLoc : Decl->getBeginLoc(),
+  PropertyLoc.isValid() ? PropertyLoc : Decl->getEndLoc(),
+  Decl->getSelector(), Decl->getReturnType(),
   Decl->getReturnTypeSourceInfo(), Impl, Decl->isInstanceMethod(),
-  Decl->isVariadic(), Decl->isPropertyAccessor(), /* isSynthesized*/ true,
-  Decl->isImplicit(), Decl->isDefined(), Decl->getImplementationControl(),
-  Decl->hasRelatedResultType());
+  Decl->isVariadic(), Decl->isPropertyAccessor(),
+  /* isSynthesized*/ true, Decl->isImplicit(), Decl->isDefined(),
+  Decl->getImplementationControl(), Decl->hasRelatedResultType());
   ImplDecl->getMethodFamily();
   if (Decl->hasAttrs())
 ImplDecl->setAttrs(Decl->getAttrs());

diff  --git a/clang/test/SemaObjC/default-synthesize-sourceloc.m 
b/clang/test/SemaObjC/default-synthesize-sourceloc.m
new file mode 100644
index ..bd549fff17c1
--- /dev/null
+++ b/clang/test/SemaObjC/default-synthesize-sourceloc.m
@@ -0,0 +1,23 @@
+// RUN: %clang_cc1 -ast-dump %s | FileCheck %s
+
+// Test that accessor stubs for default-synthesized ObjC accessors
+// have a valid source location.
+
+__attribute__((objc_root_class))
+@interface NSObject
++ (id)alloc;
+@end
+
+@interface NSString : NSObject
+@end
+
+@interface MyData : NSObject
+struct Data {
+NSString *name;
+};
+@property struct Data data;
+@end
+// CHECK: ObjCImplementationDecl {{.*}}line:[[@LINE+2]]{{.*}} MyData
+// CHECK: ObjCMethodDecl {{.*}}col:23 implicit - setData: 'void'
+@implementation MyData
+@end



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

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

[Adrian Prantl via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:384
+  else
+assert(EndLoc.isValid() && "no location for inlineable cleanup calls");
+}

vsk wrote:
> The braces around the `else` block are probably needed in the NDEBUG build.
No, this should be fine. The `;` is not part of the macro.


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

https://reviews.llvm.org/D71042



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

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

[Vedant Kumar via Phabricator via cfe-commits]

vsk added inline comments.



Comment at: clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm:16
+
+// [[LOC]] = !DILocation(line: 0
+

Heads-up: this location should be different once 
https://reviews.llvm.org/D71084 lands, which I assume this depends on.


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

https://reviews.llvm.org/D71042



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

[PATCH] D71084: Set a source location for Objective-C accessor stubs

[Vedant Kumar via Phabricator via cfe-commits]

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

LGTM.


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

https://reviews.llvm.org/D71084



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

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

[Vedant Kumar via Phabricator via cfe-commits]

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

Thanks, lgtm with a minor change.




Comment at: clang/lib/CodeGen/CodeGenFunction.cpp:384
+  else
+assert(EndLoc.isValid() && "no location for inlineable cleanup calls");
+}

The braces around the `else` block are probably needed in the NDEBUG build.


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

https://reviews.llvm.org/D71042



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

[PATCH] D67537: [clangd] Client-side support for inactive regions

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Review ping :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D67537



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

[PATCH] D61842: [clangd] [WIP] [Not ready for review] Semantic highlighting

[Nathan Ridge via Phabricator via cfe-commits]

nridge abandoned this revision.
nridge added a comment.
Herald added a subscriber: usaxena95.

Abandoning as all of the functionality in this prototype is now present in 
mainline (except for the ability to color declarations differently from 
references, which is blocked on a spec change as discussed in D66990 
).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D61842



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

[PATCH] D71042: [DebugInfo] Ensure fallback artificial location is available for cleanups

[Adrian Prantl via Phabricator via cfe-commits]

aprantl updated this revision to Diff 232431.
aprantl added a comment.

Updated patch to assert a valid EndLoc and actually pass it in.


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

https://reviews.llvm.org/D71042

Files:
  clang/lib/CodeGen/CGObjC.cpp
  clang/lib/CodeGen/CodeGenFunction.cpp
  clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm


Index: clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
===
--- /dev/null
+++ clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple arm64e-apple-ios13.0 -debug-info-kind=standalone 
-fobjc-arc \
+// RUN:   %s -emit-llvm -o - | FileCheck %s
+
+@interface NSObject
++ (id)alloc;
+@end
+
+@interface NSString : NSObject
+@end
+
+// CHECK: define {{.*}}@"\01-[MyData setData:]"
+// CHECK: [[DATA:%.*]] = alloca %struct.Data
+// CHECK: call %struct.Data* @_ZN4DataD1Ev(%struct.Data* [[DATA]]){{.*}}, !dbg 
[[LOC:![0-9]+]]
+// CHECK-NEXT: ret void
+
+// [[LOC]] = !DILocation(line: 0
+
+@interface MyData : NSObject
+struct Data {
+NSString *name;
+};
+@property struct Data data;
+@end
+@implementation MyData
+@end
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -377,9 +377,12 @@
   if (HasCleanups) {
 // Make sure the line table doesn't jump back into the body for
 // the ret after it's been at EndLoc.
-if (CGDebugInfo *DI = getDebugInfo())
+if (CGDebugInfo *DI = getDebugInfo()) {
   if (OnlySimpleReturnStmts)
 DI->EmitLocation(Builder, EndLoc);
+  else
+assert(EndLoc.isValid() && "no location for inlineable cleanup calls");
+}
 
 PopCleanupBlocks(PrologueCleanupDepth);
   }
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -981,7 +981,7 @@
 
   generateObjCGetterBody(IMP, PID, OMD, AtomicHelperFn);
 
-  FinishFunction();
+  FinishFunction(OMD->getEndLoc());
 }
 
 static bool hasTrivialGetExpr(const ObjCPropertyImplDecl *propImpl) {
@@ -1515,7 +1515,7 @@
 
   generateObjCSetterBody(IMP, PID, AtomicHelperFn);
 
-  FinishFunction();
+  FinishFunction(OMD->getEndLoc());
 }
 
 namespace {


Index: clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
===
--- /dev/null
+++ clang/test/CodeGenObjCXX/synthesized-property-cleanup.mm
@@ -0,0 +1,25 @@
+// RUN: %clang_cc1 -triple arm64e-apple-ios13.0 -debug-info-kind=standalone -fobjc-arc \
+// RUN:   %s -emit-llvm -o - | FileCheck %s
+
+@interface NSObject
++ (id)alloc;
+@end
+
+@interface NSString : NSObject
+@end
+
+// CHECK: define {{.*}}@"\01-[MyData setData:]"
+// CHECK: [[DATA:%.*]] = alloca %struct.Data
+// CHECK: call %struct.Data* @_ZN4DataD1Ev(%struct.Data* [[DATA]]){{.*}}, !dbg [[LOC:![0-9]+]]
+// CHECK-NEXT: ret void
+
+// [[LOC]] = !DILocation(line: 0
+
+@interface MyData : NSObject
+struct Data {
+NSString *name;
+};
+@property struct Data data;
+@end
+@implementation MyData
+@end
Index: clang/lib/CodeGen/CodeGenFunction.cpp
===
--- clang/lib/CodeGen/CodeGenFunction.cpp
+++ clang/lib/CodeGen/CodeGenFunction.cpp
@@ -377,9 +377,12 @@
   if (HasCleanups) {
 // Make sure the line table doesn't jump back into the body for
 // the ret after it's been at EndLoc.
-if (CGDebugInfo *DI = getDebugInfo())
+if (CGDebugInfo *DI = getDebugInfo()) {
   if (OnlySimpleReturnStmts)
 DI->EmitLocation(Builder, EndLoc);
+  else
+assert(EndLoc.isValid() && "no location for inlineable cleanup calls");
+}
 
 PopCleanupBlocks(PrologueCleanupDepth);
   }
Index: clang/lib/CodeGen/CGObjC.cpp
===
--- clang/lib/CodeGen/CGObjC.cpp
+++ clang/lib/CodeGen/CGObjC.cpp
@@ -981,7 +981,7 @@
 
   generateObjCGetterBody(IMP, PID, OMD, AtomicHelperFn);
 
-  FinishFunction();
+  FinishFunction(OMD->getEndLoc());
 }
 
 static bool hasTrivialGetExpr(const ObjCPropertyImplDecl *propImpl) {
@@ -1515,7 +1515,7 @@
 
   generateObjCSetterBody(IMP, PID, AtomicHelperFn);
 
-  FinishFunction();
+  FinishFunction(OMD->getEndLoc());
 }
 
 namespace {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D71084: Set a source location for Objective-C accessor stubs

[Adrian Prantl via Phabricator via cfe-commits]

aprantl added a comment.

In D71084#1771501 , @vsk wrote:

> Shall we add in the assert we discussed in CFG::FinishFunction, as an 
> additional test?


Done over in https://reviews.llvm.org/D71042


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

https://reviews.llvm.org/D71084



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

[PATCH] D69825: [Clang][Driver] Re-use the calling process instead of creating a new process for the cc1 invocation

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added a comment.

The environment variable should be enough IMHO


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

https://reviews.llvm.org/D69825



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

[PATCH] D71005: [AST] Enable expression of OpenCL language address spaces an attribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D71005#1771513 , @saugustine wrote:

> When I try to run tablegen on this file for clang/docs (as described in 
> clang/docs/InternalsManual.rst), I get an error, introduced by this change:
>
> $ ./bin/clang-tblgen -gen-attr-docs -I ../llvm-project/clang/include 
> ../llvm-project/clang/include/clang/Basic/Attr.td -o 
> /tmp/AttributeReference.rst
>
> ../llvm-project/clang/include/clang/Basic/Attr.td:1137:1: error: This 
> attribute requires a heading to be specified
>  def OpenCLConstantAddressSpace : TypeAttr {
>  ^
>
> I'm at a loss as to what is wrong, but I would like to revert this change 
> until it is fixed.


This should be fixed in f5193d87feaedb411255e92979abd6b62522bc38 
 now, 
thank you for pointing it out!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71005



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

[clang] f5193d8 - Add documentation headings for the OpenCL attributes.

[Aaron Ballman via cfe-commits]

Author: Aaron Ballman
Date: 2019-12-05T15:12:01-05:00
New Revision: f5193d87feaedb411255e92979abd6b62522bc38

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

LOG: Add documentation headings for the OpenCL attributes.

This fixes the documentation build.

Added: 


Modified: 
clang/include/clang/Basic/AttrDocs.td

Removed: 




diff  --git a/clang/include/clang/Basic/AttrDocs.td 
b/clang/include/clang/Basic/AttrDocs.td
index d47315a3f518..c309785a534c 100644
--- a/clang/include/clang/Basic/AttrDocs.td
+++ b/clang/include/clang/Basic/AttrDocs.td
@@ -3066,6 +3066,7 @@ More details can be found in the OpenCL C language Spec 
v2.0, Section 6.5.
 
 def OpenCLAddressSpaceGenericDocs : Documentation {
   let Category = DocOpenCLAddressSpaces;
+  let Heading = "__generic, generic, [[clang::opencl_generic]]";
   let Content = [{
 The generic address space attribute is only available with OpenCL v2.0 and 
later.
 It can be used with pointer types. Variables in global and local scope and
@@ -3078,6 +3079,7 @@ spaces.
 
 def OpenCLAddressSpaceConstantDocs : Documentation {
   let Category = DocOpenCLAddressSpaces;
+  let Heading = "__constant, constant, [[clang::opencl_constant]]";
   let Content = [{
 The constant address space attribute signals that an object is located in
 a constant (non-modifiable) memory region. It is available to all work items.
@@ -3089,6 +3091,7 @@ have an initializer.
 
 def OpenCLAddressSpaceGlobalDocs : Documentation {
   let Category = DocOpenCLAddressSpaces;
+  let Heading = "__global, global, [[clang::opencl_global]]";
   let Content = [{
 The global address space attribute specifies that an object is allocated in
 global memory, which is accessible by all work items. The content stored in 
this
@@ -3101,6 +3104,7 @@ scope) variables and static local variable as well.
 
 def OpenCLAddressSpaceLocalDocs : Documentation {
   let Category = DocOpenCLAddressSpaces;
+  let Heading = "__local, local, [[clang::opencl_local]]";
   let Content = [{
 The local address space specifies that an object is allocated in the local 
(work
 group) memory area, which is accessible to all work items in the same work
@@ -3113,6 +3117,7 @@ space are allowed. Local address space variables cannot 
have an initializer.
 
 def OpenCLAddressSpacePrivateDocs : Documentation {
   let Category = DocOpenCLAddressSpaces;
+  let Heading = "__private, private, [[clang::opencl_private]]";
   let Content = [{
 The private address space specifies that an object is allocated in the private
 (work item) memory. Other work items cannot access the same memory area and its



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

[PATCH] D69779: -fmodules-codegen should not emit extern templates

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69779



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

[PATCH] D69778: Make -fmodules-codegen and -fmodules-debuginfo work also with precompiled headers

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


Repository:
  rC Clang

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

https://reviews.llvm.org/D69778



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

[PATCH] D71082: Allow system header to provide their own implementation of some builtin

[serge via Phabricator via cfe-commits]

serge-sans-paille updated this revision to Diff 232425.
serge-sans-paille marked an inline comment as done.
serge-sans-paille added a comment.

test case updated


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71082

Files:
  clang/include/clang/AST/Decl.h
  clang/lib/AST/Decl.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/test/CodeGen/memcpy-nobuitin.c
  clang/test/CodeGen/memcpy-nobuitin.inc


Index: clang/test/CodeGen/memcpy-nobuitin.inc
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.inc
@@ -0,0 +1,12 @@
+#include 
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+*idest++ = *ifrom++;
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuitin.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.c
@@ -0,0 +1,8 @@
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -DWITH_DECL | FileCheck 
--check-prefix=CHECK-WITH-DECL %s
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -UWITH_DECL | FileCheck 
--check-prefix=CHECK-NO-DECL %s
+// CHECK-WITH-DECL-NOT: @llvm.memcpy
+// CHECK-NO-DECL: @llvm.memcpy
+#include 
+void test(void *dest, void const *from, size_t n) {
+  memcpy(dest, from, n);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -1831,6 +1831,11 @@
   else if (const auto *SA = FD->getAttr())
  F->setSection(SA->getName());
 
+  if (FD->isReplaceableSystemFunction()) {
+F->addAttribute(llvm::AttributeList::FunctionIndex,
+llvm::Attribute::NoBuiltin);
+  }
+
   if (FD->isReplaceableGlobalAllocationFunction()) {
 // A replaceable global allocation function does not act like a builtin by
 // default, only if it is invoked by a new-expression or delete-expression.
Index: clang/lib/AST/Decl.cpp
===
--- clang/lib/AST/Decl.cpp
+++ clang/lib/AST/Decl.cpp
@@ -3003,6 +3003,15 @@
   return Params == FPT->getNumParams();
 }
 
+bool FunctionDecl::isReplaceableSystemFunction() const {
+  FunctionDecl const *Definition;
+  if (hasBody(Definition)) {
+const SourceManager  = getASTContext().getSourceManager();
+return SM.isInSystemHeader(Definition->getLocation());
+  }
+  return false;
+}
+
 bool FunctionDecl::isDestroyingOperatorDelete() const {
   // C++ P0722:
   //   Within a class C, a single object deallocation function with signature
@@ -3165,6 +3174,12 @@
   // function. Determine whether it actually refers to the C library
   // function or whether it just has the same name.
 
+  // If a system-level body was provided, use it instead of the intrinsic. Some
+  // C library do that to implement fortified version.
+  if (isReplaceableSystemFunction()) {
+return 0;
+  }
+
   // If this is a static function, it's not a builtin.
   if (!ConsiderWrapperFunctions && getStorageClass() == SC_Static)
 return 0;
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -2272,6 +2272,9 @@
   /// true through IsAligned.
   bool isReplaceableGlobalAllocationFunction(bool *IsAligned = nullptr) const;
 
+  /// Determine if this function provides the implementation of a system 
Builtin
+  bool isReplaceableSystemFunction() const;
+
   /// Determine whether this is a destroying operator delete.
   bool isDestroyingOperatorDelete() const;
 


Index: clang/test/CodeGen/memcpy-nobuitin.inc
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.inc
@@ -0,0 +1,12 @@
+#include 
+extern void *memcpy(void *dest, void const *from, size_t n);
+
+#ifdef WITH_DECL
+inline void *memcpy(void *dest, void const *from, size_t n) {
+  char const *ifrom = from;
+  char *idest = dest;
+  while (n--)
+*idest++ = *ifrom++;
+  return dest;
+}
+#endif
Index: clang/test/CodeGen/memcpy-nobuitin.c
===
--- /dev/null
+++ clang/test/CodeGen/memcpy-nobuitin.c
@@ -0,0 +1,8 @@
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -DWITH_DECL | FileCheck --check-prefix=CHECK-WITH-DECL %s
+// RUN: %clang -S -emit-llvm -o- %s -isystem %S -UWITH_DECL | FileCheck --check-prefix=CHECK-NO-DECL %s
+// CHECK-WITH-DECL-NOT: @llvm.memcpy
+// CHECK-NO-DECL: @llvm.memcpy
+#include 
+void test(void *dest, void const *from, size_t n) {
+  memcpy(dest, from, n);
+}
Index: clang/lib/CodeGen/CodeGenModule.cpp
===

[PATCH] D69585: PerformPendingInstatiations() already in the PCH

[Luboš Luňák via Phabricator via cfe-commits]

llunak added a comment.

Ping.


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

https://reviews.llvm.org/D69585



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

[PATCH] D70696: [DebugInfo] Support to emit debugInfo for extern variables

[Yonghong Song via Phabricator via cfe-commits]

yonghong-song added a comment.

@dblaikie I consolidated test cases as you suggested. Along the way, I also 
added some more tests. I already answered some of your questions earlier. 
Please let me know what you think.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D70696



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