[PATCH] D39050: Add index-while-building support to Clang

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr added inline comments.
Herald added a subscriber: jdoerfert.



Comment at: include/clang/Frontend/FrontendOptions.h:377
 
+  /// The path to write index data to
+  std::string IndexStorePath;

Please end comments with a period.



Comment at: include/clang/Frontend/FrontendOptions.h:380
+  /// Whether to ignore system files when writing out index data
+  unsigned IndexIgnoreSystemSymbols : 1;
+  /// Whether to include the codegen name of symbols in the index data

Would it make more sense to flip this boolean to positive?  
"IndexIncludeSystemSymbols"?



Comment at: lib/Index/IndexingAction.cpp:102
+  /// \param CI The compiler instance used to process the input
+  /// \returns the created IndexASTConsumer.
+  virtual std::unique_ptr

Please don't duplicate the information from the signature in comments.  No need 
to say that this function returns an IndexASTConsumer (twice, in the first 
sentence and in the \returns clause), the code already says that.

Also, "The compiler instance used to process the input" does not mean much to 
me either.



Comment at: lib/Index/IndexingAction.cpp:154
+return llvm::make_unique(std::move(Consumers));
+  };
+

No semicolon.



Comment at: lib/Index/IndexingAction.cpp:163
+}
+  };
 };

No semicolon.



Comment at: lib/Index/IndexingAction.cpp:186
+  void finish(CompilerInstance &CI) override { DataConsumer->finish(); }
+};
+

No semicolon.



Comment at: lib/Index/IndexingAction.cpp:275
+/// Construct a \c UnitDetails from the invocation associated with the provided
+/// \c CompilerInstance and the provided sysroot path.
+static index::UnitDetails getUnitDetails(const CompilerInstance &CI,

Please don't duplicate type information from the signature in the comment.




Comment at: lib/Index/IndexingAction.cpp:283
+OutputFile = CI.getFrontendOpts().Inputs[0].getFile();
+OutputFile += ".o";
+  }

I don't understand... this is not really the user-specified output file.



Comment at: lib/Index/IndexingAction.cpp:303
+
+/// Construct a \c UnitDetails for the given module file.
+static index::UnitDetails getUnitDetails(serialization::ModuleFile &Mod,

Please don't duplicate type information from the signature in the comment.



Comment at: lib/Index/IndexingContext.h:40
+/// Tracks the current system root path and computes and caches whether a
+/// file is considered a system file or not
+class SystemFileCache {

Please add a period at the end of the comment.



Comment at: lib/Index/IndexingContext.h:44
+  // Records whether a directory entry is system or not.
+  llvm::DenseMap DirEntries;
+  // Keeps track of the last check for whether a FileID is system or

DirEntries => IsSystemDirEntry?



Comment at: lib/Index/IndexingContext.h:46
+  // Keeps track of the last check for whether a FileID is system or
+  // not. This is used to speed up isSystemFile() call.
+  std::pair LastFileCheck;

Triple slashes for doc comments.



Comment at: lib/Index/IndexingContext.h:46
+  // Keeps track of the last check for whether a FileID is system or
+  // not. This is used to speed up isSystemFile() call.
+  std::pair LastFileCheck;

gribozavr wrote:
> Triple slashes for doc comments.
Unclear how a boolean can keep track of the last check.

Did you mean "Whether the file is a system file or not.  This value is a 
cache."  If so, please rename the variable to something like IsSystemFileCache.



Comment at: test/Index/Core/index-source.mm:2
 // RUN: c-index-test core -print-source-symbols -- %s -target 
x86_64-apple-macosx10.7 | FileCheck %s
+// RUN: c-index-test core -print-source-unit -- %s -target 
x86_64-apple-macosx10.7 | FileCheck -check-prefixes=CHECK %s
 

No need to specify check-prefixes=CHECK.



Comment at: test/Index/Core/index-unit.mm:1
+// RUN: rm -rf %t.mcp
+// RUN: c-index-test core -print-source-unit -- -arch x86_64 
-mmacosx-version-min=10.7 -c %s -o %t.o -isystem %S/Inputs/sys -fmodules 
-fmodules-cache-path=%t.mcp -Xclang -fdisable-module-hash -I %S/Inputs/module 
-I %S/Inputs | FileCheck %s

This test is very difficult to read... it is just a dump of random internal 
data structures... what do you think about converting it to a unit test?


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

https://reviews.llvm.org/D39050



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

[PATCH] D58600: [clangd] Emit source to Diagnostic.

[Haojian Wu via Phabricator via cfe-commits]

hokein updated this revision to Diff 189285.
hokein marked 6 inline comments as done.
hokein added a comment.

Address comments.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58600

Files:
  clangd/ClangdUnit.cpp
  clangd/Diagnostics.cpp
  clangd/Diagnostics.h
  clangd/Protocol.cpp
  clangd/Protocol.h
  test/clangd/diagnostic-category.test
  test/clangd/diagnostics.test
  test/clangd/did-change-configuration-params.test
  test/clangd/execute-command.test
  test/clangd/fixits-codeaction.test
  test/clangd/fixits-command.test
  test/clangd/fixits-embed-in-diagnostic.test
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -58,6 +58,8 @@
  std::tie(LSPDiag.range, LSPDiag.severity, LSPDiag.message);
 }
 
+MATCHER_P(DiagSource, Source, "") { return arg.S == Source; }
+
 MATCHER_P(EqualToFix, Fix, "LSP fix " + llvm::to_string(Fix)) {
   if (arg.Message != Fix.Message)
 return false;
@@ -102,6 +104,7 @@
   // This range spans lines.
   AllOf(Diag(Test.range("typo"),
  "use of undeclared identifier 'goo'; did you mean 'foo'?"),
+DiagSource(Diag::Clang),
 WithFix(
 Fix(Test.range("typo"), "foo", "change 'go\\ o' to 'foo'")),
 // This is a pretty normal range.
@@ -137,6 +140,18 @@
   WithFix(Fix(Test.range(), "int", "change return type to 'int'");
 }
 
+TEST(DiagnosticsTest, DiagnosticPreamble) {
+  Annotations Test(R"cpp(
+#include $[["not-found.h"]]
+  )cpp");
+
+  auto TU = TestTU::withCode(Test.code());
+  EXPECT_THAT(TU.build().getDiagnostics(),
+  ElementsAre(testing::AllOf(
+  Diag(Test.range(), "'not-found.h' file not found"),
+  DiagSource(Diag::Clang;
+}
+
 TEST(DiagnosticsTest, ClangTidy) {
   Annotations Test(R"cpp(
 #include $deprecated[["assert.h"]]
@@ -159,6 +174,7 @@
   AllOf(Diag(Test.range("deprecated"),
  "inclusion of deprecated C++ header 'assert.h'; consider "
  "using 'cassert' instead [modernize-deprecated-headers]"),
+DiagSource(Diag::ClangTidy),
 WithFix(Fix(Test.range("deprecated"), "",
 "change '\"assert.h\"' to ''"))),
   Diag(Test.range("doubled"),
@@ -168,6 +184,7 @@
   Diag(Test.range("macroarg"),
"side effects in the 1st macro argument 'X' are repeated in "
"macro expansion [bugprone-macro-repeated-side-effects]"),
+  DiagSource(Diag::ClangTidy),
   WithNote(Diag(Test.range("macrodef"),
 "macro 'SQUARE' defined here "
 "[bugprone-macro-repeated-side-effects]"))),
Index: test/clangd/fixits-embed-in-diagnostic.test
===
--- test/clangd/fixits-embed-in-diagnostic.test
+++ test/clangd/fixits-embed-in-diagnostic.test
@@ -42,7 +42,8 @@
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:},
-# CHECK-NEXT:"severity": 1
+# CHECK-NEXT:"severity": 1,
+# CHECK-NEXT:"source": "clang"
 # CHECK-NEXT:  },
 # CHECK-NEXT:  {
 # CHECK-NEXT:"message": "Previous use is here\n\nfoo.c:1:18: error: use of 'Point' with tag type that does not match previous declaration",
Index: test/clangd/fixits-command.test
===
--- test/clangd/fixits-command.test
+++ test/clangd/fixits-command.test
@@ -17,7 +17,8 @@
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:},
-# CHECK-NEXT:"severity": 2
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang"
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
Index: test/clangd/fixits-codeaction.test
===
--- test/clangd/fixits-codeaction.test
+++ test/clangd/fixits-codeaction.test
@@ -17,7 +17,8 @@
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:},
-# CHECK-NEXT:"severity": 2
+# CHECK-NEXT:"severity": 2,
+# CHECK-NEXT:"source": "clang"
 # CHECK-NEXT:  }
 # CHECK-NEXT:],
 # CHECK-NEXT:"uri": "file://{{.*}}/foo.c"
Index: test/clangd/execute-command.test
===
--- test/clangd/execute-command.test
+++ test/clangd/execute-command.test
@@ -17,7 +17,8 @@
 # CHECK-NEXT:"line": 0
 # CHECK-NEXT:  }
 # CHECK-NEXT:},
-# CHECK-NEXT:"severity": 2
+# CHECK-NEXT:"severity"

[PATCH] D58600: [clangd] Emit source to Diagnostic.

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

add @kadircet as a reviewer, since @ilya-biryukov is OOO.




Comment at: clangd/ClangdUnit.cpp:380
   if (Preamble)
 Diags.insert(Diags.begin(), Preamble->Diags.begin(), 
Preamble->Diags.end());
   return ParsedAST(std::move(Preamble), std::move(Clang), std::move(Action),

ilya-biryukov wrote:
> Preamble diagnostics seem to be missing the source.
> Could we add a test they also have the "source" set? "unresolved include" 
> should be the easiest to get.
good catch, I missed this.



Comment at: clangd/Diagnostics.cpp:381
 LastDiag = Diag();
+LastDiag->ID = Info.getID();
 FillDiagBase(*LastDiag);

jkorous wrote:
> Nit - is this really intended to be part of this patch?
See my comment above.



Comment at: clangd/Diagnostics.h:72
+  // Diagnostic enum ID.
+  unsigned ID;
+  // The source of this diagnostic, e.g. 'clang', 'clang-tidy'.

ilya-biryukov wrote:
> jkorous wrote:
> > Nit - is this really intended to be part of this patch?
> +1, why do we need ID?
Yes, we do need this ID to determine whether a diagnostic is from clang-tidy, 
`CangTidyContext::getCheckName`.


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58600



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

[PATCH] D58556: [LibTooling] Add "smart" retrieval of AST-node source to FixIt library

[Kim Gräsman via Phabricator via cfe-commits]

kimgr added inline comments.



Comment at: clang/include/clang/Tooling/FixIt.h:73
+// context. In contrast with \p getText(), this function selects a source range
+// "automatically", extracting text that a reader might intuitively associate
+// with a node.  Currently, only specialized for \p clang::Stmt, where it will

ymandel wrote:
> ilya-biryukov wrote:
> > What are other tricky cases you have in mind for the future?
> I just assumed that we'd hit more as we dig into them, but, I'm not so sure 
> now.  The two others I can think of offhand are 1) extending to include 
> associated comments, 2) semicolons after declarations.  Commas present a 
> similar challenge (if a bit simpler) when used in a list (vs. the comma 
> operator).  Are there any other separators in C++? 
> 
> At a higher level, it would be nice to align this with your work on tree 
> transformations. That is, think of these functions as short-term shims to 
> simulate the behavior we'll ultimately get from that new library. However, it 
> might be premature to consider those details here.
Peanut gallery comment on this:

> The two others I can think of offhand are
> 1) extending to include associated comments,
> 2) semicolons after declarations.
> Commas present a similar challenge (if a bit simpler) when used in a list 
> (vs. the comma operator).
> Are there any other separators in C++?

Would it make sense to let callers choose what level of expansion they want 
with a flag mask? Somehow I think that makes it easier to name the function, 
too, e.g.:
```
StringRef getExpandedRange(const Stmt &S, ASTContext &Context, ExpansionFlags 
Flags);
```



Repository:
  rC Clang

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

https://reviews.llvm.org/D58556



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

[PATCH] D58600: [clangd] Emit source to Diagnostic.

[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


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D58600



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

r355390 - [ASTImporter] Fix redecl failures of Class and ClassTemplate

[Gabor Marton via cfe-commits]

Author: martong
Date: Tue Mar  5 03:23:24 2019
New Revision: 355390

URL: http://llvm.org/viewvc/llvm-project?rev=355390&view=rev
Log:
[ASTImporter] Fix redecl failures of Class and ClassTemplate

Summary:
Redecl chains of classes and class templates are not handled well
currently. We want to handle them similarly to functions, i.e. try to
keep the structure of the original AST as much as possible. The aim is
to not squash a prototype with a definition, rather we create both and
put them in a redecl chain.

Reviewers: a_sidorin, shafik, a.sidorin

Subscribers: rnkovacs, dkrupp, Szelethus, gamesh411, jdoerfert, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/lib/AST/ASTImporter.cpp
cfe/trunk/unittests/AST/ASTImporterTest.cpp

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=355390&r1=355389&r2=355390&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Tue Mar  5 03:23:24 2019
@@ -2553,26 +2553,6 @@ ExpectedDecl ASTNodeImporter::VisitRecor
 Decl::FOK_None;
   }
 
-  // If this record has a definition in the translation unit we're coming from,
-  // but this particular declaration is not that definition, import the
-  // definition and map to that.
-  TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D &&
-  // Friend template declaration must be imported on its own.
-  !IsFriendTemplate &&
-  // In contrast to a normal CXXRecordDecl, the implicit
-  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
-  // The definition of the implicit CXXRecordDecl in this case is the
-  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
-  // condition in order to be able to import the implict Decl.
-  !D->isImplicit()) {
-ExpectedDecl ImportedDefOrErr = import(Definition);
-if (!ImportedDefOrErr)
-  return ImportedDefOrErr.takeError();
-
-return Importer.MapImported(D, *ImportedDefOrErr);
-  }
-
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
@@ -2601,7 +2581,8 @@ ExpectedDecl ASTNodeImporter::VisitRecor
 auto FoundDecls =
 Importer.findDeclsInToCtx(DC, SearchName);
 if (!FoundDecls.empty()) {
-  // We're going to have to compare D against potentially conflicting 
Decls, so complete it.
+  // We're going to have to compare D against potentially conflicting 
Decls,
+  // so complete it.
   if (D->hasExternalLexicalStorage() && !D->isCompleteDefinition())
 D->getASTContext().getExternalSource()->CompleteType(D);
 }
@@ -4976,17 +4957,6 @@ static ClassTemplateDecl *getDefinition(
 ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   bool IsFriend = D->getFriendObjectKind() != Decl::FOK_None;
 
-  // If this template has a definition in the translation unit we're coming
-  // from, but this particular declaration is not that definition, import the
-  // definition and map to that.
-  ClassTemplateDecl *Definition = getDefinition(D);
-  if (Definition && Definition != D && !IsFriend) {
-if (ExpectedDecl ImportedDefOrErr = import(Definition))
-  return Importer.MapImported(D, *ImportedDefOrErr);
-else
-  return ImportedDefOrErr.takeError();
-  }
-
   // Import the major distinguishing characteristics of this class template.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;

Modified: cfe/trunk/unittests/AST/ASTImporterTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/AST/ASTImporterTest.cpp?rev=355390&r1=355389&r2=355390&view=diff
==
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp (original)
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp Tue Mar  5 03:23:24 2019
@@ -4241,8 +4241,7 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportDefinitionAfterImportedPrototype)
-// FIXME This does not pass, possible error with Class import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, ,
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportDefinitionAfterImportedPrototype)
@@ -4250,16 +4249,14 @@ ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
 DISABLED_,
 ImportDefinitionAfterImportedPrototype)
-// FIXME This does not 

[PATCH] D58502: [ASTImporter] Fix redecl failures of Class and ClassTemplate

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355390: [ASTImporter] Fix redecl failures of Class and 
ClassTemplate (authored by martong, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58502

Files:
  cfe/trunk/lib/AST/ASTImporter.cpp
  cfe/trunk/unittests/AST/ASTImporterTest.cpp

Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -2553,26 +2553,6 @@
 Decl::FOK_None;
   }
 
-  // If this record has a definition in the translation unit we're coming from,
-  // but this particular declaration is not that definition, import the
-  // definition and map to that.
-  TagDecl *Definition = D->getDefinition();
-  if (Definition && Definition != D &&
-  // Friend template declaration must be imported on its own.
-  !IsFriendTemplate &&
-  // In contrast to a normal CXXRecordDecl, the implicit
-  // CXXRecordDecl of ClassTemplateSpecializationDecl is its redeclaration.
-  // The definition of the implicit CXXRecordDecl in this case is the
-  // ClassTemplateSpecializationDecl itself. Thus, we start with an extra
-  // condition in order to be able to import the implict Decl.
-  !D->isImplicit()) {
-ExpectedDecl ImportedDefOrErr = import(Definition);
-if (!ImportedDefOrErr)
-  return ImportedDefOrErr.takeError();
-
-return Importer.MapImported(D, *ImportedDefOrErr);
-  }
-
   // Import the major distinguishing characteristics of this record.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
@@ -2601,7 +2581,8 @@
 auto FoundDecls =
 Importer.findDeclsInToCtx(DC, SearchName);
 if (!FoundDecls.empty()) {
-  // We're going to have to compare D against potentially conflicting Decls, so complete it.
+  // We're going to have to compare D against potentially conflicting Decls,
+  // so complete it.
   if (D->hasExternalLexicalStorage() && !D->isCompleteDefinition())
 D->getASTContext().getExternalSource()->CompleteType(D);
 }
@@ -4976,17 +4957,6 @@
 ExpectedDecl ASTNodeImporter::VisitClassTemplateDecl(ClassTemplateDecl *D) {
   bool IsFriend = D->getFriendObjectKind() != Decl::FOK_None;
 
-  // If this template has a definition in the translation unit we're coming
-  // from, but this particular declaration is not that definition, import the
-  // definition and map to that.
-  ClassTemplateDecl *Definition = getDefinition(D);
-  if (Definition && Definition != D && !IsFriend) {
-if (ExpectedDecl ImportedDefOrErr = import(Definition))
-  return Importer.MapImported(D, *ImportedDefOrErr);
-else
-  return ImportedDefOrErr.takeError();
-  }
-
   // Import the major distinguishing characteristics of this class template.
   DeclContext *DC, *LexicalDC;
   DeclarationName Name;
Index: cfe/trunk/unittests/AST/ASTImporterTest.cpp
===
--- cfe/trunk/unittests/AST/ASTImporterTest.cpp
+++ cfe/trunk/unittests/AST/ASTImporterTest.cpp
@@ -4241,8 +4241,7 @@
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportDefinitionAfterImportedPrototype)
-// FIXME This does not pass, possible error with Class import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, ,
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Variable, ,
 ImportDefinitionAfterImportedPrototype)
@@ -4250,16 +4249,14 @@
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplate,
 DISABLED_,
 ImportDefinitionAfterImportedPrototype)
-// FIXME This does not pass, possible error with ClassTemplate import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, ClassTemplate, ,
 ImportDefinitionAfterImportedPrototype)
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, FunctionTemplateSpec, ,
 ImportDefinitionAfterImportedPrototype)
 
 ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Function, ,
 ImportPrototypeAfterImportedDefinition)
-// FIXME This does not pass, possible error with Class import.
-ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, DISABLED_,
+ASTIMPORTER_INSTANTIATE_TYPED_TEST_CASE(RedeclChain, Class, ,
 ImportPrototypeAfterImportedDefinitio

[PATCH] D58880: [WIP] [Looking for API feedback] [clangd] Type hierarchy subtypes

[Haojian Wu via Phabricator via cfe-commits]

hokein added a comment.

Haven't looked at the patch. I think we can extend the existing Ref to support 
it, so that most of the stuff could be reused, rather than implementing a new 
slab:

- introduce a new RefKind, like BaseOf
- add a new field SymbolID in Ref

and `clangIndex` library has already implemented relationship functionality (we 
could use `handleDeclOccurence` to store the symbols relationships).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58880



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

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D58894#1416691 , @djtodoro wrote:

> @lebedev.ri Thanks for your comment!
>
> > Is there any way to model this more generically?
> > I.e don't duplicate every naive matcher (ignoring possible presence of , 
> > op) with an variant that does not ignore ,.
> > E.g. will this handle (a,b)+=1 ?
>
> Hmm, I am not sure if there is a place for that, since this is very specific 
> to comma operators. Any suggestions ?
>  It handles the situation (`(a,b)+=1`).
>
> > What about (a,b).callNonConstMethod(), (a,b).callConstMethod() ?
>
> This needs to be extended to support function/method calls. I guess 
> `ExprMutationAnalyzer::findFunctionArgMutation` requires an improvement.


Well, given

  // LHS of any assignment operators.
  const auto AsAssignmentLhs =
  binaryOperator(isAssignmentOperator(), hasLHS(equalsNode(Exp)));

The problem is that `hasLHS()` may get `,` op.
In `isAssigmentWithCommma()`, you check if that is `,` op, and if so, return 
it's right hand.
And this duplication is the problem as far as i can see.

Now, i don't know the final solution, but have you considered adding something 
like:

  AST_MATCHER_P(Expr, skipCommaOps,
ast_matchers::internal::Matcher, InnerMatcher) {
const Expr* Result = Node;
while (const auto *BOComma =
 dyn_cast_or_null(Result->IgnoreParens())) {
  if (!BOComma->isCommaOp())
break;
  Result = BOComma->getRHS();
}
return Result;
  }

and then simply changing

  // LHS of any assignment operators.
  const auto AsAssignmentLhs =
  binaryOperator(isAssignmentOperator(), hasLHS(equalsNode(Exp)));

to

  // LHS of any assignment operators.
  const auto AsAssignmentLhs =
  binaryOperator(isAssignmentOperator(), 
hasLHS(skipCommaOps(equalsNode(Exp;

?


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

https://reviews.llvm.org/D58894



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

r355395 - [ASTMatchers] Improved formatting in a documentation comment

[Dmitri Gribenko via cfe-commits]

Author: gribozavr
Date: Tue Mar  5 04:38:18 2019
New Revision: 355395

URL: http://llvm.org/viewvc/llvm-project?rev=355395&view=rev
Log:
[ASTMatchers] Improved formatting in a documentation comment

Modified:
cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h

Modified: cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h?rev=355395&r1=355394&r2=355395&view=diff
==
--- cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h (original)
+++ cfe/trunk/include/clang/ASTMatchers/ASTMatchersInternal.h Tue Mar  5 
04:38:18 2019
@@ -1547,8 +1547,7 @@ inline bool ValueEqualsMatcher record;
+///   const VariadicDynCastAllOfMatcher record;
 /// Creates a functor record(...) that creates a Matcher given
 /// a variable number of arguments of type Matcher.
 /// The returned matcher matches if the given Decl can by dynamically


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

r355396 - [analyzer] Fix taint propagation in GenericTaintChecker

[Kristof Umann via cfe-commits]

Author: szelethus
Date: Tue Mar  5 04:42:59 2019
New Revision: 355396

URL: http://llvm.org/viewvc/llvm-project?rev=355396&view=rev
Log:
[analyzer] Fix taint propagation in GenericTaintChecker

The gets function has no SrcArgs. Because the default value for isTainted was
false, it didn't mark its DstArgs as tainted.

Patch by Gábor Borsik!

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

Modified:
cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
cfe/trunk/test/Analysis/taint-generic.c

Modified: cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp?rev=355396&r1=355395&r2=355396&view=diff
==
--- cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp (original)
+++ cfe/trunk/lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp Tue Mar  5 
04:42:59 2019
@@ -458,7 +458,7 @@ GenericTaintChecker::TaintPropagationRul
   ProgramStateRef State = C.getState();
 
   // Check for taint in arguments.
-  bool IsTainted = false;
+  bool IsTainted = true;
   for (unsigned ArgNum : SrcArgs) {
 if (ArgNum >= CE->getNumArgs())
   return State;

Modified: cfe/trunk/test/Analysis/taint-generic.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Analysis/taint-generic.c?rev=355396&r1=355395&r2=355396&view=diff
==
--- cfe/trunk/test/Analysis/taint-generic.c (original)
+++ cfe/trunk/test/Analysis/taint-generic.c Tue Mar  5 04:42:59 2019
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT 
-analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 
-Wno-format-security -verify %s
 
 int scanf(const char *restrict format, ...);
+char *gets(char *str);
 int getchar(void);
 
 typedef struct _FILE FILE;
@@ -142,6 +143,12 @@ void testTaintSystemCall3() {
   system(buffern2); // expected-warning {{Untrusted data is passed to a system 
call}}
 }
 
+void testGets() {
+  char str[50];
+  gets(str);
+  system(str); // expected-warning {{Untrusted data is passed to a system 
call}}
+}
+
 void testTaintedBufferSize() {
   size_t ts;
   scanf("%zd", &ts);


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

[PATCH] D58828: [analyzer] Fix taint propagation in GenericTaintChecker

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355396: [analyzer] Fix taint propagation in 
GenericTaintChecker (authored by Szelethus, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D58828

Files:
  lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
  test/Analysis/taint-generic.c


Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -458,7 +458,7 @@
   ProgramStateRef State = C.getState();
 
   // Check for taint in arguments.
-  bool IsTainted = false;
+  bool IsTainted = true;
   for (unsigned ArgNum : SrcArgs) {
 if (ArgNum >= CE->getNumArgs())
   return State;
Index: test/Analysis/taint-generic.c
===
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT 
-analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 
-Wno-format-security -verify %s
 
 int scanf(const char *restrict format, ...);
+char *gets(char *str);
 int getchar(void);
 
 typedef struct _FILE FILE;
@@ -142,6 +143,12 @@
   system(buffern2); // expected-warning {{Untrusted data is passed to a system 
call}}
 }
 
+void testGets() {
+  char str[50];
+  gets(str);
+  system(str); // expected-warning {{Untrusted data is passed to a system 
call}}
+}
+
 void testTaintedBufferSize() {
   size_t ts;
   scanf("%zd", &ts);


Index: lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
===
--- lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
+++ lib/StaticAnalyzer/Checkers/GenericTaintChecker.cpp
@@ -458,7 +458,7 @@
   ProgramStateRef State = C.getState();
 
   // Check for taint in arguments.
-  bool IsTainted = false;
+  bool IsTainted = true;
   for (unsigned ArgNum : SrcArgs) {
 if (ArgNum >= CE->getNumArgs())
   return State;
Index: test/Analysis/taint-generic.c
===
--- test/Analysis/taint-generic.c
+++ test/Analysis/taint-generic.c
@@ -2,6 +2,7 @@
 // RUN: %clang_analyze_cc1  -DFILE_IS_STRUCT -analyzer-checker=alpha.security.taint,core,alpha.security.ArrayBoundV2 -Wno-format-security -verify %s
 
 int scanf(const char *restrict format, ...);
+char *gets(char *str);
 int getchar(void);
 
 typedef struct _FILE FILE;
@@ -142,6 +143,12 @@
   system(buffern2); // expected-warning {{Untrusted data is passed to a system call}}
 }
 
+void testGets() {
+  char str[50];
+  gets(str);
+  system(str); // expected-warning {{Untrusted data is passed to a system call}}
+}
+
 void testTaintedBufferSize() {
   size_t ts;
   scanf("%zd", &ts);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

[Djordje Todorovic via Phabricator via cfe-commits]

djtodoro added a comment.

@lebedev.ri I agree, thank you! I needed to be more precise in my previous 
reply, sorry for that. I thought it will be (somehow) overhead if I change 
existing, very basic, matchers.

I already implemented a static function that skips comma operands, and extended 
this to support member calls, functions, etc.
But, implementing it as a new matcher sounds like better idea.

Thanks!


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

https://reviews.llvm.org/D58894



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

[PATCH] D58894: [analyzer] Handle modification of vars inside an expr with comma operator

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri added a comment.

In D58894#1418251 , @djtodoro wrote:

> @lebedev.ri I agree, thank you! I needed to be more precise in my previous 
> reply, sorry for that. I thought it will be (somehow) overhead if I change 
> existing, very basic, matchers.


I indeed don't think the existing matchers should be changed to ignore these 
`,` ops (or implicit casts, like some issue reports propose).

> I already implemented a static function that skips comma operands, and 
> extended this to support member calls, functions, etc.



> But, implementing it as a new matcher sounds like better idea.

Yes. I think this matcher will be very baseline, and can just be added where 
needed
(with appropriate test coverage, of course), without matcher duplication like 
in this current diff.

> Thanks!

Thanks for working on this.


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

https://reviews.llvm.org/D58894



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

[PATCH] D58060: Fix diagnostic for addr spaces in reference binding

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 189306.
Anastasia retitled this revision from "Fix diagnostic for addr spaces in 
static_cast" to "Fix diagnostic for addr spaces in reference binding".
Anastasia edited the summary of this revision.
Anastasia added a comment.

- Fixed incorrect diagnostic in address space mismatch case when binding 
reference to a temporary value of a constant with incompatible type. Added 
missing test case.


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

https://reviews.llvm.org/D58060

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaInit.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-0x.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-var.cpp
  test/CXX/expr/expr.post/expr.static.cast/p3-p4-0x.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p16.cpp
  test/Misc/diag-template-diffing.cpp
  test/SemaCXX/builtins-arm.cpp
  test/SemaCXX/err_reference_bind_drops_quals.cpp
  test/SemaCXX/references.cpp
  test/SemaOpenCLCXX/address-space-castoperators.cpp
  test/SemaOpenCLCXX/address-space-references.cl

Index: test/SemaOpenCLCXX/address-space-references.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-references.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+
+int bar(const __global unsigned int &i); // expected-note{{passing argument to parameter 'i' here}}
+//FIXME: With the overload below the call should be resolved
+// successfully. However, current overload resolution logic
+// can't detect this case and therefore fails.
+//int bar(const unsigned int &i);
+
+void foo() {
+  bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
+}
Index: test/SemaOpenCLCXX/address-space-castoperators.cpp
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-castoperators.cpp
@@ -0,0 +1,17 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+
+void test_ref(int &gen, __global const int &glob) {
+  static_cast<__global int &>(gen); // expected-error{{binding reference of type '__global int' to value of type '__generic int' changes address space}}
+  static_cast<__global const int &>(gen);   //expected-error{{binding reference of type 'const __global int' to value of type '__generic int' changes address space}}
+  static_cast<__global int &>(glob);//expected-error{{binding reference of type '__global int' to value of type 'const __global int' drops 'const' qualifier}}
+  static_cast<__local int &>(glob); //expected-error{{binding reference of type '__local int' to value of type 'const __global int' changes address space}}
+  static_cast<__generic const int &>(glob); //expected-warning{{expression result unused}}
+}
+
+void test_ptr(int *gen, __global const int *glob) {
+  static_cast<__global int *>(gen); // expected-error{{static_cast from '__generic int *' to '__global int *' is not allowed}}
+  static_cast<__global const int *>(gen);   //expected-error{{static_cast from '__generic int *' to 'const __global int *' is not allowed}}
+  static_cast<__global int *>(glob);//expected-error{{static_cast from 'const __global int *' to '__global int *' is not allowed}}
+  static_cast<__local int *>(glob); //expected-error{{static_cast from 'const __global int *' to '__local int *' is not allowed}}
+  static_cast<__generic const int *>(glob); //expected-warning{{expression result unused}}
+}
Index: test/SemaCXX/references.cpp
===
--- test/SemaCXX/references.cpp
+++ test/SemaCXX/references.cpp
@@ -55,13 +55,13 @@
 void test5() {
   //  const double& rcd2 = 2; // rcd2 refers to temporary with value 2.0
   const volatile int cvi = 1;
-  const int& r = cvi; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+  const int& r = cvi; // expected-error{{binding reference of type 'const int' to value of type 'const volatile int' drops 'volatile' qualifier}}
 
 #if __cplusplus >= 201103L
-  const int& r2{cvi}; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+  const int& r2{cvi}; // expected-error{{binding reference of type 'const int' to value of type 'const volatile int' drops 'volatile' qualifier}}
 
   const int a = 2;
-  int& r3{a}; // expected-error{{binding value of type 'const int' to reference to type 'int' drops 'const'}}
+  int& r3{a}; // expected-error{{binding reference of type 'int' to value of type 'const int' drops 'const' qualifier}}
 
   const int&& r4{a}; // expected-error{{rvalue reference to type 'const int' cannot bind to lvalue of type 'const int'}}
 
Index: test/SemaCXX/err_reference_bind_drop

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

[Joerg Sonnenberger via Phabricator via cfe-commits]

joerg added a comment.

Well, that was a sample to illustrate the point. A full working (and now 
failing) example is:

  static inline void outl(unsigned port, unsigned data) {
if (__builtin_constant_p(port) && port < 0x100) {
  __asm volatile("outl %0,%w1" : : "a"(data), "n"(port));
} else {
  __asm volatile("outl %0,%w1" : : "a"(data), "d"(port));
}
  }
  
  void f(unsigned port) { outl(1, 1); }


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

https://reviews.llvm.org/D58821



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Andrew Paprocki via Phabricator via cfe-commits]

apaprocki added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

No need to be `sorry:` :) This should probably just say `error: XCOFF is 
unimplemented` to be more direct in case anything is expecting "error:" in the 
output.



Comment at: llvm/lib/Support/Triple.cpp:537
   return StringSwitch(EnvironmentName)
+// FIXME: We have to put XCOFF before COFF;
+// perhaps an order-independent pattern matching is desired?

hubert.reinterpretcast wrote:
> If the conclusion is that checking XCOFF before COFF is fine, then we should 
> remove the FIXME and just leave a normal comment.
Agreed, existing code seems fine as long as there is a comment explaining that 
`xcoff` must come before `coff` in case it isn't obvious at a quick glance.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[clang-tools-extra] r355401 - [clang-tidy] Fix bugprone-string-constructor crash

[Alexander Kornienko via cfe-commits]

Author: alexfh
Date: Tue Mar  5 06:09:57 2019
New Revision: 355401

URL: http://llvm.org/viewvc/llvm-project?rev=355401&view=rev
Log:
[clang-tidy] Fix bugprone-string-constructor crash

Modified:
clang-tools-extra/trunk/clang-tidy/bugprone/StringConstructorCheck.cpp
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-constructor.cpp

Modified: clang-tools-extra/trunk/clang-tidy/bugprone/StringConstructorCheck.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/clang-tidy/bugprone/StringConstructorCheck.cpp?rev=355401&r1=355400&r2=355401&view=diff
==
--- clang-tools-extra/trunk/clang-tidy/bugprone/StringConstructorCheck.cpp 
(original)
+++ clang-tools-extra/trunk/clang-tidy/bugprone/StringConstructorCheck.cpp Tue 
Mar  5 06:09:57 2019
@@ -138,7 +138,8 @@ void StringConstructorCheck::check(const
 }
   } else if (const auto *Ptr = Result.Nodes.getNodeAs("from-ptr")) {
 Expr::EvalResult ConstPtr;
-if (Ptr->EvaluateAsRValue(ConstPtr, Ctx) &&
+if (!Ptr->isInstantiationDependent() &&
+Ptr->EvaluateAsRValue(ConstPtr, Ctx) &&
 ((ConstPtr.Val.isInt() && ConstPtr.Val.getInt().isNullValue()) ||
  (ConstPtr.Val.isLValue() && ConstPtr.Val.isNullPointer( {
   diag(Loc, "constructing string from nullptr is undefined behaviour");

Modified: 
clang-tools-extra/trunk/test/clang-tidy/bugprone-string-constructor.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/bugprone-string-constructor.cpp?rev=355401&r1=355400&r2=355401&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/bugprone-string-constructor.cpp 
(original)
+++ clang-tools-extra/trunk/test/clang-tidy/bugprone-string-constructor.cpp Tue 
Mar  5 06:09:57 2019
@@ -65,3 +65,11 @@ void Valid() {
   std::string s2("test", 3);
   std::string s3("test");
 }
+
+namespace instantiation_dependent_exprs {
+template
+struct S {
+  bool x;
+  std::string f() { return x ? "a" : "b"; }
+};
+}


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

[PATCH] D58917: [HIP] Do not unbundle object files for -fno-gpu-rdc

[Alexey Bataev via Phabricator via cfe-commits]

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

LG with a nit




Comment at: lib/Driver/Driver.cpp:2298
+/// Flag for -fgpu-rdc.
+bool Relocatable;
   public:

Set the default initializer for the field


Repository:
  rC Clang

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

https://reviews.llvm.org/D58917



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

[PATCH] D58060: Fix diagnostic for addr spaces in reference binding

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan added inline comments.



Comment at: test/SemaOpenCLCXX/address-space-references.cl:7
+// can't detect this case and therefore fails.
+//int bar(const unsigned int &i);
+

Is there a reason the declaration is commented out? If it's just a problem with 
the resolution, wouldn't it still be possible to have the declaration here 
anyway?


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

https://reviews.llvm.org/D58060



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

[PATCH] D58634: [PR40778] Generate address space conversion when binding reference to a temporary value in different address space

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 189313.
Anastasia added a comment.

- Implement the fix correctly by added an extra address space conversion step 
after binding the reference


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

https://reviews.llvm.org/D58634

Files:
  include/clang/AST/Type.h
  lib/Sema/SemaInit.cpp
  test/CodeGenOpenCLCXX/addrspace-references.cl


Index: test/CodeGenOpenCLCXX/addrspace-references.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/addrspace-references.cl
@@ -0,0 +1,11 @@
+//RUN: %clang_cc1 %s -cl-std=c++ -triple spir -emit-llvm -o - | FileCheck %s
+
+int bar(const unsigned int &i);
+
+void foo() {
+  // The generic addr space reference parameter object will be bound
+  // to a temporary value allocated in private addr space. We need an
+  // addrspacecast before passing the value to the function.
+  // CHECK: addrspacecast i32* %ref.tmp to i32 addrspace(4)*
+  bar(1);
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4760,7 +4760,15 @@
   //copy-initialization (8.5). The reference is then bound to the
   //temporary. [...]
 
-  InitializedEntity TempEntity = InitializedEntity::InitializeTemporary(cv1T1);
+  // Ignore address space of reference type at this point and perform address
+  // space conversion after the reference binding step.
+  QualType cv1T1IgnoreAS =
+  T1Quals.hasAddressSpace()
+  ? S.Context.getQualifiedType(T1, T1Quals.withoutAddressSpace())
+  : cv1T1;
+
+  InitializedEntity TempEntity =
+  InitializedEntity::InitializeTemporary(cv1T1IgnoreAS);
 
   // FIXME: Why do we use an implicit conversion here rather than trying
   // copy-initialization?
@@ -4795,8 +4803,9 @@
   //than, cv2; otherwise, the program is ill-formed.
   unsigned T1CVRQuals = T1Quals.getCVRQualifiers();
   unsigned T2CVRQuals = T2Quals.getCVRQualifiers();
-  if (RefRelationship == Sema::Ref_Related &&
-  (T1CVRQuals | T2CVRQuals) != T1CVRQuals) {
+  if ((RefRelationship == Sema::Ref_Related &&
+   (T1CVRQuals | T2CVRQuals) != T1CVRQuals) ||
+  !T1Quals.isAddressSpaceSupersetOf(T2Quals)) {
 
Sequence.SetFailed(InitializationSequence::FK_ReferenceInitDropsQualifiers);
 return;
   }
@@ -4810,7 +4819,11 @@
 return;
   }
 
-  Sequence.AddReferenceBindingStep(cv1T1, /*bindingTemporary=*/true);
+  Sequence.AddReferenceBindingStep(cv1T1IgnoreAS, /*bindingTemporary=*/true);
+
+  if (T1Quals.hasAddressSpace())
+Sequence.AddQualificationConversionStep(cv1T1, isLValueRef ? VK_LValue
+   : VK_XValue);
 }
 
 /// Attempt character array initialization from a string literal
Index: include/clang/AST/Type.h
===
--- include/clang/AST/Type.h
+++ include/clang/AST/Type.h
@@ -317,6 +317,11 @@
 qs.removeObjCLifetime();
 return qs;
   }
+  Qualifiers withoutAddressSpace() const {
+Qualifiers qs = *this;
+qs.removeAddressSpace();
+return qs;
+  }
 
   bool hasObjCLifetime() const { return Mask & LifetimeMask; }
   ObjCLifetime getObjCLifetime() const {


Index: test/CodeGenOpenCLCXX/addrspace-references.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/addrspace-references.cl
@@ -0,0 +1,11 @@
+//RUN: %clang_cc1 %s -cl-std=c++ -triple spir -emit-llvm -o - | FileCheck %s
+
+int bar(const unsigned int &i);
+
+void foo() {
+  // The generic addr space reference parameter object will be bound
+  // to a temporary value allocated in private addr space. We need an
+  // addrspacecast before passing the value to the function.
+  // CHECK: addrspacecast i32* %ref.tmp to i32 addrspace(4)*
+  bar(1);
+}
Index: lib/Sema/SemaInit.cpp
===
--- lib/Sema/SemaInit.cpp
+++ lib/Sema/SemaInit.cpp
@@ -4760,7 +4760,15 @@
   //copy-initialization (8.5). The reference is then bound to the
   //temporary. [...]
 
-  InitializedEntity TempEntity = InitializedEntity::InitializeTemporary(cv1T1);
+  // Ignore address space of reference type at this point and perform address
+  // space conversion after the reference binding step.
+  QualType cv1T1IgnoreAS =
+  T1Quals.hasAddressSpace()
+  ? S.Context.getQualifiedType(T1, T1Quals.withoutAddressSpace())
+  : cv1T1;
+
+  InitializedEntity TempEntity =
+  InitializedEntity::InitializeTemporary(cv1T1IgnoreAS);
 
   // FIXME: Why do we use an implicit conversion here rather than trying
   // copy-initialization?
@@ -4795,8 +4803,9 @@
   //than, cv2; otherwise, the program is ill-formed.
   unsigned T1CVRQuals = T1Quals.getCVRQualifiers();
   unsigned T2CVRQuals = T2Quals.getCVRQualifiers();
-  if (RefRelationship == Sema::R

[PATCH] D58057: Allow bundle size to be 0 in clang-offload-bundler

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Alexey, could you please also review this patch? Thanks.


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

https://reviews.llvm.org/D58057



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

[PATCH] D58917: [HIP] Do not unbundle object files for -fno-gpu-rdc

[Yaxun Liu via Phabricator via cfe-commits]

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



Comment at: lib/Driver/Driver.cpp:2298
+/// Flag for -fgpu-rdc.
+bool Relocatable;
   public:

ABataev wrote:
> Set the default initializer for the field
will do. thanks.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58917



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

[PATCH] D58154: Add support for -fpermissive.

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D58154#1417474 , @rsmith wrote:

> In D58154#1417363 , @aaron.ballman 
> wrote:
>
> > I'm not super keen on supporting -fpermissive --  what is the impetus for 
> > supporting this? Is it just for GCC compatibility?
>
>
> Yes, exactly that -- this is to improve our drop-in compatibility with GCC. 
> See http://llvm.org/PR40670 for an example user request for this.


I don't think we should support -fpermissive. I'm sure that we *can*, but I 
don't think that we *should*. My strong preference is to close that report as 
WONTFIX.

> In much the same way that `-w` can be used to mean "I just want this (crufty 
> old, but presumed correct) code to compile; don't bother me with warnings 
> about it", `-fpermissive` (and in particular `-fpermissive -w`) can be used 
> to mean "I just want this (crufty, old, and relying on compiler extensions, 
> but presumably correct) code to compile; don't bother me with complaints that 
> it's technically ill-formed".

I think -fpermssive is actively harmful to support because its use cannot be 
limited to old code bases to allow them to continue to limp along in a broken 
state. Naive users will turn it on simply because it gets their code to 
compile. Also, there is a big difference between -w and -fpermissive; the 
former is reasonable because of false positive diagnostics, but the latter 
simply hides utterly broken code that may or may not happen to "work". This 
flag is so toxic that our static analyzer won't attempt to analyze code relying 
on -fpermissive to compile, so there is out-of-tree cost to Clang supporting 
this.

As much as I usually love improving compiler compatibility, I don't think 
compatibility is sufficiently compelling for this feature. Personally, I think 
supporting -fpermissive is about as compelling as trying to be bug-for-bug 
compatible with another compiler (though, it is marginally better because this 
is at least a documented compiler flag).


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D58057: Allow bundle size to be 0 in clang-offload-bundler

[Alexey Bataev via Phabricator via cfe-commits]

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

LG


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

https://reviews.llvm.org/D58057



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

[PATCH] D58791: [build] Rename clang-headers to clang-resource-headers

[Shoaib Meenai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355340: [build] Rename clang-headers to 
clang-resource-headers (authored by smeenai, committed by ).
Herald added a subscriber: delcypher.

Changed prior to commit:
  https://reviews.llvm.org/D58791?vs=189014&id=189195#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58791

Files:
  cfe/trunk/cmake/caches/Apple-stage2.cmake
  cfe/trunk/cmake/caches/BaremetalARM.cmake
  cfe/trunk/cmake/caches/DistributionExample-stage2.cmake
  cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
  cfe/trunk/cmake/modules/AddClang.cmake
  cfe/trunk/docs/LibTooling.rst
  cfe/trunk/docs/ReleaseNotes.rst
  cfe/trunk/examples/clang-interpreter/CMakeLists.txt
  cfe/trunk/lib/Headers/CMakeLists.txt
  cfe/trunk/test/CMakeLists.txt
  cfe/trunk/tools/driver/CMakeLists.txt
  cfe/trunk/tools/libclang/CMakeLists.txt
  clang-tools-extra/trunk/clang-tidy/tool/CMakeLists.txt
  clang-tools-extra/trunk/test/CMakeLists.txt
  compiler-rt/trunk/test/CMakeLists.txt
  lldb/trunk/cmake/modules/LLDBFramework.cmake
  lldb/trunk/lldb.xcodeproj/project.pbxproj
  lldb/trunk/scripts/Xcode/package-clang-headers.py
  lldb/trunk/scripts/Xcode/package-clang-resource-headers.py
  lldb/trunk/source/API/CMakeLists.txt
  llvm/trunk/docs/Docker.rst
  llvm/trunk/runtimes/CMakeLists.txt
  llvm/trunk/utils/docker/build_docker_image.sh
  openmp/trunk/cmake/OpenMPTesting.cmake

Index: cfe/trunk/cmake/modules/AddClang.cmake
===
--- cfe/trunk/cmake/modules/AddClang.cmake
+++ cfe/trunk/cmake/modules/AddClang.cmake
@@ -133,7 +133,7 @@
   endif()
 
   add_clang_executable(${name} ${ARGN})
-  add_dependencies(${name} clang-headers)
+  add_dependencies(${name} clang-resource-headers)
 
   if (CLANG_BUILD_TOOLS)
 if(${name} IN_LIST LLVM_DISTRIBUTION_COMPONENTS OR
Index: cfe/trunk/cmake/caches/BaremetalARM.cmake
===
--- cfe/trunk/cmake/caches/BaremetalARM.cmake
+++ cfe/trunk/cmake/caches/BaremetalARM.cmake
@@ -41,7 +41,7 @@
 set(LLVM_DISTRIBUTION_COMPONENTS
   clang
   lld
-  clang-headers
+  clang-resource-headers
   builtins-armv6m-none-eabi
   builtins-armv7m-none-eabi
   builtins-armv7em-none-eabi
Index: cfe/trunk/cmake/caches/Apple-stage2.cmake
===
--- cfe/trunk/cmake/caches/Apple-stage2.cmake
+++ cfe/trunk/cmake/caches/Apple-stage2.cmake
@@ -60,7 +60,7 @@
   clang
   LTO
   clang-format
-  clang-headers
+  clang-resource-headers
   cxx-headers
   ${LLVM_TOOLCHAIN_TOOLS}
   CACHE STRING "")
Index: cfe/trunk/cmake/caches/DistributionExample-stage2.cmake
===
--- cfe/trunk/cmake/caches/DistributionExample-stage2.cmake
+++ cfe/trunk/cmake/caches/DistributionExample-stage2.cmake
@@ -23,7 +23,7 @@
   clang
   LTO
   clang-format
-  clang-headers
+  clang-resource-headers
   builtins
   runtimes
   ${LLVM_TOOLCHAIN_TOOLS}
Index: cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
===
--- cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
+++ cfe/trunk/cmake/caches/Fuchsia-stage2.cmake
@@ -167,7 +167,7 @@
   LTO
   clang-apply-replacements
   clang-format
-  clang-headers
+  clang-resource-headers
   clang-include-fixer
   clang-refactor
   clang-tidy
Index: cfe/trunk/test/CMakeLists.txt
===
--- cfe/trunk/test/CMakeLists.txt
+++ cfe/trunk/test/CMakeLists.txt
@@ -46,7 +46,7 @@
 endif ()
 
 list(APPEND CLANG_TEST_DEPS
-  clang clang-headers
+  clang clang-resource-headers
   clang-format
   c-index-test diagtool
   clang-tblgen
Index: cfe/trunk/examples/clang-interpreter/CMakeLists.txt
===
--- cfe/trunk/examples/clang-interpreter/CMakeLists.txt
+++ cfe/trunk/examples/clang-interpreter/CMakeLists.txt
@@ -16,7 +16,7 @@
   )
 
 add_dependencies(clang-interpreter
-  clang-headers
+  clang-resource-headers
   )
 
 target_link_libraries(clang-interpreter
Index: cfe/trunk/lib/Headers/CMakeLists.txt
===
--- cfe/trunk/lib/Headers/CMakeLists.txt
+++ cfe/trunk/lib/Headers/CMakeLists.txt
@@ -157,8 +157,8 @@
 # Generate arm_fp16.h
 clang_generate_header(-gen-arm-fp16 arm_fp16.td arm_fp16.h)
 
-add_custom_target(clang-headers ALL DEPENDS ${out_files})
-set_target_properties(clang-headers PROPERTIES
+add_custom_target(clang-resource-headers ALL DEPENDS ${out_files})
+set_target_properties(clang-resource-headers PROPERTIES
   FOLDER "Misc"
   RUNTIME_OUTPUT_DIRECTORY "${output_dir}")
 
@@ -167,10 +167,10 @@
 install(
   DIRECTORY ${output_dir}
   DESTINATION ${header_install_dir}
-  COMPONENT clang-headers)
+  COMPONENT clang-resource-headers)
 
 if (N

r355368 - Replace clang::FileData with llvm::vfs::Status

[Harlan Haskins via cfe-commits]

Author: harlanhaskins
Date: Mon Mar  4 18:27:12 2019
New Revision: 355368

URL: http://llvm.org/viewvc/llvm-project?rev=355368&view=rev
Log:
Replace clang::FileData with llvm::vfs::Status

Summary:
FileData was only ever used as a container for the values in
llvm::vfs::Status, so they might as well be consolidated.

The `InPCH` member was also always set to false, and unused.

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Basic/FileManager.h
cfe/trunk/include/clang/Basic/FileSystemStatCache.h
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/lib/Basic/FileSystemStatCache.cpp
cfe/trunk/lib/Frontend/TextDiagnostic.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/include/clang/Basic/FileManager.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileManager.h?rev=355368&r1=355367&r2=355368&view=diff
==
--- cfe/trunk/include/clang/Basic/FileManager.h (original)
+++ cfe/trunk/include/clang/Basic/FileManager.h Mon Mar  4 18:27:12 2019
@@ -67,7 +67,6 @@ class FileEntry {
   unsigned UID;   // A unique (small) ID for the file.
   llvm::sys::fs::UniqueID UniqueID;
   bool IsNamedPipe;
-  bool InPCH;
   bool IsValid;   // Is this \c FileEntry initialized and valid?
 
   /// The open file, if it is owned by the \p FileEntry.
@@ -75,7 +74,7 @@ class FileEntry {
 
 public:
   FileEntry()
-  : UniqueID(0, 0), IsNamedPipe(false), InPCH(false), IsValid(false)
+  : UniqueID(0, 0), IsNamedPipe(false), IsValid(false)
   {}
 
   FileEntry(const FileEntry &) = delete;
@@ -87,7 +86,6 @@ public:
   off_t getSize() const { return Size; }
   unsigned getUID() const { return UID; }
   const llvm::sys::fs::UniqueID &getUniqueID() const { return UniqueID; }
-  bool isInPCH() const { return InPCH; }
   time_t getModificationTime() const { return ModTime; }
 
   /// Return the directory the file lives in.
@@ -108,8 +106,6 @@ public:
   bool isOpenForTests() const { return File != nullptr; }
 };
 
-struct FileData;
-
 /// Implements support for file system lookup, file system caching,
 /// and directory search management.
 ///
@@ -168,7 +164,7 @@ class FileManager : public RefCountedBas
   // Caching.
   std::unique_ptr StatCache;
 
-  bool getStatValue(StringRef Path, FileData &Data, bool isFile,
+  bool getStatValue(StringRef Path, llvm::vfs::Status &Status, bool isFile,
 std::unique_ptr *F);
 
   /// Add all ancestors of the given path (pointing to either a file

Modified: cfe/trunk/include/clang/Basic/FileSystemStatCache.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FileSystemStatCache.h?rev=355368&r1=355367&r2=355368&view=diff
==
--- cfe/trunk/include/clang/Basic/FileSystemStatCache.h (original)
+++ cfe/trunk/include/clang/Basic/FileSystemStatCache.h Mon Mar  4 18:27:12 2019
@@ -19,40 +19,15 @@
 #include "llvm/ADT/StringRef.h"
 #include "llvm/Support/Allocator.h"
 #include "llvm/Support/FileSystem.h"
+#include "llvm/Support/VirtualFileSystem.h"
 #include 
 #include 
 #include 
 #include 
 #include 
 
-namespace llvm {
-
-namespace vfs {
-
-class File;
-class FileSystem;
-
-} // namespace vfs
-} // namespace llvm
-
 namespace clang {
 
-// FIXME: should probably replace this with vfs::Status
-struct FileData {
-  std::string Name;
-  uint64_t Size = 0;
-  time_t ModTime = 0;
-  llvm::sys::fs::UniqueID UniqueID;
-  bool IsDirectory = false;
-  bool IsNamedPipe = false;
-  bool InPCH = false;
-
-  // FIXME: remove this when files support multiple names
-  bool IsVFSMapped = false;
-
-  FileData() = default;
-};
-
 /// Abstract interface for introducing a FileManager cache for 'stat'
 /// system calls, which is used by precompiled and pretokenized headers to
 /// improve performance.
@@ -80,7 +55,7 @@ public:
   /// success for directories (not files).  On a successful file lookup, the
   /// implementation can optionally fill in \p F with a valid \p File object 
and
   /// the client guarantees that it will close it.
-  static bool get(StringRef Path, FileData &Data, bool isFile,
+  static bool get(StringRef Path, llvm::vfs::Status &Status, bool isFile,
   std::unique_ptr *F,
   FileSystemStatCache *Cache, llvm::vfs::FileSystem &FS);
 
@@ -88,7 +63,8 @@ protected:
   // FIXME: The pointer here is a non-owning/optional reference to the
   // unique_ptr. Optional&> might be nicer, but
   // Optional needs some work to support references so this isn't possible yet.
-  virtual LookupResult getStat(StringRef Path, FileData &Data, bool isFile,
+  virtual LookupResult getStat(StringRef Path, llvm::vfs::Status &Status,
+   bool isFile,
std::unique_ptr *F,

[PATCH] D57965: Clean up ObjCPropertyDecl printing

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 189317.
dgoldman added a comment.

- Add more tests and improved printing of pointers


Repository:
  rC Clang

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

https://reviews.llvm.org/D57965

Files:
  lib/AST/DeclPrinter.cpp
  test/AST/ast-print-objc-property.m
  test/Index/comment-objc-decls.m
  test/Index/comment-unqualified-objc-pointer.m
  test/PCH/chain-remap-types.m

Index: test/PCH/chain-remap-types.m
===
--- test/PCH/chain-remap-types.m
+++ test/PCH/chain-remap-types.m
@@ -6,7 +6,7 @@
 
 // CHECK: @class X;
 // CHECK: struct Y 
-// CHECK: @property ( assign,readwrite,atomic ) X * prop
+// CHECK: @property(atomic, assign, unsafe_unretained, readwrite) X * prop
 // CHECK: void h(X *);
 // CHECK: @interface X(Blah)
 // CHECK: void g(X *);
Index: test/Index/comment-unqualified-objc-pointer.m
===
--- test/Index/comment-unqualified-objc-pointer.m
+++ test/Index/comment-unqualified-objc-pointer.m
@@ -19,7 +19,7 @@
 
 //! This is a property to get the Name.
 @property (copy) NSString *Name;
-// CHECK: @property(readwrite, copy, atomic) NSString *Name;
+// CHECK: @property(atomic, copy, readwrite) NSString *Name;
 @end
 
 @implementation NSMutableArray
Index: test/Index/comment-objc-decls.m
===
--- test/Index/comment-objc-decls.m
+++ test/Index/comment-objc-decls.m
@@ -32,7 +32,7 @@
 @end
 // CHECK: @protocol MyProto\n@end
 // CHECK: - (unsigned int)MethodMyProto:(nullable id)anObject inRange:(unsigned int)range;
-// CHECK: @optional\n@property(readwrite, copy, atomic, nonnull) id PropertyMyProto;
+// CHECK: @optional\n@property(atomic, copy, readwrite, nonnull) id PropertyMyProto;
 // CHECK: + (id)ClassMethodMyProto;
 
 /**
@@ -77,7 +77,7 @@
 // CHECK: id IvarMyClass
 // CHECK: - (id)MethodMyClass;
 // CHECK: + (id)ClassMethodMyClass;
-// CHECK: @property(readwrite, copy, atomic) id PropertyMyClass;@property(atomic, copy, readwrite) id PropertyMyClass;@interface MyClass (Category)\n@end
 // CHECK: - (void)MethodMyClassCategory;
-// CHECK: @property(readwrite, copy, atomic) id PropertyMyClassCategory;
+// CHECK: @property(atomic, copy, readwrite) id PropertyMyClassCategory;
 // CHECK: - (id)PropertyMyClassCategory;
 // CHECK: - (void)setPropertyMyClassCategory:(id)arg;
 
Index: test/AST/ast-print-objc-property.m
===
--- /dev/null
+++ test/AST/ast-print-objc-property.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -ast-print %s -o - | FileCheck %s
+
+@interface NSObject
+@end
+
+@interface Properties : NSObject
+@property(class) int classFoo;
+@property(nonatomic) int atomicBar;
+@property(readonly) int readonlyConstant;
+@property(retain, nonatomic, setter=my_setter:, getter=my_getter) id   __crazy_name;
+@property(nonatomic, strong, nullable) NSObject *   objProperty;
+@property(nonatomic, weak, null_resettable) NSObject *   weakObj;
+@property(nonatomic, copy, nonnull) NSObject * copyObj;
+@end
+
+// CHECK: @property(class, atomic, assign, unsafe_unretained, readwrite) int classFoo;
+// CHECK: @property(nonatomic, assign, unsafe_unretained, readwrite) int atomicBar;
+// CHECK: @property(atomic, readonly) int readonlyConstant;
+// CHECK: @property(nonatomic, retain, readwrite, getter = my_getter, setter = my_setter:) id __crazy_name;
+// CHECK: @property(nonatomic, strong, readwrite, nullable) NSObject *objProperty;
+// CHECK: @property(nonatomic, weak, readwrite, null_resettable) NSObject *weakObj;
+// CHECK: @property(nonatomic, copy, readwrite, nonnull) NSObject *copyObj;
Index: lib/AST/DeclPrinter.cpp
===
--- lib/AST/DeclPrinter.cpp
+++ lib/AST/DeclPrinter.cpp
@@ -1389,6 +1389,13 @@
 
 /// PrintObjCPropertyDecl - print a property declaration.
 ///
+/// Print attributes in the following order:
+/// - class
+/// - nonatomic | atomic
+/// - assign | retain | strong | copy | weak | unsafe_unretained
+/// - readwrite | readonly
+/// - getter & setter
+/// - nullability
 void DeclPrinter::VisitObjCPropertyDecl(ObjCPropertyDecl *PDecl) {
   if (PDecl->getPropertyImplementation() == ObjCPropertyDecl::Required)
 Out << "@required\n";
@@ -1400,58 +1407,69 @@
   Out << "@property";
   if (PDecl->getPropertyAttributes() != ObjCPropertyDecl::OBJC_PR_noattr) {
 bool first = true;
-Out << " (";
-if (PDecl->getPropertyAttributes() &
-ObjCPropertyDecl::OBJC_PR_readonly) {
-  Out << (first ? ' ' : ',') << "readonly";
+Out << "(";
+if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_class) {
+  Out << (first ? "" : ", ") << "class";
   first = false;
 }
 
-if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_getter) {
-  Out << (first ? ' ' : ',') << "

[PATCH] D57965: Clean up ObjCPropertyDecl printing

[David Goldman via Phabricator via cfe-commits]

dgoldman updated this revision to Diff 189318.
dgoldman added a comment.

- Fix broken test by last change


Repository:
  rC Clang

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

https://reviews.llvm.org/D57965

Files:
  lib/AST/DeclPrinter.cpp
  test/AST/ast-print-objc-property.m
  test/Index/comment-objc-decls.m
  test/Index/comment-unqualified-objc-pointer.m
  test/PCH/chain-remap-types.m

Index: test/PCH/chain-remap-types.m
===
--- test/PCH/chain-remap-types.m
+++ test/PCH/chain-remap-types.m
@@ -6,7 +6,7 @@
 
 // CHECK: @class X;
 // CHECK: struct Y 
-// CHECK: @property ( assign,readwrite,atomic ) X * prop
+// CHECK: @property(atomic, assign, unsafe_unretained, readwrite) X *prop
 // CHECK: void h(X *);
 // CHECK: @interface X(Blah)
 // CHECK: void g(X *);
Index: test/Index/comment-unqualified-objc-pointer.m
===
--- test/Index/comment-unqualified-objc-pointer.m
+++ test/Index/comment-unqualified-objc-pointer.m
@@ -19,7 +19,7 @@
 
 //! This is a property to get the Name.
 @property (copy) NSString *Name;
-// CHECK: @property(readwrite, copy, atomic) NSString *Name;
+// CHECK: @property(atomic, copy, readwrite) NSString *Name;
 @end
 
 @implementation NSMutableArray
Index: test/Index/comment-objc-decls.m
===
--- test/Index/comment-objc-decls.m
+++ test/Index/comment-objc-decls.m
@@ -32,7 +32,7 @@
 @end
 // CHECK: @protocol MyProto\n@end
 // CHECK: - (unsigned int)MethodMyProto:(nullable id)anObject inRange:(unsigned int)range;
-// CHECK: @optional\n@property(readwrite, copy, atomic, nonnull) id PropertyMyProto;
+// CHECK: @optional\n@property(atomic, copy, readwrite, nonnull) id PropertyMyProto;
 // CHECK: + (id)ClassMethodMyProto;
 
 /**
@@ -77,7 +77,7 @@
 // CHECK: id IvarMyClass
 // CHECK: - (id)MethodMyClass;
 // CHECK: + (id)ClassMethodMyClass;
-// CHECK: @property(readwrite, copy, atomic) id PropertyMyClass;@property(atomic, copy, readwrite) id PropertyMyClass;@interface MyClass (Category)\n@end
 // CHECK: - (void)MethodMyClassCategory;
-// CHECK: @property(readwrite, copy, atomic) id PropertyMyClassCategory;
+// CHECK: @property(atomic, copy, readwrite) id PropertyMyClassCategory;
 // CHECK: - (id)PropertyMyClassCategory;
 // CHECK: - (void)setPropertyMyClassCategory:(id)arg;
 
Index: test/AST/ast-print-objc-property.m
===
--- /dev/null
+++ test/AST/ast-print-objc-property.m
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 -ast-print %s -o - | FileCheck %s
+
+@interface NSObject
+@end
+
+@interface Properties : NSObject
+@property(class) int classFoo;
+@property(nonatomic) int atomicBar;
+@property(readonly) int readonlyConstant;
+@property(retain, nonatomic, setter=my_setter:, getter=my_getter) id   __crazy_name;
+@property(nonatomic, strong, nullable) NSObject *   objProperty;
+@property(nonatomic, weak, null_resettable) NSObject *   weakObj;
+@property(nonatomic, copy, nonnull) NSObject * copyObj;
+@end
+
+// CHECK: @property(class, atomic, assign, unsafe_unretained, readwrite) int classFoo;
+// CHECK: @property(nonatomic, assign, unsafe_unretained, readwrite) int atomicBar;
+// CHECK: @property(atomic, readonly) int readonlyConstant;
+// CHECK: @property(nonatomic, retain, readwrite, getter = my_getter, setter = my_setter:) id __crazy_name;
+// CHECK: @property(nonatomic, strong, readwrite, nullable) NSObject *objProperty;
+// CHECK: @property(nonatomic, weak, readwrite, null_resettable) NSObject *weakObj;
+// CHECK: @property(nonatomic, copy, readwrite, nonnull) NSObject *copyObj;
Index: lib/AST/DeclPrinter.cpp
===
--- lib/AST/DeclPrinter.cpp
+++ lib/AST/DeclPrinter.cpp
@@ -1389,6 +1389,13 @@
 
 /// PrintObjCPropertyDecl - print a property declaration.
 ///
+/// Print attributes in the following order:
+/// - class
+/// - nonatomic | atomic
+/// - assign | retain | strong | copy | weak | unsafe_unretained
+/// - readwrite | readonly
+/// - getter & setter
+/// - nullability
 void DeclPrinter::VisitObjCPropertyDecl(ObjCPropertyDecl *PDecl) {
   if (PDecl->getPropertyImplementation() == ObjCPropertyDecl::Required)
 Out << "@required\n";
@@ -1400,58 +1407,69 @@
   Out << "@property";
   if (PDecl->getPropertyAttributes() != ObjCPropertyDecl::OBJC_PR_noattr) {
 bool first = true;
-Out << " (";
-if (PDecl->getPropertyAttributes() &
-ObjCPropertyDecl::OBJC_PR_readonly) {
-  Out << (first ? ' ' : ',') << "readonly";
+Out << "(";
+if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_class) {
+  Out << (first ? "" : ", ") << "class";
   first = false;
 }
 
-if (PDecl->getPropertyAttributes() & ObjCPropertyDecl::OBJC_PR_getter) {
-  Out << (first ? ' ' : ',') << "getter = ";
-  

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

[Hans Wennborg via Phabricator via cfe-commits]

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



Comment at: clang/lib/CodeGen/CGStmt.cpp:1852
+IntResult =
+llvm::APSInt::get(EVResult.Val.getLValueOffset().getQuantity());
+  else

efriedma wrote:
> hans wrote:
> > efriedma wrote:
> > > This always returns an APSInt with width 64; is that really right?  I 
> > > guess it might not really matter given that it's only going to be used as 
> > > an immediate constant anyway, but it seems weird.
> > I agree it seems a little strange, but I think in practice it's correct. 
> > EVResult.Val.getLValueOffset().getQuantity() returns an int64_t, so we're 
> > not losing any data.
> > 
> > The code that I lifted this from, is using the bitwidth of the casted-to 
> > integer type for the result. But it's still only got maximum 64 bits since 
> > the source, getLValueOffset().getQuantity(), is the same.
> The concern isn't that we would lose data.   I'm more concerned the backend 
> might not be prepared for a value of the "wrong" width.
Oh, I see. I'll change the code to use ASTContext::MakeIntValue with the source 
type. Apparently this works even if the source type is a pointer type; I guess 
that yields an integer of the same width. I think maybe that's the best we can 
do?



Comment at: clang/lib/Sema/SemaStmtAsm.cpp:399
+  IntResult =
+  llvm::APSInt::get(EVResult.Val.getLValueOffset().getQuantity());
+else

efriedma wrote:
> hans wrote:
> > efriedma wrote:
> > > I think it makes sense to add a method to APValue specifically to do the 
> > > conversion from LValue to an APSInt, whether or not isNullPointer() is 
> > > true, and use it both here and in IntExprEvaluator::VisitCastExpr in 
> > > lib/AST/ExprConstant.cpp.  The logic is sort of subtle (and I'm not 
> > > completely sure it's right for targets where null is not zero, but you 
> > > shouldn't try to fix that here).
> > I agree (and this was also Bill's suggestion above) that it would be nice 
> > to have a utility method for this.
> > 
> > I'm not sure adding one to APValue would work for 
> > IntExprEvaluator::VisitCastExpr though, since that code is actually using 
> > its own LValue class, not an APValue until it's time to return a result.
> > 
> > I frankly also doesn't fully understand what that code is doing. If the 
> > LValue has a base value, it seems to just take that as result and ignore 
> > any offset? This is unknown territory to me, but the way I read it, if 
> > there's an lvalue base, the expression isn't going to come out as an 
> > integer constant. I think.
> > 
> > About null pointers, I'm calling getTargetNullPointerValue() so I think 
> > that should be okay, no?
> Oh, I didn't realize IntExprEvaluator::VisitCastExpr wasn't using the same 
> class to represent the value; that makes it harder to usefully refactor.  But 
> still, it would be good to reduce the duplicated code between here and 
> CodeGen.
> 
> > If the LValue has a base value, it seems to just take that as result and 
> > ignore any offset?
> 
> If there's a base value, it returns the whole LValue, including the base and 
> offset.
> 
> > I'm calling getTargetNullPointerValue() so I think that should be okay
> 
> The issue would be the case where you have a null pointer with an offset, 
> like the case in the bug.  It's sort of inconsistent if null==-1, but 
> null+1==1.  But it's not something we handle consistently elsewhere, anyway, 
> so I guess we can ignore it for now.
Ah, the null pointer issue is interesting, but like you say we don't seem to 
handle this in the cast code I was inspired by here either.


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

https://reviews.llvm.org/D58821



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

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

[Hans Wennborg via Phabricator via cfe-commits]

hans updated this revision to Diff 189319.
hans added a comment.

Extract code to a new method in APValue.

Eli, what do you think about something like this? Suggestions for better name 
welcome :-)


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

https://reviews.llvm.org/D58821

Files:
  clang/include/clang/AST/APValue.h
  clang/lib/AST/APValue.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/CodeGen/CGStmt.cpp
  clang/lib/Sema/SemaStmtAsm.cpp
  clang/test/CodeGen/x86-64-inline-asm.c
  clang/test/Sema/inline-asm-validate-x86.c

Index: clang/test/Sema/inline-asm-validate-x86.c
===
--- clang/test/Sema/inline-asm-validate-x86.c
+++ clang/test/Sema/inline-asm-validate-x86.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i686 -fsyntax-only -verify %s
-// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify %s
+// RUN: %clang_cc1 -triple x86_64 -fsyntax-only -verify -DAMD64 %s
 
 void I(int i, int j) {
   static const int BelowMin = -1;
@@ -137,3 +137,21 @@
   : "0"(i), "O"(64)); // expected-no-error
 }
 
+void pr40890(void) {
+  struct s {
+int a, b;
+  };
+  static struct s s;
+  // This null pointer can be used as an integer constant expression.
+  __asm__ __volatile__("\n#define S_A abcd%0\n" : : "n"(&((struct s*)0)->a));
+  // This offset-from-null pointer can be used as an integer constant expression.
+  __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
+  // This pointer cannot be used as an integer constant expression.
+  __asm__ __volatile__("\n#define GLOBAL_A abcd%0\n" : : "n"(&s.a)); // expected-error{{constraint 'n' expects an integer constant expression}}
+  // Floating-point is also not okay.
+  __asm__ __volatile__("\n#define PI abcd%0\n" : : "n"(3.14f)); // expected-error{{constraint 'n' expects an integer constant expression}}
+#ifdef AMD64
+  // This arbitrary pointer is fine.
+  __asm__ __volatile__("\n#define BEEF abcd%0\n" : : "n"((int*)0xdeadbeef));
+#endif
+}
Index: clang/test/CodeGen/x86-64-inline-asm.c
===
--- clang/test/CodeGen/x86-64-inline-asm.c
+++ clang/test/CodeGen/x86-64-inline-asm.c
@@ -1,6 +1,7 @@
 // REQUIRES: x86-registered-target
 // RUN: %clang_cc1 -triple x86_64 %s -S -o /dev/null -DWARN -verify
 // RUN: %clang_cc1 -triple x86_64 %s -S -o /dev/null -Werror -verify
+// RUN: %clang_cc1 -triple x86_64-linux-gnu %s -S -o - | FileCheck %s
 void f() {
   asm("movaps %xmm3, (%esi, 2)");
 // expected-note@1 {{instantiated into assembly here}}
@@ -15,3 +16,17 @@
 void g(void) { asm volatile("movd %%xmm0, %0"
 :
 : "m"(var)); }
+
+void pr40890(void) {
+  struct s {
+int a, b;
+  } s;
+  __asm__ __volatile__("\n#define S_A abcd%0\n" : : "n"(&((struct s*)0)->a));
+  __asm__ __volatile__("\n#define S_B abcd%0\n" : : "n"(&((struct s*)0)->b));
+  __asm__ __volatile__("\n#define BEEF abcd%0\n" : : "n"((int*)0xdeadbeef));
+
+// CHECK-LABEL: pr40890
+// CHECK: #define S_A abcd$0
+// CHECK: #define S_B abcd$4
+// CHECK: #define BEEF abcd$244837814038255
+}
Index: clang/lib/Sema/SemaStmtAsm.cpp
===
--- clang/lib/Sema/SemaStmtAsm.cpp
+++ clang/lib/Sema/SemaStmtAsm.cpp
@@ -385,11 +385,20 @@
   return StmtError(
   Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
   << Info.getConstraintStr() << InputExpr->getSourceRange());
-llvm::APSInt Result = EVResult.Val.getInt();
-if (!Info.isValidAsmImmediate(Result))
+
+// For compatibility with GCC, we also allow pointers that would be
+// integral constant expressions if they were cast to int.
+llvm::APSInt IntResult;
+if (!EVResult.Val.toIntegralConstant(IntResult, InputExpr->getType(),
+ Context))
+  return StmtError(
+  Diag(InputExpr->getBeginLoc(), diag::err_asm_immediate_expected)
+  << Info.getConstraintStr() << InputExpr->getSourceRange());
+
+if (!Info.isValidAsmImmediate(IntResult))
   return StmtError(Diag(InputExpr->getBeginLoc(),
 diag::err_invalid_asm_value_for_constraint)
-   << Result.toString(10) << Info.getConstraintStr()
+   << IntResult.toString(10) << Info.getConstraintStr()
<< InputExpr->getSourceRange());
   }
 
Index: clang/lib/CodeGen/CGStmt.cpp
===
--- clang/lib/CodeGen/CGStmt.cpp
+++ clang/lib/CodeGen/CGStmt.cpp
@@ -1838,8 +1838,15 @@
   // (immediate or symbolic), try to emit it as such.
   if (!Info.allowsRegister() && !Info.allowsMemory()) {
 if (Info.requiresImmediateConstant()) {
-  llvm::APSInt AsmConst = InputExpr->EvaluateKnownConstInt(getCont

[PATCH] D58966: [libc++] Fix failures on GCC

[Louis Dionne via Phabricator via cfe-commits]

ldionne created this revision.
ldionne added a reviewer: EricWF.
Herald added a reviewer: jfb.
Herald added subscribers: cfe-commits, jdoerfert, jfb, dexonsmith, jkorous, 
christof.
Herald added a project: clang.

In https://reviews.llvm.org/D58201, we turned memory_order into an enum
class in C++20 mode. However, we were not casting memory_order to its
underlying type correctly for the GCC implementation, which broke the
build bots. I also fixed a test that was failing in C++17 mode on GCC 5.


Repository:
  rC Clang

https://reviews.llvm.org/D58966

Files:
  libcxx/include/atomic
  libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp

Index: libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp
===
--- libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp
+++ libcxx/test/std/atomics/atomics.lockfree/isalwayslockfree.pass.cpp
@@ -39,8 +39,8 @@
   class LLong = long long,
   class ULLong = unsigned long long>
 void checkLongLongTypes() {
-  static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_LLONG_LOCK_FREE));
-  static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_LLONG_LOCK_FREE));
+  static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_LLONG_LOCK_FREE), "");
+  static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_LLONG_LOCK_FREE), "");
 }
 
 // Used to make the calls to __atomic_always_lock_free dependent on a template
@@ -116,22 +116,22 @@
 CHECK_ALWAYS_LOCK_FREE(union IntFloat { int i; float f; });
 
 // C macro and static constexpr must be consistent.
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_BOOL_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_CHAR16_T_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_CHAR32_T_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_WCHAR_T_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_SHORT_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_SHORT_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_INT_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_INT_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_LONG_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_LONG_LOCK_FREE));
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_BOOL_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_CHAR_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_CHAR16_T_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_CHAR32_T_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_WCHAR_T_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_SHORT_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_SHORT_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_INT_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_INT_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_LONG_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_LONG_LOCK_FREE), "");
 checkLongLongTypes();
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_POINTER_LOCK_FREE));
-static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_POINTER_LOCK_FREE));
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_POINTER_LOCK_FREE), "");
+static_assert(std::atomic::is_always_lock_free == (2 == ATOMIC_POINTER_LOCK_FREE), "");
 }
 
 int main(int, char**) { run(); return 0; }
Index: libcxx/include/atomic
===
--- libcxx/include/atomic
+++ libcxx/include/atomic
@@ -909,13 +909,13 @@
 #define __cxx_atomic_is_lock_free(__s) __c11_atomic_is_lock_free(__s)
 
 _LIBCPP_INLINE_VISIBILITY inline
-void __cxx_atomic_thread_fence(int __order) {
-__c11_atomic_thread_fence(__order);
+void __cxx_atomic_thread_fence(memory_order __order) {
+__c11_atomic_thread_fence(static_cast<__memory_order_underlying_t>(__order));
 }
 
 _LIBCPP_INLINE_VISIBILITY inline
-void __cxx_atomic_signal_fence(int __order) {
-__c11_atomic_signa

r355410 - [HIP] Do not unbundle object files for -fno-gpu-rdc

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Mar  5 08:07:56 2019
New Revision: 355410

URL: http://llvm.org/viewvc/llvm-project?rev=355410&view=rev
Log:
[HIP] Do not unbundle object files for -fno-gpu-rdc

When -fno-gpu-rdc is set, device code is compiled, linked, and assembled into 
fat binary
and embedded as string in object files. The object files are normal object 
files which
can be linked by host linker. In the linking stage, the object files should not 
be unbundled
when -fno-gpu-rdc is set since they are normal object files, not bundles. The 
object files
only need to be unbundled when -fgpu-rdc is set.

Currently clang always unbundles object files, disregarding -fgpu-rdc option.

This patch fixes that.

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

Modified:
cfe/trunk/lib/Driver/Driver.cpp
cfe/trunk/test/Driver/hip-binding.hip
cfe/trunk/test/Driver/hip-link-shared-library.hip

Modified: cfe/trunk/lib/Driver/Driver.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/Driver.cpp?rev=355410&r1=355409&r2=355410&view=diff
==
--- cfe/trunk/lib/Driver/Driver.cpp (original)
+++ cfe/trunk/lib/Driver/Driver.cpp Tue Mar  5 08:07:56 2019
@@ -2293,6 +2293,9 @@ class OffloadingActionBuilder final {
 
 /// Flag that is set to true if this builder acted on the current input.
 bool IsActive = false;
+
+/// Flag for -fgpu-rdc.
+bool Relocatable = false;
   public:
 CudaActionBuilderBase(Compilation &C, DerivedArgList &Args,
   const Driver::InputList &Inputs,
@@ -2338,6 +2341,12 @@ class OffloadingActionBuilder final {
 
   // If this is an unbundling action use it as is for each CUDA toolchain.
   if (auto *UA = dyn_cast(HostAction)) {
+
+// If -fgpu-rdc is disabled, should not unbundle since there is no
+// device code to link.
+if (!Relocatable)
+  return ABRT_Inactive;
+
 CudaDeviceActions.clear();
 auto *IA = cast(UA->getInputs().back());
 std::string FileName = IA->getInputArg().getAsString(Args);
@@ -2409,6 +2418,9 @@ class OffloadingActionBuilder final {
   !C.hasOffloadToolChain())
 return false;
 
+  Relocatable = Args.hasFlag(options::OPT_fgpu_rdc,
+  options::OPT_fno_gpu_rdc, /*Default=*/false);
+
   const ToolChain *HostTC = 
C.getSingleOffloadToolChain();
   assert(HostTC && "No toolchain for host compilation.");
   if (HostTC->getTriple().isNVPTX() ||
@@ -2594,13 +2606,11 @@ class OffloadingActionBuilder final {
   class HIPActionBuilder final : public CudaActionBuilderBase {
 /// The linker inputs obtained for each device arch.
 SmallVector DeviceLinkerInputs;
-bool Relocatable;
 
   public:
 HIPActionBuilder(Compilation &C, DerivedArgList &Args,
  const Driver::InputList &Inputs)
-: CudaActionBuilderBase(C, Args, Inputs, Action::OFK_HIP),
-  Relocatable(false) {}
+: CudaActionBuilderBase(C, Args, Inputs, Action::OFK_HIP) {}
 
 bool canUseBundlerUnbundler() const override { return true; }
 
@@ -2705,13 +2715,6 @@ class OffloadingActionBuilder final {
 ++I;
   }
 }
-
-bool initialize() override {
-  Relocatable = Args.hasFlag(options::OPT_fgpu_rdc,
-  options::OPT_fno_gpu_rdc, /*Default=*/false);
-
-  return CudaActionBuilderBase::initialize();
-}
   };
 
   /// OpenMP action builder. The host bitcode is passed to the device frontend

Modified: cfe/trunk/test/Driver/hip-binding.hip
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/hip-binding.hip?rev=355410&r1=355409&r2=355410&view=diff
==
--- cfe/trunk/test/Driver/hip-binding.hip (original)
+++ cfe/trunk/test/Driver/hip-binding.hip Tue Mar  5 08:07:56 2019
@@ -4,7 +4,7 @@
 
 // RUN: touch %t.o
 // RUN: %clang --hip-link -ccc-print-bindings -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %t.o\
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 -fgpu-rdc %t.o\
 // RUN: 2>&1 | FileCheck %s
 
 // CHECK: # "amdgcn-amd-amdhsa" - "offload bundler", inputs: ["[[IN:.*o]]"], 
outputs: ["[[OBJ1:.*o]]", "[[OBJ2:.*o]]", "[[OBJ3:.*o]]"] 
@@ -13,3 +13,10 @@
 // CHECK: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[OBJ3]]"], 
output: "[[IMG3:.*out]]"
 // CHECK-NOT: offload bundler
 // CHECK: # "x86_64-unknown-linux-gnu" - "GNU::Linker", inputs: ["[[OBJ1]]", 
"[[IMG2]]", "[[IMG3]]"], output: "a.out"
+
+// RUN: %clang --hip-link -ccc-print-bindings -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %t.o\
+// RUN: 2>&1 | FileCheck -check-prefix=NORDC %s
+
+// NORDC-NOT: offload bundler
+// NORDC: # "x86_64-unknown-linux-gnu" - "GNU::Linker", inputs: ["{{.*o}}"], 
output: "a.out"

Modified: cfe/trunk/test/Driver/hip-link-shared-library.hip
URL: 
http://llvm.org

[PATCH] D58917: [HIP] Do not unbundle object files for -fno-gpu-rdc

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355410: [HIP] Do not unbundle object files for -fno-gpu-rdc 
(authored by yaxunl, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58917?vs=189179&id=189325#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58917

Files:
  cfe/trunk/lib/Driver/Driver.cpp
  cfe/trunk/test/Driver/hip-binding.hip
  cfe/trunk/test/Driver/hip-link-shared-library.hip

Index: cfe/trunk/lib/Driver/Driver.cpp
===
--- cfe/trunk/lib/Driver/Driver.cpp
+++ cfe/trunk/lib/Driver/Driver.cpp
@@ -2293,6 +2293,9 @@
 
 /// Flag that is set to true if this builder acted on the current input.
 bool IsActive = false;
+
+/// Flag for -fgpu-rdc.
+bool Relocatable = false;
   public:
 CudaActionBuilderBase(Compilation &C, DerivedArgList &Args,
   const Driver::InputList &Inputs,
@@ -2338,6 +2341,12 @@
 
   // If this is an unbundling action use it as is for each CUDA toolchain.
   if (auto *UA = dyn_cast(HostAction)) {
+
+// If -fgpu-rdc is disabled, should not unbundle since there is no
+// device code to link.
+if (!Relocatable)
+  return ABRT_Inactive;
+
 CudaDeviceActions.clear();
 auto *IA = cast(UA->getInputs().back());
 std::string FileName = IA->getInputArg().getAsString(Args);
@@ -2409,6 +2418,9 @@
   !C.hasOffloadToolChain())
 return false;
 
+  Relocatable = Args.hasFlag(options::OPT_fgpu_rdc,
+  options::OPT_fno_gpu_rdc, /*Default=*/false);
+
   const ToolChain *HostTC = C.getSingleOffloadToolChain();
   assert(HostTC && "No toolchain for host compilation.");
   if (HostTC->getTriple().isNVPTX() ||
@@ -2594,13 +2606,11 @@
   class HIPActionBuilder final : public CudaActionBuilderBase {
 /// The linker inputs obtained for each device arch.
 SmallVector DeviceLinkerInputs;
-bool Relocatable;
 
   public:
 HIPActionBuilder(Compilation &C, DerivedArgList &Args,
  const Driver::InputList &Inputs)
-: CudaActionBuilderBase(C, Args, Inputs, Action::OFK_HIP),
-  Relocatable(false) {}
+: CudaActionBuilderBase(C, Args, Inputs, Action::OFK_HIP) {}
 
 bool canUseBundlerUnbundler() const override { return true; }
 
@@ -2705,13 +2715,6 @@
 ++I;
   }
 }
-
-bool initialize() override {
-  Relocatable = Args.hasFlag(options::OPT_fgpu_rdc,
-  options::OPT_fno_gpu_rdc, /*Default=*/false);
-
-  return CudaActionBuilderBase::initialize();
-}
   };
 
   /// OpenMP action builder. The host bitcode is passed to the device frontend
Index: cfe/trunk/test/Driver/hip-binding.hip
===
--- cfe/trunk/test/Driver/hip-binding.hip
+++ cfe/trunk/test/Driver/hip-binding.hip
@@ -4,7 +4,7 @@
 
 // RUN: touch %t.o
 // RUN: %clang --hip-link -ccc-print-bindings -target x86_64-linux-gnu \
-// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %t.o\
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 -fgpu-rdc %t.o\
 // RUN: 2>&1 | FileCheck %s
 
 // CHECK: # "amdgcn-amd-amdhsa" - "offload bundler", inputs: ["[[IN:.*o]]"], outputs: ["[[OBJ1:.*o]]", "[[OBJ2:.*o]]", "[[OBJ3:.*o]]"] 
@@ -13,3 +13,10 @@
 // CHECK: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[OBJ3]]"], output: "[[IMG3:.*out]]"
 // CHECK-NOT: offload bundler
 // CHECK: # "x86_64-unknown-linux-gnu" - "GNU::Linker", inputs: ["[[OBJ1]]", "[[IMG2]]", "[[IMG3]]"], output: "a.out"
+
+// RUN: %clang --hip-link -ccc-print-bindings -target x86_64-linux-gnu \
+// RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %t.o\
+// RUN: 2>&1 | FileCheck -check-prefix=NORDC %s
+
+// NORDC-NOT: offload bundler
+// NORDC: # "x86_64-unknown-linux-gnu" - "GNU::Linker", inputs: ["{{.*o}}"], output: "a.out"
Index: cfe/trunk/test/Driver/hip-link-shared-library.hip
===
--- cfe/trunk/test/Driver/hip-link-shared-library.hip
+++ cfe/trunk/test/Driver/hip-link-shared-library.hip
@@ -1,7 +1,7 @@
 // RUN: touch %t.o
 // RUN: %clang --hip-link -ccc-print-bindings -target x86_64-linux-gnu \
 // RUN:   --cuda-gpu-arch=gfx803 --cuda-gpu-arch=gfx900 %t.o %S/Inputs/in.so \
-// RUN: 2>&1 | FileCheck %s
+// RUN:   -fgpu-rdc 2>&1 | FileCheck %s
 
 // CHECK: # "amdgcn-amd-amdhsa" - "offload bundler", inputs: ["[[IN:.*o]]"], outputs: ["[[OBJ1:.*o]]", "[[OBJ2:.*o]]", "[[OBJ3:.*o]]"]
 // CHECK: # "amdgcn-amd-amdhsa" - "AMDGCN::Linker", inputs: ["[[OBJ2]]"], output: "[[IMG2:.*out]]"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58320: [Darwin] Introduce a new flag, -fapple-link-rtlib that forces linking of the builtins library.

[Peter Smith via Phabricator via cfe-commits]

peter.smith added a comment.

I've no objections to adding the command line as a Darwin only option. 
Implementation looks fine to me although I've not got any prior experience with 
Darwin.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58320



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

[PATCH] D58556: [LibTooling] Add "smart" retrieval of AST-node source to FixIt library

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel marked 5 inline comments as done.
ymandel added inline comments.



Comment at: clang/include/clang/Tooling/FixIt.h:73
+// context. In contrast with \p getText(), this function selects a source range
+// "automatically", extracting text that a reader might intuitively associate
+// with a node.  Currently, only specialized for \p clang::Stmt, where it will

kimgr wrote:
> ymandel wrote:
> > ilya-biryukov wrote:
> > > What are other tricky cases you have in mind for the future?
> > I just assumed that we'd hit more as we dig into them, but, I'm not so sure 
> > now.  The two others I can think of offhand are 1) extending to include 
> > associated comments, 2) semicolons after declarations.  Commas present a 
> > similar challenge (if a bit simpler) when used in a list (vs. the comma 
> > operator).  Are there any other separators in C++? 
> > 
> > At a higher level, it would be nice to align this with your work on tree 
> > transformations. That is, think of these functions as short-term shims to 
> > simulate the behavior we'll ultimately get from that new library. However, 
> > it might be premature to consider those details here.
> Peanut gallery comment on this:
> 
> > The two others I can think of offhand are
> > 1) extending to include associated comments,
> > 2) semicolons after declarations.
> > Commas present a similar challenge (if a bit simpler) when used in a list 
> > (vs. the comma operator).
> > Are there any other separators in C++?
> 
> Would it make sense to let callers choose what level of expansion they want 
> with a flag mask? Somehow I think that makes it easier to name the function, 
> too, e.g.:
> ```
> StringRef getExpandedRange(const Stmt &S, ASTContext &Context, ExpansionFlags 
> Flags);
> ```
> 
Yes, I think that's a good idea. I even like the name except that it will 
likely be confused with Expansion locs.  Maybe `getExtendedRange`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58556



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

[PATCH] D58634: [PR40778] Generate address space conversion when binding reference to a temporary value in different address space

[John McCall via Phabricator via cfe-commits]

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

This test file will probably grow over time, so please add a CHECK-LABEL line 
to the test case to make sure you're checking the body of foo().  Also, you'll 
probably need to allow the name `%ref.tmp` to be stripped for this test to pass 
in release builds; please go ahead and test a more complete pattern, including 
the alloca and the store of 1 to it.  Otherwise LGTM.


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

https://reviews.llvm.org/D58634



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

[PATCH] D58977: [clang-tidy] Add the abseil-time-comparison check

[Hyrum Wright via Phabricator via cfe-commits]

hwright created this revision.
hwright added reviewers: hokein, ioeric.
Herald added subscribers: cfe-commits, jdoerfert, xazax.hun, mgorny.
Herald added a project: clang.

This is an analog of the `abseil-duration-comparison` check, but for the 
`absl::Time` domain.  It has a similar implementation and tests.


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58977

Files:
  clang-tidy/abseil/AbseilTidyModule.cpp
  clang-tidy/abseil/CMakeLists.txt
  clang-tidy/abseil/TimeComparisonCheck.cpp
  clang-tidy/abseil/TimeComparisonCheck.h
  docs/ReleaseNotes.rst
  docs/clang-tidy/checks/abseil-time-comparison.rst
  docs/clang-tidy/checks/list.rst
  test/clang-tidy/abseil-time-comparison.cpp

Index: test/clang-tidy/abseil-time-comparison.cpp
===
--- /dev/null
+++ test/clang-tidy/abseil-time-comparison.cpp
@@ -0,0 +1,129 @@
+// RUN: %check_clang_tidy %s abseil-time-comparison %t -- -- -I%S/Inputs
+
+#include "absl/time/time.h"
+
+void f() {
+  double x;
+  absl::Duration d1, d2;
+  bool b;
+  absl::Time t1, t2;
+
+  // Check against the RHS
+  b = x > absl::ToUnixSeconds(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) > t1;
+  b = x >= absl::ToUnixSeconds(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) >= t1;
+  b = x == absl::ToUnixSeconds(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) == t1;
+  b = x <= absl::ToUnixSeconds(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) <= t1;
+  b = x < absl::ToUnixSeconds(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) < t1;
+  b = x == absl::ToUnixSeconds(t1 - d2);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixSeconds(x) == t1 - d2;
+  b = absl::ToUnixSeconds(t1) > absl::ToUnixSeconds(t2);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 > t2;
+
+  // Check against the LHS
+  b = absl::ToUnixSeconds(t1) < x;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 < absl::FromUnixSeconds(x);
+  b = absl::ToUnixSeconds(t1) <= x;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 <= absl::FromUnixSeconds(x);
+  b = absl::ToUnixSeconds(t1) == x;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 == absl::FromUnixSeconds(x);
+  b = absl::ToUnixSeconds(t1) >= x;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 >= absl::FromUnixSeconds(x);
+  b = absl::ToUnixSeconds(t1) > x;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 > absl::FromUnixSeconds(x);
+
+  // Comparison against zero
+  b = absl::ToUnixSeconds(t1) < 0.0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 < absl::UnixEpoch();
+  b = absl::ToUnixSeconds(t1) < 0;
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: t1 < absl::UnixEpoch();
+
+  // Scales other than Seconds
+  b = x > absl::ToUnixMicros(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixMicros(x) > t1;
+  b = x >= absl::ToUnixMillis(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixMillis(x) >= t1;
+  b = x == absl::ToUnixNanos(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixNanos(x) == t1;
+  b = x <= absl::ToUnixMinutes(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixMinutes(x) <= t1;
+  b = x < absl::ToUnixHours(t1);
+  // CHECK-MESSAGES: [[@LINE-1]]:7: warning: perform comparison in the time domain [abseil-time-comparison]
+  // CHECK-FIXES: absl::FromUnixHours(x) < t1;
+
+  // A long expression
+  bool some_condition;
+  int very_ve

[PATCH] D58154: Add support for -fpermissive.

[James Y Knight via Phabricator via cfe-commits]

jyknight added a comment.

The errors disabled by this feature are default-error warnings -- you can 
already get the same effect by using -Wno-. Why is it bad to 
additionally allow -fpermissive to disable them? (If we have any diagnostics 
which are currently default-on-warnings which should not _ever_ be 
disable-able, then maybe we should just fix those?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D35169: Refactor DragonFly BSD toolchain driver.

[Rimvydas via Phabricator via cfe-commits]

rimvydas added inline comments.



Comment at: lib/Driver/ToolChains/DragonFly.cpp:118
 }
-
CmdArgs.push_back(Args.MakeArgString(getToolChain().GetFilePath("crti.o")));
-if (Args.hasArg(options::OPT_shared) || Args.hasArg(options::OPT_pie))
-  CmdArgs.push_back(
-  Args.MakeArgString(getToolChain().GetFilePath("crtbeginS.o")));
+if (crt1)
+  CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath(crt1)));

mgorny wrote:
> Can't `crt1` be non-null only if `!Args.hasArg(options::OPT_shared` here? 
> i.e. is there a reason to do it like this instead of just pushing it inside 
> the above `if`?
The `crt1` can only be used if `!Args.hasArg(options::OPT_shared`, both 
declaration and `CmdArgs.push_back` could be moved inside. The only reason for 
this adjustment is to reduce stylistic differences against 
lib/Driver/ToolChains/FreeBSD.cpp driver layout.





Comment at: lib/Driver/ToolChains/DragonFly.cpp:123
+
+const char *crtbegin = nullptr;
+if (Args.hasArg(options::OPT_shared) || IsPIE)

mgorny wrote:
> This default will never be used.
Correct. Could be changed in both FreeBSD.cpp and DragonFly.cpp.



Comment at: lib/Driver/ToolChains/DragonFly.cpp:185
+if (Args.hasArg(options::OPT_shared) || IsPIE)
+  
CmdArgs.push_back(Args.MakeArgString(ToolChain.GetFilePath("crtendS.o")));
 else

mgorny wrote:
> Inconsistency here: above you used helper variable, here you duplicate the 
> whole line.
Same as two previous. Only to match lib/Driver/ToolChains/FreeBSD.cpp layout.


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

https://reviews.llvm.org/D35169



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

[PATCH] D58320: [Darwin] Introduce a new flag, -fapple-link-rtlib that forces linking of the builtins library.

[Amara Emerson via Phabricator via cfe-commits]

aemerson added a comment.

In D58320#1418477 , @peter.smith wrote:

> I've no objections to adding the command line as a Darwin only option. 
> Implementation looks fine to me although I've not got any prior experience 
> with Darwin.


Thanks, @ab ok for Darwin?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58320



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

[PATCH] D58979: [clang][OpenMP] Revert "OMPFlushClause is synthetic, no such clause exists"

[Roman Lebedev via Phabricator via cfe-commits]

lebedev.ri created this revision.
lebedev.ri added reviewers: gribozavr, ABataev, rjmccall, aaron.ballman.
lebedev.ri added a project: clang.
Herald added subscribers: jdoerfert, arphaman, guansong, dylanmckay.

This reverts rL352390  / D57280 
.

As discussed in https://reviews.llvm.org/D57112#inline-506781,
'flush' clause does not exist in the OpenMP spec, it can not be
specified, and `OMPFlushClause` class is just a helper class.

Now, here's the caveat. I have read @ABataev's

> Well, I think it would be good to filter out OMPC_flush somehow
>  because there is no such clause actually, it is a pseudo clause
>  for better handling of the flush directive.

as if that clause is pseudo clause that only exists for the sole
purpose of simplifying the parser. As in, it never reaches AST.

I did not however try to verify that. Too bad, i was wrong.
It absolutely *does* reach AST. Therefore my understanding/justification
for the change was flawed, which makes the patch a regression which **must** be 
reverted.

@gribozavr has brought that up again in 
https://reviews.llvm.org/D57112#inline-521238

>> ...
>  Sorry to be late for this discussion, but I don't think this conclusion 
>  follows. ASTMatchers are supposed to match the AST as it is. 
>  Even if OMPC_flush is synthetic, it exists in the AST, and users might 
>  want to match it. I think users would find anything else (trying to filter
>  out AST nodes that are not in the source code) to be surprising. For example,
>  there's a matcher materializeTemporaryExpr even though this AST node is a 
>  Clang invention and is not a part of the C++ spec.
> 
> Matching only constructs that appear in the source code is not feasible with 
>  ASTMatchers, because they are based on Clang's AST that exposes tons of 
> semantic 
>  information, and its design is dictated by the structure of the semantic 
> information.
>  See "RFC: Tree-based refactorings with Clang" in cfe-dev for a library that 
> will
>  focus on representing source code as faithfully as possible.
> 
> Not to even mention that this code is in ASTTypeTraits, a general library for
>  handling AST nodes, not specifically for AST Matchers...


Repository:
  rC Clang

https://reviews.llvm.org/D58979

Files:
  include/clang/AST/OpenMPClause.h
  include/clang/AST/RecursiveASTVisitor.h
  include/clang/Basic/OpenMPKinds.def
  include/clang/Basic/OpenMPKinds.h
  include/clang/Serialization/ASTReader.h
  include/clang/Serialization/ASTWriter.h
  lib/AST/OpenMPClause.cpp
  lib/AST/StmtProfile.cpp
  lib/Basic/OpenMPKinds.cpp
  lib/Sema/TreeTransform.h
  tools/libclang/CIndex.cpp

Index: tools/libclang/CIndex.cpp
===
--- tools/libclang/CIndex.cpp
+++ tools/libclang/CIndex.cpp
@@ -2130,7 +2130,6 @@
   OMPClauseEnqueue(EnqueueVisitor *Visitor) : Visitor(Visitor) { }
 #define OPENMP_CLAUSE(Name, Class) \
   void Visit##Class(const Class *C);
-  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
   void VisitOMPClauseWithPreInit(const OMPClauseWithPreInit *C);
   void VisitOMPClauseWithPostUpdate(const OMPClauseWithPostUpdate *C);
Index: lib/Sema/TreeTransform.h
===
--- lib/Sema/TreeTransform.h
+++ lib/Sema/TreeTransform.h
@@ -691,7 +691,6 @@
 #define OPENMP_CLAUSE(Name, Class)\
   LLVM_ATTRIBUTE_NOINLINE \
   OMPClause *Transform ## Class(Class *S);
-  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
 
   /// Build a new qualified type given its unqualified type and type location.
@@ -3323,7 +3322,6 @@
 #define OPENMP_CLAUSE(Name, Class) \
   case OMPC_ ## Name : \
 return getDerived().Transform ## Class(cast(S));
-  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
   }
 
Index: lib/Basic/OpenMPKinds.cpp
===
--- lib/Basic/OpenMPKinds.cpp
+++ lib/Basic/OpenMPKinds.cpp
@@ -67,8 +67,6 @@
   case OMPC_##Name:\
 return #Name;
 #include "clang/Basic/OpenMPKinds.def"
-  case OMPC_flush:
-return "flush";
   case OMPC_uniform:
 return "uniform";
   case OMPC_threadprivate:
Index: lib/AST/StmtProfile.cpp
===
--- lib/AST/StmtProfile.cpp
+++ lib/AST/StmtProfile.cpp
@@ -412,7 +412,6 @@
   OMPClauseProfiler(StmtProfiler *P) : Profiler(P) { }
 #define OPENMP_CLAUSE(Name, Class) \
   void Visit##Class(const Class *C);
-  OPENMP_CLAUSE(flush, OMPFlushClause)
 #include "clang/Basic/OpenMPKinds.def"
   void VistOMPClauseWithPreInit(const OMPClauseWithPreIn

[PATCH] D58979: [clang][OpenMP] Revert "OMPFlushClause is synthetic, no such clause exists"

[Alexey Bataev via Phabricator via cfe-commits]

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

Hmm, do we really need the matches for the AST node that is not 
described/defined by the standard? If this is really so, I'm ok with this.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58979



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

r355418 - [OPENMP]Target region: emit const firstprivates as globals with constant

[Alexey Bataev via cfe-commits]

Author: abataev
Date: Tue Mar  5 09:47:18 2019
New Revision: 355418

URL: http://llvm.org/viewvc/llvm-project?rev=355418&view=rev
Log:
[OPENMP]Target region: emit const firstprivates as globals with constant
memory.

If the variable with the constant non-scalar type is firstprivatized in
the target region, the local copy is created with the data copying.
Instead, we allocate the copy in the constant memory and avoid extra
copying in the outlined target regions. This global copy is used in the
target regions without loss of the performance.

Modified:
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntime.h
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.cpp
cfe/trunk/lib/CodeGen/CGOpenMPRuntimeNVPTX.h
cfe/trunk/lib/CodeGen/CGStmtOpenMP.cpp
cfe/trunk/test/OpenMP/nvptx_target_firstprivate_codegen.cpp
cfe/trunk/test/OpenMP/target_firstprivate_codegen.cpp

Modified: cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp?rev=355418&r1=355417&r2=355418&view=diff
==
--- cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGOpenMPRuntime.cpp Tue Mar  5 09:47:18 2019
@@ -2945,9 +2945,8 @@ Address CGOpenMPRuntime::emitThreadIDAdd
   return ThreadIDTemp;
 }
 
-llvm::Constant *
-CGOpenMPRuntime::getOrCreateInternalVariable(llvm::Type *Ty,
- const llvm::Twine &Name) {
+llvm::Constant *CGOpenMPRuntime::getOrCreateInternalVariable(
+llvm::Type *Ty, const llvm::Twine &Name, unsigned AddressSpace) {
   SmallString<256> Buffer;
   llvm::raw_svector_ostream Out(Buffer);
   Out << Name;
@@ -2962,7 +2961,8 @@ CGOpenMPRuntime::getOrCreateInternalVari
   return Elem.second = new llvm::GlobalVariable(
  CGM.getModule(), Ty, /*IsConstant*/ false,
  llvm::GlobalValue::CommonLinkage, 
llvm::Constant::getNullValue(Ty),
- Elem.first());
+ Elem.first(), /*InsertBefore=*/nullptr,
+ llvm::GlobalValue::NotThreadLocal, AddressSpace);
 }
 
 llvm::Value *CGOpenMPRuntime::getCriticalRegionLock(StringRef CriticalName) {
@@ -7285,9 +7285,14 @@ private:
 // A first private variable captured by reference will use only the
 // 'private ptr' and 'map to' flag. Return the right flags if the captured
 // declaration is known as first-private in this handler.
-if (FirstPrivateDecls.count(Cap.getCapturedVar()))
+if (FirstPrivateDecls.count(Cap.getCapturedVar())) {
+  if (Cap.getCapturedVar()->getType().isConstant(CGF.getContext()) &&
+  Cap.getCaptureKind() == CapturedStmt::VCK_ByRef)
+return MappableExprsHandler::OMP_MAP_ALWAYS |
+   MappableExprsHandler::OMP_MAP_TO;
   return MappableExprsHandler::OMP_MAP_PRIVATE |
  MappableExprsHandler::OMP_MAP_TO;
+}
 return MappableExprsHandler::OMP_MAP_TO |
MappableExprsHandler::OMP_MAP_FROM;
   }
@@ -7914,9 +7919,6 @@ public:
   }
 } else {
   assert(CI.capturesVariable() && "Expected captured reference.");
-  CurBasePointers.push_back(CV);
-  CurPointers.push_back(CV);
-
   const auto *PtrTy = cast(RI.getType().getTypePtr());
   QualType ElementType = PtrTy->getPointeeType();
   CurSizes.push_back(CGF.getTypeSize(ElementType));
@@ -7924,6 +7926,24 @@ public:
   // default the value doesn't have to be retrieved. For an aggregate
   // type, the default is 'tofrom'.
   CurMapTypes.push_back(getMapModifiersForPrivateClauses(CI));
+  const VarDecl *VD = CI.getCapturedVar();
+  if (FirstPrivateDecls.count(VD) &&
+  VD->getType().isConstant(CGF.getContext())) {
+llvm::Constant *Addr =
+CGF.CGM.getOpenMPRuntime().registerTargetFirstprivateCopy(CGF, VD);
+// Copy the value of the original variable to the new global copy.
+CGF.Builder.CreateMemCpy(
+CGF.MakeNaturalAlignAddrLValue(Addr, ElementType).getAddress(),
+Address(CV, CGF.getContext().getTypeAlignInChars(ElementType)),
+CurSizes.back(),
+/*isVolatile=*/false);
+// Use new global variable as the base pointers.
+CurBasePointers.push_back(Addr);
+CurPointers.push_back(Addr);
+  } else {
+CurBasePointers.push_back(CV);
+CurPointers.push_back(CV);
+  }
 }
 // Every default map produces a single argument which is a target 
parameter.
 CurMapTypes.back() |= OMP_MAP_TARGET_PARAM;
@@ -8725,6 +8745,40 @@ bool CGOpenMPRuntime::emitTargetGlobalVa
   return false;
 }
 
+llvm::Constant *
+CGOpenMPRuntime::registerTargetFirstprivateCopy(CodeGenFunction &CGF,
+const VarDecl *VD) {
+  assert(VD->getType().isConstant(CGM.getContext()) &&
+ "Expected constant variable.");
+  StringRef VarName;
+  llvm::Constant *Add

r355419 - Allow bundle size to be 0 in clang-offload-bundler

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Mar  5 09:52:32 2019
New Revision: 355419

URL: http://llvm.org/viewvc/llvm-project?rev=355419&view=rev
Log:
Allow bundle size to be 0 in clang-offload-bundler

HIP uses clang-offload-bundler to create fat binary. The bundle for host is 
empty.
Currently clang-offload-bundler checks if the bundle size is 0 when unbundling.
If so it will exit without unbundling the remaining bundles. This causes
clang-offload-bundler not being able to unbundle fat binaries generated for HIP.

This patch allows bundles size to be 0 when clang-offload-bundler unbundles
input files.

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

Modified:
cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Modified: cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp?rev=355419&r1=355418&r2=355419&view=diff
==
--- cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp (original)
+++ cfe/trunk/tools/clang-offload-bundler/ClangOffloadBundler.cpp Tue Mar  5 
09:52:32 2019
@@ -292,7 +292,7 @@ public:
   ReadChars += TripleSize;
 
   // Check if the offset and size make sense.
-  if (!Size || !Offset || Offset + Size > FC.size())
+  if (!Offset || Offset + Size > FC.size())
 return;
 
   assert(BundlesInfo.find(Triple) == BundlesInfo.end() &&


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

[PATCH] D58057: Allow bundle size to be 0 in clang-offload-bundler

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355419: Allow bundle size to be 0 in clang-offload-bundler 
(authored by yaxunl, committed by ).
Herald added a project: clang.

Repository:
  rC Clang

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

https://reviews.llvm.org/D58057

Files:
  tools/clang-offload-bundler/ClangOffloadBundler.cpp


Index: tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -292,7 +292,7 @@
   ReadChars += TripleSize;
 
   // Check if the offset and size make sense.
-  if (!Size || !Offset || Offset + Size > FC.size())
+  if (!Offset || Offset + Size > FC.size())
 return;
 
   assert(BundlesInfo.find(Triple) == BundlesInfo.end() &&


Index: tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -292,7 +292,7 @@
   ReadChars += TripleSize;
 
   // Check if the offset and size make sense.
-  if (!Size || !Offset || Offset + Size > FC.size())
+  if (!Offset || Offset + Size > FC.size())
 return;
 
   assert(BundlesInfo.find(Triple) == BundlesInfo.end() &&
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58979: [clang][OpenMP] Revert "OMPFlushClause is synthetic, no such clause exists"

[Dmitri Gribenko via Phabricator via cfe-commits]

gribozavr accepted this revision.
gribozavr added a comment.

> Hmm, do we really need the matches for the AST node that is not 
> described/defined by the standard? If this is really so, I'm ok with this.

When the rubber meets the road, it does not matter what is in the spec, users 
will need to work with the ASTs that they get.  If there's something in the 
AST, users will want to match it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58979



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

[PATCH] D58708: [PR40778] Preserve addr space in Derived to Base cast

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 189347.
Anastasia marked an inline comment as done.
Anastasia added a comment.

Moved adding address space to `DestType` earlier  to cover pointer and 
non-pointer case.


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

https://reviews.llvm.org/D58708

Files:
  lib/CodeGen/CGClass.cpp
  lib/Sema/SemaExpr.cpp
  test/CodeGenOpenCLCXX/addrspace-derived-base.cl


Index: test/CodeGenOpenCLCXX/addrspace-derived-base.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/addrspace-derived-base.cl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck 
%s
+
+struct B {
+  int mb;
+};
+
+class D : public B {
+public:
+  int getmb() { return mb; }
+};
+
+void foo() {
+  D d;
+  //CHECK: addrspacecast %class.D* %d to %class.D addrspace(4)*
+  //CHECK: call i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)*
+  d.getmb();
+}
+
+//Derived and Base are in the same address space.
+
+//CHECK: define linkonce_odr i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)* 
%this)
+//CHECK: bitcast %class.D addrspace(4)* %this1 to %struct.B addrspace(4)*
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -2660,6 +2660,10 @@
   bool PointerConversions = false;
   if (isa(Member)) {
 DestRecordType = Context.getCanonicalType(Context.getTypeDeclType(RD));
+DestRecordType = Context.getAddrSpaceQualType(
+DestRecordType, FromType->isPointerType()
+? FromType->getPointeeType().getAddressSpace()
+: FromType.getAddressSpace());
 
 if (FromType->getAs()) {
   DestType = Context.getPointerType(DestRecordType);
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -302,7 +302,8 @@
 
   // Get the base pointer type.
   llvm::Type *BasePtrTy =
-ConvertType((PathEnd[-1])->getType())->getPointerTo();
+  ConvertType((PathEnd[-1])->getType())
+  ->getPointerTo(Value.getType()->getPointerAddressSpace());
 
   QualType DerivedTy = getContext().getRecordType(Derived);
   CharUnits DerivedAlign = CGM.getClassPointerAlignment(Derived);


Index: test/CodeGenOpenCLCXX/addrspace-derived-base.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/addrspace-derived-base.cl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct B {
+  int mb;
+};
+
+class D : public B {
+public:
+  int getmb() { return mb; }
+};
+
+void foo() {
+  D d;
+  //CHECK: addrspacecast %class.D* %d to %class.D addrspace(4)*
+  //CHECK: call i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)*
+  d.getmb();
+}
+
+//Derived and Base are in the same address space.
+
+//CHECK: define linkonce_odr i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)* %this)
+//CHECK: bitcast %class.D addrspace(4)* %this1 to %struct.B addrspace(4)*
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -2660,6 +2660,10 @@
   bool PointerConversions = false;
   if (isa(Member)) {
 DestRecordType = Context.getCanonicalType(Context.getTypeDeclType(RD));
+DestRecordType = Context.getAddrSpaceQualType(
+DestRecordType, FromType->isPointerType()
+? FromType->getPointeeType().getAddressSpace()
+: FromType.getAddressSpace());
 
 if (FromType->getAs()) {
   DestType = Context.getPointerType(DestRecordType);
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -302,7 +302,8 @@
 
   // Get the base pointer type.
   llvm::Type *BasePtrTy =
-ConvertType((PathEnd[-1])->getType())->getPointerTo();
+  ConvertType((PathEnd[-1])->getType())
+  ->getPointerTo(Value.getType()->getPointerAddressSpace());
 
   QualType DerivedTy = getContext().getRecordType(Derived);
   CharUnits DerivedAlign = CGM.getClassPointerAlignment(Derived);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58708: [PR40778] Preserve addr space in Derived to Base cast

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:2670
 } else {
   DestType = DestRecordType;
   FromRecordType = FromType;

rjmccall wrote:
> This path (when the object is a gl-value) also needs an address-space 
> qualifier, so you should probably add it above and then just add a pointer in 
> the pointer case.
I updated this but I haven't really found any test case that would fail. I 
guess because casting non-pointer types in different address spaces works 
fine...

For example the IR is generated correctly with or without the fix.
```
  1 struct S {
  2volatile int a;
  3 }; 
  4 
  5 __global S g;
  6 
  7 void foo(){
  8 g.a; 
  9 }
``` 

However, I still think it's indeed good to add the address space in both cases 
for consistency. 


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

https://reviews.llvm.org/D58708



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

[PATCH] D58060: Fix diagnostic for addr spaces in reference binding

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added inline comments.



Comment at: test/SemaOpenCLCXX/address-space-references.cl:7
+// can't detect this case and therefore fails.
+//int bar(const unsigned int &i);
+

ebevhan wrote:
> Is there a reason the declaration is commented out? If it's just a problem 
> with the resolution, wouldn't it still be possible to have the declaration 
> here anyway?
Hmm... actually not at all! I commented it to show that this could work 
better... but FIXME is good enough to indicate this!


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

https://reviews.llvm.org/D58060



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

[PATCH] D58060: Fix diagnostic for addr spaces in reference binding

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 189351.
Anastasia marked an inline comment as done.
Anastasia added a comment.

Un-commented the line in the test


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

https://reviews.llvm.org/D58060

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaInit.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-0x.cpp
  test/CXX/dcl.decl/dcl.init/dcl.init.ref/p5-var.cpp
  test/CXX/expr/expr.post/expr.static.cast/p3-p4-0x.cpp
  test/CXX/expr/expr.prim/expr.prim.lambda/p16.cpp
  test/Misc/diag-template-diffing.cpp
  test/SemaCXX/builtins-arm.cpp
  test/SemaCXX/err_reference_bind_drops_quals.cpp
  test/SemaCXX/references.cpp
  test/SemaOpenCLCXX/address-space-castoperators.cpp
  test/SemaOpenCLCXX/address-space-references.cl

Index: test/SemaOpenCLCXX/address-space-references.cl
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-references.cl
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify -fsyntax-only
+
+int bar(const __global unsigned int &i); // expected-note{{passing argument to parameter 'i' here}}
+//FIXME: With the overload below the call should be resolved
+// successfully. However, current overload resolution logic
+// can't detect this case and therefore fails.
+int bar(const unsigned int &i);
+
+void foo() {
+  bar(1) // expected-error{{binding reference of type 'const __global unsigned int' to value of type 'int' changes address space}}
+}
Index: test/SemaOpenCLCXX/address-space-castoperators.cpp
===
--- /dev/null
+++ test/SemaOpenCLCXX/address-space-castoperators.cpp
@@ -0,0 +1,17 @@
+//RUN: %clang_cc1 %s -triple spir-unknown-unknown -cl-std=c++ -pedantic -verify
+
+void test_ref(int &gen, __global const int &glob) {
+  static_cast<__global int &>(gen); // expected-error{{binding reference of type '__global int' to value of type '__generic int' changes address space}}
+  static_cast<__global const int &>(gen);   //expected-error{{binding reference of type 'const __global int' to value of type '__generic int' changes address space}}
+  static_cast<__global int &>(glob);//expected-error{{binding reference of type '__global int' to value of type 'const __global int' drops 'const' qualifier}}
+  static_cast<__local int &>(glob); //expected-error{{binding reference of type '__local int' to value of type 'const __global int' changes address space}}
+  static_cast<__generic const int &>(glob); //expected-warning{{expression result unused}}
+}
+
+void test_ptr(int *gen, __global const int *glob) {
+  static_cast<__global int *>(gen); // expected-error{{static_cast from '__generic int *' to '__global int *' is not allowed}}
+  static_cast<__global const int *>(gen);   //expected-error{{static_cast from '__generic int *' to 'const __global int *' is not allowed}}
+  static_cast<__global int *>(glob);//expected-error{{static_cast from 'const __global int *' to '__global int *' is not allowed}}
+  static_cast<__local int *>(glob); //expected-error{{static_cast from 'const __global int *' to '__local int *' is not allowed}}
+  static_cast<__generic const int *>(glob); //expected-warning{{expression result unused}}
+}
Index: test/SemaCXX/references.cpp
===
--- test/SemaCXX/references.cpp
+++ test/SemaCXX/references.cpp
@@ -55,13 +55,13 @@
 void test5() {
   //  const double& rcd2 = 2; // rcd2 refers to temporary with value 2.0
   const volatile int cvi = 1;
-  const int& r = cvi; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+  const int& r = cvi; // expected-error{{binding reference of type 'const int' to value of type 'const volatile int' drops 'volatile' qualifier}}
 
 #if __cplusplus >= 201103L
-  const int& r2{cvi}; // expected-error{{binding value of type 'const volatile int' to reference to type 'const int' drops 'volatile' qualifier}}
+  const int& r2{cvi}; // expected-error{{binding reference of type 'const int' to value of type 'const volatile int' drops 'volatile' qualifier}}
 
   const int a = 2;
-  int& r3{a}; // expected-error{{binding value of type 'const int' to reference to type 'int' drops 'const'}}
+  int& r3{a}; // expected-error{{binding reference of type 'int' to value of type 'const int' drops 'const' qualifier}}
 
   const int&& r4{a}; // expected-error{{rvalue reference to type 'const int' cannot bind to lvalue of type 'const int'}}
 
Index: test/SemaCXX/err_reference_bind_drops_quals.cpp
===
--- test/SemaCXX/err_reference_bind_drops_quals.cpp
+++ test/SemaCXX/err_reference_bind_drops_quals.cpp
@@ -6,31 +6,31 @@
volatile ptr vp, const volatile ptr cvp, restrict volatile ptr rvp,
   

r355421 - [CUDA][HIP][Sema] Fix template kernel with function as template parameter

[Yaxun Liu via cfe-commits]

Author: yaxunl
Date: Tue Mar  5 10:19:35 2019
New Revision: 355421

URL: http://llvm.org/viewvc/llvm-project?rev=355421&view=rev
Log:
[CUDA][HIP][Sema] Fix template kernel with function as template parameter

If a kernel template has a function as its template parameter, a device 
function should be
allowed as template argument since a kernel can call a device function. However,
currently if the kernel template is instantiated in a host function, clang will 
emit an error
message saying the device function is an invalid candidate for the template 
parameter.

This happens because clang checks the reference to the device function during 
parsing
the template arguments. At this point, the template is not instantiated yet. 
Clang incorrectly
assumes the device function is called by the host function and emits the error 
message.

This patch fixes the issue by disabling checking of device function during 
parsing template
arguments and deferring the check to the instantion of the template. At that 
point, the
template decl is already available, therefore the check can be done against the 
instantiated
function template decl.

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

Modified:
cfe/trunk/lib/Sema/SemaCUDA.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/SemaCUDA/call-device-fn-from-host.cu
cfe/trunk/test/SemaCUDA/call-host-fn-from-device.cu

Modified: cfe/trunk/lib/Sema/SemaCUDA.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaCUDA.cpp?rev=355421&r1=355420&r2=355421&view=diff
==
--- cfe/trunk/lib/Sema/SemaCUDA.cpp (original)
+++ cfe/trunk/lib/Sema/SemaCUDA.cpp Tue Mar  5 10:19:35 2019
@@ -675,6 +675,11 @@ Sema::DeviceDiagBuilder Sema::CUDADiagIf
 bool Sema::CheckCUDACall(SourceLocation Loc, FunctionDecl *Callee) {
   assert(getLangOpts().CUDA && "Should only be called during CUDA 
compilation");
   assert(Callee && "Callee may not be null.");
+
+  auto &ExprEvalCtx = ExprEvalContexts.back();
+  if (ExprEvalCtx.isUnevaluated() || ExprEvalCtx.isConstantEvaluated())
+return true;
+
   // FIXME: Is bailing out early correct here?  Should we instead assume that
   // the caller is a global initializer?
   FunctionDecl *Caller = dyn_cast(CurContext);

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=355421&r1=355420&r2=355421&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Tue Mar  5 10:19:35 2019
@@ -14799,6 +14799,9 @@ void Sema::MarkFunctionReferenced(Source
   if (FPT && isUnresolvedExceptionSpec(FPT->getExceptionSpecType()))
 ResolveExceptionSpec(Loc, FPT);
 
+  if (getLangOpts().CUDA)
+CheckCUDACall(Loc, Func);
+
   // If we don't need to mark the function as used, and we don't need to
   // try to provide a definition, there's nothing more to do.
   if ((Func->isUsed(/*CheckUsedAttr=*/false) || !OdrUse) &&

Modified: cfe/trunk/test/SemaCUDA/call-device-fn-from-host.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/call-device-fn-from-host.cu?rev=355421&r1=355420&r2=355421&view=diff
==
--- cfe/trunk/test/SemaCUDA/call-device-fn-from-host.cu (original)
+++ cfe/trunk/test/SemaCUDA/call-device-fn-from-host.cu Tue Mar  5 10:19:35 2019
@@ -37,7 +37,7 @@ __host__ __device__ void T::hd3() {
 }
 
 template  __host__ __device__ void hd2() { device_fn(); }
-// expected-error@-1 {{reference to __device__ function 'device_fn' in 
__host__ __device__ function}}
+// expected-error@-1 2 {{reference to __device__ function 'device_fn' in 
__host__ __device__ function}}
 void host_fn() { hd2(); }
 
 __host__ __device__ void hd() { device_fn(); }
@@ -90,3 +90,8 @@ __host__ __device__ void fn_ptr_template
 static __host__ __device__ void hd_func() { device_fn(); }
 __global__ void kernel() { hd_func(); }
 void host_func(void) { kernel<<<1, 1>>>(); }
+
+// Should allow host function call kernel template with device function 
argument.
+__device__ void f();
+template __global__ void t() { F(); }
+__host__ void g() { t<<<1,1>>>(); }

Modified: cfe/trunk/test/SemaCUDA/call-host-fn-from-device.cu
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCUDA/call-host-fn-from-device.cu?rev=355421&r1=355420&r2=355421&view=diff
==
--- cfe/trunk/test/SemaCUDA/call-host-fn-from-device.cu (original)
+++ cfe/trunk/test/SemaCUDA/call-host-fn-from-device.cu Tue Mar  5 10:19:35 2019
@@ -56,14 +56,14 @@ __host__ __device__ void T::hd3() {
 }
 
 template  __host__ __device__ void hd2() { host_fn(); }
-// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
+// expected-error@-1 2 {{reference to __host__ f

[PATCH] D56411: [CUDA][HIP][Sema] Fix template kernel with function as template parameter

[Yaxun Liu via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355421: [CUDA][HIP][Sema] Fix template kernel with function 
as template parameter (authored by yaxunl, committed by ).
Herald added a project: clang.

Repository:
  rC Clang

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

https://reviews.llvm.org/D56411

Files:
  lib/Sema/SemaCUDA.cpp
  lib/Sema/SemaExpr.cpp
  test/SemaCUDA/call-device-fn-from-host.cu
  test/SemaCUDA/call-host-fn-from-device.cu


Index: test/SemaCUDA/call-device-fn-from-host.cu
===
--- test/SemaCUDA/call-device-fn-from-host.cu
+++ test/SemaCUDA/call-device-fn-from-host.cu
@@ -37,7 +37,7 @@
 }
 
 template  __host__ __device__ void hd2() { device_fn(); }
-// expected-error@-1 {{reference to __device__ function 'device_fn' in 
__host__ __device__ function}}
+// expected-error@-1 2 {{reference to __device__ function 'device_fn' in 
__host__ __device__ function}}
 void host_fn() { hd2(); }
 
 __host__ __device__ void hd() { device_fn(); }
@@ -90,3 +90,8 @@
 static __host__ __device__ void hd_func() { device_fn(); }
 __global__ void kernel() { hd_func(); }
 void host_func(void) { kernel<<<1, 1>>>(); }
+
+// Should allow host function call kernel template with device function 
argument.
+__device__ void f();
+template __global__ void t() { F(); }
+__host__ void g() { t<<<1,1>>>(); }
Index: test/SemaCUDA/call-host-fn-from-device.cu
===
--- test/SemaCUDA/call-host-fn-from-device.cu
+++ test/SemaCUDA/call-host-fn-from-device.cu
@@ -56,14 +56,14 @@
 }
 
 template  __host__ __device__ void hd2() { host_fn(); }
-// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
+// expected-error@-1 2 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
 __global__ void kernel() { hd2(); }
 
 __host__ __device__ void hd() { host_fn(); }
 // expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
 
 template  __host__ __device__ void hd3() { host_fn(); }
-// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
+// expected-error@-1 2 {{reference to __host__ function 'host_fn' in __host__ 
__device__ function}}
 __device__ void device_fn() { hd3(); }
 
 // No error because this is never instantiated.
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -14799,6 +14799,9 @@
   if (FPT && isUnresolvedExceptionSpec(FPT->getExceptionSpecType()))
 ResolveExceptionSpec(Loc, FPT);
 
+  if (getLangOpts().CUDA)
+CheckCUDACall(Loc, Func);
+
   // If we don't need to mark the function as used, and we don't need to
   // try to provide a definition, there's nothing more to do.
   if ((Func->isUsed(/*CheckUsedAttr=*/false) || !OdrUse) &&
Index: lib/Sema/SemaCUDA.cpp
===
--- lib/Sema/SemaCUDA.cpp
+++ lib/Sema/SemaCUDA.cpp
@@ -675,6 +675,11 @@
 bool Sema::CheckCUDACall(SourceLocation Loc, FunctionDecl *Callee) {
   assert(getLangOpts().CUDA && "Should only be called during CUDA 
compilation");
   assert(Callee && "Callee may not be null.");
+
+  auto &ExprEvalCtx = ExprEvalContexts.back();
+  if (ExprEvalCtx.isUnevaluated() || ExprEvalCtx.isConstantEvaluated())
+return true;
+
   // FIXME: Is bailing out early correct here?  Should we instead assume that
   // the caller is a global initializer?
   FunctionDecl *Caller = dyn_cast(CurContext);


Index: test/SemaCUDA/call-device-fn-from-host.cu
===
--- test/SemaCUDA/call-device-fn-from-host.cu
+++ test/SemaCUDA/call-device-fn-from-host.cu
@@ -37,7 +37,7 @@
 }
 
 template  __host__ __device__ void hd2() { device_fn(); }
-// expected-error@-1 {{reference to __device__ function 'device_fn' in __host__ __device__ function}}
+// expected-error@-1 2 {{reference to __device__ function 'device_fn' in __host__ __device__ function}}
 void host_fn() { hd2(); }
 
 __host__ __device__ void hd() { device_fn(); }
@@ -90,3 +90,8 @@
 static __host__ __device__ void hd_func() { device_fn(); }
 __global__ void kernel() { hd_func(); }
 void host_func(void) { kernel<<<1, 1>>>(); }
+
+// Should allow host function call kernel template with device function argument.
+__device__ void f();
+template __global__ void t() { F(); }
+__host__ void g() { t<<<1,1>>>(); }
Index: test/SemaCUDA/call-host-fn-from-device.cu
===
--- test/SemaCUDA/call-host-fn-from-device.cu
+++ test/SemaCUDA/call-host-fn-from-device.cu
@@ -56,14 +56,14 @@
 }
 
 template  __host__ __device__ void hd2() { host_fn(); }
-// expected-error@-1 {{reference to __host__ function 'host_fn' in __host__ __device__ f

[PATCH] D58708: [PR40778] Preserve addr space in Derived to Base cast

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:2666
+? FromType->getPointeeType().getAddressSpace()
+: FromType.getAddressSpace());
 

Please do a `getAs()` into a local and then use that here and 
below.


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

https://reviews.llvm.org/D58708



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

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 189361.
Anastasia added a comment.

Updates FiXME explaining why C++ mode is more permissive.


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

https://reviews.llvm.org/D58346

Files:
  include/clang/Basic/DiagnosticSemaKinds.td
  lib/Sema/SemaCast.cpp
  test/CodeGenOpenCLCXX/address-space-castoperators.cpp
  test/SemaCXX/address-space-conversion.cpp
  test/SemaOpenCL/address-spaces-conversions-cl2.0.cl
  test/SemaOpenCL/address-spaces.cl

Index: test/SemaOpenCL/address-spaces.cl
===
--- test/SemaOpenCL/address-spaces.cl
+++ test/SemaOpenCL/address-spaces.cl
@@ -26,24 +26,96 @@
 }
 
 void explicit_cast(__global int *g, __local int *l, __constant int *c, __private int *p, const __constant int *cc) {
-  g = (__global int *)l;  // expected-error {{casting '__local int *' to type '__global int *' changes address space of pointer}}
-  g = (__global int *)c;  // expected-error {{casting '__constant int *' to type '__global int *' changes address space of pointer}}
-  g = (__global int *)cc; // expected-error {{casting 'const __constant int *' to type '__global int *' changes address space of pointer}}
-  g = (__global int *)p;  // expected-error {{casting 'int *' to type '__global int *' changes address space of pointer}}
-
-  l = (__local int *)g;  // expected-error {{casting '__global int *' to type '__local int *' changes address space of pointer}}
-  l = (__local int *)c;  // expected-error {{casting '__constant int *' to type '__local int *' changes address space of pointer}}
-  l = (__local int *)cc; // expected-error {{casting 'const __constant int *' to type '__local int *' changes address space of pointer}}
-  l = (__local int *)p;  // expected-error {{casting 'int *' to type '__local int *' changes address space of pointer}}
-
-  c = (__constant int *)g; // expected-error {{casting '__global int *' to type '__constant int *' changes address space of pointer}}
-  c = (__constant int *)l; // expected-error {{casting '__local int *' to type '__constant int *' changes address space of pointer}}
-  c = (__constant int *)p; // expected-error {{casting 'int *' to type '__constant int *' changes address space of pointer}}
-
-  p = (__private int *)g;  // expected-error {{casting '__global int *' to type 'int *' changes address space of pointer}}
-  p = (__private int *)l;  // expected-error {{casting '__local int *' to type 'int *' changes address space of pointer}}
-  p = (__private int *)c;  // expected-error {{casting '__constant int *' to type 'int *' changes address space of pointer}}
-  p = (__private int *)cc; // expected-error {{casting 'const __constant int *' to type 'int *' changes address space of pointer}}
+  g = (__global int *)l;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting '__local int *' to type '__global int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from '__local int *' to '__global int *' converts between mismatching address spaces}}
+#endif
+  g = (__global int *)c;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting '__constant int *' to type '__global int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from '__constant int *' to '__global int *' converts between mismatching address spaces}}
+#endif
+  g = (__global int *)cc;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting 'const __constant int *' to type '__global int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from 'const __constant int *' to '__global int *' converts between mismatching address spaces}}
+#endif
+  g = (__global int *)p;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting 'int *' to type '__global int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from 'int *' to '__global int *' converts between mismatching address spaces}}
+#endif
+  l = (__local int *)g;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting '__global int *' to type '__local int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from '__global int *' to '__local int *' converts between mismatching address spaces}}
+#endif
+  l = (__local int *)c;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting '__constant int *' to type '__local int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from '__constant int *' to '__local int *' converts between mismatching address spaces}}
+#endif
+  l = (__local int *)cc;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@-2 {{casting 'const __constant int *' to type '__local int *' changes address space of pointer}}
+#else
+// expected-error@-4 {{C-style cast from 'const __constant int *' to '__local int *' converts between mismatching address spaces}}
+#endif
+  l = (__local int *)p;
+#if !__OPENCL_CPP_VERSION__
+// expected-error@

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.

In D58346#1417983 , @rjmccall wrote:

> In the abstract, it would be nice to warn about casts that always have UB 
> because the address spaces don't overlap; however, I'm worried about how 
> people might be using address spaces in strange ways today, knowing that they 
> *do* overlap, only in ways that the compiler isn't smart enough for.  I think 
> we should just be permissive in non-OpenCL mode.


Ok, I have explained this in FIXME.


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

https://reviews.llvm.org/D58346



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

[PATCH] D58346: [Sema] Change addr space diagnostics in casts to follow C++ style

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaCast.cpp:2293
+  // define overlapping address spaces currently.
+  if (Self.getLangOpts().OpenCL) {
+auto SrcType = SrcExpr.get()->getType();

Please structure this as an early exit.


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

https://reviews.llvm.org/D58346



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

[PATCH] D58708: [PR40778] Preserve addr space in Derived to Base cast

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia updated this revision to Diff 189371.
Anastasia added a comment.

Use common pointer type.


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

https://reviews.llvm.org/D58708

Files:
  lib/CodeGen/CGClass.cpp
  lib/Sema/SemaExpr.cpp
  test/CodeGenOpenCLCXX/addrspace-derived-base.cl


Index: test/CodeGenOpenCLCXX/addrspace-derived-base.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/addrspace-derived-base.cl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck 
%s
+
+struct B {
+  int mb;
+};
+
+class D : public B {
+public:
+  int getmb() { return mb; }
+};
+
+void foo() {
+  D d;
+  //CHECK: addrspacecast %class.D* %d to %class.D addrspace(4)*
+  //CHECK: call i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)*
+  d.getmb();
+}
+
+//Derived and Base are in the same address space.
+
+//CHECK: define linkonce_odr i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)* 
%this)
+//CHECK: bitcast %class.D addrspace(4)* %this1 to %struct.B addrspace(4)*
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -2660,8 +2660,13 @@
   bool PointerConversions = false;
   if (isa(Member)) {
 DestRecordType = Context.getCanonicalType(Context.getTypeDeclType(RD));
+auto FromPtrType = FromType->getAs();
+DestRecordType = Context.getAddrSpaceQualType(
+DestRecordType, FromPtrType
+? FromType->getPointeeType().getAddressSpace()
+: FromType.getAddressSpace());
 
-if (FromType->getAs()) {
+if (FromPtrType) {
   DestType = Context.getPointerType(DestRecordType);
   FromRecordType = FromType->getPointeeType();
   PointerConversions = true;
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -302,7 +302,8 @@
 
   // Get the base pointer type.
   llvm::Type *BasePtrTy =
-ConvertType((PathEnd[-1])->getType())->getPointerTo();
+  ConvertType((PathEnd[-1])->getType())
+  ->getPointerTo(Value.getType()->getPointerAddressSpace());
 
   QualType DerivedTy = getContext().getRecordType(Derived);
   CharUnits DerivedAlign = CGM.getClassPointerAlignment(Derived);


Index: test/CodeGenOpenCLCXX/addrspace-derived-base.cl
===
--- /dev/null
+++ test/CodeGenOpenCLCXX/addrspace-derived-base.cl
@@ -0,0 +1,22 @@
+// RUN: %clang_cc1 %s -triple spir -cl-std=c++ -emit-llvm -O0 -o - | FileCheck %s
+
+struct B {
+  int mb;
+};
+
+class D : public B {
+public:
+  int getmb() { return mb; }
+};
+
+void foo() {
+  D d;
+  //CHECK: addrspacecast %class.D* %d to %class.D addrspace(4)*
+  //CHECK: call i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)*
+  d.getmb();
+}
+
+//Derived and Base are in the same address space.
+
+//CHECK: define linkonce_odr i32 @_ZNU3AS41D5getmbEv(%class.D addrspace(4)* %this)
+//CHECK: bitcast %class.D addrspace(4)* %this1 to %struct.B addrspace(4)*
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -2660,8 +2660,13 @@
   bool PointerConversions = false;
   if (isa(Member)) {
 DestRecordType = Context.getCanonicalType(Context.getTypeDeclType(RD));
+auto FromPtrType = FromType->getAs();
+DestRecordType = Context.getAddrSpaceQualType(
+DestRecordType, FromPtrType
+? FromType->getPointeeType().getAddressSpace()
+: FromType.getAddressSpace());
 
-if (FromType->getAs()) {
+if (FromPtrType) {
   DestType = Context.getPointerType(DestRecordType);
   FromRecordType = FromType->getPointeeType();
   PointerConversions = true;
Index: lib/CodeGen/CGClass.cpp
===
--- lib/CodeGen/CGClass.cpp
+++ lib/CodeGen/CGClass.cpp
@@ -302,7 +302,8 @@
 
   // Get the base pointer type.
   llvm::Type *BasePtrTy =
-ConvertType((PathEnd[-1])->getType())->getPointerTo();
+  ConvertType((PathEnd[-1])->getType())
+  ->getPointerTo(Value.getType()->getPointerAddressSpace());
 
   QualType DerivedTy = getContext().getRecordType(Derived);
   CharUnits DerivedAlign = CGM.getClassPointerAlignment(Derived);
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58494: [ASTImporter] Handle redecl chain of FunctionTemplateDecls

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik accepted this revision.
shafik added a comment.

LGTM outside of the question I had.




Comment at: lib/AST/ASTImporter.cpp:4967
+template  static auto getTemplateDefinition(T *D) -> T * {
+  auto *ToTemplatedDef = D->getTemplatedDecl()->getDefinition();
   if (!ToTemplatedDef)

Can we guarantee that `D->getTemplatedDecl()` will always return a valid 
pointer? 



Comment at: lib/AST/ASTImporter.cpp:4967
+template  static auto getTemplateDefinition(T *D) -> T * {
+  auto *ToTemplatedDef = D->getTemplatedDecl()->getDefinition();
   if (!ToTemplatedDef)

shafik wrote:
> Can we guarantee that `D->getTemplatedDecl()` will always return a valid 
> pointer? 
What other types besides `CXXRecordDecl` do we expect here? 



Comment at: lib/AST/ASTImporter.cpp:5544
   // type, and in the same context as the function we're importing.
+  // FIXME Split this into a separate function.
   if (!LexicalDC->isFunctionOrMethod()) {

Would it make sense to do the split into a separate function in the PR?



Comment at: lib/AST/ASTImporter.cpp:5595
+  auto *PrevTemplated =
+  FoundByLookup->getTemplatedDecl()->getMostRecentDecl();
+  if (TemplatedFD != PrevTemplated)

Can we guarantee that `FoundByLookup->getTemplatedDecl()` will always return a 
valid pointer? 


Repository:
  rC Clang

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

https://reviews.llvm.org/D58494



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

r355432 - [PGO] Clang part of change for context-sensitive PGO (part2)

[Rong Xu via cfe-commits]

Author: xur
Date: Tue Mar  5 11:09:56 2019
New Revision: 355432

URL: http://llvm.org/viewvc/llvm-project?rev=355432&view=rev
Log:
[PGO] Clang part of change for context-sensitive PGO (part2)

Part 2 of CSPGO change in Clang: Add test cases.

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


Added:
cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext
cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext
cfe/trunk/test/CodeGen/cspgo-instrumentation.c
cfe/trunk/test/CodeGen/cspgo-instrumentation_lto.c
cfe/trunk/test/CodeGen/cspgo-instrumentation_thinlto.c

Added: cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext?rev=355432&view=auto
==
--- cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext (added)
+++ cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext Tue Mar  5 11:09:56 2019
@@ -0,0 +1,2 @@
+# IR level Instrumentation Flag
+:ir

Added: cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext?rev=355432&view=auto
==
--- cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext (added)
+++ cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext Tue Mar  5 11:09:56 2019
@@ -0,0 +1,2 @@
+# IR level Instrumentation Flag with CS
+:csir

Added: cfe/trunk/test/CodeGen/cspgo-instrumentation.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/cspgo-instrumentation.c?rev=355432&view=auto
==
--- cfe/trunk/test/CodeGen/cspgo-instrumentation.c (added)
+++ cfe/trunk/test/CodeGen/cspgo-instrumentation.c Tue Mar  5 11:09:56 2019
@@ -0,0 +1,41 @@
+// Test if CSPGO instrumentation and use pass are invoked.
+//
+// Ensure Pass PGOInstrumentationGenPass is invoked.
+// RUN: %clang_cc1 -O2 -fprofile-instrument=csllvm 
-fprofile-instrument-path=default.profraw  %s -mllvm -debug-pass=Structure 
-emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN
+// RUN: %clang_cc1 -O2 -fprofile-instrument=csllvm 
-fprofile-instrument-path=default.profraw  %s -fexperimental-new-pass-manager 
-fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-NEWPM
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN: PGOInstrumentationGenCreateVarPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN: PGOInstrumentationGenPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-NEWPM: Running pass: 
PGOInstrumentationGenCreateVar on
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-NEWPM: Running pass: 
PGOInstrumentationGen on
+//
+// RUN: rm -rf %t && mkdir %t
+// RUN: llvm-profdata merge -o %t/noncs.profdata %S/Inputs/pgotestir.proftext
+//
+// Ensure Pass PGOInstrumentationUsePass and PGOInstrumentationGenPass are 
invoked.
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata 
-fprofile-instrument=csllvm -fprofile-instrument-path=default.profraw  %s 
-mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata 
-fprofile-instrument=csllvm -fprofile-instrument-path=default.profraw  %s 
-fexperimental-new-pass-manager -fdebug-pass-manager  -emit-llvm -o - 2>&1 | 
FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2: PGOInstrumentationUsePass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2: PGOInstrumentationGenCreateVarPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2: PGOInstrumentationGenPass
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM: Running pass: 
PGOInstrumentationUse
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM: Running pass: 
PGOInstrumentationGenCreateVar on
+// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM: Running pass: 
PGOInstrumentationGen on
+
+// Ensure Pass PGOInstrumentationUsePass is invoked only once.
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata %s 
-mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-PGOUSEPASS-INVOKED-USE
+// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata %s 
-fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | 
FileCheck %s -check-prefix=CHECK-PGOUSEPASS-INVOKED-USE-NEWPM
+// CHECK-PGOUSEPASS-INVOKED-USE: PGOInstrumentationUsePass
+// CHECK-PGOUSEPASS-INVOKED-USE-NOT: PGOInstrumentationGenCreateVarPass
+// CHECK-PGOUSEPASS-INVOKED-USE-NOT: PGOInstrumentationUsePass
+// CHECK-PGOUSEPASS-INVOKED-USE-NEWPM: Running pass: PGOInstrumentationUse
+// CHECK-PGOUSEPASS-INVOKED-USE-NEWPM-NOT: Running pass: 
PGOInstrumentationGenCreateVar
+// CHECK-PGOUSEPASS-INVOKED-USE-NEWPM-NOT: Running pass: PGOInstrumentationUse
+//
+// Ensure Pass PGOInstrumentationUse

[PATCH] D58708: [PR40778] Preserve addr space in Derived to Base cast

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: lib/Sema/SemaExpr.cpp:2671
   DestType = Context.getPointerType(DestRecordType);
   FromRecordType = FromType->getPointeeType();
   PointerConversions = true;

And here.


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

https://reviews.llvm.org/D58708



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

[PATCH] D58880: [WIP] [Looking for API feedback] [clangd] Type hierarchy subtypes

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D58880#1418195 , @hokein wrote:

> I think we can extend the existing Ref to support it, so that most of the 
> stuff could be reused, rather than implementing a new slab:
>
> - introduce a new RefKind, like BaseOf
> - add a new field SymbolID in Ref


I had considered this approach as well, but figured that it would be wasteful 
in terms of storage space.

My understanding is that the storage space taken up for Refs is currently 8 
bytes per Ref (4 each for the start and end positions), plus filename strings 
which are deduplicated across all refs. If we add a SymbolID, that adds an 
additional 8 bytes to each Ref. Given that Refs are numerous, and most of them 
won't use the SymbolID, that seems wasteful.

That said, I do appreciate that this is a simpler approach in terms of 
complexity, so if folks feel the right tradeoff is to take the Refs approach, I 
am open to doing that.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58880



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

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

[Jan Korous via Phabricator via cfe-commits]

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

In D58922#1418029 , @MyDeveloperDay 
wrote:

> do you mean this case? as this seems to work for me?
>
>   verifyFormat("namespace bar {\n"
>  "auto foo{[]() -> foo { ; }};\n"
>  "} // namespace bar");
>


I meant that if you take the test and run it on unpatched master (simulating 
hypothetical test failure in the future) the message it produces starts by 
stating that the expected string (part of the testcase) is "unstable" which 
seems a bit unclear/confusing to me.

  
/Users/jankorous/src/llvm-monorepo/llvm-project/clang/unittests/Format/FormatTest.cpp:74:
 Failure
Expected: Expected.str()
Which is: "namespace bar {\nauto foo{[]() -> foo { ; }};\n} // 
namespace bar"
  To be equal to: format(Expected, Style)
Which is: "namespace bar {\nauto foo{[]() -> foo{;\n}\n}\n;\n} 
// namespace bar"
  With diff:
  @@ -1,3 +1,6 @@
   namespace bar {
  -auto foo{[]() -> foo { ; }};
  +auto foo{[]() -> foo{;
  +}
  +}
  +;
   } // namespace bar

Whereas the full original reproducer (with the " broken:" comment included) 
also starts with the same message but the diff implies that the implementation 
is wrong by duplicating the " namespace bar" comment.

  
/Users/jankorous/src/llvm-monorepo/llvm-project/clang/unittests/Format/FormatTest.cpp:74:
 Failure
Expected: Expected.str()
Which is: "namespace bar {\n// broken:\nauto foo{[]() -> foo<5 + 2> { 
return {}; }};\n} // namespace bar"
  To be equal to: format(Expected, Style)
Which is: "namespace bar {\n// broken:\nauto foo{[]() -> foo<5 + 
2>{return {};\n} // namespace bar\n}\n;\n} // namespace bar"
  With diff:
  @@ -1,4 +1,7 @@
   namespace bar {
   // broken:
  -auto foo{[]() -> foo<5 + 2> { return {}; }};
  +auto foo{[]() -> foo<5 + 2>{return {};
  +} // namespace bar
  +}
  +;
   } // namespace bar

Anyway. LGTM.


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

https://reviews.llvm.org/D58922



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

[PATCH] D58934: [clang-format] Fix lambdas returning template specialization that contains operator in parameter

[Jan Korous via Phabricator via cfe-commits]

jkorous added a comment.

In D58934#1418043 , @MyDeveloperDay 
wrote:

> I'm not sure I personally would ever write code like that ;-) , but if its 
> legal C++ and it compiles we should handle it the same as 
> foo<1>,foo,foo


I hear you :D

> As there are a number of reviews out there for formatting Lambdas I think its 
> a good idea for us to add corner cases like this to the unit tests, but it 
> does get me thinking if this shouldn't be handled by a piece of code which 
> knows about trailing return types (template or otherwise) and not be in the 
> general Lambda parsing code
> 
> I suspect that given that the switch statement handles
> 
>   tok::identifier, tok::less, tok::numeric_constant, tok::greater
>   foo<  1  >
> 
> 
> We are effectively just consuming the return type tokens.
> 
> But to handle what you have here it LGTM and handles more use cases that 
> https://bugs.llvm.org/show_bug.cgi?id=40910 would throw up.
> 
> Thanks for helping out

I had the same exact idea but since I wasn't able to find any existing function 
I just followed your lead for now. But I assume we can just factor out this 
later.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58934



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

[PATCH] D58986: Fix typo in string returned from index::getSymbolKindString for SymbolKind::ConversionFunction

[Nathan Hawes via Phabricator via cfe-commits]

nathawes created this revision.
nathawes added a reviewer: akyrtzi.
Herald added subscribers: cfe-commits, arphaman.
Herald added a project: clang.

This patch updates  SymbolKind::ConversionFunction's string from 
"coversion-func" to "conversion-func" and extends the 
Index/Core/index-source.cpp test to cover the indexing of conversion functions. 
Also changed a few repeated explicit USR matches in that test to refer to 
previous matches instead.


Repository:
  rC Clang

https://reviews.llvm.org/D58986

Files:
  lib/Index/IndexSymbol.cpp
  test/Index/Core/index-source.cpp

Index: test/Index/Core/index-source.cpp
===
--- test/Index/Core/index-source.cpp
+++ test/Index/Core/index-source.cpp
@@ -1,21 +1,27 @@
 // RUN: c-index-test core -print-source-symbols -- %s -std=c++1z -target x86_64-apple-macosx10.7 | FileCheck %s
 // RUN: c-index-test core -print-source-symbols -include-locals -- %s -std=c++1z -target x86_64-apple-macosx10.7 | FileCheck -check-prefix=LOCAL %s
 
+// CHECK: [[@LINE+1]]:7 | class/C++ | Other | [[Other_USR:.*]] |  | Def | rel: 0
+class Other {};
+
 // CHECK: [[@LINE+1]]:7 | class/C++ | Cls | [[Cls_USR:.*]] |  | Def | rel: 0
 class Cls { public:
   // CHECK: [[@LINE+3]]:3 | constructor/C++ | Cls | c:@S@Cls@F@Cls#I# | __ZN3ClsC1Ei | Decl,RelChild | rel: 1
-  // CHECK-NEXT: RelChild | Cls | c:@S@Cls
-  // CHECK: [[@LINE+1]]:3 | class/C++ | Cls | c:@S@Cls |  | Ref,RelCont | rel: 1
+  // CHECK-NEXT: RelChild | Cls | [[Cls_USR]]
+  // CHECK: [[@LINE+1]]:3 | class/C++ | Cls | [[Cls_USR]] |  | Ref,RelCont | rel: 1
   Cls(int x);
   // CHECK: [[@LINE+2]]:3 | constructor/cxx-copy-ctor/C++ | Cls | c:@S@Cls@F@Cls#&1$@S@Cls# | __ZN3ClsC1ERKS_ | Decl,RelChild | rel: 1
-  // CHECK: [[@LINE+1]]:3 | class/C++ | Cls | c:@S@Cls |  | Ref,RelCont | rel: 1
+  // CHECK: [[@LINE+1]]:3 | class/C++ | Cls | [[Cls_USR]] |  | Ref,RelCont | rel: 1
   Cls(const Cls &);
   // CHECK: [[@LINE+2]]:3 | constructor/cxx-move-ctor/C++ | Cls | c:@S@Cls@F@Cls#&&$@S@Cls# | __ZN3ClsC1EOS_ | Decl,RelChild | rel: 1
-  // CHECK: [[@LINE+1]]:3 | class/C++ | Cls | c:@S@Cls |  | Ref,RelCont | rel: 1
+  // CHECK: [[@LINE+1]]:3 | class/C++ | Cls | [[Cls_USR]] |  | Ref,RelCont | rel: 1
   Cls(Cls &&);
+  // CHECK: [[@LINE+2]]:3 | conversion-func/C++ | operator Other | [[ClsOtherConv_USR:c:@S@Cls@F@operator Other#]] | [[ClsOtherConv_CGName:.*]] | Def,RelChild | rel: 1
+  // CHECK: [[@LINE+1]]:12 | class/C++ | Other | [[Other_USR]] |  | Ref,RelCont | rel: 1
+  operator Other() { return Other(); }
 
   // CHECK: [[@LINE+2]]:3 | destructor/C++ | ~Cls | c:@S@Cls@F@~Cls# | __ZN3ClsD1Ev | Decl,RelChild | rel: 1
-  // CHECK: [[@LINE+1]]:4 | class/C++ | Cls | c:@S@Cls |  | Ref,RelCont | rel: 1
+  // CHECK: [[@LINE+1]]:4 | class/C++ | Cls | [[Cls_USR]] |  | Ref,RelCont | rel: 1
   ~Cls();
 };
 
@@ -34,13 +40,20 @@
 
 Cls::Cls(int x) {}
 // CHECK: [[@LINE-1]]:6 | constructor/C++ | Cls | c:@S@Cls@F@Cls#I# | __ZN3ClsC1Ei | Def,RelChild | rel: 1
-// CHECK: [[@LINE-2]]:1 | class/C++ | Cls | c:@S@Cls |  | Ref,RelCont | rel: 1
-// CHECK: [[@LINE-3]]:6 | class/C++ | Cls | c:@S@Cls |  | Ref,RelCont | rel: 1
+// CHECK: [[@LINE-2]]:1 | class/C++ | Cls | [[Cls_USR]] |  | Ref,RelCont | rel: 1
+// CHECK: [[@LINE-3]]:6 | class/C++ | Cls | [[Cls_USR]] |  | Ref,RelCont | rel: 1
 
 Cls::~/*a comment*/Cls() {}
 // CHECK: [[@LINE-1]]:6 | destructor/C++ | ~Cls | c:@S@Cls@F@~Cls# | __ZN3ClsD1Ev | Def,RelChild | rel: 1
-// CHECK: [[@LINE-2]]:1 | class/C++ | Cls | c:@S@Cls |  | Ref,RelCont | rel: 1
-// CHECK: [[@LINE-3]]:20 | class/C++ | Cls | c:@S@Cls |  | Ref,RelCont | rel: 1
+// CHECK: [[@LINE-2]]:1 | class/C++ | Cls | [[Cls_USR]] |  | Ref,RelCont | rel: 1
+// CHECK: [[@LINE-3]]:20 | class/C++ | Cls | [[Cls_USR]] |  | Ref,RelCont | rel: 1
+
+void testConversion() {
+  Cls foo(2);
+
+  // CHECK: [[@LINE+1]]:15 | conversion-func/C++ | operator Other | [[ClsOtherConv_USR]] | [[ClsOtherConv_CGName]] | Ref,Call,RelCall,RelCont | rel: 1
+  Other bar = foo;
+}
 
 template 
 class TemplCls {
@@ -298,17 +311,17 @@
 // CHECK: [[@LINE-1]]:7 | class(Gen,TPS)/C++ | PartialSpecilizationClass | c:@SP>1#T@PartialSpecilizationClass>#$@S@Cls#t0.0 |  | Decl,RelSpecialization | rel: 1
 // CHECK-NEXT: RelSpecialization | PartialSpecilizationClass | c:@ST>2#T#T@PartialSpecilizationClass
 // CHECK: [[@LINE-3]]:7 | class(Gen)/C++ | PartialSpecilizationClass | c:@ST>2#T#T@PartialSpecilizationClass |  | Ref | rel: 0
-// CHECK-NEXT: [[@LINE-4]]:33 | class/C++ | Cls | c:@S@Cls |  | Ref | rel: 0
+// CHECK-NEXT: [[@LINE-4]]:33 | class/C++ | Cls | [[Cls_USR]] |  | Ref | rel: 0
 
 template<>
 class PartialSpecilizationClass : Cls { };
 // CHECK: [[@LINE-1]]:7 | class(Gen,TS)/C++ | PartialSpecilizationClass | c:@S@PartialSpecilizationClass>#$@S@Cls#S0_ |  | Def,RelSpecialization | rel: 1
 // CHECK-NEXT: RelSpecialization | PartialSpecilizationClass | c:@ST>2#T#T@PartialSpecilizationClass
-// CHECK-NEXT: [[@LINE-3]]:45 | class/C++ | Cls | 

r355434 - [clang-format] Fix lambdas returning template specialization that contains operator in parameter

[Jan Korous via cfe-commits]

Author: jkorous
Date: Tue Mar  5 11:27:24 2019
New Revision: 355434

URL: http://llvm.org/viewvc/llvm-project?rev=355434&view=rev
Log:
[clang-format] Fix lambdas returning template specialization that contains 
operator in parameter

A template specialization of a template foo can contain integer 
constants and a whole bunch of operators - e. g.  foo< 1 ? !0 : (3+1)%4 >

Inspired by https://reviews.llvm.org/D58922

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

Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=355434&r1=355433&r2=355434&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Tue Mar  5 11:27:24 2019
@@ -1423,6 +1423,25 @@ bool UnwrappedLineParser::tryToParseLamb
 case tok::coloncolon:
 case tok::kw_mutable:
 case tok::kw_noexcept:
+// Specialization of a template with an integer parameter can contain
+// arithmetic, logical, comparison and ternary operators.
+case tok::plus:
+case tok::minus:
+case tok::exclaim:
+case tok::tilde:
+case tok::slash:
+case tok::percent:
+case tok::lessless:
+case tok::pipe:
+case tok::pipepipe:
+case tok::ampamp:
+case tok::caret:
+case tok::equalequal:
+case tok::exclaimequal:
+case tok::greaterequal:
+case tok::lessequal:
+case tok::question:
+case tok::colon:
   nextToken();
   break;
 case tok::arrow:

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=355434&r1=355433&r2=355434&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Mar  5 11:27:24 2019
@@ -11846,6 +11846,96 @@ TEST_F(FormatTest, FormatsLambdas) {
   verifyGoogleFormat("auto a = [&b, c](D* d) -> D& {};");
   verifyGoogleFormat("auto a = [&b, c](D* d) -> const D* {};");
   verifyFormat("[a, a]() -> a<1> {};");
+  verifyFormat("[]() -> foo<5 + 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 - 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 / 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 * 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 % 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 << 2> { return {}; };");
+  verifyFormat("[]() -> foo { return {}; };");
+  verifyFormat("[]() -> foo<~5> { return {}; };");
+  verifyFormat("[]() -> foo<5 | 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 || 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 & 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 && 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 == 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 != 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 >= 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 <= 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 < 2> { return {}; };");
+  verifyFormat("[]() -> foo<2 ? 1 : 0> { return {}; };");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 + 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 - 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 / 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 * 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 % 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 << 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<~5> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 | 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 || 2> { return {}; }};\n"
+  "} // namespace bar")

[PATCH] D58934: [clang-format] Fix lambdas returning template specialization that contains operator in parameter

[Jan Korous via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL355434: [clang-format] Fix lambdas returning template 
specialization that contains… (authored by jkorous, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58934?vs=189223&id=189376#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58934

Files:
  cfe/trunk/lib/Format/UnwrappedLineParser.cpp
  cfe/trunk/unittests/Format/FormatTest.cpp

Index: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
===
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp
@@ -1423,6 +1423,25 @@
 case tok::coloncolon:
 case tok::kw_mutable:
 case tok::kw_noexcept:
+// Specialization of a template with an integer parameter can contain
+// arithmetic, logical, comparison and ternary operators.
+case tok::plus:
+case tok::minus:
+case tok::exclaim:
+case tok::tilde:
+case tok::slash:
+case tok::percent:
+case tok::lessless:
+case tok::pipe:
+case tok::pipepipe:
+case tok::ampamp:
+case tok::caret:
+case tok::equalequal:
+case tok::exclaimequal:
+case tok::greaterequal:
+case tok::lessequal:
+case tok::question:
+case tok::colon:
   nextToken();
   break;
 case tok::arrow:
Index: cfe/trunk/unittests/Format/FormatTest.cpp
===
--- cfe/trunk/unittests/Format/FormatTest.cpp
+++ cfe/trunk/unittests/Format/FormatTest.cpp
@@ -11846,6 +11846,96 @@
   verifyGoogleFormat("auto a = [&b, c](D* d) -> D& {};");
   verifyGoogleFormat("auto a = [&b, c](D* d) -> const D* {};");
   verifyFormat("[a, a]() -> a<1> {};");
+  verifyFormat("[]() -> foo<5 + 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 - 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 / 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 * 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 % 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 << 2> { return {}; };");
+  verifyFormat("[]() -> foo { return {}; };");
+  verifyFormat("[]() -> foo<~5> { return {}; };");
+  verifyFormat("[]() -> foo<5 | 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 || 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 & 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 && 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 == 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 != 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 >= 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 <= 2> { return {}; };");
+  verifyFormat("[]() -> foo<5 < 2> { return {}; };");
+  verifyFormat("[]() -> foo<2 ? 1 : 0> { return {}; };");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 + 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 - 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 / 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 * 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 % 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 << 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<~5> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 | 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 || 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 & 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  "auto foo{[]() -> foo<5 && 2> { return {}; }};\n"
+  "} // namespace bar");
+  verifyFormat("namespace bar {\n"
+  "// broken:\n"
+  

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jason Liu via Phabricator via cfe-commits]

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



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

apaprocki wrote:
> No need to be `sorry:` :) This should probably just say `error: XCOFF is 
> unimplemented` to be more direct in case anything is expecting "error:" in 
> the output.
Sure. Will address in next revision.



Comment at: llvm/lib/Support/Triple.cpp:537
   return StringSwitch(EnvironmentName)
+// FIXME: We have to put XCOFF before COFF;
+// perhaps an order-independent pattern matching is desired?

apaprocki wrote:
> hubert.reinterpretcast wrote:
> > If the conclusion is that checking XCOFF before COFF is fine, then we 
> > should remove the FIXME and just leave a normal comment.
> Agreed, existing code seems fine as long as there is a comment explaining 
> that `xcoff` must come before `coff` in case it isn't obvious at a quick 
> glance.
Sounds good. I will remove FIXME and leave a normal comment to indicate the 
order dependency. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[PATCH] D58821: Inline asm constraints: allow ICE-like pointers for the "n" constraint (PR40890)

[Eli Friedman via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D58821



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

[PATCH] D58811: [clang] Fix misuses of char width to char align

[Eli Friedman via Phabricator via cfe-commits]

efriedma added a comment.

> The patch have been tested for a proprietary architecture, where there is a 
> difference between align and width of char.

The C standard requires that both  `alignof(char)` and `sizeof(char)` must 
equal 1, and therefore must equal each other.  Could you clarify the 
characteristics of your target?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58811



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

[PATCH] D58992: [CUDA][HIP][DebugInfo] Skip reference device function

[Michael Liao via Phabricator via cfe-commits]

hliao created this revision.
Herald added subscribers: cfe-commits, jdoerfert, aprantl.
Herald added a project: clang.

- A device functions could be used as a non-type template parameter in a 
global/host function template. However, we should not try to retrieve that 
device function and reference it in the host-side debug info as it's only valid 
at device side.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58992

Files:
  clang/lib/CodeGen/CGDebugInfo.cpp
  clang/test/CodeGenCUDA/debug-info-template.cu


Index: clang/test/CodeGenCUDA/debug-info-template.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/debug-info-template.cu
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - 
-debug-info-kind=limited -dwarf-version=2 -debugger-tuning=gdb | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+__device__ void f();
+template __global__ void t() { F(); }
+__host__ void g() { t<<<1,1>>>(); }
+
+// Ensure the value of device-function (as value template parameter) in the is
+// null.
+// CHECK: !DITemplateValueParameter(name: "F", type: !34, value: null)
Index: clang/lib/CodeGen/CGDebugInfo.cpp
===
--- clang/lib/CodeGen/CGDebugInfo.cpp
+++ clang/lib/CodeGen/CGDebugInfo.cpp
@@ -1725,31 +1725,37 @@
   QualType T = TA.getParamTypeForDecl().getDesugaredType(CGM.getContext());
   llvm::DIType *TTy = getOrCreateType(T, Unit);
   llvm::Constant *V = nullptr;
-  const CXXMethodDecl *MD;
-  // Variable pointer template parameters have a value that is the address
-  // of the variable.
-  if (const auto *VD = dyn_cast(D))
-V = CGM.GetAddrOfGlobalVar(VD);
-  // Member function pointers have special support for building them, 
though
-  // this is currently unsupported in LLVM CodeGen.
-  else if ((MD = dyn_cast(D)) && MD->isInstance())
-V = CGM.getCXXABI().EmitMemberFunctionPointer(MD);
-  else if (const auto *FD = dyn_cast(D))
-V = CGM.GetAddrOfFunction(FD);
-  // Member data pointers have special handling too to compute the fixed
-  // offset within the object.
-  else if (const auto *MPT = dyn_cast(T.getTypePtr())) {
-// These five lines (& possibly the above member function pointer
-// handling) might be able to be refactored to use similar code in
-// CodeGenModule::getMemberPointerConstant
-uint64_t fieldOffset = CGM.getContext().getFieldOffset(D);
-CharUnits chars =
-CGM.getContext().toCharUnitsFromBits((int64_t)fieldOffset);
-V = CGM.getCXXABI().EmitMemberDataPointer(MPT, chars);
+  // Skip retrieve the value if that template parameter has cuda device
+  // attribute, i.e. that value is not available at the host side.
+  if (!CGM.getLangOpts().CUDA || CGM.getLangOpts().CUDAIsDevice ||
+  !D->hasAttr()) {
+const CXXMethodDecl *MD;
+// Variable pointer template parameters have a value that is the 
address
+// of the variable.
+if (const auto *VD = dyn_cast(D))
+  V = CGM.GetAddrOfGlobalVar(VD);
+// Member function pointers have special support for building them,
+// though this is currently unsupported in LLVM CodeGen.
+else if ((MD = dyn_cast(D)) && MD->isInstance())
+  V = CGM.getCXXABI().EmitMemberFunctionPointer(MD);
+else if (const auto *FD = dyn_cast(D))
+  V = CGM.GetAddrOfFunction(FD);
+// Member data pointers have special handling too to compute the fixed
+// offset within the object.
+else if (const auto *MPT =
+ dyn_cast(T.getTypePtr())) {
+  // These five lines (& possibly the above member function pointer
+  // handling) might be able to be refactored to use similar code in
+  // CodeGenModule::getMemberPointerConstant
+  uint64_t fieldOffset = CGM.getContext().getFieldOffset(D);
+  CharUnits chars =
+  CGM.getContext().toCharUnitsFromBits((int64_t)fieldOffset);
+  V = CGM.getCXXABI().EmitMemberDataPointer(MPT, chars);
+}
+V = V->stripPointerCasts();
   }
   TemplateParams.push_back(DBuilder.createTemplateValueParameter(
-  TheCU, Name, TTy,
-  cast_or_null(V->stripPointerCasts(;
+  TheCU, Name, TTy, cast_or_null(V)));
 } break;
 case TemplateArgument::NullPtr: {
   QualType T = TA.getNullPtrType();


Index: clang/test/CodeGenCUDA/debug-info-template.cu
===
--- /dev/null
+++ clang/test/CodeGenCUDA/debug-info-template.cu
@@ -0,0 +1,11 @@
+// RUN: %clang_cc1 %s --std=c++11 -triple x86_64-unknown-linux -emit-llvm -o - -debug-info-kind=limited -dwarf-version=2 -debugger-tuning=gdb | FileCheck %s
+
+#include "Inputs/cuda.h"
+
+__device__ void f();
+templat

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jason Liu via Phabricator via cfe-commits]

jasonliu updated this revision to Diff 189387.
jasonliu added a comment.

Address some review comments


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930

Files:
  clang/lib/CodeGen/BackendUtil.cpp
  clang/lib/CodeGen/CodeGenModule.cpp
  lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp
  llvm/include/llvm/ADT/Triple.h
  llvm/include/llvm/MC/MCObjectFileInfo.h
  llvm/lib/MC/MCContext.cpp
  llvm/lib/MC/MCObjectFileInfo.cpp
  llvm/lib/MC/MCParser/AsmParser.cpp
  llvm/lib/Support/Triple.cpp
  llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
  llvm/unittests/ADT/TripleTest.cpp

Index: llvm/unittests/ADT/TripleTest.cpp
===
--- llvm/unittests/ADT/TripleTest.cpp
+++ llvm/unittests/ADT/TripleTest.cpp
@@ -1258,6 +1258,11 @@
   EXPECT_EQ(Triple::Wasm,
 Triple("wasm64-unknown-wasi-musl-wasm").getObjectFormat());
 
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc-ibm-aix").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc64-ibm-aix").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc---xcoff").getObjectFormat());
+  EXPECT_EQ(Triple::XCOFF, Triple("powerpc64---xcoff").getObjectFormat());
+
   Triple MSVCNormalized(Triple::normalize("i686-pc-windows-msvc-elf"));
   EXPECT_EQ(Triple::ELF, MSVCNormalized.getObjectFormat());
 
@@ -1276,6 +1281,9 @@
 
   T.setObjectFormat(Triple::MachO);
   EXPECT_EQ(Triple::MachO, T.getObjectFormat());
+  
+  T.setObjectFormat(Triple::XCOFF);
+  EXPECT_EQ(Triple::XCOFF, T.getObjectFormat());
 }
 
 TEST(TripleTest, NormalizeWindows) {
Index: llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
===
--- llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
+++ llvm/lib/Target/ARM/AsmParser/ARMAsmParser.cpp
@@ -5594,6 +5594,9 @@
   case MCObjectFileInfo::IsWasm:
 CurrentFormat = WASM;
 break;
+  case MCObjectFileInfo::IsXCOFF:
+llvm_unreachable("unexpected object format");
+break;
   }
 
   if (~Prefix->SupportedFormats & CurrentFormat) {
Index: llvm/lib/Support/Triple.cpp
===
--- llvm/lib/Support/Triple.cpp
+++ llvm/lib/Support/Triple.cpp
@@ -534,6 +534,9 @@
 
 static Triple::ObjectFormatType parseFormat(StringRef EnvironmentName) {
   return StringSwitch(EnvironmentName)
+// "xcoff" must come before "coff" because of the order-dependendent
+// pattern matching.
+.EndsWith("xcoff", Triple::XCOFF)
 .EndsWith("coff", Triple::COFF)
 .EndsWith("elf", Triple::ELF)
 .EndsWith("macho", Triple::MachO)
@@ -622,6 +625,7 @@
   case Triple::ELF: return "elf";
   case Triple::MachO: return "macho";
   case Triple::Wasm: return "wasm";
+  case Triple::XCOFF: return "xcoff";
   }
   llvm_unreachable("unknown object format type");
 }
@@ -686,6 +690,8 @@
   case Triple::ppc64:
 if (T.isOSDarwin())
   return Triple::MachO;
+else if (T.isOSAIX())
+  return Triple::XCOFF;
 return Triple::ELF;
 
   case Triple::wasm32:
Index: llvm/lib/MC/MCParser/AsmParser.cpp
===
--- llvm/lib/MC/MCParser/AsmParser.cpp
+++ llvm/lib/MC/MCParser/AsmParser.cpp
@@ -710,6 +710,9 @@
   case MCObjectFileInfo::IsWasm:
 PlatformParser.reset(createWasmAsmParser());
 break;
+  case MCObjectFileInfo::IsXCOFF:
+// TODO: Need to implement createXCOFFAsmParser for XCOFF format.
+break;
   }
 
   PlatformParser->Initialize(*this);
Index: llvm/lib/MC/MCObjectFileInfo.cpp
===
--- llvm/lib/MC/MCObjectFileInfo.cpp
+++ llvm/lib/MC/MCObjectFileInfo.cpp
@@ -801,6 +801,10 @@
 Env = IsWasm;
 initWasmMCObjectFileInfo(TT);
 break;
+  case Triple::XCOFF:
+Env = IsXCOFF;
+// TODO: Initialize MCObjectFileInfo for XCOFF format when MCSectionXCOFF is ready.
+break;
   case Triple::UnknownObjectFormat:
 report_fatal_error("Cannot initialize MC for unknown object file format.");
 break;
@@ -816,6 +820,7 @@
   case Triple::MachO:
   case Triple::COFF:
   case Triple::Wasm:
+  case Triple::XCOFF:
   case Triple::UnknownObjectFormat:
 report_fatal_error("Cannot get DWARF comdat section for this object file "
"format: not implemented.");
Index: llvm/lib/MC/MCContext.cpp
===
--- llvm/lib/MC/MCContext.cpp
+++ llvm/lib/MC/MCContext.cpp
@@ -161,6 +161,9 @@
   return new (Name, *this) MCSymbolMachO(Name, IsTemporary);
 case MCObjectFileInfo::IsWasm:
   return new (Name, *this) MCSymbolWasm(Name, IsTemporary);
+case MCObjectFileInfo::IsXCOFF:
+  // TODO: Need to implement class MCSymbolXCOFF.
+  break;
 }
   }
   return new (Name, *this) MCSymbol(MCSymbol::SymbolKindUnset, Name,
Index: llvm/include/llvm/MC/M

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere requested changes to this revision.
JDevlieghere added inline comments.
This revision now requires changes to proceed.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1470
+  case Triple::XCOFF:
+// TODO: Falling through for XCOFF format for now.
+break;

This is confusing, you say fall through but you break? I would prefer a 
`llvm_unreachable("XCOFF not yet implemented");` here and elsewhere in this 
patch. 




Comment at: clang/lib/CodeGen/BackendUtil.cpp:1486
+  case Triple::XCOFF:
+// TODO: Falling through for XCOFF format for now.
+break;

See previous comment.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4410
+  case llvm::Triple::XCOFF:
+llvm_unreachable("to be determined for XCOFF format");
   case llvm::Triple::COFF:

See previous comment.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

jasonliu wrote:
> apaprocki wrote:
> > No need to be `sorry:` :) This should probably just say `error: XCOFF is 
> > unimplemented` to be more direct in case anything is expecting "error:" in 
> > the output.
> Sure. Will address in next revision.
Just bundle this with the WASM case, the error message is correct for both.



Comment at: llvm/lib/MC/MCContext.cpp:165
+case MCObjectFileInfo::IsXCOFF:
+  // TODO: Need to implement class MCSymbolXCOFF.
+  break;

See previous comment.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

r355440 - [cmake] Add libRemarks to LLVM_DISTRIBUTION_COMPONENTS

[Francis Visoiu Mistrih via cfe-commits]

Author: thegameg
Date: Tue Mar  5 12:47:34 2019
New Revision: 355440

URL: http://llvm.org/viewvc/llvm-project?rev=355440&view=rev
Log:
[cmake] Add libRemarks to LLVM_DISTRIBUTION_COMPONENTS

Add this in the Apple-stage2.cmake to ship the remark tooling library
with the compiler.

Modified:
cfe/trunk/cmake/caches/Apple-stage2.cmake

Modified: cfe/trunk/cmake/caches/Apple-stage2.cmake
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/cmake/caches/Apple-stage2.cmake?rev=355440&r1=355439&r2=355440&view=diff
==
--- cfe/trunk/cmake/caches/Apple-stage2.cmake (original)
+++ cfe/trunk/cmake/caches/Apple-stage2.cmake Tue Mar  5 12:47:34 2019
@@ -62,6 +62,7 @@ set(LLVM_DISTRIBUTION_COMPONENTS
   clang-format
   clang-resource-headers
   cxx-headers
+  Remarks
   ${LLVM_TOOLCHAIN_TOOLS}
   CACHE STRING "")
 


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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jason Liu via Phabricator via cfe-commits]

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



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

JDevlieghere wrote:
> jasonliu wrote:
> > apaprocki wrote:
> > > No need to be `sorry:` :) This should probably just say `error: XCOFF is 
> > > unimplemented` to be more direct in case anything is expecting "error:" 
> > > in the output.
> > Sure. Will address in next revision.
> Just bundle this with the WASM case, the error message is correct for both.
I think they are different. 
The error message for WASM seems to suggest that it will never ever get 
supported on WASM. 
But it is not the case for XCOFF, we want to indicate that it is not 
implemented yet.  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

r355442 - [docs] Update the list of ThreadSanitizer supported OSes

[Vitaly Buka via cfe-commits]

Author: vitalybuka
Date: Tue Mar  5 12:53:34 2019
New Revision: 355442

URL: http://llvm.org/viewvc/llvm-project?rev=355442&view=rev
Log:
[docs] Update the list of ThreadSanitizer supported OSes

Modified:
cfe/trunk/docs/ThreadSanitizer.rst

Modified: cfe/trunk/docs/ThreadSanitizer.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ThreadSanitizer.rst?rev=355442&r1=355441&r2=355442&view=diff
==
--- cfe/trunk/docs/ThreadSanitizer.rst (original)
+++ cfe/trunk/docs/ThreadSanitizer.rst Tue Mar  5 12:53:34 2019
@@ -19,9 +19,11 @@ Supported Platforms
 
 ThreadSanitizer is supported on the following OS:
 
+* Android
+* Darwin
+* FreeBSD
 * Linux
 * NetBSD
-* FreeBSD
 
 Support for other 64-bit architectures is possible, contributions are welcome.
 Support for 32-bit platforms is problematic and is not planned.


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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jason Liu via Phabricator via cfe-commits]

jasonliu marked 3 inline comments as done.
jasonliu added inline comments.



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1470
+  case Triple::XCOFF:
+// TODO: Falling through for XCOFF format for now.
+break;

JDevlieghere wrote:
> This is confusing, you say fall through but you break? I would prefer a 
> `llvm_unreachable("XCOFF not yet implemented");` here and elsewhere in this 
> patch. 
> 
Thanks for the comment. Will address in next revision. 



Comment at: clang/lib/CodeGen/BackendUtil.cpp:1486
+  case Triple::XCOFF:
+// TODO: Falling through for XCOFF format for now.
+break;

JDevlieghere wrote:
> See previous comment.
Will address in next revision.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:4410
+  case llvm::Triple::XCOFF:
+llvm_unreachable("to be determined for XCOFF format");
   case llvm::Triple::COFF:

JDevlieghere wrote:
> See previous comment.
Will address in next revision.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

r355445 - [docs] Add some architectures into the list of supported ThreadSanitizer platforms

[Vitaly Buka via cfe-commits]

Author: vitalybuka
Date: Tue Mar  5 13:10:42 2019
New Revision: 355445

URL: http://llvm.org/viewvc/llvm-project?rev=355445&view=rev
Log:
[docs] Add some architectures into the list of supported ThreadSanitizer 
platforms

Some platforms for which TSAN has build rules are omitted for the lack of
known build bots.

Modified:
cfe/trunk/docs/ThreadSanitizer.rst

Modified: cfe/trunk/docs/ThreadSanitizer.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/ThreadSanitizer.rst?rev=355445&r1=355444&r2=355445&view=diff
==
--- cfe/trunk/docs/ThreadSanitizer.rst (original)
+++ cfe/trunk/docs/ThreadSanitizer.rst Tue Mar  5 13:10:42 2019
@@ -19,10 +19,10 @@ Supported Platforms
 
 ThreadSanitizer is supported on the following OS:
 
-* Android
-* Darwin
+* Android aarch64, x86_64
+* Darwin arm64, x86_64
 * FreeBSD
-* Linux
+* Linux aarch64, x86_64, powerpc64, powerpc64le
 * NetBSD
 
 Support for other 64-bit architectures is possible, contributions are welcome.


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

[PATCH] D58673: [ASTImporter] Fix redecl failures of ClassTemplateSpec

[Shafik Yaghmour via Phabricator via cfe-commits]

shafik added inline comments.



Comment at: lib/AST/ASTImporter.cpp:5147
   }
+} else { // ODR violation.
+  // FIXME HandleNameConflict

ODR violations are ill-formed no diagnostic required. So currently will this 
fail for cases that clang proper would not?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58673



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

r355450 - [clang-format] broken after lambda with return type template with boolean literal

[Paul Hoad via cfe-commits]

Author: paulhoad
Date: Tue Mar  5 14:20:25 2019
New Revision: 355450

URL: http://llvm.org/viewvc/llvm-project?rev=355450&view=rev
Log:
[clang-format] broken after lambda with return type template with boolean 
literal

Summary:
A Lamdba with a return type template with a boolean literal (true,false) 
behaves differently to an integer literal

https://bugs.llvm.org/show_bug.cgi?id=40910

Reviewers: klimek, djasper, JonasToth, alexfh, krasimir, jkorous

Reviewed By: jkorous

Subscribers: jkorous, cfe-commits

Tags: #clang-tools-extra

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

Modified:
cfe/trunk/lib/Format/UnwrappedLineParser.cpp
cfe/trunk/unittests/Format/FormatTest.cpp

Modified: cfe/trunk/lib/Format/UnwrappedLineParser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Format/UnwrappedLineParser.cpp?rev=355450&r1=355449&r2=355450&view=diff
==
--- cfe/trunk/lib/Format/UnwrappedLineParser.cpp (original)
+++ cfe/trunk/lib/Format/UnwrappedLineParser.cpp Tue Mar  5 14:20:25 2019
@@ -1442,6 +1442,8 @@ bool UnwrappedLineParser::tryToParseLamb
 case tok::lessequal:
 case tok::question:
 case tok::colon:
+case tok::kw_true:
+case tok::kw_false:
   nextToken();
   break;
 case tok::arrow:

Modified: cfe/trunk/unittests/Format/FormatTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Format/FormatTest.cpp?rev=355450&r1=355449&r2=355450&view=diff
==
--- cfe/trunk/unittests/Format/FormatTest.cpp (original)
+++ cfe/trunk/unittests/Format/FormatTest.cpp Tue Mar  5 14:20:25 2019
@@ -11936,6 +11936,21 @@ TEST_F(FormatTest, FormatsLambdas) {
   "// broken:\n"
   "auto foo{[]() -> foo<2 ? 1 : 0> { return {}; }};\n"
   "} // namespace bar");
+  verifyFormat("[]() -> a<1> {};");
+  verifyFormat("[]() -> a<1> { ; };");
+  verifyFormat("[]() -> a<1> { ; }();");
+  verifyFormat("[a, a]() -> a {};");
+  verifyFormat("[]() -> a {};");
+  verifyFormat("[]() -> a { ; };");
+  verifyFormat("[]() -> a { ; }();");
+  verifyFormat("[a, a]() -> a {};");
+  verifyFormat("[]() -> a {};");
+  verifyFormat("[]() -> a { ; };");
+  verifyFormat("[]() -> a { ; }();");
+  verifyFormat("auto foo{[]() -> foo { ; }};");
+  verifyFormat("namespace bar {\n"
+   "auto foo{[]() -> foo { ; }};\n"
+   "} // namespace bar");
   verifyFormat("auto  = [](int i, // break for some reason\n"
"   int j) -> int {\n"
"  return (i * 
j);\n"


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

[PATCH] D58922: [clang-format] broken after lambda with return type template with boolean literal

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC355450: [clang-format] broken after lambda with return type 
template with boolean… (authored by paulhoad, committed by ).
Herald added a project: clang.

Changed prior to commit:
  https://reviews.llvm.org/D58922?vs=189190&id=189399#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58922

Files:
  lib/Format/UnwrappedLineParser.cpp
  unittests/Format/FormatTest.cpp


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1442,6 +1442,8 @@
 case tok::lessequal:
 case tok::question:
 case tok::colon:
+case tok::kw_true:
+case tok::kw_false:
   nextToken();
   break;
 case tok::arrow:
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11936,6 +11936,21 @@
   "// broken:\n"
   "auto foo{[]() -> foo<2 ? 1 : 0> { return {}; }};\n"
   "} // namespace bar");
+  verifyFormat("[]() -> a<1> {};");
+  verifyFormat("[]() -> a<1> { ; };");
+  verifyFormat("[]() -> a<1> { ; }();");
+  verifyFormat("[a, a]() -> a {};");
+  verifyFormat("[]() -> a {};");
+  verifyFormat("[]() -> a { ; };");
+  verifyFormat("[]() -> a { ; }();");
+  verifyFormat("[a, a]() -> a {};");
+  verifyFormat("[]() -> a {};");
+  verifyFormat("[]() -> a { ; };");
+  verifyFormat("[]() -> a { ; }();");
+  verifyFormat("auto foo{[]() -> foo { ; }};");
+  verifyFormat("namespace bar {\n"
+   "auto foo{[]() -> foo { ; }};\n"
+   "} // namespace bar");
   verifyFormat("auto  = [](int i, // break for some reason\n"
"   int j) -> int {\n"
"  return (i * 
j);\n"


Index: lib/Format/UnwrappedLineParser.cpp
===
--- lib/Format/UnwrappedLineParser.cpp
+++ lib/Format/UnwrappedLineParser.cpp
@@ -1442,6 +1442,8 @@
 case tok::lessequal:
 case tok::question:
 case tok::colon:
+case tok::kw_true:
+case tok::kw_false:
   nextToken();
   break;
 case tok::arrow:
Index: unittests/Format/FormatTest.cpp
===
--- unittests/Format/FormatTest.cpp
+++ unittests/Format/FormatTest.cpp
@@ -11936,6 +11936,21 @@
   "// broken:\n"
   "auto foo{[]() -> foo<2 ? 1 : 0> { return {}; }};\n"
   "} // namespace bar");
+  verifyFormat("[]() -> a<1> {};");
+  verifyFormat("[]() -> a<1> { ; };");
+  verifyFormat("[]() -> a<1> { ; }();");
+  verifyFormat("[a, a]() -> a {};");
+  verifyFormat("[]() -> a {};");
+  verifyFormat("[]() -> a { ; };");
+  verifyFormat("[]() -> a { ; }();");
+  verifyFormat("[a, a]() -> a {};");
+  verifyFormat("[]() -> a {};");
+  verifyFormat("[]() -> a { ; };");
+  verifyFormat("[]() -> a { ; }();");
+  verifyFormat("auto foo{[]() -> foo { ; }};");
+  verifyFormat("namespace bar {\n"
+   "auto foo{[]() -> foo { ; }};\n"
+   "} // namespace bar");
   verifyFormat("auto  = [](int i, // break for some reason\n"
"   int j) -> int {\n"
"  return (i * j);\n"
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r355454 - Revert r355432 for buildbot failures in ppc64be-linux and s390x-linux

[Rong Xu via cfe-commits]

Author: xur
Date: Tue Mar  5 15:02:06 2019
New Revision: 355454

URL: http://llvm.org/viewvc/llvm-project?rev=355454&view=rev
Log:
Revert r355432 for buildbot failures in ppc64be-linux and s390x-linux

Removed:
cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext
cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext
cfe/trunk/test/CodeGen/cspgo-instrumentation.c
cfe/trunk/test/CodeGen/cspgo-instrumentation_lto.c
cfe/trunk/test/CodeGen/cspgo-instrumentation_thinlto.c

Removed: cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext?rev=355453&view=auto
==
--- cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext (original)
+++ cfe/trunk/test/CodeGen/Inputs/pgotestir.proftext (removed)
@@ -1,2 +0,0 @@
-# IR level Instrumentation Flag
-:ir

Removed: cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext?rev=355453&view=auto
==
--- cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext (original)
+++ cfe/trunk/test/CodeGen/Inputs/pgotestir_cs.proftext (removed)
@@ -1,2 +0,0 @@
-# IR level Instrumentation Flag with CS
-:csir

Removed: cfe/trunk/test/CodeGen/cspgo-instrumentation.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/cspgo-instrumentation.c?rev=355453&view=auto
==
--- cfe/trunk/test/CodeGen/cspgo-instrumentation.c (original)
+++ cfe/trunk/test/CodeGen/cspgo-instrumentation.c (removed)
@@ -1,41 +0,0 @@
-// Test if CSPGO instrumentation and use pass are invoked.
-//
-// Ensure Pass PGOInstrumentationGenPass is invoked.
-// RUN: %clang_cc1 -O2 -fprofile-instrument=csllvm 
-fprofile-instrument-path=default.profraw  %s -mllvm -debug-pass=Structure 
-emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN
-// RUN: %clang_cc1 -O2 -fprofile-instrument=csllvm 
-fprofile-instrument-path=default.profraw  %s -fexperimental-new-pass-manager 
-fdebug-pass-manager -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-NEWPM
-// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN: PGOInstrumentationGenCreateVarPass
-// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN: PGOInstrumentationGenPass
-// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-NEWPM: Running pass: 
PGOInstrumentationGenCreateVar on
-// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN-NEWPM: Running pass: 
PGOInstrumentationGen on
-//
-// RUN: rm -rf %t && mkdir %t
-// RUN: llvm-profdata merge -o %t/noncs.profdata %S/Inputs/pgotestir.proftext
-//
-// Ensure Pass PGOInstrumentationUsePass and PGOInstrumentationGenPass are 
invoked.
-// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata 
-fprofile-instrument=csllvm -fprofile-instrument-path=default.profraw  %s 
-mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2
-// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata 
-fprofile-instrument=csllvm -fprofile-instrument-path=default.profraw  %s 
-fexperimental-new-pass-manager -fdebug-pass-manager  -emit-llvm -o - 2>&1 | 
FileCheck %s -check-prefix=CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM
-// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2: PGOInstrumentationUsePass
-// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2: PGOInstrumentationGenCreateVarPass
-// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2: PGOInstrumentationGenPass
-// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM: Running pass: 
PGOInstrumentationUse
-// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM: Running pass: 
PGOInstrumentationGenCreateVar on
-// CHECK-CSPGOGENPASS-INVOKED-INSTR-GEN2-NEWPM: Running pass: 
PGOInstrumentationGen on
-
-// Ensure Pass PGOInstrumentationUsePass is invoked only once.
-// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata %s 
-mllvm -debug-pass=Structure -emit-llvm -o - 2>&1 | FileCheck %s 
-check-prefix=CHECK-PGOUSEPASS-INVOKED-USE
-// RUN: %clang_cc1 -O2 -fprofile-instrument-use-path=%t/noncs.profdata %s 
-fexperimental-new-pass-manager -fdebug-pass-manager -emit-llvm -o - 2>&1 | 
FileCheck %s -check-prefix=CHECK-PGOUSEPASS-INVOKED-USE-NEWPM
-// CHECK-PGOUSEPASS-INVOKED-USE: PGOInstrumentationUsePass
-// CHECK-PGOUSEPASS-INVOKED-USE-NOT: PGOInstrumentationGenCreateVarPass
-// CHECK-PGOUSEPASS-INVOKED-USE-NOT: PGOInstrumentationUsePass
-// CHECK-PGOUSEPASS-INVOKED-USE-NEWPM: Running pass: PGOInstrumentationUse
-// CHECK-PGOUSEPASS-INVOKED-USE-NEWPM-NOT: Running pass: 
PGOInstrumentationGenCreateVar
-// CHECK-PGOUSEPASS-INVOKED-USE-NEWPM-NOT: Running pass: PGOInstrumentationUse
-//
-// Ensure Pass PGOInstrumentationUsePass is invoked twice.
-// RUN: llvm-profdata merge -o %t/cs.profdata %S/Inputs/pgotestir_cs.proftext
-// RUN: %clang_cc1 -O2 

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jason Liu via Phabricator via cfe-commits]

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



Comment at: llvm/lib/MC/MCContext.cpp:165
+case MCObjectFileInfo::IsXCOFF:
+  // TODO: Need to implement class MCSymbolXCOFF.
+  break;

JDevlieghere wrote:
> See previous comment.
It is certain that we will need MCSymbolXCOFF. But before we run into cases 
where we actually need a MCSymbolXCOFF, we could use the generic MCSymbol first 
for XCOFF platform. So I don't want to put a llvm_unreachable here. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Jonas Devlieghere via Phabricator via cfe-commits]

JDevlieghere added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

jasonliu wrote:
> JDevlieghere wrote:
> > jasonliu wrote:
> > > apaprocki wrote:
> > > > No need to be `sorry:` :) This should probably just say `error: XCOFF 
> > > > is unimplemented` to be more direct in case anything is expecting 
> > > > "error:" in the output.
> > > Sure. Will address in next revision.
> > Just bundle this with the WASM case, the error message is correct for both.
> I think they are different. 
> The error message for WASM seems to suggest that it will never ever get 
> supported on WASM. 
> But it is not the case for XCOFF, we want to indicate that it is not 
> implemented yet.  
I don't think the error message suggests that at all, and it's definitely not 
true. At this point neither XCOFF nor WASM is supported, and that's exactly 
what the log message says.



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[PATCH] D58154: Add support for -fpermissive.

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

rsmith added a comment.

In D58154#1418594 , @jyknight wrote:

> The errors disabled by this feature are default-error warnings -- you can 
> already get the same effect by using -Wno-.


Right; the `-fpermissive` mode added by this patch turns off strictly less than 
`-Wno-everything`. (`-fpermissive` still produces warnings on uses of 
extensions at least -- it's very similar to what `-Wno-error=everything` would 
mean if we supported it.)


Repository:
  rC Clang

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

https://reviews.llvm.org/D58154



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

[PATCH] D56900: [Fixed Point Arithmetic] Fixed Point and Integer Conversions

[Leonard Chan via Phabricator via cfe-commits]

leonardchan updated this revision to Diff 189422.
leonardchan marked 11 inline comments as done.

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56900

Files:
  clang/include/clang/AST/OperationKinds.def
  clang/include/clang/Basic/FixedPoint.h
  clang/lib/AST/Expr.cpp
  clang/lib/AST/ExprConstant.cpp
  clang/lib/Basic/FixedPoint.cpp
  clang/lib/CodeGen/CGExpr.cpp
  clang/lib/CodeGen/CGExprAgg.cpp
  clang/lib/CodeGen/CGExprComplex.cpp
  clang/lib/CodeGen/CGExprConstant.cpp
  clang/lib/CodeGen/CGExprScalar.cpp
  clang/lib/Edit/RewriteObjCFoundationAPI.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/lib/StaticAnalyzer/Core/ExprEngineC.cpp
  clang/test/Frontend/fixed_point_conversions.c
  clang/test/Frontend/fixed_point_errors.c
  clang/test/Frontend/fixed_point_unknown_conversions.c

Index: clang/test/Frontend/fixed_point_unknown_conversions.c
===
--- clang/test/Frontend/fixed_point_unknown_conversions.c
+++ clang/test/Frontend/fixed_point_unknown_conversions.c
@@ -22,28 +22,19 @@
   _Fract fract = accum; // ok
   _Accum *accum_ptr;
 
-  accum = b;   // expected-error{{conversion between fixed point and '_Bool' is not yet supported}}
-  accum = i;   // expected-error{{conversion between fixed point and 'int' is not yet supported}}
-  accum = i;   // expected-error{{conversion between fixed point and 'int' is not yet supported}}
   accum = f;   // expected-error{{conversion between fixed point and 'float' is not yet supported}}
   accum = d;   // expected-error{{conversion between fixed point and 'double' is not yet supported}}
   accum = dc;  // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
   accum = ic;  // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
   accum = s;   // expected-error{{assigning to '_Accum' from incompatible type 'struct S'}}
-  accum = e;   // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
   accum = ptr; // expected-error{{assigning to '_Accum' from incompatible type 'int *'}}
   accum_ptr = ptr; // expected-warning{{incompatible pointer types assigning to '_Accum *' from 'int *'}}
-  accum = i2;  // expected-error{{conversion between fixed point and 'int_t' (aka 'int') is not yet supported}}
 
-  c = accum;   // expected-error{{conversion between fixed point and 'char' is not yet supported}}
-  i = accum;   // expected-error{{conversion between fixed point and 'int' is not yet supported}}
   f = accum;   // expected-error{{conversion between fixed point and 'float' is not yet supported}}
   d = accum;   // expected-error{{conversion between fixed point and 'double' is not yet supported}}
   dc = accum;  // expected-error{{conversion between fixed point and '_Complex double' is not yet supported}}
   ic = accum;  // expected-error{{conversion between fixed point and '_Complex int' is not yet supported}}
   s = accum;   // expected-error{{assigning to 'struct S' from incompatible type '_Accum'}}
-  e = accum;   // expected-error{{conversion between fixed point and 'enum E' is not yet supported}}
   ptr = accum; // expected-error{{assigning to 'int *' from incompatible type '_Accum'}}
   ptr = accum_ptr; // expected-warning{{incompatible pointer types assigning to 'int *' from '_Accum *'}}
-  i2 = accum;  // expected-error{{conversion between fixed point and 'int' is not yet supported}}
 }
Index: clang/test/Frontend/fixed_point_errors.c
===
--- clang/test/Frontend/fixed_point_errors.c
+++ clang/test/Frontend/fixed_point_errors.c
@@ -233,8 +233,20 @@
  // expected-warning@-1{{type specifier missing, defaults to 'int'}}
 }
 
+// Ok conversions
+int i_const = -2.5hk;
+_Sat short _Accum sat_sa_const2 = 256.0k;
+_Sat unsigned short _Accum sat_usa_const = -1.0hk;
+short _Accum sa_const3 = 2;
+short _Accum sa_const4 = -2;
+
 // Overflow
 short _Accum sa_const = 256.0k;   // expected-warning{{implicit conversion from 256.0 cannot fit within the range of values for 'short _Accum'}}
 short _Fract sf_const = 1.0hk;// expected-warning{{implicit conversion from 1.0 cannot fit within the range of values for 'short _Fract'}}
 unsigned _Accum ua_const = -1.0k; // expected-warning{{implicit conversion from -1.0 cannot fit within the range of values for 'unsigned _Accum'}}
 short _Accum sa_const2 = 128.0k + 128.0k; // expected-warning{{implicit conversion from 256.0 cannot fit within the range of values for 'short _Accum'}}
+short s_const = 65536.0lk;// expected-warning{{implicit conversion from 65536.0 cannot fit within the range of values for 'short'}}
+unsigned u_const = -2.5hk;// expected-warning{{impli

[PATCH] D56900: [Fixed Point Arithmetic] Fixed Point and Integer Conversions

[Leonard Chan via Phabricator via cfe-commits]

leonardchan added inline comments.



Comment at: clang/test/Frontend/fixed_point_conversions.c:45
+
+short _Accum sa_const9 = 2u; // DEFAULT-DAG: @sa_const9 = {{.*}}global i16 
256, align 2
+unsigned short _Accum usa_const3 = 2;

bjope wrote:
> Perhaps we should verify that we do not mess up signed values when using the 
> -fpadding-on-unsigned-fixed-point option (having CHECK, DEFAULT and SAME as 
> check prefixes)?
> 
> (nit: I also see that we now use different names for this check prefixes in 
> different test files, which could be confusing. Why is it called SAME here?) 
> 
> 
> 
> Perhaps we should verify that we do not mess up signed values when using the 
> -fpadding-on-unsigned-fixed-point option (having CHECK, DEFAULT and SAME as 
> check prefixes)?

Done

> (nit: I also see that we now use different names for this check prefixes in 
> different test files, which could be confusing. Why is it called SAME here?)

Sorry forgot to update these prefixes.



Comment at: clang/test/Frontend/fixed_point_conversions.c:66
 
+_Sat short _Accum sat_sa_const3 = 256;  // DEFAULT-DAG: @sat_sa_const3 = 
{{.*}}global i16 32767, align 2
+_Sat short _Accum sat_sa_const4 = -257; // DEFAULT-DAG: @sat_sa_const4 = 
{{.*}}global i16 -32768, align 2

bjope wrote:
> A little bit confusing to be with this mix of having checks left-aligned (on 
> separate lines) and some checks appended like this.
> I can see that it already is like that before, so maybe it should be cleaned 
> up in a separate commit (before this commit).
> 
> (I'm not so familiar with clang tests, so maybe it is common to have the 
> FileCheck checks at the end of the line. But I've never seen that in llvm.)
Will change this, but would be easier for me if it was done after this commit.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56900



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

[PATCH] D56900: [Fixed Point Arithmetic] Fixed Point and Integer Conversions

[Leonard Chan via Phabricator via cfe-commits]

This revision was not accepted when it landed; it landed in state "Needs 
Review".
This revision was automatically updated to reflect the committed changes.
Closed by commit rC355462: [Fixed Point Arithmetic] Fixed Point and Integer 
Conversions (authored by leonardchan, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D56900?vs=189422&id=189424#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D56900

Files:
  include/clang/AST/OperationKinds.def
  include/clang/Basic/FixedPoint.h
  lib/AST/Expr.cpp
  lib/AST/ExprConstant.cpp
  lib/Basic/FixedPoint.cpp
  lib/CodeGen/CGExpr.cpp
  lib/CodeGen/CGExprAgg.cpp
  lib/CodeGen/CGExprComplex.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGExprScalar.cpp
  lib/Edit/RewriteObjCFoundationAPI.cpp
  lib/Sema/SemaChecking.cpp
  lib/Sema/SemaExpr.cpp
  lib/StaticAnalyzer/Core/ExprEngineC.cpp
  test/Frontend/fixed_point_conversions.c
  test/Frontend/fixed_point_errors.c
  test/Frontend/fixed_point_unknown_conversions.c

Index: test/Frontend/fixed_point_conversions.c
===
--- test/Frontend/fixed_point_conversions.c
+++ test/Frontend/fixed_point_conversions.c
@@ -1,142 +1,179 @@
-// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s -check-prefix=DEFAULT
-// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s -check-prefix=SAME
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - | FileCheck %s --check-prefixes=CHECK,SIGNED
+// RUN: %clang_cc1 -ffixed-point -triple x86_64-unknown-linux-gnu -S -emit-llvm %s -o - -fpadding-on-unsigned-fixed-point | FileCheck %s --check-prefixes=CHECK,UNSIGNED
 
 // Between different fixed point types
-short _Accum sa_const = 2.5hk; // DEFAULT-DAG: @sa_const  = {{.*}}global i16 320, align 2
-_Accum a_const = 2.5hk;// DEFAULT-DAG: @a_const   = {{.*}}global i32 81920, align 4
-short _Accum sa_const2 = 2.5k; // DEFAULT-DAG: @sa_const2 = {{.*}}global i16 320, align 2
+short _Accum sa_const = 2.5hk; // CHECK-DAG: @sa_const  = {{.*}}global i16 320, align 2
+_Accum a_const = 2.5hk;// CHECK-DAG: @a_const   = {{.*}}global i32 81920, align 4
+short _Accum sa_const2 = 2.5k; // CHECK-DAG: @sa_const2 = {{.*}}global i16 320, align 2
 
-short _Accum sa_from_f_const = 0.5r; // DEFAULT-DAG: sa_from_f_const = {{.*}}global i16 64, align 2
-_Fract f_from_sa_const = 0.5hk;  // DEFAULT-DAG: f_from_sa_const = {{.*}}global i16 16384, align 2
+short _Accum sa_from_f_const = 0.5r; // CHECK-DAG: sa_from_f_const = {{.*}}global i16 64, align 2
+_Fract f_from_sa_const = 0.5hk;  // CHECK-DAG: f_from_sa_const = {{.*}}global i16 16384, align 2
 
 unsigned short _Accum usa_const = 2.5uk;
 unsigned _Accum ua_const = 2.5uhk;
-// DEFAULT-DAG: @usa_const  = {{.*}}global i16 640, align 2
-// DEFAULT-DAG: @ua_const   = {{.*}}global i32 163840, align 4
-// SAME-DAG:@usa_const  = {{.*}}global i16 320, align 2
-// SAME-DAG:@ua_const   = {{.*}}global i32 81920, align 4
+// SIGNED-DAG: @usa_const  = {{.*}}global i16 640, align 2
+// SIGNED-DAG: @ua_const   = {{.*}}global i32 163840, align 4
+// UNSIGNED-DAG:@usa_const  = {{.*}}global i16 320, align 2
+// UNSIGNED-DAG:@ua_const   = {{.*}}global i32 81920, align 4
+
+// FixedPoint to integer
+int i_const = -128.0hk;  // CHECK-DAG: @i_const  = {{.*}}global i32 -128, align 4
+int i_const2 = 128.0hk;  // CHECK-DAG: @i_const2 = {{.*}}global i32 128, align 4
+int i_const3 = -128.0k;  // CHECK-DAG: @i_const3 = {{.*}}global i32 -128, align 4
+int i_const4 = 128.0k;   // CHECK-DAG: @i_const4 = {{.*}}global i32 128, align 4
+short s_const = -128.0k; // CHECK-DAG: @s_const  = {{.*}}global i16 -128, align 2
+short s_const2 = 128.0k; // CHECK-DAG: @s_const2 = {{.*}}global i16 128, align 2
+
+// Integer to fixed point
+short _Accum sa_const5 = 2;// CHECK-DAG: @sa_const5 = {{.*}}global i16 256, align 2
+short _Accum sa_const6 = -2;   // CHECK-DAG: @sa_const6 = {{.*}}global i16 -256, align 2
+short _Accum sa_const7 = -256; // CHECK-DAG: @sa_const7 = {{.*}}global i16 -32768, align 2
 
 // Signedness
 unsigned short _Accum usa_const2 = 2.5hk;
-// DEFAULT-DAG: @usa_const2  = {{.*}}global i16 640, align 2
-// SAME-DAG:@usa_const2  = {{.*}}global i16 320, align 2
-short _Accum sa_const3 = 2.5hk; // DEFAULT-DAG: @sa_const3 = {{.*}}global i16 320, align 2
+// SIGNED-DAG: @usa_const2  = {{.*}}global i16 640, align 2
+// UNSIGNED-DAG:@usa_const2  = {{.*}}global i16 320, align 2
+short _Accum sa_const3 = 2.5hk; // CHECK-DAG: @sa_const3 = {{.*}}global i16 320, align 2
+
+int i_const5 = 128.0uhk;
+unsigned int ui_const = 128.0hk;
+// CHECK-DAG: @i_const5  = {{.*}}global i32 128, align 4
+// CHECK-DAG: @ui_const  = {{.*}}global i32 128, align 4
+
+short _Accum sa_const9 = 2u; // CHECK-DAG: @sa_cons

r355462 - [Fixed Point Arithmetic] Fixed Point and Integer Conversions

[Leonard Chan via cfe-commits]

Author: leonardchan
Date: Tue Mar  5 16:28:43 2019
New Revision: 355462

URL: http://llvm.org/viewvc/llvm-project?rev=355462&view=rev
Log:
[Fixed Point Arithmetic] Fixed Point and Integer Conversions

This patch includes the necessary code for converting between a fixed point 
type and integer.
This also includes constant expression evaluation for conversions with these 
types.

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

Modified:
cfe/trunk/include/clang/AST/OperationKinds.def
cfe/trunk/include/clang/Basic/FixedPoint.h
cfe/trunk/lib/AST/Expr.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Basic/FixedPoint.cpp
cfe/trunk/lib/CodeGen/CGExpr.cpp
cfe/trunk/lib/CodeGen/CGExprAgg.cpp
cfe/trunk/lib/CodeGen/CGExprComplex.cpp
cfe/trunk/lib/CodeGen/CGExprConstant.cpp
cfe/trunk/lib/CodeGen/CGExprScalar.cpp
cfe/trunk/lib/Edit/RewriteObjCFoundationAPI.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/StaticAnalyzer/Core/ExprEngineC.cpp
cfe/trunk/test/Frontend/fixed_point_conversions.c
cfe/trunk/test/Frontend/fixed_point_errors.c
cfe/trunk/test/Frontend/fixed_point_unknown_conversions.c

Modified: cfe/trunk/include/clang/AST/OperationKinds.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/OperationKinds.def?rev=355462&r1=355461&r2=355462&view=diff
==
--- cfe/trunk/include/clang/AST/OperationKinds.def (original)
+++ cfe/trunk/include/clang/AST/OperationKinds.def Tue Mar  5 16:28:43 2019
@@ -200,6 +200,14 @@ CAST_OPERATION(IntegralToFloating)
 ///(_Accum) 0.5r
 CAST_OPERATION(FixedPointCast)
 
+/// CK_FixedPointToIntegral - Fixed point to integral.
+///(int) 2.0k
+CAST_OPERATION(FixedPointToIntegral)
+
+/// CK_IntegralToFixedPoint - Integral to a fixed point.
+///(_Accum) 2
+CAST_OPERATION(IntegralToFixedPoint)
+
 /// CK_FixedPointToBoolean - Fixed point to boolean.
 ///(bool) 0.5r
 CAST_OPERATION(FixedPointToBoolean)

Modified: cfe/trunk/include/clang/Basic/FixedPoint.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/FixedPoint.h?rev=355462&r1=355461&r2=355462&view=diff
==
--- cfe/trunk/include/clang/Basic/FixedPoint.h (original)
+++ cfe/trunk/include/clang/Basic/FixedPoint.h Tue Mar  5 16:28:43 2019
@@ -141,6 +141,8 @@ class APFixedPoint {
 return APFixedPoint(Val << Amt, Sema);
   }
 
+  /// Return the integral part of this fixed point number, rounded towards
+  /// zero. (-2.5k -> -2)
   llvm::APSInt getIntPart() const {
 if (Val < 0 && Val != -Val) // Cover the case when we have the min val
   return -(-Val >> getScale());
@@ -148,6 +150,17 @@ class APFixedPoint {
   return Val >> getScale();
   }
 
+  /// Return the integral part of this fixed point number, rounded towards
+  /// zero. The value is stored into an APSInt with the provided width and 
sign.
+  /// If the overflow parameter is provided, and the integral value is not able
+  /// to be fully stored in the provided width and sign, the overflow parameter
+  /// is set to true.
+  ///
+  /// If the overflow parameter is provided, set this value to true or false to
+  /// indicate if this operation results in an overflow.
+  llvm::APSInt convertToInt(unsigned DstWidth, bool DstSign,
+bool *Overflow = nullptr) const;
+
   void toString(llvm::SmallVectorImpl &Str) const;
   std::string toString() const {
 llvm::SmallString<40> S;
@@ -175,6 +188,14 @@ class APFixedPoint {
   static APFixedPoint getMax(const FixedPointSemantics &Sema);
   static APFixedPoint getMin(const FixedPointSemantics &Sema);
 
+  /// Create an APFixedPoint with a value equal to that of the provided 
integer,
+  /// and in the same semantics as the provided target semantics. If the value
+  /// is not able to fit in the specified fixed point semantics, and the
+  /// overflow parameter is provided, it is set to true.
+  static APFixedPoint getFromIntValue(const llvm::APSInt &Value,
+  const FixedPointSemantics &DstFXSema,
+  bool *Overflow = nullptr);
+
 private:
   llvm::APSInt Val;
   FixedPointSemantics Sema;

Modified: cfe/trunk/lib/AST/Expr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Expr.cpp?rev=355462&r1=355461&r2=355462&view=diff
==
--- cfe/trunk/lib/AST/Expr.cpp (original)
+++ cfe/trunk/lib/AST/Expr.cpp Tue Mar  5 16:28:43 2019
@@ -1718,6 +1718,8 @@ bool CastExpr::CastConsistency() const {
   case CK_ZeroToOCLOpaqueType:
   case CK_IntToOCLSampler:
   case CK_FixedPointCast:
+  case CK_FixedPointToIntegral:
+  case CK_IntegralToFixedPoint:
 assert(!getType()->isBooleanType() && "unheralded conversion to bool");
 goto CheckNoBasePat

[PATCH] D59008: [AMDGPU] Switch default dwarf version to 5

[Scott Linder via Phabricator via cfe-commits]

scott.linder created this revision.
scott.linder added reviewers: kzhuravl, t-tye.
Herald added subscribers: cfe-commits, tpr, dstuttard, yaxunl, nhaehnle, wdng, 
jvesely.
Herald added a project: clang.

Reverts r337612.  The issues that cropped up with the last attempt appear to 
have gone away.


Repository:
  rC Clang

https://reviews.llvm.org/D59008

Files:
  lib/Driver/ToolChains/AMDGPU.h
  test/Driver/amdgpu-toolchain.c


Index: test/Driver/amdgpu-toolchain.c
===
--- test/Driver/amdgpu-toolchain.c
+++ test/Driver/amdgpu-toolchain.c
@@ -3,4 +3,4 @@
 // AS_LINK: ld.lld{{.*}} "-shared"
 
 // RUN: %clang -### -g -target amdgcn--amdhsa -mcpu=kaveri %s 2>&1 | FileCheck 
-check-prefix=DWARF_VER %s
-// DWARF_VER: "-dwarf-version=2"
+// DWARF_VER: "-dwarf-version=5"
Index: lib/Driver/ToolChains/AMDGPU.h
===
--- lib/Driver/ToolChains/AMDGPU.h
+++ lib/Driver/ToolChains/AMDGPU.h
@@ -55,7 +55,7 @@
 public:
   AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
-  unsigned GetDefaultDwarfVersion() const override { return 2; }
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   llvm::opt::DerivedArgList *
   TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,


Index: test/Driver/amdgpu-toolchain.c
===
--- test/Driver/amdgpu-toolchain.c
+++ test/Driver/amdgpu-toolchain.c
@@ -3,4 +3,4 @@
 // AS_LINK: ld.lld{{.*}} "-shared"
 
 // RUN: %clang -### -g -target amdgcn--amdhsa -mcpu=kaveri %s 2>&1 | FileCheck -check-prefix=DWARF_VER %s
-// DWARF_VER: "-dwarf-version=2"
+// DWARF_VER: "-dwarf-version=5"
Index: lib/Driver/ToolChains/AMDGPU.h
===
--- lib/Driver/ToolChains/AMDGPU.h
+++ lib/Driver/ToolChains/AMDGPU.h
@@ -55,7 +55,7 @@
 public:
   AMDGPUToolChain(const Driver &D, const llvm::Triple &Triple,
   const llvm::opt::ArgList &Args);
-  unsigned GetDefaultDwarfVersion() const override { return 2; }
+  unsigned GetDefaultDwarfVersion() const override { return 5; }
   bool IsIntegratedAssemblerDefault() const override { return true; }
   llvm::opt::DerivedArgList *
   TranslateArgs(const llvm::opt::DerivedArgList &Args, StringRef BoundArch,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58930: Add XCOFF triple object format type for AIX

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added inline comments.



Comment at: 
lldb/source/Plugins/Process/gdb-remote/GDBRemoteCommunicationClient.cpp:2079
+  if (log)
+log->Printf("sorry: unimplemented for XCOFF");
+  return false;

JDevlieghere wrote:
> jasonliu wrote:
> > JDevlieghere wrote:
> > > jasonliu wrote:
> > > > apaprocki wrote:
> > > > > No need to be `sorry:` :) This should probably just say `error: XCOFF 
> > > > > is unimplemented` to be more direct in case anything is expecting 
> > > > > "error:" in the output.
> > > > Sure. Will address in next revision.
> > > Just bundle this with the WASM case, the error message is correct for 
> > > both.
> > I think they are different. 
> > The error message for WASM seems to suggest that it will never ever get 
> > supported on WASM. 
> > But it is not the case for XCOFF, we want to indicate that it is not 
> > implemented yet.  
> I don't think the error message suggests that at all, and it's definitely not 
> true. At this point neither XCOFF nor WASM is supported, and that's exactly 
> what the log message says.
> 
I agree that the error message for WASM does not indicate that the lack of 
support is inherent or intended to be permanent; however, it is not indicative 
either of an intent to implement the support. I am not sure what the intent is 
for WASM, but I do know that the intent for XCOFF is to eventually implement 
the support. I do not see how using an ambiguous message in this commit (when 
we know what the intent is) is superior to the alternative of having an 
unambiguous message.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D58930



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak updated this revision to Diff 189436.
ahatanak marked 2 inline comments as done.
ahatanak added a comment.

If the string literal used for the boxed expression isn't a valid UTF-8 string, 
don't emit a compile-time constant.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729

Files:
  include/clang/AST/ExprObjC.h
  lib/AST/ExprConstant.cpp
  lib/CodeGen/CGExprConstant.cpp
  lib/CodeGen/CGObjC.cpp
  lib/Sema/SemaExprObjC.cpp
  test/CodeGenObjC/boxing.m
  test/SemaObjC/boxing-illegal.m
  test/SemaObjC/objc-literal-sig.m
  test/SemaObjC/transfer-boxed-string-nullability.m
  test/SemaObjCXX/literals.mm

Index: test/SemaObjCXX/literals.mm
===
--- test/SemaObjCXX/literals.mm
+++ test/SemaObjCXX/literals.mm
@@ -50,6 +50,9 @@
 + (id)dictionaryWithObjects:(const id [])objects forKeys:(const id [])keys count:(unsigned long)cnt;
 @end
 
+@interface NSString
+@end
+
 template
 struct ConvertibleTo {
   operator T();
@@ -185,3 +188,8 @@
 void test_dictionary_colon() {
   id dict = @{ key : value };
 }
+
+void testConstExpr() {
+  constexpr NSString *t0 = @"abc";
+  constexpr NSString *t1 = @("abc");
+}
Index: test/SemaObjC/transfer-boxed-string-nullability.m
===
--- test/SemaObjC/transfer-boxed-string-nullability.m
+++ test/SemaObjC/transfer-boxed-string-nullability.m
@@ -16,12 +16,24 @@
 void takesNonNull(NSString * _Nonnull ptr);
 
 void testBoxedString() {
+  // No diagnostic emitted as this doesn't need a stringWithUTF8String message
+  // send.
+  takesNonNull(@("hey"));
+  takesNonNull(@(u8"hey"));
+
+  // If the string isn't a valid UTF-8 string, a diagnostic is emitted since the
+  // boxed expression turns into a message send.
+  takesNonNull(@(u8"\xFF"));
+  takesNonNull(@(u8"\xC8"));
+
   const char *str = "hey";
   takesNonNull([NSString stringWithUTF8String:str]);
   takesNonNull(@(str));
 #ifndef NOWARN
-  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
-  // expected-warning@-3 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-7 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-7 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-5 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
+  // expected-warning@-5 {{implicit conversion from nullable pointer 'NSString * _Nullable' to non-nullable pointer type 'NSString * _Nonnull'}}
 #else
   // expected-no-diagnostics
 #endif
Index: test/SemaObjC/objc-literal-sig.m
===
--- test/SemaObjC/objc-literal-sig.m
+++ test/SemaObjC/objc-literal-sig.m
@@ -39,6 +39,8 @@
 
 // All tests are doubled to make sure that a bad method is not saved
 // and then used un-checked.
+const char *getStr(void);
+
 void test_sig() {
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
   (void)@__objc_yes; // expected-error{{literal construction method 'numberWithBool:' has incompatible signature}}
@@ -46,6 +48,6 @@
   id array2 = @[ @17 ]; // expected-error{{literal construction method 'arrayWithObjects:count:' has incompatible signature}}
   id dict = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
   id dict2 = @{ @"hello" : @17 }; // expected-error{{literal construction method 'dictionaryWithObjects:forKeys:count:' has incompatible signature}}
-  id str = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
-  id str2 = @("hello"); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
+  id str2 = @(getStr()); // expected-error{{literal construction method 'stringWithUTF8String:' has incompatible signature}}
 }
Index: test/SemaObjC/boxing-illegal.m
===
--- test/SemaObjC/boxing-illegal.m
+++ test/SemaObjC/boxing-illegal.m
@@ -1,4 +1,4 @@
-// RUN: %clang_cc1 -fsyntax-only -verify -Wattributes %s
+// RUN: %clang_cc1 -fsyntax-only -verify -Wattributes -fpascal-strings %s
 
 typedef long NSInteger;
 typedef unsigned long NSUInteger;
@@ -57,6 +57,19 @@
   box = @(*(enum ForwE*)p); // expected-error {{incomplete type 'enum ForwE' used in a boxed exp

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

ahatanak added inline comments.



Comment at: test/SemaObjC/boxing-illegal.m:70
+  s = @(L"abc"); // expected-error {{illegal type 'int *' used in a boxed 
expression}}
+  s = @("\pabc"); // expected-error {{illegal type 'unsigned char *' used in a 
boxed expression}}
+}

rjmccall wrote:
> I don't know what `\p` is supposed to be or why it apparently changes the 
> type of the literal to `unsigned char *`, but none of these are ordinary 
> string literals that are invalid as UTF-8.  I mean something like "\xFF", 
> which still has type `char *` but will fail to parse as UTF-8, which will 
> cause normal boxing to fail and return `nil`.
Fixed and added test cases. Boxed expressions now have to be valid UTF-8 string 
literals in order to be emitted as compile-time constants.

If the string literal in a boxed expression is an invalid UTF-8 string, should 
we reject it the same way we reject other kinds of string literals (e.g., 
UTF-16)?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[Akira Hatanaka via Phabricator via cfe-commits]

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



Comment at: test/SemaObjC/boxing-illegal.m:70
+  s = @(L"abc"); // expected-error {{illegal type 'int *' used in a boxed 
expression}}
+  s = @("\pabc"); // expected-error {{illegal type 'unsigned char *' used in a 
boxed expression}}
+}

ahatanak wrote:
> rjmccall wrote:
> > I don't know what `\p` is supposed to be or why it apparently changes the 
> > type of the literal to `unsigned char *`, but none of these are ordinary 
> > string literals that are invalid as UTF-8.  I mean something like "\xFF", 
> > which still has type `char *` but will fail to parse as UTF-8, which will 
> > cause normal boxing to fail and return `nil`.
> Fixed and added test cases. Boxed expressions now have to be valid UTF-8 
> string literals in order to be emitted as compile-time constants.
> 
> If the string literal in a boxed expression is an invalid UTF-8 string, 
> should we reject it the same way we reject other kinds of string literals 
> (e.g., UTF-16)?
Or at least warn about it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

r355477 - Modules: Add -Rmodule-import

[Duncan P. N. Exon Smith via cfe-commits]

Author: dexonsmith
Date: Tue Mar  5 18:50:46 2019
New Revision: 355477

URL: http://llvm.org/viewvc/llvm-project?rev=355477&view=rev
Log:
Modules: Add -Rmodule-import

Add a remark for importing modules.  Depending on whether this is a
direct import (into the TU being built by this compiler instance) or
transitive import (into an already-imported module), the diagnostic has
two forms:

importing module 'Foo' from 'path/to/Foo.pcm'
importing module 'Foo' into 'Bar' from 'path/to/Foo.pcm'

Also drop a redundant FileCheck invocation in Rmodule-build.m that was
using -Reverything, since the notes from -Rmodule-import were confusing
it.

https://reviews.llvm.org/D58891

Added:
cfe/trunk/test/Modules/Inputs/Rmodule-import/
cfe/trunk/test/Modules/Inputs/Rmodule-import/A.h
cfe/trunk/test/Modules/Inputs/Rmodule-import/B.h
cfe/trunk/test/Modules/Inputs/Rmodule-import/C.h
cfe/trunk/test/Modules/Inputs/Rmodule-import/D.h
cfe/trunk/test/Modules/Inputs/Rmodule-import/module.modulemap
cfe/trunk/test/Modules/Rmodule-import.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/test/Modules/Rmodule-build.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=355477&r1=355476&r2=355477&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Tue Mar  5 18:50:46 2019
@@ -351,6 +351,7 @@ def MismatchedReturnTypes : DiagGroup<"m
 def MismatchedTags : DiagGroup<"mismatched-tags">;
 def MissingFieldInitializers : DiagGroup<"missing-field-initializers">;
 def ModuleBuild : DiagGroup<"module-build">;
+def ModuleImport : DiagGroup<"module-import">;
 def ModuleConflict : DiagGroup<"module-conflict">;
 def ModuleFileExtension : DiagGroup<"module-file-extension">;
 def NewlineEOF : DiagGroup<"newline-eof">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=355477&r1=355476&r2=355477&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Tue Mar  5 
18:50:46 2019
@@ -72,6 +72,10 @@ def note_module_file_imported_by : Note<
 def err_module_file_not_module : Error<
   "AST file '%0' was not built as a module">, DefaultFatal;
 
+def remark_module_import : Remark<
+  "importing module '%0'%select{| into '%3'}2 from '%1'">,
+  InGroup;
+
 def err_imported_module_not_found : Error<
 "module '%0' in AST file '%1' (imported by AST file '%2') "
 "is not defined in any loaded module map file; "

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=355477&r1=355476&r2=355477&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Tue Mar  5 18:50:46 2019
@@ -2610,6 +2610,9 @@ ASTReader::ReadControlBlock(ModuleFile &
 
 case MODULE_NAME:
   F.ModuleName = Blob;
+  Diag(diag::remark_module_import)
+  << F.ModuleName << F.FileName << (ImportedBy ? true : false)
+  << (ImportedBy ? StringRef(ImportedBy->ModuleName) : StringRef());
   if (Listener)
 Listener->ReadModuleName(F.ModuleName);
 
@@ -4141,6 +4144,9 @@ ASTReader::ReadASTCore(StringRef FileNam
 
   switch (AddResult) {
   case ModuleManager::AlreadyLoaded:
+Diag(diag::remark_module_import)
+<< M->ModuleName << M->FileName << (ImportedBy ? true : false)
+<< (ImportedBy ? StringRef(ImportedBy->ModuleName) : StringRef());
 return Success;
 
   case ModuleManager::NewlyLoaded:

Added: cfe/trunk/test/Modules/Inputs/Rmodule-import/A.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/Rmodule-import/A.h?rev=355477&view=auto
==
--- cfe/trunk/test/Modules/Inputs/Rmodule-import/A.h (added)
+++ cfe/trunk/test/Modules/Inputs/Rmodule-import/A.h Tue Mar  5 18:50:46 2019
@@ -0,0 +1,2 @@
+// A
+#include "B.h"

Added: cfe/trunk/test/Modules/Inputs/Rmodule-import/B.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/Rmodule-import/B.h?rev=355477&view=auto
==
--- cfe/trunk/test/Modules/Inputs/Rmodule-import/B.h (added)
+++ cfe/trunk/test/Modules/Inputs/Rmodule-import/B.h Tue Mar  5 18:50:46 2019
@@ -0,0 +1,2 

[PATCH] D58891: Modules: Add -Rmodule-import

[Duncan P. N. Exon Smith via Phabricator via cfe-commits]

dexonsmith closed this revision.
dexonsmith marked 2 inline comments as done.
dexonsmith added a comment.

Committed in r355477.




Comment at: clang/test/Modules/Inputs/Rmodule-import/B.h:1
+// B
+#include "C.h"

bruno wrote:
> Can you make one of the tests to use `@import` (or the pragma version) 
> instead of #include? 
I changed the import of `D`.


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

https://reviews.llvm.org/D58891



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

[PATCH] D58729: Emit boxed expression as a compile-time constant if the expression inside the parentheses is a string literal

[John McCall via Phabricator via cfe-commits]

rjmccall added inline comments.



Comment at: test/SemaObjC/boxing-illegal.m:70
+  s = @(L"abc"); // expected-error {{illegal type 'int *' used in a boxed 
expression}}
+  s = @("\pabc"); // expected-error {{illegal type 'unsigned char *' used in a 
boxed expression}}
+}

ahatanak wrote:
> ahatanak wrote:
> > rjmccall wrote:
> > > I don't know what `\p` is supposed to be or why it apparently changes the 
> > > type of the literal to `unsigned char *`, but none of these are ordinary 
> > > string literals that are invalid as UTF-8.  I mean something like "\xFF", 
> > > which still has type `char *` but will fail to parse as UTF-8, which will 
> > > cause normal boxing to fail and return `nil`.
> > Fixed and added test cases. Boxed expressions now have to be valid UTF-8 
> > string literals in order to be emitted as compile-time constants.
> > 
> > If the string literal in a boxed expression is an invalid UTF-8 string, 
> > should we reject it the same way we reject other kinds of string literals 
> > (e.g., UTF-16)?
> Or at least warn about it.
Thanks, fix looks good.  I think a warning would be extremely reasonable. 


Repository:
  rC Clang

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

https://reviews.llvm.org/D58729



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

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Nathan Ridge via Phabricator via cfe-commits]

nridge updated this revision to Diff 189444.
nridge marked 10 inline comments as done.
nridge added a comment.

Address most of the latest review comments.

The infinite recursion issue remains to be fixed.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370

Files:
  clang-tools-extra/clangd/ClangdLSPServer.cpp
  clang-tools-extra/clangd/ClangdLSPServer.h
  clang-tools-extra/clangd/ClangdServer.cpp
  clang-tools-extra/clangd/ClangdServer.h
  clang-tools-extra/clangd/FindSymbols.cpp
  clang-tools-extra/clangd/FindSymbols.h
  clang-tools-extra/clangd/Protocol.cpp
  clang-tools-extra/clangd/Protocol.h
  clang-tools-extra/clangd/XRefs.cpp
  clang-tools-extra/clangd/XRefs.h
  clang-tools-extra/clangd/index/SymbolCollector.cpp
  clang-tools-extra/clangd/index/SymbolCollector.h
  clang-tools-extra/test/clangd/initialize-params.test
  clang-tools-extra/unittests/clangd/Matchers.h
  clang-tools-extra/unittests/clangd/XRefsTests.cpp

Index: clang-tools-extra/unittests/clangd/XRefsTests.cpp
===
--- clang-tools-extra/unittests/clangd/XRefsTests.cpp
+++ clang-tools-extra/unittests/clangd/XRefsTests.cpp
@@ -15,6 +15,8 @@
 #include "XRefs.h"
 #include "index/FileIndex.h"
 #include "index/SymbolCollector.h"
+#include "clang/AST/DeclCXX.h"
+#include "clang/AST/DeclTemplate.h"
 #include "clang/Index/IndexingAction.h"
 #include "llvm/Support/Path.h"
 #include "llvm/Support/ScopedPrinter.h"
@@ -25,9 +27,13 @@
 namespace clangd {
 namespace {
 
+using testing::AllOf;
 using testing::ElementsAre;
+using testing::Eq;
+using testing::Field;
 using testing::IsEmpty;
 using testing::Matcher;
+using testing::Pointee;
 using testing::UnorderedElementsAreArray;
 
 class IgnoreDiagnostics : public DiagnosticsConsumer {
@@ -39,6 +45,15 @@
   return Location{URIForFile::canonicalize(File, testRoot()), Range} == arg;
 }
 
+// GMock helpers for matching TypeHierarchyItem.
+MATCHER_P(WithName, N, "") { return arg.name == N; }
+MATCHER_P(WithKind, Kind, "") { return arg.kind == Kind; }
+MATCHER_P(SelectionRangeIs, R, "") { return arg.selectionRange == R; }
+template 
+testing::Matcher Parents(ParentMatchers... ParentsM) {
+  return Field(&TypeHierarchyItem::parents, HasValue(ElementsAre(ParentsM...)));
+}
+
 // Extracts ranges from an annotated example, and constructs a matcher for a
 // highlight set. Ranges should be named $read/$write as appropriate.
 Matcher &>
@@ -1478,6 +1493,361 @@
   }
 }
 
+TEST(FindRecordTypeAt, TypeOrVariable) {
+  Annotations Source(R"cpp(
+struct Ch^ild2 {
+  int c;
+};
+
+int main() {
+  Ch^ild2 ch^ild2;
+  ch^ild2.c = 1;
+}
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  for (Position Pt : Source.points()) {
+const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
+EXPECT_EQ(&findDecl(AST, "Child2"), static_cast(RD));
+  }
+}
+
+TEST(FindRecordTypeAt, Method) {
+  Annotations Source(R"cpp(
+struct Child2 {
+  void met^hod ();
+  void met^hod (int x);
+};
+
+int main() {
+  Child2 child2;
+  child2.met^hod(5);
+}
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  for (Position Pt : Source.points()) {
+const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
+EXPECT_EQ(&findDecl(AST, "Child2"), static_cast(RD));
+  }
+}
+
+TEST(FindRecordTypeAt, Field) {
+  Annotations Source(R"cpp(
+struct Child2 {
+  int fi^eld;
+};
+
+int main() {
+  Child2 child2;
+  child2.fi^eld = 5;
+}
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  for (Position Pt : Source.points()) {
+const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
+// A field does not unambiguously specify a record type
+// (possible associated reocrd types could be the field's type,
+// or the type of the record that the field is a member of).
+EXPECT_EQ(nullptr, RD);
+  }
+}
+
+TEST(TypeParents, SimpleInheritance) {
+  Annotations Source(R"cpp(
+struct Parent {
+  int a;
+};
+
+struct Child1 : Parent {
+  int b;
+};
+
+struct Child2 : Child1 {
+  int c;
+};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+
+  ASSERT_TRUE(AST.getDiagnostics().empty());
+
+  const CXXRecordDecl *Parent =
+  dyn_cast(&findDecl(AST, "Parent"));
+  const CXXRecordDecl *Child1 =
+  dyn_cast(&findDecl(AST, "Child1"));
+  const CXXRecordDecl *Child2 =
+  dyn_cast(&findDecl(AST, "Child2"));
+
+  EXPECT_THAT(typeParents(Parent), ElementsAre());
+  EXPECT_THAT(typeParents(Child1), ElementsAre(Parent));
+  EXPECT_THAT(typeParents(Child2), ElementsAre(Child1));
+}
+
+TEST(TypeParents, MultipleInheritance) {
+  Annotations Source(R"cpp(
+struct Parent1 {
+  int a;
+};
+
+struct Parent2 {
+  int b;
+};
+

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[Nathan Ridge via Phabricator via cfe-commits]

nridge added inline comments.



Comment at: clang-tools-extra/clangd/ClangdServer.cpp:365
   Callback CB) {
-  auto Action = [Sel](decltype(CB) CB, std::string File,
-std::string TweakID,
-Expected InpAST) {
+  auto Action = [Sel](decltype(CB) CB, std::string File, std::string TweakID,
+  Expected InpAST) {

kadircet wrote:
> Can you revert this change?
I tried, but clang-format automatically makes the change again.

Is there some particular clang-format configuration I should apply, so it 
produces the same results? I don't currently have any configuration, i.e. I 
think it's just reading the .clang-format file in the monorepo root.



Comment at: clang-tools-extra/clangd/XRefs.cpp:885
+static Optional
+getTypeHierarchy(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx, int Levels,
+ TypeHierarchyDirection Direction) {

sammccall wrote:
> kadircet wrote:
> > nridge wrote:
> > > sammccall wrote:
> > > > The scope and complexity of this function seems unneccesarily large:
> > > >  - it's (in future?) capable of resolving in both directions
> > > >  - it calls itself recursively, modifying TypeHierarchyDirection to 
> > > > control the search
> > > >  - it handles levels, even though this optimization is only important 
> > > > for derived types
> > > > 
> > > > Can we restrict this to retrieving (all) recursive base types?
> > > > i.e. `Optional getTypeAncestors(const CXXRecordDecl 
> > > > &CXXRD, ASTContext &Ctx)`
> > > > Then later, subtype resolution can be the job of another function: 
> > > > `resolveTypeDescendants(TypeHierarchyItem& Item, int depth)`
> > > > 
> > > > That way the recursion of getTypeAncestors is simpler to understand, as 
> > > > it has much smaller scope. And code sharing between the two LSP calls 
> > > > is clearer: fetching type hierarchy is a call to `getTypeAncestors` and 
> > > > a call to `resolveTypeDescendants` (if direction is children or both, 
> > > > and resolve is > 0), and resolving a hierarchy is a call to 
> > > > `resolveTypeDescendants`.
> > > If we remove "levels" here, should we introduce some kind of guard for 
> > > infinite recursion, in case the user writes something like:
> > > 
> > > ```
> > > template 
> > > struct S : S {};
> > > 
> > > S<0> s;
> > > ```
> > clang should be limiting recursive template instantiations. Also since we 
> > are just traversing the AST produced by clang, it should never be infinite, 
> > but still a nice test case can you add one?
> I think there is a point here...
> 
> Consider our `S` template with the direction reversed and a base case 
> specialized:
> ```
> template 
> struct S : S {};
> template<>
> struct S<0>{};
> 
> S<2> instance;
> ```
> 
> Now the hierarchy of `S<2>` is well defined and finite: `S<2> : S<1> : S<0>`.
> However IIUC the `CXXRecordDecl` for `S<2>` is the instantiation of the 
> primary template, whose base is `S`, which is dependent[1], so the 
> base's `CXXRecordDecl` is the primary template, whose base is `S`... 
> and we never reach the base case.
> 
> Actually I'm not sure whether this happens if the base is dependent merely 
> where it's spelled, or still dependent after instantiation? Even in the 
> latter case one can construct examples where we'll infloop:
> ```template 
> struct Foo {
>   S instance;
> }```
> Trying to get the hierarchy on `S` could infloop. (I agree these should 
> both be test cases)
> 
> What's the Right Thing?
>  - not sure about a recursion limit, as showing 10 `S`s in the hierarchy 
> is silly.
>  - not sure about bailing out on dependent types either, as knowing that e.g. 
> my `SmallSet` inherits from `SmallMap` is meaningful or useful.
>  - maybe we should bail out once we see instantiations of the same template 
> decl twice in a parent chain. I.e. keep the seen non-null 
> `CXXRecordDecl->getTemplateInstantiationPattern()`s in a set and stop 
> recursing if insertion fails.
> 
> However for this patch I'd suggest just bailing out on dependent types with a 
> TODO as the simplest, this is an edge case that can be fixed later.
> 
The current patch does actually recurse infinitely on this test case, likely 
for the reasons Sam outlined (our handling of dependent types), and eventually 
runs into the segfault.

I will update the patch to address this.



Comment at: clang-tools-extra/unittests/clangd/XRefsTests.cpp:1562
+const CXXRecordDecl *RD = findRecordTypeAt(AST, Pt);
+EXPECT_EQ(nullptr, RD);
+  }

kadircet wrote:
> Can you put a TODO?
I added a comment to clarify that a field does not unambiguously specify a 
record type. i don't think there's anything further "to do" here.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370

[PATCH] D56370: [clangd] Add support for type hierarchy (super types only for now)

[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:885
+static Optional
+getTypeHierarchy(const CXXRecordDecl &CXXRD, ASTContext &ASTCtx, int Levels,
+ TypeHierarchyDirection Direction) {

nridge wrote:
> sammccall wrote:
> > kadircet wrote:
> > > nridge wrote:
> > > > sammccall wrote:
> > > > > The scope and complexity of this function seems unneccesarily large:
> > > > >  - it's (in future?) capable of resolving in both directions
> > > > >  - it calls itself recursively, modifying TypeHierarchyDirection to 
> > > > > control the search
> > > > >  - it handles levels, even though this optimization is only important 
> > > > > for derived types
> > > > > 
> > > > > Can we restrict this to retrieving (all) recursive base types?
> > > > > i.e. `Optional getTypeAncestors(const 
> > > > > CXXRecordDecl &CXXRD, ASTContext &Ctx)`
> > > > > Then later, subtype resolution can be the job of another function: 
> > > > > `resolveTypeDescendants(TypeHierarchyItem& Item, int depth)`
> > > > > 
> > > > > That way the recursion of getTypeAncestors is simpler to understand, 
> > > > > as it has much smaller scope. And code sharing between the two LSP 
> > > > > calls is clearer: fetching type hierarchy is a call to 
> > > > > `getTypeAncestors` and a call to `resolveTypeDescendants` (if 
> > > > > direction is children or both, and resolve is > 0), and resolving a 
> > > > > hierarchy is a call to `resolveTypeDescendants`.
> > > > If we remove "levels" here, should we introduce some kind of guard for 
> > > > infinite recursion, in case the user writes something like:
> > > > 
> > > > ```
> > > > template 
> > > > struct S : S {};
> > > > 
> > > > S<0> s;
> > > > ```
> > > clang should be limiting recursive template instantiations. Also since we 
> > > are just traversing the AST produced by clang, it should never be 
> > > infinite, but still a nice test case can you add one?
> > I think there is a point here...
> > 
> > Consider our `S` template with the direction reversed and a base case 
> > specialized:
> > ```
> > template 
> > struct S : S {};
> > template<>
> > struct S<0>{};
> > 
> > S<2> instance;
> > ```
> > 
> > Now the hierarchy of `S<2>` is well defined and finite: `S<2> : S<1> : 
> > S<0>`.
> > However IIUC the `CXXRecordDecl` for `S<2>` is the instantiation of the 
> > primary template, whose base is `S`, which is dependent[1], so the 
> > base's `CXXRecordDecl` is the primary template, whose base is `S`... 
> > and we never reach the base case.
> > 
> > Actually I'm not sure whether this happens if the base is dependent merely 
> > where it's spelled, or still dependent after instantiation? Even in the 
> > latter case one can construct examples where we'll infloop:
> > ```template 
> > struct Foo {
> >   S instance;
> > }```
> > Trying to get the hierarchy on `S` could infloop. (I agree these should 
> > both be test cases)
> > 
> > What's the Right Thing?
> >  - not sure about a recursion limit, as showing 10 `S`s in the 
> > hierarchy is silly.
> >  - not sure about bailing out on dependent types either, as knowing that 
> > e.g. my `SmallSet` inherits from `SmallMap` is meaningful or 
> > useful.
> >  - maybe we should bail out once we see instantiations of the same template 
> > decl twice in a parent chain. I.e. keep the seen non-null 
> > `CXXRecordDecl->getTemplateInstantiationPattern()`s in a set and stop 
> > recursing if insertion fails.
> > 
> > However for this patch I'd suggest just bailing out on dependent types with 
> > a TODO as the simplest, this is an edge case that can be fixed later.
> > 
> The current patch does actually recurse infinitely on this test case, likely 
> for the reasons Sam outlined (our handling of dependent types), and 
> eventually runs into the segfault.
> 
> I will update the patch to address this.
I meant to say "eventually runs into a stack overflow".


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D56370



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