[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added inline comments.



Comment at: clang-tidy/tool/clang-tidy-diff.py:51
+
+  if not timeout is None:
+watchdog = threading.Timer(timeout, proc.kill)

`if timeout is not None` is more readable.



Comment at: clang-tidy/tool/clang-tidy-diff.py:58
+  with lock:
+sys.stdout.write(' '.join(command) + '\n' + stdout.decode('utf-8') + 
'\n')
+if stderr:

the `command` could be utf-8, too not?
If for example file-names use special characters they should be decoded as 
well. This is probably not done properly in the `run-clang-tidy.py` script, but 
can be adressed separatly.



Comment at: clang-tidy/tool/clang-tidy-diff.py:76
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()

why daemonize? Who kills that thread in the end? I think it's not the best 
choice.



Comment at: clang-tidy/tool/clang-tidy-diff.py:131
 
+  if args.j == 0 or args.j > 1:
+if args.export_fixes:

I would prefer going the `clang-apply-replacements` route here as well.
It feels weird if there is an inconsistency between these parallel runners.


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

https://reviews.llvm.org/D57662



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

[PATCH] D57883: [clang-tidy] refactor ExceptionAnalyzer further to give ternary answer

[Jonas Toth via Phabricator via cfe-commits]

JonasToth added a comment.

ping :)


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57883



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

[PATCH] D57232: [ASTImporter] Check visibility/linkage of functions and variables

[Gabor Marton via Phabricator via cfe-commits]

martong updated this revision to Diff 186802.
martong added a comment.

Rebase to master(trunk)


Repository:
  rC Clang

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

https://reviews.llvm.org/D57232

Files:
  include/clang/AST/ASTImporter.h
  lib/AST/ASTImporter.cpp
  unittests/AST/ASTImporterTest.cpp

Index: unittests/AST/ASTImporterTest.cpp
===
--- unittests/AST/ASTImporterTest.cpp
+++ unittests/AST/ASTImporterTest.cpp
@@ -10,6 +10,10 @@
 //
 //===--===//
 
+// Define this to have ::testing::Combine available.
+// FIXME: Better solution for this?
+#define GTEST_HAS_COMBINE 1
+
 #include "clang/AST/ASTImporter.h"
 #include "MatchVerifier.h"
 #include "clang/AST/ASTContext.h"
@@ -461,6 +465,10 @@
 return FromTU->import(*LookupTablePtr, ToAST.get(), From);
   }
 
+  template  DeclT *Import(DeclT *From, Language Lang) {
+return cast_or_null(Import(cast(From), Lang));
+  }
+
   QualType ImportType(QualType FromType, Decl *TUDecl, Language ToLang) {
 lazyInitToAST(ToLang, "", OutputFileName);
 TU *FromTU = findFromTU(TUDecl);
@@ -2441,6 +2449,266 @@
   EXPECT_EQ(ToDFOutOfClass->getPreviousDecl(), ToDFInClass);
 }
 
+//FIXME Move these tests to a separate test file.
+namespace TypeAndValueParameterizedTests {
+
+// Type parameters for type-parameterized test fixtures.
+struct GetFunPattern {
+  using DeclTy = FunctionDecl;
+  BindableMatcher operator()() { return functionDecl(hasName("f")); }
+};
+struct GetVarPattern {
+  using DeclTy = VarDecl;
+  BindableMatcher operator()() { return varDecl(hasName("v")); }
+};
+
+// Values for the value-parameterized test fixtures.
+// FunctionDecl:
+auto *ExternF = "void f();";
+auto *StaticF = "static void f();";
+auto *AnonF = "namespace { void f(); }";
+// VarDecl:
+auto *ExternV = "extern int v;";
+auto *StaticV = "static int v;";
+auto *AnonV = "namespace { extern int v; }";
+
+// First value in tuple: Compile options.
+// Second value in tuple: Source code to be used in the test.
+using ImportVisibilityChainParams =
+::testing::WithParamInterface>;
+// Fixture to test the redecl chain of Decls with the same visibility.  Gtest
+// makes it possible to have either value-parameterized or type-parameterized
+// fixtures.  However, we cannot have both value- and type-parameterized test
+// fixtures.  This is a value-parameterized test fixture in the gtest sense. We
+// intend to mimic gtest's type-parameters via the PatternFactory template
+// parameter.  We manually instantiate the different tests with the each types.
+template 
+class ImportVisibilityChain
+: public ASTImporterTestBase, public ImportVisibilityChainParams {
+protected:
+  using DeclTy = typename PatternFactory::DeclTy;
+  ArgVector getExtraArgs() const override { return std::get<0>(GetParam()); }
+  std::string getCode() const { return std::get<1>(GetParam()); }
+  BindableMatcher getPattern() const { return PatternFactory()(); }
+
+  // Type-parameterized test.
+  void TypedTest_ImportChain() {
+std::string Code = getCode() + getCode();
+auto Pattern = getPattern();
+
+TranslationUnitDecl *FromTu = getTuDecl(Code, Lang_CXX, "input0.cc");
+
+auto *FromF0 = FirstDeclMatcher().match(FromTu, Pattern);
+auto *FromF1 = LastDeclMatcher().match(FromTu, Pattern);
+
+auto *ToF0 = Import(FromF0, Lang_CXX);
+auto *ToF1 = Import(FromF1, Lang_CXX);
+
+EXPECT_TRUE(ToF0);
+ASSERT_TRUE(ToF1);
+EXPECT_NE(ToF0, ToF1);
+EXPECT_EQ(ToF1->getPreviousDecl(), ToF0);
+  }
+};
+
+// Manual instantiation of the fixture with each type.
+using ImportFunctionsVisibilityChain = ImportVisibilityChain;
+using ImportVariablesVisibilityChain = ImportVisibilityChain;
+// Value-parameterized test for the first type.
+TEST_P(ImportFunctionsVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
+// Value-parameterized test for the second type.
+TEST_P(ImportVariablesVisibilityChain, ImportChain) {
+  TypedTest_ImportChain();
+}
+
+// Automatic instantiation of the value-parameterized tests.
+INSTANTIATE_TEST_CASE_P(ParameterizedTests, ImportFunctionsVisibilityChain,
+::testing::Combine(
+   DefaultTestValuesForRunOptions,
+   ::testing::Values(ExternF, StaticF, AnonF)), );
+INSTANTIATE_TEST_CASE_P(
+ParameterizedTests, ImportVariablesVisibilityChain,
+::testing::Combine(
+DefaultTestValuesForRunOptions,
+// There is no point to instantiate with StaticV, because in C++ we can
+// forward declare a variable only with the 'extern' keyword.
+// Consequently, each fwd declared variable has external linkage.  This
+// is different in the C language where any declaration without an
+// initializer is a tentative definition, subsequent definitions may be
+// provided but they must have the s

[PATCH] D57662: [clang-tidy] Parallelise clang-tidy-diff.py

[Zinovy Nis via Phabricator via cfe-commits]

zinovy.nis marked an inline comment as done.
zinovy.nis added inline comments.



Comment at: clang-tidy/tool/clang-tidy-diff.py:76
+t = threading.Thread(target=tidy_caller, args=(task_queue, lock, timeout))
+t.daemon = True
+t.start()

JonasToth wrote:
> why daemonize? Who kills that thread in the end? I think it's not the best 
> choice.
The script can be interrupted with keyabord or other signal, so it need not 
wait for any worker thread.


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

https://reviews.llvm.org/D57662



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

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

xazax.hun added a comment.

In D50488#1395483 , @mgrang wrote:

> Reviving this now that I have some cycles to work on this.
>
> So I tried running this on csa-testbench projects but I didn't have much 
> success. I always run into a bunch of build/env related errors:
>
>   python run_experiments.py --config myconfig.json
>  
>   15:05:20 [libcxx] Checking out project... 
>   [ERROR] Unknown option: json
>  
>   15:05:22 [libcxx] LOC: ?.
>   15:05:22 [libcxx] Generating build log... 
>   15:05:22 [libcxx_master] Analyzing project... 
>   [ERROR] Traceback (most recent call last):
> File 
> "/local/mnt/workspace/mgrang/comm_analyzer/CodeChecker/cc_bin/CodeChecker.py",
>  line 20, in 
>   from shared.ttypes import RequestFailed
>   ImportError: No module named shared.ttypes
>
>
>


Hi!

Sorry for the late response. Does CodeChecker work for you (when not using the 
testbench)?
I think one of the most common reason for such errors is when we do not source 
the virtualenv of CodeChecker so the dependencies are not available.


Repository:
  rC Clang

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

https://reviews.llvm.org/D50488



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

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 added a comment.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.

I am going to abandon this modification, as setting ODR violations as warnings, 
seems like a change that could have unforeseen consequences.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55646



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

[PATCH] D45978: dllexport const variables must have external linkage.

[Zahira Ammarguellat via Phabricator via cfe-commits]

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



Comment at: lib/Sema/SemaDecl.cpp:11370
 
+// In Microsoft C++ mode, a const variable defined in namespace scope has
+// external linkage by default if the variable is declared with

aaron.ballman wrote:
> thakis wrote:
> > Even in unnamed namespaces?
> That would definitely be good to test.
It looks like not. 

ksh-3.2$ cat test4.cpp
namespace {
__declspec(dllexport) int const x = 3;
}

int main ()
{
  int y = x + 10;

  return y;
}

ksh-3.2$ cl -c test4.cpp
Microsoft (R) C/C++ Optimizing Compiler Version 19.00.24210 for x64
Copyright (C) Microsoft Corporation.  All rights reserved.

test4.cpp
ksh-3.2$ dumpbin /symbols test4.obj | grep External
**008  SECT3  notype ()External | main**
ksh-3.2$

ksh-3.2$ clang -c test4.cpp
test4.cpp:2:33: error: '(anonymous namespace)::x' must have external linkage 
when declared 'dllexport'
__declspec(dllexport) int const x = 3;
^
1 error generated.
ksh-3.2$

So the diag shouldn't go off when the variable is in an anonymous namespace? Do 
you agree?



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

https://reviews.llvm.org/D45978



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

[PATCH] D55646: [ASTImporter] Make ODR diagnostics warning by default

[Endre Fülöp via Phabricator via cfe-commits]

gamesh411 abandoned this revision.
gamesh411 added a comment.

I am creating a new revision that keeps the old handling of ODR violations the 
same when used by parts of the Sema, but emit warnings when used by the 
ASTImporter.


Repository:
  rC Clang

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

https://reviews.llvm.org/D55646



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

Re: r353976 - [Sema] Delay checking whether objc_designated_initializer is being applied to an init method

[Hans Wennborg via cfe-commits]

Thanks everyone, merged in r354015.

On Wed, Feb 13, 2019 at 9:48 PM Aaron Ballman  wrote:
>
> I'm fine with rolling it into 8.0.
>
> ~Aaron
>
> On Wed, Feb 13, 2019 at 3:47 PM Erik Pilkington via cfe-commits
>  wrote:
> >
> > It isn’t a super common issue, but this is a pretty low-risk fix so I think 
> > it would be nice to have in 8.0.
> >
> > On Feb 13, 2019, at 12:40 PM, Shoaib Meenai  wrote:
> >
> > Should this be considered for 8.0? It's late in the branch and I don't know 
> > how prevalent the issue being fixed is, but it caught my eye.
> >
> > From: cfe-commits  on behalf of Erik 
> > Pilkington via cfe-commits 
> > Reply-To: Erik Pilkington 
> > Date: Wednesday, February 13, 2019 at 12:32 PM
> > To: "cfe-commits@lists.llvm.org" 
> > Subject: r353976 - [Sema] Delay checking whether 
> > objc_designated_initializer is being applied to an init method
> >
> > Author: epilk
> > Date: Wed Feb 13 12:32:37 2019
> > New Revision: 353976
> >
> > URL: 
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D353976-26view-3Drev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=W245MwgFUCCNz9Ug2Tc0dY5lJ35n855YKxo_fwmicxM&e=
> > Log:
> > [Sema] Delay checking whether objc_designated_initializer is being applied 
> > to an init method
> >
> > This fixes a regression that was caused by r335084, which reversed
> > the order that attributes are applied. objc_method_family can change
> > whether a method is an init method, so the order that these
> > attributes are applied matters. The commit fixes this by delaying the
> > init check until after all attributes have been applied.
> >
> > rdar://47829358
> >
> > Differential revision: 
> > https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D58152&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=-4Nd4QBW0KxmFdmqS-2w90_5AoHoRQbdwd8K3Z4JzuY&e=
> >
> > Modified:
> > cfe/trunk/include/clang/Basic/Attr.td
> > cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> > cfe/trunk/lib/Sema/SemaDeclAttr.cpp
> > cfe/trunk/test/Misc/pragma-attribute-supported-attributes-list.test
> > cfe/trunk/test/SemaObjC/attr-designated-init.m
> >
> > Modified: cfe/trunk/include/clang/Basic/Attr.td
> > URL: 
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_include_clang_Basic_Attr.td-3Frev-3D353976-26r1-3D353975-26r2-3D353976-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=tgqq2EylVgj2FgGry6ZO46KA6teeO-xJ1TlvJWvEy6Q&e=
> > ==
> > --- cfe/trunk/include/clang/Basic/Attr.td (original)
> > +++ cfe/trunk/include/clang/Basic/Attr.td Wed Feb 13 12:32:37 2019
> > @@ -102,13 +102,6 @@ def ObjCInstanceMethod : SubsetSubject > [{S->isInstanceMethod()}],
> > "Objective-C instance methods">;
> > -def ObjCInterfaceDeclInitMethod : SubsetSubject > -   [{S->getMethodFamily() == OMF_init &&
> > - 
> > (isa(S->getDeclContext()) ||
> > -  
> > (isa(S->getDeclContext()) &&
> > -
> > cast(S->getDeclContext())->IsClassExtension()))}],
> > -"init methods of interface or class extension declarations">;
> > -
> > def Struct : SubsetSubject > [{!S->isUnion()}], "structs">;
> > @@ -1786,7 +1779,7 @@ def ObjCExplicitProtocolImpl : Inheritab
> > def ObjCDesignatedInitializer : Attr {
> >let Spellings = [Clang<"objc_designated_initializer">];
> > -  let Subjects = SubjectList<[ObjCInterfaceDeclInitMethod], ErrorDiag>;
> > +  let Subjects = SubjectList<[ObjCMethod], ErrorDiag>;
> >let Documentation = [Undocumented];
> > }
> >
> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
> > URL: 
> > https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_include_clang_Basic_DiagnosticSemaKinds.td-3Frev-3D353976-26r1-3D353975-26r2-3D353976-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=SS1gIdRrXBd-rCKs5rBEEPmEevgsHAkkERh1ko2vPEc&s=d6MOBmrgs96qcCuR_Q5j82acKFzDnHxqDkcDiUM_IIk&e=
> > ==
> > --- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
> > +++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Wed Feb 13 
> > 12:32:37 2019
> > @@ -3481,6 +3481,9 @@ def warn_objc_secondary_init_missing_ini
> > def warn_objc_implementation_missing_designated_init_override : Warning<
> >"method override for the designated initializer of the superclass 
> > %objcinstance0 not found">,
> >InGroup;
> > +def err_designated_init_attr_non_init : Error<
> > +  "'objc_de

[PATCH] D58120: [Builtins] Treat `bcmp` as a builtin.

[Clement Courbet via Phabricator via cfe-commits]

courbet added a comment.

Thanks.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58120



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

[PATCH] D57922: [analyzer] Insert checker options into AnalyzerOption::ConfigTable

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

Szelethus added a comment.
Herald added a subscriber: jdoerfert.

Hmm, supplying the default value for `AnalyzerOptions::getChecker*Option` is 
not only redundant with this patch, but also extremely bug-prone. I should 
remove them too in the same breath.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57922



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

[PATCH] D57768: [SYCL] Add SYCL device compilation flow.

[Anastasia Stulova via Phabricator via cfe-commits]

Anastasia added a comment.
Herald added a subscriber: jdoerfert.

In D57768#1389012 , @rjmccall wrote:

> In D57768#1386862 , @Anastasia wrote:
>
> > - SYCL seem to require adding tight dependencies from the standard 
> > libraries into the compiler because many language features are hidden 
> > behind library classes. This is not common for Clang. We had a discussion 
> > about this issue during the implementation of OpenCL C++ and it was decided 
> > not to go this route for upstream Clang. Can you explain your current 
> > approach to implement this? I think @rjmccall  or @rsmith might need to be 
> > involved in this.
>
>
> I'd like to know more about this, but I'll point out that this isn't 
> unprecedented:
>
> - C compilers have hard-coded knowledge about `va_list`.
> - C++ compilers have hard-coded knowledge about `std::type_info` and 
> `std::initializer_list` (and possibly others I've forgotten).
>
>   Whether that's the right direction for SYCL, though, I can't say until I 
> understand more about what dependencies are being proposed.


In OpenCL we hard-coded `printf` btw! I think it would be good to understand 
how much of those is needed for SYCL. The fact that the entire language is 
implemented as a library is a bit worrying. But hopefully we can already map 
most of the things to some existing language constructs (address space 
qualifiers, function attributes, etc...).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D57768



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

[PATCH] D53866: [Preamble] Stop generating preamble for circular #includes

[Nikolai Kosjar via Phabricator via cfe-commits]

nik marked 2 inline comments as done.
nik added inline comments.
Herald added a subscriber: jdoerfert.
Herald added a project: clang.



Comment at: include/clang/Lex/Preprocessor.h:391
   } PreambleConditionalStack;
+  bool PreambleGenerationFailed = false;
 

ilya-biryukov wrote:
> nik wrote:
> > ilya-biryukov wrote:
> > > nik wrote:
> > > > ilya-biryukov wrote:
> > > > > There's a mechanism to handle preamble with errors, see 
> > > > > `PreprocessorOpts::AllowPCHWithCompilerErrors`.
> > > > > Maybe rely on it and avoid adding a different one?
> > > > I'm not sure how to make use of AllowPCHWithCompilerErrors. It's 
> > > > clearly meant as an input/readonly option - do you suggest setting that 
> > > > one to "false" in case we detect the cyclic include (and later checking 
> > > > for it...)? That feels a bit hacky. Have you meant something else?
> > > We emit an error, so the preamble will **not** be emitted. 
> > > Unless the users specify `AllowPCHWithCompilerErrors`, in which case 
> > > they've signed up for this anyway.
> > > 
> > > I propose to **not** add the new flag at all. Would that work?
> > As stated further above, no.
> > 
> > That's because for the libclang/c-index-test case, 
> > AllowPCHWithCompilerErrors=true - see clang_parseTranslationUnit_Impl. As 
> > such, the preamble will be generated/emitted as the second early return in 
> > PCHGenerator::HandleTranslationUnit is never hit.
> if `AllowPCHWithCompilerErrors=true` is set to true, why not simply pretend 
> the include was not found? That would still generate the preamble, but users 
> have the error message to see that something went wrong.
> 
> `c-index-test` produces the errors, so we can check the error is present in 
> the output.
> if AllowPCHWithCompilerErrors=true is set to true, why not simply pretend the 
> include was not found?

Sounds good, especially that the preamble is still generated. 

> That would still generate the preamble, but users have the error message to 
> see that something went wrong.

The vanilla diagnostic in this case might be a bit confusing, I kept the 
introduced diagnostics as it carries more information.



Repository:
  rC Clang

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

https://reviews.llvm.org/D53866



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

[PATCH] D53866: [Preamble] Stop generating preamble for circular #includes

[Nikolai Kosjar via Phabricator via cfe-commits]

nik updated this revision to Diff 186813.
nik added a comment.

Addressed comments.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53866

Files:
  include/clang/Basic/DiagnosticLexKinds.td
  lib/Basic/SourceManager.cpp
  lib/Lex/PPDirectives.cpp
  test/Index/preamble-cyclic-include.cpp


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test 
-test-annotate-tokens=%s:5:1:10:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+// CHECK: error: main file cannot be included recursively when building a 
preamble
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1890,6 +1890,18 @@
 return;
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_for_preamble);
+return;
+  }
+
   // Should we enter the source file? Set to false if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined or is a modular 
header
Index: lib/Basic/SourceManager.cpp
===
--- lib/Basic/SourceManager.cpp
+++ lib/Basic/SourceManager.cpp
@@ -1589,7 +1589,7 @@
 // as the main file.
 const FileEntry *MainFile = MainContentCache->OrigEntry;
 SourceFileName = llvm::sys::path::filename(SourceFile->getName());
-if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) 
{
+if (MainFile && *SourceFileName == 
llvm::sys::path::filename(MainFile->getName())) {
   SourceFileUID = getActualFileUID(SourceFile);
   if (SourceFileUID) {
 if (Optional MainFileUID =
Index: include/clang/Basic/DiagnosticLexKinds.td
===
--- include/clang/Basic/DiagnosticLexKinds.td
+++ include/clang/Basic/DiagnosticLexKinds.td
@@ -422,6 +422,8 @@
   "did not find header '%0' in framework '%1' (loaded from '%2')">;
 def err_pp_error_opening_file : Error<
   "error opening file '%0': %1">, DefaultFatal;
+def err_pp_including_mainfile_for_preamble : Error<
+  "main file cannot be included recursively when building a preamble">;
 def err_pp_empty_filename : Error<"empty filename">;
 def err_pp_include_too_deep : Error<"#include nested too deeply">;
 def err_pp_expects_filename : Error<"expected \"FILENAME\" or ">;


Index: test/Index/preamble-cyclic-include.cpp
===
--- /dev/null
+++ test/Index/preamble-cyclic-include.cpp
@@ -0,0 +1,9 @@
+// RUN: env CINDEXTEST_EDITING=1 c-index-test -test-annotate-tokens=%s:5:1:10:1 %s 2>&1 | FileCheck %s
+// CHECK-NOT: error: unterminated conditional directive
+// CHECK-NOT: Skipping: [4:1 - 8:7]
+// CHECK: error: main file cannot be included recursively when building a preamble
+#ifndef A_H
+#define A_H
+#  include "preamble-cyclic-include.cpp"
+int bar();
+#endif
Index: lib/Lex/PPDirectives.cpp
===
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -1890,6 +1890,18 @@
 return;
   }
 
+  // Check for circular inclusion of the main file.
+  // We can't generate a consistent preamble with regard to the conditional
+  // stack if the main file is included again as due to the preamble bounds
+  // some directives (e.g. #endif of a header guard) will never be seen.
+  // Since this will lead to confusing errors, avoid the inclusion.
+  if (File && PreambleConditionalStack.isRecording() &&
+  SourceMgr.translateFile(File) == SourceMgr.getMainFileID()) {
+Diag(FilenameTok.getLocation(),
+ diag::err_pp_including_mainfile_for_preamble);
+return;
+  }
+
   // Should we enter the source file? Set to false if either the source file is
   // known to have no effect beyond its effect on module visibility -- that is,
   // if it's got an include guard that is already defined or is a modular header
Index: lib/Basic/SourceManager.cpp
=

r354023 - [Builtins] Treat `bcmp` as a builtin.

[Clement Courbet via cfe-commits]

Author: courbet
Date: Thu Feb 14 04:00:34 2019
New Revision: 354023

URL: http://llvm.org/viewvc/llvm-project?rev=354023&view=rev
Log:
[Builtins] Treat `bcmp` as a builtin.

Summary:
This makes it consistent with `memcmp` and `__builtin_bcmp`.

Also see the discussion in https://reviews.llvm.org/D56593.

Reviewers: jyknight

Subscribers: kristina, cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/Basic/Builtins.def
cfe/trunk/lib/AST/Decl.cpp
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/Analysis/bstring.c
cfe/trunk/test/Analysis/security-syntax-checks.m
cfe/trunk/test/SemaCXX/constexpr-string.cpp
cfe/trunk/test/SemaCXX/warn-bad-memaccess.cpp

Modified: cfe/trunk/include/clang/Basic/Builtins.def
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/Builtins.def?rev=354023&r1=354022&r2=354023&view=diff
==
--- cfe/trunk/include/clang/Basic/Builtins.def (original)
+++ cfe/trunk/include/clang/Basic/Builtins.def Thu Feb 14 04:00:34 2019
@@ -448,7 +448,7 @@ BUILTIN(__builtin_va_end, "vA", "n")
 BUILTIN(__builtin_va_copy, "vAA", "n")
 BUILTIN(__builtin_stdarg_start, "vA.", "n")
 BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
-BUILTIN(__builtin_bcmp, "iv*v*z", "Fn")
+BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")
 BUILTIN(__builtin_bzero, "vv*z", "nF")
 BUILTIN(__builtin_fprintf, "iP*cC*.", "Fp:1:")
@@ -953,6 +953,7 @@ LIBBUILTIN(strndup, "c*cC*z", "f",
 LIBBUILTIN(index, "c*cC*i",   "f", "strings.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(rindex, "c*cC*i",  "f", "strings.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(bzero, "vv*z", "f", "strings.h", ALL_GNU_LANGUAGES)
+LIBBUILTIN(bcmp, "ivC*vC*z",  "f", "strings.h", ALL_GNU_LANGUAGES)
 // In some systems str[n]casejmp is a macro that expands to _str[n]icmp.
 // We undefine then here to avoid wrong name.
 #undef strcasecmp

Modified: cfe/trunk/lib/AST/Decl.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/Decl.cpp?rev=354023&r1=354022&r2=354023&view=diff
==
--- cfe/trunk/lib/AST/Decl.cpp (original)
+++ cfe/trunk/lib/AST/Decl.cpp Thu Feb 14 04:00:34 2019
@@ -3672,6 +3672,10 @@ unsigned FunctionDecl::getMemoryFunction
   case Builtin::BImemcmp:
 return Builtin::BImemcmp;
 
+  case Builtin::BI__builtin_bcmp:
+  case Builtin::BIbcmp:
+return Builtin::BIbcmp;
+
   case Builtin::BI__builtin_strncpy:
   case Builtin::BI__builtin___strncpy_chk:
   case Builtin::BIstrncpy:
@@ -3712,6 +3716,8 @@ unsigned FunctionDecl::getMemoryFunction
 return Builtin::BImemmove;
   else if (FnInfo->isStr("memcmp"))
 return Builtin::BImemcmp;
+  else if (FnInfo->isStr("bcmp"))
+return Builtin::BIbcmp;
   else if (FnInfo->isStr("strncpy"))
 return Builtin::BIstrncpy;
   else if (FnInfo->isStr("strncmp"))

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=354023&r1=354022&r2=354023&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Thu Feb 14 04:00:34 2019
@@ -8426,6 +8426,7 @@ bool IntExprEvaluator::VisitBuiltinCallE
   case Builtin::BIstrncmp:
   case Builtin::BIwcsncmp:
   case Builtin::BImemcmp:
+  case Builtin::BIbcmp:
   case Builtin::BIwmemcmp:
 // A call to strlen is not a constant expression.
 if (Info.getLangOpts().CPlusPlus11)
@@ -8440,6 +8441,7 @@ bool IntExprEvaluator::VisitBuiltinCallE
   case Builtin::BI__builtin_strncmp:
   case Builtin::BI__builtin_wcsncmp:
   case Builtin::BI__builtin_memcmp:
+  case Builtin::BI__builtin_bcmp:
   case Builtin::BI__builtin_wmemcmp: {
 LValue String1, String2;
 if (!EvaluatePointer(E->getArg(0), String1, Info) ||
@@ -8470,7 +8472,9 @@ bool IntExprEvaluator::VisitBuiltinCallE
 QualType CharTy2 = String2.Designator.getType(Info.Ctx);
 
 bool IsRawByte = BuiltinOp == Builtin::BImemcmp ||
- BuiltinOp == Builtin::BI__builtin_memcmp;
+ BuiltinOp == Builtin::BIbcmp ||
+ BuiltinOp == Builtin::BI__builtin_memcmp ||
+ BuiltinOp == Builtin::BI__builtin_bcmp;
 
 assert(IsRawByte ||
(Info.Ctx.hasSameUnqualifiedType(
@@ -8538,10 +8542,12 @@ bool IntExprEvaluator::VisitBuiltinCallE
   return Success(0, E);
 }
 
-bool StopAtNull = (BuiltinOp != Builtin::BImemcmp &&
-   BuiltinOp != Builtin::BIwmemcmp &&
-   BuiltinOp != Builtin::BI__builtin_memcmp &&
-   BuiltinOp != Builtin::BI__builtin_wmemcmp);
+bool StopAtNull =
+(BuiltinOp !=

[PATCH] D58120: [Builtins] Treat `bcmp` as a builtin.

[Clement Courbet via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC354023: [Builtins] Treat `bcmp` as a builtin. (authored by 
courbet, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D58120?vs=186651&id=186814#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D58120

Files:
  include/clang/Basic/Builtins.def
  lib/AST/Decl.cpp
  lib/AST/ExprConstant.cpp
  lib/Sema/SemaChecking.cpp
  test/Analysis/bstring.c
  test/Analysis/security-syntax-checks.m
  test/SemaCXX/constexpr-string.cpp
  test/SemaCXX/warn-bad-memaccess.cpp

Index: include/clang/Basic/Builtins.def
===
--- include/clang/Basic/Builtins.def
+++ include/clang/Basic/Builtins.def
@@ -448,7 +448,7 @@
 BUILTIN(__builtin_va_copy, "vAA", "n")
 BUILTIN(__builtin_stdarg_start, "vA.", "n")
 BUILTIN(__builtin_assume_aligned, "v*vC*z.", "nc")
-BUILTIN(__builtin_bcmp, "iv*v*z", "Fn")
+BUILTIN(__builtin_bcmp, "ivC*vC*z", "Fn")
 BUILTIN(__builtin_bcopy, "vv*v*z", "n")
 BUILTIN(__builtin_bzero, "vv*z", "nF")
 BUILTIN(__builtin_fprintf, "iP*cC*.", "Fp:1:")
@@ -953,6 +953,7 @@
 LIBBUILTIN(index, "c*cC*i",   "f", "strings.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(rindex, "c*cC*i",  "f", "strings.h", ALL_GNU_LANGUAGES)
 LIBBUILTIN(bzero, "vv*z", "f", "strings.h", ALL_GNU_LANGUAGES)
+LIBBUILTIN(bcmp, "ivC*vC*z",  "f", "strings.h", ALL_GNU_LANGUAGES)
 // In some systems str[n]casejmp is a macro that expands to _str[n]icmp.
 // We undefine then here to avoid wrong name.
 #undef strcasecmp
Index: test/Analysis/bstring.c
===
--- test/Analysis/bstring.c
+++ test/Analysis/bstring.c
@@ -361,8 +361,7 @@
 #ifdef VARIANT
 
 #define bcmp BUILTIN(bcmp)
-// __builtin_bcmp is not defined with const in Builtins.def.
-int bcmp(/*const*/ void *s1, /*const*/ void *s2, size_t n);
+int bcmp(const void *s1, const void *s2, size_t n);
 #define memcmp bcmp
 // 
 #else /* VARIANT */
Index: test/Analysis/security-syntax-checks.m
===
--- test/Analysis/security-syntax-checks.m
+++ test/Analysis/security-syntax-checks.m
@@ -41,7 +41,7 @@
 }
 
 // Obsolete function bcmp
-int bcmp(void *, void *, size_t);
+int bcmp(const void *, const void *, size_t);
 
 int test_bcmp(void *a, void *b, size_t n) {
   return bcmp(a, b, n); // expected-warning{{The bcmp() function is obsoleted by memcmp()}}
Index: test/SemaCXX/constexpr-string.cpp
===
--- test/SemaCXX/constexpr-string.cpp
+++ test/SemaCXX/constexpr-string.cpp
@@ -1,7 +1,9 @@
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
+// RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=gnu++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -DGNUMODE
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple x86_64-linux-gnu -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension
+// RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=gnu++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -DGNUMODE
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-signed-char
 // RUN: %clang_cc1 %s -triple armebv7-unknown-linux -std=c++2a -fsyntax-only -verify -pedantic -Wno-vla-extension -fno-wchar -DNO_PREDEFINED_WCHAR_T
 
@@ -15,6 +17,10 @@
   extern int strncmp(const char *s1, const char *s2, size_t n);
   extern int memcmp(const void *s1, const void *s2, size_t n);
 
+#ifdef GNUMODE
+  extern int bcmp(const void *s1, const void *s2, size_t n);
+#endif
+
   extern char *strchr(const char *s, int c);
   extern void *memchr(const void *s, int c, size_t n);
 
@@ -101,12 +107,28 @@
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 6) == -1);
   static_assert(__builtin_memcmp("abab\0banana", "abab\0canada", 5) == 0);
 
+  static_assert(__builtin_bcmp("abaa", "abba", 3) != 0);
+  static_assert(__builtin_bcmp("abaa", "abba", 2) == 0);
+  static_assert(__builtin_bcmp("a\203", "a", 2) != 0);
+  static_assert(__builtin_bcmp("a\203", "a\003", 2) != 0);
+  static_assert(__builtin_bcmp(0, 0, 0) == 0);
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0banana", 100) == 0); // expected-error {{not an integral constant}} expected-note {{dereferenced one-past-the-end}}
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 100) != 0); // FIXME: Should we reject this?
+  static_assert(__builtin_bcmp("abab\0banana", "abab\0canada", 7) != 0);
+  static_assert(__builtin_bcmp("abab\0

[PATCH] D53866: [Preamble] Stop generating preamble for circular #includes

[Nikolai Kosjar via Phabricator via cfe-commits]

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



Comment at: lib/Basic/SourceManager.cpp:1592
 SourceFileName = llvm::sys::path::filename(SourceFile->getName());
-if (*SourceFileName == llvm::sys::path::filename(MainFile->getName())) 
{
+if (MainFile && *SourceFileName == 
llvm::sys::path::filename(MainFile->getName())) {
   SourceFileUID = getActualFileUID(SourceFile);

I've added the nullptr check here as LibclangReparseTest.ReparseWithModule 
fails/crashes otherwise.


Repository:
  rC Clang

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

https://reviews.llvm.org/D53866



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

r353990 - [HWASAN] Updated HWASAN design document to better portray the chance of missing a bug.

[Mitch Phillips via cfe-commits]

Author: hctim
Date: Wed Feb 13 15:14:54 2019
New Revision: 353990

URL: http://llvm.org/viewvc/llvm-project?rev=353990&view=rev
Log:
[HWASAN] Updated HWASAN design document to better portray the chance of missing 
a bug.

Summary: Provided rule of thumb percentage chances of miss for 4 and 8 bit tag 
sizes.

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst

Modified: cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst?rev=353990&r1=353989&r2=353990&view=diff
==
--- cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst (original)
+++ cfe/trunk/docs/HardwareAssistedAddressSanitizerDesign.rst Wed Feb 13 
15:14:54 2019
@@ -131,7 +131,8 @@ HWASAN:
 https://www.kernel.org/doc/Documentation/arm64/tagged-pointers.txt).
   * **Does not require redzones to detect buffer overflows**,
 but the buffer overflow detection is probabilistic, with roughly
-`(2**TS-1)/(2**TS)` probability of catching a bug.
+`1/(2**TS)` chance of missing a bug (6.25% or 0.39% with 4 and 8-bit TS
+respectively).
   * **Does not require quarantine to detect heap-use-after-free,
 or stack-use-after-return**.
 The detection is similarly probabilistic.


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

r354027 - [ASTImporter] Check visibility/linkage of functions and variables

[Gabor Marton via cfe-commits]

Author: martong
Date: Thu Feb 14 05:07:03 2019
New Revision: 354027

URL: http://llvm.org/viewvc/llvm-project?rev=354027&view=rev
Log:
[ASTImporter] Check visibility/linkage of functions and variables

Summary:
During import of a global variable with external visibility the lookup
will find variables (with the same name) but with static visibility.
Clearly, we cannot put them into the same redecl chain.  The same is
true in case of functions.  In this fix we filter the lookup results and
consider only those which have the same visibility as the decl we
currently import.

We consider two decls in two anonymous namsepaces to have the same
visibility only if they are imported from the very same translation
unit.

Reviewers: a_sidorin, shafik, a.sidorin

Reviewed By: shafik

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

Tags: #clang

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

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

Modified: cfe/trunk/include/clang/AST/ASTImporter.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/ASTImporter.h?rev=354027&r1=354026&r2=354027&view=diff
==
--- cfe/trunk/include/clang/AST/ASTImporter.h (original)
+++ cfe/trunk/include/clang/AST/ASTImporter.h Thu Feb 14 05:07:03 2019
@@ -32,6 +32,7 @@
 namespace clang {
 
 class ASTContext;
+class Attr;
 class ASTImporterLookupTable;
 class CXXBaseSpecifier;
 class CXXCtorInitializer;
@@ -42,8 +43,8 @@ class FileManager;
 class NamedDecl;
 class Stmt;
 class TagDecl;
+class TranslationUnitDecl;
 class TypeSourceInfo;
-class Attr;
 
   class ImportError : public llvm::ErrorInfo {
   public:
@@ -115,6 +116,10 @@ class Attr;
 /// context to the corresponding declarations in the "to" context.
 llvm::DenseMap ImportedDecls;
 
+/// Mapping from the already-imported declarations in the "to"
+/// context to the corresponding declarations in the "from" context.
+llvm::DenseMap ImportedFromDecls;
+
 /// Mapping from the already-imported statements in the "from"
 /// context to the corresponding statements in the "to" context.
 llvm::DenseMap ImportedStmts;
@@ -226,9 +231,13 @@ class Attr;
 
 /// Return the copy of the given declaration in the "to" context if
 /// it has already been imported from the "from" context.  Otherwise return
-/// NULL.
+/// nullptr.
 Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
 
+/// Return the translation unit from where the declaration was
+/// imported. If it does not exist nullptr is returned.
+TranslationUnitDecl *GetFromTU(Decl *ToD);
+
 /// Import the given declaration context from the "from"
 /// AST context into the "to" AST context.
 ///

Modified: cfe/trunk/lib/AST/ASTImporter.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ASTImporter.cpp?rev=354027&r1=354026&r2=354027&view=diff
==
--- cfe/trunk/lib/AST/ASTImporter.cpp (original)
+++ cfe/trunk/lib/AST/ASTImporter.cpp Thu Feb 14 05:07:03 2019
@@ -439,6 +439,9 @@ namespace clang {
 
 Error ImportFunctionDeclBody(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
+template 
+bool hasSameVisibilityContext(T *Found, T *From);
+
 bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
 bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
bool Complain = true);
@@ -2957,6 +2960,19 @@ Error ASTNodeImporter::ImportFunctionDec
   return Error::success();
 }
 
+template 
+bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
+  if (From->hasExternalFormalLinkage())
+return Found->hasExternalFormalLinkage();
+  if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
+return false;
+  if (From->isInAnonymousNamespace())
+return Found->isInAnonymousNamespace();
+  else
+return !Found->isInAnonymousNamespace() &&
+   !Found->hasExternalFormalLinkage();
+}
+
 ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   SmallVector Redecls = getCanonicalForwardRedeclChain(D);
@@ -3010,33 +3026,30 @@ ExpectedDecl ASTNodeImporter::VisitFunct
 continue;
 
   if (auto *FoundFunction = dyn_cast(FoundDecl)) {
-if (FoundFunction->hasExternalFormalLinkage() &&
-D->hasExternalFormalLinkage()) {
-  if (IsStructuralMatch(D, FoundFunction)) {
-const FunctionDecl *Definition = nullptr;
-if (D->doesThisDeclarationHaveABody() &&
-FoundFunction->hasBody(Definition)) {
-  return Importer.MapImported(
-  D, const_cast(Definition));
-}
-FoundByLookup = FoundFunction;
-break;
-  }
+if (!hasSameVisibilit

[PATCH] D57232: [ASTImporter] Check visibility/linkage of functions and variables

[Gabor Marton via Phabricator via cfe-commits]

martong added a comment.

@shafik Thanks for the review!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57232



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

[PATCH] D57232: [ASTImporter] Check visibility/linkage of functions and variables

[Gabor Marton via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354027: [ASTImporter] Check visibility/linkage of functions 
and variables (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/D57232/new/

https://reviews.llvm.org/D57232

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

Index: cfe/trunk/include/clang/AST/ASTImporter.h
===
--- cfe/trunk/include/clang/AST/ASTImporter.h
+++ cfe/trunk/include/clang/AST/ASTImporter.h
@@ -32,6 +32,7 @@
 namespace clang {
 
 class ASTContext;
+class Attr;
 class ASTImporterLookupTable;
 class CXXBaseSpecifier;
 class CXXCtorInitializer;
@@ -42,8 +43,8 @@
 class NamedDecl;
 class Stmt;
 class TagDecl;
+class TranslationUnitDecl;
 class TypeSourceInfo;
-class Attr;
 
   class ImportError : public llvm::ErrorInfo {
   public:
@@ -115,6 +116,10 @@
 /// context to the corresponding declarations in the "to" context.
 llvm::DenseMap ImportedDecls;
 
+/// Mapping from the already-imported declarations in the "to"
+/// context to the corresponding declarations in the "from" context.
+llvm::DenseMap ImportedFromDecls;
+
 /// Mapping from the already-imported statements in the "from"
 /// context to the corresponding statements in the "to" context.
 llvm::DenseMap ImportedStmts;
@@ -226,9 +231,13 @@
 
 /// Return the copy of the given declaration in the "to" context if
 /// it has already been imported from the "from" context.  Otherwise return
-/// NULL.
+/// nullptr.
 Decl *GetAlreadyImportedOrNull(const Decl *FromD) const;
 
+/// Return the translation unit from where the declaration was
+/// imported. If it does not exist nullptr is returned.
+TranslationUnitDecl *GetFromTU(Decl *ToD);
+
 /// Import the given declaration context from the "from"
 /// AST context into the "to" AST context.
 ///
Index: cfe/trunk/lib/AST/ASTImporter.cpp
===
--- cfe/trunk/lib/AST/ASTImporter.cpp
+++ cfe/trunk/lib/AST/ASTImporter.cpp
@@ -439,6 +439,9 @@
 
 Error ImportFunctionDeclBody(FunctionDecl *FromFD, FunctionDecl *ToFD);
 
+template 
+bool hasSameVisibilityContext(T *Found, T *From);
+
 bool IsStructuralMatch(Decl *From, Decl *To, bool Complain);
 bool IsStructuralMatch(RecordDecl *FromRecord, RecordDecl *ToRecord,
bool Complain = true);
@@ -2957,6 +2960,19 @@
   return Error::success();
 }
 
+template 
+bool ASTNodeImporter::hasSameVisibilityContext(T *Found, T *From) {
+  if (From->hasExternalFormalLinkage())
+return Found->hasExternalFormalLinkage();
+  if (Importer.GetFromTU(Found) != From->getTranslationUnitDecl())
+return false;
+  if (From->isInAnonymousNamespace())
+return Found->isInAnonymousNamespace();
+  else
+return !Found->isInAnonymousNamespace() &&
+   !Found->hasExternalFormalLinkage();
+}
+
 ExpectedDecl ASTNodeImporter::VisitFunctionDecl(FunctionDecl *D) {
 
   SmallVector Redecls = getCanonicalForwardRedeclChain(D);
@@ -3010,33 +3026,30 @@
 continue;
 
   if (auto *FoundFunction = dyn_cast(FoundDecl)) {
-if (FoundFunction->hasExternalFormalLinkage() &&
-D->hasExternalFormalLinkage()) {
-  if (IsStructuralMatch(D, FoundFunction)) {
-const FunctionDecl *Definition = nullptr;
-if (D->doesThisDeclarationHaveABody() &&
-FoundFunction->hasBody(Definition)) {
-  return Importer.MapImported(
-  D, const_cast(Definition));
-}
-FoundByLookup = FoundFunction;
-break;
-  }
+if (!hasSameVisibilityContext(FoundFunction, D))
+  continue;
 
-  // FIXME: Check for overloading more carefully, e.g., by boosting
-  // Sema::IsOverload out to the AST library.
+if (IsStructuralMatch(D, FoundFunction)) {
+  const FunctionDecl *Definition = nullptr;
+  if (D->doesThisDeclarationHaveABody() &&
+  FoundFunction->hasBody(Definition))
+return Importer.MapImported(D,
+const_cast(Definition));
+  FoundByLookup = FoundFunction;
+  break;
+}
+// FIXME: Check for overloading more carefully, e.g., by boosting
+// Sema::IsOverload out to the AST library.
 
-  // Function overloading is okay in C++.
-  if (Importer.getToContext().getLangOpts().CPlusPlus)
-continue;
+// Function overloading is okay in C++.
+if (Importer.getToContext().getLangOpts().CPlusPlus)
+  continue;
 
-  // Complain about inconsistent function types.
-  

[PATCH] D57966: [clang-tidy] add camelBackOrCase casing style to readability-identifier-naming to support change to variable naming policy (if adopted)

[Jonas Toth via Phabricator via cfe-commits]

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



Comment at: 
test/clang-tidy/readability-identifier-naming-camelback-or-case.cpp:97
+class aB_def
+// CHECK-MESSAGES: :[[@LINE-1]]:7: warning: invalid case style
+{

why no test for fixing?


Repository:
  rCTE Clang Tools Extra

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

https://reviews.llvm.org/D57966



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

[PATCH] D47127: [RISCV] Default enable RISCV linker relaxation

[Alex Bradbury via Phabricator via cfe-commits]

asb edited subscribers, added: cfe-commits; removed: llvm-commits.
asb added a comment.

Removing llvm-commits and adding cfe-commits. This patch is mistakenly marked 
as targeting the LLVM repo rather than Clang.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D47127



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

r354035 - [Sema] Fix a regression introduced in "[AST][Sema] Remove CallExpr::setNumArgs"

[Bruno Ricci via cfe-commits]

Author: brunoricci
Date: Thu Feb 14 07:43:17 2019
New Revision: 354035

URL: http://llvm.org/viewvc/llvm-project?rev=354035&view=rev
Log:
[Sema] Fix a regression introduced in "[AST][Sema] Remove CallExpr::setNumArgs"

D54902 removed CallExpr::setNumArgs in preparation of tail-allocating the
arguments of CallExpr. It did this by allocating storage for
max(number of arguments, number of parameters in the prototype). The
temporarily nulled arguments however causes issues in BuildResolvedCallExpr
when typo correction is done just after the creation of the call expression.

This was unfortunately missed by the tests /:

To fix this, delay setting the number of arguments to
max(number of arguments, number of parameters in the prototype) until we are
ready for it. It would be nice to have this encapsulated in CallExpr but this
is the best I can come up with under the constraint that we cannot add
anything the CallExpr.

Fixes PR40286.

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

Reviewed By: aaron.ballman


Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/test/Sema/typo-correction.c

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=354035&r1=354034&r2=354035&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Feb 14 07:43:17 2019
@@ -2610,6 +2610,11 @@ public:
 NumArgs = NewNumArgs;
   }
 
+  /// Bluntly set a new number of arguments without doing any checks 
whatsoever.
+  /// Only used during construction of a CallExpr in a few places in Sema.
+  /// FIXME: Find a way to remove it.
+  void setNumArgsUnsafe(unsigned NewNumArgs) { NumArgs = NewNumArgs; }
+
   typedef ExprIterator arg_iterator;
   typedef ConstExprIterator const_arg_iterator;
   typedef llvm::iterator_range arg_range;

Modified: cfe/trunk/lib/Sema/SemaExpr.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaExpr.cpp?rev=354035&r1=354034&r2=354035&view=diff
==
--- cfe/trunk/lib/Sema/SemaExpr.cpp (original)
+++ cfe/trunk/lib/Sema/SemaExpr.cpp Thu Feb 14 07:43:17 2019
@@ -5793,18 +5793,36 @@ ExprResult Sema::BuildResolvedCallExpr(E
   }
 
   if (!getLangOpts().CPlusPlus) {
+// Forget about the nulled arguments since typo correction
+// do not handle them well.
+TheCall->shrinkNumArgs(Args.size());
 // C cannot always handle TypoExpr nodes in builtin calls and direct
 // function calls as their argument checking don't necessarily handle
 // dependent types properly, so make sure any TypoExprs have been
 // dealt with.
 ExprResult Result = CorrectDelayedTyposInExpr(TheCall);
 if (!Result.isUsable()) return ExprError();
+CallExpr *TheOldCall = TheCall;
 TheCall = dyn_cast(Result.get());
+bool CorrectedTypos = TheCall != TheOldCall;
 if (!TheCall) return Result;
-// TheCall at this point has max(Args.size(), NumParams) arguments,
-// with extra arguments nulled. We don't want to introduce nulled
-// arguments in Args and so we only take the first Args.size() arguments.
-Args = llvm::makeArrayRef(TheCall->getArgs(), Args.size());
+Args = llvm::makeArrayRef(TheCall->getArgs(), TheCall->getNumArgs());
+
+// A new call expression node was created if some typos were corrected.
+// However it may not have been constructed with enough storage. In this
+// case, rebuild the node with enough storage. The waste of space is
+// immaterial since this only happens when some typos were corrected.
+if (CorrectedTypos && Args.size() < NumParams) {
+  if (Config)
+TheCall = CUDAKernelCallExpr::Create(
+Context, Fn, cast(Config), Args, ResultTy, VK_RValue,
+RParenLoc, NumParams);
+  else
+TheCall = CallExpr::Create(Context, Fn, Args, ResultTy, VK_RValue,
+   RParenLoc, NumParams, UsesADL);
+}
+// We can now handle the nulled arguments for the default arguments.
+TheCall->setNumArgsUnsafe(std::max(Args.size(), NumParams));
   }
 
   // Bail out early if calling a builtin with custom type checking.

Modified: cfe/trunk/test/Sema/typo-correction.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Sema/typo-correction.c?rev=354035&r1=354034&r2=354035&view=diff
==
--- cfe/trunk/test/Sema/typo-correction.c (original)
+++ cfe/trunk/test/Sema/typo-correction.c Thu Feb 14 07:43:17 2019
@@ -100,3 +100,18 @@ void rdar38642201_caller() {
   structVar1.fieldName1.member1, //expected-error{{use of undeclared 
identifier 'structVar1'}}
   structVar2.fieldName2.member2); //expected-error{{use of undeclared 
identifier 'structVar2'}}
 }
+
+void PR40286_g(int 

[PATCH] D57948: [Sema] Fix a regression introduced in "[AST][Sema] Remove CallExpr::setNumArgs"

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC354035: [Sema] Fix a regression introduced in 
"[AST][Sema] Remove CallExpr::setNumArgs" (authored by brunoricci, 
committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D57948

Files:
  include/clang/AST/Expr.h
  lib/Sema/SemaExpr.cpp
  test/Sema/typo-correction.c


Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -2610,6 +2610,11 @@
 NumArgs = NewNumArgs;
   }
 
+  /// Bluntly set a new number of arguments without doing any checks 
whatsoever.
+  /// Only used during construction of a CallExpr in a few places in Sema.
+  /// FIXME: Find a way to remove it.
+  void setNumArgsUnsafe(unsigned NewNumArgs) { NumArgs = NewNumArgs; }
+
   typedef ExprIterator arg_iterator;
   typedef ConstExprIterator const_arg_iterator;
   typedef llvm::iterator_range arg_range;
Index: test/Sema/typo-correction.c
===
--- test/Sema/typo-correction.c
+++ test/Sema/typo-correction.c
@@ -100,3 +100,18 @@
   structVar1.fieldName1.member1, //expected-error{{use of undeclared 
identifier 'structVar1'}}
   structVar2.fieldName2.member2); //expected-error{{use of undeclared 
identifier 'structVar2'}}
 }
+
+void PR40286_g(int x, int y);
+void PR40286_h(int x, int y, int z);
+void PR40286_1(int the_value) {
+  PR40286_g(the_walue); // expected-error {{use of undeclared identifier 
'the_walue'}}
+}
+void PR40286_2(int the_value) {
+  PR40286_h(the_value, the_walue); // expected-error {{use of undeclared 
identifier 'the_walue'}}
+}
+void PR40286_3(int the_value) {
+  PR40286_h(the_walue); // expected-error {{use of undeclared identifier 
'the_walue'}}
+}
+void PR40286_4(int the_value) { // expected-note {{'the_value' declared here}}
+  PR40286_h(the_value, the_value, the_walue); // expected-error {{use of 
undeclared identifier 'the_walue'; did you mean 'the_value'?}}
+}
Index: lib/Sema/SemaExpr.cpp
===
--- lib/Sema/SemaExpr.cpp
+++ lib/Sema/SemaExpr.cpp
@@ -5793,18 +5793,36 @@
   }
 
   if (!getLangOpts().CPlusPlus) {
+// Forget about the nulled arguments since typo correction
+// do not handle them well.
+TheCall->shrinkNumArgs(Args.size());
 // C cannot always handle TypoExpr nodes in builtin calls and direct
 // function calls as their argument checking don't necessarily handle
 // dependent types properly, so make sure any TypoExprs have been
 // dealt with.
 ExprResult Result = CorrectDelayedTyposInExpr(TheCall);
 if (!Result.isUsable()) return ExprError();
+CallExpr *TheOldCall = TheCall;
 TheCall = dyn_cast(Result.get());
+bool CorrectedTypos = TheCall != TheOldCall;
 if (!TheCall) return Result;
-// TheCall at this point has max(Args.size(), NumParams) arguments,
-// with extra arguments nulled. We don't want to introduce nulled
-// arguments in Args and so we only take the first Args.size() arguments.
-Args = llvm::makeArrayRef(TheCall->getArgs(), Args.size());
+Args = llvm::makeArrayRef(TheCall->getArgs(), TheCall->getNumArgs());
+
+// A new call expression node was created if some typos were corrected.
+// However it may not have been constructed with enough storage. In this
+// case, rebuild the node with enough storage. The waste of space is
+// immaterial since this only happens when some typos were corrected.
+if (CorrectedTypos && Args.size() < NumParams) {
+  if (Config)
+TheCall = CUDAKernelCallExpr::Create(
+Context, Fn, cast(Config), Args, ResultTy, VK_RValue,
+RParenLoc, NumParams);
+  else
+TheCall = CallExpr::Create(Context, Fn, Args, ResultTy, VK_RValue,
+   RParenLoc, NumParams, UsesADL);
+}
+// We can now handle the nulled arguments for the default arguments.
+TheCall->setNumArgsUnsafe(std::max(Args.size(), NumParams));
   }
 
   // Bail out early if calling a builtin with custom type checking.


Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -2610,6 +2610,11 @@
 NumArgs = NewNumArgs;
   }
 
+  /// Bluntly set a new number of arguments without doing any checks whatsoever.
+  /// Only used during construction of a CallExpr in a few places in Sema.
+  /// FIXME: Find a way to remove it.
+  void setNumArgsUnsafe(unsigned NewNumArgs) { NumArgs = NewNumArgs; }
+
   typedef ExprIterator arg_iterator;
   typedef ConstExprIterator const_arg_iterator;
   typedef llvm::iterator_range arg_range;
Index: test/Sema/typo-correction.c
===
--- test/Sema/ty

[PATCH] D58236: Make address space conversions a bit stricter.

[Bevin Hansson via Phabricator via cfe-commits]

ebevhan created this revision.
ebevhan added reviewers: Anastasia, rjmccall.
Herald added subscribers: cfe-commits, jdoerfert.
Herald added a project: clang.

The semantics for converting nested pointers between address
spaces are not very well defined. Some conversions which do not
really carry any meaning only produce warnings, and in some cases
warnings hide invalid conversions, such as 'global int*' to
'local float*'!

This patch changes the logic in checkPointerTypesForAssignment
and checkAddressSpaceCast to fail properly on conversions that
should definitely not be permitted. We also dig deeper into the
pointer types and fail on conversions where the address space
in a nested pointer changes, regardless of whether the address
space is compatible with the corresponding pointer nesting level
on the destination type.

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


Repository:
  rC Clang

https://reviews.llvm.org/D58236

Files:
  lib/Sema/SemaCast.cpp
  lib/Sema/SemaExpr.cpp
  test/CodeGenOpenCL/numbered-address-space.cl
  test/SemaOpenCL/address-spaces.cl
  test/SemaOpenCL/event_t_overload.cl
  test/SemaOpenCL/numbered-address-space.cl
  test/SemaOpenCL/queue_t_overload.cl

Index: test/SemaOpenCL/queue_t_overload.cl
===
--- test/SemaOpenCL/queue_t_overload.cl
+++ test/SemaOpenCL/queue_t_overload.cl
@@ -7,6 +7,6 @@
   queue_t q;
   foo(q, src1);
   foo(0, src2);
-  foo(q, src3); // expected-error {{call to 'foo' is ambiguous}}
+  foo(q, src3); // expected-error {{no matching function for call to 'foo'}}
   foo(1, src3); // expected-error {{no matching function for call to 'foo'}}
 }
Index: test/SemaOpenCL/numbered-address-space.cl
===
--- test/SemaOpenCL/numbered-address-space.cl
+++ test/SemaOpenCL/numbered-address-space.cl
@@ -26,6 +26,6 @@
 
 void test_generic_as_to_builtin_parameterimplicit_cast_numeric(__attribute__((address_space(3))) int *as3_ptr, float src) {
   generic int* generic_ptr = as3_ptr;
-  volatile float result = __builtin_amdgcn_ds_fmaxf(generic_ptr, src, 0, 0, false); // expected-warning {{incompatible pointer types passing '__generic int *' to parameter of type '__local float *'}}
+  volatile float result = __builtin_amdgcn_ds_fmaxf(generic_ptr, src, 0, 0, false); // expected-error {{passing '__generic int *' to parameter of type '__local float *' changes address space of pointer}}
 }
 
Index: test/SemaOpenCL/event_t_overload.cl
===
--- test/SemaOpenCL/event_t_overload.cl
+++ test/SemaOpenCL/event_t_overload.cl
@@ -7,5 +7,5 @@
   event_t evt;
   foo(evt, src1);
   foo(0, src2);
-  foo(evt, src3); // expected-error {{call to 'foo' is ambiguous}}
+  foo(evt, src3); // expected-error {{no matching function for call to 'foo'}}
 }
Index: test/SemaOpenCL/address-spaces.cl
===
--- test/SemaOpenCL/address-spaces.cl
+++ test/SemaOpenCL/address-spaces.cl
@@ -52,6 +52,76 @@
   p = (__private int *)p2;
 }
 
+#if !__OPENCL_CPP_VERSION__
+void nested(__global int *g, __global int * __private *gg, __local int *l, __local int * __private *ll, __global float * __private *gg_f) {
+  g = gg;// expected-error {{assigning '__global int **' to '__global int *' changes address space of pointer}}
+  g = l; // expected-error {{assigning '__local int *' to '__global int *' changes address space of pointer}}
+  g = ll;// expected-error {{assigning '__local int **' to '__global int *' changes address space of pointer}}
+  g = gg_f;  // expected-error {{assigning '__global float **' to '__global int *' changes address space of pointer}}
+  g = (__global int *)gg_f; // expected-error {{casting '__global float **' to type '__global int *' changes address space of pointer}}
+
+  gg = g;// expected-error {{assigning '__global int *' to '__global int **' changes address space of pointer}}
+  gg = l;// expected-error {{assigning '__local int *' to '__global int **' changes address space of pointer}}
+  gg = ll;   // expected-error {{assigning '__local int **' to '__global int **' changes address space of pointer}}
+  gg = gg_f; // expected-warning {{incompatible pointer types assigning to '__global int **' from '__global float **'}}
+  gg = (__global int * __private *)gg_f;
+
+  l = g; // expected-error {{assigning '__global int *' to '__local int *' changes address space of pointer}}
+  l = gg;// expected-error {{assigning '__global int **' to '__local int *' changes address space of pointer}}
+  l = ll;// expected-error {{assigning '__local int **' to '__local int *' changes address space of pointer}}
+  l = gg_f;  // expected-error {{assigning '__global float **' to '__local int *' changes address space of pointer}}
+  l = (__local int *)gg_f; // expected-error {{casting '__global float **' to type '__local int *' changes addres

[PATCH] D58239: [clangd] Cache include fixes for diagnostics caused by the same unresolved name or incomplete type.

[Eric Liu via Phabricator via cfe-commits]

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

Multiple diagnostics can be caused by the same unresolved name or incomplete 
type,
especially if the code is copy-pasted without #includes. The cache can avoid 
making
repetitive index requests, and thus reduce latency and allow more diagnostics 
to be
fixed (we limit the number of index requests for each parse).


Repository:
  rCTE Clang Tools Extra

https://reviews.llvm.org/D58239

Files:
  clangd/IncludeFixer.cpp
  clangd/IncludeFixer.h
  unittests/clangd/DiagnosticsTests.cpp

Index: unittests/clangd/DiagnosticsTests.cpp
===
--- unittests/clangd/DiagnosticsTests.cpp
+++ unittests/clangd/DiagnosticsTests.cpp
@@ -449,6 +449,44 @@
   UnorderedElementsAre(Diag(Test.range(), "no member named 'xy' in 'X'")));
 }
 
+TEST(IncludeFixerTest, UseCachedIndexResults) {
+  // As index results for the identical request are cached, more than 5 fixes
+  // are generated.
+  Annotations Test(R"cpp(
+$insert[[]]void foo() {
+  $x1[[X]] x;
+  $x2[[X]] x;
+  $x3[[X]] x;
+  $x4[[X]] x;
+  $x5[[X]] x;
+  $x6[[X]] x;
+  $x7[[X]] x;
+}
+
+class X;
+void bar(X *x) {
+  x$a1[[->]]f();
+  x$a2[[->]]f();
+  x$a3[[->]]f();
+  x$a4[[->]]f();
+  x$a5[[->]]f();
+  x$a6[[->]]f();
+  x$a7[[->]]f();
+}
+  )cpp");
+  auto TU = TestTU::withCode(Test.code());
+  auto Index =
+  buildIndexWithSymbol(SymbolWithHeader{"X", "unittest:///a.h", "\"a.h\""});
+  TU.ExternalIndex = Index.get();
+
+  auto Parsed = TU.build();
+  for (const auto &D : Parsed.getDiagnostics()) {
+EXPECT_EQ(D.Fixes.size(), 1u);
+EXPECT_EQ(D.Fixes[0].Message,
+  std::string("Add include \"a.h\" for symbol X"));
+  }
+}
+
 } // namespace
 } // namespace clangd
 } // namespace clang
Index: clangd/IncludeFixer.h
===
--- clangd/IncludeFixer.h
+++ clangd/IncludeFixer.h
@@ -18,9 +18,9 @@
 #include "clang/Sema/ExternalSemaSource.h"
 #include "clang/Sema/Sema.h"
 #include "llvm/ADT/ArrayRef.h"
-#include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/IntrusiveRefCntPtr.h"
 #include "llvm/ADT/Optional.h"
+#include "llvm/ADT/StringMap.h"
 #include "llvm/ADT/StringRef.h"
 #include 
 
@@ -79,6 +79,15 @@
   // These collect the last unresolved name so that we can associate it with the
   // diagnostic.
   llvm::Optional LastUnresolvedName;
+
+  // There can be multiple diagnostics that are caused by the same unresolved
+  // name or incomplete type in one parse, especially when code is
+  // copy-and-pasted without #includes. As fixes are purely dependent on index
+  // requests and index results at this point, we can cache fixes by index
+  // requests to avoid repetitive index queries (assuming index results are
+  // consistent during the single AST parse).
+  mutable llvm::StringMap> FuzzyReqToFixesCache;
+  mutable llvm::StringMap> LookupIDToFixesCache;
 };
 
 } // namespace clangd
Index: clangd/IncludeFixer.cpp
===
--- clangd/IncludeFixer.cpp
+++ clangd/IncludeFixer.cpp
@@ -57,8 +57,6 @@
 
 std::vector IncludeFixer::fix(DiagnosticsEngine::Level DiagLevel,
const clang::Diagnostic &Info) const {
-  if (IndexRequestCount >= IndexRequestLimit)
-return {}; // Avoid querying index too many times in a single parse.
   switch (Info.getID()) {
   case diag::err_incomplete_type:
   case diag::err_incomplete_member_access:
@@ -118,11 +116,17 @@
   auto ID = getSymbolID(TD);
   if (!ID)
 return {};
-  ++IndexRequestCount;
-  // FIXME: consider batching the requests for all diagnostics.
-  // FIXME: consider caching the lookup results.
   LookupRequest Req;
   Req.IDs.insert(*ID);
+
+  std::string IDStr = ID->str();
+  auto I = LookupIDToFixesCache.find(IDStr);
+  if (I != LookupIDToFixesCache.end())
+return I->second;
+
+  if (IndexRequestCount++ >= IndexRequestLimit)
+return {};
+  // FIXME: consider batching the requests for all diagnostics.
   llvm::Optional Matched;
   Index.lookup(Req, [&](const Symbol &Sym) {
 if (Matched)
@@ -130,10 +134,12 @@
 Matched = Sym;
   });
 
-  if (!Matched || Matched->IncludeHeaders.empty() || !Matched->Definition ||
-  Matched->CanonicalDeclaration.FileURI != Matched->Definition.FileURI)
-return {};
-  return fixesForSymbols({*Matched});
+  std::vector Fixes;
+  if (Matched && !Matched->IncludeHeaders.empty() && Matched->Definition &&
+  Matched->CanonicalDeclaration.FileURI == Matched->Definition.FileURI)
+Fixes = fixesForSymbols({*Matched});
+  LookupIDToFixesCache[IDStr] = Fixes;
+  return Fixes;
 }
 
 std::vector
@@ -289,6 +295,14 @@
   Req.RestrictForCodeCompletion = true;
   Req.Limit = 100;
 
+  auto ReqStr = llvm::formatv("{0}", toJSON(Req)).str();
+  auto I = Fuz

[PATCH] D58147: [CodeGen] Fix calling llvm.var.annotation outside of a basic block.

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Okay.  Is it acceptable for the annotation to simply disappear in this case?  I 
don't really understand the purposes of annotations well enough to judge.


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

https://reviews.llvm.org/D58147



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

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D56411#1365878 , @yaxunl wrote:

> In D56411#1365745 , @rjmccall wrote:
>
> > In D56411#1365727 , @yaxunl wrote:
> >
> > > In D56411#1360010 , @rjmccall 
> > > wrote:
> > >
> > > > I think the diagnostic should come during instantiation when you find 
> > > > an evaluated use of a host function within a device function.
> > >
> > >
> > > It seems the body of function template is checked only during parsing of 
> > > the definition of the template itself. When a function
> > >  template is instantiated, the body of the instantiated function is not 
> > > checked again.
> >
> >
> > No, that's not correct.  However, it's checked somewhat differently, and 
> > it's possible that the existing diagnostic is not set up to fire along all 
> > common paths.  Try moving the diagnostic to `MarkFunctionReferenced`, and 
> > note that `OdrUse` will be `false` in all the unevaluated contexts.
>
>
> You are right. After I disable current diagnostic, I saw 
> PerformPendingInstantiations at the end of parsing the TU, where the AST of 
> the instantiated function is iterated and MarkFunctionReferenced is called. I 
> will try to fix my patch as suggested. Thanks.


I got one concern. If we want to do overload resolution of function type 
template argument based on host or device, we need to do that before template 
instantiation, right?

e.g. we have two functions having the same name f and type, but one is 
`__host__` and the other is `__device__`, and we pass it as a template argument 
to a template function g. We want to choose `__device__ f` if g itself is 
`__device__` and `__host__ f` if g itself is `__host__`. If we want to do this 
we have to do the check before template instantiation, right?


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

https://reviews.llvm.org/D56411



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

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

Well, you can always make a variable with a more directly-applicable name than 
`capturedByInit` and update it as appropriate, like `locIsByrefHeader`.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58218



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

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D56411#1398097 , @yaxunl wrote:

> In D56411#1365878 , @yaxunl wrote:
>
> > In D56411#1365745 , @rjmccall 
> > wrote:
> >
> > > In D56411#1365727 , @yaxunl 
> > > wrote:
> > >
> > > > In D56411#1360010 , @rjmccall 
> > > > wrote:
> > > >
> > > > > I think the diagnostic should come during instantiation when you find 
> > > > > an evaluated use of a host function within a device function.
> > > >
> > > >
> > > > It seems the body of function template is checked only during parsing 
> > > > of the definition of the template itself. When a function
> > > >  template is instantiated, the body of the instantiated function is not 
> > > > checked again.
> > >
> > >
> > > No, that's not correct.  However, it's checked somewhat differently, and 
> > > it's possible that the existing diagnostic is not set up to fire along 
> > > all common paths.  Try moving the diagnostic to `MarkFunctionReferenced`, 
> > > and note that `OdrUse` will be `false` in all the unevaluated contexts.
> >
> >
> > You are right. After I disable current diagnostic, I saw 
> > PerformPendingInstantiations at the end of parsing the TU, where the AST of 
> > the instantiated function is iterated and MarkFunctionReferenced is called. 
> > I will try to fix my patch as suggested. Thanks.
>
>
> I got one concern. If we want to do overload resolution of function type 
> template argument based on host or device, we need to do that before template 
> instantiation, right?
>
> e.g. we have two functions having the same name f and type, but one is 
> `__host__` and the other is `__device__`, and we pass it as a template 
> argument to a template function g. We want to choose `__device__ f` if g 
> itself is `__device__` and `__host__ f` if g itself is `__host__`. If we want 
> to do this we have to do the check before template instantiation, right?


Yes, you would need to check that when resolving the overload to a single 
declaration.  That would be separate from diagnosing uses.

That said, does CUDA have a general rule resolving `__host__` vs. `__device__` 
overloads based on context?  And does it allow overloading based solely on 
`__host__` vs. `__device__`?


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

https://reviews.llvm.org/D56411



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

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D58218#1398096 , @rjmccall wrote:

> Well, you can always make a variable with a more directly-applicable name 
> than `capturedByInit` and update it as appropriate, like `locIsByrefHeader`.


Sounds good. I made it `const` too, to avoid inadvertent modification.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58218



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

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 186853.
jfb added a comment.

- Update with John's suggestion.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58218

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -45,14 +45,35 @@
 // PATTERN:   %captured1 = getelementptr inbounds 
%struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 
0, i32 4
 // PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to 
%struct.XYZ*), %struct.XYZ** %captured1, align 8
 // PATTERN:   %call = call %struct.XYZ* @create(
+using Block = void (^)();
+typedef struct XYZ {
+  Block block;
+} * xyz_t;
 void test_block_self_init() {
-  using Block = void (^)();
-  typedef struct XYZ {
-Block block;
-  } * xyz_t;
   extern xyz_t create(Block block);
   __block xyz_t captured = create(^() {
-(void)captured;
+used(captured);
+  });
+}
+
+// Capturing with escape after initialization is also an edge case.
+//
+// UNINIT-LABEL:  test_block_captures_self_after_init(
+// ZERO-LABEL:test_block_captures_self_after_init(
+// ZERO:  %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// ZERO:  %captured1 = getelementptr inbounds 
%struct.__block_byref_captured.1, %struct.__block_byref_captured.1* %captured, 
i32 0, i32 4
+// ZERO-NEXT: store %struct.XYZ* null, %struct.XYZ** %captured1, align 8
+// ZERO:  %call = call %struct.XYZ* @create(
+// PATTERN-LABEL: test_block_captures_self_after_init(
+// PATTERN:   %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// PATTERN:   %captured1 = getelementptr inbounds 
%struct.__block_byref_captured.1, %struct.__block_byref_captured.1* %captured, 
i32 0, i32 4
+// PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to 
%struct.XYZ*), %struct.XYZ** %captured1, align 8
+// PATTERN:   %call = call %struct.XYZ* @create(
+void test_block_captures_self_after_init() {
+  extern xyz_t create(Block block);
+  __block xyz_t captured;
+  captured = create(^() {
+used(captured);
   });
 }
 
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1620,8 +1620,9 @@
   bool capturedByInit =
   Init && emission.IsEscapingByRef && isCapturedBy(D, Init);
 
-  Address Loc =
-  capturedByInit ? emission.Addr : emission.getObjectAddress(*this);
+  bool locIsByrefHeader = capturedByInit;
+  const Address Loc =
+  locIsByrefHeader ? emission.Addr : emission.getObjectAddress(*this);
 
   // Note: constexpr already initializes everything correctly.
   LangOptions::TrivialAutoVarInitKind trivialAutoVarInit =
@@ -1637,7 +1638,7 @@
   return;
 
 // Only initialize a __block's storage: we always initialize the header.
-if (emission.IsEscapingByRef)
+if (emission.IsEscapingByRef && !locIsByrefHeader)
   Loc = emitBlockByrefAddress(Loc, &D, /*follow=*/false);
 
 CharUnits Size = getContext().getTypeSizeInChars(type);
@@ -1744,10 +1745,9 @@
   }
 
   llvm::Type *BP = CGM.Int8Ty->getPointerTo(Loc.getAddressSpace());
-  if (Loc.getType() != BP)
-Loc = Builder.CreateBitCast(Loc, BP);
-
-  emitStoresForConstant(CGM, D, Loc, isVolatile, Builder, constant);
+  emitStoresForConstant(
+  CGM, D, (Loc.getType() == BP) ? Loc : Builder.CreateBitCast(Loc, BP),
+  isVolatile, Builder, constant);
 }
 
 /// Emit an expression as an initializer for an object (variable, field, etc.)


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -45,14 +45,35 @@
 // PATTERN:   %captured1 = getelementptr inbounds %struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 0, i32 4
 // PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to %struct.XYZ*), %struct.XYZ** %captured1, align 8
 // PATTERN:   %call = call %struct.XYZ* @create(
+using Block = void (^)();
+typedef struct XYZ {
+  Block block;
+} * xyz_t;
 void test_block_self_init() {
-  using Block = void (^)();
-  typedef struct XYZ {
-Block block;
-  } * xyz_t;
   extern xyz_t create(Block block);
   __block xyz_t captured = create(^() {
-(void)captured;
+used(captured);
+  });
+}
+
+// Capturing with escape after initialization is also an edge case.
+//
+// UNINIT-LABEL:  test_block_captures_self_after_init(
+// ZERO-LABEL:test_block_captures_self_after_init(
+// ZERO:  %block = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i8* }>, alig

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D56411#1398103 , @rjmccall wrote:

> In D56411#1398097 , @yaxunl wrote:
>
> > In D56411#1365878 , @yaxunl wrote:
> >
> > > In D56411#1365745 , @rjmccall 
> > > wrote:
> > >
> > > > In D56411#1365727 , @yaxunl 
> > > > wrote:
> > > >
> > > > > In D56411#1360010 , 
> > > > > @rjmccall wrote:
> > > > >
> > > > > > I think the diagnostic should come during instantiation when you 
> > > > > > find an evaluated use of a host function within a device function.
> > > > >
> > > > >
> > > > > It seems the body of function template is checked only during parsing 
> > > > > of the definition of the template itself. When a function
> > > > >  template is instantiated, the body of the instantiated function is 
> > > > > not checked again.
> > > >
> > > >
> > > > No, that's not correct.  However, it's checked somewhat differently, 
> > > > and it's possible that the existing diagnostic is not set up to fire 
> > > > along all common paths.  Try moving the diagnostic to 
> > > > `MarkFunctionReferenced`, and note that `OdrUse` will be `false` in all 
> > > > the unevaluated contexts.
> > >
> > >
> > > You are right. After I disable current diagnostic, I saw 
> > > PerformPendingInstantiations at the end of parsing the TU, where the AST 
> > > of the instantiated function is iterated and MarkFunctionReferenced is 
> > > called. I will try to fix my patch as suggested. Thanks.
> >
> >
> > I got one concern. If we want to do overload resolution of function type 
> > template argument based on host or device, we need to do that before 
> > template instantiation, right?
> >
> > e.g. we have two functions having the same name f and type, but one is 
> > `__host__` and the other is `__device__`, and we pass it as a template 
> > argument to a template function g. We want to choose `__device__ f` if g 
> > itself is `__device__` and `__host__ f` if g itself is `__host__`. If we 
> > want to do this we have to do the check before template instantiation, 
> > right?
>
>
> Yes, you would need to check that when resolving the overload to a single 
> declaration.  That would be separate from diagnosing uses.
>
> That said, does CUDA have a general rule resolving `__host__` vs. 
> `__device__` overloads based on context?  And does it allow overloading based 
> solely on `__host__` vs. `__device__`?


https://docs.nvidia.com/cuda/cuda-c-programming-guide/index.html#function-declaration-specifiers
 does not talk about that.

Experimenting with nvcc shows that two functions cannot differ only by 
host/device attr, otherwise it is treated as redefinition of one function.

So I withdraw my concern.


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

https://reviews.llvm.org/D56411



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

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

Ping for reviews please.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57986



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

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

Check, please, that adding this to the map clause does not crash the codegen. 
Would be good to ignore this construct in codegen for now, if it is used in the 
code.




Comment at: include/clang/AST/OpenMPClause.h:4193
 ArrayRef MapModifiersLoc,
+NestedNameSpecifierLoc MapperQualifierLoc,
+DeclarationNameInfo MapperIdInfo,

Also would be good to pack the parameters into the structure.


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

https://reviews.llvm.org/D58074



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

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

[Lingda Li via Phabricator via cfe-commits]

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

Hi Alexey,

Again thanks for your review! The codegen completely ignores any mapper related 
info for now, so it should not crash map clause codegen. It also passed the 
regression test, so map clause codegen should be fine.




Comment at: include/clang/AST/OpenMPClause.h:4193
 ArrayRef MapModifiersLoc,
+NestedNameSpecifierLoc MapperQualifierLoc,
+DeclarationNameInfo MapperIdInfo,

ABataev wrote:
> Also would be good to pack the parameters into the structure.
Among these parameters:
1. 2 of them are for modifiers. Since mapper can also be used in `to` and 
`from` clauses, it is not good to combine mapper info with them.
2. 2 of them are mapper info. I can combine `MapperQualifierLoc` and 
`MapperIdInfo` into a structure. It's not a big saving though (only reduce one 
parameter). Do you want me to do that?
3. 4 of them are number of vars... It seems not a good idea to combine them 
into one structure.
5. The rest are locations. They are kinda all over the place. It doesn't seem a 
good idea to combine them as well.


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

https://reviews.llvm.org/D58074



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

[PATCH] D58243: [OPENMP] Delay emission of the asm target-specific error messages.

[Alexey Bataev via Phabricator via cfe-commits]

ABataev created this revision.
ABataev added a reviewer: rjmccall.
Herald added subscribers: jdoerfert, guansong.
Herald added a project: clang.

Added the ability to emit target-specific builtin assembler error
messages only in case if the function is really is going to be emitted
for the device.


Repository:
  rC Clang

https://reviews.llvm.org/D58243

Files:
  include/clang/Sema/Sema.h
  lib/Sema/Sema.cpp
  lib/Sema/SemaStmtAsm.cpp
  test/OpenMP/nvptx_asm_delayed_diags.c

Index: test/OpenMP/nvptx_asm_delayed_diags.c
===
--- /dev/null
+++ test/OpenMP/nvptx_asm_delayed_diags.c
@@ -0,0 +1,118 @@
+// RUN: %clang_cc1 -fopenmp -x c -triple i386-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda -emit-llvm-bc %s -o %t-x86-host.bc
+// RUN: %clang_cc1 -verify -fopenmp -x c -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -fsyntax-only
+// RUN: %clang_cc1 -verify -DDIAGS -DIMMEDIATE -fopenmp -x c -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -fsyntax-only
+// RUN: %clang_cc1 -verify -DDIAGS -DDELAYED -fopenmp -x c -triple nvptx-unknown-unknown -fopenmp-targets=nvptx-nvidia-cuda %s -fopenmp-is-device -fopenmp-host-ir-file-path %t-x86-host.bc -fsyntax-only
+// REQUIRES: x86-registered-target
+// REQUIRES: nvptx-registered-target
+
+#ifndef DIAGS
+// expected-no-diagnostics
+#endif // DIAGS
+
+#ifdef IMMEDIATE
+#pragma omp declare target
+#endif //IMMEDIATE
+void t1(int r) {
+#ifdef DIAGS
+// expected-error@+4 {{invalid input constraint 'mx' in asm}}
+#endif // DIAGS
+  __asm__("PR3908 %[lf] %[xx] %[li] %[r]"
+  : [ r ] "+r"(r)
+  : [ lf ] "mx"(0), [ li ] "mr"(0), [ xx ] "x"((double)(0)));
+}
+
+unsigned t2(signed char input) {
+  unsigned output;
+#ifdef DIAGS
+// expected-error@+3 {{invalid output constraint '=a' in asm}}
+#endif // DIAGS
+  __asm__("xyz"
+  : "=a"(output)
+  : "0"(input));
+  return output;
+}
+
+double t3(double x) {
+  register long double result;
+#ifdef DIAGS
+// expected-error@+3 {{invalid output constraint '=t' in asm}}
+#endif // DIAGS
+  __asm __volatile("frndint"
+   : "=t"(result)
+   : "0"(x));
+  return result;
+}
+
+unsigned char t4(unsigned char a, unsigned char b) {
+  unsigned int la = a;
+  unsigned int lb = b;
+  unsigned int bigres;
+  unsigned char res;
+#ifdef DIAGS
+// expected-error@+3 {{invalid output constraint '=la' in asm}}
+#endif // DIAGS
+  __asm__("0:\n1:\n"
+  : [ bigres ] "=la"(bigres)
+  : [ la ] "0"(la), [ lb ] "c"(lb)
+  : "edx", "cc");
+  res = bigres;
+  return res;
+}
+
+void t5(void) {
+#ifdef DIAGS
+// expected-error@+6 {{unknown register name 'st' in asm}}
+#endif // DIAGS
+  __asm__ __volatile__(
+  "finit"
+  :
+  :
+  : "st", "st(1)", "st(2)", "st(3)",
+"st(4)", "st(5)", "st(6)", "st(7)",
+"fpsr", "fpcr");
+}
+
+typedef long long __m256i __attribute__((__vector_size__(32)));
+void t6(__m256i *p) {
+#ifdef DIAGS
+// expected-error@+3 {{unknown register name 'ymm0' in asm}}
+#endif // DIAGS
+  __asm__ volatile("vmovaps  %0, %%ymm0" ::"m"(*(__m256i *)p)
+   : "ymm0");
+}
+#ifdef IMMEDIATE
+#pragma omp end declare target
+#endif //IMMEDIATE
+
+int main() {
+#ifdef DELAYED
+#pragma omp target
+#endif // DELAYED
+  {
+#ifdef DELAYED
+// expected-note@+2 {{called by 'main'}}
+#endif // DELAYED
+t1(0);
+#ifdef DELAYED
+// expected-note@+2 {{called by 'main'}}
+#endif // DELAYED
+t2(0);
+#ifdef DELAYED
+// expected-note@+2 {{called by 'main'}}
+#endif // DELAYED
+t3(0);
+#ifdef DELAYED
+// expected-note@+2 {{called by 'main'}}
+#endif // DELAYED
+t4(0, 0);
+#ifdef DELAYED
+// expected-note@+2 {{called by 'main'}}
+#endif // DELAYED
+t5();
+#ifdef DELAYED
+// expected-note@+2 {{called by 'main'}}
+#endif // DELAYED
+t6(0);
+  }
+  return 0;
+}
Index: lib/Sema/SemaStmtAsm.cpp
===
--- lib/Sema/SemaStmtAsm.cpp
+++ lib/Sema/SemaStmtAsm.cpp
@@ -272,9 +272,9 @@
 
 TargetInfo::ConstraintInfo Info(Literal->getString(), OutputName);
 if (!Context.getTargetInfo().validateOutputConstraint(Info))
-  return StmtError(
-  Diag(Literal->getBeginLoc(), diag::err_asm_invalid_output_constraint)
-  << Info.getConstraintStr());
+  return StmtResult(targetDiag(Literal->getBeginLoc(),
+   diag::err_asm_invalid_output_constraint)
+<< Info.getConstraintStr());
 
 ExprResult ER = CheckPlaceholderExpr(Exprs[i]);
 if (ER.isInvalid())
@@ -327,11 +327,10 @@
 }
 
 unsigned Size = Context.getTypeSize(OutputExpr->getType());
-if (!Context.getTargetInfo().validateOutputSize(Literal->getString(),
- 

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added a comment.

In D58074#1398187 , @lildmh wrote:

> Hi Alexey,
>
> Again thanks for your review! The codegen completely ignores any mapper 
> related info for now, so it should not crash map clause codegen. It also 
> passed the regression test, so map clause codegen should be fine.


Hi Lingda, no problems!
It would be good to have at least one codegen test with the mapper construct to 
prove that it does not crash the codegen. We don't have any codegen test for 
this new construct.




Comment at: include/clang/AST/OpenMPClause.h:4193
 ArrayRef MapModifiersLoc,
+NestedNameSpecifierLoc MapperQualifierLoc,
+DeclarationNameInfo MapperIdInfo,

lildmh wrote:
> ABataev wrote:
> > Also would be good to pack the parameters into the structure.
> Among these parameters:
> 1. 2 of them are for modifiers. Since mapper can also be used in `to` and 
> `from` clauses, it is not good to combine mapper info with them.
> 2. 2 of them are mapper info. I can combine `MapperQualifierLoc` and 
> `MapperIdInfo` into a structure. It's not a big saving though (only reduce 
> one parameter). Do you want me to do that?
> 3. 4 of them are number of vars... It seems not a good idea to combine them 
> into one structure.
> 5. The rest are locations. They are kinda all over the place. It doesn't seem 
> a good idea to combine them as well.
1. The same structure must be used for `to` and `from` clauses, not a problem.
2-4. you can combine it with the number of vars and locations into one 
structure, no?


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

https://reviews.llvm.org/D58074



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

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

[Lingda Li via Phabricator via cfe-commits]

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

Sure I'll add a codegen test with mapper. Thanks!




Comment at: include/clang/AST/OpenMPClause.h:4193
 ArrayRef MapModifiersLoc,
+NestedNameSpecifierLoc MapperQualifierLoc,
+DeclarationNameInfo MapperIdInfo,

ABataev wrote:
> lildmh wrote:
> > ABataev wrote:
> > > Also would be good to pack the parameters into the structure.
> > Among these parameters:
> > 1. 2 of them are for modifiers. Since mapper can also be used in `to` and 
> > `from` clauses, it is not good to combine mapper info with them.
> > 2. 2 of them are mapper info. I can combine `MapperQualifierLoc` and 
> > `MapperIdInfo` into a structure. It's not a big saving though (only reduce 
> > one parameter). Do you want me to do that?
> > 3. 4 of them are number of vars... It seems not a good idea to combine them 
> > into one structure.
> > 5. The rest are locations. They are kinda all over the place. It doesn't 
> > seem a good idea to combine them as well.
> 1. The same structure must be used for `to` and `from` clauses, not a problem.
> 2-4. you can combine it with the number of vars and locations into one 
> structure, no?
None of these locations are for mapper, so I think it's not good to combine 
them with mapper.
Those numbers (`NumVars`, `NumUniqueDeclarations`, `NumComponentLists`, 
`NumComponents`) are not for mapper either. `NumVars` can be viewed as 
partially for mapper, but its main purpose is for the number of list items in 
map clause. So logically, I think they should not be combined with mapper.
What do you think?


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

https://reviews.llvm.org/D58074



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

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

[Alexey Bataev via Phabricator via cfe-commits]

ABataev added inline comments.



Comment at: include/clang/AST/OpenMPClause.h:4193
 ArrayRef MapModifiersLoc,
+NestedNameSpecifierLoc MapperQualifierLoc,
+DeclarationNameInfo MapperIdInfo,

lildmh wrote:
> ABataev wrote:
> > lildmh wrote:
> > > ABataev wrote:
> > > > Also would be good to pack the parameters into the structure.
> > > Among these parameters:
> > > 1. 2 of them are for modifiers. Since mapper can also be used in `to` and 
> > > `from` clauses, it is not good to combine mapper info with them.
> > > 2. 2 of them are mapper info. I can combine `MapperQualifierLoc` and 
> > > `MapperIdInfo` into a structure. It's not a big saving though (only 
> > > reduce one parameter). Do you want me to do that?
> > > 3. 4 of them are number of vars... It seems not a good idea to combine 
> > > them into one structure.
> > > 5. The rest are locations. They are kinda all over the place. It doesn't 
> > > seem a good idea to combine them as well.
> > 1. The same structure must be used for `to` and `from` clauses, not a 
> > problem.
> > 2-4. you can combine it with the number of vars and locations into one 
> > structure, no?
> None of these locations are for mapper, so I think it's not good to combine 
> them with mapper.
> Those numbers (`NumVars`, `NumUniqueDeclarations`, `NumComponentLists`, 
> `NumComponents`) are not for mapper either. `NumVars` can be viewed as 
> partially for mapper, but its main purpose is for the number of list items in 
> map clause. So logically, I think they should not be combined with mapper.
> What do you think?
It is not about mapper. We just have too many params in functions and need to 
reduce it somehow. I don't care about the logic of the parameters packing.


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

https://reviews.llvm.org/D58074



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

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D58218#1398124 , @jfb wrote:

> In D58218#1398096 , @rjmccall wrote:
>
> > Well, you can always make a variable with a more directly-applicable name 
> > than `capturedByInit` and update it as appropriate, like `locIsByrefHeader`.
>
>
> Sounds good. I made it `const` too, to avoid inadvertent modification.


This is actually the reverse of the sense I would expect it to have based on 
the name.  If you want these semantics, maybe `locIsObject`?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58218



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

[PATCH] D58145: [Sema] Fix a bogus -Wconversion warning

[Nick Desaulniers via Phabricator via cfe-commits]

nickdesaulniers added a comment.

> Fix this by just directly calling DiagnoseImpCast, since we don't really need 
> anything DiagnoseFloatingImpCast does anyways.

Good catch; thanks for fixing and improving the tests.


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

https://reviews.llvm.org/D58145



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

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

[Vedant Kumar via Phabricator via cfe-commits]

vsk added subscribers: dblaikie, vsk.
vsk added a comment.

I think this could roughly double the memory utilization of the writer, which 
might be problematic because the number of records to write can be high. 
(@dblaikie did some work on reducing memory usage in this area, he might have 
hard data about this.)

As the write should only occur once, maybe the better tradeoff would be to sort?


Repository:
  rC Clang

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

https://reviews.llvm.org/D57986



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

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

[Artem Belevich via Phabricator via cfe-commits]

tra added a comment.

>> That said, does CUDA have a general rule resolving `__host__` vs. 
>> `__device__` overloads based on context?  And does it allow overloading 
>> based solely on `__host__` vs. `__device__`?

NVCC does not. Clang does. See https://goo.gl/EXnymm for the details.

AFAICT, NVIDIA is starting to consider adopting Clang's approach:
http://lists.llvm.org/pipermail/cfe-dev/2018-November/060070.html (original 
message from Bryce apparently didn't make it to the cfe-dev archive)


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

https://reviews.llvm.org/D56411



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

[PATCH] D58236: Make address space conversions a bit stricter.

[Bevin Hansson via Phabricator via cfe-commits]

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



Comment at: test/SemaOpenCL/address-spaces.cl:89
+  __local int * __global * __private * lll;
+  lll = gg; // expected-warning {{incompatible pointer types assigning to 
'__local int *__global **' from '__global int **'}}
+}

This doesn't seem entirely correct still, but I'm not sure what to do about it.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58236



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

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D56411#1398291 , @tra wrote:

> >> That said, does CUDA have a general rule resolving `__host__` vs. 
> >> `__device__` overloads based on context?  And does it allow overloading 
> >> based solely on `__host__` vs. `__device__`?
>
> NVCC does not. Clang does. See https://goo.gl/EXnymm for the details.
>
> AFAICT, NVIDIA is starting to consider adopting Clang's approach:
>  http://lists.llvm.org/pipermail/cfe-dev/2018-November/060070.html (original 
> message from Bryce apparently didn't make it to the cfe-dev archive)


Okay.  Probably the template-argument rule ought to be the same as the 
address-of-function rule, which I assume means that there's a final pass that 
resolves ambiguities in favor of functions that can be used from the current 
context, to the extent that that's meaningful.  It's hard to tell because that 
document does not appear to include a formal specification.


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

https://reviews.llvm.org/D56411



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

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

In D56411#1398328 , @rjmccall wrote:

> In D56411#1398291 , @tra wrote:
>
> > >> That said, does CUDA have a general rule resolving `__host__` vs. 
> > >> `__device__` overloads based on context?  And does it allow overloading 
> > >> based solely on `__host__` vs. `__device__`?
> >
> > NVCC does not. Clang does. See https://goo.gl/EXnymm for the details.
> >
> > AFAICT, NVIDIA is starting to consider adopting Clang's approach:
> >  http://lists.llvm.org/pipermail/cfe-dev/2018-November/060070.html 
> > (original message from Bryce apparently didn't make it to the cfe-dev 
> > archive)
>
>
> Okay.  Probably the template-argument rule ought to be the same as the 
> address-of-function rule, which I assume means that there's a final pass that 
> resolves ambiguities in favor of functions that can be used from the current 
> context, to the extent that that's meaningful.  It's hard to tell because 
> that document does not appear to include a formal specification.


Regardless, that has no effect on this patch.


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

https://reviews.llvm.org/D56411



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

r354053 - [X86] Add clang support for X86 flag output parameters.

[Nirav Dave via cfe-commits]

Author: niravd
Date: Thu Feb 14 11:27:25 2019
New Revision: 354053

URL: http://llvm.org/viewvc/llvm-project?rev=354053&view=rev
Log:
[X86] Add clang support for X86 flag output parameters.

Summary:
Add frontend support and expected flags for X86 inline assembly flag
parameters.

Reviewers: craig.topper, rnk, echristo

Subscribers: eraman, nickdesaulniers, void, llvm-commits

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

Added:
cfe/trunk/test/CodeGen/inline-asm-x86-flag-output.c
cfe/trunk/test/Preprocessor/x86_asm_flag_output.c
Modified:
cfe/trunk/lib/Basic/Targets/X86.cpp
cfe/trunk/test/Preprocessor/predefined-win-macros.c

Modified: cfe/trunk/lib/Basic/Targets/X86.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Targets/X86.cpp?rev=354053&r1=354052&r2=354053&view=diff
==
--- cfe/trunk/lib/Basic/Targets/X86.cpp (original)
+++ cfe/trunk/lib/Basic/Targets/X86.cpp Thu Feb 14 11:27:25 2019
@@ -864,6 +864,9 @@ bool X86TargetInfo::handleTargetFeatures
 /// definitions for this particular subtarget.
 void X86TargetInfo::getTargetDefines(const LangOptions &Opts,
  MacroBuilder &Builder) const {
+  // Inline assembly supports X86 flag outputs. 
+  Builder.defineMacro("__GCC_ASM_FLAG_OUTPUTS__");
+
   std::string CodeModel = getTargetOpts().CodeModel;
   if (CodeModel == "default")
 CodeModel = "small";
@@ -1553,6 +1556,40 @@ bool X86TargetInfo::validateCpuIs(String
   .Default(false);
 }
 
+static unsigned matchAsmCCConstraint(const char *&Name) {
+  auto RV = llvm::StringSwitch(Name)
+.Case("@cca", 4)
+.Case("@ccae", 5)
+.Case("@ccb", 4)
+.Case("@ccbe", 5)
+.Case("@ccc", 4)
+.Case("@cce", 4)
+.Case("@ccz", 4)
+.Case("@ccg", 4)
+.Case("@ccge", 5)
+.Case("@ccl", 4)
+.Case("@ccle", 5)
+.Case("@ccna", 5)
+.Case("@ccnae", 6)
+.Case("@ccnb", 5)
+.Case("@ccnbe", 6)
+.Case("@ccnc", 5)
+.Case("@ccne", 5)
+.Case("@ccnz", 5)
+.Case("@ccng", 5)
+.Case("@ccnge", 6)
+.Case("@ccnl", 5)
+.Case("@ccnle", 6)
+.Case("@ccno", 5)
+.Case("@ccnp", 5)
+.Case("@ccns", 5)
+.Case("@cco", 4)
+.Case("@ccp", 4)
+.Case("@ccs", 4)
+.Default(0);
+  return RV;
+}
+
 bool X86TargetInfo::validateAsmConstraint(
 const char *&Name, TargetInfo::ConstraintInfo &Info) const {
   switch (*Name) {
@@ -1635,6 +1672,14 @@ bool X86TargetInfo::validateAsmConstrain
   case 'C': // SSE floating point constant.
   case 'G': // x87 floating point constant.
 return true;
+  case '@':
+// CC condition changes.
+if (auto Len = matchAsmCCConstraint(Name)) {
+  Name += Len - 1;
+  Info.setAllowsRegister();
+  return true;
+}
+return false;
   }
 }
 
@@ -1706,6 +1751,13 @@ bool X86TargetInfo::validateOperandSize(
 
 std::string X86TargetInfo::convertConstraint(const char *&Constraint) const {
   switch (*Constraint) {
+  case '@':
+if (auto Len = matchAsmCCConstraint(Constraint)) {
+  std::string Converted = "{" + std::string(Constraint, Len) + "}";
+  Constraint += Len - 1;
+  return Converted;
+}
+return std::string(1, *Constraint);
   case 'a':
 return std::string("{ax}");
   case 'b':

Added: cfe/trunk/test/CodeGen/inline-asm-x86-flag-output.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CodeGen/inline-asm-x86-flag-output.c?rev=354053&view=auto
==
--- cfe/trunk/test/CodeGen/inline-asm-x86-flag-output.c (added)
+++ cfe/trunk/test/CodeGen/inline-asm-x86-flag-output.c Thu Feb 14 11:27:25 2019
@@ -0,0 +1,365 @@
+// RUN: %clang_cc1 -O2 -emit-llvm %s -o - -triple x86_64-unknown-linux-gnu | 
FileCheck %s
+
+int test_cca(long nr, volatile long *addr) {
+  //CHECK-LABEL: @test_cca
+  //CHECK: = tail call i32 asm "cmp $2,$1", 
"={@cca},=*m,r,~{cc},~{dirflag},~{fpsr},~{flags}"(i64* %addr, i64 %nr)
+  int x;
+  asm("cmp %2,%1"
+  : "=@cca"(x), "=m"(*(volatile long *)(addr))
+  : "r"(nr)
+  : "cc");
+  if (x)
+return 0;
+  return 1;
+}
+
+int test_ccae(long nr, volatile long *addr) {
+  //CHECK-LABEL: @test_ccae
+  //CHECK: = tail call i32 asm "cmp $2,$1", 
"={@ccae},=*m,r,~{cc},~{dirflag},~{fpsr},~{flags}"(i64* %addr, i64 %nr)
+  int x;
+  asm("cmp %2,%1"
+  : "=@ccae"(x), "=m"(*(volatile long *)(addr))
+  : "r"(nr)
+  : "cc");
+  if (x)
+return 0;
+  return 1;
+}
+
+int test_ccb(long nr, volatile long *addr) {
+  //CHECK-LABEL: @test_ccb
+  //CHECK: = tail call i32 asm "cmp $2,$1

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D56411#1398291 , @tra wrote:

> >> That said, does CUDA have a general rule resolving `__host__` vs. 
> >> `__device__` overloads based on context?  And does it allow overloading 
> >> based solely on `__host__` vs. `__device__`?
>
> NVCC does not. Clang does. See https://goo.gl/EXnymm for the details.
>
> AFAICT, NVIDIA is starting to consider adopting Clang's approach:
>  http://lists.llvm.org/pipermail/cfe-dev/2018-November/060070.html (original 
> message from Bryce apparently didn't make it to the cfe-dev archive)


So my concern about checking host/device compatibility in template 
instantiation is still valid.

I verified the following code is valid with clang

  #define __device__ __attribute__((device))
  
  __device__ void f();
  
  void f();
  
  __device__ void g() {
f();
  }
  
  template __device__ void t() {
F();
  }
  
  __device__ void h() {
t();
  }

To be able to resolve function type template argument based on host/device 
attribute, we need to do the check before template instantiation.


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

https://reviews.llvm.org/D56411



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

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!




Comment at: lib/Parse/ParseStmt.cpp:1090-1091
+R = handleExprStmt(Res, SubStmtCtx);
+if (R.isUsable())
+  R = Actions.ProcessStmtAttributes(R.get(), attrs, attrs.Range);
   }

rsmith wrote:
> aaron.ballman wrote:
> > Should this be done as part of `handleExprStmt()`?
> In principle that might make sense, but it'd require passing the attributes 
> through a few extra layers, and also making sure the attributes aren't 
> handled twice along the more-common statement parsing codepaths. I'm inclined 
> to defer that until we have a need to have the attributes in hand when 
> processing a full-expression (and I suspect that time may never come).
Yeah, I think it's worth delaying that change then. Thank you for the 
explanation!


Repository:
  rC Clang

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

https://reviews.llvm.org/D57984



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

[PATCH] D17444: [MSVC] Recognize "static_assert" keyword in C mode

[Aaron Ballman via Phabricator via cfe-commits]

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

LGTM!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D17444



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

[PATCH] D57986: [ProfileData] Remove non-determinism: Change DenseMap to MapVector

[David Blaikie via Phabricator via cfe-commits]

dblaikie added a comment.

In D57986#1398271 , @vsk wrote:

> I think this could roughly double the memory utilization of the writer, which 
> might be problematic because the number of records to write can be high. 
> (@dblaikie did some work on reducing memory usage in this area, he might have 
> hard data about this.)
>
> As the write should only occur once, maybe the better tradeoff would be to 
> sort?


Yeah, unless I'm missing something (quite possibly) this seems like the sort of 
place where "do a bunch of processing, then sort, then iterate" is a valid 
strategy.

@mgrang - does it look like any use case is emitting more than once & modifying 
in between? I would find that pretty surprising. I think it's just "build build 
build, stop building, emit" and adding a sort-before-emit would be OK there.


Repository:
  rC Clang

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

https://reviews.llvm.org/D57986



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

r354056 - [CodeGenObjC] Emit [[X alloc] init] as objc_alloc_init(X) when available

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Thu Feb 14 11:58:37 2019
New Revision: 354056

URL: http://llvm.org/viewvc/llvm-project?rev=354056&view=rev
Log:
[CodeGenObjC] Emit [[X alloc] init] as objc_alloc_init(X) when available

This provides a code size win on the caller side, since the init
message send is done in the runtime function.

rdar://44987038

Differential revision: https://reviews.llvm.org/D57936

Added:
cfe/trunk/test/CodeGenObjC/objc-alloc-init.m
Modified:
cfe/trunk/include/clang/Basic/ObjCRuntime.h
cfe/trunk/lib/CodeGen/CGObjC.cpp
cfe/trunk/lib/CodeGen/CodeGenFunction.h
cfe/trunk/lib/CodeGen/CodeGenModule.h

Modified: cfe/trunk/include/clang/Basic/ObjCRuntime.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/ObjCRuntime.h?rev=354056&r1=354055&r2=354056&view=diff
==
--- cfe/trunk/include/clang/Basic/ObjCRuntime.h (original)
+++ cfe/trunk/include/clang/Basic/ObjCRuntime.h Thu Feb 14 11:58:37 2019
@@ -246,6 +246,22 @@ public:
 llvm_unreachable("bad kind");
   }
 
+  /// Does this runtime provide the objc_alloc_init entrypoint? This can apply
+  /// the same optimization as objc_alloc, but also sends an -init message,
+  /// reducing code size on the caller.
+  bool shouldUseRuntimeFunctionForCombinedAllocInit() const {
+switch (getKind()) {
+case MacOSX:
+  return getVersion() >= VersionTuple(10, 14, 4);
+case iOS:
+  return getVersion() >= VersionTuple(12, 2);
+case WatchOS:
+  return getVersion() >= VersionTuple(5, 2);
+default:
+  return false;
+}
+  }
+
   /// Does this runtime supports optimized setter entrypoints?
   bool hasOptimizedSetter() const {
 switch (getKind()) {

Modified: cfe/trunk/lib/CodeGen/CGObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/CodeGen/CGObjC.cpp?rev=354056&r1=354055&r2=354056&view=diff
==
--- cfe/trunk/lib/CodeGen/CGObjC.cpp (original)
+++ cfe/trunk/lib/CodeGen/CGObjC.cpp Thu Feb 14 11:58:37 2019
@@ -422,6 +422,40 @@ tryGenerateSpecializedMessageSend(CodeGe
   return None;
 }
 
+/// Instead of '[[MyClass alloc] init]', try to generate
+/// 'objc_alloc_init(MyClass)'. This provides a code size improvement on the
+/// caller side, as well as the optimized objc_alloc.
+static Optional
+tryEmitSpecializedAllocInit(CodeGenFunction &CGF, const ObjCMessageExpr *OME) {
+  auto &Runtime = CGF.getLangOpts().ObjCRuntime;
+  if (!Runtime.shouldUseRuntimeFunctionForCombinedAllocInit())
+return None;
+
+  // Match the exact pattern '[[MyClass alloc] init]'.
+  Selector Sel = OME->getSelector();
+  if (!OME->isInstanceMessage() || !OME->getType()->isObjCObjectPointerType() 
||
+  !Sel.isUnarySelector() || Sel.getNameForSlot(0) != "init")
+return None;
+
+  // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]'.
+  auto *SubOME =
+  dyn_cast(OME->getInstanceReceiver()->IgnoreParens());
+  if (!SubOME)
+return None;
+  Selector SubSel = SubOME->getSelector();
+  if (SubOME->getReceiverKind() != ObjCMessageExpr::Class ||
+  !SubOME->getType()->isObjCObjectPointerType() ||
+  !SubSel.isUnarySelector() || SubSel.getNameForSlot(0) != "alloc")
+return None;
+
+  QualType ReceiverType = SubOME->getClassReceiver();
+  const ObjCObjectType *ObjTy = ReceiverType->getAs();
+  const ObjCInterfaceDecl *ID = ObjTy->getInterface();
+  assert(ID && "null interface should be impossible here");
+  llvm::Value *Receiver = CGF.CGM.getObjCRuntime().GetClass(CGF, ID);
+  return CGF.EmitObjCAllocInit(Receiver, CGF.ConvertType(OME->getType()));
+}
+
 RValue CodeGenFunction::EmitObjCMessageExpr(const ObjCMessageExpr *E,
 ReturnValueSlot Return) {
   // Only the lookup mechanism and first two arguments of the method
@@ -443,6 +477,9 @@ RValue CodeGenFunction::EmitObjCMessageE
 }
   }
 
+  if (Optional Val = tryEmitSpecializedAllocInit(*this, E))
+return AdjustObjCObjectType(*this, E->getType(), RValue::get(*Val));
+
   // We don't retain the receiver in delegate init calls, and this is
   // safe because the receiver value is always loaded from 'self',
   // which we zero out.  We don't want to Block_copy block receivers,
@@ -2503,6 +2540,13 @@ llvm::Value *CodeGenFunction::EmitObjCAl
 "objc_allocWithZone", /*MayThrow=*/true);
 }
 
+llvm::Value *CodeGenFunction::EmitObjCAllocInit(llvm::Value *value,
+llvm::Type *resultType) {
+  return emitObjCValueOperation(*this, value, resultType,
+CGM.getObjCEntrypoints().objc_alloc_init,
+"objc_alloc_init", /*MayThrow=*/true);
+}
+
 /// Produce the code to do a primitive release.
 /// [tmp drain];
 void CodeGenFunction::EmitObjCMRRAutoreleasePoolPop(llvm::Value *Arg) {

Modified: cfe/trun

[PATCH] D57936: [CodeGenObjC] When available, emit a direct call to objc_alloc_init(cls) instead of [objc_alloc(cls) init]

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rC354056: [CodeGenObjC] Emit [[X alloc] init] as 
objc_alloc_init(X) when available (authored by epilk, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57936?vs=186784&id=186894#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57936

Files:
  include/clang/Basic/ObjCRuntime.h
  lib/CodeGen/CGObjC.cpp
  lib/CodeGen/CodeGenFunction.h
  lib/CodeGen/CodeGenModule.h
  test/CodeGenObjC/objc-alloc-init.m

Index: lib/CodeGen/CodeGenModule.h
===
--- lib/CodeGen/CodeGenModule.h
+++ lib/CodeGen/CodeGenModule.h
@@ -124,6 +124,9 @@
   /// void objc_allocWithZone(id);
   llvm::FunctionCallee objc_allocWithZone;
 
+  /// void objc_alloc_init(id);
+  llvm::FunctionCallee objc_alloc_init;
+
   /// void objc_autoreleasePoolPop(void*);
   llvm::FunctionCallee objc_autoreleasePoolPop;
 
Index: lib/CodeGen/CGObjC.cpp
===
--- lib/CodeGen/CGObjC.cpp
+++ lib/CodeGen/CGObjC.cpp
@@ -422,6 +422,40 @@
   return None;
 }
 
+/// Instead of '[[MyClass alloc] init]', try to generate
+/// 'objc_alloc_init(MyClass)'. This provides a code size improvement on the
+/// caller side, as well as the optimized objc_alloc.
+static Optional
+tryEmitSpecializedAllocInit(CodeGenFunction &CGF, const ObjCMessageExpr *OME) {
+  auto &Runtime = CGF.getLangOpts().ObjCRuntime;
+  if (!Runtime.shouldUseRuntimeFunctionForCombinedAllocInit())
+return None;
+
+  // Match the exact pattern '[[MyClass alloc] init]'.
+  Selector Sel = OME->getSelector();
+  if (!OME->isInstanceMessage() || !OME->getType()->isObjCObjectPointerType() ||
+  !Sel.isUnarySelector() || Sel.getNameForSlot(0) != "init")
+return None;
+
+  // Okay, this is '[receiver init]', check if 'receiver' is '[cls alloc]'.
+  auto *SubOME =
+  dyn_cast(OME->getInstanceReceiver()->IgnoreParens());
+  if (!SubOME)
+return None;
+  Selector SubSel = SubOME->getSelector();
+  if (SubOME->getReceiverKind() != ObjCMessageExpr::Class ||
+  !SubOME->getType()->isObjCObjectPointerType() ||
+  !SubSel.isUnarySelector() || SubSel.getNameForSlot(0) != "alloc")
+return None;
+
+  QualType ReceiverType = SubOME->getClassReceiver();
+  const ObjCObjectType *ObjTy = ReceiverType->getAs();
+  const ObjCInterfaceDecl *ID = ObjTy->getInterface();
+  assert(ID && "null interface should be impossible here");
+  llvm::Value *Receiver = CGF.CGM.getObjCRuntime().GetClass(CGF, ID);
+  return CGF.EmitObjCAllocInit(Receiver, CGF.ConvertType(OME->getType()));
+}
+
 RValue CodeGenFunction::EmitObjCMessageExpr(const ObjCMessageExpr *E,
 ReturnValueSlot Return) {
   // Only the lookup mechanism and first two arguments of the method
@@ -443,6 +477,9 @@
 }
   }
 
+  if (Optional Val = tryEmitSpecializedAllocInit(*this, E))
+return AdjustObjCObjectType(*this, E->getType(), RValue::get(*Val));
+
   // We don't retain the receiver in delegate init calls, and this is
   // safe because the receiver value is always loaded from 'self',
   // which we zero out.  We don't want to Block_copy block receivers,
@@ -2503,6 +2540,13 @@
 "objc_allocWithZone", /*MayThrow=*/true);
 }
 
+llvm::Value *CodeGenFunction::EmitObjCAllocInit(llvm::Value *value,
+llvm::Type *resultType) {
+  return emitObjCValueOperation(*this, value, resultType,
+CGM.getObjCEntrypoints().objc_alloc_init,
+"objc_alloc_init", /*MayThrow=*/true);
+}
+
 /// Produce the code to do a primitive release.
 /// [tmp drain];
 void CodeGenFunction::EmitObjCMRRAutoreleasePoolPop(llvm::Value *Arg) {
Index: lib/CodeGen/CodeGenFunction.h
===
--- lib/CodeGen/CodeGenFunction.h
+++ lib/CodeGen/CodeGenFunction.h
@@ -3831,6 +3831,8 @@
  llvm::Type *returnType);
   llvm::Value *EmitObjCAllocWithZone(llvm::Value *value,
  llvm::Type *returnType);
+  llvm::Value *EmitObjCAllocInit(llvm::Value *value, llvm::Type *resultType);
+
   llvm::Value *EmitObjCThrowOperand(const Expr *expr);
   llvm::Value *EmitObjCConsumeObject(QualType T, llvm::Value *Ptr);
   llvm::Value *EmitObjCExtendObjectLifetime(QualType T, llvm::Value *Ptr);
Index: include/clang/Basic/ObjCRuntime.h
===
--- include/clang/Basic/ObjCRuntime.h
+++ include/clang/Basic/ObjCRuntime.h
@@ -246,6 +246,22 @@
 llvm_unreachable("bad kind");
   }
 
+  /// Does this runtime provide the objc_alloc_init entrypoint? This can apply
+  /// the same optimization as objc_alloc, but also sends an -init message,
+  /// reducing cod

[PATCH] D58157: Stop enabling clang-tools-extra automatically when clang is in LLVM_ENABLE_PROJECTS

[Phabricator via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354057: Stop enabling clang-tools-extra automatically when 
clang is in… (authored by nico, committed by ).
Herald added a project: LLVM.

Changed prior to commit:
  https://reviews.llvm.org/D58157?vs=186559&id=186895#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58157

Files:
  llvm/trunk/CMakeLists.txt
  llvm/trunk/docs/GettingStarted.rst


Index: llvm/trunk/CMakeLists.txt
===
--- llvm/trunk/CMakeLists.txt
+++ llvm/trunk/CMakeLists.txt
@@ -104,7 +104,7 @@
 # LLVM_EXTERNAL_${project}_SOURCE_DIR using LLVM_ALL_PROJECTS
 # This allows an easy way of setting up a build directory for llvm and another
 # one for llvm+clang+... using the same sources.
-set(LLVM_ALL_PROJECTS 
"clang;compiler-rt;debuginfo-tests;libclc;libcxx;libcxxabi;libunwind;lld;lldb;llgo;llvm;openmp;parallel-libs;polly;pstl")
+set(LLVM_ALL_PROJECTS 
"clang;clang-tools-extra;compiler-rt;debuginfo-tests;libclc;libcxx;libcxxabi;libunwind;lld;lldb;llgo;llvm;openmp;parallel-libs;polly;pstl")
 set(LLVM_ENABLE_PROJECTS "" CACHE STRING
"Semicolon-separated list of projects to build (${LLVM_ALL_PROJECTS}), 
or \"all\".")
 if( LLVM_ENABLE_PROJECTS STREQUAL "all" )
@@ -141,12 +141,6 @@
 message(FATAL_ERROR "LLVM_ENABLE_PROJECTS requests ${proj} but 
directory not found: ${PROJ_DIR}")
   endif()
   set(LLVM_EXTERNAL_${upper_proj}_SOURCE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
-  # There is a widely spread opinion that clang-tools-extra should be 
merged
-  # into clang. The following simulates it by always enabling 
clang-tools-extra
-  # when enabling clang.
-  if (proj STREQUAL "clang")
-set(LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR 
"${CMAKE_CURRENT_SOURCE_DIR}/../clang-tools-extra")
-  endif()
 else()
   message(STATUS "${proj} project is disabled")
   set(SHOULD_ENABLE_PROJECT FALSE)
Index: llvm/trunk/docs/GettingStarted.rst
===
--- llvm/trunk/docs/GettingStarted.rst
+++ llvm/trunk/docs/GettingStarted.rst
@@ -64,8 +64,8 @@
 
  * ``-DLLVM_ENABLE_PROJECTS='...'`` --- semicolon-separated list of the 
LLVM
subprojects you'd like to additionally build. Can include any of: clang,
-   libcxx, libcxxabi, libunwind, lldb, compiler-rt, lld, polly, or
-   debuginfo-tests.
+   clang-tools-extra, libcxx, libcxxabi, libunwind, lldb, compiler-rt, lld,
+   polly, or debuginfo-tests.
 
For example, to build LLVM, Clang, libcxx, and libcxxabi, use
``-DLLVM_ENABLE_PROJECTS="clang;libcxx;libcxxabi"``.


Index: llvm/trunk/CMakeLists.txt
===
--- llvm/trunk/CMakeLists.txt
+++ llvm/trunk/CMakeLists.txt
@@ -104,7 +104,7 @@
 # LLVM_EXTERNAL_${project}_SOURCE_DIR using LLVM_ALL_PROJECTS
 # This allows an easy way of setting up a build directory for llvm and another
 # one for llvm+clang+... using the same sources.
-set(LLVM_ALL_PROJECTS "clang;compiler-rt;debuginfo-tests;libclc;libcxx;libcxxabi;libunwind;lld;lldb;llgo;llvm;openmp;parallel-libs;polly;pstl")
+set(LLVM_ALL_PROJECTS "clang;clang-tools-extra;compiler-rt;debuginfo-tests;libclc;libcxx;libcxxabi;libunwind;lld;lldb;llgo;llvm;openmp;parallel-libs;polly;pstl")
 set(LLVM_ENABLE_PROJECTS "" CACHE STRING
 	"Semicolon-separated list of projects to build (${LLVM_ALL_PROJECTS}), or \"all\".")
 if( LLVM_ENABLE_PROJECTS STREQUAL "all" )
@@ -141,12 +141,6 @@
 message(FATAL_ERROR "LLVM_ENABLE_PROJECTS requests ${proj} but directory not found: ${PROJ_DIR}")
   endif()
   set(LLVM_EXTERNAL_${upper_proj}_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../${proj}")
-  # There is a widely spread opinion that clang-tools-extra should be merged
-  # into clang. The following simulates it by always enabling clang-tools-extra
-  # when enabling clang.
-  if (proj STREQUAL "clang")
-set(LLVM_EXTERNAL_CLANG_TOOLS_EXTRA_SOURCE_DIR "${CMAKE_CURRENT_SOURCE_DIR}/../clang-tools-extra")
-  endif()
 else()
   message(STATUS "${proj} project is disabled")
   set(SHOULD_ENABLE_PROJECT FALSE)
Index: llvm/trunk/docs/GettingStarted.rst
===
--- llvm/trunk/docs/GettingStarted.rst
+++ llvm/trunk/docs/GettingStarted.rst
@@ -64,8 +64,8 @@
 
  * ``-DLLVM_ENABLE_PROJECTS='...'`` --- semicolon-separated list of the LLVM
subprojects you'd like to additionally build. Can include any of: clang,
-   libcxx, libcxxabi, libunwind, lldb, compiler-rt, lld, polly, or
-   debuginfo-tests.
+   clang-tools-extra, libcxx, libcxxabi, libunwind, lldb, compiler-rt, lld,
+   polly, or debuginfo-tests.
 
For example, to build LLVM, Clang, libcxx, and libcxxabi, use
 

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Basic/FileManager.cpp:271
+  } else if (!openFile) {
+// Since we didn't return early after getStatValue() call the file exists.
+fillRealPathName(&UFE, InterndFileName);

I don't really understand what this comment is trying to say. Can you rephrase 
it?



Comment at: unittests/Basic/FileManagerTest.cpp:349
 
+// rdar://problem/47536127
+TEST_F(FileManagerTest, getFileDontOpenRealPath) {

Please leave this comment out


Repository:
  rC Clang

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

https://reviews.llvm.org/D58213



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

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

[Jan Korous via Phabricator via cfe-commits]

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

Thanks for taking a look!




Comment at: lib/Basic/FileManager.cpp:214
 
 return nullptr;
   }

If we can't stat the file we return here.



Comment at: lib/Basic/FileManager.cpp:271
+  } else if (!openFile) {
+// Since we didn't return early after getStatValue() call the file exists.
+fillRealPathName(&UFE, InterndFileName);

arphaman wrote:
> I don't really understand what this comment is trying to say. Can you 
> rephrase it?
Please see the above comment (line 214).

What I meant is that it's guaranteed that the file exists as we would have 
returned otherwise. Does that make sense?

Any suggestion how to rephrase the comment? (Or shall I just remove it?)


Repository:
  rC Clang

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

https://reviews.llvm.org/D58213



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

In D50488#1397721 , @xazax.hun wrote:

> In D50488#1395483 , @mgrang wrote:
>
> > Reviving this now that I have some cycles to work on this.
> >
> > So I tried running this on csa-testbench projects but I didn't have much 
> > success. I always run into a bunch of build/env related errors:
> >
> >   python run_experiments.py --config myconfig.json
> >  
> >   15:05:20 [libcxx] Checking out project... 
> >   [ERROR] Unknown option: json
> >  
> >   15:05:22 [libcxx] LOC: ?.
> >   15:05:22 [libcxx] Generating build log... 
> >   15:05:22 [libcxx_master] Analyzing project... 
> >   [ERROR] Traceback (most recent call last):
> > File 
> > "/local/mnt/workspace/mgrang/comm_analyzer/CodeChecker/cc_bin/CodeChecker.py",
> >  line 20, in 
> >   from shared.ttypes import RequestFailed
> >   ImportError: No module named shared.ttypes
> >
> >
> >
>
>
> Hi!
>
> Sorry for the late response. Does CodeChecker work for you (when not using 
> the testbench)?
>  I think one of the most common reason for such errors is when we do not 
> source the virtualenv of CodeChecker so the dependencies are not available.


I followed the instructions at https://github.com/Ericsson/codechecker and 
sourced the venv and here's what I see now:

  12:46:30 [libcxx] Checking out project... 
  [ERROR] Unknown option: json
  
  
  12:46:32 [libcxx] LOC: ?.
  12:46:32 [libcxx] Generating build log... 
  12:46:32 [libcxx_master] Analyzing project... 
  [ERROR] [ERROR 2019-02-14 12:46] - Connection failed.
  [ERROR 2019-02-14 12:46] - Connection refused
  [ERROR 2019-02-14 12:46] - [Errno 111] Connection refused
  [ERROR 2019-02-14 12:46] - Check if your CodeChecker server is running.
  
  12:46:34 [libcxx_master] Done. Storing results...
  12:46:35 [libcxx_master] Results stored.
  Traceback (most recent call last):
File "run_experiments.py", line 482, in 
  main()
File "run_experiments.py", line 470, in main
  post_process_project(project, source_dir, config, printer)
File "run_experiments.py", line 355, in post_process_project
  runs = json.loads(stdout)
File "/usr/lib/python2.7/json/__init__.py", line 339, in loads
  return _default_decoder.decode(s)
File "/usr/lib/python2.7/json/decoder.py", line 364, in decode
  obj, end = self.raw_decode(s, idx=_w(s, 0).end())
File "/usr/lib/python2.7/json/decoder.py", line 382, in raw_decode
  raise ValueError("No JSON object could be decoded")
  ValueError: No JSON object could be decoded


Repository:
  rC Clang

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

https://reviews.llvm.org/D50488



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

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

[Jan Korous via Phabricator via cfe-commits]

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



Comment at: unittests/Basic/FileManagerTest.cpp:349
 
+// rdar://problem/47536127
+TEST_F(FileManagerTest, getFileDontOpenRealPath) {

arphaman wrote:
> Please leave this comment out
Sure, no problem.

Just so I understand it - when it is appropriate to quote the radar? I see a 
lot of them in clang/test. Are these just from the "ancient" history?


Repository:
  rC Clang

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

https://reviews.llvm.org/D58213



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

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: lib/Basic/FileManager.cpp:271
+  } else if (!openFile) {
+// Since we didn't return early after getStatValue() call the file exists.
+fillRealPathName(&UFE, InterndFileName);

jkorous wrote:
> arphaman wrote:
> > I don't really understand what this comment is trying to say. Can you 
> > rephrase it?
> Please see the above comment (line 214).
> 
> What I meant is that it's guaranteed that the file exists as we would have 
> returned otherwise. Does that make sense?
> 
> Any suggestion how to rephrase the comment? (Or shall I just remove it?)
The throwback here is unnecessary. You can just say that RealFilePath should 
still be filled even for `stat`s.



Comment at: unittests/Basic/FileManagerTest.cpp:357
+
+  const FileEntry *file = manager.getFile(Path);
+

NIT: You might want to make `OpenFile = false` explicit to emphasize the stat.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58213



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

[PATCH] D50488: [Analyzer] Checker for non-determinism caused by sorting of pointer-like elements

[Mandeep Singh Grang via Phabricator via cfe-commits]

mgrang added a comment.

I guess I hadn't sourced the CodeChecker venv correctly. Now that I did source 
it and started the CodeChecker server I seem to be progressing. The error I get 
now is compiler related (which I will dig into).

  12:51:35 [bitcoin] Checking out project... 
  [ERROR] Unknown option: json
  
  [ERROR] configure: error: in `csa-testbench/projects/bitcoin':
  configure: error: C++ compiler cannot create executables
  See `config.log' for more details

Should I be worried about the "[ERROR] Unknown option: json" message?


Repository:
  rC Clang

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

https://reviews.llvm.org/D50488



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

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

[Alex Lorenz via Phabricator via cfe-commits]

arphaman added inline comments.



Comment at: unittests/Basic/FileManagerTest.cpp:349
 
+// rdar://problem/47536127
+TEST_F(FileManagerTest, getFileDontOpenRealPath) {

jkorous wrote:
> arphaman wrote:
> > Please leave this comment out
> Sure, no problem.
> 
> Just so I understand it - when it is appropriate to quote the radar? I see a 
> lot of them in clang/test. Are these just from the "ancient" history?
There are some radars but that doesn't mean we should still be adding them. 
They don't provide any context or value for the developers here, especially in 
a comment. For a unit test like this the name should generally provide some 
form of self-contained description of the scenario, which it does here. I would 
recommend leaving the radar number in a commit message instead.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58213



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

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

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

In D56411#1398329 , @rjmccall wrote:

> In D56411#1398328 , @rjmccall wrote:
>
> > In D56411#1398291 , @tra wrote:
> >
> > > >> That said, does CUDA have a general rule resolving `__host__` vs. 
> > > >> `__device__` overloads based on context?  And does it allow 
> > > >> overloading based solely on `__host__` vs. `__device__`?
> > >
> > > NVCC does not. Clang does. See https://goo.gl/EXnymm for the details.
> > >
> > > AFAICT, NVIDIA is starting to consider adopting Clang's approach:
> > >  http://lists.llvm.org/pipermail/cfe-dev/2018-November/060070.html 
> > > (original message from Bryce apparently didn't make it to the cfe-dev 
> > > archive)
> >
> >
> > Okay.  Probably the template-argument rule ought to be the same as the 
> > address-of-function rule, which I assume means that there's a final pass 
> > that resolves ambiguities in favor of functions that can be used from the 
> > current context, to the extent that that's meaningful.  It's hard to tell 
> > because that document does not appear to include a formal specification.
>
>
> Regardless, that has no effect on this patch.


The check for host/device to resolve template argument already exists in clang 
before this patch. This patch is trying to fix a bug in that check.
e.g.

  __device__ void f();
  __host__ void f();
  template __global__ void kernel() { F(); }
  __host__ void g() { kernel<<<1,1>>>(); }

Template kernel is trying to resove f, it is supposed to get `__device__ f` but 
it gets `__host__ f`, because
Sema::CheckCUDACall thinks the caller of f is g but actually the caller of f is 
the template kernel.

This check cannot be deferred to template instantiation since it is too late. 
It has to be done in
a constant evalucation context where template argument is checked. Since there 
is no existing way
to tell Sema::CheckCUDACall that clang is checking template argument, the 
template is passed through
a newly added member to ExpressionEvaluationContextRecord.


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

https://reviews.llvm.org/D56411



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

[PATCH] D58062: Support framework import/include auto-completion

[David Goldman via Phabricator via cfe-commits]

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

- Add test and fix subfolder bug


Repository:
  rC Clang

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

https://reviews.llvm.org/D58062

Files:
  lib/Sema/SemaCodeComplete.cpp
  test/CodeCompletion/included-frameworks.m

Index: test/CodeCompletion/included-frameworks.m
===
--- /dev/null
+++ test/CodeCompletion/included-frameworks.m
@@ -0,0 +1,29 @@
+// RUN: rm -rf %t && mkdir -p %t/Foo.framework/Headers/SubFolder && mkdir %t/NotAFramework/
+// RUN: touch %t/Foo.framework/Headers/Foo.h && touch %t/Foo.framework/Headers/FOOClass.h
+// RUN: touch %t/Foo.framework/Headers/SubFolder/FOOInternal.h
+
+#import 
+
+#import 
+
+// Note: the run lines follow their respective tests, since line/column
+// matter in this test.
+
+// Autocomplete frameworks without the ".framework" extension.
+//
+// RUN: %clang -fsyntax-only -F %t -Xclang -code-completion-at=%s:5:10 %s -o - | FileCheck -check-prefix=CHECK-1 %s
+// CHECK-1-NOT: Foo.framework/
+// CHECK-1-NOT: NotAFramework/
+// CHECK-1: Foo/
+
+// Autocomplete for frameworks inside its Headers folder.
+//
+// RUN: %clang -fsyntax-only -F %t -Xclang -code-completion-at=%s:5:14 %s -o - | FileCheck -check-prefix=CHECK-2 %s
+// CHECK-2: Foo.h>
+// CHECK-2: FOOClass.h>
+// CHECK-2: SubFolder/
+
+// Autocomplete for folders inside of a frameworks.
+//
+// RUN: %clang -fsyntax-only -F %t -Xclang -code-completion-at=%s:7:24 %s -o - | FileCheck -check-prefix=CHECK-3 %s
+// CHECK-3: FOOInternal.h>
Index: lib/Sema/SemaCodeComplete.cpp
===
--- lib/Sema/SemaCodeComplete.cpp
+++ lib/Sema/SemaCodeComplete.cpp
@@ -8371,10 +8371,23 @@
   };
 
   // Helper: scans IncludeDir for nice files, and adds results for each.
-  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir, bool IsSystem) {
+  auto AddFilesFromIncludeDir = [&](StringRef IncludeDir,
+bool IsSystem,
+DirectoryLookup::LookupType_t LookupType) {
 llvm::SmallString<128> Dir = IncludeDir;
-if (!NativeRelDir.empty())
-  llvm::sys::path::append(Dir, NativeRelDir);
+if (!NativeRelDir.empty()) {
+  if (LookupType == DirectoryLookup::LT_Framework) {
+// For a framework dir, #include  actually maps to
+// a path of Foo.framework/Headers/Bar/.
+auto Begin = llvm::sys::path::begin(NativeRelDir);
+auto End = llvm::sys::path::end(NativeRelDir);
+
+llvm::sys::path::append(Dir, *Begin + ".framework", "Headers");
+llvm::sys::path::append(Dir, ++Begin, End);
+  } else {
+llvm::sys::path::append(Dir, NativeRelDir);
+  }
+}
 
 std::error_code EC;
 unsigned Count = 0;
@@ -8385,6 +8398,12 @@
   StringRef Filename = llvm::sys::path::filename(It->path());
   switch (It->type()) {
   case llvm::sys::fs::file_type::directory_file:
+// All entries in a framework directory must have a ".framework" suffix,
+// but the suffix does not appear in the source code's include/import.
+if (LookupType == DirectoryLookup::LT_Framework &&
+NativeRelDir.empty() && !Filename.consume_back(".framework"))
+  break;
+
 AddCompletion(Filename, /*IsDirectory=*/true);
 break;
   case llvm::sys::fs::file_type::regular_file:
@@ -8413,10 +8432,12 @@
   // header maps are not (currently) enumerable.
   break;
 case DirectoryLookup::LT_NormalDir:
-  AddFilesFromIncludeDir(IncludeDir.getDir()->getName(), IsSystem);
+  AddFilesFromIncludeDir(IncludeDir.getDir()->getName(), IsSystem,
+ DirectoryLookup::LT_NormalDir);
   break;
 case DirectoryLookup::LT_Framework:
-  AddFilesFromIncludeDir(IncludeDir.getFrameworkDir()->getName(), IsSystem);
+  AddFilesFromIncludeDir(IncludeDir.getFrameworkDir()->getName(), IsSystem,
+ DirectoryLookup::LT_Framework);
   break;
 }
   };
@@ -8430,7 +8451,8 @@
 // The current directory is on the include path for "quoted" includes.
 auto *CurFile = PP.getCurrentFileLexer()->getFileEntry();
 if (CurFile && CurFile->getDir())
-  AddFilesFromIncludeDir(CurFile->getDir()->getName(), false);
+  AddFilesFromIncludeDir(CurFile->getDir()->getName(), false,
+ DirectoryLookup::LT_NormalDir);
 for (const auto &D : make_range(S.quoted_dir_begin(), S.quoted_dir_end()))
   AddFilesFromDirLookup(D, false);
   }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

[Jan Korous via Phabricator via cfe-commits]

jkorous updated this revision to Diff 186906.
jkorous added a comment.

- comments
- explicit openFile = false in test


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

https://reviews.llvm.org/D58213

Files:
  lib/Basic/FileManager.cpp
  unittests/Basic/FileManagerTest.cpp


Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -346,4 +346,18 @@
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }
 
+TEST_F(FileManagerTest, getFileDontOpenRealPath) {
+  auto statCache = llvm::make_unique();
+  statCache->InjectDirectory("/tmp/abc", 42);
+  SmallString<64> Path("/tmp/abc/foo.cpp");
+  statCache->InjectFile(Path.str().str().c_str(), 43);
+  manager.setStatCache(std::move(statCache));
+
+  const FileEntry *file = manager.getFile(Path, /*openFile=*/false);
+
+  ASSERT_TRUE(file != nullptr);
+
+  ASSERT_EQ(file->tryGetRealPathName(), Path);
+}
+
 } // anonymous namespace
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -267,6 +267,9 @@
   if (UFE.File) {
 if (auto PathName = UFE.File->getName())
   fillRealPathName(&UFE, *PathName);
+  } else if (!openFile) {
+// We should still fill the path even if we aren't opening the file.
+fillRealPathName(&UFE, InterndFileName);
   }
   return &UFE;
 }


Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -346,4 +346,18 @@
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }
 
+TEST_F(FileManagerTest, getFileDontOpenRealPath) {
+  auto statCache = llvm::make_unique();
+  statCache->InjectDirectory("/tmp/abc", 42);
+  SmallString<64> Path("/tmp/abc/foo.cpp");
+  statCache->InjectFile(Path.str().str().c_str(), 43);
+  manager.setStatCache(std::move(statCache));
+
+  const FileEntry *file = manager.getFile(Path, /*openFile=*/false);
+
+  ASSERT_TRUE(file != nullptr);
+
+  ASSERT_EQ(file->tryGetRealPathName(), Path);
+}
+
 } // anonymous namespace
Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -267,6 +267,9 @@
   if (UFE.File) {
 if (auto PathName = UFE.File->getName())
   fillRealPathName(&UFE, *PathName);
+  } else if (!openFile) {
+// We should still fill the path even if we aren't opening the file.
+fillRealPathName(&UFE, InterndFileName);
   }
   return &UFE;
 }
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D58062: Support framework import/include auto-completion

[David Goldman via Phabricator via cfe-commits]

dgoldman marked 3 inline comments as done.
dgoldman added a comment.

added test


Repository:
  rC Clang

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

https://reviews.llvm.org/D58062



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

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

[Jan Korous via Phabricator via cfe-commits]

jkorous marked 4 inline comments as done.
jkorous added a comment.

Updated the patch.




Comment at: unittests/Basic/FileManagerTest.cpp:349
 
+// rdar://problem/47536127
+TEST_F(FileManagerTest, getFileDontOpenRealPath) {

arphaman wrote:
> jkorous wrote:
> > arphaman wrote:
> > > Please leave this comment out
> > Sure, no problem.
> > 
> > Just so I understand it - when it is appropriate to quote the radar? I see 
> > a lot of them in clang/test. Are these just from the "ancient" history?
> There are some radars but that doesn't mean we should still be adding them. 
> They don't provide any context or value for the developers here, especially 
> in a comment. For a unit test like this the name should generally provide 
> some form of self-contained description of the scenario, which it does here. 
> I would recommend leaving the radar number in a commit message instead.
Got it. Thanks!


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

https://reviews.llvm.org/D58213



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

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

[Alex Lorenz via Phabricator via cfe-commits]

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

LGTM


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

https://reviews.llvm.org/D58213



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

r354063 - Further relax restriction in tests to include where "-E" and "-S" must appear.

[Douglas Yung via cfe-commits]

Author: dyung
Date: Thu Feb 14 13:37:19 2019
New Revision: 354063

URL: http://llvm.org/viewvc/llvm-project?rev=354063&view=rev
Log:
Further relax restriction in tests to include where "-E" and "-S" must appear.

Also updated a few instances of "-emit-llvm-bc" and "-emit-obj" that were 
missed in the previous change.

Modified:
cfe/trunk/test/Driver/openmp-offload.c

Modified: cfe/trunk/test/Driver/openmp-offload.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/openmp-offload.c?rev=354063&r1=354062&r2=354063&view=diff
==
--- cfe/trunk/test/Driver/openmp-offload.c (original)
+++ cfe/trunk/test/Driver/openmp-offload.c Thu Feb 14 13:37:19 2019
@@ -297,7 +297,7 @@
 // CHK-COMMANDS: clang{{.*}}" "-cc1" "-triple" "powerpc64le-unknown-linux" 
{{.*}}"-emit-llvm-bc" {{.*}}"-o" "
 // CHK-COMMANDS-SAME: [[HOSTBC:[^\\/]+\.bc]]" "-x" "c" "
 // CHK-COMMANDS-SAME: [[INPUT:[^\\/]+\.c]]" 
"-fopenmp-targets=powerpc64le-ibm-linux-gnu,x86_64-pc-linux-gnu"
-// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-unknown-linux" 
"-E" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-unknown-linux" 
{{.*}}"-E" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[HOSTPP:[^\\/]+\.i]]" "-x" "c" "
 // CHK-COMMANDS-ST-SAME: [[INPUT:[^\\/]+\.c]]"
 // CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-unknown-linux" 
{{.*}}"-emit-llvm-bc" {{.*}}"-fopenmp" {{.*}}"-o" "
@@ -306,15 +306,15 @@
 //
 // Compile for the powerpc device.
 //
-// CHK-COMMANDS: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" "-emit-obj" {{.*}}"-pic-level" "2" 
{{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" {{.*}}"-emit-obj" {{.*}}"-pic-level" 
"2" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-SAME: [[T1OBJ:[^\\/]+\.o]]" "-x" "c" "{{.*}}[[INPUT]]" 
"-fopenmp-is-device" "-fopenmp-host-ir-file-path" "{{.*}}[[HOSTBC]]"
 // CHK-COMMANDS: ld{{(\.exe)?}}" {{.*}}"-o" "
 // CHK-COMMANDS-SAME: [[T1BIN:[^\\/]+\.out]]" {{.*}}"{{.*}}[[T1OBJ]]"
-// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" "-E" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" {{.*}}"-E" {{.*}}"-fopenmp" 
{{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T1PP:[^\\/]+\.i]]" "-x" "c" "{{.*}}[[INPUT]]"
 // CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" {{.*}}"-emit-llvm-bc" 
{{.*}}"-pic-level" "2" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T1BC:[^\\/]+\.bc]]" "-x" "cpp-output" 
"{{.*}}[[T1PP]]" "-fopenmp-is-device" "-fopenmp-host-ir-file-path" 
"{{.*}}[[HOSTBC]]"
-// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" "-S" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "powerpc64le-ibm-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" {{.*}}"-S" {{.*}}"-fopenmp" 
{{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T1ASM:[^\\/]+\.s]]" "-x" "ir" "{{.*}}[[T1BC]]"
 // CHK-COMMANDS-ST: clang{{.*}}" "-cc1as" "-triple" 
"powerpc64le-ibm-linux-gnu" "-filetype" "obj" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T1OBJ:[^\\/]+\.o]]" "{{.*}}[[T1ASM]]"
@@ -323,15 +323,15 @@
 //
 // Compile for the x86 device.
 //
-// CHK-COMMANDS: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" "-emit-obj" {{.*}}"-pic-level" "2" 
{{.*}}"-fopenmp"  {{.*}}"-o" "
+// CHK-COMMANDS: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" {{.*}}"-emit-obj" {{.*}}"-pic-level" 
"2" {{.*}}"-fopenmp"  {{.*}}"-o" "
 // CHK-COMMANDS-SAME: [[T2OBJ:[^\\/]+\.o]]" "-x" "c" "{{.*}}[[INPUT]]" 
"-fopenmp-is-device" "-fopenmp-host-ir-file-path" "{{.*}}[[HOSTBC]]"
 // CHK-COMMANDS: ld{{(\.exe)?}}" {{.*}}"-o" "
 // CHK-COMMANDS-SAME: [[T2BIN:[^\\/]+\.out]]" {{.*}}"{{.*}}[[T2OBJ]]"
-// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" "-E" {{.*}}"-fopenmp" {{.*}}"-o" "
+// CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" {{.*}}"-E" {{.*}}"-fopenmp" 
{{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T2PP:[^\\/]+\.i]]" "-x" "c" "{{.*}}[[INPUT]]"
 // CHK-COMMANDS-ST: clang{{.*}}" "-cc1" "-triple" "x86_64-pc-linux-gnu" 
"-aux-triple" "powerpc64le-unknown-linux" {{.*}}"-emit-llvm-bc" 
{{.*}}"-pic-level" "2" {{.*}}"-fopenmp" {{.*}}"-o" "
 // CHK-COMMANDS-ST-SAME: [[T2BC:[^\\/]+\.bc]]" "-x" "cpp-output" 
"{{.*}}[[T2PP]]" "-fopenmp-is-device" "-fopenmp-host-ir-file-path" 
"{{.*}}[[HOSTBC]]"
-// CHK-COMMANDS-ST: clang{{.*}}" "

[PATCH] D58074: [OpenMP 5.0] Parsing/sema support for map clause with mapper modifier

[Lingda Li via Phabricator via cfe-commits]

lildmh updated this revision to Diff 186912.
lildmh added a comment.

Introduce a structure `OMPMappableExprListSizeTy` within 
`OMPMappableExprListClause` to aggregate all 4 sizes needed by such clause, and 
thus reduce the number of parameters when creating a map clause. This can also 
be used by other similar clauses. I'll have another patch to let other clauses 
to use this structure as well.


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

https://reviews.llvm.org/D58074

Files:
  include/clang/AST/DeclOpenMP.h
  include/clang/AST/OpenMPClause.h
  include/clang/Basic/DiagnosticParseKinds.td
  include/clang/Basic/DiagnosticSemaKinds.td
  include/clang/Basic/OpenMPKinds.def
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/OpenMPClause.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Sema/SemaOpenMP.cpp
  lib/Sema/SemaTemplateInstantiateDecl.cpp
  lib/Sema/TreeTransform.h
  lib/Serialization/ASTReader.cpp
  lib/Serialization/ASTWriter.cpp
  test/OpenMP/declare_mapper_ast_print.c
  test/OpenMP/declare_mapper_ast_print.cpp
  test/OpenMP/declare_mapper_codegen.cpp
  test/OpenMP/declare_mapper_messages.c
  test/OpenMP/declare_mapper_messages.cpp
  test/OpenMP/target_map_messages.cpp
  test/OpenMP/target_parallel_for_map_messages.cpp
  test/OpenMP/target_parallel_for_simd_map_messages.cpp
  test/OpenMP/target_parallel_map_messages.cpp
  test/OpenMP/target_simd_map_messages.cpp
  test/OpenMP/target_teams_distribute_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_map_messages.cpp
  test/OpenMP/target_teams_distribute_parallel_for_simd_map_messages.cpp
  test/OpenMP/target_teams_distribute_simd_map_messages.cpp
  test/OpenMP/target_teams_map_messages.cpp

Index: test/OpenMP/target_teams_map_messages.cpp
===
--- test/OpenMP/target_teams_map_messages.cpp
+++ test/OpenMP/target_teams_map_messages.cpp
@@ -454,7 +454,7 @@
 
 #pragma omp target data map(always, tofrom: x)
 #pragma omp target data map(always: x) // expected-error {{missing map type}}
-#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
 #pragma omp target data map(always, tofrom: always, tofrom, x)
 #pragma omp target teams map(tofrom j) // expected-error {{expected ',' or ')' in 'map' clause}}
   foo();
@@ -529,7 +529,7 @@
 
 #pragma omp target data map(always, tofrom: x)
 #pragma omp target data map(always: x) // expected-error {{missing map type}}
-#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target data map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
 #pragma omp target data map(always, tofrom: always, tofrom, x)
 #pragma omp target teams map(tofrom j) // expected-error {{expected ',' or ')' in 'map' clause}}
   foo();
Index: test/OpenMP/target_teams_distribute_simd_map_messages.cpp
===
--- test/OpenMP/target_teams_distribute_simd_map_messages.cpp
+++ test/OpenMP/target_teams_distribute_simd_map_messages.cpp
@@ -163,7 +163,7 @@
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always: x) // expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
-#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always, tofrom: always, tofrom, x)
   for (i = 0; i < argc; ++i) foo();
@@ -271,7 +271,7 @@
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always: x) // expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
-#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always' or 'close'}} expected-error {{missing map type}}
+#pragma omp target teams distribute simd map(tofrom, always: x) // expected-error {{incorrect map type modifier, expected 'always', 'close', or 'mapper'}} expected-error {{missing map type}}
   for (i = 0; i < argc; ++i) foo();
 #pragma omp target teams distribute simd map(always, tofrom: always, tofrom, x)
   for (i = 0; i < argc; ++i) foo();
Index: test/OpenMP/target_teams_distribute_parallel

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

[John McCall via Phabricator via cfe-commits]

rjmccall added a comment.

But what we've just been talking about is not a validity rule, it's an 
overload-resolution rule.  It's not *invalid* to use a device function as a 
template argument to a host function template (or to a class template, which of 
course is neither host nor device).  All you need to do is to resolve 
otherwise-intractable overload ambiguities by matching with the host-ness of 
the current context, which there's probably already code to do for when an 
overload set is used as e.g. a function argument.


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

https://reviews.llvm.org/D56411



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

[clang-tools-extra] r354069 - [clang-tidy] Mention language version in test explicitly.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Thu Feb 14 14:37:30 2019
New Revision: 354069

URL: http://llvm.org/viewvc/llvm-project?rev=354069&view=rev
Log:
[clang-tidy] Mention language version in test explicitly.

"modernize-use-using" check is applicable only to C++11 and later. Spell it out
to avoid relying on default language version.

rdar://problem/47932196

Modified:
clang-tools-extra/trunk/test/clang-tidy/clang-tidy-mac-libcxx.cpp

Modified: clang-tools-extra/trunk/test/clang-tidy/clang-tidy-mac-libcxx.cpp
URL: 
http://llvm.org/viewvc/llvm-project/clang-tools-extra/trunk/test/clang-tidy/clang-tidy-mac-libcxx.cpp?rev=354069&r1=354068&r2=354069&view=diff
==
--- clang-tools-extra/trunk/test/clang-tidy/clang-tidy-mac-libcxx.cpp (original)
+++ clang-tools-extra/trunk/test/clang-tidy/clang-tidy-mac-libcxx.cpp Thu Feb 
14 14:37:30 2019
@@ -8,7 +8,7 @@
 // RUN: cp -r %S/Inputs/mock-libcxx %t/
 //
 // Pretend clang is installed beside the mock library that we provided.
-// RUN: echo '[{"directory":"%t","command":"%t/mock-libcxx/bin/clang++ 
-stdlib=libc++ -target x86_64-apple-darwin -c test.cpp","file":"test.cpp"}]' | 
sed -e 's/\\/\//g' > %t/compile_commands.json
+// RUN: echo '[{"directory":"%t","command":"%t/mock-libcxx/bin/clang++ 
-stdlib=libc++ -std=c++11 -target x86_64-apple-darwin -c 
test.cpp","file":"test.cpp"}]' | sed -e 's/\\/\//g' > %t/compile_commands.json
 // RUN: cp "%s" "%t/test.cpp"
 // RUN: clang-tidy -header-filter='.*' -system-headers 
-checks='-*,modernize-use-using'  "%t/test.cpp" | FileCheck %s
 // CHECK: mock_vector:{{[0-9]+}}:{{[0-9]+}}: warning: use 'using' instead of 
'typedef'


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

[PATCH] D58254: [Sema] Diagnose floating point conversions based on target semantics

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington created this revision.
erik.pilkington added reviewers: rjmccall, nickdesaulniers.
Herald added subscribers: jdoerfert, dexonsmith, jkorous.

...instead of just comparing rank. Also, fix a bad warning about `_Float16`, 
since its declared out of order in BuiltinTypes.def, meaning comparing rank 
using `BuiltinType::getKind()` is incorrect.

Came out of some discussion on D58145 

Thanks!


https://reviews.llvm.org/D58254

Files:
  clang/include/clang/AST/ASTContext.h
  clang/lib/AST/ASTContext.cpp
  clang/lib/Sema/SemaChecking.cpp
  clang/test/Sema/conversion-target-dep.c

Index: clang/test/Sema/conversion-target-dep.c
===
--- /dev/null
+++ clang/test/Sema/conversion-target-dep.c
@@ -0,0 +1,24 @@
+// RUN: %clang_cc1 -Wdouble-promotion -Wimplicit-float-conversion %s -triple x86_64-apple-macosx10.12 -verify=x86,expected
+// RUN: %clang_cc1 -Wdouble-promotion -Wimplicit-float-conversion %s -triple armv7-apple-ios9.0 -verify=arm,expected
+
+// On ARM, long double and double both map to double precision 754s, so there
+// isn't any reason to warn on conversions back and forth.
+
+long double ld;
+double d;
+_Float16 f16; // x86-error {{_Float16 is not supported on this target}}
+
+int main() {
+  ld = d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
+  d = ld; // x86-warning {{implicit conversion loses floating-point precision: 'long double' to 'double'}}
+
+  ld += d; // x86-warning {{implicit conversion increases floating-point precision: 'double' to 'long double'}}
+  d += ld; // x86-warning {{implicit conversion when assigning computation result loses floating-point precision: 'long double' to 'double'}}
+
+  f16 = ld; // expected-warning {{implicit conversion loses floating-point precision: 'long double' to '_Float16'}}
+  ld = f16; // expected-warning {{implicit conversion increases floating-point precision: '_Float16' to 'long double'}}
+
+  f16 += ld; // expected-warning {{implicit conversion when assigning computation result loses floating-point precision: 'long double' to '_Float16'}}
+  ld += f16; // expected-warning {{implicit conversion increases floating-point precision: '_Float16' to 'long double'}}
+}
+
Index: clang/lib/Sema/SemaChecking.cpp
===
--- clang/lib/Sema/SemaChecking.cpp
+++ clang/lib/Sema/SemaChecking.cpp
@@ -10626,14 +10626,16 @@
 
   // If source is floating point but target is an integer.
   if (ResultBT->isInteger())
-DiagnoseImpCast(S, E, E->getRHS()->getType(), E->getLHS()->getType(),
-E->getExprLoc(), diag::warn_impcast_float_integer);
-  // If both source and target are floating points. Builtin FP kinds are ordered
-  // by increasing FP rank. FIXME: except _Float16, we currently emit a bogus
-  // warning.
-  else if (ResultBT->isFloatingPoint() && ResultBT->getKind() < RBT->getKind() &&
-   // We don't want to warn for system macro.
-   !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+return DiagnoseImpCast(S, E, E->getRHS()->getType(), E->getLHS()->getType(),
+   E->getExprLoc(), diag::warn_impcast_float_integer);
+
+  if (!ResultBT->isFloatingPoint())
+return;
+
+  // If both source and target are floating points, warn about losing precision.
+  int Order = S.getASTContext().getFloatingTypeSemanticOrder(
+  QualType(ResultBT, 0), QualType(RBT, 0));
+  if (Order < 0 && !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
 // warn about dropping FP rank.
 DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), E->getOperatorLoc(),
 diag::warn_impcast_float_result_precision);
@@ -10952,8 +10954,9 @@
 if (TargetBT && TargetBT->isFloatingPoint()) {
   // ...then warn if we're dropping FP rank.
 
-  // Builtin FP kinds are ordered by increasing FP rank.
-  if (SourceBT->getKind() > TargetBT->getKind()) {
+  int Order = S.getASTContext().getFloatingTypeSemanticOrder(
+  QualType(SourceBT, 0), QualType(TargetBT, 0));
+  if (Order > 0) {
 // Don't warn about float constants that are precisely
 // representable in the target type.
 Expr::EvalResult result;
@@ -10971,7 +10974,7 @@
 DiagnoseImpCast(S, E, T, CC, diag::warn_impcast_float_precision);
   }
   // ... or possibly if we're increasing rank, too
-  else if (TargetBT->getKind() > SourceBT->getKind()) {
+  else if (Order < 0) {
 if (S.SourceMgr.isInSystemMacro(CC))
   return;
 
Index: clang/lib/AST/ASTContext.cpp
===
--- clang/lib/AST/ASTContext.cpp
+++ clang/lib/AST/ASTContext.cpp
@@ -5608,6 +5608,12 @@
   return -1;
 }
 
+int ASTContext::getFloatingTypeSemanticOrder(QualType LHS, QualType RHS) const {
+  if (&getFloatTypeSemantics(LHS) == 

[PATCH] D58145: [Sema] Fix a bogus -Wconversion warning

[Erik Pilkington via Phabricator via cfe-commits]

erik.pilkington marked an inline comment as done.
erik.pilkington added inline comments.



Comment at: clang/lib/Sema/SemaChecking.cpp:10634
+  // warning.
+  else if (ResultBT->isFloatingPoint() && ResultBT->getKind() < RBT->getKind() 
&&
+   // We don't want to warn for system macro.

rjmccall wrote:
> Yeah, can we just make a predicate function for this rank comparison?  And 
> really it shouldn't be based on rank: while a difference in FP rank *often* 
> corresponds to a difference in representable range, there are exceptions 
> (notably with `long double` on non-x86 targets (as well as Win64) which is 
> often equivalent to either `double` or `__float128`), and we should probably 
> treat those like we do the integer types, i.e. ignore them.
There is another place that we're doing this in this file, so I put it in a 
follow up: https://reviews.llvm.org/D58254. Thanks!


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

https://reviews.llvm.org/D58145



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

r354074 - [Sema] Fix-up a -Wfloat-conversion diagnostic

[Erik Pilkington via cfe-commits]

Author: epilk
Date: Thu Feb 14 14:48:01 2019
New Revision: 354074

URL: http://llvm.org/viewvc/llvm-project?rev=354074&view=rev
Log:
[Sema] Fix-up a -Wfloat-conversion diagnostic

We were warning on valid ObjC property reference exprs, and passing
in the wrong arguments to DiagnoseFloatingImpCast (leading to a badly
worded diagnostic).

rdar://47644670

Differential revision: https://reviews.llvm.org/D58145

Modified:
cfe/trunk/lib/Sema/SemaChecking.cpp
cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
cfe/trunk/test/SemaObjC/conversion.m

Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=354074&r1=354073&r2=354074&view=diff
==
--- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
+++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Feb 14 14:48:01 2019
@@ -10624,16 +10624,16 @@ static void AnalyzeCompoundAssignment(Se
   // The below checks assume source is floating point.
   if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
 
-  // If source is floating point but target is not.
-  if (!ResultBT->isFloatingPoint())
-return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
-   E->getExprLoc());
-
-  // If both source and target are floating points.
-  // Builtin FP kinds are ordered by increasing FP rank.
-  if (ResultBT->getKind() < RBT->getKind() &&
-  // We don't want to warn for system macro.
-  !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+  // If source is floating point but target is an integer.
+  if (ResultBT->isInteger())
+DiagnoseImpCast(S, E, E->getRHS()->getType(), E->getLHS()->getType(),
+E->getExprLoc(), diag::warn_impcast_float_integer);
+  // If both source and target are floating points. Builtin FP kinds are 
ordered
+  // by increasing FP rank. FIXME: except _Float16, we currently emit a bogus
+  // warning.
+  else if (ResultBT->isFloatingPoint() && ResultBT->getKind() < RBT->getKind() 
&&
+   // We don't want to warn for system macro.
+   !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
 // warn about dropping FP rank.
 DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), 
E->getOperatorLoc(),
 diag::warn_impcast_float_result_precision);

Modified: cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-float-conversion.cpp?rev=354074&r1=354073&r2=354074&view=diff
==
--- cfe/trunk/test/SemaCXX/warn-float-conversion.cpp (original)
+++ cfe/trunk/test/SemaCXX/warn-float-conversion.cpp Thu Feb 14 14:48:01 2019
@@ -44,17 +44,17 @@ void Convert(float f, double d, long dou
 void CompoundAssignment() {
   int x = 3;
 
-  x += 1.234;  //expected-warning{{conversion}}
-  x -= -0.0;  //expected-warning{{conversion}}
-  x *= 1.1f;  //expected-warning{{conversion}}
-  x /= -2.2f;  //expected-warning{{conversion}}
+  x += 1.234; // expected-warning {{implicit conversion turns floating-point 
number into integer: 'double' to 'int'}}
+  x -= -0.0;  // expected-warning {{implicit conversion turns floating-point 
number into integer: 'double' to 'int'}}
+  x *= 1.1f;  // expected-warning {{implicit conversion turns floating-point 
number into integer: 'float' to 'int'}}
+  x /= -2.2f; // expected-warning {{implicit conversion turns floating-point 
number into integer: 'float' to 'int'}}
 
-  int y = x += 1.4f;  //expected-warning{{conversion}}
+  int y = x += 1.4f; // expected-warning {{implicit conversion turns 
floating-point number into integer: 'float' to 'int'}}
 
   float z = 1.1f;
   double w = -2.2;
 
-  y += z + w;  //expected-warning{{conversion}}
+  y += z + w; // expected-warning {{implicit conversion turns floating-point 
number into integer: 'double' to 'int'}}
 }
 
 # 1 "foo.h" 3

Modified: cfe/trunk/test/SemaObjC/conversion.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/conversion.m?rev=354074&r1=354073&r2=354074&view=diff
==
--- cfe/trunk/test/SemaObjC/conversion.m (original)
+++ cfe/trunk/test/SemaObjC/conversion.m Thu Feb 14 14:48:01 2019
@@ -14,4 +14,11 @@ void radar14415662(RDar14415662 *f, char
   x = y; // expected-warning {{implicit conversion loses integer precision: 
'int' to 'char'}}
 }
 
+__attribute__((objc_root_class)) @interface DoubleProp
+@property double d;
+@end
 
+void use_double_prop(DoubleProp *dp) {
+  double local = 42;
+  dp.d += local; // no warning
+}


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

[PATCH] D58145: [Sema] Fix a bogus -Wconversion warning

[Phabricator 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 rL354074: [Sema] Fix-up a -Wfloat-conversion diagnostic 
(authored by epilk, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D58145?vs=186774&id=186922#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D58145

Files:
  cfe/trunk/lib/Sema/SemaChecking.cpp
  cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
  cfe/trunk/test/SemaObjC/conversion.m


Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -10624,16 +10624,16 @@
   // The below checks assume source is floating point.
   if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
 
-  // If source is floating point but target is not.
-  if (!ResultBT->isFloatingPoint())
-return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
-   E->getExprLoc());
-
-  // If both source and target are floating points.
-  // Builtin FP kinds are ordered by increasing FP rank.
-  if (ResultBT->getKind() < RBT->getKind() &&
-  // We don't want to warn for system macro.
-  !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+  // If source is floating point but target is an integer.
+  if (ResultBT->isInteger())
+DiagnoseImpCast(S, E, E->getRHS()->getType(), E->getLHS()->getType(),
+E->getExprLoc(), diag::warn_impcast_float_integer);
+  // If both source and target are floating points. Builtin FP kinds are 
ordered
+  // by increasing FP rank. FIXME: except _Float16, we currently emit a bogus
+  // warning.
+  else if (ResultBT->isFloatingPoint() && ResultBT->getKind() < RBT->getKind() 
&&
+   // We don't want to warn for system macro.
+   !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
 // warn about dropping FP rank.
 DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), 
E->getOperatorLoc(),
 diag::warn_impcast_float_result_precision);
Index: cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
===
--- cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
+++ cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
@@ -44,17 +44,17 @@
 void CompoundAssignment() {
   int x = 3;
 
-  x += 1.234;  //expected-warning{{conversion}}
-  x -= -0.0;  //expected-warning{{conversion}}
-  x *= 1.1f;  //expected-warning{{conversion}}
-  x /= -2.2f;  //expected-warning{{conversion}}
+  x += 1.234; // expected-warning {{implicit conversion turns floating-point 
number into integer: 'double' to 'int'}}
+  x -= -0.0;  // expected-warning {{implicit conversion turns floating-point 
number into integer: 'double' to 'int'}}
+  x *= 1.1f;  // expected-warning {{implicit conversion turns floating-point 
number into integer: 'float' to 'int'}}
+  x /= -2.2f; // expected-warning {{implicit conversion turns floating-point 
number into integer: 'float' to 'int'}}
 
-  int y = x += 1.4f;  //expected-warning{{conversion}}
+  int y = x += 1.4f; // expected-warning {{implicit conversion turns 
floating-point number into integer: 'float' to 'int'}}
 
   float z = 1.1f;
   double w = -2.2;
 
-  y += z + w;  //expected-warning{{conversion}}
+  y += z + w; // expected-warning {{implicit conversion turns floating-point 
number into integer: 'double' to 'int'}}
 }
 
 # 1 "foo.h" 3
Index: cfe/trunk/test/SemaObjC/conversion.m
===
--- cfe/trunk/test/SemaObjC/conversion.m
+++ cfe/trunk/test/SemaObjC/conversion.m
@@ -14,4 +14,11 @@
   x = y; // expected-warning {{implicit conversion loses integer precision: 
'int' to 'char'}}
 }
 
+__attribute__((objc_root_class)) @interface DoubleProp
+@property double d;
+@end
 
+void use_double_prop(DoubleProp *dp) {
+  double local = 42;
+  dp.d += local; // no warning
+}


Index: cfe/trunk/lib/Sema/SemaChecking.cpp
===
--- cfe/trunk/lib/Sema/SemaChecking.cpp
+++ cfe/trunk/lib/Sema/SemaChecking.cpp
@@ -10624,16 +10624,16 @@
   // The below checks assume source is floating point.
   if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
 
-  // If source is floating point but target is not.
-  if (!ResultBT->isFloatingPoint())
-return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
-   E->getExprLoc());
-
-  // If both source and target are floating points.
-  // Builtin FP kinds are ordered by increasing FP rank.
-  if (ResultBT->getKind() < RBT->getKind() &&
-  // We don't want to warn for system macro.
-  !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
+  // 

[PATCH] D58213: [FileManager] fillRealPathName even if we aren't opening the file

[Jan Korous via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
jkorous marked an inline comment as done.
Closed by commit rC354075: [clang][FileManager] fillRealPathName even if we 
aren't opening the file (authored by jkorous, committed by ).

Repository:
  rC Clang

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

https://reviews.llvm.org/D58213

Files:
  lib/Basic/FileManager.cpp
  unittests/Basic/FileManagerTest.cpp


Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -267,6 +267,9 @@
   if (UFE.File) {
 if (auto PathName = UFE.File->getName())
   fillRealPathName(&UFE, *PathName);
+  } else if (!openFile) {
+// We should still fill the path even if we aren't opening the file.
+fillRealPathName(&UFE, InterndFileName);
   }
   return &UFE;
 }
Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -346,4 +346,18 @@
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }
 
+TEST_F(FileManagerTest, getFileDontOpenRealPath) {
+  auto statCache = llvm::make_unique();
+  statCache->InjectDirectory("/tmp/abc", 42);
+  SmallString<64> Path("/tmp/abc/foo.cpp");
+  statCache->InjectFile(Path.str().str().c_str(), 43);
+  manager.setStatCache(std::move(statCache));
+
+  const FileEntry *file = manager.getFile(Path, /*openFile=*/false);
+
+  ASSERT_TRUE(file != nullptr);
+
+  ASSERT_EQ(file->tryGetRealPathName(), Path);
+}
+
 } // anonymous namespace


Index: lib/Basic/FileManager.cpp
===
--- lib/Basic/FileManager.cpp
+++ lib/Basic/FileManager.cpp
@@ -267,6 +267,9 @@
   if (UFE.File) {
 if (auto PathName = UFE.File->getName())
   fillRealPathName(&UFE, *PathName);
+  } else if (!openFile) {
+// We should still fill the path even if we aren't opening the file.
+fillRealPathName(&UFE, InterndFileName);
   }
   return &UFE;
 }
Index: unittests/Basic/FileManagerTest.cpp
===
--- unittests/Basic/FileManagerTest.cpp
+++ unittests/Basic/FileManagerTest.cpp
@@ -346,4 +346,18 @@
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }
 
+TEST_F(FileManagerTest, getFileDontOpenRealPath) {
+  auto statCache = llvm::make_unique();
+  statCache->InjectDirectory("/tmp/abc", 42);
+  SmallString<64> Path("/tmp/abc/foo.cpp");
+  statCache->InjectFile(Path.str().str().c_str(), 43);
+  manager.setStatCache(std::move(statCache));
+
+  const FileEntry *file = manager.getFile(Path, /*openFile=*/false);
+
+  ASSERT_TRUE(file != nullptr);
+
+  ASSERT_EQ(file->tryGetRealPathName(), Path);
+}
+
 } // anonymous namespace
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r354075 - [clang][FileManager] fillRealPathName even if we aren't opening the file

[Jan Korous via cfe-commits]

Author: jkorous
Date: Thu Feb 14 15:02:35 2019
New Revision: 354075

URL: http://llvm.org/viewvc/llvm-project?rev=354075&view=rev
Log:
[clang][FileManager] fillRealPathName even if we aren't opening the file

The pathname wasn't previously filled when the getFile() method was called with 
openFile = false.
We are caching FileEntry-s in ParsedAST::Includes in clangd and this caused the 
problem.

This fixes an internal test failure in clangd - ClangdTests.GoToInclude.All

rdar://47536127

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

Modified:
cfe/trunk/lib/Basic/FileManager.cpp
cfe/trunk/unittests/Basic/FileManagerTest.cpp

Modified: cfe/trunk/lib/Basic/FileManager.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/FileManager.cpp?rev=354075&r1=354074&r2=354075&view=diff
==
--- cfe/trunk/lib/Basic/FileManager.cpp (original)
+++ cfe/trunk/lib/Basic/FileManager.cpp Thu Feb 14 15:02:35 2019
@@ -267,6 +267,9 @@ const FileEntry *FileManager::getFile(St
   if (UFE.File) {
 if (auto PathName = UFE.File->getName())
   fillRealPathName(&UFE, *PathName);
+  } else if (!openFile) {
+// We should still fill the path even if we aren't opening the file.
+fillRealPathName(&UFE, InterndFileName);
   }
   return &UFE;
 }

Modified: cfe/trunk/unittests/Basic/FileManagerTest.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/unittests/Basic/FileManagerTest.cpp?rev=354075&r1=354074&r2=354075&view=diff
==
--- cfe/trunk/unittests/Basic/FileManagerTest.cpp (original)
+++ cfe/trunk/unittests/Basic/FileManagerTest.cpp Thu Feb 14 15:02:35 2019
@@ -346,4 +346,18 @@ TEST_F(FileManagerTest, getVirtualFileFi
   EXPECT_EQ(file->tryGetRealPathName(), ExpectedResult);
 }
 
+TEST_F(FileManagerTest, getFileDontOpenRealPath) {
+  auto statCache = llvm::make_unique();
+  statCache->InjectDirectory("/tmp/abc", 42);
+  SmallString<64> Path("/tmp/abc/foo.cpp");
+  statCache->InjectFile(Path.str().str().c_str(), 43);
+  manager.setStatCache(std::move(statCache));
+
+  const FileEntry *file = manager.getFile(Path, /*openFile=*/false);
+
+  ASSERT_TRUE(file != nullptr);
+
+  ASSERT_EQ(file->tryGetRealPathName(), Path);
+}
+
 } // anonymous namespace


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

Re: r354074 - [Sema] Fix-up a -Wfloat-conversion diagnostic

[Erik Pilkington via cfe-commits]

Hans, can you merge this diagnostic regression fix into LLVM 8?

Thanks!

> On Feb 14, 2019, at 2:48 PM, Erik Pilkington via cfe-commits 
>  wrote:
> 
> Author: epilk
> Date: Thu Feb 14 14:48:01 2019
> New Revision: 354074
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=354074&view=rev
> Log:
> [Sema] Fix-up a -Wfloat-conversion diagnostic
> 
> We were warning on valid ObjC property reference exprs, and passing
> in the wrong arguments to DiagnoseFloatingImpCast (leading to a badly
> worded diagnostic).
> 
> rdar://47644670
> 
> Differential revision: https://reviews.llvm.org/D58145
> 
> Modified:
>cfe/trunk/lib/Sema/SemaChecking.cpp
>cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
>cfe/trunk/test/SemaObjC/conversion.m
> 
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaChecking.cpp?rev=354074&r1=354073&r2=354074&view=diff
> ==
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Feb 14 14:48:01 2019
> @@ -10624,16 +10624,16 @@ static void AnalyzeCompoundAssignment(Se
>   // The below checks assume source is floating point.
>   if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
> 
> -  // If source is floating point but target is not.
> -  if (!ResultBT->isFloatingPoint())
> -return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
> -   E->getExprLoc());
> -
> -  // If both source and target are floating points.
> -  // Builtin FP kinds are ordered by increasing FP rank.
> -  if (ResultBT->getKind() < RBT->getKind() &&
> -  // We don't want to warn for system macro.
> -  !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
> +  // If source is floating point but target is an integer.
> +  if (ResultBT->isInteger())
> +DiagnoseImpCast(S, E, E->getRHS()->getType(), E->getLHS()->getType(),
> +E->getExprLoc(), diag::warn_impcast_float_integer);
> +  // If both source and target are floating points. Builtin FP kinds are 
> ordered
> +  // by increasing FP rank. FIXME: except _Float16, we currently emit a bogus
> +  // warning.
> +  else if (ResultBT->isFloatingPoint() && ResultBT->getKind() < 
> RBT->getKind() &&
> +   // We don't want to warn for system macro.
> +   !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
> // warn about dropping FP rank.
> DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), 
> E->getOperatorLoc(),
> diag::warn_impcast_float_result_precision);
> 
> Modified: cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaCXX/warn-float-conversion.cpp?rev=354074&r1=354073&r2=354074&view=diff
> ==
> --- cfe/trunk/test/SemaCXX/warn-float-conversion.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-float-conversion.cpp Thu Feb 14 14:48:01 2019
> @@ -44,17 +44,17 @@ void Convert(float f, double d, long dou
> void CompoundAssignment() {
>   int x = 3;
> 
> -  x += 1.234;  //expected-warning{{conversion}}
> -  x -= -0.0;  //expected-warning{{conversion}}
> -  x *= 1.1f;  //expected-warning{{conversion}}
> -  x /= -2.2f;  //expected-warning{{conversion}}
> +  x += 1.234; // expected-warning {{implicit conversion turns floating-point 
> number into integer: 'double' to 'int'}}
> +  x -= -0.0;  // expected-warning {{implicit conversion turns floating-point 
> number into integer: 'double' to 'int'}}
> +  x *= 1.1f;  // expected-warning {{implicit conversion turns floating-point 
> number into integer: 'float' to 'int'}}
> +  x /= -2.2f; // expected-warning {{implicit conversion turns floating-point 
> number into integer: 'float' to 'int'}}
> 
> -  int y = x += 1.4f;  //expected-warning{{conversion}}
> +  int y = x += 1.4f; // expected-warning {{implicit conversion turns 
> floating-point number into integer: 'float' to 'int'}}
> 
>   float z = 1.1f;
>   double w = -2.2;
> 
> -  y += z + w;  //expected-warning{{conversion}}
> +  y += z + w; // expected-warning {{implicit conversion turns floating-point 
> number into integer: 'double' to 'int'}}
> }
> 
> # 1 "foo.h" 3
> 
> Modified: cfe/trunk/test/SemaObjC/conversion.m
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/SemaObjC/conversion.m?rev=354074&r1=354073&r2=354074&view=diff
> ==
> --- cfe/trunk/test/SemaObjC/conversion.m (original)
> +++ cfe/trunk/test/SemaObjC/conversion.m Thu Feb 14 14:48:01 2019
> @@ -14,4 +14,11 @@ void radar14415662(RDar14415662 *f, char
>   x = y; // expected-warning {{implicit conversion loses integer precision: 
> 'int' to 'char'}}
> }
> 
> +__attribute__((objc_root_class)) @interface DoubleProp
> +@property double d;
> +@end
> 
> +void use_double_prop(DoubleProp *dp) {
> +  doub

[PATCH] D54978: Move the SMT API to LLVM

[Brian Rzycki via Phabricator via cfe-commits]

brzycki added a comment.

In D54978#1397354 , @ddcc wrote:

> In D54978#1397316 , @mikhail.ramalho 
> wrote:
>
> > I just sent the first prototype of the solution. All the magic happens 
> > inside the `CHECK_CXX_SOURCE_RUNS` call.
>
>
> I think hardcoding the version into the binary is too brittle. Instead, your 
> program can just print out the current z3 version (much like the current 
> execution of the z3 binary), and the remaining logic can remain in CMake, 
> with the fallbacks as suggested above by @brzycki.


I don't think there is a mechanism to do so easily within CMake. The 
`CHECK_CXX_SOURCE_RUNS` call only returns a boolean answer, not stdout. You 
have to use the underlying `try_run()` command along with `RUN_OUTPUT_VARIABLE` 
to obtain a given version. This interface seems less simple than the 
`CHECK_CXX_*` macros.


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

https://reviews.llvm.org/D54978



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

[PATCH] D54978: Move the SMT API to LLVM

[Brian Rzycki via Phabricator via cfe-commits]

brzycki added a comment.

Here's my proposed logic as actual CMake code. @mikhail.ramalho if you can get 
your code to emit the version string I made a `TODO` placeholder for it in the 
3rd block below.

  diff --git a/llvm/cmake/modules/FindZ3.cmake b/llvm/cmake/modules/FindZ3.cmake
  index 5c6d3f0..f913686 100644
  --- a/llvm/cmake/modules/FindZ3.cmake
  +++ b/llvm/cmake/modules/FindZ3.cmake
  @@ -30,7 +30,23 @@ find_program(Z3_EXECUTABLE z3
  PATH_SUFFIXES bin
  )
   
  -if(Z3_INCLUDE_DIR AND Z3_LIBRARIES AND Z3_EXECUTABLE)
  +# Searching for the version of the Z3 library is a best-effort task. The 
version
  +# is only recently added to a public header.
  +unset(Z3_VERSION_STRING)
  +
  +if(Z3_INCLUDE_DIR AND EXISTS "${Z3_INCLUDE_DIR}/z3_version.h")
  +# Z3 4.8.1+ has the version is in a public header.
  +file(STRINGS "${Z3_INCLUDE_DIR}/z3_version.h"
  + z3_version_str REGEX "^#define[\t ]+Z3_FULL_VERSION[\t ]+\".*\"")
  +
  +string(REGEX REPLACE "^.*Z3_FULL_VERSION[\t ]+\"Z3 ([0-9\.]+)\".*$" "\\1"
  + Z3_VERSION_STRING "${z3_version_str}")
  +unset(z3_version_str)
  +endif()
  +
  +if(NOT Z3_VERSION_STRING AND (Z3_INCLUDE_DIR AND Z3_LIBRARIES AND
  +  Z3_EXECUTABLE))
  +# Run the found z3 executable and parse the version string output.
   execute_process (COMMAND ${Z3_EXECUTABLE} -version
 OUTPUT_VARIABLE libz3_version_str
 ERROR_QUIET
  @@ -41,6 +57,20 @@ if(Z3_INCLUDE_DIR AND Z3_LIBRARIES AND Z3_EXECUTABLE)
   unset(libz3_version_str)
   endif()
   
  +if(NOT Z3_VERSION_STRING AND (Z3_INCLUDE_DIR AND Z3_LIBRARIES))
  +# We do not have the Z3 binary to query for a version. Try to use
  +# a small C++ program to detect it via the Z3_get_version() API call.
  +# TODO: add code from Mikhail Ramalho to obtain the version.
  +# TODO: set(Z3_VERSION_STRING) if successful.
  +endif()
  +
  +if(NOT Z3_VERSION_STRING)
  +# Give up: we are unable to obtain a version of the Z3 library. Be
  +# conservative and force the found version to 0.0.0 to make version
  +# checks always fail.
  +set(Z3_VERSION_STRING "0.0.0")
  +endif()
  +


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

https://reviews.llvm.org/D54978



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

r354084 - [Driver][Darwin] Emit an error when using -pg on OS without support for it.

[Volodymyr Sapsai via cfe-commits]

Author: vsapsai
Date: Thu Feb 14 15:50:44 2019
New Revision: 354084

URL: http://llvm.org/viewvc/llvm-project?rev=354084&view=rev
Log:
[Driver][Darwin] Emit an error when using -pg on OS without support for it.

Instead of letting a program fail at runtime, emit an error during
compilation.

rdar://problem/12206955

Reviewers: dexonsmith, bob.wilson, steven_wu

Reviewed By: steven_wu

Subscribers: jkorous, cfe-commits

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
cfe/trunk/test/Driver/darwin-ld.c

Modified: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td?rev=354084&r1=354083&r2=354084&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td Thu Feb 14 15:50:44 
2019
@@ -96,6 +96,8 @@ def err_drv_clang_unsupported : Error<
   "the clang compiler does not support '%0'">;
 def err_drv_clang_unsupported_opt_cxx_darwin_i386 : Error<
   "the clang compiler does not support '%0' for C++ on Darwin/i386">;
+def err_drv_clang_unsupported_opt_pg_darwin: Error<
+  "the clang compiler does not support -pg option on %select{Darwin|versions 
of OS X 10.9 and later}0">;
 def err_drv_clang_unsupported_opt_faltivec : Error<
   "the clang compiler does not support '%0', %1">;
 def err_drv_command_failed : Error<

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=354084&r1=354083&r2=354084&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Thu Feb 14 15:50:44 2019
@@ -2289,22 +2289,27 @@ void Darwin::addStartObjectFileArgs(cons
   }
 } else {
   if (Args.hasArg(options::OPT_pg) && SupportsProfiling()) {
-if (Args.hasArg(options::OPT_static) ||
-Args.hasArg(options::OPT_object) ||
-Args.hasArg(options::OPT_preload)) {
-  CmdArgs.push_back("-lgcrt0.o");
-} else {
-  CmdArgs.push_back("-lgcrt1.o");
+if (isTargetMacOS() && isMacosxVersionLT(10, 9)) {
+  if (Args.hasArg(options::OPT_static) ||
+  Args.hasArg(options::OPT_object) ||
+  Args.hasArg(options::OPT_preload)) {
+CmdArgs.push_back("-lgcrt0.o");
+  } else {
+CmdArgs.push_back("-lgcrt1.o");
 
-  // darwin_crt2 spec is empty.
+// darwin_crt2 spec is empty.
+  }
+  // By default on OS X 10.8 and later, we don't link with a crt1.o
+  // file and the linker knows to use _main as the entry point.  But,
+  // when compiling with -pg, we need to link with the gcrt1.o file,
+  // so pass the -no_new_main option to tell the linker to use the
+  // "start" symbol as the entry point.
+  if (isTargetMacOS() && !isMacosxVersionLT(10, 8))
+CmdArgs.push_back("-no_new_main");
+} else {
+  getDriver().Diag(diag::err_drv_clang_unsupported_opt_pg_darwin)
+  << isTargetMacOS();
 }
-// By default on OS X 10.8 and later, we don't link with a crt1.o
-// file and the linker knows to use _main as the entry point.  But,
-// when compiling with -pg, we need to link with the gcrt1.o file,
-// so pass the -no_new_main option to tell the linker to use the
-// "start" symbol as the entry point.
-if (isTargetMacOS() && !isMacosxVersionLT(10, 8))
-  CmdArgs.push_back("-no_new_main");
   } else {
 if (Args.hasArg(options::OPT_static) ||
 Args.hasArg(options::OPT_object) ||

Modified: cfe/trunk/test/Driver/darwin-ld.c
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Driver/darwin-ld.c?rev=354084&r1=354083&r2=354084&view=diff
==
--- cfe/trunk/test/Driver/darwin-ld.c (original)
+++ cfe/trunk/test/Driver/darwin-ld.c Thu Feb 14 15:50:44 2019
@@ -203,6 +203,14 @@
 // LINK_PG: -lgcrt1.o
 // LINK_PG: -no_new_main
 
+// RUN: %clang -target i386-apple-darwin13 -pg -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=LINK_PG_NO_SUPPORT_OSX %s < %t.log
+// LINK_PG_NO_SUPPORT_OSX: error: the clang compiler does not support -pg 
option on versions of OS X
+
+// RUN: %clang -target x86_64-apple-ios5.0 -pg -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=LINK_PG_NO_SUPPORT %s < %t.log
+// LINK_PG_NO_SUPPORT: error: the clang compiler does not support -pg option 
on Darwin
+
 // Check that clang links with libgcc_s.1 for iOS 4 and earlier, but not arm64.
 // RUN: %clang -target arm

[PATCH] D57991: [Driver][Darwin] Emit an error when using -pg on OS without support for it.

[Volodymyr Sapsai via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rL354084: [Driver][Darwin] Emit an error when using -pg on OS 
without support for it. (authored by vsapsai, committed by ).
Herald added a project: LLVM.
Herald added a subscriber: llvm-commits.

Changed prior to commit:
  https://reviews.llvm.org/D57991?vs=186522&id=186935#toc

Repository:
  rL LLVM

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

https://reviews.llvm.org/D57991

Files:
  cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
  cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
  cfe/trunk/test/Driver/darwin-ld.c


Index: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
@@ -2289,22 +2289,27 @@
   }
 } else {
   if (Args.hasArg(options::OPT_pg) && SupportsProfiling()) {
-if (Args.hasArg(options::OPT_static) ||
-Args.hasArg(options::OPT_object) ||
-Args.hasArg(options::OPT_preload)) {
-  CmdArgs.push_back("-lgcrt0.o");
-} else {
-  CmdArgs.push_back("-lgcrt1.o");
+if (isTargetMacOS() && isMacosxVersionLT(10, 9)) {
+  if (Args.hasArg(options::OPT_static) ||
+  Args.hasArg(options::OPT_object) ||
+  Args.hasArg(options::OPT_preload)) {
+CmdArgs.push_back("-lgcrt0.o");
+  } else {
+CmdArgs.push_back("-lgcrt1.o");
 
-  // darwin_crt2 spec is empty.
+// darwin_crt2 spec is empty.
+  }
+  // By default on OS X 10.8 and later, we don't link with a crt1.o
+  // file and the linker knows to use _main as the entry point.  But,
+  // when compiling with -pg, we need to link with the gcrt1.o file,
+  // so pass the -no_new_main option to tell the linker to use the
+  // "start" symbol as the entry point.
+  if (isTargetMacOS() && !isMacosxVersionLT(10, 8))
+CmdArgs.push_back("-no_new_main");
+} else {
+  getDriver().Diag(diag::err_drv_clang_unsupported_opt_pg_darwin)
+  << isTargetMacOS();
 }
-// By default on OS X 10.8 and later, we don't link with a crt1.o
-// file and the linker knows to use _main as the entry point.  But,
-// when compiling with -pg, we need to link with the gcrt1.o file,
-// so pass the -no_new_main option to tell the linker to use the
-// "start" symbol as the entry point.
-if (isTargetMacOS() && !isMacosxVersionLT(10, 8))
-  CmdArgs.push_back("-no_new_main");
   } else {
 if (Args.hasArg(options::OPT_static) ||
 Args.hasArg(options::OPT_object) ||
Index: cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
===
--- cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
+++ cfe/trunk/include/clang/Basic/DiagnosticDriverKinds.td
@@ -96,6 +96,8 @@
   "the clang compiler does not support '%0'">;
 def err_drv_clang_unsupported_opt_cxx_darwin_i386 : Error<
   "the clang compiler does not support '%0' for C++ on Darwin/i386">;
+def err_drv_clang_unsupported_opt_pg_darwin: Error<
+  "the clang compiler does not support -pg option on %select{Darwin|versions 
of OS X 10.9 and later}0">;
 def err_drv_clang_unsupported_opt_faltivec : Error<
   "the clang compiler does not support '%0', %1">;
 def err_drv_command_failed : Error<
Index: cfe/trunk/test/Driver/darwin-ld.c
===
--- cfe/trunk/test/Driver/darwin-ld.c
+++ cfe/trunk/test/Driver/darwin-ld.c
@@ -203,6 +203,14 @@
 // LINK_PG: -lgcrt1.o
 // LINK_PG: -no_new_main
 
+// RUN: %clang -target i386-apple-darwin13 -pg -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=LINK_PG_NO_SUPPORT_OSX %s < %t.log
+// LINK_PG_NO_SUPPORT_OSX: error: the clang compiler does not support -pg 
option on versions of OS X
+
+// RUN: %clang -target x86_64-apple-ios5.0 -pg -### %t.o 2> %t.log
+// RUN: FileCheck -check-prefix=LINK_PG_NO_SUPPORT %s < %t.log
+// LINK_PG_NO_SUPPORT: error: the clang compiler does not support -pg option 
on Darwin
+
 // Check that clang links with libgcc_s.1 for iOS 4 and earlier, but not arm64.
 // RUN: %clang -target armv7-apple-ios4.0 -miphoneos-version-min=4.0 -### %t.o 
2> %t.log
 // RUN: FileCheck -check-prefix=LINK_IOS_LIBGCC_S %s < %t.log


Index: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
===
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
@@ -2289,22 +2289,27 @@
   }
 } else {
   if (Args.hasArg(options::OPT_pg) && SupportsProfiling()) {
-if (Args.hasArg(options::OPT_static) ||
-Args.hasArg(options::OPT_object) ||
-Args.hasArg(options::OPT_preload)) {
-

[PATCH] D57270: [ObjC] Fix non-canonical types preventing type arguments substitution.

[Erik Pilkington via Phabricator via cfe-commits]

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

LGTM!


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

https://reviews.llvm.org/D57270



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

[PATCH] D57075: [ObjC] For type substitution in generics use a regular recursive type visitor.

[Erik Pilkington via Phabricator via cfe-commits]

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

LGTM too, I agree this should be NFC with the parent commit.


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

https://reviews.llvm.org/D57075



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

r354089 - Fixed failure on Darwin due to r354064

[Ana Pazos via cfe-commits]

Author: apazos
Date: Thu Feb 14 16:19:45 2019
New Revision: 354089

URL: http://llvm.org/viewvc/llvm-project?rev=354089&view=rev
Log:
Fixed failure on Darwin due to r354064

Summary:
instrprof-darwin-exports.c test fails on Darwin due  to r354064.

Updated clang list of exported symbols to fix the issue.

Reviewers: vsk

Reviewed By: vsk

Subscribers: davidxl, efriedma

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

Modified:
cfe/trunk/lib/Driver/ToolChains/Darwin.cpp

Modified: cfe/trunk/lib/Driver/ToolChains/Darwin.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Driver/ToolChains/Darwin.cpp?rev=354089&r1=354088&r2=354089&view=diff
==
--- cfe/trunk/lib/Driver/ToolChains/Darwin.cpp (original)
+++ cfe/trunk/lib/Driver/ToolChains/Darwin.cpp Thu Feb 14 16:19:45 2019
@@ -1048,7 +1048,6 @@ void Darwin::addProfileRTLibs(const ArgL
   addExportedSymbol(CmdArgs, "___llvm_profile_filename");
   addExportedSymbol(CmdArgs, "___llvm_profile_raw_version");
   addExportedSymbol(CmdArgs, "_lprofCurFilename");
-  addExportedSymbol(CmdArgs, "_lprofMergeValueProfData");
 }
 addExportedSymbol(CmdArgs, "_lprofDirMode");
   }


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

Re: r354074 - [Sema] Fix-up a -Wfloat-conversion diagnostic

[Shoaib Meenai via cfe-commits]

I don't think Hans ended up getting added to the recipients? (Unless Outlook is 
being silly or it was a BCC, in which case I apologize for the spam.)

On 2/14/19, 3:16 PM, "cfe-commits on behalf of Erik Pilkington via 
cfe-commits"  wrote:

Hans, can you merge this diagnostic regression fix into LLVM 8?

Thanks!

> On Feb 14, 2019, at 2:48 PM, Erik Pilkington via cfe-commits 
 wrote:
> 
> Author: epilk
> Date: Thu Feb 14 14:48:01 2019
> New Revision: 354074
> 
> URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject-3Frev-3D354074-26view-3Drev&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=DO5cOhG2p0lJyoGW_0kLao-RuuAsjuXhKcexb8Du4S8&s=pHB5-cVv8xjeI6USEYPO7XIpvGQena6iXejLqlviUes&e=
> Log:
> [Sema] Fix-up a -Wfloat-conversion diagnostic
> 
> We were warning on valid ObjC property reference exprs, and passing
> in the wrong arguments to DiagnoseFloatingImpCast (leading to a badly
> worded diagnostic).
> 
> rdar://47644670
> 
> Differential revision: 
https://urldefense.proofpoint.com/v2/url?u=https-3A__reviews.llvm.org_D58145&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=DO5cOhG2p0lJyoGW_0kLao-RuuAsjuXhKcexb8Du4S8&s=ohcFH9VMyZgBARLdvdnbJXaIkchLNxHzkt9R3y_Dy4E&e=
> 
> Modified:
>cfe/trunk/lib/Sema/SemaChecking.cpp
>cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
>cfe/trunk/test/SemaObjC/conversion.m
> 
> Modified: cfe/trunk/lib/Sema/SemaChecking.cpp
> URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_lib_Sema_SemaChecking.cpp-3Frev-3D354074-26r1-3D354073-26r2-3D354074-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=DO5cOhG2p0lJyoGW_0kLao-RuuAsjuXhKcexb8Du4S8&s=noPXZAz2o1d7SWaLkeUQGcFR9mRffBwYz07dWx-vUsM&e=
> 
==
> --- cfe/trunk/lib/Sema/SemaChecking.cpp (original)
> +++ cfe/trunk/lib/Sema/SemaChecking.cpp Thu Feb 14 14:48:01 2019
> @@ -10624,16 +10624,16 @@ static void AnalyzeCompoundAssignment(Se
>   // The below checks assume source is floating point.
>   if (!ResultBT || !RBT || !RBT->isFloatingPoint()) return;
> 
> -  // If source is floating point but target is not.
> -  if (!ResultBT->isFloatingPoint())
> -return DiagnoseFloatingImpCast(S, E, E->getRHS()->getType(),
> -   E->getExprLoc());
> -
> -  // If both source and target are floating points.
> -  // Builtin FP kinds are ordered by increasing FP rank.
> -  if (ResultBT->getKind() < RBT->getKind() &&
> -  // We don't want to warn for system macro.
> -  !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
> +  // If source is floating point but target is an integer.
> +  if (ResultBT->isInteger())
> +DiagnoseImpCast(S, E, E->getRHS()->getType(), E->getLHS()->getType(),
> +E->getExprLoc(), diag::warn_impcast_float_integer);
> +  // If both source and target are floating points. Builtin FP kinds are 
ordered
> +  // by increasing FP rank. FIXME: except _Float16, we currently emit a 
bogus
> +  // warning.
> +  else if (ResultBT->isFloatingPoint() && ResultBT->getKind() < 
RBT->getKind() &&
> +   // We don't want to warn for system macro.
> +   !S.SourceMgr.isInSystemMacro(E->getOperatorLoc()))
> // warn about dropping FP rank.
> DiagnoseImpCast(S, E->getRHS(), E->getLHS()->getType(), 
E->getOperatorLoc(),
> diag::warn_impcast_float_result_precision);
> 
> Modified: cfe/trunk/test/SemaCXX/warn-float-conversion.cpp
> URL: 
https://urldefense.proofpoint.com/v2/url?u=http-3A__llvm.org_viewvc_llvm-2Dproject_cfe_trunk_test_SemaCXX_warn-2Dfloat-2Dconversion.cpp-3Frev-3D354074-26r1-3D354073-26r2-3D354074-26view-3Ddiff&d=DwIGaQ&c=5VD0RTtNlTh3ycd41b3MUw&r=o3kDXzdBUE3ljQXKeTWOMw&m=DO5cOhG2p0lJyoGW_0kLao-RuuAsjuXhKcexb8Du4S8&s=LPTdxv5StV8_bmyhdyjrJ6m4L5puzINH3F5R2azFXIc&e=
> 
==
> --- cfe/trunk/test/SemaCXX/warn-float-conversion.cpp (original)
> +++ cfe/trunk/test/SemaCXX/warn-float-conversion.cpp Thu Feb 14 14:48:01 
2019
> @@ -44,17 +44,17 @@ void Convert(float f, double d, long dou
> void CompoundAssignment() {
>   int x = 3;
> 
> -  x += 1.234;  //expected-warning{{conversion}}
> -  x -= -0.0;  //expected-warning{{conversion}}
> -  x *= 1.1f;  //expected-warning{{conversion}}
> -  x /= -2.2f;  //expected-warning{{conversion}}
> +  x += 1.234; // expected-warning {{implicit conversion turns 
floating-point number into integer: 'double' to 'int'}}
> +  x -= -0.0;  // expected-warning {{implicit conversion turns 
floating-point number into integer: 'double' to '

r354090 - PR40642: Fix determination of whether the final statement of a statement

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Feb 14 16:27:53 2019
New Revision: 354090

URL: http://llvm.org/viewvc/llvm-project?rev=354090&view=rev
Log:
PR40642: Fix determination of whether the final statement of a statement
expression is a discarded-value expression.

Summary:
We used to get this wrong in three ways:

1) During parsing, an expression-statement followed by the }) ending a
   statement expression was always treated as producing the value of the
   statement expression. That's wrong for ({ if (1) expr; })
2) During template instantiation, various kinds of statement (most
   statements not appearing directly in a compound-statement) were not
   treated as discarded-value expressions, resulting in missing volatile
   loads (etc).
3) In all contexts, an expression-statement with attributes was not
   treated as producing the value of the statement expression, eg
   ({ [[attr]] expr; }).

Also fix incorrect enforcement of OpenMP rule that directives can "only
be placed in the program at a position where ignoring or deleting the
directive would result in a program with correct syntax". In particular,
a label (be it goto, case, or default) should not affect whether
directives are permitted.

Reviewers: aaron.ballman, rjmccall

Subscribers: cfe-commits

Tags: #clang

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

Modified:
cfe/trunk/include/clang/AST/Expr.h
cfe/trunk/include/clang/AST/Stmt.h
cfe/trunk/include/clang/Basic/StmtNodes.td
cfe/trunk/include/clang/Parse/Parser.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/AST/Stmt.cpp
cfe/trunk/lib/CodeGen/CGStmt.cpp
cfe/trunk/lib/Parse/ParseObjc.cpp
cfe/trunk/lib/Parse/ParseOpenMP.cpp
cfe/trunk/lib/Parse/ParseStmt.cpp
cfe/trunk/lib/Sema/SemaExpr.cpp
cfe/trunk/lib/Sema/SemaStmt.cpp
cfe/trunk/lib/Sema/TreeTransform.h
cfe/trunk/test/CodeGenCXX/stmtexpr.cpp
cfe/trunk/test/CodeGenCXX/volatile.cpp
cfe/trunk/test/OpenMP/barrier_messages.cpp
cfe/trunk/test/OpenMP/cancel_messages.cpp
cfe/trunk/test/OpenMP/cancellation_point_messages.cpp
cfe/trunk/test/OpenMP/flush_messages.cpp
cfe/trunk/test/OpenMP/taskwait_messages.cpp
cfe/trunk/test/OpenMP/taskyield_messages.cpp

Modified: cfe/trunk/include/clang/AST/Expr.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Expr.h?rev=354090&r1=354089&r2=354090&view=diff
==
--- cfe/trunk/include/clang/AST/Expr.h (original)
+++ cfe/trunk/include/clang/AST/Expr.h Thu Feb 14 16:27:53 2019
@@ -105,13 +105,13 @@ struct SubobjectAdjustment {
 /// This represents one expression.  Note that Expr's are subclasses of Stmt.
 /// This allows an expression to be transparently used any place a Stmt is
 /// required.
-class Expr : public Stmt {
+class Expr : public ValueStmt {
   QualType TR;
 
 protected:
   Expr(StmtClass SC, QualType T, ExprValueKind VK, ExprObjectKind OK,
bool TD, bool VD, bool ID, bool ContainsUnexpandedParameterPack)
-: Stmt(SC)
+: ValueStmt(SC)
   {
 ExprBits.TypeDependent = TD;
 ExprBits.ValueDependent = VD;
@@ -124,7 +124,7 @@ protected:
   }
 
   /// Construct an empty expression.
-  explicit Expr(StmtClass SC, EmptyShell) : Stmt(SC) { }
+  explicit Expr(StmtClass SC, EmptyShell) : ValueStmt(SC) { }
 
 public:
   QualType getType() const { return TR; }

Modified: cfe/trunk/include/clang/AST/Stmt.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/AST/Stmt.h?rev=354090&r1=354089&r2=354090&view=diff
==
--- cfe/trunk/include/clang/AST/Stmt.h (original)
+++ cfe/trunk/include/clang/AST/Stmt.h Thu Feb 14 16:27:53 2019
@@ -1584,21 +1584,44 @@ Stmt *SwitchCase::getSubStmt() {
   llvm_unreachable("SwitchCase is neither a CaseStmt nor a DefaultStmt!");
 }
 
+/// Represents a statement that could possibly have a value and type. This
+/// covers expression-statements, as well as labels and attributed statements.
+///
+/// Value statements have a special meaning when they are the last non-null
+/// statement in a GNU statement expression, where they determine the value
+/// of the statement expression.
+class ValueStmt : public Stmt {
+protected:
+  using Stmt::Stmt;
+
+public:
+  const Expr *getExprStmt() const;
+  Expr *getExprStmt() {
+const ValueStmt *ConstThis = this;
+return const_cast(ConstThis->getExprStmt());
+  }
+
+  static bool classof(const Stmt *T) {
+return T->getStmtClass() >= firstValueStmtConstant &&
+   T->getStmtClass() <= lastValueStmtConstant;
+  }
+};
+
 /// LabelStmt - Represents a label, which has a substatement.  For example:
 ///foo: return;
-class LabelStmt : public Stmt {
+class LabelStmt : public ValueStmt {
   LabelDecl *TheDecl;
   Stmt *SubStmt;
 
 public:
   /// Build a label statement.
   LabelStmt(SourceLocation IL, LabelDecl *D, Stmt *substmt)
-  : Stmt(LabelStmtClass), TheDecl(

r354091 - Fix implementation of [temp.local]p4.

[Richard Smith via cfe-commits]

Author: rsmith
Date: Thu Feb 14 16:29:04 2019
New Revision: 354091

URL: http://llvm.org/viewvc/llvm-project?rev=354091&view=rev
Log:
Fix implementation of [temp.local]p4.

When a template-name is looked up, we need to give injected-class-name
declarations of class templates special treatment, as they denote a
template rather than a type.

Previously we achieved this by applying a filter to the lookup results
after completing name lookup, but that is incorrect in various ways, not
least of which is that it lost all information about access and how
members were named, and the filtering caused us to generally lose
all ambiguity errors between templates and non-templates.

We now preserve the lookup results exactly, and the few places that need
to map from a declaration found by name lookup into a declaration of a
template do so explicitly. Deduplication of repeated lookup results of
the same injected-class-name declaration is done by name lookup instead
of after the fact.

Modified:
cfe/trunk/include/clang/Sema/Lookup.h
cfe/trunk/include/clang/Sema/Sema.h
cfe/trunk/lib/Sema/SemaDecl.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/lib/Sema/SemaLookup.cpp
cfe/trunk/lib/Sema/SemaTemplate.cpp
cfe/trunk/test/CXX/class.access/p4.cpp
cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p1.cpp
cfe/trunk/test/SemaTemplate/temp.cpp

Modified: cfe/trunk/include/clang/Sema/Lookup.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=354091&r1=354090&r2=354091&view=diff
==
--- cfe/trunk/include/clang/Sema/Lookup.h (original)
+++ cfe/trunk/include/clang/Sema/Lookup.h Thu Feb 14 16:29:04 2019
@@ -172,7 +172,8 @@ public:
   : SemaPtr(Other.SemaPtr), NameInfo(Other.NameInfo),
 LookupKind(Other.LookupKind), IDNS(Other.IDNS), Redecl(Other.Redecl),
 ExternalRedecl(Other.ExternalRedecl), HideTags(Other.HideTags),
-AllowHidden(Other.AllowHidden) {}
+AllowHidden(Other.AllowHidden),
+TemplateNameLookup(Other.TemplateNameLookup) {}
 
   // FIXME: Remove these deleted methods once the default build includes
   // -Wdeprecated.
@@ -193,7 +194,8 @@ public:
 HideTags(std::move(Other.HideTags)),
 Diagnose(std::move(Other.Diagnose)),
 AllowHidden(std::move(Other.AllowHidden)),
-Shadowed(std::move(Other.Shadowed)) {
+Shadowed(std::move(Other.Shadowed)),
+TemplateNameLookup(std::move(Other.TemplateNameLookup)) {
 Other.Paths = nullptr;
 Other.Diagnose = false;
   }
@@ -216,6 +218,7 @@ public:
 Diagnose = std::move(Other.Diagnose);
 AllowHidden = std::move(Other.AllowHidden);
 Shadowed = std::move(Other.Shadowed);
+TemplateNameLookup = std::move(Other.TemplateNameLookup);
 Other.Paths = nullptr;
 Other.Diagnose = false;
 return *this;
@@ -286,6 +289,15 @@ public:
 HideTags = Hide;
   }
 
+  /// Sets whether this is a template-name lookup. For template-name lookups,
+  /// injected-class-names are treated as naming a template rather than a
+  /// template specialization.
+  void setTemplateNameLookup(bool TemplateName) {
+TemplateNameLookup = TemplateName;
+  }
+
+  bool isTemplateNameLookup() const { return TemplateNameLookup; }
+
   bool isAmbiguous() const {
 return getResultKind() == Ambiguous;
   }
@@ -739,6 +751,9 @@ private:
   /// declaration that we skipped. This only happens when \c LookupKind
   /// is \c LookupRedeclarationWithLinkage.
   bool Shadowed = false;
+
+  /// True if we're looking up a template-name.
+  bool TemplateNameLookup = false;
 };
 
 /// Consumes visible declarations found when searching for

Modified: cfe/trunk/include/clang/Sema/Sema.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=354091&r1=354090&r2=354091&view=diff
==
--- cfe/trunk/include/clang/Sema/Sema.h (original)
+++ cfe/trunk/include/clang/Sema/Sema.h Thu Feb 14 16:29:04 2019
@@ -6212,9 +6212,21 @@ public:
   // C++ Templates [C++ 14]
   //
   void FilterAcceptableTemplateNames(LookupResult &R,
- bool AllowFunctionTemplates = true);
+ bool AllowFunctionTemplates = true,
+ bool AllowDependent = true);
   bool hasAnyAcceptableTemplateNames(LookupResult &R,
- bool AllowFunctionTemplates = true);
+ bool AllowFunctionTemplates = true,
+ bool AllowDependent = true);
+  /// Try to interpret the lookup result D as a template-name.
+  ///
+  /// \param D A declaration found by name lookup.
+  /// \param AllowFunctionTemplates Whether function templates should be
+  ///considered valid results.
+  /// \param AllowDependent Whether unresolved using declaratio

[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 186944.
nridge added a comment.

- Rework tests to test the lower-level functions like typeAncestors()
- Remove support for typeHierarchy/resolve


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/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 &>
@@ -1389,6 +1404,337 @@
   }
 }
 
+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);
+EXPECT_EQ(nullptr, RD);
+  }
+}
+
+// TODO: FindRecordTypeAt, TemplateSpec
+
+TEST(TypeAncestors, 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"));
+
+  ASTContext &Ctx = AST.getASTContext();
+
+  EXPECT_THAT(typeAncestors(Ctx, Parent), ElementsAre());
+  EXPECT_THAT(typeAncestors(Ctx, Child1), ElementsAre(Parent));
+  EXPECT_THAT(typeAncestors(Ctx, Child2), ElementsAre(Child1));
+}
+
+TEST(TypeAncestors, MultipleInheritance) {
+  Annotations Source(R"cpp(
+struct Parent1 {
+  int a;
+};
+
+struct Parent2 {
+  int b;
+};
+
+struct Parent3 : Parent2 {
+  int c;
+};
+
+struct Child : Parent1, Parent3 {
+  int d;
+};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST

[PATCH] D53633: [AArch64] Implement FP16FML intrinsics

[Ahmed Bougacha via Phabricator via cfe-commits]

ab added inline comments.
Herald added a subscriber: jdoerfert.
Herald added a project: LLVM.



Comment at: cfe/trunk/test/CodeGen/aarch64-neon-fp16fml.c:12
+
+float32x2_t test_vfmlal_low_u32(float32x2_t a, float16x4_t b, float16x4_t c) {
+// CHECK-LABEL: define <2 x float> @test_vfmlal_low_u32(<2 x float> %a, <4 x 
half> %b, <4 x half> %c)

Hey folks, I'm curious: where does the "_u32" suffix come from? Should it be 
_f16?

Also, are there any new ACLE/intrinsic list documents? As far as I can tell 
there hasn't been any release since IHI0073B/IHI0053D.


Repository:
  rL LLVM

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

https://reviews.llvm.org/D53633



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

[PATCH] D57984: PR40642: Fix determination of whether the final statement of a statementexpression is a discarded-value expression.

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

This revision was automatically updated to reflect the committed changes.
Closed by commit rC354090: PR40642: Fix determination of whether the final 
statement of a statement (authored by rsmith, committed by ).

Changed prior to commit:
  https://reviews.llvm.org/D57984?vs=186731&id=186945#toc

Repository:
  rC Clang

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

https://reviews.llvm.org/D57984

Files:
  include/clang/AST/Expr.h
  include/clang/AST/Stmt.h
  include/clang/Basic/StmtNodes.td
  include/clang/Parse/Parser.h
  include/clang/Sema/Sema.h
  lib/AST/Stmt.cpp
  lib/CodeGen/CGStmt.cpp
  lib/Parse/ParseObjc.cpp
  lib/Parse/ParseOpenMP.cpp
  lib/Parse/ParseStmt.cpp
  lib/Sema/SemaExpr.cpp
  lib/Sema/SemaStmt.cpp
  lib/Sema/TreeTransform.h
  test/CodeGenCXX/stmtexpr.cpp
  test/CodeGenCXX/volatile.cpp
  test/OpenMP/barrier_messages.cpp
  test/OpenMP/cancel_messages.cpp
  test/OpenMP/cancellation_point_messages.cpp
  test/OpenMP/flush_messages.cpp
  test/OpenMP/taskwait_messages.cpp
  test/OpenMP/taskyield_messages.cpp

Index: include/clang/AST/Stmt.h
===
--- include/clang/AST/Stmt.h
+++ include/clang/AST/Stmt.h
@@ -1584,21 +1584,44 @@
   llvm_unreachable("SwitchCase is neither a CaseStmt nor a DefaultStmt!");
 }
 
+/// Represents a statement that could possibly have a value and type. This
+/// covers expression-statements, as well as labels and attributed statements.
+///
+/// Value statements have a special meaning when they are the last non-null
+/// statement in a GNU statement expression, where they determine the value
+/// of the statement expression.
+class ValueStmt : public Stmt {
+protected:
+  using Stmt::Stmt;
+
+public:
+  const Expr *getExprStmt() const;
+  Expr *getExprStmt() {
+const ValueStmt *ConstThis = this;
+return const_cast(ConstThis->getExprStmt());
+  }
+
+  static bool classof(const Stmt *T) {
+return T->getStmtClass() >= firstValueStmtConstant &&
+   T->getStmtClass() <= lastValueStmtConstant;
+  }
+};
+
 /// LabelStmt - Represents a label, which has a substatement.  For example:
 ///foo: return;
-class LabelStmt : public Stmt {
+class LabelStmt : public ValueStmt {
   LabelDecl *TheDecl;
   Stmt *SubStmt;
 
 public:
   /// Build a label statement.
   LabelStmt(SourceLocation IL, LabelDecl *D, Stmt *substmt)
-  : Stmt(LabelStmtClass), TheDecl(D), SubStmt(substmt) {
+  : ValueStmt(LabelStmtClass), TheDecl(D), SubStmt(substmt) {
 setIdentLoc(IL);
   }
 
   /// Build an empty label statement.
-  explicit LabelStmt(EmptyShell Empty) : Stmt(LabelStmtClass, Empty) {}
+  explicit LabelStmt(EmptyShell Empty) : ValueStmt(LabelStmtClass, Empty) {}
 
   SourceLocation getIdentLoc() const { return LabelStmtBits.IdentLoc; }
   void setIdentLoc(SourceLocation L) { LabelStmtBits.IdentLoc = L; }
@@ -1627,7 +1650,7 @@
 /// Represents an attribute applied to a statement. For example:
 ///   [[omp::for(...)]] for (...) { ... }
 class AttributedStmt final
-: public Stmt,
+: public ValueStmt,
   private llvm::TrailingObjects {
   friend class ASTStmtReader;
   friend TrailingObjects;
@@ -1636,14 +1659,14 @@
 
   AttributedStmt(SourceLocation Loc, ArrayRef Attrs,
  Stmt *SubStmt)
-  : Stmt(AttributedStmtClass), SubStmt(SubStmt) {
+  : ValueStmt(AttributedStmtClass), SubStmt(SubStmt) {
 AttributedStmtBits.NumAttrs = Attrs.size();
 AttributedStmtBits.AttrLoc = Loc;
 std::copy(Attrs.begin(), Attrs.end(), getAttrArrayPtr());
   }
 
   explicit AttributedStmt(EmptyShell Empty, unsigned NumAttrs)
-  : Stmt(AttributedStmtClass, Empty) {
+  : ValueStmt(AttributedStmtClass, Empty) {
 AttributedStmtBits.NumAttrs = NumAttrs;
 AttributedStmtBits.AttrLoc = SourceLocation{};
 std::fill_n(getAttrArrayPtr(), NumAttrs, nullptr);
Index: include/clang/AST/Expr.h
===
--- include/clang/AST/Expr.h
+++ include/clang/AST/Expr.h
@@ -105,13 +105,13 @@
 /// This represents one expression.  Note that Expr's are subclasses of Stmt.
 /// This allows an expression to be transparently used any place a Stmt is
 /// required.
-class Expr : public Stmt {
+class Expr : public ValueStmt {
   QualType TR;
 
 protected:
   Expr(StmtClass SC, QualType T, ExprValueKind VK, ExprObjectKind OK,
bool TD, bool VD, bool ID, bool ContainsUnexpandedParameterPack)
-: Stmt(SC)
+: ValueStmt(SC)
   {
 ExprBits.TypeDependent = TD;
 ExprBits.ValueDependent = VD;
@@ -124,7 +124,7 @@
   }
 
   /// Construct an empty expression.
-  explicit Expr(StmtClass SC, EmptyShell) : Stmt(SC) { }
+  explicit Expr(StmtClass SC, EmptyShell) : ValueStmt(SC) { }
 
 public:
   QualType getType() const { return TR; }
Index: include/clang/Sema/Sema.h
===
--- include/clang/Sema/Sema.h
+++ include/clang/Sema/Sema.h
@@ -1403,7 +1403,6 @@
   vo

[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/unittests/clangd/XRefsTests.cpp:1604
+  dyn_cast(&findDecl(AST, 
"Parent"))->getTemplatedDecl();
+  // TODO: This fails because findDecl() doesn't support template-ids
+  // const CXXRecordDecl *ParentSpec =

Any suggestions for how we can make `findDecl()` support //template-id//s?


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

[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 186947.
nridge added a comment.

Fix a test failure


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 &>
@@ -1389,6 +1404,337 @@
   }
 }
 
+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);
+EXPECT_EQ(nullptr, RD);
+  }
+}
+
+// TODO: FindRecordTypeAt, TemplateSpec
+
+TEST(TypeAncestors, 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"));
+
+  ASTContext &Ctx = AST.getASTContext();
+
+  EXPECT_THAT(typeAncestors(Ctx, Parent), ElementsAre());
+  EXPECT_THAT(typeAncestors(Ctx, Child1), ElementsAre(Parent));
+  EXPECT_THAT(typeAncestors(Ctx, Child2), ElementsAre(Child1));
+}
+
+TEST(TypeAncestors, MultipleInheritance) {
+  Annotations Source(R"cpp(
+struct Parent1 {
+  int a;
+};
+
+struct Parent2 {
+  int b;
+};
+
+struct Parent3 : Parent2 {
+  int c;
+};
+
+struct Child : Parent1, Parent3 {
+  int d;
+};
+)cpp");
+
+  TestTU TU = TestTU::withCode(Source.code());
+  auto AST = TU.build();
+
+  ASSERT_TRUE(AST.get

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

[JF Bastien via Phabricator via cfe-commits]

jfb updated this revision to Diff 186950.
jfb added a comment.

- Fix polarity


Repository:
  rC Clang

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

https://reviews.llvm.org/D58218

Files:
  lib/CodeGen/CGDecl.cpp
  test/CodeGenCXX/trivial-auto-var-init.cpp


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -45,14 +45,35 @@
 // PATTERN:   %captured1 = getelementptr inbounds 
%struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 
0, i32 4
 // PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to 
%struct.XYZ*), %struct.XYZ** %captured1, align 8
 // PATTERN:   %call = call %struct.XYZ* @create(
+using Block = void (^)();
+typedef struct XYZ {
+  Block block;
+} * xyz_t;
 void test_block_self_init() {
-  using Block = void (^)();
-  typedef struct XYZ {
-Block block;
-  } * xyz_t;
   extern xyz_t create(Block block);
   __block xyz_t captured = create(^() {
-(void)captured;
+used(captured);
+  });
+}
+
+// Capturing with escape after initialization is also an edge case.
+//
+// UNINIT-LABEL:  test_block_captures_self_after_init(
+// ZERO-LABEL:test_block_captures_self_after_init(
+// ZERO:  %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// ZERO:  %captured1 = getelementptr inbounds 
%struct.__block_byref_captured.1, %struct.__block_byref_captured.1* %captured, 
i32 0, i32 4
+// ZERO-NEXT: store %struct.XYZ* null, %struct.XYZ** %captured1, align 8
+// ZERO:  %call = call %struct.XYZ* @create(
+// PATTERN-LABEL: test_block_captures_self_after_init(
+// PATTERN:   %block = alloca <{ i8*, i32, i32, i8*, 
%struct.__block_descriptor*, i8* }>, align 8
+// PATTERN:   %captured1 = getelementptr inbounds 
%struct.__block_byref_captured.1, %struct.__block_byref_captured.1* %captured, 
i32 0, i32 4
+// PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to 
%struct.XYZ*), %struct.XYZ** %captured1, align 8
+// PATTERN:   %call = call %struct.XYZ* @create(
+void test_block_captures_self_after_init() {
+  extern xyz_t create(Block block);
+  __block xyz_t captured;
+  captured = create(^() {
+used(captured);
   });
 }
 
Index: lib/CodeGen/CGDecl.cpp
===
--- lib/CodeGen/CGDecl.cpp
+++ lib/CodeGen/CGDecl.cpp
@@ -1620,8 +1620,9 @@
   bool capturedByInit =
   Init && emission.IsEscapingByRef && isCapturedBy(D, Init);
 
-  Address Loc =
-  capturedByInit ? emission.Addr : emission.getObjectAddress(*this);
+  bool locIsByrefHeader = !capturedByInit;
+  const Address Loc =
+  locIsByrefHeader ? emission.getObjectAddress(*this) : emission.Addr;
 
   // Note: constexpr already initializes everything correctly.
   LangOptions::TrivialAutoVarInitKind trivialAutoVarInit =
@@ -1637,7 +1638,7 @@
   return;
 
 // Only initialize a __block's storage: we always initialize the header.
-if (emission.IsEscapingByRef)
+if (emission.IsEscapingByRef && !locIsByrefHeader)
   Loc = emitBlockByrefAddress(Loc, &D, /*follow=*/false);
 
 CharUnits Size = getContext().getTypeSizeInChars(type);
@@ -1744,10 +1745,9 @@
   }
 
   llvm::Type *BP = CGM.Int8Ty->getPointerTo(Loc.getAddressSpace());
-  if (Loc.getType() != BP)
-Loc = Builder.CreateBitCast(Loc, BP);
-
-  emitStoresForConstant(CGM, D, Loc, isVolatile, Builder, constant);
+  emitStoresForConstant(
+  CGM, D, (Loc.getType() == BP) ? Loc : Builder.CreateBitCast(Loc, BP),
+  isVolatile, Builder, constant);
 }
 
 /// Emit an expression as an initializer for an object (variable, field, etc.)


Index: test/CodeGenCXX/trivial-auto-var-init.cpp
===
--- test/CodeGenCXX/trivial-auto-var-init.cpp
+++ test/CodeGenCXX/trivial-auto-var-init.cpp
@@ -45,14 +45,35 @@
 // PATTERN:   %captured1 = getelementptr inbounds %struct.__block_byref_captured, %struct.__block_byref_captured* %captured, i32 0, i32 4
 // PATTERN-NEXT:  store %struct.XYZ* inttoptr (i64 -6148914691236517206 to %struct.XYZ*), %struct.XYZ** %captured1, align 8
 // PATTERN:   %call = call %struct.XYZ* @create(
+using Block = void (^)();
+typedef struct XYZ {
+  Block block;
+} * xyz_t;
 void test_block_self_init() {
-  using Block = void (^)();
-  typedef struct XYZ {
-Block block;
-  } * xyz_t;
   extern xyz_t create(Block block);
   __block xyz_t captured = create(^() {
-(void)captured;
+used(captured);
+  });
+}
+
+// Capturing with escape after initialization is also an edge case.
+//
+// UNINIT-LABEL:  test_block_captures_self_after_init(
+// ZERO-LABEL:test_block_captures_self_after_init(
+// ZERO:  %block = alloca <{ i8*, i32, i32, i8*, %struct.__block_descriptor*, i8* }>, align 8
+// ZERO:

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

[JF Bastien via Phabricator via cfe-commits]

jfb added a comment.

In D58218#1398251 , @rjmccall wrote:

> In D58218#1398124 , @jfb wrote:
>
> > In D58218#1398096 , @rjmccall 
> > wrote:
> >
> > > Well, you can always make a variable with a more directly-applicable name 
> > > than `capturedByInit` and update it as appropriate, like 
> > > `locIsByrefHeader`.
> >
> >
> > Sounds good. I made it `const` too, to avoid inadvertent modification.
>
>
> This is actually the reverse of the sense I would expect it to have based on 
> the name.  If you want these semantics, maybe `locIsObject`?


You're right, I've fixed the polarity.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58218



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

Re: r354091 - Fix implementation of [temp.local]p4.

[Francis Visoiu Mistrih via cfe-commits]

Hi Richard,

This seems to now emit an error when building the sanitizer tests: 
http://green.lab.llvm.org/green/job/clang-stage1-configure-RA/53965/consoleFull 
.

I managed to reproduce it locally and when reverting your commit the error goes 
away.

I am not sure if the error is in the sanitizer test’s code or actually a 
compiler error. Can you please take a look?

Thanks,

-- 
Francis

> On Feb 14, 2019, at 4:29 PM, Richard Smith via cfe-commits 
>  wrote:
> 
> Author: rsmith
> Date: Thu Feb 14 16:29:04 2019
> New Revision: 354091
> 
> URL: http://llvm.org/viewvc/llvm-project?rev=354091&view=rev
> Log:
> Fix implementation of [temp.local]p4.
> 
> When a template-name is looked up, we need to give injected-class-name
> declarations of class templates special treatment, as they denote a
> template rather than a type.
> 
> Previously we achieved this by applying a filter to the lookup results
> after completing name lookup, but that is incorrect in various ways, not
> least of which is that it lost all information about access and how
> members were named, and the filtering caused us to generally lose
> all ambiguity errors between templates and non-templates.
> 
> We now preserve the lookup results exactly, and the few places that need
> to map from a declaration found by name lookup into a declaration of a
> template do so explicitly. Deduplication of repeated lookup results of
> the same injected-class-name declaration is done by name lookup instead
> of after the fact.
> 
> Modified:
>cfe/trunk/include/clang/Sema/Lookup.h
>cfe/trunk/include/clang/Sema/Sema.h
>cfe/trunk/lib/Sema/SemaDecl.cpp
>cfe/trunk/lib/Sema/SemaDeclCXX.cpp
>cfe/trunk/lib/Sema/SemaLookup.cpp
>cfe/trunk/lib/Sema/SemaTemplate.cpp
>cfe/trunk/test/CXX/class.access/p4.cpp
>cfe/trunk/test/CXX/temp/temp.decls/temp.friend/p1.cpp
>cfe/trunk/test/SemaTemplate/temp.cpp
> 
> Modified: cfe/trunk/include/clang/Sema/Lookup.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Lookup.h?rev=354091&r1=354090&r2=354091&view=diff
> ==
> --- cfe/trunk/include/clang/Sema/Lookup.h (original)
> +++ cfe/trunk/include/clang/Sema/Lookup.h Thu Feb 14 16:29:04 2019
> @@ -172,7 +172,8 @@ public:
>   : SemaPtr(Other.SemaPtr), NameInfo(Other.NameInfo),
> LookupKind(Other.LookupKind), IDNS(Other.IDNS), Redecl(Other.Redecl),
> ExternalRedecl(Other.ExternalRedecl), HideTags(Other.HideTags),
> -AllowHidden(Other.AllowHidden) {}
> +AllowHidden(Other.AllowHidden),
> +TemplateNameLookup(Other.TemplateNameLookup) {}
> 
>   // FIXME: Remove these deleted methods once the default build includes
>   // -Wdeprecated.
> @@ -193,7 +194,8 @@ public:
> HideTags(std::move(Other.HideTags)),
> Diagnose(std::move(Other.Diagnose)),
> AllowHidden(std::move(Other.AllowHidden)),
> -Shadowed(std::move(Other.Shadowed)) {
> +Shadowed(std::move(Other.Shadowed)),
> +TemplateNameLookup(std::move(Other.TemplateNameLookup)) {
> Other.Paths = nullptr;
> Other.Diagnose = false;
>   }
> @@ -216,6 +218,7 @@ public:
> Diagnose = std::move(Other.Diagnose);
> AllowHidden = std::move(Other.AllowHidden);
> Shadowed = std::move(Other.Shadowed);
> +TemplateNameLookup = std::move(Other.TemplateNameLookup);
> Other.Paths = nullptr;
> Other.Diagnose = false;
> return *this;
> @@ -286,6 +289,15 @@ public:
> HideTags = Hide;
>   }
> 
> +  /// Sets whether this is a template-name lookup. For template-name lookups,
> +  /// injected-class-names are treated as naming a template rather than a
> +  /// template specialization.
> +  void setTemplateNameLookup(bool TemplateName) {
> +TemplateNameLookup = TemplateName;
> +  }
> +
> +  bool isTemplateNameLookup() const { return TemplateNameLookup; }
> +
>   bool isAmbiguous() const {
> return getResultKind() == Ambiguous;
>   }
> @@ -739,6 +751,9 @@ private:
>   /// declaration that we skipped. This only happens when \c LookupKind
>   /// is \c LookupRedeclarationWithLinkage.
>   bool Shadowed = false;
> +
> +  /// True if we're looking up a template-name.
> +  bool TemplateNameLookup = false;
> };
> 
> /// Consumes visible declarations found when searching for
> 
> Modified: cfe/trunk/include/clang/Sema/Sema.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Sema/Sema.h?rev=354091&r1=354090&r2=354091&view=diff
> ==
> --- cfe/trunk/include/clang/Sema/Sema.h (original)
> +++ cfe/trunk/include/clang/Sema/Sema.h Thu Feb 14 16:29:04 2019
> @@ -6212,9 +6212,21 @@ public:
>   // C++ Templates [C++ 14]
>   //
>   void FilterAcceptableTemplateNames(LookupResult &R,
> - bool AllowFunctionTemplates 

[PATCH] D58218: Variable auto-init of blocks capturing self after init bugfix

[John McCall via Phabricator via cfe-commits]

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

LGTM.


Repository:
  rC Clang

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

https://reviews.llvm.org/D58218



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

[PATCH] D58268: [clang] Create install targets for non-shared libraries

[Shoaib Meenai via Phabricator via cfe-commits]

smeenai created this revision.
smeenai added reviewers: beanz, phosek.
Herald added a subscriber: mgorny.
Herald added a project: clang.

I don't see a reason for these to not have install targets created,
which in turn allows them to be bundled in distributions. This doesn't
affect the "install" target, since that just runs all CMake install
rules (and we were already creating install rules for these).


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D58268

Files:
  clang/cmake/modules/AddClang.cmake


Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -103,7 +103,7 @@
 ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX}
 RUNTIME DESTINATION bin)
 
-  if (${ARG_SHARED} AND NOT CMAKE_CONFIGURATION_TYPES)
+  if (NOT CMAKE_CONFIGURATION_TYPES)
 add_llvm_install_targets(install-${name}
  DEPENDS ${name}
  COMPONENT ${name})


Index: clang/cmake/modules/AddClang.cmake
===
--- clang/cmake/modules/AddClang.cmake
+++ clang/cmake/modules/AddClang.cmake
@@ -103,7 +103,7 @@
 ARCHIVE DESTINATION lib${LLVM_LIBDIR_SUFFIX}
 RUNTIME DESTINATION bin)
 
-  if (${ARG_SHARED} AND NOT CMAKE_CONFIGURATION_TYPES)
+  if (NOT CMAKE_CONFIGURATION_TYPES)
 add_llvm_install_targets(install-${name}
  DEPENDS ${name}
  COMPONENT ${name})
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits