[clang] e4422ae - Rewrite the non-trivial structs section of the ARC spec.

[John McCall via cfe-commits]

Author: John McCall
Date: 2020-03-06T02:51:45-05:00
New Revision: e4422ae0f6e4159a8560514ce221306c30a7f2c1

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

LOG: Rewrite the non-trivial structs section of the ARC spec.

As part of this, set down the general rules for non-trivial types
in C in their full and gory detail, and then separately describe how
they apply to the ARC qualified types.

I'm not totally satisfied with the drafting of the dynamic-objects UB
rules here, but I feel like I'm building on a lot of wreckage.

Added: 


Modified: 
clang/docs/AutomaticReferenceCounting.rst

Removed: 




diff  --git a/clang/docs/AutomaticReferenceCounting.rst 
b/clang/docs/AutomaticReferenceCounting.rst
index 7b86d169ac49..c75ef025415b 100644
--- a/clang/docs/AutomaticReferenceCounting.rst
+++ b/clang/docs/AutomaticReferenceCounting.rst
@@ -1101,7 +1101,13 @@ Ownership-qualified fields of structs and unions
 
 
 A member of a struct or union may be declared to have ownership-qualified
-type, except that it may not be declared to be ``__autoreleasing``.
+type.  If the type is qualified with ``__unsafe_unretained``, the semantics
+of the containing aggregate are unchanged from the semantics of an unqualified 
type in a non-ARC mode.  If the type is qualified with ``__autoreleasing``, the 
program is ill-formed.  Otherwise, if the type is nontrivially 
ownership-qualified, additional rules apply.
+
+Both Objective-C and Objective-C++ support nontrivially ownership-qualified
+fields.  Due to formal 
diff erences between the standards, the formal
+treatment is 
diff erent; however, the basic language model is intended to
+be the same for identical code.
 
 .. admonition:: Rationale
 
@@ -,7 +1117,7 @@ type, except that it may not be declared to be 
``__autoreleasing``.
   usually simpler and more idiomatic to use Objective-C objects for
   secondary data structures, doing so can introduce extra allocation
   and message-send overhead, which can cause to unacceptable
-  performance.  Using structs can resolve this tension.
+  performance.  Using structs can resolve some of this tension.
 
   ``__autoreleasing`` is forbidden because it is treacherous to rely
   on autoreleases as an ownership tool outside of a function-local
@@ -1122,36 +1128,186 @@ type, except that it may not be declared to be 
``__autoreleasing``.
   restriction was an undesirable short-term constraint arising from the
   complexity of adding support for non-trivial struct types to C.
 
-In Objective-C++, for the purposes of determining triviality of special
-members, nontrivially ownership-qualified types are treated as if they
-were class types with:
-- non-trivial default, copy, and move constructors,
-- non-trivial copy and move assignment operators, and
-- non-trivial destructors.
-
-In Objective-C, language rules have been added to cover non-trivial
-members of struct and union types.  These rules generally match the
-Objective-C++ behavior and can be summarized as follows:
-
-- Initializing, copying, or destroying a struct with non-trivial
-  members recursively initializes, copies or destroys those members
-  as appropriate for their type.
-
-- Copying or destroying a union with non-trivial members is ill-formed.
-  (This also applies to structs containing such unions.)  It is
-  nonetheless possible to create objects of these types by
-  zero-initializing suitable memory before accessing it through the
-  union type, and they may be destroyed by ensuring that any active
-  members are reset to ``nil`` before the memory is re-used.  These
-  techniques mirror the precautions necessary when working with
-  dynamically-allocated arrays of nontrivially ownership-qualified type.
+In Objective-C++, nontrivially ownership-qualified types are treated
+for nearly all purposes as if they were class types with non-trivial
+default constructors, copy constructors, move constructors, copy assignment
+operators, move assignment operators, and destructors.  This includes the
+determination of the triviality of special members of classes with a
+non-static data member of such a type.
+
+In Objective-C, the definition cannot be so succinct: because the C
+standard lacks rules for non-trivial types, those rules must first be
+developed.  They are given in the next section.  The intent is that these
+rules are largely consistent with the rules of C++ for code expressible
+in both languages.
+
+Formal rules for non-trivial types in C
+~~~
+
+The following are base rules which can be added to C to support
+implementation-defined non-trivial types.
+
+A type in C is said to be *non-trivial to copy*, *non-trivial to destroy*,
+or *non-trivial to 

[PATCH] D75569: [clang-tidy] New check for methods marked __attribute__((unavailable)) that do not override a method from a superclass.

[Michael Wyman via Phabricator via cfe-commits]

mwyman updated this revision to Diff 248654.
mwyman marked 4 inline comments as done.
mwyman added a comment.

Updated per review feedback.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75569

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp
  clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.h
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst
  
clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m

Index: clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s objc-method-unavailable-not-override %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: objc-method-unavailable-not-override.FixMacroNames, \
+// RUN:  value: "UNAVAILABLE_FIXIT"} \
+// RUN: ]}' --
+
+__attribute__((objc_root_class))
+@interface Object
+- (instancetype)init;
+@end
+
+@interface MyObject : Object
+- (instancetype)init __attribute__((unavailable));
+
+// A new method that is not overriding, and is available, should not trigger.
+- (void)notOverridingMethod;
+
+- (void)methodA __attribute__((unavailable)); // methodA
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: method 'methodA' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override]
+// CHECK-FIXES: {{^\s*}}// methodA{{$}}
+
+// Verify check when unavailable attribute has a message.
+- (void)methodB __attribute__((unavailable("use methodE"))); // methodB
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: method 'methodB' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override]
+// CHECK-FIXES: {{^\s*}}// methodB{{$}}
+
+#define UNAVAILABLE_ATTRIBUTE __attribute__((unavailable))
+#define UNAVAILABLE_FIXIT UNAVAILABLE_ATTRIBUTE
+#define UNAVAILABLE_NO_FIXIT UNAVAILABLE_ATTRIBUTE
+
+// Verify check with fix-it when using an configured macro that expands to the unavailable attribute.
+- (void)methodC UNAVAILABLE_FIXIT; // methodC
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: method 'methodC' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override]
+// CHECK-FIXES: {{^\s*}}// methodC{{$}}
+
+// Verify check without fix-it when using a non-configured macro that expands to the unavailable attribute.
+- (void)methodD UNAVAILABLE_NO_FIXIT; // methodD
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: method 'methodD' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override]
+// CHECK-FIXES-NOT: {{^\s*}}// methodD{{$}}
+@end
+
+
+@implementation MyObject
+
+- (void)notOverridingMethod {}
+
+// Should not flag implementations for methods declared unavailable; sometimes
+// implemementations are provided that assert, to catch such codepaths that
+// lead to those calls during development.
+- (void)methodA {}
+
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst
@@ -0,0 +1,20 @@
+.. title:: clang-tidy - objc-method-unavailable-not-override
+
+objc-method-unavailable-not-override
+
+
+Checks that a method marked with ``__attribute__((unavailable))`` is overriding
+a method declaration from a superclass. That declaration can usually be
+deleted entirely.
+
+.. code-block:: objc
+
+   @interface ClassA : NSObject
+   // Neither ClassA nor any superclasses define method -foo.
+   @end
+
+   @interface ClassB : ClassA
+   - (void)foo __attribute__((unavailable));
+   @end
+
+Suggests a fix to remove the method declaration entirely.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -236,6 +236,7 @@
`objc-avoid-nserror-init `_,
`objc-dealloc-in-category `_,
`objc-forbidden-subclassing `_,
+   `objc-method-unavailable-not-override `_, "Yes"
`objc-missing-hash `_,
`objc-property-declaration `_, "Yes"
`objc-super-self `_, "Yes"
@@ -283,7 +284,7 @@
`readability-redundant-member-init `_, "Yes"
`readability-redundant-preprocessor `_,
`readability-redundant-smartptr-get `_, "Yes"
-   `readability-redundant-string-cstr 

[PATCH] D75569: [clang-tidy] New check for methods marked __attribute__((unavailable)) that do not override a method from a superclass.

[Michael Wyman via Phabricator via cfe-commits]

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



Comment at: 
clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp:80
+void MethodUnavailableNotOverrideCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().ObjC)
+return;

njames93 wrote:
> This check is surplus to requirements now that there is the 
> `isLanguageVersionSupported` check
Oops, forgot to delete it here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75569



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

[PATCH] D75716: [clangd] Have visibleNamespaces() and getEligiblePoints() take a LangOptions rather than a FormatStyle

[Kadir Cetinkaya via Phabricator via cfe-commits]

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

LGTM, thanks for doing this!




Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1378
 Scopes.AccessibleScopes =
-visibleNamespaces(Content.take_front(Offset), Style);
+visibleNamespaces(Content.take_front(Offset), LangOpts);
 for (std::string  : Scopes.AccessibleScopes)

nit: please inline



Comment at: clang-tools-extra/clangd/CodeComplete.cpp:1776
   semaCodeComplete(
-  std::make_unique(Options, Index, Result),
-  Options,
+  std::make_unique(Options, Index, Result), 
Options,
   {FileName, Command, Preamble, Contents, *Offset, std::move(VFS)});

nit: formatting change could you please revert ?



Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:219
+EXPECT_THAT_EXPECTED(
+positionToOffset(File, position(L.Number, L.Length + 1)),
+llvm::HasValue(L.Offset + L.Length));

nit: formatting change could you please revert ?



Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:455
 
-TEST(SourceCodeTests, IsInsideMainFile){
+TEST(SourceCodeTests, IsInsideMainFile) {
   TestTU TU;

nit: formatting change could you please revert ?



Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:474
   auto AST = TU.build();
-  const auto& SM = AST.getSourceManager();
+  const auto  = AST.getSourceManager();
   auto DeclLoc = [](llvm::StringRef Name) {

nit: formatting change could you please revert ?



Comment at: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp:574
   auto AST = TU.build();
-  const auto& SM = AST.getSourceManager();
+  const auto  = AST.getSourceManager();
 

nit: formatting change could you please revert ?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75716



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

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

[Craig Topper via Phabricator via cfe-commits]

craig.topper added inline comments.



Comment at: clang/include/clang/Basic/BuiltinsX86.def:1904
+// BITSCAN
+TARGET_BUILTIN(_BitScanForward, "UcUNi*UNi", "n", "")
+TARGET_BUILTIN(_BitScanReverse, "UcUNi*UNi", "n", "")

The N specifier here is sort of MSVC mode specific. I need to think about this.

This also makes this available without including a header file which isn't good 
if it doesn't start with __builtin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75723



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

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

[Kan Shengchen via Phabricator via cfe-commits]

skan updated this revision to Diff 248652.
skan added a comment.

Format the patch due to the warning given by pre-merge check


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75723

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/test/CodeGen/bitscan-builtins.c


Index: clang/test/CodeGen/bitscan-builtins.c
===
--- clang/test/CodeGen/bitscan-builtins.c
+++ clang/test/CodeGen/bitscan-builtins.c
@@ -3,6 +3,46 @@
 // PR33722
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown 
-fms-extensions -fms-compatibility-version=19.00 -emit-llvm -o - %s | FileCheck 
%s
 
+// Using _BitScan* intrinsics should not need any header file, so the tests for
+// them are placed before any include directive intentionally.
+unsigned char test_BitScanForward(unsigned *index, unsigned mask) {
+  return _BitScanForward(index, mask);
+  // CHECK: @test_BitScanForward
+  // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+  // CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+  // CHECK: ret i8 %[[result]]
+  // CHECK: %{{.*}} = call i32 @llvm.cttz.i32(i32 %{{.*}}, i1 true)
+}
+
+unsigned char test_BitScanForward64(unsigned *index, unsigned long long mask) {
+  return _BitScanForward64(index, mask);
+  // CHECK: @test_BitScanForward64
+  // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+  // CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+  // CHECK: ret i8 %[[result]]
+  // CHECK: %{{.*}} = call i64 @llvm.cttz.i64(i64 %{{.*}}, i1 true)
+}
+
+unsigned char test_BitScanReverse(unsigned *index, unsigned mask) {
+  return _BitScanReverse(index, mask);
+  // CHECK: @test_BitScanReverse
+  // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+  // CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+  // CHECK: ret i8 %[[result]]
+  // CHECK:  %[[call:.*]] = call i32 @llvm.ctlz.i32(i32 %{{.*}}, i1 true)
+  // CHECK:  %[[sub:.*]] = sub nsw i32 31, %[[call]]
+}
+
+unsigned char test_BitScanReverse64(unsigned *index, unsigned long long mask) {
+  return _BitScanReverse64(index, mask);
+  // CHECK: @test_BitScanReverse64
+  // CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+  // CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+  // CHECK: ret i8 %[[result]]
+  // CHECK:  %{{.*}} = call i64 @llvm.ctlz.i64(i64 %{{.*}}, i1 true)
+  // CHECK:  %{{.*}} = sub nsw i32 63, %{{.*}}
+}
+
 #include 
 
 int test_bit_scan_forward(int a) {
Index: clang/include/clang/Basic/BuiltinsX86_64.def
===
--- clang/include/clang/Basic/BuiltinsX86_64.def
+++ clang/include/clang/Basic/BuiltinsX86_64.def
@@ -21,9 +21,6 @@
 #  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) 
BUILTIN(ID, TYPE, ATTRS)
 #endif
 
-TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
-
 TARGET_HEADER_BUILTIN(__mulh,  "LLiLLiLLi","nch", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(__umulh, "ULLiULLiULLi", "nch", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_mul128, "LLiLLiLLiLLi*",  "nch",   "intrin.h", 
ALL_MS_LANGUAGES, "")
@@ -43,6 +40,10 @@
 TARGET_HEADER_BUILTIN(_InterlockedXor64, "LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedCompareExchange128, "UcLLiD*LLiLLiLLi*", 
"nh", "intrin.h", ALL_MS_LANGUAGES, "cx16")
 
+// BITSCAN
+TARGET_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "n", "")
+TARGET_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "n", "")
+
 TARGET_BUILTIN(__builtin_ia32_readeflags_u64, "UOi", "n", "")
 TARGET_BUILTIN(__builtin_ia32_writeeflags_u64, "vUOi", "n", "")
 TARGET_BUILTIN(__builtin_ia32_cvtss2si64, "OiV4f", "ncV:128:", "sse")
Index: clang/include/clang/Basic/BuiltinsX86.def
===
--- clang/include/clang/Basic/BuiltinsX86.def
+++ clang/include/clang/Basic/BuiltinsX86.def
@@ -1900,10 +1900,11 @@
 TARGET_BUILTIN(__builtin_ia32_enqcmd, "Ucv*vC*", "n", "enqcmd")
 TARGET_BUILTIN(__builtin_ia32_enqcmds, "Ucv*vC*", "n", "enqcmd")
 
-// MSVC
-TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+// BITSCAN
+TARGET_BUILTIN(_BitScanForward, "UcUNi*UNi", "n", "")
+TARGET_BUILTIN(_BitScanReverse, "UcUNi*UNi", "n", "")
 
+// MSVC
 TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_ReadBarrier,  "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_WriteBarrier, "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")


Index: 

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

[Kan Shengchen via Phabricator via cfe-commits]

skan updated this revision to Diff 248650.
skan added a comment.

Fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75723

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/test/CodeGen/bitscan-builtins.c


Index: clang/test/CodeGen/bitscan-builtins.c
===
--- clang/test/CodeGen/bitscan-builtins.c
+++ clang/test/CodeGen/bitscan-builtins.c
@@ -3,6 +3,45 @@
 // PR33722
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown 
-fms-extensions -fms-compatibility-version=19.00 -emit-llvm -o - %s | FileCheck 
%s
 
+// Using _BitScan* intrinsics should not need any header file, so the tests for
+// them are placed before any include directive intentionally.
+unsigned char test_BitScanForward(unsigned *index, unsigned mask) {
+  return _BitScanForward(index, mask);
+// CHECK: @test_BitScanForward
+// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+// CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+// CHECK: ret i8 %[[result]]
+// CHECK: %{{.*}} = call i32 @llvm.cttz.i32(i32 %{{.*}}, i1 true)
+}
+
+unsigned char test_BitScanForward64(unsigned *index, unsigned long long mask) {
+  return _BitScanForward64(index, mask);
+// CHECK: @test_BitScanForward64
+// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+// CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+// CHECK: ret i8 %[[result]]
+// CHECK: %{{.*}} = call i64 @llvm.cttz.i64(i64 %{{.*}}, i1 true)
+}
+
+unsigned char test_BitScanReverse(unsigned *index, unsigned mask) {
+  return _BitScanReverse(index, mask);
+// CHECK: @test_BitScanReverse
+// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+// CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+// CHECK: ret i8 %[[result]]
+// CHECK:  %[[call:.*]] = call i32 @llvm.ctlz.i32(i32 %{{.*}}, i1 true)
+// CHECK:  %[[sub:.*]] = sub nsw i32 31, %[[call]]
+}
+
+unsigned char test_BitScanReverse64(unsigned *index, unsigned long long mask) {
+  return _BitScanReverse64(index, mask);
+// CHECK: @test_BitScanReverse64
+// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+// CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+// CHECK: ret i8 %[[result]]
+// CHECK:  %{{.*}} = call i64 @llvm.ctlz.i64(i64 %{{.*}}, i1 true)
+// CHECK:  %{{.*}} = sub nsw i32 63, %{{.*}}
+}
 #include 
 
 int test_bit_scan_forward(int a) {
Index: clang/include/clang/Basic/BuiltinsX86_64.def
===
--- clang/include/clang/Basic/BuiltinsX86_64.def
+++ clang/include/clang/Basic/BuiltinsX86_64.def
@@ -21,9 +21,6 @@
 #  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) 
BUILTIN(ID, TYPE, ATTRS)
 #endif
 
-TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
-
 TARGET_HEADER_BUILTIN(__mulh,  "LLiLLiLLi","nch", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(__umulh, "ULLiULLiULLi", "nch", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_mul128, "LLiLLiLLiLLi*",  "nch",   "intrin.h", 
ALL_MS_LANGUAGES, "")
@@ -43,6 +40,10 @@
 TARGET_HEADER_BUILTIN(_InterlockedXor64, "LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedCompareExchange128, "UcLLiD*LLiLLiLLi*", 
"nh", "intrin.h", ALL_MS_LANGUAGES, "cx16")
 
+// BITSCAN
+TARGET_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "n", "")
+TARGET_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "n", "")
+
 TARGET_BUILTIN(__builtin_ia32_readeflags_u64, "UOi", "n", "")
 TARGET_BUILTIN(__builtin_ia32_writeeflags_u64, "vUOi", "n", "")
 TARGET_BUILTIN(__builtin_ia32_cvtss2si64, "OiV4f", "ncV:128:", "sse")
Index: clang/include/clang/Basic/BuiltinsX86.def
===
--- clang/include/clang/Basic/BuiltinsX86.def
+++ clang/include/clang/Basic/BuiltinsX86.def
@@ -1900,10 +1900,11 @@
 TARGET_BUILTIN(__builtin_ia32_enqcmd, "Ucv*vC*", "n", "enqcmd")
 TARGET_BUILTIN(__builtin_ia32_enqcmds, "Ucv*vC*", "n", "enqcmd")
 
-// MSVC
-TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+// BITSCAN
+TARGET_BUILTIN(_BitScanForward, "UcUNi*UNi", "n", "")
+TARGET_BUILTIN(_BitScanReverse, "UcUNi*UNi", "n", "")
 
+// MSVC
 TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_ReadBarrier,  "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_WriteBarrier, "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")


Index: clang/test/CodeGen/bitscan-builtins.c
===
--- 

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

FWIW i'm personally still unconvinced this is fixing a real problem




Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp:43-45
+llvm::errs()
+<< "Using ',' as a file extension delimiter is deprecated. Please "
+   "switch your configuration to use ';'.\n";

Can this spam be avoided?
It's going to be really intrusive, and it is not always possible to
just perform the migration (think: using more than one clang-tidy version.)
The deprecation message should be in the docs/releasenotes.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75621



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

[PATCH] D72860: [modules] Do not cache invalid state for modules that we attempted to load.

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

rsmith added inline comments.



Comment at: clang/lib/Serialization/ModuleManager.cpp:183
   // Get a buffer of the file and close the file descriptor when done.
-  Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
+  Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/true);
 }

vsapsai wrote:
> dblaikie wrote:
> > vsapsai wrote:
> > > dexonsmith wrote:
> > > > rsmith wrote:
> > > > > dexonsmith wrote:
> > > > > > vsapsai wrote:
> > > > > > > Made this change because if we don't have a valid module but 
> > > > > > > opened a corresponding .pcm file earlier, there is a high chance 
> > > > > > > that .pcm file was rebuilt.
> > > > > > Please add a comment in the code explaining that.
> > > > > This change is proving really bad for us. This prevents using `mmap` 
> > > > > for loading module files, and instead forces the entire file to be 
> > > > > loaded every time. Please revert.
> > > > Can we limit the revert to explicit vs. implicit module builds?  The 
> > > > scenario Volodymyr was defending against is implicit-only.
> > > Richard, can you please tell what is your modules configuration and how 
> > > you are invoking clang? For implicit builds loading a module instead of 
> > > `mmap` is still better than building a module. But for explicit modules I 
> > > see there is no need to use `isVolatile` as modules aren't changing all 
> > > the time.
> > Richard/Google use explicit modules, if that's the particular parameter 
> > you're asking about.
> > 
> > So, yes, for Google's needs a solution that allows mmap to continue to be 
> > used for explicit modules would suffice, I believe.
> > 
> > (not to in any way derail that from being addressed - I thought implicit 
> > modules weren't race-y - they're hashed, etc, so they shouldn't collide/be 
> > overwritten? Seems like a loss in performance to have to copy the whole 
> > thing into memory rather than using mmap, if that could be avoided by more 
> > precise hashing/collision avoidance?)
> I need a specific command-line invocation to test with. If tests in 
> `clang/test/Modules/explicit*` are relevant to Google's setup, I can use 
> those as an example. It's just they were touched last time in 2016.
> 
> To discuss racy-ness, that is kinda inherent problem for incremental builds. 
> If a module becomes invalid, all of its users will try to get an up-to-date 
> version of it. And they can try to build that module themselves or check and 
> load already rebuilt module. Unfortunately, I don't see how different hashing 
> can help with it, after all multiple compiler invocations do want the same 
> .pcm file.
Preventing the use of `mmap` seems unacceptable in general -- for either 
implicit or explicit modules. Anything that forces compile time to be linear in 
the total size of transitive modules used is not reasonable. To really see the 
problems here you'll need a real world testcase with thousands to tens of 
thousands of modules, which I can't reasonably give you.

There should be ways we can avoid problems with racyness. Perhaps we could 
switch to a content-addressed module file store?
In any case, this patch has introduced a regression, so we should revert this 
change while we work out what to do.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72860



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

[PATCH] D75723: [X86] Make intrinsics _BitScan* not limited to Windows

[Kan Shengchen via Phabricator via cfe-commits]

skan created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Currently if we want to use _BitScan*, we have to include intrin.h, which can
only be included on Windows. Intrinsics _BitScan* are implemented by LLVM
IR, so the restriction doesn't make sense.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75723

Files:
  clang/include/clang/Basic/BuiltinsX86.def
  clang/include/clang/Basic/BuiltinsX86_64.def
  clang/test/CodeGen/bitscan-builtins.c


Index: clang/test/CodeGen/bitscan-builtins.c
===
--- clang/test/CodeGen/bitscan-builtins.c
+++ clang/test/CodeGen/bitscan-builtins.c
@@ -3,6 +3,45 @@
 // PR33722
 // RUN: %clang_cc1 -ffreestanding -triple x86_64-unknown-unknown 
-fms-extensions -fms-compatibility-version=19.00 -emit-llvm -o - %s | FileCheck 
%s
 
+// Using _BitScan* intrinsics should not need any header file, so the tests for
+// them are put placed before any include directive intentionally.
+unsigned char test_BitScanForward(unsigned *index, unsigned mask) {
+  return _BitScanForward(index, mask);
+// CHECK: @test_BitScanForward
+// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+// CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+// CHECK: ret i8 %[[result]]
+// CHECK: %{{.*}} = call i32 @llvm.cttz.i32(i32 %{{.*}}, i1 true)
+}
+
+unsigned char test_BitScanForward64(unsigned *index, unsigned long long mask) {
+  return _BitScanForward64(index, mask);
+// CHECK: @test_BitScanForward64
+// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+// CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+// CHECK: ret i8 %[[result]]
+// CHECK: %{{.*}} = call i64 @llvm.cttz.i64(i64 %{{.*}}, i1 true)
+}
+
+unsigned char test_BitScanReverse(unsigned *index, unsigned mask) {
+  return _BitScanReverse(index, mask);
+// CHECK: @test_BitScanReverse
+// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+// CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+// CHECK: ret i8 %[[result]]
+// CHECK:  %[[call:.*]] = call i32 @llvm.ctlz.i32(i32 %{{.*}}, i1 true)
+// CHECK:  %[[sub:.*]] = sub nsw i32 31, %[[call]]
+}
+
+unsigned char test_BitScanReverse64(unsigned *index, unsigned long long mask) {
+  return _BitScanReverse64(index, mask);
+// CHECK: @test_BitScanReverse64
+// CHECK: br i1 %{{.*}}, label %{{.*}}, label %{{.*}}
+// CHECK: %[[result:.*]] = phi i8 [ 0, %{{.*}} ], [ 1, %{{.*}} ]
+// CHECK: ret i8 %[[result]]
+// CHECK:  %{{.*}} = call i64 @llvm.ctlz.i64(i64 %{{.*}}, i1 true)
+// CHECK:  %{{.*}} = sub nsw i32 63, %{{.*}}
+}
 #include 
 
 int test_bit_scan_forward(int a) {
Index: clang/include/clang/Basic/BuiltinsX86_64.def
===
--- clang/include/clang/Basic/BuiltinsX86_64.def
+++ clang/include/clang/Basic/BuiltinsX86_64.def
@@ -21,9 +21,6 @@
 #  define TARGET_HEADER_BUILTIN(ID, TYPE, ATTRS, HEADER, LANG, FEATURE) 
BUILTIN(ID, TYPE, ATTRS)
 #endif
 
-TARGET_HEADER_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
-
 TARGET_HEADER_BUILTIN(__mulh,  "LLiLLiLLi","nch", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(__umulh, "ULLiULLiULLi", "nch", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_mul128, "LLiLLiLLiLLi*",  "nch",   "intrin.h", 
ALL_MS_LANGUAGES, "")
@@ -43,6 +40,10 @@
 TARGET_HEADER_BUILTIN(_InterlockedXor64, "LLiLLiD*LLi", "nh", 
"intrin.h", ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_InterlockedCompareExchange128, "UcLLiD*LLiLLiLLi*", 
"nh", "intrin.h", ALL_MS_LANGUAGES, "cx16")
 
+// BITSCAN
+TARGET_BUILTIN(_BitScanForward64, "UcUNi*ULLi", "n", "")
+TARGET_BUILTIN(_BitScanReverse64, "UcUNi*ULLi", "n", "")
+
 TARGET_BUILTIN(__builtin_ia32_readeflags_u64, "UOi", "n", "")
 TARGET_BUILTIN(__builtin_ia32_writeeflags_u64, "vUOi", "n", "")
 TARGET_BUILTIN(__builtin_ia32_cvtss2si64, "OiV4f", "ncV:128:", "sse")
Index: clang/include/clang/Basic/BuiltinsX86.def
===
--- clang/include/clang/Basic/BuiltinsX86.def
+++ clang/include/clang/Basic/BuiltinsX86.def
@@ -1900,10 +1900,11 @@
 TARGET_BUILTIN(__builtin_ia32_enqcmd, "Ucv*vC*", "n", "enqcmd")
 TARGET_BUILTIN(__builtin_ia32_enqcmds, "Ucv*vC*", "n", "enqcmd")
 
-// MSVC
-TARGET_HEADER_BUILTIN(_BitScanForward, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
-TARGET_HEADER_BUILTIN(_BitScanReverse, "UcUNi*UNi", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
+// BITSCAN
+TARGET_BUILTIN(_BitScanForward, "UcUNi*UNi", "n", "")
+TARGET_BUILTIN(_BitScanReverse, "UcUNi*UNi", "n", "")
 
+// MSVC
 TARGET_HEADER_BUILTIN(_ReadWriteBarrier, "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_ReadBarrier,  "v", "nh", "intrin.h", 
ALL_MS_LANGUAGES, "")
 TARGET_HEADER_BUILTIN(_WriteBarrier, "v", "nh", 

[PATCH] D72875: [clang][cmake] Include generated rst files in html built by docs-clang-html target

[Tom Stellard via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG87d8ae700b80: [clang][cmake] Include generated rst files in 
html built by docs-clang-html… (authored by tstellar).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72875

Files:
  clang/docs/AttributeReference.rst
  clang/docs/CMakeLists.txt
  llvm/cmake/modules/AddSphinxTarget.cmake


Index: llvm/cmake/modules/AddSphinxTarget.cmake
===
--- llvm/cmake/modules/AddSphinxTarget.cmake
+++ llvm/cmake/modules/AddSphinxTarget.cmake
@@ -18,6 +18,7 @@
 #
 # ``project`` should be the project name
 function (add_sphinx_target builder project)
+  cmake_parse_arguments(ARG "" "SOURCE_DIR" "" ${ARGN})
   set(SPHINX_BUILD_DIR "${CMAKE_CURRENT_BINARY_DIR}/${builder}")
   set(SPHINX_DOC_TREE_DIR 
"${CMAKE_CURRENT_BINARY_DIR}/_doctrees-${project}-${builder}")
   set(SPHINX_TARGET_NAME docs-${project}-${builder})
@@ -28,13 +29,17 @@
 set(SPHINX_WARNINGS_AS_ERRORS_FLAG "")
   endif()
 
+  if (NOT ARG_SOURCE_DIR)
+set(ARG_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}")
+  endif()
+
   add_custom_target(${SPHINX_TARGET_NAME}
 COMMAND ${SPHINX_EXECUTABLE}
 -b ${builder}
 -d "${SPHINX_DOC_TREE_DIR}"
 -q # Quiet: no output other than errors and 
warnings.
 ${SPHINX_WARNINGS_AS_ERRORS_FLAG} # Treat warnings 
as errors if requested
-"${CMAKE_CURRENT_SOURCE_DIR}" # Source
+"${ARG_SOURCE_DIR}" # Source
 "${SPHINX_BUILD_DIR}" # Output
 COMMENT
 "Generating ${builder} Sphinx documentation for ${project} 
into \"${SPHINX_BUILD_DIR}\"")
Index: clang/docs/CMakeLists.txt
===
--- clang/docs/CMakeLists.txt
+++ clang/docs/CMakeLists.txt
@@ -90,15 +90,43 @@
 endif()
 endif()
 
+function (gen_rst_file_from_td output_file td_option source docs_target)
+  if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
+message(FATAL_ERROR "Cannot find source file: ${source} in 
${CMAKE_CURRENT_SOURCE_DIR}")
+  endif()
+  get_filename_component(TABLEGEN_INCLUDE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
+  list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")
+  clang_tablegen(${output_file} ${td_option} SOURCE ${source} TARGET 
"gen-${output_file}")
+  add_dependencies(${docs_target} "gen-${output_file}")
+endfunction()
+
 if (LLVM_ENABLE_SPHINX)
   include(AddSphinxTarget)
   if (SPHINX_FOUND)
 if (${SPHINX_OUTPUT_HTML})
-  add_sphinx_target(html clang)
+  add_sphinx_target(html clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+
+  # Copy rst files to build directory before generating the html
+  # documentation.  Some of the rst files are generated, so they
+  # only exist in the build directory.  Sphinx needs all files in
+  # the same directory in order to genrate the html, so we need to
+  # copy all the non-gnerated rst files from the source to the build
+  # directory before we run sphinx.
+  add_custom_target(copy-clang-rst-docs
+COMMAND "${CMAKE_COMMAND}" -E copy_directory
+"${CMAKE_CURRENT_SOURCE_DIR}"
+"${CMAKE_CURRENT_BINARY_DIR}")
+  add_dependencies(docs-clang-html copy-clang-rst-docs)
+
   add_custom_command(TARGET docs-clang-html POST_BUILD
-COMMAND ${CMAKE_COMMAND} -E copy
+COMMAND "${CMAKE_COMMAND}" -E copy
 "${CMAKE_CURRENT_SOURCE_DIR}/LibASTMatchersReference.html"
 "${CMAKE_CURRENT_BINARY_DIR}/html/LibASTMatchersReference.html")
+
+  # Generated files
+  gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td docs-clang-html)
+  gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td docs-clang-html)
+  gen_rst_file_from_td(ClangCommandLineReference.rst -gen-opt-docs 
../include/clang/Driver/ClangOptionDocs.td docs-clang-html)
 endif()
 if (${SPHINX_OUTPUT_MAN})
   add_sphinx_target(man clang)
Index: clang/docs/AttributeReference.rst
===
--- clang/docs/AttributeReference.rst
+++ /dev/null
@@ -1,13 +0,0 @@
-..
-  ---
-  NOTE: This file is automatically generated by running clang-tblgen
-  -gen-attr-docs. Do not edit this file by hand!! The contents for
-  this file are automatically generated by a server-side process.
-  
-  Please do not commit this file. The file exists for local testing
-  purposes only.
-  ---
-
-===
-Attributes in Clang

[clang-tools-extra] 45e2c6d - [clang-tools-extra/clang-tidy] Mark modernize-make-shared as offering fixes

[Jonas Devlieghere via cfe-commits]

Author: Jonas Devlieghere
Date: 2020-03-05T21:45:20-08:00
New Revision: 45e2c6d956141618683d31a683d762aaf0e7168d

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

LOG: [clang-tools-extra/clang-tidy] Mark modernize-make-shared as offering fixes

The list incorrectly states that modernize-make-shared does not offer
fixes, which is incorrect. Just like modernize-make-unique it does.

Added: 


Modified: 
clang-tools-extra/docs/clang-tidy/checks/list.rst

Removed: 




diff  --git a/clang-tools-extra/docs/clang-tidy/checks/list.rst 
b/clang-tools-extra/docs/clang-tidy/checks/list.rst
index 3b9d12ec56ad..05741385615a 100644
--- a/clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ b/clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -207,7 +207,7 @@ Clang-Tidy Checks
`modernize-deprecated-headers `_, "Yes"
`modernize-deprecated-ios-base-aliases 
`_, "Yes"
`modernize-loop-convert `_, "Yes"
-   `modernize-make-shared `_,
+   `modernize-make-shared `_, "Yes"
`modernize-make-unique `_, "Yes"
`modernize-pass-by-value `_, "Yes"
`modernize-raw-string-literal `_, "Yes"



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

[clang] 87d8ae7 - [clang][cmake] Include generated rst files in html built by docs-clang-html target

[Tom Stellard via cfe-commits]

Author: Tom Stellard
Date: 2020-03-05T21:30:37-08:00
New Revision: 87d8ae700b80d58d69bb92b601f946f02f089398

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

LOG: [clang][cmake] Include generated rst files in html built by 
docs-clang-html target

Summary:
This is an attempt to simply the process of building the clang
documentation, which should help avoid some of the recent issues we've
had generating the documentation for the website.

The html documentation for clang is generated by sphinx from the
reStructuredText (rst) files we have in the clang/docs directory.
There are also some rst files that need to be generated by TableGen,
before they can be passed to sphinx.  Prior to this patch we were not
generating those rst files as part with the build system and they had to be
generated manually.

This patch enables the automatic generation of these rst files, but
since they are generated at build time the cannot be placed in the
clang/docs directory and must go into the cmake build directory.

Unfortunately sphinx does not currently support multiple source
directories[1], so in order to be able to generate the full
documentation, we need to work around this by copying the
rst files from the clang/docs into the  build directory before
generating the html documentation.

[1] https://github.com/sphinx-doc/sphinx/issues/3132

Reviewers: rsmith, aaron.ballman, beanz, smeenai, phosek, compnerd, mgorny, 
delcypher

Reviewed By: mgorny, delcypher

Subscribers: delcypher, merge_guards_bot, mgorny, llvm-commits, cfe-commits

Tags: #clang, #llvm

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

Added: 


Modified: 
clang/docs/CMakeLists.txt
llvm/cmake/modules/AddSphinxTarget.cmake

Removed: 
clang/docs/AttributeReference.rst



diff  --git a/clang/docs/AttributeReference.rst 
b/clang/docs/AttributeReference.rst
deleted file mode 100644
index a763ddeaeb10..
--- a/clang/docs/AttributeReference.rst
+++ /dev/null
@@ -1,13 +0,0 @@
-..
-  ---
-  NOTE: This file is automatically generated by running clang-tblgen
-  -gen-attr-docs. Do not edit this file by hand!! The contents for
-  this file are automatically generated by a server-side process.
-  
-  Please do not commit this file. The file exists for local testing
-  purposes only.
-  ---
-
-===
-Attributes in Clang
-===
\ No newline at end of file

diff  --git a/clang/docs/CMakeLists.txt b/clang/docs/CMakeLists.txt
index d2956c18f80c..221ac302579b 100644
--- a/clang/docs/CMakeLists.txt
+++ b/clang/docs/CMakeLists.txt
@@ -90,15 +90,43 @@ if (LLVM_ENABLE_DOXYGEN)
 endif()
 endif()
 
+function (gen_rst_file_from_td output_file td_option source docs_target)
+  if (NOT EXISTS "${CMAKE_CURRENT_SOURCE_DIR}/${source}")
+message(FATAL_ERROR "Cannot find source file: ${source} in 
${CMAKE_CURRENT_SOURCE_DIR}")
+  endif()
+  get_filename_component(TABLEGEN_INCLUDE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/${source}" DIRECTORY)
+  list(APPEND LLVM_TABLEGEN_FLAGS "-I${TABLEGEN_INCLUDE_DIR}")
+  clang_tablegen(${output_file} ${td_option} SOURCE ${source} TARGET 
"gen-${output_file}")
+  add_dependencies(${docs_target} "gen-${output_file}")
+endfunction()
+
 if (LLVM_ENABLE_SPHINX)
   include(AddSphinxTarget)
   if (SPHINX_FOUND)
 if (${SPHINX_OUTPUT_HTML})
-  add_sphinx_target(html clang)
+  add_sphinx_target(html clang SOURCE_DIR "${CMAKE_CURRENT_BINARY_DIR}")
+
+  # Copy rst files to build directory before generating the html
+  # documentation.  Some of the rst files are generated, so they
+  # only exist in the build directory.  Sphinx needs all files in
+  # the same directory in order to genrate the html, so we need to
+  # copy all the non-gnerated rst files from the source to the build
+  # directory before we run sphinx.
+  add_custom_target(copy-clang-rst-docs
+COMMAND "${CMAKE_COMMAND}" -E copy_directory
+"${CMAKE_CURRENT_SOURCE_DIR}"
+"${CMAKE_CURRENT_BINARY_DIR}")
+  add_dependencies(docs-clang-html copy-clang-rst-docs)
+
   add_custom_command(TARGET docs-clang-html POST_BUILD
-COMMAND ${CMAKE_COMMAND} -E copy
+COMMAND "${CMAKE_COMMAND}" -E copy
 "${CMAKE_CURRENT_SOURCE_DIR}/LibASTMatchersReference.html"
 "${CMAKE_CURRENT_BINARY_DIR}/html/LibASTMatchersReference.html")
+
+  # Generated files
+  gen_rst_file_from_td(AttributeReference.rst -gen-attr-docs 
../include/clang/Basic/Attr.td docs-clang-html)
+  gen_rst_file_from_td(DiagnosticsReference.rst -gen-diag-docs 
../include/clang/Basic/Diagnostic.td docs-clang-html)
+  

[PATCH] D72860: [modules] Do not cache invalid state for modules that we attempted to load.

[Volodymyr Sapsai via Phabricator via cfe-commits]

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



Comment at: clang/lib/Serialization/ModuleManager.cpp:183
   // Get a buffer of the file and close the file descriptor when done.
-  Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
+  Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/true);
 }

dblaikie wrote:
> vsapsai wrote:
> > dexonsmith wrote:
> > > rsmith wrote:
> > > > dexonsmith wrote:
> > > > > vsapsai wrote:
> > > > > > Made this change because if we don't have a valid module but opened 
> > > > > > a corresponding .pcm file earlier, there is a high chance that .pcm 
> > > > > > file was rebuilt.
> > > > > Please add a comment in the code explaining that.
> > > > This change is proving really bad for us. This prevents using `mmap` 
> > > > for loading module files, and instead forces the entire file to be 
> > > > loaded every time. Please revert.
> > > Can we limit the revert to explicit vs. implicit module builds?  The 
> > > scenario Volodymyr was defending against is implicit-only.
> > Richard, can you please tell what is your modules configuration and how you 
> > are invoking clang? For implicit builds loading a module instead of `mmap` 
> > is still better than building a module. But for explicit modules I see 
> > there is no need to use `isVolatile` as modules aren't changing all the 
> > time.
> Richard/Google use explicit modules, if that's the particular parameter 
> you're asking about.
> 
> So, yes, for Google's needs a solution that allows mmap to continue to be 
> used for explicit modules would suffice, I believe.
> 
> (not to in any way derail that from being addressed - I thought implicit 
> modules weren't race-y - they're hashed, etc, so they shouldn't collide/be 
> overwritten? Seems like a loss in performance to have to copy the whole thing 
> into memory rather than using mmap, if that could be avoided by more precise 
> hashing/collision avoidance?)
I need a specific command-line invocation to test with. If tests in 
`clang/test/Modules/explicit*` are relevant to Google's setup, I can use those 
as an example. It's just they were touched last time in 2016.

To discuss racy-ness, that is kinda inherent problem for incremental builds. If 
a module becomes invalid, all of its users will try to get an up-to-date 
version of it. And they can try to build that module themselves or check and 
load already rebuilt module. Unfortunately, I don't see how different hashing 
can help with it, after all multiple compiler invocations do want the same .pcm 
file.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72860



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

Re: [clang] bdad0a1 - PR45083: Mark statement expressions as being dependent if they appear in

[Richard Smith via cfe-commits]

I implemented a completely different fix in
a95cc77be154433c37a3110ac9af394b7447fcba. Please can you let me know if it
works out?

The old approach was to treat statement expressions as being value- and
instantiation-dependent if they appear in a dependent context (matching
GCC's apparent behavior). But it turns out that Clang doesn't really know
whether it's in a dependent context! (In particular, Sema::CurContext might
be a template when substituting into that template, might be the enclosing
context, and might be the resulting declaration if it already exists. This
has other implications -- we at least get access checks wrong in some cases
-- but fixing it would be far to invasive to do for Clang 10.)
The new approach is to walk the body of the statement expression and see if
it contains any value- or instantiation-dependent subexpressions, and treat
the statement expression as being correspondingly dependent if so.

Hans, the new fix will need some work to backport due to ec3060c (it should
be largely mechanical, but perhaps not completely obvious what the
mechanical steps are). Let me know if you'd like me to do the port.

On Thu, 5 Mar 2020 at 12:20, Richard Smith  wrote:

> On Thu, 5 Mar 2020 at 06:13, Benjamin Kramer via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
>
>> creduce produced this. It's a crash on invalid, but was created from a
>> valid input.
>>
>
> Well, "valid" is unclear when using language extensions, but OK.
>
>
>> $ cat r.ii
>> template  auto b(a) {
>>   auto c = [](auto, int) -> decltype(({})) {};
>>   return c(0, 0);
>> }
>> using d = decltype(b(0));
>> bool f = d ::e;
>>
>
> How important is this testcase? The fix you reverted was fixing a
> release-blocker; is this also a release-blocker in your view?
>
>
>> $ clang r.ii -std=c++17 -w
>> clang-11: clang/lib/AST/Decl.cpp:2343: clang::APValue
>> *clang::VarDecl::evaluateValue(SmallVectorImpl
>> &) const: Assertion `!Init->isValueDependent()' failed.
>>
>> On Thu, Mar 5, 2020 at 2:18 PM Benjamin Kramer 
>> wrote:
>> >
>> > It's still crashing clang, reverted this and
>> > f545ede91c9d9f271e7504282cab7bf509607ead in 66addf8e8036. c-reduce is
>> > still chewing on the reproducer.
>> >
>> > On Wed, Mar 4, 2020 at 10:20 PM Richard Smith via cfe-commits
>> >  wrote:
>> > >
>> > > We found a regression introduced by this patch; fixed in
>> f545ede91c9d9f271e7504282cab7bf509607ead.
>> > >
>> > > On Wed, 4 Mar 2020 at 00:30, Hans Wennborg via cfe-commits <
>> cfe-commits@lists.llvm.org> wrote:
>> > >>
>> > >> Pushed to 10.x as 3a843031a5ad83a00d2603f623881cb2b2bf719d. Please
>> let
>> > >> me know if you hear about any follow-up issues.
>> > >>
>> > >> Thanks!
>> > >>
>> > >> On Wed, Mar 4, 2020 at 12:28 AM Richard Smith via cfe-commits
>> > >>  wrote:
>> > >> >
>> > >> >
>> > >> > Author: Richard Smith
>> > >> > Date: 2020-03-03T15:20:40-08:00
>> > >> > New Revision: bdad0a1b79273733df9acc1be4e992fa5d70ec56
>> > >> >
>> > >> > URL:
>> https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56
>> > >> > DIFF:
>> https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56.diff
>> > >> >
>> > >> > LOG: PR45083: Mark statement expressions as being dependent if
>> they appear in
>> > >> > dependent contexts.
>> > >> >
>> > >> > We previously assumed they were neither value- nor
>> > >> > instantiation-dependent under any circumstances, which would lead
>> to
>> > >> > crashes and other misbehavior.
>> > >> >
>> > >> > Added:
>> > >> >
>> > >> >
>> > >> > Modified:
>> > >> > clang/include/clang/AST/Expr.h
>> > >> > clang/include/clang/Sema/Sema.h
>> > >> > clang/lib/AST/ASTImporter.cpp
>> > >> > clang/lib/Parse/ParseExpr.cpp
>> > >> > clang/lib/Sema/SemaExpr.cpp
>> > >> > clang/lib/Sema/SemaExprCXX.cpp
>> > >> > clang/lib/Sema/TreeTransform.h
>> > >> > clang/test/SemaTemplate/dependent-expr.cpp
>> > >> >
>> > >> > Removed:
>> > >> >
>> > >> >
>> > >> >
>> > >> >
>> 
>> > >> > diff  --git a/clang/include/clang/AST/Expr.h
>> b/clang/include/clang/AST/Expr.h
>> > >> > index fcdb0b992134..87f9b883486a 100644
>> > >> > --- a/clang/include/clang/AST/Expr.h
>> > >> > +++ b/clang/include/clang/AST/Expr.h
>> > >> > @@ -3960,14 +3960,14 @@ class StmtExpr : public Expr {
>> > >> >Stmt *SubStmt;
>> > >> >SourceLocation LParenLoc, RParenLoc;
>> > >> >  public:
>> > >> > -  // FIXME: Does type-dependence need to be computed
>> > >> > diff erently?
>> > >> > -  // FIXME: Do we need to compute instantiation
>> instantiation-dependence for
>> > >> > -  // statements? (ugh!)
>> > >> >StmtExpr(CompoundStmt *substmt, QualType T,
>> > >> > -   SourceLocation lp, SourceLocation rp) :
>> > >> > +   SourceLocation lp, SourceLocation rp, bool
>> InDependentContext) :
>> > >> > +// Note: we treat a statement-expression in a dependent
>> context as always
>> > >> > +

[clang] a95cc77 - PR45083: Mark statement expressions as being dependent if they contain

[Richard Smith via cfe-commits]

Author: Richard Smith
Date: 2020-03-05T19:03:05-08:00
New Revision: a95cc77be154433c37a3110ac9af394b7447fcba

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

LOG: PR45083: Mark statement expressions as being dependent if they contain
dependent constructs.

We previously assumed they were neither value- nor
instantiation-dependent under any circumstances, which would lead to
crashes and other misbehavior.

This doesn't match GCC's behavior (where statement expressions appear to
be treated as value-dependent if they appear in a dependent context),
but seems to be the best thing we can do in the short term: it turns out
to be remarkably difficult for us to correctly determine whether we are
in a dependent context (and it's not even possible in some cases, such
as in a generic lambda where we might not have seen the 'auto' yet).

Added: 


Modified: 
clang/include/clang/AST/Expr.h
clang/lib/AST/Expr.cpp
clang/test/SemaTemplate/dependent-expr.cpp

Removed: 




diff  --git a/clang/include/clang/AST/Expr.h b/clang/include/clang/AST/Expr.h
index 7271dbb830a2..75b7a5f6ecd3 100644
--- a/clang/include/clang/AST/Expr.h
+++ b/clang/include/clang/AST/Expr.h
@@ -3959,14 +3959,8 @@ class StmtExpr : public Expr {
   Stmt *SubStmt;
   SourceLocation LParenLoc, RParenLoc;
 public:
-  // FIXME: Does type-dependence need to be computed 
diff erently?
-  // FIXME: Do we need to compute instantiation instantiation-dependence for
-  // statements? (ugh!)
-  StmtExpr(CompoundStmt *substmt, QualType T,
-   SourceLocation lp, SourceLocation rp) :
-Expr(StmtExprClass, T, VK_RValue, OK_Ordinary,
- T->isDependentType(), false, false, false),
-SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { }
+  StmtExpr(CompoundStmt *SubStmt, QualType T,
+   SourceLocation LParen, SourceLocation RParen);
 
   /// Build an empty statement expression.
   explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty) { }

diff  --git a/clang/lib/AST/Expr.cpp b/clang/lib/AST/Expr.cpp
index 79f9f42224d0..78fd96fd76e6 100644
--- a/clang/lib/AST/Expr.cpp
+++ b/clang/lib/AST/Expr.cpp
@@ -4097,6 +4097,53 @@ void ExtVectorElementExpr::getEncodedElementAccess(
   }
 }
 
+StmtExpr::StmtExpr(CompoundStmt *SubStmt, QualType T, SourceLocation LParen,
+   SourceLocation RParen)
+: Expr(StmtExprClass, T, VK_RValue, OK_Ordinary, T->isDependentType(),
+   false, false, false),
+  SubStmt(SubStmt), LParenLoc(LParen), RParenLoc(RParen) {
+  llvm::SmallVector Queue(1, SubStmt);
+  while (!Queue.empty()) {
+Stmt *S = Queue.pop_back_val();
+if (!S)
+  continue;
+
+// If any subexpression is dependent, the statement expression is dependent
+// in the same way.
+if (Expr *E = dyn_cast(S)) {
+  addDependence(E->getDependence());
+  continue;
+}
+
+// FIXME: Need to properly compute whether DeclStmts contain unexpanded
+// parameter packs.
+if (DeclStmt *DS = dyn_cast(S)) {
+  for (Decl *D : DS->decls()) {
+// If any contained declaration is in a dependent context, then it
+// needs to be instantiated, so the statement expression itself is
+// instantiation-dependent.
+//
+// Note that we don't need to worry about the case where the context is
+// non-dependent but contains dependent entities here (eg, inside a
+// variable template or alias template): that can only happen at file
+// scope, where statement expressions are prohibited.
+if (D->getLexicalDeclContext()->isDependentContext())
+  addDependence(ExprDependence::Instantiation);
+
+// If any contained variable declaration has a dependent type, we can't
+// evaluate that declaration.
+if (auto *VD = dyn_cast(D))
+  if (VD->getType()->isDependentType())
+addDependence(ExprDependence::Value);
+  }
+}
+
+// Recurse to substatements.
+// FIXME: Should we skip the unchosen side of 'if constexpr' if known?
+Queue.insert(Queue.end(), S->child_begin(), S->child_end());
+  }
+}
+
 ShuffleVectorExpr::ShuffleVectorExpr(const ASTContext , ArrayRef args,
  QualType Type, SourceLocation BLoc,
  SourceLocation RP)

diff  --git a/clang/test/SemaTemplate/dependent-expr.cpp 
b/clang/test/SemaTemplate/dependent-expr.cpp
index bb1e239c3490..12a99acc21cd 100644
--- a/clang/test/SemaTemplate/dependent-expr.cpp
+++ b/clang/test/SemaTemplate/dependent-expr.cpp
@@ -1,5 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify %s
-// expected-no-diagnostics
+// RUN: %clang_cc1 -fsyntax-only -verify -std=c++2a %s
 
 // PR5908
 template 
@@ -108,3 +107,42 @@ namespace PR18152 {
   };
   

[PATCH] D75719: [clang][Headers] Use __has_builtin instead of _MSC_VER.

[Michael Spencer via Phabricator via cfe-commits]

Bigcheese created this revision.
Bigcheese added reviewers: compnerd, kongyi.
Bigcheese added a project: clang.
Herald added subscribers: danielkiss, dexonsmith, kristof.beyls.

arm_acle.h relied on `_MSC_VER` to determine if a given function was
already defined as a builtin. This was incorrect because
`-fms-extensions` enables these builtins, but is not responsible for
defining `_MSC_VER` on any target. The next closest thing is
`_MSC_EXTENSIONS`, which is only defined on Windows targets, but even
this is suboptimal. What this conditional is actually trying to
determine is if the given functions are defined as builtins, so just
check that directly.

I also attempted to do this for `__nop`, but in that case intrin.h,
which is only includable if `_MSC_VER` is defined, has its own
definition. So in that case `_MSC_VER` is correct.

rdar://60102353


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75719

Files:
  clang/lib/Headers/arm_acle.h
  clang/test/Headers/arm-acle-header.c


Index: clang/test/Headers/arm-acle-header.c
===
--- clang/test/Headers/arm-acle-header.c
+++ clang/test/Headers/arm-acle-header.c
@@ -6,6 +6,7 @@
 // RUN: %clang_cc1 -x c++ -triple thumbv7-windows -target-cpu cortex-a15 
-fsyntax-only -ffreestanding %s
 // RUN: %clang_cc1 -x c++ -triple thumbv7-windows -target-cpu cortex-a15 
-fsyntax-only -ffreestanding -fms-extensions -fms-compatibility 
-fms-compatibility-version=19.11 %s
 // RUN: %clang_cc1 -x c++ -triple aarch64-windows -target-cpu cortex-a53 
-fsyntax-only -ffreestanding -fms-extensions -fms-compatibility 
-fms-compatibility-version=19.11 %s
+// RUN: %clang_cc1 -x c++ -triple arm64-apple-ios -target-cpu apple-a7 
-fsyntax-only -ffreestanding -fms-extensions %s
 // expected-no-diagnostics
 
 #include 
Index: clang/lib/Headers/arm_acle.h
===
--- clang/lib/Headers/arm_acle.h
+++ clang/lib/Headers/arm_acle.h
@@ -22,31 +22,43 @@
 
 /* 8 SYNCHRONIZATION, BARRIER AND HINT INTRINSICS */
 /* 8.3 Memory barriers */
-#if !defined(_MSC_VER)
+#if !__has_builtin(__dmb)
 #define __dmb(i) __builtin_arm_dmb(i)
+#endif
+#if !__has_builtin(__dsb)
 #define __dsb(i) __builtin_arm_dsb(i)
+#endif
+#if !__has_builtin(__isb)
 #define __isb(i) __builtin_arm_isb(i)
 #endif
 
 /* 8.4 Hints */
 
-#if !defined(_MSC_VER)
+#if !__has_builtin(__wfi)
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) 
__wfi(void) {
   __builtin_arm_wfi();
 }
+#endif
 
+#if !__has_builtin(__wfe)
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) 
__wfe(void) {
   __builtin_arm_wfe();
 }
+#endif
 
+#if !__has_builtin(__sev)
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) 
__sev(void) {
   __builtin_arm_sev();
 }
+#endif
 
+#if !__has_builtin(__sevl)
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) 
__sevl(void) {
   __builtin_arm_sevl();
 }
+#endif
 
+#if !__has_builtin(__yield)
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) 
__yield(void) {
   __builtin_arm_yield();
 }


Index: clang/test/Headers/arm-acle-header.c
===
--- clang/test/Headers/arm-acle-header.c
+++ clang/test/Headers/arm-acle-header.c
@@ -6,6 +6,7 @@
 // RUN: %clang_cc1 -x c++ -triple thumbv7-windows -target-cpu cortex-a15 -fsyntax-only -ffreestanding %s
 // RUN: %clang_cc1 -x c++ -triple thumbv7-windows -target-cpu cortex-a15 -fsyntax-only -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=19.11 %s
 // RUN: %clang_cc1 -x c++ -triple aarch64-windows -target-cpu cortex-a53 -fsyntax-only -ffreestanding -fms-extensions -fms-compatibility -fms-compatibility-version=19.11 %s
+// RUN: %clang_cc1 -x c++ -triple arm64-apple-ios -target-cpu apple-a7 -fsyntax-only -ffreestanding -fms-extensions %s
 // expected-no-diagnostics
 
 #include 
Index: clang/lib/Headers/arm_acle.h
===
--- clang/lib/Headers/arm_acle.h
+++ clang/lib/Headers/arm_acle.h
@@ -22,31 +22,43 @@
 
 /* 8 SYNCHRONIZATION, BARRIER AND HINT INTRINSICS */
 /* 8.3 Memory barriers */
-#if !defined(_MSC_VER)
+#if !__has_builtin(__dmb)
 #define __dmb(i) __builtin_arm_dmb(i)
+#endif
+#if !__has_builtin(__dsb)
 #define __dsb(i) __builtin_arm_dsb(i)
+#endif
+#if !__has_builtin(__isb)
 #define __isb(i) __builtin_arm_isb(i)
 #endif
 
 /* 8.4 Hints */
 
-#if !defined(_MSC_VER)
+#if !__has_builtin(__wfi)
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) __wfi(void) {
   __builtin_arm_wfi();
 }
+#endif
 
+#if !__has_builtin(__wfe)
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) __wfe(void) {
   __builtin_arm_wfe();
 }
+#endif
 
+#if !__has_builtin(__sev)
 static __inline__ void __attribute__((__always_inline__, __nodebug__)) __sev(void) {
   __builtin_arm_sev();
 }
+#endif

[PATCH] D75560: Make Decl:: setOwningModuleID() public. (NFC)

[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.

I'm surprised so few setters needed to be made public for this to work. If this 
required making lots more setters public, I'd be concerned, because we 
generally don't want large chunks of the AST to be mutable after creation. But 
this seems fine.




Comment at: clang/include/clang/AST/DeclBase.h:629-630
 
+public:
+  void setFromASTFile() { FromASTFile = true; }
+

aprantl wrote:
> rsmith wrote:
> > Setting this after creating a `Decl` is not safe in general; declarations 
> > with this flag set have 8 bytes of additional storage allocated before 
> > their start.
> > 
> > An `assert(FromASTFile || hasLocalOwningModuleStorage());` would at least 
> > catch the unsafe cases. This also needs a comment saying that it's not safe 
> > to use in general, and perhaps a scarier name to discourage people from 
> > calling it. (Unless you're creating declarations with 
> > `::CreateDeserialized` -- in which case you'd need all the `Decl` 
> > subclasses to befriend the creator, making this function unnecessary -- 
> > this is currently only going to be safe if `ModulesLocalVisibility` is 
> > enabled.)
> I think the warning is unnecessary, since the storage is always allocated. 
> Please let me know if I misunderstood something.
The storage isn't necessarily allocated if the decl is created with `::Create` 
rather than `::CreateDeserialized`. Just changing the comment to say the 
declaration must have been created with `CreateDeserialized` would be fine.


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

https://reviews.llvm.org/D75560



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

[PATCH] D64464: [CodeGen] Emit destructor calls to destruct compound literals

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:4100
+  if (E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+pushDestroy(QualType::DK_nontrivial_c_struct, DeclPtr, E->getType());
+

ahatanak wrote:
> rjmccall wrote:
> > ahatanak wrote:
> > > rjmccall wrote:
> > > > rjmccall wrote:
> > > > > Unfortunately, the lifetime of compound literals in C is not this 
> > > > > simple; they're like blocks in that they're destroyed at the end of 
> > > > > the enclosing scope rather than at the end of the current statement. 
> > > > > (The cleanup here will be popped at the end of the full-expression if 
> > > > > we've entered an `ExprWithCleanups`.) And the l-value case is exactly 
> > > > > the case where this matters.
> > > > > 
> > > > > I think you need to do something like what we do with blocks, where 
> > > > > we record all the blocks in the full-expression on the 
> > > > > `ExprWithCleanups` so that we can push an inactive cleanup for them 
> > > > > and then activate it when we emit the block.
> > > > Can we make the check here something like (1) this is a block-scope 
> > > > compound literal and (2) it has a non-trivially-destructed type (of any 
> > > > kind)?  That way we're not conflating two potentially unrelated 
> > > > elements, the lifetime of the object and the kinds of types that can be 
> > > > constructed by the literal.
> > > > 
> > > > Oh, actually, there's a concrete reason to do this: C99 compound 
> > > > literals are not required to have struct type; they can have any object 
> > > > type, including arrays but also scalars.  So we could, even without 
> > > > non-trivial C structs, have a block-scope compound of type `__strong 
> > > > id[]`; I guess we've always just gotten this wrong.  Please add tests 
> > > > for this case. :)
> > > There is a check `E->isFileScope()` above this. Is that sufficient to 
> > > check for block-scoped compound literals?
> > That plus the C/C++ difference; compound literals in C++ are just 
> > temporaries.
> I haven't been able to come up with a piece of C++ code that executes 
> `EmitCompoundLiteralLValue`. The following code gets rejected because you 
> can't take the address of a temporary object in C++:
> 
> ```
> StrongSmall *p = &(StrongSmall){ 1, 0 };
> ```
> 
> If a bind a reference to it, `AggExprEmitter::VisitCompoundLiteralExpr` is 
> called.
That makes sense; they're not gl-values in C++.  It would be reasonable to 
assert that.  But the C++ point does apply elsewhere.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64464



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

[PATCH] D64464: [CodeGen] Emit destructor calls to destruct compound literals

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: lib/CodeGen/CGExpr.cpp:4100
+  if (E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+pushDestroy(QualType::DK_nontrivial_c_struct, DeclPtr, E->getType());
+

rjmccall wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > rjmccall wrote:
> > > > Unfortunately, the lifetime of compound literals in C is not this 
> > > > simple; they're like blocks in that they're destroyed at the end of the 
> > > > enclosing scope rather than at the end of the current statement. (The 
> > > > cleanup here will be popped at the end of the full-expression if we've 
> > > > entered an `ExprWithCleanups`.) And the l-value case is exactly the 
> > > > case where this matters.
> > > > 
> > > > I think you need to do something like what we do with blocks, where we 
> > > > record all the blocks in the full-expression on the `ExprWithCleanups` 
> > > > so that we can push an inactive cleanup for them and then activate it 
> > > > when we emit the block.
> > > Can we make the check here something like (1) this is a block-scope 
> > > compound literal and (2) it has a non-trivially-destructed type (of any 
> > > kind)?  That way we're not conflating two potentially unrelated elements, 
> > > the lifetime of the object and the kinds of types that can be constructed 
> > > by the literal.
> > > 
> > > Oh, actually, there's a concrete reason to do this: C99 compound literals 
> > > are not required to have struct type; they can have any object type, 
> > > including arrays but also scalars.  So we could, even without non-trivial 
> > > C structs, have a block-scope compound of type `__strong id[]`; I guess 
> > > we've always just gotten this wrong.  Please add tests for this case. :)
> > There is a check `E->isFileScope()` above this. Is that sufficient to check 
> > for block-scoped compound literals?
> That plus the C/C++ difference; compound literals in C++ are just temporaries.
I haven't been able to come up with a piece of C++ code that executes 
`EmitCompoundLiteralLValue`. The following code gets rejected because you can't 
take the address of a temporary object in C++:

```
StrongSmall *p = &(StrongSmall){ 1, 0 };
```

If a bind a reference to it, `AggExprEmitter::VisitCompoundLiteralExpr` is 
called.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64464



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

[PATCH] D72874: [clangd] Add a textual fallback for go-to-definition

[Nathan Ridge via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clangd/XRefs.cpp:489
+  Index->fuzzyFind(Req, [&](const Symbol ) {
+auto MaybeDeclLoc =
+symbolLocationToLocation(Sym.CanonicalDeclaration, MainFilePath);

Sorry this location-setting code is so messy. All my attempts to make it more 
concise have been thwarted by `llvm::Expected`'s very restrictive API.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72874



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

[PATCH] D72874: [clangd] Add a textual fallback for go-to-definition

[Nathan Ridge via Phabricator via cfe-commits]

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

In D72874#1900648 , @sammccall wrote:

> This reminds me: it's not completely obvious what set of "act on symbol under 
> the cursor" things this should (eventually) apply to.
>  I think not having e.g. find-references work makes sense - user should 
> navigate to a "real" occurrence to resolve the ambiguity, and things like 
> code actions are right out.
>  However having `textDocument/hover` work when we have high confidence in 
> results would be really cool.
>  Obviously nothing in scope for this patch, but it seems worth writing this 
> down somewhere, precisely because we shouldn't do it now.


Agreed. Filed https://github.com/clangd/clangd/issues/303 for hover.

In D72874#1901722 , @sammccall wrote:

> (Tactically I think it makes sense to add the basic fallback logic, and 
> follow up with the dependent-code entrypoints, but up to you)


Yep, will handle dependent code in a folow-up patch.




Comment at: clang-tools-extra/clangd/SourceCode.h:93
+/// the entire comment or string token.
+SourceRange getWordAtPosition(const Position , const SourceManager ,
+  const LangOptions );

sammccall wrote:
> sammccall wrote:
> > consider moving the isLikelyToBeIdentifier check inside. The current API is 
> > pretty general and it's not clear yet what (else) it's good for so it's 
> > nice to direct towards intended usage.
> > 
> > Also doing the identifier check inside this function is more convenient 
> > when it relies on markers outside the identifier range (like doxygen `\p` 
> > or backtick-quoted identifiers)
> > 
> > That said, you may still want to return the range when it's not a likely 
> > identifier, with a signature like `StringRef getWordAtPosition(bool 
> > *LikelyIdentifier = nullptr)`. I'm thinking of the future case where the 
> > caller wants to find a nearby matching token and resolve it - resolving 
> > belongs in the caller so there's not much point having this function 
> > duplicate the check.
> This doesn't use the SourceManager-structure of the file, so the natural 
> signature would be `StringRef getWordAtPosition(StringRef Code, unsigned 
> Offset)`.
> 
> (what are the practical cases where langopts is relevant?)
Now that I'm using `wordTouching()` from D75479, I think this comment no longer 
applies?



Comment at: clang-tools-extra/clangd/XRefs.cpp:226
+
+std::vector navigationFallback(ParsedAST ,
+  const SymbolIndex *Index,

nridge wrote:
> sammccall wrote:
> > this function should have a high-level comment describing the strategy and 
> > the limitations (e.g. idea of extending it to resolve nearby matching 
> > tokens).
> > 
> > A name like `locateSymbolNamedTextuallyAt` would better describe what this 
> > does, rather than what its caller does.
> > 
> > I would strongly consider exposing this function publicly for the detailed 
> > tests, and only smoke-testing it through `locateSymbolAt`. Having to break 
> > the AST in tests or otherwise rely on the "primary" logic not working is 
> > brittle and hard to verify.
> > I would strongly consider exposing this function publicly for the detailed 
> > tests, and only smoke-testing it through locateSymbolAt. Having to break 
> > the AST in tests or otherwise rely on the "primary" logic not working is 
> > brittle and hard to verify.
> 
> I was going to push back against this, but I ended up convincing myself that 
> your suggestion is better :)
> 
> For the record, the consideration that convinced me was:
> 
> Suppose in the future we add fancier AST-based logic that handles a case like 
> `T().foo()` (for example, by surveying types for which `T` is actually 
> substituted, and offering `foo()` inside those types). If all we're testing 
> is "navigation works for this case" rather than "navigation works for this 
> case //via the AST-based mechanism//", we could regress the AST logic but 
> have our test still pass because the testcase is simple enough that the 
> text-based navigation fallback (that we're adding here) works as well.
Renamed and comment added. I still need to revise the tests.



Comment at: clang-tools-extra/clangd/XRefs.cpp:240
+  Req.ProximityPaths = {MainFilePath};
+  // FIXME: Set Req.Scopes to the lexically enclosing scopes.
+  // For extra strictness, consider AnyScope=false.

sammccall wrote:
> FWIW the API for this is visibleNamespaces() from SourceCode.cpp.
> (No enclosing classes, but I suspect we can live without them once we have a 
> nearby-tokens solution too)
Thanks, that's convenient!

Out of curiosity, though: is the reason to prefer this lexer-based approach 
over hit-testing the query location against `NamespaceDecl`s in the AST, mainly 
for performance?



Comment 

[PATCH] D72874: [clangd] Add a textual fallback for go-to-definition

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 248630.
nridge marked 16 inline comments as done.
nridge added a comment.

Rebase onto D75479  and address most review 
comments

Comments remaining to be addressed:

- revising the tests to exercise locateSymbolNamedTextuallyAt() directly
- comments related to fuzzy matching (I have an outstanding question about that)

Handling of dependent code has been deferred to a follow-up change


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72874

Files:
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/FindSymbols.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  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
@@ -596,6 +596,44 @@
   }
 }
 
+TEST(LocateSymbol, Textual) {
+  const char *Tests[] = {
+  R"cpp(// Comment
+struct [[MyClass]] {};
+// Comment mentioning M^yClass
+  )cpp",
+  R"cpp(// String
+struct [[MyClass]] {};
+const char* s = "String literal mentioning M^yClass";
+  )cpp",
+  R"cpp(// Ifdef'ed out code
+struct [[MyClass]] {};
+#ifdef WALDO
+  M^yClass var;
+#endif
+  )cpp"};
+
+  for (const char *Test : Tests) {
+Annotations T(Test);
+llvm::Optional WantDecl;
+if (!T.ranges().empty())
+  WantDecl = T.range();
+
+auto TU = TestTU::withCode(T.code());
+
+auto AST = TU.build();
+auto Index = TU.index();
+auto Results = locateSymbolAt(AST, T.point(), Index.get());
+
+if (!WantDecl) {
+  EXPECT_THAT(Results, IsEmpty()) << Test;
+} else {
+  ASSERT_THAT(Results, ::testing::SizeIs(1)) << Test;
+  EXPECT_EQ(Results[0].PreferredDeclaration.range, *WantDecl) << Test;
+}
+  }
+}
+
 TEST(LocateSymbol, Ambiguous) {
   auto T = Annotations(R"cpp(
 struct Foo {
@@ -670,6 +708,26 @@
Sym("baz", T.range("StaticOverload2";
 }
 
+TEST(LocateSymbol, TextualAmbiguous) {
+  auto T = Annotations(R"cpp(
+struct Foo {
+  void $FooLoc[[uniqueMethodName]]();
+};
+struct Bar {
+  void $BarLoc[[uniqueMethodName]]();
+};
+// Will call u^niqueMethodName() on t.
+template 
+void f(T t);
+  )cpp");
+  auto TU = TestTU::withCode(T.code());
+  auto AST = TU.build();
+  auto Index = TU.index();
+  EXPECT_THAT(locateSymbolAt(AST, T.point(), Index.get()),
+  UnorderedElementsAre(Sym("uniqueMethodName", T.range("FooLoc")),
+   Sym("uniqueMethodName", T.range("BarLoc";
+}
+
 TEST(LocateSymbol, TemplateTypedefs) {
   auto T = Annotations(R"cpp(
 template  struct function {};
@@ -869,37 +927,37 @@
 
 TEST(LocateSymbol, NearbyIdentifier) {
   const char *Tests[] = {
-R"cpp(
+  R"cpp(
   // regular identifiers (won't trigger)
   int hello;
   int y = he^llo;
 )cpp",
-R"cpp(
+  R"cpp(
   // disabled preprocessor sections
   int [[hello]];
   #if 0
   int y = ^hello;
   #endif
 )cpp",
-R"cpp(
+  R"cpp(
   // comments
   // he^llo, world
   int [[hello]];
 )cpp",
-R"cpp(
+  R"cpp(
   // string literals
   int [[hello]];
   const char* greeting = "h^ello, world";
 )cpp",
 
-R"cpp(
+  R"cpp(
   // can refer to macro invocations (even if they expand to nothing)
   #define INT int
   [[INT]] x;
   // I^NT
 )cpp",
 
-R"cpp(
+  R"cpp(
   // prefer nearest occurrence
   int hello;
   int x = hello;
@@ -908,12 +966,12 @@
   int z = hello;
 )cpp",
 
-R"cpp(
+  R"cpp(
   // short identifiers find near results
   int [[hi]];
   // h^i
 )cpp",
-R"cpp(
+  R"cpp(
   // short identifiers don't find far results
   int hi;
 
@@ -922,13 +980,13 @@
   // h^i
 )cpp",
   };
-  for (const char* Test : Tests) {
+  for (const char *Test : Tests) {
 Annotations T(Test);
 auto AST = TestTU::withCode(T.code()).build();
 const auto  = AST.getSourceManager();
 llvm::Optional Nearby;
-if (const auto*Tok = findNearbyIdentifier(
-cantFail(sourceLocationInMainFile(SM, T.point())), AST.getTokens()))
+if (const auto *Tok = findNearbyIdentifier(
+cantFail(sourceLocationInMainFile(SM, T.point())), AST.getTokens()))
   Nearby = halfOpenToRange(SM, CharSourceRange::getCharRange(
Tok->location(), Tok->endLocation()));
 if (T.ranges().empty())
Index: clang-tools-extra/clangd/XRefs.h
===

[PATCH] D75479: [clangd] go-to-def on names in comments etc that are used nearby.

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

Here's a test case that gives a less desirable result with this approach than 
D72874 :

  struct Foo {
void uniqueMethodName();
  };
  struct Bar {
void uniqueMethodName();
  };
  
  // Will call u^niqueMethodName() on t.
  template 
  void f(T t);

This method only returns `Bar::uniqueMethodName()` because it's closer in terms 
of distance, whereas D72874  returns both 
methods.

Is that perhaps a reason to adjust the order in which we try the approaches 
(i.e. use this one as a fallback if index lookup fails)? Or should we try to 
allow multiple results with this approach as well?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75479



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

[PATCH] D75716: [clangd] Have visibleNamespaces() and getEligiblePoints() take a LangOptions rather than a FormatStyle

[Nathan Ridge via Phabricator via cfe-commits]

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

These functions only use the FormatStyle to obtain a LangOptions via
format::getFormattingLangOpts(), and some callers can more easily obtain
a LangOptions more directly.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75716

Files:
  clang-tools-extra/clangd/CodeComplete.cpp
  clang-tools-extra/clangd/SourceCode.cpp
  clang-tools-extra/clangd/SourceCode.h
  clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
  clang-tools-extra/clangd/unittests/SourceCodeTests.cpp

Index: clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
===
--- clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
+++ clang-tools-extra/clangd/unittests/SourceCodeTests.cpp
@@ -215,8 +215,9 @@
 for (unsigned I = 0; I <= L.Length; ++I)
   EXPECT_THAT_EXPECTED(positionToOffset(File, position(L.Number, I)),
llvm::HasValue(L.Offset + I));
-EXPECT_THAT_EXPECTED(positionToOffset(File, position(L.Number, L.Length+1)),
- llvm::HasValue(L.Offset + L.Length));
+EXPECT_THAT_EXPECTED(
+positionToOffset(File, position(L.Number, L.Length + 1)),
+llvm::HasValue(L.Offset + L.Length));
 EXPECT_THAT_EXPECTED(
 positionToOffset(File, position(L.Number, L.Length + 1), false),
 llvm::Failed()); // out of range
@@ -413,9 +414,10 @@
   {""},
   },
   };
-  for (const auto& Case : Cases) {
+  for (const auto  : Cases) {
 EXPECT_EQ(Case.second,
-  visibleNamespaces(Case.first, format::getLLVMStyle()))
+  visibleNamespaces(Case.first, format::getFormattingLangOpts(
+format::getLLVMStyle(
 << Case.first;
   }
 }
@@ -450,7 +452,7 @@
   EXPECT_THAT(*Result, MacroName("MACRO"));
 }
 
-TEST(SourceCodeTests, IsInsideMainFile){
+TEST(SourceCodeTests, IsInsideMainFile) {
   TestTU TU;
   TU.HeaderCode = R"cpp(
 #define DEFINE_CLASS(X) class X {};
@@ -469,7 +471,7 @@
   TU.ExtraArgs.push_back("-DHeader=Header3");
   TU.ExtraArgs.push_back("-DMain=Main3");
   auto AST = TU.build();
-  const auto& SM = AST.getSourceManager();
+  const auto  = AST.getSourceManager();
   auto DeclLoc = [](llvm::StringRef Name) {
 return findDecl(AST, Name).getLocation();
   };
@@ -569,7 +571,7 @@
   TU.AdditionalFiles["foo.inc"] = "int foo;\n";
   TU.AdditionalFiles["bar.inc"] = "int bar;\n";
   auto AST = TU.build();
-  const auto& SM = AST.getSourceManager();
+  const auto  = AST.getSourceManager();
 
   FileID Foo = SM.getFileID(findDecl(AST, "foo").getLocation());
   EXPECT_EQ(SM.getFileOffset(includeHashLoc(Foo, SM)),
@@ -633,8 +635,9 @@
   for (auto Case : Cases) {
 Annotations Test(Case.Code);
 
-auto Res = getEligiblePoints(Test.code(), Case.FullyQualifiedName,
- format::getLLVMStyle());
+auto Res = getEligiblePoints(
+Test.code(), Case.FullyQualifiedName,
+format::getFormattingLangOpts(format::getLLVMStyle()));
 EXPECT_THAT(Res.EligiblePoints, testing::ElementsAreArray(Test.points()))
 << Test.code();
 EXPECT_EQ(Res.EnclosingNamespace, Case.EnclosingNamespace) << Test.code();
Index: clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
===
--- clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
+++ clang-tools-extra/clangd/refactor/tweaks/DefineOutline.cpp
@@ -284,10 +284,10 @@
 // should also try to follow ordering of declarations. For example, if decls
 // come in order `foo, bar, baz` then this function should return some point
 // between foo and baz for inserting bar.
-llvm::Expected
-getInsertionPoint(llvm::StringRef Contents, llvm::StringRef QualifiedName,
-  const format::FormatStyle ) {
-  auto Region = getEligiblePoints(Contents, QualifiedName, Style);
+llvm::Expected getInsertionPoint(llvm::StringRef Contents,
+ llvm::StringRef QualifiedName,
+ const LangOptions ) {
+  auto Region = getEligiblePoints(Contents, QualifiedName, LangOpts);
 
   assert(!Region.EligiblePoints.empty());
   // FIXME: This selection can be made smarter by looking at the definition
@@ -416,9 +416,10 @@
   return llvm::createStringError(Buffer.getError(),
  Buffer.getError().message());
 auto Contents = Buffer->get()->getBuffer();
-auto InsertionPoint =
-getInsertionPoint(Contents, Source->getQualifiedNameAsString(),
-  getFormatStyleForFile(*CCFile, Contents, ));
+auto LangOpts = format::getFormattingLangOpts(
+getFormatStyleForFile(*CCFile, Contents, ));
+auto InsertionPoint = 

[PATCH] D64464: [CodeGen] Emit destructor calls to destruct compound literals

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

This patch looks good except for that C/C++ semantic difference.  A compound 
literal temporary can be lifetime-extended in C++, but only in the standard way 
of binding a reference to it.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64464



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

[PATCH] D72875: [clang][cmake] Include generated rst files in html built by docs-clang-html target

[Dan Liew via Phabricator via cfe-commits]

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

LGTM. Thanks for addressing all the issues I raised.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72875



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

[PATCH] D64464: [CodeGen] Emit destructor calls to destruct compound literals

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/CodeGen/CGExpr.cpp:4100
+  if (E->getType().isDestructedType() == QualType::DK_nontrivial_c_struct)
+pushDestroy(QualType::DK_nontrivial_c_struct, DeclPtr, E->getType());
+

ahatanak wrote:
> rjmccall wrote:
> > rjmccall wrote:
> > > Unfortunately, the lifetime of compound literals in C is not this simple; 
> > > they're like blocks in that they're destroyed at the end of the enclosing 
> > > scope rather than at the end of the current statement. (The cleanup here 
> > > will be popped at the end of the full-expression if we've entered an 
> > > `ExprWithCleanups`.) And the l-value case is exactly the case where this 
> > > matters.
> > > 
> > > I think you need to do something like what we do with blocks, where we 
> > > record all the blocks in the full-expression on the `ExprWithCleanups` so 
> > > that we can push an inactive cleanup for them and then activate it when 
> > > we emit the block.
> > Can we make the check here something like (1) this is a block-scope 
> > compound literal and (2) it has a non-trivially-destructed type (of any 
> > kind)?  That way we're not conflating two potentially unrelated elements, 
> > the lifetime of the object and the kinds of types that can be constructed 
> > by the literal.
> > 
> > Oh, actually, there's a concrete reason to do this: C99 compound literals 
> > are not required to have struct type; they can have any object type, 
> > including arrays but also scalars.  So we could, even without non-trivial C 
> > structs, have a block-scope compound of type `__strong id[]`; I guess we've 
> > always just gotten this wrong.  Please add tests for this case. :)
> There is a check `E->isFileScope()` above this. Is that sufficient to check 
> for block-scoped compound literals?
That plus the C/C++ difference; compound literals in C++ are just temporaries.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64464



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

[PATCH] D75714: Add Optional overload to DiagnosticBuilder operator <

[Nathan James via Phabricator via cfe-commits]

njames93 created this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75714

Files:
  clang/include/clang/Basic/Diagnostic.h


Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -1311,6 +1312,15 @@
   return DB;
 }
 
+template 
+inline const DiagnosticBuilder <<(const DiagnosticBuilder ,
+   const llvm::Optional ) {
+  if (Opt) {
+DB << *Opt;
+  }
+  return DB;
+}
+
 inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
   return Report(SourceLocation(), DiagID);
 }


Index: clang/include/clang/Basic/Diagnostic.h
===
--- clang/include/clang/Basic/Diagnostic.h
+++ clang/include/clang/Basic/Diagnostic.h
@@ -21,6 +21,7 @@
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
+#include "llvm/ADT/Optional.h"
 #include "llvm/ADT/SmallVector.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/iterator_range.h"
@@ -1311,6 +1312,15 @@
   return DB;
 }
 
+template 
+inline const DiagnosticBuilder <<(const DiagnosticBuilder ,
+   const llvm::Optional ) {
+  if (Opt) {
+DB << *Opt;
+  }
+  return DB;
+}
+
 inline DiagnosticBuilder DiagnosticsEngine::Report(unsigned DiagID) {
   return Report(SourceLocation(), DiagID);
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] dcba401 - Fix 45129: Incorrect generated configuration modernize-make-shared.IncludeStyle

[Nathan James via cfe-commits]

Author: Nathan James
Date: 2020-03-06T00:07:08Z
New Revision: dcba401a39d7f3c915fdea4a220380e07fe965a3

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

LOG: Fix 45129: Incorrect generated configuration 
modernize-make-shared.IncludeStyle

Added: 


Modified: 
clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp

Removed: 




diff  --git a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp 
b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
index a41a5a2ca25d..b23ef6dd6b7d 100644
--- a/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
+++ b/clang-tools-extra/clang-tidy/modernize/MakeSmartPtrCheck.cpp
@@ -54,7 +54,7 @@ MakeSmartPtrCheck::MakeSmartPtrCheck(StringRef Name,
   IgnoreMacros(Options.getLocalOrGlobal("IgnoreMacros", true)) {}
 
 void MakeSmartPtrCheck::storeOptions(ClangTidyOptions::OptionMap ) {
-  Options.store(Opts, "IncludeStyle", IncludeStyle);
+  Options.store(Opts, "IncludeStyle", 
utils::IncludeSorter::toString(IncludeStyle));
   Options.store(Opts, "MakeSmartPtrFunctionHeader", 
MakeSmartPtrFunctionHeader);
   Options.store(Opts, "MakeSmartPtrFunction", MakeSmartPtrFunctionName);
   Options.store(Opts, "IgnoreMacros", IgnoreMacros);



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

[PATCH] D75560: Make Decl:: setOwningModuleID() public. (NFC)

[Adrian Prantl via Phabricator via cfe-commits]

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

I took your suggestion to heart and changed the downstream LLDB patch to use 
CreateDeserialized() everywhere, which is frankly a better fit there anyway. To 
make this possible I had to make a few more setters public, see the updated 
patch.

I also discovered that `DeclBase::operator new()` now always allocates 2*4 
bytes extra storage. I think it needs to do this for local submodule 
visibility. This makes my change to CreateDeserialized() not strictly 
necessary, but I think it is better anyway.


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

https://reviews.llvm.org/D75560

Files:
  clang/include/clang/AST/Decl.h
  clang/include/clang/AST/DeclBase.h
  clang/include/clang/AST/DeclCXX.h
  clang/include/clang/AST/DeclTemplate.h


Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -1891,6 +1891,10 @@
 return *TemplateArgs;
   }
 
+  void setTemplateArgs(TemplateArgumentList *Args) {
+TemplateArgs = Args;
+  }
+
   /// Determine the kind of specialization that this
   /// declaration represents.
   TemplateSpecializationKind getSpecializationKind() const {
Index: clang/include/clang/AST/DeclCXX.h
===
--- clang/include/clang/AST/DeclCXX.h
+++ clang/include/clang/AST/DeclCXX.h
@@ -2722,8 +2722,6 @@
 
   ExplicitSpecifier ExplicitSpec;
 
-  void setExplicitSpecifier(ExplicitSpecifier ES) { ExplicitSpec = ES; }
-
 public:
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
@@ -2745,6 +2743,7 @@
 
   /// Return true if the declartion is already resolved to be explicit.
   bool isExplicit() const { return getExplicitSpecifier().isExplicit(); }
+  void setExplicitSpecifier(ExplicitSpecifier ES) { ExplicitSpec = ES; }
 
   /// Returns the type that this conversion function is converting to.
   QualType getConversionType() const {
Index: clang/include/clang/AST/DeclBase.h
===
--- clang/include/clang/AST/DeclBase.h
+++ clang/include/clang/AST/DeclBase.h
@@ -626,7 +626,16 @@
 setModuleOwnershipKind(ModuleOwnershipKind::ModulePrivate);
   }
 
-  /// Set the owning module ID.
+public:
+  /// Set the FromASTFile flag. This indicates that this declaration
+  /// was deserialized and not parsed from source code and enables
+  /// features such as module ownership information.
+  void setFromASTFile() {
+FromASTFile = true;
+  }
+
+  /// Set the owning module ID.  This may only be called for
+  /// deserialized Decls.
   void setOwningModuleID(unsigned ID) {
 assert(isFromASTFile() && "Only works on a deserialized declaration");
 *((unsigned*)this - 2) = ID;
Index: clang/include/clang/AST/Decl.h
===
--- clang/include/clang/AST/Decl.h
+++ clang/include/clang/AST/Decl.h
@@ -3539,6 +3539,7 @@
   /// negative enumerators of this enum. (see getNumNegativeBits)
   void setNumNegativeBits(unsigned Num) { EnumDeclBits.NumNegativeBits = Num; }
 
+public:
   /// True if this tag declaration is a scoped enumeration. Only
   /// possible in C++11 mode.
   void setScoped(bool Scoped = true) { EnumDeclBits.IsScoped = Scoped; }
@@ -3555,6 +3556,7 @@
   /// Microsoft-style enumeration with a fixed underlying type.
   void setFixed(bool Fixed = true) { EnumDeclBits.IsFixed = Fixed; }
 
+private:
   /// True if a valid hash is stored in ODRHash.
   bool hasODRHash() const { return EnumDeclBits.HasODRHash; }
   void setHasODRHash(bool Hash = true) { EnumDeclBits.HasODRHash = Hash; }


Index: clang/include/clang/AST/DeclTemplate.h
===
--- clang/include/clang/AST/DeclTemplate.h
+++ clang/include/clang/AST/DeclTemplate.h
@@ -1891,6 +1891,10 @@
 return *TemplateArgs;
   }
 
+  void setTemplateArgs(TemplateArgumentList *Args) {
+TemplateArgs = Args;
+  }
+
   /// Determine the kind of specialization that this
   /// declaration represents.
   TemplateSpecializationKind getSpecializationKind() const {
Index: clang/include/clang/AST/DeclCXX.h
===
--- clang/include/clang/AST/DeclCXX.h
+++ clang/include/clang/AST/DeclCXX.h
@@ -2722,8 +2722,6 @@
 
   ExplicitSpecifier ExplicitSpec;
 
-  void setExplicitSpecifier(ExplicitSpecifier ES) { ExplicitSpec = ES; }
-
 public:
   friend class ASTDeclReader;
   friend class ASTDeclWriter;
@@ -2745,6 +2743,7 @@
 
   /// Return true if the declartion is already resolved to be explicit.
   bool isExplicit() const { return getExplicitSpecifier().isExplicit(); }
+  void setExplicitSpecifier(ExplicitSpecifier ES) { ExplicitSpec = ES; }
 
   /// Returns the type that this conversion function is converting to.
   QualType 

[PATCH] D75560: Make Decl:: setOwningModuleID() public. (NFC)

[Adrian Prantl via Phabricator via cfe-commits]

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



Comment at: clang/include/clang/AST/DeclBase.h:629-630
 
+public:
+  void setFromASTFile() { FromASTFile = true; }
+

rsmith wrote:
> Setting this after creating a `Decl` is not safe in general; declarations 
> with this flag set have 8 bytes of additional storage allocated before their 
> start.
> 
> An `assert(FromASTFile || hasLocalOwningModuleStorage());` would at least 
> catch the unsafe cases. This also needs a comment saying that it's not safe 
> to use in general, and perhaps a scarier name to discourage people from 
> calling it. (Unless you're creating declarations with `::CreateDeserialized` 
> -- in which case you'd need all the `Decl` subclasses to befriend the 
> creator, making this function unnecessary -- this is currently only going to 
> be safe if `ModulesLocalVisibility` is enabled.)
I think the warning is unnecessary, since the storage is always allocated. 
Please let me know if I misunderstood something.


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

https://reviews.llvm.org/D75560



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

[PATCH] D75569: [clang-tidy] New check for methods marked __attribute__((unavailable)) that do not override a method from a superclass.

[Nathan James via Phabricator via cfe-commits]

njames93 added inline comments.



Comment at: 
clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp:47-49
+  auto Iter =
+  std::find(Macros.begin(), Macros.end(), Tok.getRawIdentifier().str());
+  return Iter != Macros.end();

`return llvm::is_contained(Macros, Tok.getRawIdentifier().str())`



Comment at: 
clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp:52
+
+static FixItHint
+fixItRemovingUnavailableMethod(const ObjCMethodDecl *MD,

Should probably return an `llvm::Optional`



Comment at: 
clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp:80
+void MethodUnavailableNotOverrideCheck::registerMatchers(MatchFinder *Finder) {
+  if (!getLangOpts().ObjC)
+return;

This check is surplus to requirements now that there is the 
`isLanguageVersionSupported` check



Comment at: 
clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp:84
+  Finder->addMatcher(
+  objcMethodDecl(allOf(hasAttr(clang::attr::Unavailable), 
isNotOverriding(),
+   hasDeclContext(objcInterfaceDecl(

`objcMethodDecl` is implicitly an `allOf` matcher, so there is no need to 
specify `allOf`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75569



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

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added a comment.

In D74935#1908607 , @jeroen.dobbelaere 
wrote:

> [...]
>  So, adding the 'noalias' attribute to %pA and %pB will change the behavior 
> of foo().
>
> hmm... this explanation is maybe getting too long ?
>  The short conclusion should be:
>
> - adding 'noalias' to a pointer argument, because it is only used to read 
> memory, might lead to 'wrong code'
> - two 'noalias' pointer arguments should not overlap or relaxed:
> - two 'noalias' pointer arguments can only overlap if they (at any level of 
> indirection) are never used to write to memory.


I think we conflate two things here:

1. The modifications to the LangRef which should be in accordance with the C 
standard (at least I haven't seen you contradict the new wording directly).
2. The extended `noalias` deduction D73428 .

If you look at the commit message in D73428 , 
it says that `noalias` is not just derived for all `readonly` arguments.
In your example we access `unknown` memory, e.g., `**pA=42`, so case (1) cannot 
be applied.
For case (2) we need to show that the loads of `pA` and `pB` do not alias, 
which we cannot.

WDYT?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74935



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

[PATCH] D75708: Add warnings for casting ptr -> smaller int for C++ in Microsoft mode

[Reid Kleckner via Phabricator via cfe-commits]

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

Thanks, looks good! I'll push it in a sec.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75708



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

[PATCH] D75708: Add warnings for casting ptr -> smaller int for C++ in Microsoft mode

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 248614.
aeubanks added a comment.

- Format


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75708

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp


Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -200,17 +200,31 @@
 static const int static_var = 3; // expected-warning {{redeclaring non-static 
'static_var' as static is a Microsoft extension}}
 
 void pointer_to_integral_type_conv(char* ptr) {
-   char ch = (char)ptr;
-   short sh = (short)ptr;
-   ch = (char)ptr;
-   sh = (short)ptr;
+  char ch = (char)ptr;   // expected-warning {{cast to smaller integer type 
'char' from 'char *'}}
+  short sh = (short)ptr; // expected-warning {{cast to smaller integer type 
'short' from 'char *'}}
+  ch = (char)ptr;// expected-warning {{cast to smaller integer type 
'char' from 'char *'}}
+  sh = (short)ptr;   // expected-warning {{cast to smaller integer type 
'short' from 'char *'}}
 
-   // These are valid C++.
-   bool b = (bool)ptr;
-   b = static_cast(ptr);
+  // These are valid C++.
+  bool b = (bool)ptr;
+  b = static_cast(ptr);
 
-   // This is bad.
-   b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
+  // This is bad.
+  b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
+}
+
+void void_pointer_to_integral_type_conv(void *ptr) {
+  char ch = (char)ptr;   // expected-warning {{cast to smaller integer type 
'char' from 'void *'}}
+  short sh = (short)ptr; // expected-warning {{cast to smaller integer type 
'short' from 'void *'}}
+  ch = (char)ptr;// expected-warning {{cast to smaller integer type 
'char' from 'void *'}}
+  sh = (short)ptr;   // expected-warning {{cast to smaller integer type 
'short' from 'void *'}}
+
+  // These are valid C++.
+  bool b = (bool)ptr;
+  b = static_cast(ptr);
+
+  // This is bad.
+  b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
 }
 
 struct PR11150 {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2204,13 +2204,19 @@
 // C++ 5.2.10p4: A pointer can be explicitly converted to any integral
 //   type large enough to hold it; except in Microsoft mode, where the
 //   integral type size doesn't matter (except we don't allow bool).
-bool MicrosoftException = Self.getLangOpts().MicrosoftExt &&
-  !DestType->isBooleanType();
 if ((Self.Context.getTypeSize(SrcType) >
- Self.Context.getTypeSize(DestType)) &&
- !MicrosoftException) {
-  msg = diag::err_bad_reinterpret_cast_small_int;
-  return TC_Failed;
+ Self.Context.getTypeSize(DestType))) {
+  bool MicrosoftException =
+  Self.getLangOpts().MicrosoftExt && !DestType->isBooleanType();
+  if (MicrosoftException) {
+unsigned Diag = SrcType->isVoidPointerType()
+? diag::warn_void_pointer_to_int_cast
+: diag::warn_pointer_to_int_cast;
+Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
+  } else {
+msg = diag::err_bad_reinterpret_cast_small_int;
+return TC_Failed;
+  }
 }
 Kind = CK_PointerToIntegral;
 return TC_Success;


Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -200,17 +200,31 @@
 static const int static_var = 3; // expected-warning {{redeclaring non-static 'static_var' as static is a Microsoft extension}}
 
 void pointer_to_integral_type_conv(char* ptr) {
-   char ch = (char)ptr;
-   short sh = (short)ptr;
-   ch = (char)ptr;
-   sh = (short)ptr;
+  char ch = (char)ptr;   // expected-warning {{cast to smaller integer type 'char' from 'char *'}}
+  short sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' from 'char *'}}
+  ch = (char)ptr;// expected-warning {{cast to smaller integer type 'char' from 'char *'}}
+  sh = (short)ptr;   // expected-warning {{cast to smaller integer type 'short' from 'char *'}}
 
-   // These are valid C++.
-   bool b = (bool)ptr;
-   b = static_cast(ptr);
+  // These are valid C++.
+  bool b = (bool)ptr;
+  b = static_cast(ptr);
 
-   // This is bad.
-   b = reinterpret_cast(ptr); // expected-error {{cast from pointer to smaller type 'bool' loses information}}
+  // This is bad.
+  b = reinterpret_cast(ptr); // expected-error {{cast from 

[PATCH] D75708: Add warnings for casting ptr -> smaller int for C++ in Microsoft mode

[Reid Kleckner via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGcfff4851acc5: Add warnings for casting ptr - smaller int 
for C++ in Microsoft mode (authored by aeubanks, committed by rnk).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75708

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp


Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -200,17 +200,31 @@
 static const int static_var = 3; // expected-warning {{redeclaring non-static 
'static_var' as static is a Microsoft extension}}
 
 void pointer_to_integral_type_conv(char* ptr) {
-   char ch = (char)ptr;
-   short sh = (short)ptr;
-   ch = (char)ptr;
-   sh = (short)ptr;
+  char ch = (char)ptr;   // expected-warning {{cast to smaller integer type 
'char' from 'char *'}}
+  short sh = (short)ptr; // expected-warning {{cast to smaller integer type 
'short' from 'char *'}}
+  ch = (char)ptr;// expected-warning {{cast to smaller integer type 
'char' from 'char *'}}
+  sh = (short)ptr;   // expected-warning {{cast to smaller integer type 
'short' from 'char *'}}
 
-   // These are valid C++.
-   bool b = (bool)ptr;
-   b = static_cast(ptr);
+  // These are valid C++.
+  bool b = (bool)ptr;
+  b = static_cast(ptr);
 
-   // This is bad.
-   b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
+  // This is bad.
+  b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
+}
+
+void void_pointer_to_integral_type_conv(void *ptr) {
+  char ch = (char)ptr;   // expected-warning {{cast to smaller integer type 
'char' from 'void *'}}
+  short sh = (short)ptr; // expected-warning {{cast to smaller integer type 
'short' from 'void *'}}
+  ch = (char)ptr;// expected-warning {{cast to smaller integer type 
'char' from 'void *'}}
+  sh = (short)ptr;   // expected-warning {{cast to smaller integer type 
'short' from 'void *'}}
+
+  // These are valid C++.
+  bool b = (bool)ptr;
+  b = static_cast(ptr);
+
+  // This is bad.
+  b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
 }
 
 struct PR11150 {
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2204,13 +2204,19 @@
 // C++ 5.2.10p4: A pointer can be explicitly converted to any integral
 //   type large enough to hold it; except in Microsoft mode, where the
 //   integral type size doesn't matter (except we don't allow bool).
-bool MicrosoftException = Self.getLangOpts().MicrosoftExt &&
-  !DestType->isBooleanType();
 if ((Self.Context.getTypeSize(SrcType) >
- Self.Context.getTypeSize(DestType)) &&
- !MicrosoftException) {
-  msg = diag::err_bad_reinterpret_cast_small_int;
-  return TC_Failed;
+ Self.Context.getTypeSize(DestType))) {
+  bool MicrosoftException =
+  Self.getLangOpts().MicrosoftExt && !DestType->isBooleanType();
+  if (MicrosoftException) {
+unsigned Diag = SrcType->isVoidPointerType()
+? diag::warn_void_pointer_to_int_cast
+: diag::warn_pointer_to_int_cast;
+Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
+  } else {
+msg = diag::err_bad_reinterpret_cast_small_int;
+return TC_Failed;
+  }
 }
 Kind = CK_PointerToIntegral;
 return TC_Success;


Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -200,17 +200,31 @@
 static const int static_var = 3; // expected-warning {{redeclaring non-static 'static_var' as static is a Microsoft extension}}
 
 void pointer_to_integral_type_conv(char* ptr) {
-   char ch = (char)ptr;
-   short sh = (short)ptr;
-   ch = (char)ptr;
-   sh = (short)ptr;
+  char ch = (char)ptr;   // expected-warning {{cast to smaller integer type 'char' from 'char *'}}
+  short sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' from 'char *'}}
+  ch = (char)ptr;// expected-warning {{cast to smaller integer type 'char' from 'char *'}}
+  sh = (short)ptr;   // expected-warning {{cast to smaller integer type 'short' from 'char *'}}
 
-   // These are valid C++.
-   bool b = (bool)ptr;
-   b = static_cast(ptr);
+  // These are valid C++.
+  bool b = (bool)ptr;
+  b = static_cast(ptr);
 
-   // This is bad.
-   b = reinterpret_cast(ptr); // expected-error {{cast 

[clang] cfff485 - Add warnings for casting ptr -> smaller int for C++ in Microsoft mode

[Reid Kleckner via cfe-commits]

Author: Arthur Eubanks
Date: 2020-03-05T15:17:06-08:00
New Revision: cfff4851acc5132c8dcaebca6af92f817e133d66

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

LOG: Add warnings for casting ptr -> smaller int for C++ in Microsoft mode

Adds warnings to groups recently added in https://reviews.llvm.org/D72231.

Reviewed By: rnk

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

Added: 


Modified: 
clang/lib/Sema/SemaCast.cpp
clang/test/SemaCXX/MicrosoftExtensions.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaCast.cpp b/clang/lib/Sema/SemaCast.cpp
index 2b3b60e6bde4..17d07c57a412 100644
--- a/clang/lib/Sema/SemaCast.cpp
+++ b/clang/lib/Sema/SemaCast.cpp
@@ -2204,13 +2204,19 @@ static TryCastResult TryReinterpretCast(Sema , 
ExprResult ,
 // C++ 5.2.10p4: A pointer can be explicitly converted to any integral
 //   type large enough to hold it; except in Microsoft mode, where the
 //   integral type size doesn't matter (except we don't allow bool).
-bool MicrosoftException = Self.getLangOpts().MicrosoftExt &&
-  !DestType->isBooleanType();
 if ((Self.Context.getTypeSize(SrcType) >
- Self.Context.getTypeSize(DestType)) &&
- !MicrosoftException) {
-  msg = diag::err_bad_reinterpret_cast_small_int;
-  return TC_Failed;
+ Self.Context.getTypeSize(DestType))) {
+  bool MicrosoftException =
+  Self.getLangOpts().MicrosoftExt && !DestType->isBooleanType();
+  if (MicrosoftException) {
+unsigned Diag = SrcType->isVoidPointerType()
+? diag::warn_void_pointer_to_int_cast
+: diag::warn_pointer_to_int_cast;
+Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
+  } else {
+msg = diag::err_bad_reinterpret_cast_small_int;
+return TC_Failed;
+  }
 }
 Kind = CK_PointerToIntegral;
 return TC_Success;

diff  --git a/clang/test/SemaCXX/MicrosoftExtensions.cpp 
b/clang/test/SemaCXX/MicrosoftExtensions.cpp
index 4dca1b613421..c9703828366c 100644
--- a/clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ b/clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -200,17 +200,31 @@ extern const int static_var; // expected-note {{previous 
declaration is here}}
 static const int static_var = 3; // expected-warning {{redeclaring non-static 
'static_var' as static is a Microsoft extension}}
 
 void pointer_to_integral_type_conv(char* ptr) {
-   char ch = (char)ptr;
-   short sh = (short)ptr;
-   ch = (char)ptr;
-   sh = (short)ptr;
+  char ch = (char)ptr;   // expected-warning {{cast to smaller integer type 
'char' from 'char *'}}
+  short sh = (short)ptr; // expected-warning {{cast to smaller integer type 
'short' from 'char *'}}
+  ch = (char)ptr;// expected-warning {{cast to smaller integer type 
'char' from 'char *'}}
+  sh = (short)ptr;   // expected-warning {{cast to smaller integer type 
'short' from 'char *'}}
 
-   // These are valid C++.
-   bool b = (bool)ptr;
-   b = static_cast(ptr);
+  // These are valid C++.
+  bool b = (bool)ptr;
+  b = static_cast(ptr);
 
-   // This is bad.
-   b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
+  // This is bad.
+  b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
+}
+
+void void_pointer_to_integral_type_conv(void *ptr) {
+  char ch = (char)ptr;   // expected-warning {{cast to smaller integer type 
'char' from 'void *'}}
+  short sh = (short)ptr; // expected-warning {{cast to smaller integer type 
'short' from 'void *'}}
+  ch = (char)ptr;// expected-warning {{cast to smaller integer type 
'char' from 'void *'}}
+  sh = (short)ptr;   // expected-warning {{cast to smaller integer type 
'short' from 'void *'}}
+
+  // These are valid C++.
+  bool b = (bool)ptr;
+  b = static_cast(ptr);
+
+  // This is bad.
+  b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
 }
 
 struct PR11150 {



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

[PATCH] D75700: [NFC] Let mangler accept GlobalDecl

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:1028
 else
-  MC.mangleName(ND, Out);
+  MC.mangleName(GD, Out);
   } else {

What would be necessary in order for this to turn into just `mangleName(GD, 
Out)`?   I suspect there are a lot of subtle assumptions between the different 
mangling methods today that could probably be broken down a bit and we'd end up 
with something much cleaner.


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

https://reviews.llvm.org/D75700



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

[PATCH] D72875: [clang][cmake] Include generated rst files in html built by docs-clang-html target

[Tom Stellard via Phabricator via cfe-commits]

tstellar added a comment.

Ping.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72875



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

[PATCH] D75569: [clang-tidy] New check for methods marked __attribute__((unavailable)) that do not override a method from a superclass.

[Michael Wyman via Phabricator via cfe-commits]

mwyman updated this revision to Diff 248611.
mwyman added a comment.

Don't provide fix-it hints when the unavailable attribute is inside a macro, 
unless within a config-whitelisted macro.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75569

Files:
  clang-tools-extra/clang-tidy/objc/CMakeLists.txt
  clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.cpp
  clang-tools-extra/clang-tidy/objc/MethodUnavailableNotOverrideCheck.h
  clang-tools-extra/clang-tidy/objc/ObjCTidyModule.cpp
  clang-tools-extra/docs/ReleaseNotes.rst
  clang-tools-extra/docs/clang-tidy/checks/list.rst
  
clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst
  
clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m

Index: clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m
===
--- /dev/null
+++ clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m
@@ -0,0 +1,52 @@
+// RUN: %check_clang_tidy %s objc-method-unavailable-not-override %t -- \
+// RUN: -config='{CheckOptions: [ \
+// RUN: {key: objc-method-unavailable-not-override.FixMacroNames, \
+// RUN:  value: "UNAVAILABLE_FIXIT"} \
+// RUN: ]}' --
+
+__attribute__((objc_root_class))
+@interface Object
+- (instancetype)init;
+@end
+
+@interface MyObject : Object
+- (instancetype)init __attribute__((unavailable));
+
+// A new method that is not overriding, and is available, should not trigger.
+- (void)notOverridingMethod;
+
+- (void)methodA __attribute__((unavailable)); // methodA
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: method 'methodA' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override]
+// CHECK-FIXES: {{^\s*}}// methodA{{$}}
+
+// Verify check when unavailable attribute has a message.
+- (void)methodB __attribute__((unavailable("use methodE"))); // methodB
+// CHECK-MESSAGES: :[[@LINE-1]]:32: warning: method 'methodB' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override]
+// CHECK-FIXES: {{^\s*}}// methodB{{$}}
+
+#define UNAVAILABLE_ATTRIBUTE __attribute__((unavailable))
+#define UNAVAILABLE_FIXIT UNAVAILABLE_ATTRIBUTE
+#define UNAVAILABLE_NO_FIXIT UNAVAILABLE_ATTRIBUTE
+
+// Verify check with fix-it when using an configured macro that expands to the unavailable attribute.
+- (void)methodC UNAVAILABLE_FIXIT; // methodC
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: method 'methodC' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override]
+// CHECK-FIXES: {{^\s*}}// methodC{{$}}
+
+// Verify check without fix-it when using a non-configured macro that expands to the unavailable attribute.
+- (void)methodD UNAVAILABLE_NO_FIXIT; // methodD
+// CHECK-MESSAGES: :[[@LINE-1]]:17: warning: method 'methodD' is marked unavailable but does not override a superclass method [objc-method-unavailable-not-override]
+// CHECK-FIXES-NOT: {{^\s*}}// methodD{{$}}
+@end
+
+
+@implementation MyObject
+
+- (void)notOverridingMethod {}
+
+// Should not flag implementations for methods declared unavailable; sometimes
+// implemementations are provided that assert, to catch such codepaths that
+// lead to those calls during development.
+- (void)methodA {}
+
+@end
Index: clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst
===
--- /dev/null
+++ clang-tools-extra/docs/clang-tidy/checks/objc-method-unavailable-not-override.rst
@@ -0,0 +1,20 @@
+.. title:: clang-tidy - objc-method-unavailable-not-override
+
+objc-method-unavailable-not-override
+
+
+Checks that a method marked with ``__attribute__((unavailable))`` is overriding
+a method declaration from a superclass. That declaration can usually be
+deleted entirely.
+
+.. code-block:: objc
+
+   @interface ClassA : NSObject
+   // Neither ClassA nor any superclasses define method -foo.
+   @end
+
+   @interface ClassB : ClassA
+   - (void)foo __attribute__((unavailable));
+   @end
+
+Suggests a fix to remove the method declaration entirely.
Index: clang-tools-extra/docs/clang-tidy/checks/list.rst
===
--- clang-tools-extra/docs/clang-tidy/checks/list.rst
+++ clang-tools-extra/docs/clang-tidy/checks/list.rst
@@ -236,6 +236,7 @@
`objc-avoid-nserror-init `_,
`objc-dealloc-in-category `_,
`objc-forbidden-subclassing `_,
+   `objc-method-unavailable-not-override `_, "Yes"
`objc-missing-hash `_,
`objc-property-declaration `_, "Yes"
`objc-super-self `_, "Yes"
@@ -283,7 +284,7 @@
`readability-redundant-member-init `_, "Yes"
`readability-redundant-preprocessor `_,

[PATCH] D75708: Add warnings for casting ptr -> smaller int for C++ in Microsoft mode

[Arthur Eubanks via Phabricator via cfe-commits]

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

> I can push this for you and even give attribution (hooray, git!), but I think 
> you have enough uploaded code reviews that you could request push access as 
> described here:
>  https://llvm.org/docs/DeveloperPolicy.html#new-contributors
>  Link to all the phab issues you have opened to show contributions.

Sounds good, I'll send an email to request push access.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75708



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

[PATCH] D75569: [clang-tidy] New check for methods marked __attribute__((unavailable)) that do not override a method from a superclass.

[Michael Wyman via Phabricator via cfe-commits]

mwyman marked 2 inline comments as done.
mwyman added a comment.

I goofed on updating with Arcanist—the changes I marked done will be incoming 
shortly!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75569



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

[PATCH] D75708: Add warnings for casting ptr -> smaller int for C++ in Microsoft mode

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks updated this revision to Diff 248610.
aeubanks added a comment.

- Add tests for void *


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75708

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp


Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -200,10 +200,24 @@
 static const int static_var = 3; // expected-warning {{redeclaring non-static 
'static_var' as static is a Microsoft extension}}
 
 void pointer_to_integral_type_conv(char* ptr) {
-   char ch = (char)ptr;
-   short sh = (short)ptr;
-   ch = (char)ptr;
-   sh = (short)ptr;
+   char ch = (char)ptr; // expected-warning {{cast to smaller integer type 
'char' from 'char *'}}
+   short sh = (short)ptr; // expected-warning {{cast to smaller integer type 
'short' from 'char *'}}
+   ch = (char)ptr; // expected-warning {{cast to smaller integer type 'char' 
from 'char *'}}
+   sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' 
from 'char *'}}
+
+   // These are valid C++.
+   bool b = (bool)ptr;
+   b = static_cast(ptr);
+
+   // This is bad.
+   b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
+}
+
+void void_pointer_to_integral_type_conv(void* ptr) {
+   char ch = (char)ptr; // expected-warning {{cast to smaller integer type 
'char' from 'void *'}}
+   short sh = (short)ptr; // expected-warning {{cast to smaller integer type 
'short' from 'void *'}}
+   ch = (char)ptr; // expected-warning {{cast to smaller integer type 'char' 
from 'void *'}}
+   sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' 
from 'void *'}}
 
// These are valid C++.
bool b = (bool)ptr;
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2204,13 +2204,19 @@
 // C++ 5.2.10p4: A pointer can be explicitly converted to any integral
 //   type large enough to hold it; except in Microsoft mode, where the
 //   integral type size doesn't matter (except we don't allow bool).
-bool MicrosoftException = Self.getLangOpts().MicrosoftExt &&
-  !DestType->isBooleanType();
 if ((Self.Context.getTypeSize(SrcType) >
- Self.Context.getTypeSize(DestType)) &&
- !MicrosoftException) {
-  msg = diag::err_bad_reinterpret_cast_small_int;
-  return TC_Failed;
+ Self.Context.getTypeSize(DestType))) {
+  bool MicrosoftException =
+  Self.getLangOpts().MicrosoftExt && !DestType->isBooleanType();
+  if (MicrosoftException) {
+unsigned Diag = SrcType->isVoidPointerType()
+? diag::warn_void_pointer_to_int_cast
+: diag::warn_pointer_to_int_cast;
+Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
+  } else {
+msg = diag::err_bad_reinterpret_cast_small_int;
+return TC_Failed;
+  }
 }
 Kind = CK_PointerToIntegral;
 return TC_Success;


Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -200,10 +200,24 @@
 static const int static_var = 3; // expected-warning {{redeclaring non-static 'static_var' as static is a Microsoft extension}}
 
 void pointer_to_integral_type_conv(char* ptr) {
-   char ch = (char)ptr;
-   short sh = (short)ptr;
-   ch = (char)ptr;
-   sh = (short)ptr;
+   char ch = (char)ptr; // expected-warning {{cast to smaller integer type 'char' from 'char *'}}
+   short sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' from 'char *'}}
+   ch = (char)ptr; // expected-warning {{cast to smaller integer type 'char' from 'char *'}}
+   sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' from 'char *'}}
+
+   // These are valid C++.
+   bool b = (bool)ptr;
+   b = static_cast(ptr);
+
+   // This is bad.
+   b = reinterpret_cast(ptr); // expected-error {{cast from pointer to smaller type 'bool' loses information}}
+}
+
+void void_pointer_to_integral_type_conv(void* ptr) {
+   char ch = (char)ptr; // expected-warning {{cast to smaller integer type 'char' from 'void *'}}
+   short sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' from 'void *'}}
+   ch = (char)ptr; // expected-warning {{cast to smaller integer type 'char' from 'void *'}}
+   sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' from 'void *'}}
 
// These are valid C++.
bool b = (bool)ptr;
Index: clang/lib/Sema/SemaCast.cpp

[PATCH] D75569: [clang-tidy] New check for methods marked __attribute__((unavailable)) that do not override a method from a superclass.

[Michael Wyman via Phabricator via cfe-commits]

mwyman marked 6 inline comments as done.
mwyman added inline comments.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m:34
+// Verify check when using a macro that expands to the unavailable attribute.
+- (void)methodC NS_UNAVAILABLE;
+// CHECK-MESSAGES: :[[@LINE-1]]:1: warning: method 'methodC' is marked 
unavailable but does not override a superclass method 
[objc-method-unavailable-not-override]

njames93 wrote:
> Generally we are cautious about modifying MACRO usages in clang_tidy as we 
> don't know if its definition can change based on configuration, perhaps its 
> safer to just warn instead of providing a fix it
Sounds reasonable; I made this the default behavior.

However for Objective-C, it's quite common for developers to use a macro from 
the Apple SDKs like NS_UNAVAILABLE that are unconditional in any situations I 
know of. I added a config option to allow whitelisting macros that should have 
fix-its provided.



Comment at: 
clang-tools-extra/test/clang-tidy/checkers/objc-method-unavailable-not-override.m:51
+// Verify that fixes exist to delete entire method declarations:
+// CHECK-FIXES: {{^\s*$}}

njames93 wrote:
> This is not a satisfactory check, it will ensure at least one method has been 
> deleted but it wont ensure all methods have been deleted. Would probably be 
> safer putting a unique comment on the line and having a check fix that checks 
> for an empty line and the comment for each case.
Thanks for the suggestion! Done.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75569



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

[PATCH] D74935: [LangRef][AliasAnalysis] Clarify `noalias` affects only modified objects

[Jeroen Dobbelaere via Phabricator via cfe-commits]

jeroen.dobbelaere added a comment.

In D74935#1907939 , @jdoerfert wrote:

> In D74935#1907100 , 
> @jeroen.dobbelaere wrote:
>
> > Just to give an example:
> >
> >   int foo(int* restrict *pA, int* restrict *pB) {
> > int tmp=**pB;
> > **pA=42;
> > return tmp - **pB; // **pA and **pB can refer to the same objects
> >   }
> >
> >
> > [...]
>
>
> `*pA` and `*pB` are both restrict qualified pointers, are they not?
>  If so, why can they point to the same location?


For the original example, *pA and *pB might be the same restrict pointer:

  int* restrict pC = ;
  foo(, );

> I mean, w/o the side channel stuff the two `int *` values may alias
>  even if you have `noalias` on the arguments (https://godbolt.org/z/J9v_gK).

And that is correct, as *pA might be equal to *pB for that case (even if pA and 
pB are not equal).

Restrictness adds additional information (ignoring for a moment the 'read only' 
case (see note *4)):

   // case A
   int * restrict prC;
   int * restrict prD;
   // pC and pD are separate (*1) restrict pointers  => they point to their own 
separate sets of objects set(prC), set(prD), aka prC != prD (*2)
  
   // case B
   int * restrict * prpE; 
   int * restrict * prpF;
   // prpE and prpF are _not_ restrict pointers, so prpE could be equal to prpF 
(prpE == prpF) or not (prpE != prpF)
   // *prpE and *prpF are restrict pointers. When prpE == prpF, they represent 
the same restrict pointer.
   // *prpE could point to prC or prD or a different restrict pointer => 
set(*prpE) =union(set(prC), set(prD), ...)
   // also *prpF can point to union(set(prC), set(prD), ...)
  
   // case C
   int * restrict * restrict prprG;  
   int * restrict * restrict prprH;
   // prprG and prprH are separate (*3) restrict pointers => they point to 
their own separate sets of objects set(prprG) and set(prprH)
   // because of that: prprG != prprH
   // and *prprG and *prprH represent also separate (see previous line) 
restrict points
   // So *prprG and *prprH must point to their own separate sets of objects 
set(*prprG) and set(*prprH) 
  
   // The difference between between case B and case C is that in B, prpE could 
be equal to prpF (mayAlias), and because of that, *prpE could be equal to *prpF.
   // By proving that prprG != prprH, it automatically follows that *prprG != 
*prprH (except for (*4))
  
  // Notes:
  // (*1)   !=  
  // (*2) it actually is: for all i,j for which prC[i] and prD[j] are 'valid',  
[i] != [j]
  // (*3)   != 
  // (*4) The restrict definition contains a relaxation when restrict pointers 
are only used for reading memory: 
  //   In that case, separate restrict pointers are allowed to point to the 
same set of objects... except for:
  //  But: it also states that (6.7.3.1 paragraph 4 : '.. Every  access  that 
modifies X shall be considered also to modify P,for the purposes of this 
subclause. .. '
  //  Which means that if your write to **prprG, it is _as if_ you also write 
to *prprG and because of that, both prprG and *prprG must point to their own 
set of objects set(prprG) and set(*prprG)
  //  and those sets must also be different from set(prprH) and set(*prprH)

Now, in the original example, we have 'case B'.  pA could be equal to pB 
(mayAlias), and because of that, *pA could be equal to *pB.

  int foo(int* restrict *pA, int* restrict *pB);

When we can proof that pA != pB (noalias), then *pA and *pB represent different 
restrict pointers and *pA != *pB. (different restrict pointers point to their 
own set of objects)

So, adding the 'noalias' attribute to %pA and %pB will change the behavior of 
foo().

hmm... this explanation is maybe getting too long ?
The short conclusion should be:

- adding 'noalias' to a pointer argument, because it is only used to read 
memory, might lead to 'wrong code'
- two 'noalias' pointer arguments should not overlap

or relaxed:

- two 'noalias' pointer arguments can only overlap if they (at any level of 
indirection) are never used to write to memory.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D74935



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

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers marked 2 inline comments as done and an inline comment as not 
done.
nickdesaulniers added inline comments.



Comment at: clang/lib/Parse/ParseStmtAsm.cpp:725
 /// [GNU] gnu-asm-statement:
-/// 'asm' type-qualifier[opt] '(' asm-argument ')' ';'
+/// 'asm' asm-qualifier[opt] '(' asm-argument ')' ';'
 ///

I guess this should be `asm-qualifier-list[opt]`?



Comment at: clang/test/Parser/asm-qualifiers.c:54
+asm ("");
+asm volatile (""); // expected-warning {{meaningless 'volatile' on asm outside 
function}}
+asm inline (""); // expected-error {{expected '(' after 'asm'}}

I'll probably also drop parsing for this here, too, in a follow up; GCC treats 
this case as a parse error.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75563



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

[PATCH] D75571: [Sema][SVE] Add tests for valid and invalid type usage

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm marked 2 inline comments as done.
rsandifo-arm added inline comments.



Comment at: clang/test/SemaCXX/sizeless-1.cpp:8
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall 
-Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall 
-Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=gnu++17 
%s
+

efriedma wrote:
> This is a lot of RUN lines (both here and in the other file).  Are you really 
> getting any significant benefit from them?
Yeah, I probably went a bit overboard there.  The idea originally was to make 
sure that sizeless types worked with every standard, but of course that list is 
going to get out of date fast.  (E.g. it predated C++20.)

I've now restricted it to:

* the earliest standard
* standards that are tested explicitly in preprocessor conditions
* one gnu standard per test


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75571



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

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers updated this revision to Diff 248603.
nickdesaulniers marked 9 inline comments as done.
nickdesaulniers added a comment.

- address review comments, add tests for global asm statements.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75563

Files:
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticParseKinds.td
  clang/include/clang/Parse/Parser.h
  clang/include/clang/Sema/DeclSpec.h
  clang/lib/Parse/ParseStmtAsm.cpp
  clang/lib/Sema/DeclSpec.cpp
  clang/test/Parser/asm-qualifiers.c

Index: clang/test/Parser/asm-qualifiers.c
===
--- /dev/null
+++ clang/test/Parser/asm-qualifiers.c
@@ -0,0 +1,62 @@
+// RUN: %clang_cc1 -triple x86_64-linux-gnu -fsyntax-only -verify %s
+
+void qualifiers(void) {
+  asm("");
+  asm volatile("");
+  asm inline("");
+  asm goto("" foo);
+foo:;
+}
+
+void unknown_qualifiers(void) {
+  asm noodle(""); // expected-error {{expected '(' after 'asm'}}
+  asm goto noodle("" foo); // expected-error {{expected '(' after 'asm'}}
+  asm volatile noodle inline(""); // expected-error {{expected '(' after 'asm'}}
+foo:;
+}
+
+void underscores(void) {
+  __asm__("");
+  __asm__ __volatile__("");
+  __asm__ __inline__("");
+  // Note: goto is not supported with underscore prefix+suffix.
+  __asm__ goto("" foo);
+foo:;
+}
+
+void permutations(void) {
+  asm goto inline volatile("" foo);
+  asm goto inline("");
+  asm goto volatile inline("" foo);
+  asm goto volatile("");
+  asm inline goto volatile("" foo);
+  asm inline goto("" foo);
+  asm inline volatile goto("" foo);
+  asm inline volatile("");
+  asm volatile goto("" foo);
+  asm volatile inline goto("" foo);
+  asm volatile inline("");
+foo:;
+}
+
+void duplicates(void) {
+  asm volatile volatile(""); // expected-error {{duplicate asm qualifier 'volatile'}}
+  __asm__ __volatile__ __volatile__(""); // expected-error {{duplicate asm qualifier 'volatile'}}
+  asm inline inline(""); // expected-error {{duplicate asm qualifier 'inline'}}
+  __asm__ __inline__ __inline__(""); // expected-error {{duplicate asm qualifier 'inline'}}
+  asm goto goto("" foo); // expected-error {{duplicate asm qualifier 'goto'}}
+  __asm__ goto goto("" foo); // expected-error {{duplicate asm qualifier 'goto'}}
+foo:;
+}
+
+// globals
+asm ("");
+asm volatile (""); // expected-warning {{meaningless 'volatile' on asm outside function}}
+asm inline (""); // expected-error {{expected '(' after 'asm'}}
+// expected-error@-1 {{expected ';' after top-level asm block}}
+// expected-error@-2 {{expected identifier or '('}}
+// expected-error@-3 {{expected ')'}}
+// expected-note@-4 {{to match this '('}}
+asm goto (""noodle); // expected-error {{expected '(' after 'asm'}}
+// expected-error@-1 {{expected ';' after top-level asm block}}
+// expected-error@-2 {{expected identifier or '('}}
Index: clang/lib/Sema/DeclSpec.cpp
===
--- clang/lib/Sema/DeclSpec.cpp
+++ clang/lib/Sema/DeclSpec.cpp
@@ -589,6 +589,16 @@
   llvm_unreachable("Unknown typespec!");
 }
 
+const char *DeclSpec::getSpecifierName(AQ A) {
+  switch (A) {
+case DeclSpec::AQ_unspecified: return "unspecified";
+case DeclSpec::AQ_volatile: return "volatile";
+case DeclSpec::AQ_inline: return "inline";
+case DeclSpec::AQ_goto: return "goto";
+  }
+  llvm_unreachable("Unknown GNUAsmQualifier!");
+}
+
 bool DeclSpec::SetStorageClassSpec(Sema , SCS SC, SourceLocation Loc,
const char *,
unsigned ,
@@ -938,6 +948,12 @@
   llvm_unreachable("Unknown type qualifier!");
 }
 
+bool DeclSpec::setGNUAsmQualifier(AQ A, SourceLocation Loc) {
+  bool IsInvalid = GNUAsmQualifiers & A;
+  GNUAsmQualifiers |= A;
+  return IsInvalid;
+}
+
 bool DeclSpec::setFunctionSpecInline(SourceLocation Loc, const char *,
  unsigned ) {
   // 'inline inline' is ok.  However, since this is likely not what the user
Index: clang/lib/Parse/ParseStmtAsm.cpp
===
--- clang/lib/Parse/ParseStmtAsm.cpp
+++ clang/lib/Parse/ParseStmtAsm.cpp
@@ -684,13 +684,45 @@
 ClobberRefs, Exprs, EndLoc);
 }
 
+/// ParseGNUAsmQualifierListOpt - Parse a GNU extended asm qualifier list.
+///   asm-qualifier:
+/// volatile
+/// inline
+/// goto
+///
+///   asm-qualifier-list:
+/// asm-qualifier
+/// asm-qualifier-list asm-qualifier
+void Parser::ParseGNUAsmQualifierListOpt(DeclSpec ) {
+  SourceLocation EndLoc;
+  while (1) {
+SourceLocation Loc = Tok.getLocation();
+DeclSpec::AQ AQ = DeclSpec::AQ_unspecified;
+
+if (Tok.getKind() == tok::kw_volatile)
+

[PATCH] D75571: [Sema][SVE] Add tests for valid and invalid type usage

[Richard Sandiford via Phabricator via cfe-commits]

rsandifo-arm updated this revision to Diff 248601.
rsandifo-arm added a comment.

Changes in v3:

- Prune the number of RUN: lines.

- Minor tweaks to the tests themselves.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75571

Files:
  clang/test/Sema/sizeless-1.c
  clang/test/SemaCXX/sizeless-1.cpp

Index: clang/test/SemaCXX/sizeless-1.cpp
===
--- /dev/null
+++ clang/test/SemaCXX/sizeless-1.cpp
@@ -0,0 +1,469 @@
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++98 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++11 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=c++17 %s
+// RUN: %clang_cc1 -fcxx-exceptions -fsyntax-only -verify -W -Wall -Wrange-loop-analysis -triple arm64-linux-gnu -target-feature +sve -std=gnu++17 %s
+
+namespace std {
+struct type_info;
+}
+
+typedef __SVInt8_t svint8_t;
+typedef __SVInt16_t svint16_t;
+
+svint8_t *global_int8_ptr;
+extern svint8_t *extern_int8_ptr;
+static svint8_t *static_int8_ptr;
+
+typedef svint8_t int8_typedef;
+typedef svint8_t *int8_ptr_typedef;
+
+void pass_int8(svint8_t); // expected-note {{no known conversion}}
+
+svint8_t return_int8();
+
+typedef svint8_t vec_int8_a __attribute__((vector_size(64)));// expected-error {{invalid vector element type}}
+typedef svint8_t vec_int8_b __attribute__((ext_vector_type(4))); // expected-error {{invalid vector element type}}
+
+void dump(const volatile void *);
+
+void overf(svint8_t);
+void overf(svint16_t);
+
+void overf8(svint8_t); // expected-note + {{not viable}}
+void overf8(int);  // expected-note + {{not viable}}
+
+void overf16(svint16_t); // expected-note + {{not viable}}
+void overf16(int);   // expected-note + {{not viable}}
+
+void varargs(int, ...);
+
+void unused() {
+  svint8_t unused_int8; // expected-warning {{unused}}
+}
+
+struct incomplete_struct *incomplete_ptr;
+
+void func(int sel) {
+  svint8_t local_int8;
+  svint16_t local_int16;
+
+  // Using pointers to sizeless data isn't wrong here, but because the
+  // type is incomplete, it doesn't provide any alignment guarantees.
+  _Static_assert(__atomic_is_lock_free(1, _int8) == __atomic_is_lock_free(1, incomplete_ptr), "");
+  _Static_assert(__atomic_is_lock_free(2, _int8) == __atomic_is_lock_free(2, incomplete_ptr), ""); // expected-error {{static_assert expression is not an integral constant expression}}
+  _Static_assert(__atomic_always_lock_free(1, _int8) == __atomic_always_lock_free(1, incomplete_ptr), "");
+
+  local_int8; // expected-warning {{expression result unused}}
+
+  (void)local_int8;
+
+  local_int8, 0; // expected-warning + {{expression result unused}}
+
+  0, local_int8; // expected-warning + {{expression result unused}}
+
+  svint8_t init_int8 = local_int8;
+  svint8_t bad_init_int8 = for; // expected-error {{expected expression}}
+
+#if __cplusplus >= 201103L
+  int empty_brace_init_int = {};
+#else
+  int empty_brace_init_int = {}; // expected-error {{scalar initializer cannot be empty}}
+#endif
+
+  const svint8_t const_int8 = local_int8; // expected-note {{declared const here}}
+  const svint8_t uninit_const_int8;   // expected-error {{default initialization of an object of const type 'const svint8_t'}}
+
+  volatile svint8_t volatile_int8;
+
+  const volatile svint8_t const_volatile_int8 = local_int8; // expected-note {{declared const here}}
+  const volatile svint8_t uninit_const_volatile_int8;   // expected-error {{default initialization of an object of const type 'const volatile svint8_t'}}
+
+  __restrict svint8_t restrict_int8; // expected-error {{requires a pointer or reference}}
+
+  bool test_int8 = init_int8; // expected-error {{cannot initialize a variable of type 'bool' with an lvalue of type 'svint8_t'}}
+
+  int int_int8 = init_int8; // expected-error {{cannot initialize a variable of type 'int' with an lvalue of type 'svint8_t'}}
+
+  init_int8 = local_int8;
+  init_int8 = local_int16; // expected-error {{assigning to 'svint8_t' (aka '__SVInt8_t') from incompatible type 'svint16_t'}}
+  init_int8 = sel; // expected-error {{assigning to 'svint8_t' (aka '__SVInt8_t') from incompatible type 'int'}}
+
+  sel = local_int8; // expected-error {{assigning to 'int' from incompatible type 'svint8_t'}}
+
+  local_int8 = (svint8_t)local_int8;
+  local_int8 = (const svint8_t)local_int8;
+  local_int8 = (svint8_t)local_int16; // expected-error {{C-style cast from 'svint16_t' (aka '__SVInt16_t') to 'svint8_t' (aka '__SVInt8_t') is not allowed}}
+  local_int8 = (svint8_t)0;   // expected-error {{C-style cast from 'int' to 'svint8_t' (aka '__SVInt8_t') is not 

[PATCH] D75708: Add warnings for casting ptr -> smaller int for C++ in Microsoft mode

[Reid Kleckner via Phabricator via cfe-commits]

rnk added a comment.

Looks good, but we should add a test.

I can push this for you and even give attribution (hooray, git!), but I think 
you have enough uploaded code reviews that you could request push access as 
described here:
https://llvm.org/docs/DeveloperPolicy.html#new-contributors
Link to all the phab issues you have opened to show contributions.




Comment at: clang/test/SemaCXX/MicrosoftExtensions.cpp:214
b = reinterpret_cast(ptr); // expected-error {{cast from pointer to 
smaller type 'bool' loses information}}
 }
 

Can you please add a test for the `void*` diagnostic to exercise the other side 
of the ternary?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75708



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

[PATCH] D75563: [clang][Parse] properly parse asm-qualifiers, asm inline

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added inline comments.



Comment at: clang/lib/Parse/ParseStmtAsm.cpp:704
+  AQ = DeclSpec::AQ_goto;
+else {
+  if (EndLoc.isValid())

aaron.ballman wrote:
> I would expect a diagnostic if the unknown token is anything other than a 
> left paren.
That currently is is handled by the caller, see `if (Tok.isNot(tok::l_paren)) 
{` in `Parser::ParseAsmStatement` (line 762 of 
clang/lib/Parse/ParseStmtAsm.cpp).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75563



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

[PATCH] D75613: [Sema] Reword -Wrange-loop-analysis warning messages

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75613



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

[PATCH] D75489: [clang-tidy] Generalize HeaderFileExtensions.{h, cpp}. NFC

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75489



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

[PATCH] D72860: [modules] Do not cache invalid state for modules that we attempted to load.

[David Blaikie via Phabricator via cfe-commits]

dblaikie added inline comments.



Comment at: clang/lib/Serialization/ModuleManager.cpp:183
   // Get a buffer of the file and close the file descriptor when done.
-  Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/false);
+  Buf = FileMgr.getBufferForFile(NewModule->File, /*isVolatile=*/true);
 }

vsapsai wrote:
> dexonsmith wrote:
> > rsmith wrote:
> > > dexonsmith wrote:
> > > > vsapsai wrote:
> > > > > Made this change because if we don't have a valid module but opened a 
> > > > > corresponding .pcm file earlier, there is a high chance that .pcm 
> > > > > file was rebuilt.
> > > > Please add a comment in the code explaining that.
> > > This change is proving really bad for us. This prevents using `mmap` for 
> > > loading module files, and instead forces the entire file to be loaded 
> > > every time. Please revert.
> > Can we limit the revert to explicit vs. implicit module builds?  The 
> > scenario Volodymyr was defending against is implicit-only.
> Richard, can you please tell what is your modules configuration and how you 
> are invoking clang? For implicit builds loading a module instead of `mmap` is 
> still better than building a module. But for explicit modules I see there is 
> no need to use `isVolatile` as modules aren't changing all the time.
Richard/Google use explicit modules, if that's the particular parameter you're 
asking about.

So, yes, for Google's needs a solution that allows mmap to continue to be used 
for explicit modules would suffice, I believe.

(not to in any way derail that from being addressed - I thought implicit 
modules weren't race-y - they're hashed, etc, so they shouldn't collide/be 
overwritten? Seems like a loss in performance to have to copy the whole thing 
into memory rather than using mmap, if that could be avoided by more precise 
hashing/collision avoidance?)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72860



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

[PATCH] D75708: Add warnings for casting ptr -> smaller int for C++ in Microsoft mode

[Arthur Eubanks via Phabricator via cfe-commits]

aeubanks created this revision.
aeubanks added a reviewer: rnk.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Adds warnings to groups recently added in https://reviews.llvm.org/D72231.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D75708

Files:
  clang/lib/Sema/SemaCast.cpp
  clang/test/SemaCXX/MicrosoftExtensions.cpp


Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -200,10 +200,10 @@
 static const int static_var = 3; // expected-warning {{redeclaring non-static 
'static_var' as static is a Microsoft extension}}
 
 void pointer_to_integral_type_conv(char* ptr) {
-   char ch = (char)ptr;
-   short sh = (short)ptr;
-   ch = (char)ptr;
-   sh = (short)ptr;
+   char ch = (char)ptr; // expected-warning {{cast to smaller integer type 
'char' from 'char *'}}
+   short sh = (short)ptr; // expected-warning {{cast to smaller integer type 
'short' from 'char *'}}
+   ch = (char)ptr; // expected-warning {{cast to smaller integer type 'char' 
from 'char *'}}
+   sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' 
from 'char *'}}
 
// These are valid C++.
bool b = (bool)ptr;
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2204,13 +2204,19 @@
 // C++ 5.2.10p4: A pointer can be explicitly converted to any integral
 //   type large enough to hold it; except in Microsoft mode, where the
 //   integral type size doesn't matter (except we don't allow bool).
-bool MicrosoftException = Self.getLangOpts().MicrosoftExt &&
-  !DestType->isBooleanType();
 if ((Self.Context.getTypeSize(SrcType) >
- Self.Context.getTypeSize(DestType)) &&
- !MicrosoftException) {
-  msg = diag::err_bad_reinterpret_cast_small_int;
-  return TC_Failed;
+ Self.Context.getTypeSize(DestType))) {
+  bool MicrosoftException =
+  Self.getLangOpts().MicrosoftExt && !DestType->isBooleanType();
+  if (MicrosoftException) {
+unsigned Diag = SrcType->isVoidPointerType()
+? diag::warn_void_pointer_to_int_cast
+: diag::warn_pointer_to_int_cast;
+Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
+  } else {
+msg = diag::err_bad_reinterpret_cast_small_int;
+return TC_Failed;
+  }
 }
 Kind = CK_PointerToIntegral;
 return TC_Success;


Index: clang/test/SemaCXX/MicrosoftExtensions.cpp
===
--- clang/test/SemaCXX/MicrosoftExtensions.cpp
+++ clang/test/SemaCXX/MicrosoftExtensions.cpp
@@ -200,10 +200,10 @@
 static const int static_var = 3; // expected-warning {{redeclaring non-static 'static_var' as static is a Microsoft extension}}
 
 void pointer_to_integral_type_conv(char* ptr) {
-   char ch = (char)ptr;
-   short sh = (short)ptr;
-   ch = (char)ptr;
-   sh = (short)ptr;
+   char ch = (char)ptr; // expected-warning {{cast to smaller integer type 'char' from 'char *'}}
+   short sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' from 'char *'}}
+   ch = (char)ptr; // expected-warning {{cast to smaller integer type 'char' from 'char *'}}
+   sh = (short)ptr; // expected-warning {{cast to smaller integer type 'short' from 'char *'}}
 
// These are valid C++.
bool b = (bool)ptr;
Index: clang/lib/Sema/SemaCast.cpp
===
--- clang/lib/Sema/SemaCast.cpp
+++ clang/lib/Sema/SemaCast.cpp
@@ -2204,13 +2204,19 @@
 // C++ 5.2.10p4: A pointer can be explicitly converted to any integral
 //   type large enough to hold it; except in Microsoft mode, where the
 //   integral type size doesn't matter (except we don't allow bool).
-bool MicrosoftException = Self.getLangOpts().MicrosoftExt &&
-  !DestType->isBooleanType();
 if ((Self.Context.getTypeSize(SrcType) >
- Self.Context.getTypeSize(DestType)) &&
- !MicrosoftException) {
-  msg = diag::err_bad_reinterpret_cast_small_int;
-  return TC_Failed;
+ Self.Context.getTypeSize(DestType))) {
+  bool MicrosoftException =
+  Self.getLangOpts().MicrosoftExt && !DestType->isBooleanType();
+  if (MicrosoftException) {
+unsigned Diag = SrcType->isVoidPointerType()
+? diag::warn_void_pointer_to_int_cast
+: diag::warn_pointer_to_int_cast;
+Self.Diag(OpRange.getBegin(), Diag) << SrcType << DestType << OpRange;
+  } else {
+msg = diag::err_bad_reinterpret_cast_small_int;
+return TC_Failed;
+  }
 }
 Kind = 

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 248598.
nridge marked 4 inline comments as done.
nridge added a comment.

Address review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286

Files:
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.cpp
  clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
  clang-tools-extra/clangd/ParsedAST.cpp
  clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp

Index: clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
===
--- clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
+++ clang-tools-extra/clangd/unittests/DiagnosticsTests.cpp
@@ -61,9 +61,7 @@
  arg.Edits[0].range == Range && arg.Edits[0].newText == Replacement;
 }
 
-MATCHER_P(FixMessage, Message, "") {
-  return arg.Message == Message;
-}
+MATCHER_P(FixMessage, Message, "") { return arg.Message == Message; }
 
 MATCHER_P(EqualToLSPDiag, LSPDiag,
   "LSP diagnostic " + llvm::to_string(LSPDiag)) {
@@ -258,13 +256,13 @@
   Diag(Test.range("macroarg"),
"multiple unsequenced modifications to 'y'"),
   AllOf(
-Diag(Test.range("main"),
- "use a trailing return type for this function"),
-DiagSource(Diag::ClangTidy),
-DiagName("modernize-use-trailing-return-type"),
-// Verify that we don't have "[check-name]" suffix in the message.
-WithFix(FixMessage("use a trailing return type for this function")))
-  ));
+  Diag(Test.range("main"),
+   "use a trailing return type for this function"),
+  DiagSource(Diag::ClangTidy),
+  DiagName("modernize-use-trailing-return-type"),
+  // Verify that we don't have "[check-name]" suffix in the message.
+  WithFix(FixMessage(
+  "use a trailing return type for this function");
 }
 
 TEST(DiagnosticTest, ClangTidySuppressionComment) {
@@ -274,7 +272,11 @@
   double d = 8 / i;  // NOLINT
   // NOLINTNEXTLINE
   double e = 8 / i;
-  double f = [[8]] / i;
+  #define BAD 8 / i
+  double f = BAD;  // NOLINT
+  double g = [[8]] / i;
+  #define BAD2 BAD
+  double h = BAD2;  // NOLINT
 }
   )cpp");
   TestTU TU = TestTU::withCode(Main.code());
@@ -836,8 +838,9 @@
   auto Index = buildIndexWithSymbol({});
   TU.ExternalIndex = Index.get();
 
-  EXPECT_THAT(TU.build().getDiagnostics(),
-  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
+  EXPECT_THAT(
+  TU.build().getDiagnostics(),
+  ElementsAre(Diag(Test.range(), "use of undeclared identifier 'a'")));
 }
 
 TEST(DiagsInHeaders, DiagInsideHeader) {
Index: clang-tools-extra/clangd/ParsedAST.cpp
===
--- clang-tools-extra/clangd/ParsedAST.cpp
+++ clang-tools-extra/clangd/ParsedAST.cpp
@@ -285,14 +285,13 @@
   // Check for suppression comment. Skip the check for diagnostics not
   // in the main file, because we don't want that function to query the
   // source buffer for preamble files. For the same reason, we ask
-  // shouldSuppressDiagnostic not to follow macro expansions, since
-  // those might take us into a preamble file as well.
+  // shouldSuppressDiagnostic to avoid I/O.
   bool IsInsideMainFile =
   Info.hasSourceManager() &&
   isInsideMainFile(Info.getLocation(), Info.getSourceManager());
-  if (IsInsideMainFile && tidy::shouldSuppressDiagnostic(
-  DiagLevel, Info, *CTContext,
-  /* CheckMacroExpansion = */ false)) {
+  if (IsInsideMainFile &&
+  tidy::shouldSuppressDiagnostic(DiagLevel, Info, *CTContext,
+ /*AvoidIO=*/true)) {
 return DiagnosticsEngine::Ignored;
   }
 }
Index: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
===
--- clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
+++ clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h
@@ -212,15 +212,12 @@
 /// This does not handle suppression of notes following a suppressed diagnostic;
 /// that is left to the caller is it requires maintaining state in between calls
 /// to this function.
-/// The `CheckMacroExpansion` parameter determines whether the function should
-/// handle the case where the diagnostic is inside a macro expansion. A degree
-/// of control over this is needed because handling this case can require
-/// examining source files other than the one in which the diagnostic is
-/// located, and in some use cases we cannot rely on such other files being
-/// mapped in the SourceMapper.

[PATCH] D75286: [clangd] Handle clang-tidy suppression comments for diagnostics inside macro expansions

[Nathan Ridge via Phabricator via cfe-commits]

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



Comment at: clang-tools-extra/clang-tidy/ClangTidyDiagnosticConsumer.h:215
 /// to this function.
-/// The `CheckMacroExpansion` parameter determines whether the function should
-/// handle the case where the diagnostic is inside a macro expansion. A degree
-/// of control over this is needed because handling this case can require
-/// examining source files other than the one in which the diagnostic is
-/// located, and in some use cases we cannot rely on such other files being
-/// mapped in the SourceMapper.
+/// If a valid FileID is passed as `FileFilter`, the function does not attempt
+/// to read source files other than the one identified by the filter. A degree

sammccall wrote:
> I do wonder a how hard it would be to approach this more directly: 
> (conditionally )use some API for pulling code out of the SourceManager that 
> *won't* silently do IO.
> ContentCache::getRawBuffer() or something?
> 
> Up to you whether to look at this at all of course, this approach is better 
> than what we have now.
I gave this a try in the updated patch.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75286



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

[PATCH] D75621: [clang-tidy] Use ; as separator for HeaderFileExtensions

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM with a minor nit.




Comment at: clang-tools-extra/clang-tidy/utils/FileExtensionsUtils.cpp:40
   SmallVector Suffixes;
-  AllFileExtensions.split(Suffixes, Delimiter);
+  for (const char Delimiter : Delimiters) {
+if (AllFileExtensions.contains(Delimiter)) {

Can drop the top-level `const` here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75621



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

[PATCH] D75469: Add C standard upgrade in clang-11 release note

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers abandoned this revision.
nickdesaulniers added a comment.

Abandoning; I've already pushed this to the right branch. I suspect phabricator 
doesn't watch the release brances.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75469



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

[PATCH] D75332: [clang-tidy] Add module for llvm-libc and restrict-system-libc-header-check.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D75332#1904317 , @MaskRay wrote:

> In D75332#1904264 , @PaulkaToast 
> wrote:
>
> > In D75332#1903570 , @MaskRay wrote:
> >
> > > > `RestrictSystemLibcHeadersCheck`
> > >
> > > As I commented previously, I think the checker name should be 
> > > generalized, as it does not need to be coupled with llvm-libc. Other 
> > > projects may have similar needs. For example, they don't want to 
> > > accidentally include a system zlib.h -> they may ship a bundled zlib 
> > > (say, in third_party/zlib/).
> > >
> > > Maybe `misc/` (or `portability/`) is more suitable?
> >
> >
> > This is a simple check made precisely for llvm libc's use-case and doesn't 
> > require a human maintained list. As I mentioned, if a more 
> > general/configurable check is desired then porting out the 
> > fuchsia-restrict-system-includes would make more sense as it already 
> > implements white-lists and would handle the situation you described.
>
>
>
>
> 1. D43778  fuchsia-restrict-system-includes
> 2. This patch llvmlibc-restrict-system-libc-headers
> 3. The zlib use case I mentioned.
>
>   I think we should consider generalizing the existing 
> fuchsia-restrict-system-includes (I did not know it). +@juliehockett
>
>   Fuchsia and llvm-libc developers can use a config once the general checker 
> is ready.


+1


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75332



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

[PATCH] D31343: Add an attribute plugin example

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D31343#1902903 , @john.brawn wrote:

> I've been looking into attribute merging, and as far as I can tell there's 
> two things going on:
>
> - In SemaDeclAttr.cpp, the various handleXyzAttr functions may call 
> mergeXyzAttr to handle clashes with other attributes within the same 
> declaration.
> - In SemaDecl.cpp, several functions call mergeDeclAttributes, which calls 
> mergeXyzAttr, to handle clashes with attributes in a previous declaration of 
> the same thing.


Agreed.

> For the first kind, the handleDeclAttribute function defined for a plugin 
> attribute can check for and handle clashes with other attributes.

Agreed.

> For the second kind, at that point we're using subclasses of Attr which means 
> we're beyond the scope of plugin attributes which just work in the realm of 
> ParsedAttrs.

Yeah, we are using semantic attributes at that point rather than parsed 
attributes.

> So I think things are OK, though I guess it depends on what exactly your 
> plugin is doing - our use-case is that the plugin-defined attribute is just 
> used to set an AnnotateAttr, which is used to tell a plugin-defined LLVM pass 
> that it needs to do something in that function, so we don't need anything 
> special to happen with regards to attribute merging.

I think things are fine for your example plugin. The scenario I'm worried about 
is when users want to add their own semantic attributes that need to be merged, 
or if they're using an existing semantic attribute but need to add custom 
merging logic to it for their own needs.


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

https://reviews.llvm.org/D31343



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

Re: [clang] bdad0a1 - PR45083: Mark statement expressions as being dependent if they appear in

[Benjamin Kramer via cfe-commits]

The test case is not important at all, but the crasher seems to be
rather bad. I'll send you the full test case.

On Thu, Mar 5, 2020 at 9:20 PM Richard Smith via cfe-commits
 wrote:
>
> On Thu, 5 Mar 2020 at 06:13, Benjamin Kramer via cfe-commits 
>  wrote:
>>
>> creduce produced this. It's a crash on invalid, but was created from a
>> valid input.
>
>
> Well, "valid" is unclear when using language extensions, but OK.
>
>>
>> $ cat r.ii
>> template  auto b(a) {
>>   auto c = [](auto, int) -> decltype(({})) {};
>>   return c(0, 0);
>> }
>> using d = decltype(b(0));
>> bool f = d ::e;
>
>
> How important is this testcase? The fix you reverted was fixing a 
> release-blocker; is this also a release-blocker in your view?
>
>>
>> $ clang r.ii -std=c++17 -w
>> clang-11: clang/lib/AST/Decl.cpp:2343: clang::APValue
>> *clang::VarDecl::evaluateValue(SmallVectorImpl
>> &) const: Assertion `!Init->isValueDependent()' failed.
>>
>> On Thu, Mar 5, 2020 at 2:18 PM Benjamin Kramer  wrote:
>> >
>> > It's still crashing clang, reverted this and
>> > f545ede91c9d9f271e7504282cab7bf509607ead in 66addf8e8036. c-reduce is
>> > still chewing on the reproducer.
>> >
>> > On Wed, Mar 4, 2020 at 10:20 PM Richard Smith via cfe-commits
>> >  wrote:
>> > >
>> > > We found a regression introduced by this patch; fixed in 
>> > > f545ede91c9d9f271e7504282cab7bf509607ead.
>> > >
>> > > On Wed, 4 Mar 2020 at 00:30, Hans Wennborg via cfe-commits 
>> > >  wrote:
>> > >>
>> > >> Pushed to 10.x as 3a843031a5ad83a00d2603f623881cb2b2bf719d. Please let
>> > >> me know if you hear about any follow-up issues.
>> > >>
>> > >> Thanks!
>> > >>
>> > >> On Wed, Mar 4, 2020 at 12:28 AM Richard Smith via cfe-commits
>> > >>  wrote:
>> > >> >
>> > >> >
>> > >> > Author: Richard Smith
>> > >> > Date: 2020-03-03T15:20:40-08:00
>> > >> > New Revision: bdad0a1b79273733df9acc1be4e992fa5d70ec56
>> > >> >
>> > >> > URL: 
>> > >> > https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56
>> > >> > DIFF: 
>> > >> > https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56.diff
>> > >> >
>> > >> > LOG: PR45083: Mark statement expressions as being dependent if they 
>> > >> > appear in
>> > >> > dependent contexts.
>> > >> >
>> > >> > We previously assumed they were neither value- nor
>> > >> > instantiation-dependent under any circumstances, which would lead to
>> > >> > crashes and other misbehavior.
>> > >> >
>> > >> > Added:
>> > >> >
>> > >> >
>> > >> > Modified:
>> > >> > clang/include/clang/AST/Expr.h
>> > >> > clang/include/clang/Sema/Sema.h
>> > >> > clang/lib/AST/ASTImporter.cpp
>> > >> > clang/lib/Parse/ParseExpr.cpp
>> > >> > clang/lib/Sema/SemaExpr.cpp
>> > >> > clang/lib/Sema/SemaExprCXX.cpp
>> > >> > clang/lib/Sema/TreeTransform.h
>> > >> > clang/test/SemaTemplate/dependent-expr.cpp
>> > >> >
>> > >> > Removed:
>> > >> >
>> > >> >
>> > >> >
>> > >> > 
>> > >> > diff  --git a/clang/include/clang/AST/Expr.h 
>> > >> > b/clang/include/clang/AST/Expr.h
>> > >> > index fcdb0b992134..87f9b883486a 100644
>> > >> > --- a/clang/include/clang/AST/Expr.h
>> > >> > +++ b/clang/include/clang/AST/Expr.h
>> > >> > @@ -3960,14 +3960,14 @@ class StmtExpr : public Expr {
>> > >> >Stmt *SubStmt;
>> > >> >SourceLocation LParenLoc, RParenLoc;
>> > >> >  public:
>> > >> > -  // FIXME: Does type-dependence need to be computed
>> > >> > diff erently?
>> > >> > -  // FIXME: Do we need to compute instantiation 
>> > >> > instantiation-dependence for
>> > >> > -  // statements? (ugh!)
>> > >> >StmtExpr(CompoundStmt *substmt, QualType T,
>> > >> > -   SourceLocation lp, SourceLocation rp) :
>> > >> > +   SourceLocation lp, SourceLocation rp, bool 
>> > >> > InDependentContext) :
>> > >> > +// Note: we treat a statement-expression in a dependent context 
>> > >> > as always
>> > >> > +// being value- and instantiation-dependent. This matches the 
>> > >> > behavior of
>> > >> > +// lambda-expressions and GCC.
>> > >> >  Expr(StmtExprClass, T, VK_RValue, OK_Ordinary,
>> > >> > - T->isDependentType(), false, false, false),
>> > >> > -SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { }
>> > >> > + T->isDependentType(), InDependentContext, 
>> > >> > InDependentContext, false),
>> > >> > +SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) {}
>> > >> >
>> > >> >/// Build an empty statement expression.
>> > >> >explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty) { 
>> > >> > }
>> > >> >
>> > >> > diff  --git a/clang/include/clang/Sema/Sema.h 
>> > >> > b/clang/include/clang/Sema/Sema.h
>> > >> > index 2304a9718567..5739808753e3 100644
>> > >> > --- a/clang/include/clang/Sema/Sema.h
>> > >> > +++ b/clang/include/clang/Sema/Sema.h
>> > >> > @@ -4964,7 +4964,7 @@ class Sema final {
>> > >> >  LabelDecl *TheDecl);

[PATCH] D64464: [CodeGen] Emit destructor calls to destruct compound literals

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 248586.
ahatanak marked an inline comment as done.
ahatanak added a comment.

Return `make_error` instead of a nullptr.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D64464

Files:
  clang/include/clang/AST/ASTImporter.h
  clang/include/clang/AST/ExprCXX.h
  clang/include/clang/AST/TextNodeDumper.h
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/include/clang/Sema/Sema.h
  clang/include/clang/Serialization/ASTBitCodes.h
  clang/lib/AST/ASTImporter.cpp
  clang/lib/AST/JSONNodeDumper.cpp
  clang/lib/AST/TextNodeDumper.cpp
  clang/lib/CodeGen/CGBlocks.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/Sema/JumpDiagnostics.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/Serialization/ASTReaderStmt.cpp
  clang/lib/Serialization/ASTWriterStmt.cpp
  clang/test/AST/ast-dump-objc-arc-json.m
  clang/test/AST/ast-dump-stmt.m
  clang/test/CodeGenObjC/arc-ternary-op.m
  clang/test/CodeGenObjC/arc.m
  clang/test/CodeGenObjC/strong-in-c-struct.m
  clang/test/Import/objc-arc/Inputs/cleanup-objects.m
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/PCH/non-trivial-c-compound-literal.m
  clang/test/SemaObjC/strong-in-c-struct.m
  clang/tools/clang-import-test/clang-import-test.cpp

Index: clang/tools/clang-import-test/clang-import-test.cpp
===
--- clang/tools/clang-import-test/clang-import-test.cpp
+++ clang/tools/clang-import-test/clang-import-test.cpp
@@ -64,6 +64,10 @@
   llvm::cl::desc("The language to parse (default: c++)"),
   llvm::cl::init("c++"));
 
+static llvm::cl::opt
+ObjCARC("objc-arc", llvm::cl::init(false),
+llvm::cl::desc("Emable ObjC ARC"));
+
 static llvm::cl::opt DumpAST("dump-ast", llvm::cl::init(false),
llvm::cl::desc("Dump combined AST"));
 
@@ -183,6 +187,8 @@
   Inv->getLangOpts()->ObjC = 1;
 }
   }
+  Inv->getLangOpts()->ObjCAutoRefCount = ObjCARC;
+
   Inv->getLangOpts()->Bool = true;
   Inv->getLangOpts()->WChar = true;
   Inv->getLangOpts()->Blocks = true;
Index: clang/test/SemaObjC/strong-in-c-struct.m
===
--- clang/test/SemaObjC/strong-in-c-struct.m
+++ clang/test/SemaObjC/strong-in-c-struct.m
@@ -54,3 +54,21 @@
   func(^{ func2(x); });
   goto *ips; // expected-error {{cannot jump}}
 }
+
+void test_compound_literal0(int cond, id x) {
+  switch (cond) {
+  case 0:
+(void)(Strong){ .a = x }; // expected-note {{jump enters lifetime of a compound literal that is non-trivial to destruct}}
+break;
+  default: // expected-error {{cannot jump from switch statement to this case label}}
+break;
+  }
+}
+
+void test_compound_literal1(id x) {
+  static void *ips[] = { & };
+L0:  // expected-note {{possible target of indirect goto}}
+  ;
+  (void)(Strong){ .a = x }; // expected-note {{jump exits lifetime of a compound literal that is non-trivial to destruct}}
+  goto *ips; // expected-error {{cannot jump}}
+}
Index: clang/test/PCH/non-trivial-c-compound-literal.m
===
--- /dev/null
+++ clang/test/PCH/non-trivial-c-compound-literal.m
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -x objective-c -fobjc-arc -emit-pch -o %t %s
+// RUN: %clang_cc1 -triple x86_64-apple-darwin10 -x objective-c -fobjc-arc -include-pch %t -emit-llvm -o - %s | FileCheck %s
+
+#ifndef HEADER
+#define HEADER
+
+typedef struct {
+  id f;
+} S;
+
+static inline id getObj(id a) {
+  S *p = &(S){ .f = a };
+  return p->f;
+}
+
+#else
+
+// CHECK: %[[STRUCT_S:.*]] = type { i8* }
+
+// CHECK: define internal i8* @getObj(
+// CHECK: %[[_COMPOUNDLITERAL:.*]] = alloca %[[STRUCT_S]],
+// CHECK: %[[V5:.*]] = bitcast %[[STRUCT_S]]* %[[_COMPOUNDLITERAL]] to i8**
+// CHECK: call void @__destructor_8_s0(i8** %[[V5]])
+
+id test(id a) {
+  return getObj(a);
+}
+
+#endif
Index: clang/test/Import/objc-arc/test-cleanup-object.m
===
--- /dev/null
+++ clang/test/Import/objc-arc/test-cleanup-object.m
@@ -0,0 +1,10 @@
+// RUN: clang-import-test -x objective-c -objc-arc -import %S/Inputs/cleanup-objects.m -dump-ast -expression %s | FileCheck %s
+
+// CHECK: FunctionDecl {{.*}} getObj '
+// CHECK: ExprWithCleanups
+// CHECK-NEXT: cleanup CompoundLiteralExpr
+
+void test(int c, id a) {
+  (void)getObj(c, a);
+}
+
Index: clang/test/Import/objc-arc/Inputs/cleanup-objects.m
===
--- /dev/null
+++ clang/test/Import/objc-arc/Inputs/cleanup-objects.m
@@ -0,0 +1,10 @@
+typedef struct {
+  id x;
+} S;
+
+id getObj(int c, id a) {
+  // Commenting out the following line because AST importer crashes when trying
+  // to import a BlockExpr.
+  // return c ? ^{ return a; }() : 

[PATCH] D73245: Depend stddef.h to provide max_align_t for C++11 and provide better fallback in

[Louis Dionne via Phabricator via cfe-commits]

ldionne added a comment.

In D73245#1896786 , @ldionne wrote:

> In D73245#1894420 , @EricWF wrote:
>
> > @ldionne Since this has the possibility of breaking existing users of 
> > `std::max_align_t`, can you test this against your c++03 codebases?
>


LGTM as far as that is concerned. (personal note: rdar://60008079)


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

https://reviews.llvm.org/D73245



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

[PATCH] D69573: [clang-format] [PR36294] AlwaysBreakAfterReturnType works incorrectly for some operator functions

[Sylvestre Ledru via Phabricator via cfe-commits]

sylvestre.ledru added a comment.

@hans done here: 
https://github.com/llvm/llvm-project/commit/50eedc134a219ef6d2345e4efc5471a2e3824223
 :)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D69573



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

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson updated this revision to Diff 248582.
tejohnson marked an inline comment as done.
tejohnson added a comment.

Fix gold plugin option


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655

Files:
  clang/docs/LTOVisibility.rst


Index: clang/docs/LTOVisibility.rst
===
--- clang/docs/LTOVisibility.rst
+++ clang/docs/LTOVisibility.rst
@@ -35,6 +35,16 @@
 (e.g. classes declared in unnamed namespaces) also receive hidden LTO
 visibility.
 
+During the LTO link, all classes with public LTO visibility will be refined
+to hidden LTO visibility when the ``-lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole-program-visibility`` for gold). This
+can be used when it is known that the LTO link has whole program visibility,
+because we are LTO linking all translation units as bitcode, except certain
+(e.g.  system) libraries or other native objects known to be safe by the user 
or
+build system, and the binary will not dlopen any libraries deriving from the
+binary’s classes. This is useful in situations where it is not safe to specify
+``-fvisibility=hidden`` at compile time.
+
 A class defined in a translation unit built without LTO receives public
 LTO visibility regardless of its object file visibility, linkage or other
 attributes.


Index: clang/docs/LTOVisibility.rst
===
--- clang/docs/LTOVisibility.rst
+++ clang/docs/LTOVisibility.rst
@@ -35,6 +35,16 @@
 (e.g. classes declared in unnamed namespaces) also receive hidden LTO
 visibility.
 
+During the LTO link, all classes with public LTO visibility will be refined
+to hidden LTO visibility when the ``-lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole-program-visibility`` for gold). This
+can be used when it is known that the LTO link has whole program visibility,
+because we are LTO linking all translation units as bitcode, except certain
+(e.g.  system) libraries or other native objects known to be safe by the user or
+build system, and the binary will not dlopen any libraries deriving from the
+binary’s classes. This is useful in situations where it is not safe to specify
+``-fvisibility=hidden`` at compile time.
+
 A class defined in a translation unit built without LTO receives public
 LTO visibility regardless of its object file visibility, linkage or other
 attributes.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Teresa Johnson via Phabricator via cfe-commits]

tejohnson marked 3 inline comments as done.
tejohnson added a subscriber: evgeny777.
tejohnson added inline comments.



Comment at: clang/docs/LTOVisibility.rst:40
+to hidden LTO visibility when the ``-lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole_program_visibility`` for gold). This
+can be used when it is known that the LTO link has whole program visibility,

pcc wrote:
> Isn't it spelled `whole-program-visibility`?
good catch, yes



Comment at: clang/docs/LTOVisibility.rst:45
+build system, and the binary will not dlopen any libraries deriving from the
+binary’s classes. This is useful in situations where it is not safe to specify
+``-fvisibility=hidden`` at compile time.

pcc wrote:
> I thought that the motivation was avoiding duplicate work in the case where 
> you need varying LTO visibility? Otherwise you could just build with and 
> without `-fvisibility=hidden` and it would be the same from an LTO visibility 
> perspective.
Yeah I started to write that out but it seemed too verbose (essentially we 
can't share a single bitcode object anymore without this, because it isn't safe 
for all dependent links, which to me is included in "situations where it is not 
safe to specify -fvisibility=hidden"). Also, on D71913, @evgeny777 mentioned he 
had a case were the symbol visibility constraints didn't allow for 
-fvisibility=hidden, but where LTO visibility for vtables can be hidden. I 
could add something more verbose about my case if you prefer.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655



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

[PATCH] D74166: [AIX][Frontend] Static init implementation for AIX considering no priority

[Xiangling Liao via Phabricator via cfe-commits]

Xiangling_L updated this revision to Diff 248571.
Xiangling_L marked 2 inline comments as done.
Xiangling_L added a comment.

Fix the formatting issue;
Address the 1st round reviews;


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

https://reviews.llvm.org/D74166

Files:
  clang/include/clang/AST/Mangle.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCXXABI.h
  clang/lib/CodeGen/CGDeclCXX.cpp
  clang/lib/CodeGen/CodeGenFunction.h
  clang/lib/CodeGen/CodeGenModule.h
  clang/lib/CodeGen/ItaniumCXXABI.cpp
  clang/lib/Sema/SemaDeclAttr.cpp
  clang/test/CodeGen/aix-priority-attribute.cpp
  clang/test/CodeGen/static-init.cpp

Index: clang/test/CodeGen/static-init.cpp
===
--- clang/test/CodeGen/static-init.cpp
+++ clang/test/CodeGen/static-init.cpp
@@ -1,12 +1,55 @@
-// RUN: not %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
-// RUN: not %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ %s \
-// RUN: -o /dev/null 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -S -emit-llvm -x c++ < %s \
+// RUN: | FileCheck %s
 
 struct test {
   test();
   ~test();
 } t;
 
-// CHECK: error in backend: Static initialization has not been implemented on XL ABI yet.
+// CHECK: @llvm.global_ctors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: @llvm.global_dtors = appending global [1 x { i32, void ()*, i8* }] [{ i32, void ()*, i8* } { i32 65535, void ()* @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd, i8* null }]
+// CHECK: define dso_local void @__cxx_global_var_init() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testC1Ev(%struct.test* @t)
+// CHECK:   %0 = call i32 @atexit(void ()* @__dtor_t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__dtor_t() #0 {
+// CHECK: entry:
+// CHECK:   call void @_ZN4testD1Ev(%struct.test* @t)
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @atexit(void ()*) #3
+
+// CHECK: define dso_local void @__cxx_global_var_destruct_t() #0 {
+// CHECK: entry:
+// CHECK:   %0 = call i32 @unatexit(void ()* @__dtor_t)
+// CHECK:   %guard.hasSrterm = icmp eq i32 %0, 0
+// CHECK:   br i1 %guard.hasSrterm, label %destruct.check, label %destruct.end
+
+// CHECK: destruct.check:
+// CHECK:   call void @__dtor_t()
+// CHECK:   br label %destruct.end
+
+// CHECK: destruct.end:
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: declare i32 @unatexit(void ()*) #3
+
+// CHECK: define dso_local void @__sinit8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_init()
+// CHECK:   ret void
+// CHECK: }
+
+// CHECK: define dso_local void @__sterm8000_clang_b2e4830f1c9d2d063e5ea946868f3bfd() #0 {
+// CHECK: entry:
+// CHECK:   call void @__cxx_global_var_destruct_t()
+// CHECK:   ret void
+// CHECK: }
Index: clang/test/CodeGen/aix-priority-attribute.cpp
===
--- /dev/null
+++ clang/test/CodeGen/aix-priority-attribute.cpp
@@ -0,0 +1,26 @@
+// RUN: %clang_cc1 -triple powerpc-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+// RUN: %clang_cc1 -triple powerpc64-ibm-aix-xcoff -x c++ -emit-llvm < %s 2>&1 | \
+// RUN: FileCheck %s
+
+int foo() __attribute__((constructor(180)));
+int bar() __attribute__((destructor(180)));
+
+class test {
+   int a;
+public:
+test(int c) {a = c;}
+~test() {a = 0;}
+};
+
+__attribute__ ((init_priority (2000)))
+test t(1);
+
+// CHECK: warning: 'constructor' attribute argument not supported:
+// CHECK: int foo() __attribute__((constructor(180)));
+
+// CHECK: warning: 'destructor' attribute argument not supported:
+// check: int bar() __attribute__((destructor(180)));
+
+// CHECK: warning: 'init_priority' attribute argument not supported:
+// CHECK: __attribute__ ((init_priority (2000)))
Index: clang/lib/Sema/SemaDeclAttr.cpp
===
--- clang/lib/Sema/SemaDeclAttr.cpp
+++ clang/lib/Sema/SemaDeclAttr.cpp
@@ -6821,7 +6821,10 @@
 handlePassObjectSizeAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Constructor:
-handleConstructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  S.Diag(AL.getLoc(), diag::warn_attribute_type_not_supported) << AL << "";
+else
+  handleConstructorAttr(S, D, AL);
 break;
   case ParsedAttr::AT_CXX11NoReturn:
 handleSimpleAttribute(S, D, AL);
@@ -6830,7 +6833,10 @@
 handleDeprecatedAttr(S, D, AL);
 break;
   case ParsedAttr::AT_Destructor:
-handleDestructorAttr(S, D, AL);
+if (S.Context.getTargetInfo().getTriple().isOSAIX())
+  

[PATCH] D75698: [analyzer][WIP] Suppress bug reports where a tracked expression's latest value change was a result of an invalidation

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus abandoned this revision.
Szelethus added a comment.

This sounds exciting! Back to work then.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75698



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

[PATCH] D75292: [clangd] Remove vsc-extension-quickstart.md from the vscode-clangd plugin

[Nathan Ridge via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGa7c655f1480f: [clangd] Remove vsc-extension-quickstart.md 
from the vscode-clangd plugin (authored by nridge).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75292

Files:
  clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md


Index: 
clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
===
--- clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
+++ /dev/null
@@ -1,33 +0,0 @@
-# Toy VS Code Extension for clangd
-
-## What's in the folder
-* This folder contains all of the files necessary for your extension
-* `package.json` - this is the manifest file in which you declare your 
extension and command.
-The sample plugin registers a command and defines its title and command name. 
With this information
-VS Code can show the command in the command palette. It doesn’t yet need to 
load the plugin.
-* `src/extension.ts` - this is the main file where you will provide the 
implementation of your command.
-The file exports one function, `activate`, which is called the very first time 
your extension is
-activated (in this case by executing the command). Inside the `activate` 
function we call `registerCommand`.
-We pass the function containing the implementation of the command as the 
second parameter to
-`registerCommand`.
-
-## Get up and running straight away
-* press `F5` to open a new window with your extension loaded
-* run your command from the command palette by pressing (`Ctrl+Shift+P` or 
`Cmd+Shift+P` on Mac) and typing `Hello World`
-* set breakpoints in your code inside `src/extension.ts` to debug your 
extension
-* find output from your extension in the debug console
-
-## Make changes
-* you can relaunch the extension from the debug toolbar after changing code in 
`src/extension.ts`
-* you can also reload (`Ctrl+R` or `Cmd+R` on Mac) the VS Code window with 
your extension to load your changes
-
-## Explore the API
-* you can open the full set of our API when you open the file 
`node_modules/vscode/vscode.d.ts`
-
-## Run tests
-* open the debug viewlet (`Ctrl+Shift+D` or `Cmd+Shift+D` on Mac) and from the 
launch configuration dropdown pick `Launch Tests`
-* press `F5` to run the tests in a new window with your extension loaded
-* see the output of the test result in the debug console
-* make changes to `test/extension.test.ts` or create new test files inside the 
`test` folder
-* by convention, the test runner will only consider files matching the 
name pattern `**.test.ts`
-* you can create folders inside the `test` folder to structure your tests 
any way you want
\ No newline at end of file


Index: clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
===
--- clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
+++ /dev/null
@@ -1,33 +0,0 @@
-# Toy VS Code Extension for clangd
-
-## What's in the folder
-* This folder contains all of the files necessary for your extension
-* `package.json` - this is the manifest file in which you declare your extension and command.
-The sample plugin registers a command and defines its title and command name. With this information
-VS Code can show the command in the command palette. It doesn’t yet need to load the plugin.
-* `src/extension.ts` - this is the main file where you will provide the implementation of your command.
-The file exports one function, `activate`, which is called the very first time your extension is
-activated (in this case by executing the command). Inside the `activate` function we call `registerCommand`.
-We pass the function containing the implementation of the command as the second parameter to
-`registerCommand`.
-
-## Get up and running straight away
-* press `F5` to open a new window with your extension loaded
-* run your command from the command palette by pressing (`Ctrl+Shift+P` or `Cmd+Shift+P` on Mac) and typing `Hello World`
-* set breakpoints in your code inside `src/extension.ts` to debug your extension
-* find output from your extension in the debug console
-
-## Make changes
-* you can relaunch the extension from the debug toolbar after changing code in `src/extension.ts`
-* you can also reload (`Ctrl+R` or `Cmd+R` on Mac) the VS Code window with your extension to load your changes
-
-## Explore the API
-* you can open the full set of our API when you open the file `node_modules/vscode/vscode.d.ts`
-
-## Run tests
-* open the debug viewlet (`Ctrl+Shift+D` or `Cmd+Shift+D` on Mac) and from the launch configuration dropdown pick `Launch Tests`
-* press `F5` to run the tests in a new window with your extension loaded
-* see the output of the test result in the debug console
-* make changes to 

[PATCH] D72841: [RFC] Add support for pragma float_control, to control precision and exception behavior at the source level

[Melanie Blower via Phabricator via cfe-commits]

mibintc added a comment.

I got an email like this "Harbormaster failed remote builds in B48237 
: Diff 248536!" but there is no further 
information.  It builds OK from my workstation.  I did have to paste the review 
because an upload to Phabricator exceeded the size limit, could that be why?  
Is there a way to get more information about the build failure?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D72841



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

[PATCH] D75271: [analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions.

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

Thanks for bringing it up, I'll attend to it -- though both of those changes 
are out of scope of this change, and I'd be strongly against solving any of 
those issues within this revision.


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

https://reviews.llvm.org/D75271



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

[PATCH] D31342: Add ParsedAttrInfo::handleDeclAttribute

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added inline comments.



Comment at: clang/lib/Sema/SemaDeclAttr.cpp:6701-6707
 if (!AL.isStmtAttr()) {
   // Type attributes are handled elsewhere; silently move on.
   assert(AL.isTypeAttr() && "Non-type attribute not handled");
   break;
 }
 S.Diag(AL.getLoc(), diag::err_stmt_attribute_invalid_on_decl)
 << AL << D->getLocation();

If we're in the situation where we parse a plugin-based attribute (so 
`AL.getKind()` falls into the `default` label) and its `handleDeclAttributes()` 
(incorrectly) returns false rather than true, it seems like we would not want 
to trigger either of these tests, correct? I'm thinking about a case where the 
plugin knows about that attribute but cannot apply it because of some other 
issues (maybe the subject is wrong, or args are incorrect, etc) and returns 
`false` here. We wouldn't want to assert or claim that this is an invalid 
statement attribute applied to a decl in that case.

Rather than requiring the user to return true from `handleDeclAttribute()`, it 
seems that what we really want is something more like:
```
if (AL.getInfo().hasHandlerFunc()) {
  AL.getInfo().handleDeclAttribute(S, D, AL);
  break;
}
if (!AL.isStmtAttr()) {
  ...
}
S.Diag(...);
```
where an unknown attribute does not have a handler, but simple and plugin 
attributes do have a handler. This way the user doesn't have the chance to 
accidentally get confused about what it means to handle an attribute. WDYT?


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

https://reviews.llvm.org/D31342



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

[PATCH] D75698: [analyzer][WIP] Suppress bug reports where a tracked expression's latest value change was a result of an invalidation

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In D75698#1908370 , @Szelethus wrote:

> In D75698#1908335 , @NoQ wrote:
>
> > In my head this patch should ideally be reduced to a single if-statement: 
> > "This value is a `SymbolDerived` //therefore// it was produced by 
> > invalidation".
>
>
> Getting to this point already forced me to learn a lot about the inner 
> workings of the analyzer, and I suspect I have quite a bit of work ahead of 
> me still. Infact, I'm not even sure what `SymbolDerived` is.


It's a chunk of a bigger symbol. Like, if we already have a symbol for an 
unknown value of a structure written in a certain region, then the symbol for a 
field of this structure would be a derived symbol from that first symbol for 
the field region.

Now, the only reason we have symbols for structures (or sometimes even for 
entire memory spaces) is because of invalidation. What remains is to tweak the 
patch in order to account for different kinds of invalidation.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75698



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

[PATCH] D75698: [analyzer][WIP] Suppress bug reports where a tracked expression's latest value change was a result of an invalidation

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Like, the only reason why we have `trackExpressionValue` is because our 
concrete values are indistinguishable from each other. Regardless of where it 
comes from, `0 (Loc)` is the same as all other `0 (Loc)`s, so we have to 
manually observe how it was copied around in order to figure out where it comes 
from.

Symbols, by design, have their origin as their identity. Whenever we encounter 
an unknown runtime value, we denote it with a symbol, therefore the symbol //by 
construction// has all the information about that runtime value: how it 
appeared, why it's unknown, and so on.

Because invalidation produces symbols and not concrete values (as it destroys 
information about the program state, while concrete values //are// such 
information), this patch doesn't need to have anything to do with value 
tracking.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75698



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

[PATCH] D75698: [analyzer][WIP] Suppress bug reports where a tracked expression's latest value change was a result of an invalidation

[Kristóf Umann via Phabricator via cfe-commits]

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

In D75698#1908335 , @NoQ wrote:

> In my head this patch should ideally be reduced to a single if-statement: 
> "This value is a `SymbolDerived` //therefore// it was produced by 
> invalidation".


Getting to this point already forced me to learn a lot about the inner workings 
of the analyzer, and I suspect I have quite a bit of work ahead of me still. 
Infact, I'm not even sure what `SymbolDerived` is. I suspect this approach 
should be abandoned then?

> It's harder than that, of course, because some derived symbols are legitimate 
> (i.e., values returned through out-parameters). But this is a separate 
> problem: instead of producing anonymous conjured symbols, invalidation should 
> produce richer symbols that explain what kind of invalidation has happened 
> and which specific effect of this invalidation is represented by this symbol. 
> Or we could instead make better symbols for representing out-parameters.

Alright, I've got some learning to then :) Thanks!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75698



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

[clang-tools-extra] a7c655f - [clangd] Remove vsc-extension-quickstart.md from the vscode-clangd plugin

[Nathan Ridge via cfe-commits]

Author: Nathan Ridge
Date: 2020-03-05T15:28:40-05:00
New Revision: a7c655f1480fbcee853fb7d9f6fd4cff80a0dc2f

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

LOG: [clangd] Remove vsc-extension-quickstart.md from the vscode-clangd plugin

Summary:

Subscribers: ilya-biryukov, MaskRay, jkorous, arphaman, kadircet, usaxena95, 
cfe-commits

Tags: #clang

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

Added: 


Modified: 


Removed: 
clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md



diff  --git 
a/clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md 
b/clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
deleted file mode 100644
index 24675ec630ca..
--- a/clang-tools-extra/clangd/clients/clangd-vscode/vsc-extension-quickstart.md
+++ /dev/null
@@ -1,33 +0,0 @@
-# Toy VS Code Extension for clangd
-
-## What's in the folder
-* This folder contains all of the files necessary for your extension
-* `package.json` - this is the manifest file in which you declare your 
extension and command.
-The sample plugin registers a command and defines its title and command name. 
With this information
-VS Code can show the command in the command palette. It doesn’t yet need to 
load the plugin.
-* `src/extension.ts` - this is the main file where you will provide the 
implementation of your command.
-The file exports one function, `activate`, which is called the very first time 
your extension is
-activated (in this case by executing the command). Inside the `activate` 
function we call `registerCommand`.
-We pass the function containing the implementation of the command as the 
second parameter to
-`registerCommand`.
-
-## Get up and running straight away
-* press `F5` to open a new window with your extension loaded
-* run your command from the command palette by pressing (`Ctrl+Shift+P` or 
`Cmd+Shift+P` on Mac) and typing `Hello World`
-* set breakpoints in your code inside `src/extension.ts` to debug your 
extension
-* find output from your extension in the debug console
-
-## Make changes
-* you can relaunch the extension from the debug toolbar after changing code in 
`src/extension.ts`
-* you can also reload (`Ctrl+R` or `Cmd+R` on Mac) the VS Code window with 
your extension to load your changes
-
-## Explore the API
-* you can open the full set of our API when you open the file 
`node_modules/vscode/vscode.d.ts`
-
-## Run tests
-* open the debug viewlet (`Ctrl+Shift+D` or `Cmd+Shift+D` on Mac) and from the 
launch configuration dropdown pick `Launch Tests`
-* press `F5` to run the tests in a new window with your extension loaded
-* see the output of the test result in the debug console
-* make changes to `test/extension.test.ts` or create new test files inside the 
`test` folder
-* by convention, the test runner will only consider files matching the 
name pattern `**.test.ts`
-* you can create folders inside the `test` folder to structure your tests 
any way you want
\ No newline at end of file



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

Re: [clang] bdad0a1 - PR45083: Mark statement expressions as being dependent if they appear in

[Richard Smith via cfe-commits]

On Thu, 5 Mar 2020 at 06:13, Benjamin Kramer via cfe-commits <
cfe-commits@lists.llvm.org> wrote:

> creduce produced this. It's a crash on invalid, but was created from a
> valid input.
>

Well, "valid" is unclear when using language extensions, but OK.


> $ cat r.ii
> template  auto b(a) {
>   auto c = [](auto, int) -> decltype(({})) {};
>   return c(0, 0);
> }
> using d = decltype(b(0));
> bool f = d ::e;
>

How important is this testcase? The fix you reverted was fixing a
release-blocker; is this also a release-blocker in your view?


> $ clang r.ii -std=c++17 -w
> clang-11: clang/lib/AST/Decl.cpp:2343: clang::APValue
> *clang::VarDecl::evaluateValue(SmallVectorImpl
> &) const: Assertion `!Init->isValueDependent()' failed.
>
> On Thu, Mar 5, 2020 at 2:18 PM Benjamin Kramer 
> wrote:
> >
> > It's still crashing clang, reverted this and
> > f545ede91c9d9f271e7504282cab7bf509607ead in 66addf8e8036. c-reduce is
> > still chewing on the reproducer.
> >
> > On Wed, Mar 4, 2020 at 10:20 PM Richard Smith via cfe-commits
> >  wrote:
> > >
> > > We found a regression introduced by this patch; fixed in
> f545ede91c9d9f271e7504282cab7bf509607ead.
> > >
> > > On Wed, 4 Mar 2020 at 00:30, Hans Wennborg via cfe-commits <
> cfe-commits@lists.llvm.org> wrote:
> > >>
> > >> Pushed to 10.x as 3a843031a5ad83a00d2603f623881cb2b2bf719d. Please let
> > >> me know if you hear about any follow-up issues.
> > >>
> > >> Thanks!
> > >>
> > >> On Wed, Mar 4, 2020 at 12:28 AM Richard Smith via cfe-commits
> > >>  wrote:
> > >> >
> > >> >
> > >> > Author: Richard Smith
> > >> > Date: 2020-03-03T15:20:40-08:00
> > >> > New Revision: bdad0a1b79273733df9acc1be4e992fa5d70ec56
> > >> >
> > >> > URL:
> https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56
> > >> > DIFF:
> https://github.com/llvm/llvm-project/commit/bdad0a1b79273733df9acc1be4e992fa5d70ec56.diff
> > >> >
> > >> > LOG: PR45083: Mark statement expressions as being dependent if they
> appear in
> > >> > dependent contexts.
> > >> >
> > >> > We previously assumed they were neither value- nor
> > >> > instantiation-dependent under any circumstances, which would lead to
> > >> > crashes and other misbehavior.
> > >> >
> > >> > Added:
> > >> >
> > >> >
> > >> > Modified:
> > >> > clang/include/clang/AST/Expr.h
> > >> > clang/include/clang/Sema/Sema.h
> > >> > clang/lib/AST/ASTImporter.cpp
> > >> > clang/lib/Parse/ParseExpr.cpp
> > >> > clang/lib/Sema/SemaExpr.cpp
> > >> > clang/lib/Sema/SemaExprCXX.cpp
> > >> > clang/lib/Sema/TreeTransform.h
> > >> > clang/test/SemaTemplate/dependent-expr.cpp
> > >> >
> > >> > Removed:
> > >> >
> > >> >
> > >> >
> > >> >
> 
> > >> > diff  --git a/clang/include/clang/AST/Expr.h
> b/clang/include/clang/AST/Expr.h
> > >> > index fcdb0b992134..87f9b883486a 100644
> > >> > --- a/clang/include/clang/AST/Expr.h
> > >> > +++ b/clang/include/clang/AST/Expr.h
> > >> > @@ -3960,14 +3960,14 @@ class StmtExpr : public Expr {
> > >> >Stmt *SubStmt;
> > >> >SourceLocation LParenLoc, RParenLoc;
> > >> >  public:
> > >> > -  // FIXME: Does type-dependence need to be computed
> > >> > diff erently?
> > >> > -  // FIXME: Do we need to compute instantiation
> instantiation-dependence for
> > >> > -  // statements? (ugh!)
> > >> >StmtExpr(CompoundStmt *substmt, QualType T,
> > >> > -   SourceLocation lp, SourceLocation rp) :
> > >> > +   SourceLocation lp, SourceLocation rp, bool
> InDependentContext) :
> > >> > +// Note: we treat a statement-expression in a dependent
> context as always
> > >> > +// being value- and instantiation-dependent. This matches the
> behavior of
> > >> > +// lambda-expressions and GCC.
> > >> >  Expr(StmtExprClass, T, VK_RValue, OK_Ordinary,
> > >> > - T->isDependentType(), false, false, false),
> > >> > -SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) { }
> > >> > + T->isDependentType(), InDependentContext,
> InDependentContext, false),
> > >> > +SubStmt(substmt), LParenLoc(lp), RParenLoc(rp) {}
> > >> >
> > >> >/// Build an empty statement expression.
> > >> >explicit StmtExpr(EmptyShell Empty) : Expr(StmtExprClass, Empty)
> { }
> > >> >
> > >> > diff  --git a/clang/include/clang/Sema/Sema.h
> b/clang/include/clang/Sema/Sema.h
> > >> > index 2304a9718567..5739808753e3 100644
> > >> > --- a/clang/include/clang/Sema/Sema.h
> > >> > +++ b/clang/include/clang/Sema/Sema.h
> > >> > @@ -4964,7 +4964,7 @@ class Sema final {
> > >> >  LabelDecl *TheDecl);
> > >> >
> > >> >void ActOnStartStmtExpr();
> > >> > -  ExprResult ActOnStmtExpr(SourceLocation LPLoc, Stmt *SubStmt,
> > >> > +  ExprResult ActOnStmtExpr(Scope *S, SourceLocation LPLoc, Stmt
> *SubStmt,
> > >> > SourceLocation RPLoc); // "({..})"
> > >> >// Handle the final expression in a statement expression.
> > >> 

[PATCH] D75697: [analyzer] Allow null false positive suppression for conditions

[Kristóf Umann via Phabricator via cfe-commits]

Szelethus added a comment.

I'm too sure what the implications of such a change is, so I'll get some 
real-life results before even thinking of commiting.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75697



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

[PATCH] D73979: [HIP] Allow non-incomplete array type for extern shared var

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D73979#1907965 , @JonChesterfield 
wrote:

> In D73979#1857736 , @yaxunl wrote:
>
> > BTW this is requested by HIP users, who have similar code for CUDA and HIP. 
> > They found it surprised that nvcc allows it but hip-clang does not.
>
>
> I think I'm one of the HIP users here, but the above change is not what I was 
> hoping for.
>
> I'd like:
>
>   __shared__ int x;
>   __shared__ int y;
>   __device__ void foo()
>   {
> assert( != );
> x = 2 * y;
>   }
>
>
> to compile and behave as it does on cuda, i.e. the 'x' variable gets 
> allocated in __shared__ memory for each kernel which accesses it, and so does 
> 'y'.
>
> The 'extern __shared__' feature where nvcc builds a union out of all the 
> things it sees and the user indexes into it at runtime is totally 
> unappealing. That cuda uses the 'extern' keyword to opt into this magic union 
> also seems undesirable.


Clang emits correct IR for this code. If you use x and y in a kernel directly, 
amdgcn backend can generate correct ISA where  and  are different. What the 
backend does is to accumulate all shared memory and get the total shared memory 
usage and assign address to different shared variables. Therefore x and y get 
different addresses.

Currently amdgcn backend emits a diagnostic message if shared variable is used 
in non-kernel function:

https://github.com/llvm/llvm-project/blob/6085593c128e91fd7db998c5441ebe120c7e4f04/llvm/lib/Target/AMDGPU/AMDGPUISelLowering.cpp#L1232

https://github.com/llvm/llvm-project/blob/3fda1fde8f7bdf3b90d8700f5a386f63409b4313/llvm/lib/Target/AMDGPU/AMDGPULegalizerInfo.cpp#L1952

This is unreasonable if the backend is able to calculate total shared memory 
usage, so this is a bug. With this bug fixed, you should be able to use shared 
variables in device functions.


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

https://reviews.llvm.org/D73979



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

[PATCH] D75556: [AST Matchers] Restrict `optionally` matcher to a single argument.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGc359f9537ffb: [AST Matchers] Restrict `optionally` matcher 
to a single argument. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75556

Files:
  clang/docs/LibASTMatchersReference.html
  clang/include/clang/ASTMatchers/ASTMatchers.h
  clang/lib/ASTMatchers/ASTMatchersInternal.cpp
  clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Index: clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
===
--- clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
+++ clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp
@@ -2007,9 +2007,8 @@
 TEST(Optionally, SubmatchersDoNotMatch) {
   EXPECT_TRUE(matchAndVerifyResultFalse(
   "class A { int a; int b; };",
-  recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
-has(fieldDecl(hasName("d")).bind("v",
-  std::make_unique>("v")));
+  recordDecl(optionally(has(fieldDecl(hasName("c")).bind("c",
+  std::make_unique>("c")));
 }
 
 // Regression test.
@@ -2028,14 +2027,8 @@
 TEST(Optionally, SubmatchersMatch) {
   EXPECT_TRUE(matchAndVerifyResultTrue(
   "class A { int a; int c; };",
-  recordDecl(optionally(has(fieldDecl(hasName("a")).bind("v")),
-has(fieldDecl(hasName("b")).bind("v",
-  std::make_unique>("v", 1)));
-  EXPECT_TRUE(matchAndVerifyResultTrue(
-  "class A { int c; int b; };",
-  recordDecl(optionally(has(fieldDecl(hasName("c")).bind("v")),
-has(fieldDecl(hasName("b")).bind("v",
-  std::make_unique>("v", 2)));
+  recordDecl(optionally(has(fieldDecl(hasName("a")).bind("v",
+  std::make_unique>("v")));
 }
 
 TEST(IsTemplateInstantiation, MatchesImplicitClassTemplateInstantiation) {
Index: clang/lib/ASTMatchers/ASTMatchersInternal.cpp
===
--- clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -346,18 +346,11 @@
 ASTMatchFinder *Finder,
 BoundNodesTreeBuilder *Builder,
 ArrayRef InnerMatchers) {
-  BoundNodesTreeBuilder Result;
-  bool Matched = false;
-  for (const DynTypedMatcher  : InnerMatchers) {
-BoundNodesTreeBuilder BuilderInner(*Builder);
-if (InnerMatcher.matches(DynNode, Finder, )) {
-  Matched = true;
-  Result.addMatch(BuilderInner);
-}
-  }
-  // If there were no matches, we can't assign to `*Builder`; we'd (incorrectly)
-  // clear it because `Result` is empty.
-  if (Matched)
+  if (InnerMatchers.size() != 1)
+return false;
+
+  BoundNodesTreeBuilder Result(*Builder);
+  if (InnerMatchers[0].matches(DynNode, Finder, ))
 *Builder = std::move(Result);
   return true;
 }
@@ -859,9 +852,8 @@
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 allOf = {internal::DynTypedMatcher::VO_AllOf};
-const internal::VariadicOperatorMatcherFunc<
-1, std::numeric_limits::max()>
-optionally = {internal::DynTypedMatcher::VO_Optionally};
+const internal::VariadicOperatorMatcherFunc<1, 1> optionally = {
+internal::DynTypedMatcher::VO_Optionally};
 const internal::VariadicFunction, StringRef,
  internal::hasAnyNameFunc>
 hasAnyName = {};
Index: clang/include/clang/ASTMatchers/ASTMatchers.h
===
--- clang/include/clang/ASTMatchers/ASTMatchers.h
+++ clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2586,13 +2586,11 @@
 2, std::numeric_limits::max()>
 allOf;
 
-/// Matches any node regardless of the submatchers.
+/// Matches any node regardless of the submatcher.
 ///
-/// However, \c optionally will generate a result binding for each matching
-/// submatcher.
-///
-/// Useful when additional information which may or may not present about a
-/// main matching node is desired.
+/// However, \c optionally will retain any bindings generated by the submatcher.
+/// Useful when additional information which may or may not present about a main
+/// matching node is desired.
 ///
 /// For example, in:
 /// \code
@@ -2612,9 +2610,7 @@
 /// member named "bar" in that class.
 ///
 /// Usable as: Any Matcher
-extern const internal::VariadicOperatorMatcherFunc<
-1, std::numeric_limits::max()>
-optionally;
+extern const internal::VariadicOperatorMatcherFunc<1, 1> optionally;
 
 /// Matches sizeof (C99), alignof (C++11) and vec_step (OpenCL)
 ///
Index: clang/docs/LibASTMatchersReference.html
===
--- clang/docs/LibASTMatchersReference.html
+++ clang/docs/LibASTMatchersReference.html
@@ -4793,14 

[PATCH] D75698: [analyzer][WIP] Suppress bug reports where a tracked expression's latest value change was a result of an invalidation

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

In my head this patch should ideally be reduced to a single if-statement: "This 
value is a `SymbolDerived` //therefore// it was produced by invalidation".

It's harder than that, of course, because some derived symbols are legitimate 
(i.e., values returned through out-parameters). But this is a separate problem: 
instead of producing anonymous conjured symbols, invalidation should produce 
richer symbols that explain what kind of invalidation has happened and which 
specific effect of this invalidation is represented by this symbol. Or we could 
instead make better symbols for representing out-parameters.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75698



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

[PATCH] D75591: [OpenMP] Add firstprivate as a default data-sharing attribute to clang

[Johannes Doerfert via Phabricator via cfe-commits]

jdoerfert added inline comments.



Comment at: clang/test/OpenMP/parallel_master_codegen.cpp:143
+// CK31:   [[A_VAL:%.+]] = alloca i32
+// CK31:   call void (%struct.ident_t*, i32, void (i32*, i32*, ...)*, ...) 
@__kmpc_fork_call(%struct.ident_t* {{.+}}, i32 1, void (i32*, i32*, ...)* 
bitcast (void (i32*, i32*, i32*)* [[OMP_OUTLINED:@.+]] to void
+

This does not look like `firstprivate` is actually doing anything here. Could 
you please confirm that the IR is the same for `default(shared)` and 
`default(firstprivate)`? If so, we need to figure out why the variable is not 
marked firstprivate.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75591



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

[clang] c359f95 - [AST Matchers] Restrict `optionally` matcher to a single argument.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2020-03-05T14:48:40-05:00
New Revision: c359f9537ffb17c4f40a933980ddb515d7ee923b

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

LOG: [AST Matchers] Restrict `optionally` matcher to a single argument.

Summary:
Currently, `optionally` can take multiple arguments, which commits it to a
particular strategy for those arguments (in this case, "for each"). We limit the
matcher to a single argument, which avoids any potential confusion and
simplifies the implementation. The user can retrieve multiple-argument
optionality, by explicitly using the desired operator (like `forEach`, `anyOf`,
`allOf`, etc.) with all children wrapped in `optionally`.

Reviewers: sbenza, aaron.ballman

Subscribers: cfe-commits

Tags: #clang

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

Added: 


Modified: 
clang/docs/LibASTMatchersReference.html
clang/include/clang/ASTMatchers/ASTMatchers.h
clang/lib/ASTMatchers/ASTMatchersInternal.cpp
clang/unittests/ASTMatchers/ASTMatchersNarrowingTest.cpp

Removed: 




diff  --git a/clang/docs/LibASTMatchersReference.html 
b/clang/docs/LibASTMatchersReference.html
index f8c56d7d9efc..dca7aa5c2cf7 100644
--- a/clang/docs/LibASTMatchersReference.html
+++ b/clang/docs/LibASTMatchersReference.html
@@ -4793,14 +4793,12 @@ AST Traversal Matchers
 
 
 
-Matcher*optionallyMatcher*, ..., 
Matcher*
-Matches any node 
regardless of the submatchers.
+Matcher*optionallyMatcher*
+Matches any node 
regardless of the submatcher.
 
-However, optionally will generate a result binding for each matching
-submatcher.
-
-Useful when additional information which may or may not present about a
-main matching node is desired.
+However, optionally will retain any bindings generated by the submatcher.
+Useful when additional information which may or may not present about a main
+matching node is desired.
 
 For example, in:
   class Foo {

diff  --git a/clang/include/clang/ASTMatchers/ASTMatchers.h 
b/clang/include/clang/ASTMatchers/ASTMatchers.h
index d45825de67db..da7e23052b28 100644
--- a/clang/include/clang/ASTMatchers/ASTMatchers.h
+++ b/clang/include/clang/ASTMatchers/ASTMatchers.h
@@ -2586,13 +2586,11 @@ extern const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 allOf;
 
-/// Matches any node regardless of the submatchers.
+/// Matches any node regardless of the submatcher.
 ///
-/// However, \c optionally will generate a result binding for each matching
-/// submatcher.
-///
-/// Useful when additional information which may or may not present about a
-/// main matching node is desired.
+/// However, \c optionally will retain any bindings generated by the 
submatcher.
+/// Useful when additional information which may or may not present about a 
main
+/// matching node is desired.
 ///
 /// For example, in:
 /// \code
@@ -2612,9 +2610,7 @@ extern const internal::VariadicOperatorMatcherFunc<
 /// member named "bar" in that class.
 ///
 /// Usable as: Any Matcher
-extern const internal::VariadicOperatorMatcherFunc<
-1, std::numeric_limits::max()>
-optionally;
+extern const internal::VariadicOperatorMatcherFunc<1, 1> optionally;
 
 /// Matches sizeof (C99), alignof (C++11) and vec_step (OpenCL)
 ///

diff  --git a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp 
b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
index 6f4132c19aed..033a5f19fd8c 100644
--- a/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
+++ b/clang/lib/ASTMatchers/ASTMatchersInternal.cpp
@@ -346,18 +346,11 @@ bool OptionallyVariadicOperator(const DynTypedNode 
,
 ASTMatchFinder *Finder,
 BoundNodesTreeBuilder *Builder,
 ArrayRef InnerMatchers) {
-  BoundNodesTreeBuilder Result;
-  bool Matched = false;
-  for (const DynTypedMatcher  : InnerMatchers) {
-BoundNodesTreeBuilder BuilderInner(*Builder);
-if (InnerMatcher.matches(DynNode, Finder, )) {
-  Matched = true;
-  Result.addMatch(BuilderInner);
-}
-  }
-  // If there were no matches, we can't assign to `*Builder`; we'd 
(incorrectly)
-  // clear it because `Result` is empty.
-  if (Matched)
+  if (InnerMatchers.size() != 1)
+return false;
+
+  BoundNodesTreeBuilder Result(*Builder);
+  if (InnerMatchers[0].matches(DynNode, Finder, ))
 *Builder = std::move(Result);
   return true;
 }
@@ -859,9 +852,8 @@ const internal::VariadicOperatorMatcherFunc<
 const internal::VariadicOperatorMatcherFunc<
 2, std::numeric_limits::max()>
 allOf = {internal::DynTypedMatcher::VO_AllOf};
-const internal::VariadicOperatorMatcherFunc<
-1, std::numeric_limits::max()>
-optionally = {internal::DynTypedMatcher::VO_Optionally};
+const 

[clang] 8d7b118 - [OPENMP50]Add codegen for update clause in depobj directive.

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-03-05T14:31:07-05:00
New Revision: 8d7b1188751b9fcc3345e80fb48a2bc00b7c315f

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

LOG: [OPENMP50]Add codegen for update clause in depobj directive.

Added codegen for update clause in depobj. Reads the number of the
elements from the first element and updates flags for each element in
the loop.
```
omp_depend_t x;
kmp_depend_info *base = (kmp_depend_info *)x;
intptr_t num = x[-1].base_addr;
kmp_depend_info *end = x + num;
kmp_depend_info *el = base;
do {
  el.flags = new_flag;
  el = [1];
} while (el != end);
```

Added: 


Modified: 
clang/lib/CodeGen/CGOpenMPRuntime.cpp
clang/lib/CodeGen/CGOpenMPRuntime.h
clang/lib/CodeGen/CGStmtOpenMP.cpp
clang/test/OpenMP/depobj_codegen.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 690634458622..620f6ea3e654 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -5360,6 +5360,67 @@ void CGOpenMPRuntime::emitDestroyClause(CodeGenFunction 
, LValue DepobjLVal,
   (void)CGF.EmitRuntimeCall(createRuntimeFunction(OMPRTL__kmpc_free), Args);
 }
 
+void CGOpenMPRuntime::emitUpdateClause(CodeGenFunction , LValue DepobjLVal,
+   OpenMPDependClauseKind NewDepKind,
+   SourceLocation Loc) {
+  ASTContext  = CGM.getContext();
+  QualType FlagsTy;
+  getDependTypes(C, KmpDependInfoTy, FlagsTy);
+  RecordDecl *KmpDependInfoRD =
+  cast(KmpDependInfoTy->getAsTagDecl());
+  llvm::Type *LLVMFlagsTy = CGF.ConvertTypeForMem(FlagsTy);
+  LValue Base = CGF.EmitLoadOfPointerLValue(
+  DepobjLVal.getAddress(CGF),
+  C.getPointerType(C.VoidPtrTy).castAs());
+  QualType KmpDependInfoPtrTy = C.getPointerType(KmpDependInfoTy);
+  Address Addr = CGF.Builder.CreatePointerBitCastOrAddrSpaceCast(
+  Base.getAddress(CGF), CGF.ConvertTypeForMem(KmpDependInfoPtrTy));
+  Base = CGF.MakeAddrLValue(Addr, KmpDependInfoTy, Base.getBaseInfo(),
+Base.getTBAAInfo());
+  llvm::Value *DepObjAddr = CGF.Builder.CreateGEP(
+  Addr.getPointer(),
+  llvm::ConstantInt::get(CGF.IntPtrTy, -1, /*isSigned=*/true));
+  LValue NumDepsBase = CGF.MakeAddrLValue(
+  Address(DepObjAddr, Addr.getAlignment()), KmpDependInfoTy,
+  Base.getBaseInfo(), Base.getTBAAInfo());
+  // NumDeps = deps[i].base_addr;
+  LValue BaseAddrLVal = CGF.EmitLValueForField(
+  NumDepsBase, *std::next(KmpDependInfoRD->field_begin(), BaseAddr));
+  llvm::Value *NumDeps = CGF.EmitLoadOfScalar(BaseAddrLVal, Loc);
+
+  Address Begin = Base.getAddress(CGF);
+  // Cast from pointer to array type to pointer to single element.
+  llvm::Value *End = CGF.Builder.CreateGEP(Begin.getPointer(), NumDeps);
+  // The basic structure here is a while-do loop.
+  llvm::BasicBlock *BodyBB = CGF.createBasicBlock("omp.body");
+  llvm::BasicBlock *DoneBB = CGF.createBasicBlock("omp.done");
+  llvm::BasicBlock *EntryBB = CGF.Builder.GetInsertBlock();
+  CGF.EmitBlock(BodyBB);
+  llvm::PHINode *ElementPHI =
+  CGF.Builder.CreatePHI(Begin.getType(), 2, "omp.elementPast");
+  ElementPHI->addIncoming(Begin.getPointer(), EntryBB);
+  Begin = Address(ElementPHI, Begin.getAlignment());
+  Base = CGF.MakeAddrLValue(Begin, KmpDependInfoTy, Base.getBaseInfo(),
+Base.getTBAAInfo());
+  // deps[i].flags = NewDepKind;
+  RTLDependenceKindTy DepKind = translateDependencyKind(NewDepKind);
+  LValue FlagsLVal = CGF.EmitLValueForField(
+  Base, *std::next(KmpDependInfoRD->field_begin(), Flags));
+  CGF.EmitStoreOfScalar(llvm::ConstantInt::get(LLVMFlagsTy, DepKind),
+FlagsLVal);
+
+  // Shift the address forward by one element.
+  Address ElementNext =
+  CGF.Builder.CreateConstGEP(Begin, /*Index=*/1, "omp.elementNext");
+  ElementPHI->addIncoming(ElementNext.getPointer(),
+  CGF.Builder.GetInsertBlock());
+  llvm::Value *IsEmpty =
+  CGF.Builder.CreateICmpEQ(ElementNext.getPointer(), End, "omp.isempty");
+  CGF.Builder.CreateCondBr(IsEmpty, DoneBB, BodyBB);
+  // Done.
+  CGF.EmitBlock(DoneBB, /*IsFinished=*/true);
+}
+
 void CGOpenMPRuntime::emitTaskCall(CodeGenFunction , SourceLocation Loc,
const OMPExecutableDirective ,
llvm::Function *TaskFunction,

diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.h 
b/clang/lib/CodeGen/CGOpenMPRuntime.h
index fe526464bb1d..f176af69b1d0 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.h
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.h
@@ -1791,6 +1791,12 @@ class CGOpenMPRuntime {
   /// directive.
   void 

[PATCH] D75494: [PowerPC] Delete PPCMachObjectWriter and triple for darwin

[Fangrui Song via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG3e851f4a688c: [PowerPC] Delete PPCMachObjectWriter and 
powerpc{,64}-apple-darwin (authored by MaskRay).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75494

Files:
  clang/test/Driver/darwin-arch-default.c
  clang/test/Driver/darwin-header-search-libstdcxx.cpp
  clang/test/Tooling/ms-asm-no-target.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/PowerPC/MCTargetDesc/CMakeLists.txt
  llvm/lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp
  llvm/lib/Target/PowerPC/MCTargetDesc/PPCMachObjectWriter.cpp
  llvm/unittests/BinaryFormat/MachOTest.cpp
  llvm/utils/gn/secondary/llvm/lib/Target/PowerPC/MCTargetDesc/BUILD.gn

Index: llvm/utils/gn/secondary/llvm/lib/Target/PowerPC/MCTargetDesc/BUILD.gn
===
--- llvm/utils/gn/secondary/llvm/lib/Target/PowerPC/MCTargetDesc/BUILD.gn
+++ llvm/utils/gn/secondary/llvm/lib/Target/PowerPC/MCTargetDesc/BUILD.gn
@@ -58,7 +58,6 @@
 "PPCMCCodeEmitter.cpp",
 "PPCMCExpr.cpp",
 "PPCMCTargetDesc.cpp",
-"PPCMachObjectWriter.cpp",
 "PPCPredicates.cpp",
 "PPCXCOFFObjectWriter.cpp",
   ]
Index: llvm/unittests/BinaryFormat/MachOTest.cpp
===
--- llvm/unittests/BinaryFormat/MachOTest.cpp
+++ llvm/unittests/BinaryFormat/MachOTest.cpp
@@ -58,8 +58,6 @@
   CHECK_CPUTYPE("arm64-apple-darwin", MachO::CPU_TYPE_ARM64);
   CHECK_CPUTYPE("arm64e-apple-darwin", MachO::CPU_TYPE_ARM64);
   CHECK_CPUTYPE("arm64_32-apple-darwin", MachO::CPU_TYPE_ARM64_32);
-  CHECK_CPUTYPE("powerpc-apple-darwin", MachO::CPU_TYPE_POWERPC);
-  CHECK_CPUTYPE("powerpc64-apple-darwin", MachO::CPU_TYPE_POWERPC64);
 
   {
 // Not a mach-o.
@@ -101,8 +99,6 @@
   CHECK_CPUSUBTYPE("arm64-apple-darwin", MachO::CPU_SUBTYPE_ARM64_ALL);
   CHECK_CPUSUBTYPE("arm64e-apple-darwin", MachO::CPU_SUBTYPE_ARM64E);
   CHECK_CPUSUBTYPE("arm64_32-apple-darwin", MachO::CPU_SUBTYPE_ARM64_32_V8);
-  CHECK_CPUSUBTYPE("powerpc-apple-darwin", MachO::CPU_SUBTYPE_POWERPC_ALL);
-  CHECK_CPUSUBTYPE("powerpc64-apple-darwin", MachO::CPU_SUBTYPE_POWERPC_ALL);
 
   {
 // Not a mach-o.
Index: llvm/lib/Target/PowerPC/MCTargetDesc/PPCMachObjectWriter.cpp
===
--- llvm/lib/Target/PowerPC/MCTargetDesc/PPCMachObjectWriter.cpp
+++ /dev/null
@@ -1,380 +0,0 @@
-//===-- PPCMachObjectWriter.cpp - PPC Mach-O Writer ---===//
-//
-// 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 "MCTargetDesc/PPCFixupKinds.h"
-#include "MCTargetDesc/PPCMCTargetDesc.h"
-#include "llvm/ADT/Twine.h"
-#include "llvm/BinaryFormat/MachO.h"
-#include "llvm/MC/MCAsmLayout.h"
-#include "llvm/MC/MCAssembler.h"
-#include "llvm/MC/MCContext.h"
-#include "llvm/MC/MCMachObjectWriter.h"
-#include "llvm/MC/MCSectionMachO.h"
-#include "llvm/MC/MCValue.h"
-#include "llvm/Support/ErrorHandling.h"
-#include "llvm/Support/Format.h"
-
-using namespace llvm;
-
-namespace {
-class PPCMachObjectWriter : public MCMachObjectTargetWriter {
-  bool recordScatteredRelocation(MachObjectWriter *Writer,
- const MCAssembler ,
- const MCAsmLayout ,
- const MCFragment *Fragment,
- const MCFixup , MCValue Target,
- unsigned Log2Size, uint64_t );
-
-  void RecordPPCRelocation(MachObjectWriter *Writer, const MCAssembler ,
-   const MCAsmLayout ,
-   const MCFragment *Fragment, const MCFixup ,
-   MCValue Target, uint64_t );
-
-public:
-  PPCMachObjectWriter(bool Is64Bit, uint32_t CPUType, uint32_t CPUSubtype)
-  : MCMachObjectTargetWriter(Is64Bit, CPUType, CPUSubtype) {}
-
-  void recordRelocation(MachObjectWriter *Writer, MCAssembler ,
-const MCAsmLayout , const MCFragment *Fragment,
-const MCFixup , MCValue Target,
-uint64_t ) override {
-if (Writer->is64Bit()) {
-  report_fatal_error("Relocation emission for MachO/PPC64 unimplemented.");
-} else
-  RecordPPCRelocation(Writer, Asm, Layout, Fragment, Fixup, Target,
-  FixedValue);
-  }
-};
-}
-
-/// computes the log2 of the size of the relocation,
-/// used for relocation_info::r_length.
-static unsigned getFixupKindLog2Size(unsigned Kind) {
-  switch (Kind) {
-  default:
-report_fatal_error("log2size(FixupKind): Unhandled fixup kind!");
-  case FK_PCRel_1:
-  case 

[PATCH] D75697: [analyzer] Allow null false positive suppression for conditions

[Artem Dergachev via Phabricator via cfe-commits]

NoQ added a comment.

Aha! We need some positive results tho, given that these are heuristics.




Comment at: clang/test/Analysis/inlining/inline-defensive-checks.m:113
   if (!mem)
-return 5/zero; // expected-warning {{Division by zero}}
+return 5/zero;
   return 0;

This code definitely deserves a warning tho. Regardless of any path-sensitive 
reasoning, it's trivial to prove that either this line has a division by zero 
or it's outright dead code. I'd leave a FIXME here. This shouldn't be 
suppressed.

More importantly, we should have more tests to demonstrate how this patch is 
useful. I.e., some code snippets in which it's obvious that the suppressed 
warning is a false positive.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75697



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

[PATCH] D71966: [Wdocumentation][RFC] Improve identifier's of \param

[Aaron Puchert via Phabricator via cfe-commits]

aaronpuchert added a comment.

Thanks for working on this, I'd like to see this being fixed.




Comment at: clang/include/clang-c/Documentation.h:383
 CINDEX_LINKAGE
-CXString clang_ParamCommandComment_getParamName(CXComment Comment);
 

gribozavr2 wrote:
> Mordante wrote:
> > gribozavr2 wrote:
> > > Mordante wrote:
> > > > gribozavr2 wrote:
> > > > > Please don't modify existing APIs in libclang -- it provides a stable 
> > > > > API and ABI, and what has shipped, can't be changed. New 
> > > > > functionality has to be exposed as new functions, while old functions 
> > > > > should be kept working to the extent possible. It means that the 
> > > > > resulting API can be subpar, but oh well, a stable ABI is a contract 
> > > > > of libclang.
> > > > I thought I had read this API was allowed to change, but required 
> > > > adding information to the release notes. (I can't find it quickly.)
> > > > I'll undo the changes to the existing functions and add new functions 
> > > > instead.
> > > > I thought I had read this API was allowed to change
> > > 
> > > It would be interesting to find that doc. As far as I understand, 
> > > libclang has a strict API & ABI stability rule.
> > > 
> > > > I'll undo the changes to the existing functions and add new functions 
> > > > instead.
> > > 
> > > Thanks!
> > > 
> > >> I thought I had read this API was allowed to change
> > > It would be interesting to find that doc. As far as I understand, 
> > > libclang has a strict API & ABI stability rule.
> > My interpretation of 
> > http://llvm.org/docs/DeveloperPolicy.html#c-api-changes gave me this 
> > impression.
> I see. Index.h explains the policy for libclang: 
> https://github.com/llvm/llvm-project/blob/master/clang/include/clang-c/Index.h#L32-L33
> 
> If you feel like doing so, feel free to submit a patch for the developer 
> policy to clarify libclang stability guarantees.
I think @gribozavr2 is correct here, the chapter in the developer policy refers 
to the C API of LLVM, not Clang. The Clang documentation also says that 
libclang should be stable (http://clang.llvm.org/docs/Tooling.html#libclang).


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

https://reviews.llvm.org/D71966



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

[PATCH] D71913: [LTO/WPD] Enable aggressive WPD under LTO option

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/test/CodeGenCXX/lto-visibility-inference.cpp:73
   c1->f();
-  // ITANIUM-NOT: type.test{{.*}}!"_ZTS2C2"
+  // ITANIUM: type.test{{.*}}!"_ZTS2C2"
   // MS: type.test{{.*}}!"?AUC2@@"

tejohnson wrote:
> pcc wrote:
> > tejohnson wrote:
> > > evgeny777 wrote:
> > > > tejohnson wrote:
> > > > > evgeny777 wrote:
> > > > > > tejohnson wrote:
> > > > > > > evgeny777 wrote:
> > > > > > > > What caused this and other changes in this file?
> > > > > > > Because we now will insert type tests for non-hidden vtables. 
> > > > > > > This is enabled by the changes to LTO to interpret these based on 
> > > > > > > the vcall_visibility metadata.
> > > > > > The results of this test case
> > > > > > ```
> > > > > > %clang_cc1 -flto -triple x86_64-pc-windows-msvc -std=c++11 
> > > > > > -fms-extensions -fwhole-program-vtables -flto-visibility-public-std 
> > > > > > -emit-llvm -o - %s | FileCheck --check-prefix=MS 
> > > > > > --check-prefix=MS-NOSTD %s
> > > > > > ```
> > > > > > look not correct to me. I think you shouldn't generate type tests 
> > > > > > for standard library classes with  `-flto-visibility-public-std`. 
> > > > > > Currently if this flag is given, clang doesn't do this either even 
> > > > > > with `-fvisibility=hidden`
> > > > > The associated vtables would get the vcall_visibility public 
> > > > > metadata, so the type tests themselves aren't problematic. I tend to 
> > > > > think that an application using such options should simply stick with 
> > > > > -fvisibility=hidden to get WPD and not use the new LTO option to 
> > > > > convert all public vcall_visibility metadata to hidden.
> > > > > The associated vtables would get the vcall_visibility public 
> > > > > metadata, so the type tests themselves aren't problematic. I tend to 
> > > > > think that an application using such options should simply stick with 
> > > > > -fvisibility=hidden to get WPD and not use the new LTO option to 
> > > > > convert all public vcall_visibility metadata to hidden.
> > > > 
> > > > I see two issues here:
> > > > 1) It's not always good option to force hidden visibility for 
> > > > everything. For instance I work on proprietary platform which demands 
> > > > public visibility for certain symbols in order for dynamic loader to 
> > > > work properly. In this context your patch does a great job.
> > > > 
> > > > 2) Standard library is almost never LTOed so in general we can't narrow 
> > > > std::* vtables visibility to linkage unit
> > > > 
> > > > Is there anything which prevents from implementing the same 
> > > > functionality with new -lto-whole-program-visibility option (i.e 
> > > > without forcing hidden visibility)? In other words the following looks 
> > > > good to me:
> > > > 
> > > > ```
> > > > # Compile with lto/devirtualization support
> > > > clang -flto=thin -flto-visibility-public-std -fwhole-program-vtables -c 
> > > > *.cpp
> > > > 
> > > > # Link: everything is devirtualized except standard library classes 
> > > > virtual methods
> > > > clang -Wl,-lto-whole-program-visibility -fuse-ld=lld *.o
> > > > ```
> > > Ok, thanks for the info. I will go ahead and change the code to not 
> > > insert the type tests in this case to support this usage.
> > > 
> > > Ultimately, I would like to decouple the existence of the type tests from 
> > > visibility implications. I'm working on another change to delay 
> > > lowering/removal of type tests until after indirect call promotion, so we 
> > > can use them in other cases (streamlined indirect call promotion checks 
> > > against the vtable instead of the function pointers, also useful if we 
> > > want to implement speculative devirtualization based on WPD info). In 
> > > those cases we need the type tests, either to locate information in the 
> > > summary, or to get the address point offset for a vtable address compare. 
> > > In that case it would be helpful to have the type tests in this type of 
> > > code as well. So we'll need another way to communicate down to WPD that 
> > > they should never be devirtualized. But I don't think it makes sense to 
> > > design this support until there is a concrete use case and need. In the 
> > > meantime I will change the code to be conservative and not insert the 
> > > type tests in this case.
> > Note that `-flto-visibility-public-std` is a cc1-only option and only used 
> > on Windows, and further that `-lto-whole-program-visibility` as implemented 
> > doesn't really make sense on Windows because the classes with public 
> > visibility are going to be marked dllimport/dllexport/uuid (COM), and 
> > `-lto-whole-program-visibility` corresponds to flags such as 
> > `--version-script` or the absence of `-shared` in which the linker 
> > automatically relaxes the visibility of some symbols, while there is no 
> > such concept of relaxing symbol visibility on Windows.
> > 
> >  I would be inclined to remove this support and 

[PATCH] D75271: [analyzer][NFC] Change LangOptions to CheckerManager in the shouldRegister* functions.

[Balogh, Ádám via Phabricator via cfe-commits]

baloghadamsoftware added a comment.

It is even worse: the `shouldRegister()` functions do not prevent the 
registration of a checker, at least in cases when they are not enabled directly 
but via dependencies. See D75171 .


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

https://reviews.llvm.org/D75271



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

[PATCH] D75215: [DebugInfo] Fix for adding "returns cxx udt" option to functions in CodeView.

[Amy Huang via Phabricator via cfe-commits]

akhuang marked an inline comment as done and an inline comment as not done.
akhuang added inline comments.



Comment at: llvm/include/llvm/IR/DebugInfoFlags.def:61
 HANDLE_DI_FLAG((1 << 29), AllCallsDescribed)
+HANDLE_DI_FLAG((1 << 30), CxxReturnUdt)
 

akhuang wrote:
> rnk wrote:
> > aprantl wrote:
> > > dblaikie wrote:
> > > > rnk wrote:
> > > > > @dblaikie @aprantl, does this seem like a reasonable flag to add, or 
> > > > > should we mark record forward decls as trivial/nontrivial instead?
> > > > Currently we only have a trivial/nontrivial flag that goes one way, 
> > > > right? (ie: true/false, not three state true/false/unknown)
> > > > 
> > > > That would present a problem for forward declarations - because for a 
> > > > true forward decl you can't know if it's trivial/non-trivial for 
> > > > passing, right? so that'd present a subtle difference between 
> > > > trivial/non-trivial on a decl (where it might be trivial/unknown) and 
> > > > on a def (where it's trivial/non-trivial), yes?
> > > Should this perhaps be a DI_SPFLAG instead?
> > It's true that there is an ambiguity between known trivial, known 
> > non-trivial, and forward declared, unknown triviality. Amy's solution to 
> > this problem was to mark forward declarations as nontrivial, which matches 
> > the logic MSVC uses to set CxxReturnUdt, but might not be the right thing 
> > for other consumers.
> Alternatively, I guess we can FlagNonTrivial to FlagTrivial, and then we can 
> mark all the unknown things with cxxreturnudt.
maybe this doesn't work because then it would also mark C structs with 
cxxreturnudt.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75215



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

[clang] ea5b3ef - [OPENMP50]Skip the first element when storing the list of dependencies

[Alexey Bataev via cfe-commits]

Author: Alexey Bataev
Date: 2020-03-05T14:26:07-05:00
New Revision: ea5b3ef5935c1a64b78e06fb4b4dcc919fd585b5

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

LOG: [OPENMP50]Skip the first element when storing the list of dependencies
in depobj object.

The first element in the list of the dependencies is used for internal
purposes to store the number of the elements in the provided list.
The first element now is skipped and depobj object poits exactly to the
list of dependencies.

Added: 


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

Removed: 




diff  --git a/clang/lib/CodeGen/CGOpenMPRuntime.cpp 
b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
index 09749270b830..690634458622 100644
--- a/clang/lib/CodeGen/CGOpenMPRuntime.cpp
+++ b/clang/lib/CodeGen/CGOpenMPRuntime.cpp
@@ -5186,6 +5186,55 @@ CGOpenMPRuntime::emitTaskInit(CodeGenFunction , 
SourceLocation Loc,
   return Result;
 }
 
+namespace {
+/// Dependence kind for RTL.
+enum RTLDependenceKindTy {
+  DepIn = 0x01,
+  DepInOut = 0x3,
+  DepMutexInOutSet = 0x4
+};
+/// Fields ids in kmp_depend_info record.
+enum RTLDependInfoFieldsTy { BaseAddr, Len, Flags };
+} // namespace
+
+/// Translates internal dependency kind into the runtime kind.
+static RTLDependenceKindTy translateDependencyKind(OpenMPDependClauseKind K) {
+  RTLDependenceKindTy DepKind;
+  switch (K) {
+  case OMPC_DEPEND_in:
+DepKind = DepIn;
+break;
+  // Out and InOut dependencies must use the same code.
+  case OMPC_DEPEND_out:
+  case OMPC_DEPEND_inout:
+DepKind = DepInOut;
+break;
+  case OMPC_DEPEND_mutexinoutset:
+DepKind = DepMutexInOutSet;
+break;
+  case OMPC_DEPEND_source:
+  case OMPC_DEPEND_sink:
+  case OMPC_DEPEND_unknown:
+llvm_unreachable("Unknown task dependence type");
+  }
+  return DepKind;
+}
+
+/// Builds kmp_depend_info, if it is not built yet, and builds flags type.
+static void getDependTypes(ASTContext , QualType ,
+   QualType ) {
+  FlagsTy = C.getIntTypeForBitwidth(C.getTypeSize(C.BoolTy), /*Signed=*/false);
+  if (KmpDependInfoTy.isNull()) {
+RecordDecl *KmpDependInfoRD = C.buildImplicitRecord("kmp_depend_info");
+KmpDependInfoRD->startDefinition();
+addFieldToRecordDecl(C, KmpDependInfoRD, C.getIntPtrType());
+addFieldToRecordDecl(C, KmpDependInfoRD, C.getSizeType());
+addFieldToRecordDecl(C, KmpDependInfoRD, FlagsTy);
+KmpDependInfoRD->completeDefinition();
+KmpDependInfoTy = C.getRecordType(KmpDependInfoRD);
+  }
+}
+
 Address CGOpenMPRuntime::emitDependClause(
 CodeGenFunction ,
 ArrayRef> Dependencies,
@@ -5195,28 +5244,11 @@ Address CGOpenMPRuntime::emitDependClause(
   Address DependenciesArray = Address::invalid();
   unsigned NumDependencies = Dependencies.size();
   if (NumDependencies) {
-// Dependence kind for RTL.
-enum RTLDependenceKindTy {
-  DepIn = 0x01,
-  DepInOut = 0x3,
-  DepMutexInOutSet = 0x4
-};
-enum RTLDependInfoFieldsTy { BaseAddr, Len, Flags };
-RecordDecl *KmpDependInfoRD;
-QualType FlagsTy =
-C.getIntTypeForBitwidth(C.getTypeSize(C.BoolTy), /*Signed=*/false);
+QualType FlagsTy;
+getDependTypes(C, KmpDependInfoTy, FlagsTy);
+RecordDecl *KmpDependInfoRD =
+cast(KmpDependInfoTy->getAsTagDecl());
 llvm::Type *LLVMFlagsTy = CGF.ConvertTypeForMem(FlagsTy);
-if (KmpDependInfoTy.isNull()) {
-  KmpDependInfoRD = C.buildImplicitRecord("kmp_depend_info");
-  KmpDependInfoRD->startDefinition();
-  addFieldToRecordDecl(C, KmpDependInfoRD, C.getIntPtrType());
-  addFieldToRecordDecl(C, KmpDependInfoRD, C.getSizeType());
-  addFieldToRecordDecl(C, KmpDependInfoRD, FlagsTy);
-  KmpDependInfoRD->completeDefinition();
-  KmpDependInfoTy = C.getRecordType(KmpDependInfoRD);
-} else {
-  KmpDependInfoRD = cast(KmpDependInfoTy->getAsTagDecl());
-}
 // Define type kmp_depend_info[];
 // For depobj reserve one extra element to store the number of elements.
 // It is required to handle depobj(x) update(in) construct.
@@ -5289,38 +5321,36 @@ Address CGOpenMPRuntime::emitDependClause(
   Base, *std::next(KmpDependInfoRD->field_begin(), Len));
   CGF.EmitStoreOfScalar(Size, LenLVal);
   // deps[i].flags = ;
-  RTLDependenceKindTy DepKind;
-  switch (Dependencies[I].first) {
-  case OMPC_DEPEND_in:
-DepKind = DepIn;
-break;
-  // Out and InOut dependencies must use the same code.
-  case OMPC_DEPEND_out:
-  case OMPC_DEPEND_inout:
-DepKind = DepInOut;
-break;
-  case OMPC_DEPEND_mutexinoutset:
-DepKind = DepMutexInOutSet;
-break;
-  case OMPC_DEPEND_source:
-  case 

[clang] 3e851f4 - [PowerPC] Delete PPCMachObjectWriter and powerpc{,64}-apple-darwin

[Fangrui Song via cfe-commits]

Author: Fangrui Song
Date: 2020-03-05T11:05:26-08:00
New Revision: 3e851f4a688c42315355aae743b403dddeba9860

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

LOG: [PowerPC] Delete PPCMachObjectWriter and powerpc{,64}-apple-darwin

Reviewed By: #powerpc, sfertile

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

Added: 


Modified: 
clang/test/Driver/darwin-header-search-libstdcxx.cpp
clang/test/Tooling/ms-asm-no-target.cpp
llvm/lib/Support/Triple.cpp
llvm/lib/Target/PowerPC/MCTargetDesc/CMakeLists.txt
llvm/lib/Target/PowerPC/MCTargetDesc/PPCAsmBackend.cpp
llvm/unittests/BinaryFormat/MachOTest.cpp
llvm/utils/gn/secondary/llvm/lib/Target/PowerPC/MCTargetDesc/BUILD.gn

Removed: 
clang/test/Driver/darwin-arch-default.c
llvm/lib/Target/PowerPC/MCTargetDesc/PPCMachObjectWriter.cpp



diff  --git a/clang/test/Driver/darwin-arch-default.c 
b/clang/test/Driver/darwin-arch-default.c
deleted file mode 100644
index e7e5e89ed085..
--- a/clang/test/Driver/darwin-arch-default.c
+++ /dev/null
@@ -1,43 +0,0 @@
-// Check that the name of the arch we bind is "ppc" not "powerpc".
-//
-// RUN: %clang -target powerpc-apple-darwin8 -### \
-// RUN:   -ccc-print-phases %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-BIND-PPC < %t %s
-//
-// CHECK-BIND-PPC: bind-arch, "ppc"
-//
-// RUN: %clang -target powerpc64-apple-darwin8 -### \
-// RUN:   -ccc-print-phases %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-BIND-PPC64 < %t %s
-//
-// CHECK-BIND-PPC64: bind-arch, "ppc64"
-
-// Check that the correct arch name is passed to the external assembler
-//
-// RUN: %clang -target powerpc-apple-darwin8 -### \
-// RUN:   -no-integrated-as -c %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-AS-PPC < %t %s
-//
-// CHECK-AS-PPC: {{as(.exe)?"}}
-// CHECK-AS-PPC: "-arch" "ppc"
-//
-// RUN: %clang -target powerpc64-apple-darwin8 -### \
-// RUN:   -no-integrated-as -c %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-AS-PPC64 < %t %s
-//
-// CHECK-AS-PPC64: {{as(.exe)?"}}
-// CHECK-AS-PPC64: "-arch" "ppc64"
-
-// Check that the correct arch name is passed to the external linker
-//
-// RUN: %clang -target powerpc-apple-darwin8 -### %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-LD-PPC < %t %s
-//
-// CHECK-LD-PPC: {{ld(.exe)?"}}
-// CHECK-LD-PPC: "-arch" "ppc"
-//
-// RUN: %clang -target powerpc64-apple-darwin8 -### %s 2> %t
-// RUN: FileCheck --check-prefix=CHECK-LD-PPC64 < %t %s
-//
-// CHECK-LD-PPC64: {{ld(.exe)?"}}
-// CHECK-LD-PPC64: "-arch" "ppc64"

diff  --git a/clang/test/Driver/darwin-header-search-libstdcxx.cpp 
b/clang/test/Driver/darwin-header-search-libstdcxx.cpp
index ce9d2c1b7eba..070aa80b8905 100644
--- a/clang/test/Driver/darwin-header-search-libstdcxx.cpp
+++ b/clang/test/Driver/darwin-header-search-libstdcxx.cpp
@@ -3,34 +3,6 @@
 // General tests that the header search paths for libstdc++ detected by the
 // driver and passed to CC1 are correct on Darwin platforms.
 
-// Check ppc and ppc64
-//
-// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
-// RUN: -target ppc-apple-darwin \
-// RUN: -stdlib=libstdc++ \
-// RUN: -isysroot %S/Inputs/basic_darwin_sdk_libstdcxx_ppc \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_libstdcxx_ppc 
--check-prefix=CHECK-LIBSTDCXX-PPC %s
-// CHECK-LIBSTDCXX-PPC: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
-// CHECK-LIBSTDCXX-PPC: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/4.2.1
-// CHECK-LIBSTDCXX-PPC: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/4.2.1/powerpc-apple-darwin10"
-// CHECK-LIBSTDCXX-PPC: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/4.2.1/backward"
-// CHECK-LIBSTDCXX-PPC: "-internal-isystem" "[[SYSROOT]]/usr/include/c++/4.0.0"
-// CHECK-LIBSTDCXX-PPC: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/4.0.0/powerpc-apple-darwin10"
-// CHECK-LIBSTDCXX-PPC: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/4.0.0/backward"
-//
-// RUN: %clang -no-canonical-prefixes %s -### -fsyntax-only 2>&1 \
-// RUN: -target ppc64-apple-darwin \
-// RUN: -stdlib=libstdc++ \
-// RUN: -isysroot %S/Inputs/basic_darwin_sdk_libstdcxx_ppc \
-// RUN:   | FileCheck -DSYSROOT=%S/Inputs/basic_darwin_sdk_libstdcxx_ppc 
--check-prefix=CHECK-LIBSTDCXX-PPC64 %s
-// CHECK-LIBSTDCXX-PPC64: "{{[^"]*}}clang{{[^"]*}}" "-cc1"
-// CHECK-LIBSTDCXX-PPC64: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/4.2.1"
-// CHECK-LIBSTDCXX-PPC64: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/4.2.1/powerpc-apple-darwin10/ppc64"
-// CHECK-LIBSTDCXX-PPC64: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/4.2.1/backward"
-// CHECK-LIBSTDCXX-PPC64: "-internal-isystem" 
"[[SYSROOT]]/usr/include/c++/4.0.0"
-// CHECK-LIBSTDCXX-PPC64: "-internal-isystem" 

[PATCH] D75632: Comment parsing: Treat \ref as inline command

[Aaron Puchert via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rGf23df1b2a323: Comment parsing: Treat \ref as inline command 
(authored by aaronpuchert).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75632

Files:
  clang/include/clang/AST/CommentCommands.td
  clang/test/Sema/warn-documentation.cpp


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -294,6 +294,9 @@
 /// \retval 0 Blah blah.
 int test_param23(int a);
 
+/// \param a \ref test_param23 has an empty paragraph, this doesn't.
+int test_param24(int a);
+
 //===---
 // Test that we treat typedefs to some non-function types as functions for the
 // purposes of documentation comment parsing.
Index: clang/include/clang/AST/CommentCommands.td
===
--- clang/include/clang/AST/CommentCommands.td
+++ clang/include/clang/AST/CommentCommands.td
@@ -87,6 +87,7 @@
 def A  : InlineCommand<"a">;
 def E  : InlineCommand<"e">;
 def Em : InlineCommand<"em">;
+def Ref: InlineCommand<"ref">;
 def Anchor : InlineCommand<"anchor">;
 
 
//===--===//
@@ -205,7 +206,6 @@
 
 def Mainpage : VerbatimLineCommand<"mainpage">;
 def Subpage  : VerbatimLineCommand<"subpage">;
-def Ref  : VerbatimLineCommand<"ref">;
 
 def Relates : VerbatimLineCommand<"relates">;
 def Related : VerbatimLineCommand<"related">;


Index: clang/test/Sema/warn-documentation.cpp
===
--- clang/test/Sema/warn-documentation.cpp
+++ clang/test/Sema/warn-documentation.cpp
@@ -294,6 +294,9 @@
 /// \retval 0 Blah blah.
 int test_param23(int a);
 
+/// \param a \ref test_param23 has an empty paragraph, this doesn't.
+int test_param24(int a);
+
 //===---
 // Test that we treat typedefs to some non-function types as functions for the
 // purposes of documentation comment parsing.
Index: clang/include/clang/AST/CommentCommands.td
===
--- clang/include/clang/AST/CommentCommands.td
+++ clang/include/clang/AST/CommentCommands.td
@@ -87,6 +87,7 @@
 def A  : InlineCommand<"a">;
 def E  : InlineCommand<"e">;
 def Em : InlineCommand<"em">;
+def Ref: InlineCommand<"ref">;
 def Anchor : InlineCommand<"anchor">;
 
 //===--===//
@@ -205,7 +206,6 @@
 
 def Mainpage : VerbatimLineCommand<"mainpage">;
 def Subpage  : VerbatimLineCommand<"subpage">;
-def Ref  : VerbatimLineCommand<"ref">;
 
 def Relates : VerbatimLineCommand<"relates">;
 def Related : VerbatimLineCommand<"related">;
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D75655: [Docs] Document -lto-whole-program-visibility

[Peter Collingbourne via Phabricator via cfe-commits]

pcc added inline comments.



Comment at: clang/docs/LTOVisibility.rst:40
+to hidden LTO visibility when the ``-lto-whole-program-visibility`` lld linker
+option is applied (``-plugin-opt=whole_program_visibility`` for gold). This
+can be used when it is known that the LTO link has whole program visibility,

Isn't it spelled `whole-program-visibility`?



Comment at: clang/docs/LTOVisibility.rst:45
+build system, and the binary will not dlopen any libraries deriving from the
+binary’s classes. This is useful in situations where it is not safe to specify
+``-fvisibility=hidden`` at compile time.

I thought that the motivation was avoiding duplicate work in the case where you 
need varying LTO visibility? Otherwise you could just build with and without 
`-fvisibility=hidden` and it would be the same from an LTO visibility 
perspective.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75655



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

[PATCH] D75701: Initialize IsSurrogate

[Rafael Avila de Espindola via Phabricator via cfe-commits]

respindola created this revision.
respindola added a reviewer: rsmith.

This fixes https://bugs.llvm.org/show_bug.cgi?id=45096


https://reviews.llvm.org/D75701

Files:
  clang/include/clang/Sema/Overload.h


Index: clang/include/clang/Sema/Overload.h
===
--- clang/include/clang/Sema/Overload.h
+++ clang/include/clang/Sema/Overload.h
@@ -908,7 +908,7 @@
   private:
 friend class OverloadCandidateSet;
 OverloadCandidate()
-: IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
+: IsSurrogate(false), IsADLCandidate(CallExpr::NotADL), 
RewriteKind(CRK_None) {}
   };
 
   /// OverloadCandidateSet - A set of overload candidates, used in C++


Index: clang/include/clang/Sema/Overload.h
===
--- clang/include/clang/Sema/Overload.h
+++ clang/include/clang/Sema/Overload.h
@@ -908,7 +908,7 @@
   private:
 friend class OverloadCandidateSet;
 OverloadCandidate()
-: IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
+: IsSurrogate(false), IsADLCandidate(CallExpr::NotADL), RewriteKind(CRK_None) {}
   };
 
   /// OverloadCandidateSet - A set of overload candidates, used in C++
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D68578: [HIP] Fix device stub name

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl updated this revision to Diff 248544.
yaxunl marked 11 inline comments as done.
yaxunl added a comment.

Revised by John's and Artem's comments.


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

https://reviews.llvm.org/D68578

Files:
  clang/include/clang/AST/GlobalDecl.h
  clang/lib/AST/ItaniumMangle.cpp
  clang/lib/CodeGen/CGCUDANV.cpp
  clang/lib/CodeGen/CGCUDARuntime.h
  clang/lib/CodeGen/CGDecl.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/CodeGen/CodeGenModule.h
  clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
  clang/test/CodeGenCUDA/kernel-stub-name.cu
  clang/test/CodeGenCUDA/unnamed-types.cu

Index: clang/test/CodeGenCUDA/unnamed-types.cu
===
--- clang/test/CodeGenCUDA/unnamed-types.cu
+++ clang/test/CodeGenCUDA/unnamed-types.cu
@@ -36,4 +36,4 @@
   }(p);
 }
 // HOST: @__hip_register_globals
-// HOST: __hipRegisterFunction{{.*}}@_Z2k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_{{.*}}@0
+// HOST: __hipRegisterFunction{{.*}}@_Z17__device_stub__k0IZZ2f1PfENKUlS0_E_clES0_EUlfE_EvS0_T_{{.*}}@0
Index: clang/test/CodeGenCUDA/kernel-stub-name.cu
===
--- clang/test/CodeGenCUDA/kernel-stub-name.cu
+++ clang/test/CodeGenCUDA/kernel-stub-name.cu
@@ -6,15 +6,50 @@
 
 #include "Inputs/cuda.h"
 
+extern "C" __global__ void ckernel() {}
+
+namespace ns {
+__global__ void nskernel() {}
+} // namespace ns
+
 template
 __global__ void kernelfunc() {}
 
+__global__ void kernel_decl();
+
+// Device side kernel names
+
+// CHECK: @[[CKERN:[0-9]*]] = {{.*}} c"ckernel\00"
+// CHECK: @[[NSKERN:[0-9]*]] = {{.*}} c"_ZN2ns8nskernelEv\00"
+// CHECK: @[[TKERN:[0-9]*]] = {{.*}} c"_Z10kernelfuncIiEvv\00"
+
+// Non-template kernel stub functions
+
+// CHECK: define{{.*}}@[[CSTUB:__device_stub__ckernel]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[CSTUB]]
+// CHECK: define{{.*}}@[[NSSTUB:_ZN2ns23__device_stub__nskernelEv]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[NSSTUB]]
+
 // CHECK-LABEL: define{{.*}}@_Z8hostfuncv()
-// CHECK: call void @[[STUB:_Z10kernelfuncIiEvv.stub]]()
-void hostfunc(void) { kernelfunc<<<1, 1>>>(); }
+// CHECK: call void @[[CSTUB]]()
+// CHECK: call void @[[NSSTUB]]()
+// CHECK: call void @[[TSTUB:_Z25__device_stub__kernelfuncIiEvv]]()
+// CHECK: call void @[[DSTUB:_Z26__device_stub__kernel_declv]]()
+void hostfunc(void) {
+  ckernel<<<1, 1>>>();
+  ns::nskernel<<<1, 1>>>();
+  kernelfunc<<<1, 1>>>();
+  kernel_decl<<<1, 1>>>();
+}
+
+// Template kernel stub functions
+
+// CHECK: define{{.*}}@[[TSTUB]]
+// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[TSTUB]]
 
-// CHECK: define{{.*}}@[[STUB]]
-// CHECK: call{{.*}}@hipLaunchByPtr{{.*}}@[[STUB]]
+// CHECK: declare{{.*}}@[[DSTUB]]
 
 // CHECK-LABEL: define{{.*}}@__hip_register_globals
-// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[STUB]]
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[CSTUB]]{{.*}}@[[CKERN]]
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[NSSTUB]]{{.*}}@[[NSKERN]]
+// CHECK: call{{.*}}@__hipRegisterFunction{{.*}}@[[TSTUB]]{{.*}}@[[TKERN]]
Index: clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
===
--- clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
+++ clang/test/CodeGenCUDA/amdgpu-kernel-arg-pointer-type.cu
@@ -13,19 +13,19 @@
 // HOST-NOT: %struct.T.coerce
 
 // CHECK: define amdgpu_kernel void  @_Z7kernel1Pi(i32 addrspace(1)* %x.coerce)
-// HOST: define void @_Z7kernel1Pi.stub(i32* %x)
+// HOST: define void @_Z22__device_stub__kernel1Pi(i32* %x)
 __global__ void kernel1(int *x) {
   x[0]++;
 }
 
 // CHECK: define amdgpu_kernel void  @_Z7kernel2Ri(i32 addrspace(1)* dereferenceable(4) %x.coerce)
-// HOST: define void @_Z7kernel2Ri.stub(i32* dereferenceable(4) %x)
+// HOST: define void @_Z22__device_stub__kernel2Ri(i32* dereferenceable(4) %x)
 __global__ void kernel2(int ) {
   x++;
 }
 
 // CHECK: define amdgpu_kernel void  @_Z7kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
-// HOST: define void @_Z7kernel3PU3AS2iPU3AS1i.stub(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
+// HOST: define void @_Z22__device_stub__kernel3PU3AS2iPU3AS1i(i32 addrspace(2)* %x, i32 addrspace(1)* %y)
 __global__ void kernel3(__attribute__((address_space(2))) int *x,
 __attribute__((address_space(1))) int *y) {
   y[0] = x[0];
@@ -43,7 +43,7 @@
 // `by-val` struct will be coerced into a similar struct with all generic
 // pointers lowerd into global ones.
 // CHECK: define amdgpu_kernel void @_Z7kernel41S(%struct.S.coerce %s.coerce)
-// HOST: define void @_Z7kernel41S.stub(i32* %s.coerce0, float* %s.coerce1)
+// HOST: define void @_Z22__device_stub__kernel41S(i32* %s.coerce0, float* %s.coerce1)
 __global__ void kernel4(struct S s) {
   s.x[0]++;
   s.y[0] += 1.f;
@@ -51,7 +51,7 @@
 
 // If a pointer to struct is passed, only the pointer 

[PATCH] D75665: [analyzer] On-demand parsing capability for CTU

[Gabor Marton via Phabricator via cfe-commits]

martong added inline comments.



Comment at: clang/lib/StaticAnalyzer/Core/CallEvent.cpp:578
+
+  // Optional OnDemandParsingDatabase;
+  // if (Opts.CTUOnDemandParsing)

left here some debugging


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D75665



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

[PATCH] D68578: [HIP] Fix device stub name

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl marked 38 inline comments as done.
yaxunl added inline comments.



Comment at: clang/include/clang/AST/GlobalDecl.h:40
+  Stub = 1,
+};
+

tra wrote:
> rjmccall wrote:
> > tra wrote:
> > > rjmccall wrote:
> > > > tra wrote:
> > > > > rjmccall wrote:
> > > > > > The attribute here is `CUDAGlobalAttr`; should this be named in 
> > > > > > terms of CUDA, or is the CUDA model sufficiently different  from 
> > > > > > HIP that the same implementation concept doesn't apply?
> > > > > I believe the attribute serves the same purpose in both CUDA and HIP 
> > > > > and could be renamed appropriately in a separate patch.
> > > > > 
> > > > > While the changes in this patch are not required for CUDA, CUDA would 
> > > > > benefit from them. We could use a generic GPU prefix and migrate CUDA 
> > > > > to the same model later. A TODO comment about that would be nice. 
> > > > I'd just like consistency.  If they're serving the same purpose, then 
> > > > as someone with no dog in this fight, I would give precedence to CUDA 
> > > > over HIP in names since it's both the older language and was 
> > > > implemented first in Clang (even if only partially, IIUC).  I don't 
> > > > think a generic name works unless we can meaningfully generalize it to 
> > > > all languages with a similar feature, e.g. OpenCL and so on.
> > > Naming, the hardest problem in computer science. :-)
> > > I personally would prefer generalization-with-exclusions over specific 
> > > name which is inconsistently commingles things that's really specific to 
> > > that name and things that are more widely used. Alas, right now CUDA is 
> > > the example of the latter case -- some parts are CUDA-specific and a lot 
> > > are shared with HIP.
> > > 
> > > For the new features we've been sort of sticking with using CUDA/HIP for 
> > > specific parts and GPU for shared functionality, but as things are a lot 
> > > of shared bits are still 'CUDA' and it's hard to tell them apart. As you 
> > > point it out, renaming the incumbent names would be a pain, so here we 
> > > are.
> > > 
> > > I think using `GPUKernelKind` with a comment that it reflects HIP & CUDA 
> > > kernels would be somewhat better choice than `CUDAKernelKind` which would 
> > > be somewhat confusing at this point given that CUDA actually does not use 
> > > it yet. I'm also fine with keeping it `HIPKernelKind` and postpone the 
> > > naming decision until CUDA-related parts are actually implemented.
> > Maybe `KernelReferenceKind`?  It's probably a common concept across all 
> > heterogenous-computing models.
> SGTM.
changed.



Comment at: clang/include/clang/AST/GlobalDecl.h:52
+assert((!D || !isa(D)) && "Use other ctor with ctor 
decls!");
+assert((!D || !isa(D)) && "Use other ctor with dtor 
decls!");
 

rjmccall wrote:
> This function exists primarily to be used as a common initializer for all the 
> constructors that don't require any of the extra fields.  (This file predates 
> LLVM's adoption of C++14, which allows constructors to delegate to other 
> constructors.)  That's why it asserts that it's not used with constructors or 
> destructors.  So, two questions:
> 
> - Is there a reason this function now needs to tolerate null declarations?
> - There's some subtle implicit behavior here, in that references to kernels 
> default to `HIPKernelKind::Kernel`.  Is that reasonable, or should there be a 
> third assertion that this function isn't used with kernel declarations?
By using default constructor of GlobalDecl, null declaration is no longer 
necessary. I have reverted change to assert.

I think it is a good idea to force GlobalDecl of kernels to be instantiated 
with the specific ctor. I have added assert to Init to prevent kernels to be 
instantiated through it.



Comment at: clang/include/clang/AST/GlobalDecl.h:131
 
+  operator bool() const { return getAsOpaquePtr(); }
+

rjmccall wrote:
> `explicit`, please.
fixed



Comment at: clang/include/clang/AST/GlobalDecl.h:162
+   getDecl()) &&
+   !cast(getDecl())->hasAttr() &&
!isa(getDecl()) &&

rjmccall wrote:
> The indentation here seems odd.
fixed



Comment at: clang/lib/AST/ItaniumMangle.cpp:401
 : Context(C), Out(Out_), Structor(getStructor(D)), StructorType(Type),
   SeqID(0), AbiTagsRoot(AbiTags) { }
 

rjmccall wrote:
> Does passing down a `GlobalDecl` everywhere allow us to remove these 
> constructors, i.e. to eliminate the `Structor` and `StructorType` fields?
Removing eliminate the Structor and StructorType will incur significantly more 
changes. Can it be done later? Thanks.



Comment at: clang/lib/AST/ItaniumMangle.cpp:645
+void CXXNameMangler::mangle(GlobalDecl GD) {
+  const NamedDecl *D = dyn_cast(GD.getDecl());
   //  ::= _Z 

rjmccall wrote:
> This can just