[PATCH] D157497: feat: Migrate isArch16Bit

2023-08-22 Thread Alex Langford via Phabricator via cfe-commits
bulbazord requested changes to this revision.
bulbazord added a comment.

In D157497#4592330 , @Pivnoy wrote:

> At the moment, the TargetParser architecture is extensible. This complicates 
> the addition of new architectures, operating systems, and so on.
> In the main discussion, it was proposed to redesign the architecture of this 
> module in order to increase the possibility of adding new architectures, etc.
> The main idea of   the rework was to separate the Triple entity into a 
> data-class, and create a number of interfaces from components that would 
> include Triple, through the implementation of which it would be possible to 
> represent various data bindings of the components. At the moment, Triple is 
> overflowing with various methods that do not fully meet the ideas of OOP.
> Since the TargetParser module is quite large and has many dependencies 
> throughout the llvm-project, it was first of all supposed to remove these 
> methods from Triple, since they would not correspond to OOP ideas.
> This would help to gradually rid Triple of unnecessary dependencies, and 
> gradually change the architecture inside Triple, without breaking code of 
> another LLVM developers.

I'm still not sure how this would make things simpler. I'll be as specific is 
possible in what does not click for me.

There is an inherent complexity in parsing triples. Architectures can have 
variants and subvariants, compilers and other tools do different things for the 
same architecture and OS when there are different vendors, the environment can 
subtly change things like ABI, the list goes on. I don't think you're going to 
be able to wholesale reduce complexity here. The proposal you've laid out here 
is certainly very OOP-like (in some sense of the phrase "OOP") but you present 
your ideas under the assumption that this style of OOP is the ideal to strive 
for. Why is that? Why is this better than what exists today? Is it easier to 
debug? Is it more performant? Is it easier to maintain? I personally do not 
think it will be better than what already exists in any of those ways.

In the original discourse thread, you also made the argument that the potential 
performance overhead and binary size increase should be unnoticeable and that 
with modern machines we do not need to fight for each microsecond. Without any 
numbers for performance or size, this is not an argument I can accept. 
Knowingly adding a performance/size regression to your build tools without an 
appropriate tradeoff does not make sense to do.

If you want to do this, please provide a concrete argument for what benefit 
this brings.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157497

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


[PATCH] D157497: feat: Migrate isArch16Bit

2023-08-14 Thread Alex Langford via Phabricator via cfe-commits
bulbazord added a comment.

In D157497#4586165 , @MaskRay wrote:

> In D157497#4584621 , @Pivnoy wrote:
>
>> This discussion was the main motivation for this change.
>> https://discourse.llvm.org/t/rfc-refactor-triple-related-classes/70410/11
>> As a result, Triple should become a data class, and its dependencies such as 
>> Architecture, Operating System etc. represent in the form of interfaces that 
>> can be implemented for the necessary instances.
>
> FWIW I still don't see advantages by switching to the new 
> `llvm::TripleUtils::isArch32Bit` style. How is it better than the current way?

+1
What advantages does this approach bring? I re-read the RFC in the discourse 
link you posted and I'm still not sure why this approach is desirable. Happy to 
discuss further either on this PR or in the discourse thread.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D157497

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


[PATCH] D146595: [clang] Add clang trampoline_mangled_target attribute

2023-03-24 Thread Alex Langford via Phabricator via cfe-commits
bulbazord added a comment.

I had one question about this:

I think through the comments you've made it quite clear that the intent is for 
this to be used by compiler-generated code (i.e. "tooling"). Given that even 
tools have bugs, what does the debugging/remediation story look like when a 
tool generates a "bad" argument for `trampoline_mangled_target`? My 
understanding is that you can mark any arbitrary function as a trampoline for 
any other arbitrary symbol, even ones that don't exist in your compilation unit 
(so clang or llvm may not be able to verify that your trampoline "target" is 
correct). This may be even challenging for the dsymutil (and other debug info 
"linkers") because it's possible the symbol isn't described in debugging 
information. I suppose then it would fall on LLDB to detect these things? Since 
LLDB is meant to place a breakpoint on the trampoline target, would we emit a 
warning when LLDB has no idea where to place the breakpoint?

Not trying to cause trouble or disagree with the idea, but I would like to 
better understand what we can do to detect a failure.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D146595

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


[PATCH] D134911: [clang][DebugInfo] Respect fmodule-file-home-is-cwd in skeleton CUs for clang modules

2022-10-04 Thread Alex Langford via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rG266ec801fb23: [clang][DebugInfo] Respect 
fmodule-file-home-is-cwd in skeleton CUs for clang… (authored by bulbazord).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D134911

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/Modules/module-file-home-is-cwd.m


Index: clang/test/Modules/module-file-home-is-cwd.m
===
--- clang/test/Modules/module-file-home-is-cwd.m
+++ clang/test/Modules/module-file-home-is-cwd.m
@@ -1,8 +1,22 @@
 // RUN: cd %S
-// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-file-home-is-cwd 
-fmodule-name=libA -emit-module Inputs/normal-module-map/module.map -o 
%t/mod.pcm
+// RUN: %clang_cc1 -x objective-c -fmodules -fno-implicit-modules \
+// RUN: -fmodule-file-home-is-cwd -fmodule-name=libA -emit-module \
+// RUN: -fmodules-embed-all-files %S/Inputs/normal-module-map/module.map \
+// RUN: -o %t/mod.pcm
 // RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
 
 // CHECK:  blob data = 
'Inputs{{/|\\}}normal-module-map{{/|\\}}module.map'
 // CHECK:  blob data = 
'Inputs{{/|\\}}normal-module-map{{/|\\}}a2.h'
 // CHECK:  blob data = 
'Inputs{{/|\\}}normal-module-map{{/|\\}}a1.h'
 // CHECK-NOT: MODULE_DIRECTORY
+
+@import libA;
+
+// RUN: cd %t
+// RUN: %clang_cc1 -x objective-c -fmodules -fno-implicit-modules 
-debug-info-kind=limited \
+// RUN: -debugger-tuning=lldb -dwarf-ext-refs -fmodule-file-home-is-cwd \
+// RUN: -fmodule-map-file=%S/Inputs/normal-module-map/module.map \
+// RUN: -fmodule-file=libA=mod.pcm -emit-llvm -o %t-mod.ll %s
+// RUN: cat %t-mod.ll | FileCheck %s --check-prefix=SKELETON
+
+// SKELETON: !DICompileUnit(language: DW_LANG_ObjC, {{.*}}, 
splitDebugFilename: "mod.pcm"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2768,8 +2768,12 @@
 
 llvm::DIBuilder DIB(CGM.getModule());
 SmallString<0> PCM;
-if (!llvm::sys::path::is_absolute(Mod.getASTFile()))
-  PCM = Mod.getPath();
+if (!llvm::sys::path::is_absolute(Mod.getASTFile())) {
+  if (CGM.getHeaderSearchOpts().ModuleFileHomeIsCwd)
+PCM = getCurrentDirname();
+  else
+PCM = Mod.getPath();
+}
 llvm::sys::path::append(PCM, Mod.getASTFile());
 DIB.createCompileUnit(
 TheCU->getSourceLanguage(),


Index: clang/test/Modules/module-file-home-is-cwd.m
===
--- clang/test/Modules/module-file-home-is-cwd.m
+++ clang/test/Modules/module-file-home-is-cwd.m
@@ -1,8 +1,22 @@
 // RUN: cd %S
-// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-file-home-is-cwd -fmodule-name=libA -emit-module Inputs/normal-module-map/module.map -o %t/mod.pcm
+// RUN: %clang_cc1 -x objective-c -fmodules -fno-implicit-modules \
+// RUN: -fmodule-file-home-is-cwd -fmodule-name=libA -emit-module \
+// RUN: -fmodules-embed-all-files %S/Inputs/normal-module-map/module.map \
+// RUN: -o %t/mod.pcm
 // RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
 
 // CHECK:  blob data = 'Inputs{{/|\\}}normal-module-map{{/|\\}}module.map'
 // CHECK:  blob data = 'Inputs{{/|\\}}normal-module-map{{/|\\}}a2.h'
 // CHECK:  blob data = 'Inputs{{/|\\}}normal-module-map{{/|\\}}a1.h'
 // CHECK-NOT: MODULE_DIRECTORY
+
+@import libA;
+
+// RUN: cd %t
+// RUN: %clang_cc1 -x objective-c -fmodules -fno-implicit-modules -debug-info-kind=limited \
+// RUN: -debugger-tuning=lldb -dwarf-ext-refs -fmodule-file-home-is-cwd \
+// RUN: -fmodule-map-file=%S/Inputs/normal-module-map/module.map \
+// RUN: -fmodule-file=libA=mod.pcm -emit-llvm -o %t-mod.ll %s
+// RUN: cat %t-mod.ll | FileCheck %s --check-prefix=SKELETON
+
+// SKELETON: !DICompileUnit(language: DW_LANG_ObjC, {{.*}}, splitDebugFilename: "mod.pcm"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2768,8 +2768,12 @@
 
 llvm::DIBuilder DIB(CGM.getModule());
 SmallString<0> PCM;
-if (!llvm::sys::path::is_absolute(Mod.getASTFile()))
-  PCM = Mod.getPath();
+if (!llvm::sys::path::is_absolute(Mod.getASTFile())) {
+  if (CGM.getHeaderSearchOpts().ModuleFileHomeIsCwd)
+PCM = getCurrentDirname();
+  else
+PCM = Mod.getPath();
+}
 llvm::sys::path::append(PCM, Mod.getASTFile());
 DIB.createCompileUnit(
 TheCU->getSourceLanguage(),
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D134911: [clang][DebugInfo] Respect fmodule-file-home-is-cwd in skeleton CUs for clang modules

2022-09-29 Thread Alex Langford via Phabricator via cfe-commits
bulbazord created this revision.
bulbazord added reviewers: rmaz, aprantl.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

When `-fmodule-file-home-is-cwd` and the path to the PCM is relative, we 
shouldn't assume that the path to the PCM is relative to the modulemap that 
produced it. To respect the option `-fmodule-file-home-is-cwd`, we should 
assume the path is relative to the current working directory.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D134911

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/Modules/module-file-home-is-cwd.m


Index: clang/test/Modules/module-file-home-is-cwd.m
===
--- clang/test/Modules/module-file-home-is-cwd.m
+++ clang/test/Modules/module-file-home-is-cwd.m
@@ -1,8 +1,22 @@
 // RUN: cd %S
-// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-file-home-is-cwd 
-fmodule-name=libA -emit-module Inputs/normal-module-map/module.map -o 
%t/mod.pcm
+// RUN: %clang_cc1 -x objective-c -fmodules -fno-implicit-modules \
+// RUN: -fmodule-file-home-is-cwd -fmodule-name=libA -emit-module \
+// RUN: -fmodules-embed-all-files %S/Inputs/normal-module-map/module.map \
+// RUN: -o %t/mod.pcm
 // RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
 
 // CHECK:  blob data = 
'Inputs{{/|\\}}normal-module-map{{/|\\}}module.map'
 // CHECK:  blob data = 
'Inputs{{/|\\}}normal-module-map{{/|\\}}a2.h'
 // CHECK:  blob data = 
'Inputs{{/|\\}}normal-module-map{{/|\\}}a1.h'
 // CHECK-NOT: MODULE_DIRECTORY
+
+@import libA;
+
+// RUN: cd %t
+// RUN: %clang_cc1 -x objective-c -fmodules -fno-implicit-modules 
-debug-info-kind=limited \
+// RUN: -debugger-tuning=lldb -dwarf-ext-refs -fmodule-file-home-is-cwd \
+// RUN: -fmodule-map-file=%S/Inputs/normal-module-map/module.map \
+// RUN: -fmodule-file=libA=mod.pcm -emit-llvm -o %t-mod.ll %s
+// RUN: cat %t-mod.ll | FileCheck %s --check-prefix=SKELETON
+
+// SKELETON: !DICompileUnit(language: DW_LANG_ObjC, {{.*}}, 
splitDebugFilename: "mod.pcm"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2768,8 +2768,12 @@
 
 llvm::DIBuilder DIB(CGM.getModule());
 SmallString<0> PCM;
-if (!llvm::sys::path::is_absolute(Mod.getASTFile()))
-  PCM = Mod.getPath();
+if (!llvm::sys::path::is_absolute(Mod.getASTFile())) {
+  if (CGM.getHeaderSearchOpts().ModuleFileHomeIsCwd)
+PCM = getCurrentDirname();
+  else
+PCM = Mod.getPath();
+}
 llvm::sys::path::append(PCM, Mod.getASTFile());
 DIB.createCompileUnit(
 TheCU->getSourceLanguage(),


Index: clang/test/Modules/module-file-home-is-cwd.m
===
--- clang/test/Modules/module-file-home-is-cwd.m
+++ clang/test/Modules/module-file-home-is-cwd.m
@@ -1,8 +1,22 @@
 // RUN: cd %S
-// RUN: %clang_cc1 -fmodules -fno-implicit-modules -fmodule-file-home-is-cwd -fmodule-name=libA -emit-module Inputs/normal-module-map/module.map -o %t/mod.pcm
+// RUN: %clang_cc1 -x objective-c -fmodules -fno-implicit-modules \
+// RUN: -fmodule-file-home-is-cwd -fmodule-name=libA -emit-module \
+// RUN: -fmodules-embed-all-files %S/Inputs/normal-module-map/module.map \
+// RUN: -o %t/mod.pcm
 // RUN: llvm-bcanalyzer --dump --disable-histogram %t/mod.pcm | FileCheck %s
 
 // CHECK:  blob data = 'Inputs{{/|\\}}normal-module-map{{/|\\}}module.map'
 // CHECK:  blob data = 'Inputs{{/|\\}}normal-module-map{{/|\\}}a2.h'
 // CHECK:  blob data = 'Inputs{{/|\\}}normal-module-map{{/|\\}}a1.h'
 // CHECK-NOT: MODULE_DIRECTORY
+
+@import libA;
+
+// RUN: cd %t
+// RUN: %clang_cc1 -x objective-c -fmodules -fno-implicit-modules -debug-info-kind=limited \
+// RUN: -debugger-tuning=lldb -dwarf-ext-refs -fmodule-file-home-is-cwd \
+// RUN: -fmodule-map-file=%S/Inputs/normal-module-map/module.map \
+// RUN: -fmodule-file=libA=mod.pcm -emit-llvm -o %t-mod.ll %s
+// RUN: cat %t-mod.ll | FileCheck %s --check-prefix=SKELETON
+
+// SKELETON: !DICompileUnit(language: DW_LANG_ObjC, {{.*}}, splitDebugFilename: "mod.pcm"
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -2768,8 +2768,12 @@
 
 llvm::DIBuilder DIB(CGM.getModule());
 SmallString<0> PCM;
-if (!llvm::sys::path::is_absolute(Mod.getASTFile()))
-  PCM = Mod.getPath();
+if (!llvm::sys::path::is_absolute(Mod.getASTFile())) {
+  if (CGM.getHeaderSearchOpts().ModuleFileHomeIsCwd)
+PCM = getCurrentDirname();
+  else
+PCM = Mod.getPath();
+}
 llvm::sys::path::append(PCM, Mod.getASTFile());
 DIB.createCompileUnit(
 

[PATCH] D122691: [clang][Sema] Add flag to LookupName to force C/ObjC codepath

2022-04-19 Thread Alex Langford via Phabricator via cfe-commits
bulbazord closed this revision.
bulbazord added a comment.

Gah, forgot to add the phabricator link to the commit message. I added the 
commit object to this diff, hopefully people can track it down the review if 
needed.


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

https://reviews.llvm.org/D122691

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


[PATCH] D122691: [clang][Sema] Add flag to LookupName to force C/ObjC codepath

2022-04-18 Thread Alex Langford via Phabricator via cfe-commits
bulbazord updated this revision to Diff 423437.
bulbazord edited the summary of this revision.
bulbazord added a comment.
Herald added a subscriber: mgorny.

Added a test


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

https://reviews.llvm.org/D122691

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaLookup.cpp
  clang/unittests/Sema/CMakeLists.txt
  clang/unittests/Sema/SemaLookupTest.cpp

Index: clang/unittests/Sema/SemaLookupTest.cpp
===
--- /dev/null
+++ clang/unittests/Sema/SemaLookupTest.cpp
@@ -0,0 +1,60 @@
+#include "clang/AST/DeclarationName.h"
+#include "clang/Frontend/CompilerInstance.h"
+#include "clang/Frontend/FrontendAction.h"
+#include "clang/Parse/ParseAST.h"
+#include "clang/Sema/Lookup.h"
+#include "clang/Sema/Sema.h"
+#include "clang/Tooling/Tooling.h"
+#include "gtest/gtest.h"
+
+using namespace llvm;
+using namespace clang;
+using namespace clang::tooling;
+
+namespace {
+
+class LookupAction : public ASTFrontendAction {
+  std::unique_ptr
+  CreateASTConsumer(CompilerInstance , StringRef /*Unused*/) override {
+return std::make_unique();
+  }
+
+  void ExecuteAction() override {
+CompilerInstance  = getCompilerInstance();
+ASSERT_FALSE(CI.hasSema());
+CI.createSema(getTranslationUnitKind(), nullptr);
+ASSERT_TRUE(CI.hasSema());
+Sema  = CI.getSema();
+ParseAST(S);
+
+ASTContext  = S.getASTContext();
+auto Name = ("Foo");
+LookupResult R_cpp(S, Name, SourceLocation(), Sema::LookupOrdinaryName);
+S.LookupName(R_cpp, S.TUScope, /*AllowBuiltinCreation=*/false,
+ /*ForceNoCPlusPlus=*/false);
+// By this point, parsing is done and S.TUScope is nullptr
+// CppLookupName will perform an early return with no results if the Scope
+// we pass in is nullptr. We expect to find nothing.
+ASSERT_TRUE(R_cpp.empty());
+
+// On the other hand, the non-C++ path doesn't care if the Scope passed in
+// is nullptr. We'll force the non-C++ path with a flag.
+LookupResult R_nocpp(S, Name, SourceLocation(), Sema::LookupOrdinaryName);
+S.LookupName(R_nocpp, S.TUScope, /*AllowBuiltinCreation=*/false,
+ /*ForceNoCPlusPlus=*/true);
+ASSERT_TRUE(!R_nocpp.empty());
+  }
+};
+
+TEST(SemaLookupTest, ForceNoCPlusPlusPath) {
+  const char *file_contents = R"objcxx(
+@protocol Foo
+@end
+@interface Foo 
+@end
+  )objcxx";
+  ASSERT_TRUE(runToolOnCodeWithArgs(std::make_unique(),
+file_contents, {"-x", "objective-c++"},
+"test.mm"));
+}
+} // namespace
Index: clang/unittests/Sema/CMakeLists.txt
===
--- clang/unittests/Sema/CMakeLists.txt
+++ clang/unittests/Sema/CMakeLists.txt
@@ -7,6 +7,7 @@
   ExternalSemaSourceTest.cpp
   CodeCompleteTest.cpp
   GslOwnerPointerInference.cpp
+  SemaLookupTest.cpp
   )
 
 clang_target_link_libraries(SemaTests
Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1931,13 +1931,14 @@
 /// used to diagnose ambiguities.
 ///
 /// @returns \c true if lookup succeeded and false otherwise.
-bool Sema::LookupName(LookupResult , Scope *S, bool AllowBuiltinCreation) {
+bool Sema::LookupName(LookupResult , Scope *S, bool AllowBuiltinCreation,
+  bool ForceNoCPlusPlus) {
   DeclarationName Name = R.getLookupName();
   if (!Name) return false;
 
   LookupNameKind NameKind = R.getLookupKind();
 
-  if (!getLangOpts().CPlusPlus) {
+  if (!getLangOpts().CPlusPlus || ForceNoCPlusPlus) {
 // Unqualified name lookup in C/Objective-C is purely lexical, so
 // search in the declarations attached to the name.
 if (NameKind == Sema::LookupRedeclarationWithLinkage) {
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -4252,8 +4252,8 @@
 = NotForRedeclaration);
   bool LookupBuiltin(LookupResult );
   void LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID);
-  bool LookupName(LookupResult , Scope *S,
-  bool AllowBuiltinCreation = false);
+  bool LookupName(LookupResult , Scope *S, bool AllowBuiltinCreation = false,
+  bool ForceNoCPlusPlus = false);
   bool LookupQualifiedName(LookupResult , DeclContext *LookupCtx,
bool InUnqualifiedLookup = false);
   bool LookupQualifiedName(LookupResult , DeclContext *LookupCtx,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D122691: [clang][Sema] Add flag to LookupName to force C/ObjC codepath

2022-03-29 Thread Alex Langford via Phabricator via cfe-commits
bulbazord created this revision.
bulbazord added a reviewer: arphaman.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Motivation: The intent here is for use in Swift.
When building a clang module for swift consumption, swift adds an
extension block to the module for name lookup purposes. Swift calls
this a SwiftLookupTable. One purpose that this serves is to handle
conflicting names between ObjC classes and ObjC protocols. They exist in
different namespaces in ObjC programs, but in Swift they would exist in
the same namespace. Swift handles this by appending a suffix to a
protocol name if it shares a name with a class. For example, if you have
an ObjC class named "Foo" and a protocol with the same name, the
protocol would be renamed to "FooProtocol" when imported into swift.

When constructing the previously mentioned SwiftLookupTable, we use
`Sema::LookupName` to look up name conflicts for the previous problem.
By this time, the Parser has long finished its job so the call to
`LookupName` gets `nullptr` for its Scope (`TUScope` will be `nullptr`
by this point). The C/ObjC path does not have this problem because it
only uses the Scope in specific scenarios. The C++ codepath uses the
Scope quite extensively and will fail early on if the Scope it gets is
null. In our very specific case of looking up ObjC classes with a
specific name, we want to force `sema::LookupName` to take the C/ObjC
codepath even if C++ or ObjC++ is enabled.

I'm not sure how to test this, unfortunately. That being said, I have an
accompanying downstream patch to swift that exercises this codepath.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D122691

Files:
  clang/include/clang/Sema/Sema.h
  clang/lib/Sema/SemaLookup.cpp


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1931,13 +1931,14 @@
 /// used to diagnose ambiguities.
 ///
 /// @returns \c true if lookup succeeded and false otherwise.
-bool Sema::LookupName(LookupResult , Scope *S, bool AllowBuiltinCreation) {
+bool Sema::LookupName(LookupResult , Scope *S, bool AllowBuiltinCreation,
+  bool ForceNoCPlusPlus) {
   DeclarationName Name = R.getLookupName();
   if (!Name) return false;
 
   LookupNameKind NameKind = R.getLookupKind();
 
-  if (!getLangOpts().CPlusPlus) {
+  if (!getLangOpts().CPlusPlus || ForceNoCPlusPlus) {
 // Unqualified name lookup in C/Objective-C is purely lexical, so
 // search in the declarations attached to the name.
 if (NameKind == Sema::LookupRedeclarationWithLinkage) {
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -4217,8 +4217,8 @@
 = NotForRedeclaration);
   bool LookupBuiltin(LookupResult );
   void LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID);
-  bool LookupName(LookupResult , Scope *S,
-  bool AllowBuiltinCreation = false);
+  bool LookupName(LookupResult , Scope *S, bool AllowBuiltinCreation = false,
+  bool ForceNoCPlusPlus = false);
   bool LookupQualifiedName(LookupResult , DeclContext *LookupCtx,
bool InUnqualifiedLookup = false);
   bool LookupQualifiedName(LookupResult , DeclContext *LookupCtx,


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1931,13 +1931,14 @@
 /// used to diagnose ambiguities.
 ///
 /// @returns \c true if lookup succeeded and false otherwise.
-bool Sema::LookupName(LookupResult , Scope *S, bool AllowBuiltinCreation) {
+bool Sema::LookupName(LookupResult , Scope *S, bool AllowBuiltinCreation,
+  bool ForceNoCPlusPlus) {
   DeclarationName Name = R.getLookupName();
   if (!Name) return false;
 
   LookupNameKind NameKind = R.getLookupKind();
 
-  if (!getLangOpts().CPlusPlus) {
+  if (!getLangOpts().CPlusPlus || ForceNoCPlusPlus) {
 // Unqualified name lookup in C/Objective-C is purely lexical, so
 // search in the declarations attached to the name.
 if (NameKind == Sema::LookupRedeclarationWithLinkage) {
Index: clang/include/clang/Sema/Sema.h
===
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -4217,8 +4217,8 @@
 = NotForRedeclaration);
   bool LookupBuiltin(LookupResult );
   void LookupNecessaryTypesForBuiltin(Scope *S, unsigned ID);
-  bool LookupName(LookupResult , Scope *S,
-  bool AllowBuiltinCreation = false);
+  bool LookupName(LookupResult , Scope *S, bool AllowBuiltinCreation = false,
+  

[PATCH] D121748: [clang][Sema] Better support for ObjC++ in Sema::LookupName

2022-03-21 Thread Alex Langford via Phabricator via cfe-commits
bulbazord added a comment.

In D121748#3397715 , @arphaman wrote:

> I don't think this patch is sound. I found this problem with this change when 
> the following file is compiled in Objective-C++ mode:
>
>   template 
>   T umax(T a, T b) {
> return a;
>   }
>
> produces this error:
>
>   test.mm:3:3: error: declaration of 'umax' shadows template parameter
>   T umax(T a, T b) {
> ^
>   test.mm:2:25: note: template parameter is declared here
>   template 
>   ^
>
> which is unexpected and isn't produced in C++ mode.

Could you tell me what flags you gave to clang to compile the test? I'd like to 
test a few things.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121748

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


[PATCH] D121748: [clang][Sema] Better support for ObjC++ in Sema::LookupName

2022-03-16 Thread Alex Langford via Phabricator via cfe-commits
bulbazord added a comment.

In D121748#3384357 , @arphaman wrote:

> Is there a Swift-based test case you have that demonstrates the original 
> problem from Swift's clang importer side? I'm curious to see where it 
> manifests in Swift.



  % cat Test.h
  @protocol Foo
  @end
  @interface Foo 
  @end
  
  % cat test.swift
  import Test
  
  public protocol Thing: FooProtocol {
  }
  
  % cat module.modulemap
  module Test [extern_c] {
header "Test.h"
  }
  
  % $SWIFT_BUILD_DIR/bin/swiftc -frontend -c -enable-cxx-interop 
-enable-objc-interop -sdk 
/Applications/Xcode.app/Contents/Developer/Platforms/MacOSX.platform/Developer/SDKs/MacOSX.sdk/
 -I. -o /tmp/foo.o test.swift
  test.swift:3:24: error: cannot find type 'FooProtocol' in scope
  public protocol Thing: FooProtocol {
 ^~~

There's a call to `Sema::LookupName` that fails for the reason I outlined in 
the description when building the `SwiftLookupTable` during PCM creation time. 
The renaming of `@protocol Foo` to `protocol FooProtocol` doesn't occur because 
it thinks there is no naming conflict.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121748

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


[PATCH] D121748: [clang][Sema] Better support for ObjC++ in Sema::LookupName

2022-03-15 Thread Alex Langford via Phabricator via cfe-commits
bulbazord added a comment.

I'm not quite sure the best way to test this. Looks like `Sema::LookupName` 
isn't currently directly tested. I've run the test suite and did not find any 
new failing tests.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121748

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


[PATCH] D121748: [clang][Sema] Better support for ObjC++ in Sema::LookupName

2022-03-15 Thread Alex Langford via Phabricator via cfe-commits
bulbazord created this revision.
bulbazord added a reviewer: arphaman.
Herald added a project: All.
bulbazord requested review of this revision.
Herald added a project: clang.

`Sema::LookupName` essentially has two possible code paths based on what
language is currently being used. The predicate selecting the code path
is whether or not C++ is being used. In this scenario, the `Scope`
argument that is being passed needs to be not `nullptr` in order to
succeed. In the C/ObjC case, the `Scope` argument can potentially be
`nullptr` and still succeed. In the case where the `Scope` argument is
`nullptr` and you are dealing with Objective-C++ while looking up an
Objective-C name, you will fail where you otherwise should succeed.

This was surfaced when working on Swift/C++ interop. Swift passes
`nullptr` for the `Scope` when looking up some Objective-C name with the
experimental C++ interop option enabled resulting in a failure.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D121748

Files:
  clang/lib/Sema/SemaLookup.cpp


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1937,7 +1937,10 @@
 
   LookupNameKind NameKind = R.getLookupKind();
 
-  if (!getLangOpts().CPlusPlus) {
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC) {
+// In the case of Objective-C++, try C++ unqualified name lookup first.
+if (getLangOpts().CPlusPlus && CppLookupName(R, S))
+  return true;
 // Unqualified name lookup in C/Objective-C is purely lexical, so
 // search in the declarations attached to the name.
 if (NameKind == Sema::LookupRedeclarationWithLinkage) {


Index: clang/lib/Sema/SemaLookup.cpp
===
--- clang/lib/Sema/SemaLookup.cpp
+++ clang/lib/Sema/SemaLookup.cpp
@@ -1937,7 +1937,10 @@
 
   LookupNameKind NameKind = R.getLookupKind();
 
-  if (!getLangOpts().CPlusPlus) {
+  if (!getLangOpts().CPlusPlus || getLangOpts().ObjC) {
+// In the case of Objective-C++, try C++ unqualified name lookup first.
+if (getLangOpts().CPlusPlus && CppLookupName(R, S))
+  return true;
 // Unqualified name lookup in C/Objective-C is purely lexical, so
 // search in the declarations attached to the name.
 if (NameKind == Sema::LookupRedeclarationWithLinkage) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71585: [perf-training] Change profile file pattern string to use %4m instead of %p

2019-12-17 Thread Alex Langford via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rGb3f789e037cb: [perf-training] Change profile file pattern 
string to use %4m instead of %p (authored by xinxinw1, committed by xiaobai).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71585

Files:
  clang/utils/perf-training/lit.cfg


Index: clang/utils/perf-training/lit.cfg
===
--- clang/utils/perf-training/lit.cfg
+++ clang/utils/perf-training/lit.cfg
@@ -37,5 +37,5 @@
 config.substitutions.append( ('%clang', ' %s %s ' % (config.clang, 
sysroot_flags) ) )
 config.substitutions.append( ('%test_root', config.test_exec_root ) )
 
-config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%p.profraw'
+config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%4m.profraw'
 


Index: clang/utils/perf-training/lit.cfg
===
--- clang/utils/perf-training/lit.cfg
+++ clang/utils/perf-training/lit.cfg
@@ -37,5 +37,5 @@
 config.substitutions.append( ('%clang', ' %s %s ' % (config.clang, sysroot_flags) ) )
 config.substitutions.append( ('%test_root', config.test_exec_root ) )
 
-config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%p.profraw'
+config.environment['LLVM_PROFILE_FILE'] = 'perf-training-%4m.profraw'
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D71585: [perf-training] Change profile file pattern string to use %4m instead of %p

2019-12-17 Thread Alex Langford via Phabricator via cfe-commits
xiaobai added a reviewer: vsk.
xiaobai added a subscriber: vsk.
xiaobai added a comment.

Adding @vsk since he added the code that you're referencing in your summary.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D71585



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


[PATCH] D66265: [NFCI] Always initialize BugReport const fields

2019-08-14 Thread Alex Langford via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL368950: [NFCI] Always initialize BugReport const fields 
(authored by xiaobai, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D66265?vs=215292=215299#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66265

Files:
  cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h


Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -180,10 +180,12 @@
   /// to the user. This method allows to rest the location which should be used
   /// for uniquing reports. For example, memory leaks checker, could set this 
to
   /// the allocation site, rather then the location where the bug is reported.
-  BugReport(BugType& bt, StringRef desc, const ExplodedNode *errornode,
+  BugReport(BugType , StringRef desc, const ExplodedNode *errornode,
 PathDiagnosticLocation LocationToUnique, const Decl *DeclToUnique)
   : BT(bt), Description(desc), UniqueingLocation(LocationToUnique),
-UniqueingDecl(DeclToUnique), ErrorNode(errornode) {}
+UniqueingDecl(DeclToUnique), ErrorNode(errornode),
+ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+ : SourceRange()) {}
 
   virtual ~BugReport() = default;
 


Index: cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===
--- cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ cfe/trunk/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -180,10 +180,12 @@
   /// to the user. This method allows to rest the location which should be used
   /// for uniquing reports. For example, memory leaks checker, could set this to
   /// the allocation site, rather then the location where the bug is reported.
-  BugReport(BugType& bt, StringRef desc, const ExplodedNode *errornode,
+  BugReport(BugType , StringRef desc, const ExplodedNode *errornode,
 PathDiagnosticLocation LocationToUnique, const Decl *DeclToUnique)
   : BT(bt), Description(desc), UniqueingLocation(LocationToUnique),
-UniqueingDecl(DeclToUnique), ErrorNode(errornode) {}
+UniqueingDecl(DeclToUnique), ErrorNode(errornode),
+ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+ : SourceRange()) {}
 
   virtual ~BugReport() = default;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66265: [NFCI] Always initialize BugReport const fields

2019-08-14 Thread Alex Langford via Phabricator via cfe-commits
xiaobai created this revision.
xiaobai added reviewers: compnerd, Szelethus, NoQ.
Herald added a project: clang.

Some compilers require that const fields of an object must be explicitly
initialized by the constructor. I ran into this issue building with clang
3.8 on Ubuntu 16.04.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66265

Files:
  clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h


Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -180,10 +180,12 @@
   /// to the user. This method allows to rest the location which should be used
   /// for uniquing reports. For example, memory leaks checker, could set this 
to
   /// the allocation site, rather then the location where the bug is reported.
-  BugReport(BugType& bt, StringRef desc, const ExplodedNode *errornode,
+  BugReport(BugType , StringRef desc, const ExplodedNode *errornode,
 PathDiagnosticLocation LocationToUnique, const Decl *DeclToUnique)
   : BT(bt), Description(desc), UniqueingLocation(LocationToUnique),
-UniqueingDecl(DeclToUnique), ErrorNode(errornode) {}
+UniqueingDecl(DeclToUnique), ErrorNode(errornode),
+ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+ : SourceRange()) {}
 
   virtual ~BugReport() = default;
 


Index: clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
===
--- clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
+++ clang/include/clang/StaticAnalyzer/Core/BugReporter/BugReporter.h
@@ -180,10 +180,12 @@
   /// to the user. This method allows to rest the location which should be used
   /// for uniquing reports. For example, memory leaks checker, could set this to
   /// the allocation site, rather then the location where the bug is reported.
-  BugReport(BugType& bt, StringRef desc, const ExplodedNode *errornode,
+  BugReport(BugType , StringRef desc, const ExplodedNode *errornode,
 PathDiagnosticLocation LocationToUnique, const Decl *DeclToUnique)
   : BT(bt), Description(desc), UniqueingLocation(LocationToUnique),
-UniqueingDecl(DeclToUnique), ErrorNode(errornode) {}
+UniqueingDecl(DeclToUnique), ErrorNode(errornode),
+ErrorNodeRange(getStmt() ? getStmt()->getSourceRange()
+ : SourceRange()) {}
 
   virtual ~BugReport() = default;
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66192: [analyzer] Don't delete TaintConfig copy constructor

2019-08-13 Thread Alex Langford via Phabricator via cfe-commits
This revision was automatically updated to reflect the committed changes.
Closed by commit rL368779: [analyzer] Dont delete TaintConfig copy 
constructor (authored by xiaobai, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D66192?vs=214998=214999#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D66192

Files:
  cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -71,9 +71,9 @@
 std::vector Sinks;
 
 TaintConfiguration() = default;
-TaintConfiguration(const TaintConfiguration &) = delete;
+TaintConfiguration(const TaintConfiguration &) = default;
 TaintConfiguration(TaintConfiguration &&) = default;
-TaintConfiguration =(const TaintConfiguration &) = delete;
+TaintConfiguration =(const TaintConfiguration &) = default;
 TaintConfiguration =(TaintConfiguration &&) = default;
   };
 


Index: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -71,9 +71,9 @@
 std::vector Sinks;
 
 TaintConfiguration() = default;
-TaintConfiguration(const TaintConfiguration &) = delete;
+TaintConfiguration(const TaintConfiguration &) = default;
 TaintConfiguration(TaintConfiguration &&) = default;
-TaintConfiguration =(const TaintConfiguration &) = delete;
+TaintConfiguration =(const TaintConfiguration &) = default;
 TaintConfiguration =(TaintConfiguration &&) = default;
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D66192: [analyzer] Don't delete TaintConfig copy constructor

2019-08-13 Thread Alex Langford via Phabricator via cfe-commits
xiaobai created this revision.
xiaobai added reviewers: compnerd, Szelethus, boga95, NoQ, alexshap.
Herald added subscribers: Charusso, dkrupp, donat.nagy, mikhail.ramalho, 
a.sidorin, szepet, baloghadamsoftware, xazax.hun.
Herald added a project: clang.

Explicitly deleting the copy constructor makes compiling the function 
`ento::registerGenericTaintChecker` difficult with some compilers. When we
construct an `llvm::Optional`, the optional is constructed with a
const TaintConfig reference which it then uses to invoke the deleted TaintConfig
copy constructor.

I've observered this failing with clang 3.8 on Ubuntu 16.04.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D66192

Files:
  clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp


Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -71,9 +71,9 @@
 std::vector Sinks;
 
 TaintConfiguration() = default;
-TaintConfiguration(const TaintConfiguration &) = delete;
+TaintConfiguration(const TaintConfiguration &) = default;
 TaintConfiguration(TaintConfiguration &&) = default;
-TaintConfiguration =(const TaintConfiguration &) = delete;
+TaintConfiguration =(const TaintConfiguration &) = default;
 TaintConfiguration =(TaintConfiguration &&) = default;
   };
 


Index: clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ clang/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -71,9 +71,9 @@
 std::vector Sinks;
 
 TaintConfiguration() = default;
-TaintConfiguration(const TaintConfiguration &) = delete;
+TaintConfiguration(const TaintConfiguration &) = default;
 TaintConfiguration(TaintConfiguration &&) = default;
-TaintConfiguration =(const TaintConfiguration &) = delete;
+TaintConfiguration =(const TaintConfiguration &) = default;
 TaintConfiguration =(TaintConfiguration &&) = default;
   };
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits


[PATCH] D59555: [analyzer] Add yaml parser to GenericTaintChecker

2019-08-13 Thread Alex Langford via Phabricator via cfe-commits
xiaobai added inline comments.



Comment at: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp:74
+TaintConfiguration() = default;
+TaintConfiguration(const TaintConfiguration &) = delete;
+TaintConfiguration(TaintConfiguration &&) = default;

Is there a particular reason this is marked as delete? This present an issue 
because the Optional constructed in `ento::registerGenericTaintChecker` will 
use this constructor. I don't hit this issue with any recent version of clang 
(tried with 6, 7, and 8) but I am hitting it with clang 3.8 on the swift-ci 
ubuntu bots.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D59555



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