[PATCH] D115187: [clangd] Expose CoawaitExpr's operand in the AST

[Nathan Ridge via Phabricator via cfe-commits]

nridge added a comment.

In D115187#3192034 , @sammccall wrote:

> A change that probably goes along with this one is having RecursiveASTVisitor 
> traverse the syntactic form instead of the semantic one if traversal of 
> implicit code is off.

I believe that's already the behaviour that falls out of the current 
implementation 
:

  DEF_TRAVERSE_STMT(CoawaitExpr, {
if (!getDerived().shouldVisitImplicitCode()) {
  TRY_TO_TRAVERSE_OR_ENQUEUE_STMT(S->getOperand());
  ShouldVisitChildren = false;
}
  })

If we are not visiting implicit code, we only visit the operand, otherwise we 
visit all subexpressions.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115187

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

[PATCH] D122808: [clang] Fix warnings when `-Wdeprecated-enum-enum-conversion` is enabled

[Antonio Frighetto via Phabricator via cfe-commits]

antoniofrighetto added a comment.

Looks definitely better! How about this slightly changed version protecting the 
interface?

  /// Helper which adds two underlying types of enumeration type.
  template , 
std::underlying_type_t>,
typename UT2 = std::enable_if_t, 
std::underlying_type_t>>
  constexpr auto addEnumValues(EnumTy1 LHS, EnumTy2 RHS) {
return static_cast(LHS) + static_cast(RHS);
  }

I feel like this is something we may wish to readopt in the future for other 
similar cases as well  (e.g., `-Wdeprecated-anon-enum-enum-conversion` in 
`Comment.h`).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122808

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

[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added a comment.

Have had problems with failing stage 2 builds since this patch:

  FAILED: 
tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o
 
  /repo//install/stage2/bin/clang++  -DGTEST_HAS_RTTI=0 -D_DEBUG -D_GNU_SOURCE 
-D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS -D__STDC_LIMIT_MACROS 
-Itools/clang/tools/extra/pseudo/unittests 
-I/repo//llvm-project/clang-tools-extra/pseudo/unittests 
-I/repo//llvm-project/clang/include -Itools/clang/include -Iinclude 
-I/repo//llvm-project/llvm/include 
-I/repo//llvm-project/clang-tools-extra/pseudo/include 
-Itools/clang/tools/extra/pseudo/include -fPIC -fno-semantic-interposition 
-fvisibility-inlines-hidden -Werror -Werror=date-time 
-Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
-Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
-Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
-Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
-Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
-Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
-fdata-sections -flto=thin -fno-common -Woverloaded-virtual 
-Wno-nested-anon-types -O3 -DNDEBUG-Wno-variadic-macros 
-Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti -UNDEBUG 
-std=c++14 -MD -MT 
tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o
 -MF 
tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o.d
 -o 
tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o
 -c /repo//llvm-project/clang-tools-extra/pseudo/unittests/DirectiveMapTest.cpp
  
/repo//llvm-project/clang-tools-extra/pseudo/unittests/DirectiveMapTest.cpp:16:10:
 fatal error: 'gmock/gmock.h' file not found
  #include "gmock/gmock.h"
   ^~~
  1 error generated.

Is there some dependency to googletest missing somewhere?

(Unless someone not just knows about these things, then I guess I need to debug 
our build setup a bit more to give more details about it.)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121233

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

[PATCH] D115187: [clangd] Expose CoawaitExpr's operand in the AST

[Sam McCall via Phabricator via cfe-commits]

sammccall added a subscriber: rsmith.
sammccall added a comment.

This generally looks really good, I'm worried about the separate transformation 
in the template case though.




Comment at: clang/include/clang/AST/ExprCXX.h:4731
 
+  Expr *getOperand() const {
+return static_cast(SubExprs[SubExpr::Operand]);

I think a comment here like `// The syntactic operand written in the code` or 
so would help clarify the distinction between this and the 
common/ready/suspend/resume family.

I'm just thinking of all the times working on tooling when you're trying to 
understand how to handle every node type without deep-diving into the semantics 
of each one :-)

Also maybe group getOperand(), getKeywordLoc(), getBeginLoc(), getEndLoc() 
together?



Comment at: clang/lib/Sema/SemaCoroutine.cpp:737
   return StmtError();
-Suspend = BuildResolvedCoawaitExpr(Loc, Suspend.get(),
+Suspend = BuildResolvedCoawaitExpr(Loc, Suspend.get(), Suspend.get(),
/*IsImplicit*/ true);

I'm not sure Suspend.get() here is the most appropriate value for Operand.

It's a little abstract, since we're talking about the syntactic operand of an 
implicit call :-)
But for consistency with the explicit case, I think the operand should be the 
result of `initial_suspend()` etc, *before* we call `operator co_await`.

i.e. the value of Suspend.get() a couple of lines above.



Comment at: clang/lib/Sema/SemaCoroutine.cpp:838
 
+  Expr *Operand = E;
+

This has become confusing I think. The old code was was changing the meaning of 
the `E`, because we didn't need the original anymore. Rather than introduce a 
new variable for the original meaning, I'd prefer to introduce one for the 
changed meaning:

```
// line 848
auto *Transformed = E;
if (lookupMember(...)) {
  ...
  Transformed = R.get();
}
buildOperatorCoAwaitCall(..., Transformed);
```

(if you also want to rename E -> Operand, that makes sense to me too!)



Comment at: clang/lib/Sema/SemaCoroutine.cpp:858
   }
   ExprResult Awaitable = buildOperatorCoawaitCall(*this, Loc, E, Lookup);
   if (Awaitable.isInvalid())

(aside, this variable name is really unfortunate: I think `E` here is the 
awaitable, and `Awaitable` is the _awaiter_. If this is right, feel free to 
change it or not...)



Comment at: clang/lib/Sema/SemaCoroutine.cpp:865
 
-ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation Loc, Expr *E,
-  bool IsImplicit) {
+ExprResult Sema::BuildResolvedCoawaitExpr(SourceLocation Loc, Expr *Operand,
+  Expr *E, bool IsImplicit) {

Not really your fault, but having two Exprs `Operand` and `E` is terribly 
confusing in isolation. I think `E` is `Awaiter` in standardese. Again feel 
free to change or not.



Comment at: clang/lib/Sema/TreeTransform.h:7934
 TreeTransform::TransformCoawaitExpr(CoawaitExpr *E) {
-  ExprResult Result = getDerived().TransformInitializer(E->getOperand(),
-/*NotCopyInit*/false);
-  if (Result.isInvalid())
+  // XXX is transforming the operand and the common-expr separately the
+  // right thing to do?

Ah, this doesn't *sound* right - it's going to create duplicate subexprs I 
think, and we should really try to have the operand still be a subexpr of the 
commonexpr like in the non-instantiated case.

TransformInitListExpr() is a fairly similar case, and it just discards the 
semantic form and rebuilds from the syntactic one. TransformPseudoObjectExpr 
seems to be similar.

I suppose the analogous thing to do here would be to have RebuildCoawaitExpr 
call Build**Un**resolvedCoawaitExpr with the operand only, but this is a large 
change! (And suggests maybe we should stop building the semantic forms of 
dependent coawaits entirely, though that probably would regress diagnostics).
Maybe worth giving this a spin anyway?

@rsmith, care to weight in here?



Comment at: clang/lib/Sema/TreeTransform.h:7947
 
   // Always rebuild; we don't know if this needs to be injected into a new
   // context or if the promise type has changed.

(FWIW I don't know what "injected into a new context" means, but I don't think 
the promise type can change since DependentCoawaitExpr was added in 
20f25cb6dfb3364847f4c570b1914fe51e585def.)



Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D115187

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

[PATCH] D123019: [WIP][clang][extract-api] Add support for typedefs

[Daniel Grumberg via Phabricator via cfe-commits]

dang created this revision.
dang added reviewers: zixuw, ributzka, QuietMisdreavus.
Herald added a subscriber: mgorny.
Herald added a project: All.
dang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

Typedef records consist of the symbol associated with the underlying
TypedefDecl and a SymbolReference to the underlying type. Additionally
typedefs for anonymous TagTypes use the typedef'd name as the symbol
name in their respective records and USRs. As a result the declaration
fragments for the anonymous TagType are those for the associated
typedef. This means that when the user is defining a typedef to a
typedef to a anonymous type, we use a reference the anonymous TagType
itself and do not emit the typedef to the anonymous type in the
generated symbol graph, including in the type destination of further
typedef symbol records.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123019

Files:
  clang/include/clang/ExtractAPI/API.h
  clang/include/clang/ExtractAPI/DeclarationFragments.h
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/API.cpp
  clang/lib/ExtractAPI/CMakeLists.txt
  clang/lib/ExtractAPI/DeclarationFragments.cpp
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.cpp
  clang/lib/ExtractAPI/TypedefUnderlyingTypeResolver.h
  clang/test/ExtractAPI/typedef.c
  clang/test/ExtractAPI/typedef_anonymous_record.c
  clang/test/ExtractAPI/typedef_chain.c

Index: clang/test/ExtractAPI/typedef_chain.c
===
--- /dev/null
+++ clang/test/ExtractAPI/typedef_chain.c
@@ -0,0 +1,193 @@
+// RUN: rm -rf %t
+// RUN: split-file %s %t
+// RUN: sed -e "s@INPUT_DIR@%/t@g" %t/reference.output.json.in >> \
+// RUN: %t/reference.output.json
+// RUN: %clang -extract-api --product-name=TypedefChain -target arm64-apple-macosx \
+// RUN: -x objective-c-header %t/input.h -o %t/output.json | FileCheck -allow-empty %s
+
+// Generator version is not consistent across test runs, normalize it.
+// RUN: sed -e "s@\"generator\": \".*\"@\"generator\": \"?\"@g" \
+// RUN: %t/output.json >> %t/output-normalized.json
+// RUN: diff %t/reference.output.json %t/output-normalized.json
+
+// CHECK-NOT: error:
+// CHECK-NOT: warning:
+
+//--- input.h
+typedef int MyInt;
+typedef MyInt MyIntInt;
+typedef MyIntInt MyIntIntInt;
+
+//--- reference.output.json.in
+{
+  "metadata": {
+"formatVersion": {
+  "major": 0,
+  "minor": 5,
+  "patch": 3
+},
+"generator": "?"
+  },
+  "module": {
+"name": "TypedefChain",
+"platform": {
+  "architecture": "arm64",
+  "operatingSystem": {
+"minimumVersion": {
+  "major": 11,
+  "minor": 0,
+  "patch": 0
+},
+"name": "macosx"
+  },
+  "vendor": "apple"
+}
+  },
+  "relationhips": [],
+  "symbols": [
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "typedef"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:I",
+  "spelling": "int"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:input.h@T@MyInt"
+  },
+  "kind": {
+"displayName": "Type Alias",
+"identifier": "objective-c.typealias"
+  },
+  "location": {
+"character": 13,
+"line": 1,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+"subHeading": [
+  {
+"kind": "identifier",
+"spelling": "MyInt"
+  }
+],
+"title": "MyInt"
+  },
+  "type": "c:I"
+},
+{
+  "declarationFragments": [
+{
+  "kind": "keyword",
+  "spelling": "typedef"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "typeIdentifier",
+  "preciseIdentifier": "c:input.h@T@MyInt",
+  "spelling": "MyInt"
+},
+{
+  "kind": "text",
+  "spelling": " "
+},
+{
+  "kind": "identifier",
+  "spelling": "MyIntInt"
+}
+  ],
+  "identifier": {
+"interfaceLanguage": "objective-c",
+"precise": "c:input.h@T@MyIntInt"
+  },
+  "kind": {
+"displayName": "Type Alias",
+"identifier": "objective-c.typealias"
+  },
+  "location": {
+"character": 15,
+"line": 2,
+"uri": "file://INPUT_DIR/input.h"
+  },
+  "names": {
+ 

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 420138.
aaron.ballman added a comment.

Fix clang-tools-extra test caught by precommit CI.


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

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m

Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
+// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -fdiagnostics-parseable-fixits %s 2>&1 | FileCheck %s
 
 // function definition with 0 params, no prototype, no preceding declaration.
 void foo0() {} // expected-warning {{this old-style function definition is not preceded by a prototype}}
Index: clang/test/Sema/vl

[PATCH] D120374: [clang-format] Do not insert space after new/delete keywords in C function declarations

[Hans Wennborg via Phabricator via cfe-commits]

hans added a comment.
Herald added a project: All.

This seems to have caused https://github.com/llvm/llvm-project/issues/54703


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120374

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

[clang] 87b28f5 - [clang][NFC] Extract the EmitAssemblyHelper::TargetTriple member

[Alexey Bader via cfe-commits]

Author: Pavel Samolysov
Date: 2022-04-04T12:16:39+03:00
New Revision: 87b28f5092f2f92fc380f18e8578746bdd2a54b2

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

LOG: [clang][NFC] Extract the EmitAssemblyHelper::TargetTriple member

Few times in different methods of the EmitAssemblyHelper class the following
code snippet is used to get the TargetTriple and then use it's single method
to check some conditions:

TargetTriple(TheModule->getTargetTriple())

The parsing of a target triple string is not a trivial operation and it takes
time to repeat the parsing many times in different methods of the class and
even numerous times in one method just to call a getter
(llvm::Triple(TheModule->getTargetTriple()).getVendor()), for example.
The patch extracts the TargetTriple member of the EmitAssemblyHelper class to
parse the triple only once in the class' constructor.

Reviewed By: tejohnson

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

Added: 


Modified: 
clang/lib/CodeGen/BackendUtil.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/BackendUtil.cpp 
b/clang/lib/CodeGen/BackendUtil.cpp
index 61677d368029e..36776d39b952a 100644
--- a/clang/lib/CodeGen/BackendUtil.cpp
+++ b/clang/lib/CodeGen/BackendUtil.cpp
@@ -118,6 +118,8 @@ class EmitAssemblyHelper {
 
   std::unique_ptr OS;
 
+  Triple TargetTriple;
+
   TargetIRAnalysis getTargetIRAnalysis() const {
 if (TM)
   return TM->getTargetIRAnalysis();
@@ -170,7 +172,8 @@ class EmitAssemblyHelper {
  const LangOptions &LOpts, Module *M)
   : Diags(_Diags), HSOpts(HeaderSearchOpts), CodeGenOpts(CGOpts),
 TargetOpts(TOpts), LangOpts(LOpts), TheModule(M),
-CodeGenerationTime("codegen", "Code Generation Time") {}
+CodeGenerationTime("codegen", "Code Generation Time"),
+TargetTriple(TheModule->getTargetTriple()) {}
 
   ~EmitAssemblyHelper() {
 if (CodeGenOpts.DisableFree)
@@ -695,7 +698,6 @@ void EmitAssemblyHelper::CreatePasses(legacy::PassManager 
&MPM,
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
   // are inserted before PMBuilder ones - they'd get the default-constructed
   // TLI with an unknown target otherwise.
-  Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
 
@@ -965,7 +967,6 @@ bool EmitAssemblyHelper::AddEmitPasses(legacy::PassManager 
&CodeGenPasses,
raw_pwrite_stream &OS,
raw_pwrite_stream *DwoOS) {
   // Add LibraryInfo.
-  llvm::Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   CodeGenPasses.add(new TargetLibraryInfoWrapperPass(*TLII));
@@ -1054,10 +1055,8 @@ void 
EmitAssemblyHelper::EmitAssemblyWithLegacyPassManager(
   // Emit a module summary by default for Regular LTO except for ld64
   // targets
   bool EmitLTOSummary =
-  (CodeGenOpts.PrepareForLTO &&
-   !CodeGenOpts.DisableLLVMPasses &&
-   llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
-   llvm::Triple::Apple);
+  (CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
+   TargetTriple.getVendor() != llvm::Triple::Apple);
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
@@ -1338,7 +1337,6 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
 
   // Register the target library analysis directly and give it a customized
   // preset TLI.
-  Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
@@ -1474,8 +1472,7 @@ void EmitAssemblyHelper::RunOptimizationPipeline(
   // targets
   bool EmitLTOSummary =
   (CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
-   llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
-   llvm::Triple::Apple);
+   TargetTriple.getVendor() != llvm::Triple::Apple);
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));



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

[PATCH] D122587: [clang][NFC] Extract the EmitAssemblyHelper::TargetTriple member

[Alexey Bader via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG87b28f5092f2: [clang][NFC] Extract the 
EmitAssemblyHelper::TargetTriple member (authored by psamolysov-intel, 
committed by bader).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122587

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -118,6 +118,8 @@
 
   std::unique_ptr OS;
 
+  Triple TargetTriple;
+
   TargetIRAnalysis getTargetIRAnalysis() const {
 if (TM)
   return TM->getTargetIRAnalysis();
@@ -170,7 +172,8 @@
  const LangOptions &LOpts, Module *M)
   : Diags(_Diags), HSOpts(HeaderSearchOpts), CodeGenOpts(CGOpts),
 TargetOpts(TOpts), LangOpts(LOpts), TheModule(M),
-CodeGenerationTime("codegen", "Code Generation Time") {}
+CodeGenerationTime("codegen", "Code Generation Time"),
+TargetTriple(TheModule->getTargetTriple()) {}
 
   ~EmitAssemblyHelper() {
 if (CodeGenOpts.DisableFree)
@@ -695,7 +698,6 @@
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
   // are inserted before PMBuilder ones - they'd get the default-constructed
   // TLI with an unknown target otherwise.
-  Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
 
@@ -965,7 +967,6 @@
raw_pwrite_stream &OS,
raw_pwrite_stream *DwoOS) {
   // Add LibraryInfo.
-  llvm::Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   CodeGenPasses.add(new TargetLibraryInfoWrapperPass(*TLII));
@@ -1054,10 +1055,8 @@
   // Emit a module summary by default for Regular LTO except for ld64
   // targets
   bool EmitLTOSummary =
-  (CodeGenOpts.PrepareForLTO &&
-   !CodeGenOpts.DisableLLVMPasses &&
-   llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
-   llvm::Triple::Apple);
+  (CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
+   TargetTriple.getVendor() != llvm::Triple::Apple);
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
@@ -1338,7 +1337,6 @@
 
   // Register the target library analysis directly and give it a customized
   // preset TLI.
-  Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   FAM.registerPass([&] { return TargetLibraryAnalysis(*TLII); });
@@ -1474,8 +1472,7 @@
   // targets
   bool EmitLTOSummary =
   (CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
-   llvm::Triple(TheModule->getTargetTriple()).getVendor() !=
-   llvm::Triple::Apple);
+   TargetTriple.getVendor() != llvm::Triple::Apple);
   if (EmitLTOSummary) {
 if (!TheModule->getModuleFlag("ThinLTO"))
   TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -118,6 +118,8 @@
 
   std::unique_ptr OS;
 
+  Triple TargetTriple;
+
   TargetIRAnalysis getTargetIRAnalysis() const {
 if (TM)
   return TM->getTargetIRAnalysis();
@@ -170,7 +172,8 @@
  const LangOptions &LOpts, Module *M)
   : Diags(_Diags), HSOpts(HeaderSearchOpts), CodeGenOpts(CGOpts),
 TargetOpts(TOpts), LangOpts(LOpts), TheModule(M),
-CodeGenerationTime("codegen", "Code Generation Time") {}
+CodeGenerationTime("codegen", "Code Generation Time"),
+TargetTriple(TheModule->getTargetTriple()) {}
 
   ~EmitAssemblyHelper() {
 if (CodeGenOpts.DisableFree)
@@ -695,7 +698,6 @@
   // manually (and not via PMBuilder), since some passes (eg. InstrProfiling)
   // are inserted before PMBuilder ones - they'd get the default-constructed
   // TLI with an unknown target otherwise.
-  Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
 
@@ -965,7 +967,6 @@
raw_pwrite_stream &OS,
raw_pwrite_stream *DwoOS) {
   // Add LibraryInfo.
-  llvm::Triple TargetTriple(TheModule->getTargetTriple());
   std::unique_ptr TLII(
   createTLII(TargetTriple, CodeGenOpts));
   CodeGenPasses.add(new TargetLibraryInfoWrapperPass(*TLII));
@@ -1054,10 +1055,8 @@
   // Emit a module summary by default for Regular LTO except for ld6

[clang] 506ec85 - [clang][dataflow] Add support for clang's `__builtin_expect`.

[Yitzhak Mandelbaum via cfe-commits]

Author: Yitzhak Mandelbaum
Date: 2022-04-04T12:20:43Z
New Revision: 506ec85ba82a39748f8fb07c15b93e1e958a9611

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

LOG: [clang][dataflow] Add support for clang's `__builtin_expect`.

This patch adds basic modeling of `__builtin_expect`, just to propagate the
(first) argument, making the call transparent.

Driveby: adds tests for proper handling of other builtins.

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

Added: 


Modified: 
clang/lib/Analysis/FlowSensitive/Transfer.cpp
clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Removed: 




diff  --git a/clang/lib/Analysis/FlowSensitive/Transfer.cpp 
b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
index 1f5de9d67c135..271167b90030d 100644
--- a/clang/lib/Analysis/FlowSensitive/Transfer.cpp
+++ b/clang/lib/Analysis/FlowSensitive/Transfer.cpp
@@ -22,6 +22,7 @@
 #include "clang/AST/StmtVisitor.h"
 #include "clang/Analysis/FlowSensitive/DataflowEnvironment.h"
 #include "clang/Analysis/FlowSensitive/Value.h"
+#include "clang/Basic/Builtins.h"
 #include "clang/Basic/OperatorKinds.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/Support/Casting.h"
@@ -444,6 +445,9 @@ class TransferVisitor : public 
ConstStmtVisitor {
   }
 
   void VisitCallExpr(const CallExpr *S) {
+// Of clang's builtins, only `__builtin_expect` is handled explicitly, 
since
+// others (like trap, debugtrap, and unreachable) are handled by CFG
+// construction.
 if (S->isCallToStdMove()) {
   assert(S->getNumArgs() == 1);
 
@@ -455,6 +459,18 @@ class TransferVisitor : public 
ConstStmtVisitor {
 return;
 
   Env.setStorageLocation(*S, *ArgLoc);
+} else if (S->getDirectCallee() != nullptr &&
+   S->getDirectCallee()->getBuiltinID() ==
+   Builtin::BI__builtin_expect) {
+  assert(S->getNumArgs() > 0);
+  assert(S->getArg(0) != nullptr);
+  // `__builtin_expect` returns by-value, so strip away any potential
+  // references in the argument.
+  auto *ArgLoc = Env.getStorageLocation(
+  *S->getArg(0)->IgnoreParenImpCasts(), SkipPast::Reference);
+  if (ArgLoc == nullptr)
+return;
+  Env.setStorageLocation(*S, *ArgLoc);
 }
   }
 

diff  --git a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp 
b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
index 540ed4781aebf..2fa5be182eea5 100644
--- a/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ b/clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2368,6 +2368,131 @@ TEST_F(TransferTest, AssignFromBoolNegation) {
   });
 }
 
+TEST_F(TransferTest, BuiltinExpect) {
+  std::string Code = R"(
+void target(long Foo) {
+  long Bar = __builtin_expect(Foo, true);
+  /*[[p]]*/
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const auto &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, BuiltinUnreachable) {
+  std::string Code = R"(
+void target(bool Foo) {
+  bool Bar = false;
+  if (Foo)
+Bar = Foo;
+  else
+__builtin_unreachable();
+  (void)0;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const auto &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+// `__builtin_unreachable` promises that the code is
+// unreachable, so the compiler treats the "then" branch as the
+// only possible predecessor of this statement.
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, BuiltinTrap) {
+  std::string Code = R"(
+void target(bool Foo) 

[PATCH] D122908: [clang][dataflow] Add support for clang's `__builtin_expect`.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

This revision was automatically updated to reflect the committed changes.
Closed by commit rG506ec85ba82a: [clang][dataflow] Add support for clang's 
`__builtin_expect`. (authored by ymandel).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122908

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -2368,6 +2368,131 @@
   });
 }
 
+TEST_F(TransferTest, BuiltinExpect) {
+  std::string Code = R"(
+void target(long Foo) {
+  long Bar = __builtin_expect(Foo, true);
+  /*[[p]]*/
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const auto &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, BuiltinUnreachable) {
+  std::string Code = R"(
+void target(bool Foo) {
+  bool Bar = false;
+  if (Foo)
+Bar = Foo;
+  else
+__builtin_unreachable();
+  (void)0;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const auto &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+// `__builtin_unreachable` promises that the code is
+// unreachable, so the compiler treats the "then" branch as the
+// only possible predecessor of this statement.
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, BuiltinTrap) {
+  std::string Code = R"(
+void target(bool Foo) {
+  bool Bar = false;
+  if (Foo)
+Bar = Foo;
+  else
+__builtin_trap();
+  (void)0;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const auto &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+// `__builtin_trap` ensures program termination, so only the
+// "then" branch is a predecessor of this statement.
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, BuiltinDebugTrap) {
+  std::string Code = R"(
+void target(bool Foo) {
+  bool Bar = false;
+  if (Foo)
+Bar = Foo;
+  else
+__builtin_debugtrap();
+  (void)0;
+  /*[[p]]*/
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const auto &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+// `__builtin_debugtrap` doesn't ensure program termination.
+EXPECT_NE(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
 TEST_F(TransferTest, StaticIntSingleVarDecl) {
   std::string Code = R"(
 void target() {
Index: clang/l

[PATCH] D122774: [clang][extract-api] Add Objective-C Category support

[Daniel Grumberg via Phabricator via cfe-commits]

dang added inline comments.



Comment at: clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp:396
 break;
+  case APIRecord::RK_ObjCCategory:
+Kind["identifier"] = AddLangPrefix("category");

Since we currently never actually emit a standalone category symbol object, 
should we not just label this as `llvm_unreachable` with a comment explaining 
why that is?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122774

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

[PATCH] D122885: [clang] Draft: Implement P1703R1

[Nathan Sidwell via Phabricator via cfe-commits]

urnathan added a comment.

Ah, I'd had a thinko about what 1703 was.  that's superseded by p1757.  That's 
what is needed.  (your patch makes more sense in the 1703 context)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122885

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

[PATCH] D123026: [clang][NFC] Extract the EmitAssemblyHelper::emitLTOSummary method

[Pavel Samolysov via Phabricator via cfe-commits]

psamolysov-intel created this revision.
psamolysov-intel added reviewers: tejohnson, ddunbar, tobiasvk, tobiasvk_caf, 
paulkirth, greened.
psamolysov-intel added a project: clang.
Herald added subscribers: ormris, inglorion.
Herald added a project: All.
psamolysov-intel requested review of this revision.
Herald added a subscriber: cfe-commits.

The code to check if the LTO summary should be emitted and to add the
corresponding module flags was duplicated in the
'EmitAssemblyHelper::EmitAssemblyWithLegacyPassManager' and
'EmitAssemblyHelper::RunOptimizationPipeline' methods.

  

In order to eliminate these code duplications, the
'EmitAssemblyHelper::emitLTOSummary' method has been extracted. The method
returns a bool value, the value is true if the module summary should be emitted.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123026

Files:
  clang/lib/CodeGen/BackendUtil.cpp


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -164,6 +164,25 @@
   std::unique_ptr &OS,
   std::unique_ptr &DwoOS);
 
+  /// Emit a module summary by default for Regular LTO except for ld64
+  /// targets.
+  ///
+  /// \return True if the module summary should be emitted.
+  bool emitLTOSummary() {
+bool EmitLTOSummary =
+(CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
+ TargetTriple.getVendor() != llvm::Triple::Apple);
+if (EmitLTOSummary) {
+  if (!TheModule->getModuleFlag("ThinLTO"))
+TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
+  if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
+TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+ uint32_t(1));
+}
+
+return EmitLTOSummary;
+  }
+
 public:
   EmitAssemblyHelper(DiagnosticsEngine &_Diags,
  const HeaderSearchOptions &HeaderSearchOpts,
@@ -1052,19 +1071,7 @@
   PerModulePasses.add(createWriteThinLTOBitcodePass(
   *OS, ThinLinkOS ? &ThinLinkOS->os() : nullptr));
 } else {
-  // Emit a module summary by default for Regular LTO except for ld64
-  // targets
-  bool EmitLTOSummary =
-  (CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
-   TargetTriple.getVendor() != llvm::Triple::Apple);
-  if (EmitLTOSummary) {
-if (!TheModule->getModuleFlag("ThinLTO"))
-  TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
-if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
-  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
-   uint32_t(1));
-  }
-
+  bool EmitLTOSummary = emitLTOSummary();
   PerModulePasses.add(createBitcodeWriterPass(
   *OS, CodeGenOpts.EmitLLVMUseLists, EmitLTOSummary));
 }
@@ -1468,18 +1475,7 @@
   MPM.addPass(ThinLTOBitcodeWriterPass(*OS, ThinLinkOS ? &ThinLinkOS->os()
: nullptr));
 } else {
-  // Emit a module summary by default for Regular LTO except for ld64
-  // targets
-  bool EmitLTOSummary =
-  (CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
-   TargetTriple.getVendor() != llvm::Triple::Apple);
-  if (EmitLTOSummary) {
-if (!TheModule->getModuleFlag("ThinLTO"))
-  TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
-if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
-  TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
-   uint32_t(1));
-  }
+  bool EmitLTOSummary = emitLTOSummary();
   MPM.addPass(
   BitcodeWriterPass(*OS, CodeGenOpts.EmitLLVMUseLists, 
EmitLTOSummary));
 }


Index: clang/lib/CodeGen/BackendUtil.cpp
===
--- clang/lib/CodeGen/BackendUtil.cpp
+++ clang/lib/CodeGen/BackendUtil.cpp
@@ -164,6 +164,25 @@
   std::unique_ptr &OS,
   std::unique_ptr &DwoOS);
 
+  /// Emit a module summary by default for Regular LTO except for ld64
+  /// targets.
+  ///
+  /// \return True if the module summary should be emitted.
+  bool emitLTOSummary() {
+bool EmitLTOSummary =
+(CodeGenOpts.PrepareForLTO && !CodeGenOpts.DisableLLVMPasses &&
+ TargetTriple.getVendor() != llvm::Triple::Apple);
+if (EmitLTOSummary) {
+  if (!TheModule->getModuleFlag("ThinLTO"))
+TheModule->addModuleFlag(Module::Error, "ThinLTO", uint32_t(0));
+  if (!TheModule->getModuleFlag("EnableSplitLTOUnit"))
+TheModule->addModuleFlag(Module::Error, "EnableSplitLTOUnit",
+ uint32_t(1));
+}
+
+return EmitLTOSummary;
+  }
+
 public:
   Emi

[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D121233#3425793 , @bjope wrote:

> Have had problems with failing stage 2 builds since this patch:
>
>   FAILED: 
> tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o
>  
>   /repo//install/stage2/bin/clang++  -DGTEST_HAS_RTTI=0 -D_DEBUG 
> -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
> -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/pseudo/unittests 
> -I/repo//llvm-project/clang-tools-extra/pseudo/unittests 
> -I/repo//llvm-project/clang/include -Itools/clang/include -Iinclude 
> -I/repo//llvm-project/llvm/include 
> -I/repo//llvm-project/clang-tools-extra/pseudo/include 
> -Itools/clang/tools/extra/pseudo/include -fPIC -fno-semantic-interposition 
> -fvisibility-inlines-hidden -Werror -Werror=date-time 
> -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
> -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
> -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
> -Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
> -fdata-sections -flto=thin -fno-common -Woverloaded-virtual 
> -Wno-nested-anon-types -O3 -DNDEBUG-Wno-variadic-macros 
> -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti -UNDEBUG 
> -std=c++14 -MD -MT 
> tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o
>  -MF 
> tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o.d
>  -o 
> tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o
>  -c 
> /repo//llvm-project/clang-tools-extra/pseudo/unittests/DirectiveMapTest.cpp
>   
> /repo//llvm-project/clang-tools-extra/pseudo/unittests/DirectiveMapTest.cpp:16:10:
>  fatal error: 'gmock/gmock.h' file not found
>   #include "gmock/gmock.h"
>^~~
>   1 error generated.
>
> Is there some dependency to googletest missing somewhere?
>
> (Unless someone not just knows about these things, then I guess I need to 
> debug our build setup a bit more to give more details about it.)

The `-I` is supposed to be added by t

In D121233#3425793 , @bjope wrote:

> Have had problems with failing stage 2 builds since this patch:
>
>   FAILED: 
> tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o
>  
>   /repo//install/stage2/bin/clang++  -DGTEST_HAS_RTTI=0 -D_DEBUG 
> -D_GNU_SOURCE -D__STDC_CONSTANT_MACROS -D__STDC_FORMAT_MACROS 
> -D__STDC_LIMIT_MACROS -Itools/clang/tools/extra/pseudo/unittests 
> -I/repo//llvm-project/clang-tools-extra/pseudo/unittests 
> -I/repo//llvm-project/clang/include -Itools/clang/include -Iinclude 
> -I/repo//llvm-project/llvm/include 
> -I/repo//llvm-project/clang-tools-extra/pseudo/include 
> -Itools/clang/tools/extra/pseudo/include -fPIC -fno-semantic-interposition 
> -fvisibility-inlines-hidden -Werror -Werror=date-time 
> -Werror=unguarded-availability-new -Wall -Wextra -Wno-unused-parameter 
> -Wwrite-strings -Wcast-qual -Wmissing-field-initializers -pedantic 
> -Wno-long-long -Wc++98-compat-extra-semi -Wimplicit-fallthrough 
> -Wcovered-switch-default -Wno-noexcept-type -Wnon-virtual-dtor 
> -Wdelete-non-virtual-dtor -Wsuggest-override -Wstring-conversion 
> -Wmisleading-indentation -fdiagnostics-color -ffunction-sections 
> -fdata-sections -flto=thin -fno-common -Woverloaded-virtual 
> -Wno-nested-anon-types -O3 -DNDEBUG-Wno-variadic-macros 
> -Wno-gnu-zero-variadic-macro-arguments -fno-exceptions -fno-rtti -UNDEBUG 
> -std=c++14 -MD -MT 
> tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o
>  -MF 
> tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o.d
>  -o 
> tools/clang/tools/extra/pseudo/unittests/CMakeFiles/ClangPseudoTests.dir/DirectiveMapTest.cpp.o
>  -c 
> /repo//llvm-project/clang-tools-extra/pseudo/unittests/DirectiveMapTest.cpp
>   
> /repo//llvm-project/clang-tools-extra/pseudo/unittests/DirectiveMapTest.cpp:16:10:
>  fatal error: 'gmock/gmock.h' file not found
>   #include "gmock/gmock.h"
>^~~
>   1 error generated.
>
> Is there some dependency to googletest missing somewhere?
>
> (Unless someone not just knows about these things, then I guess I need to 
> debug our build setup a bit more to give more details about it.)

The way this is meant to work is:

- pseudo/unittests/CMakeLists.txt calls `add_unittest`
- add_unittest (in AddLLVM.cmake) adds a dependency on `llvm_gtest`
- `llvm_gtest` is defined in llvm/utils/unittest/CMakeLists.txt, and specifies 
`target_include_directories(llvm_gtest PUBLIC ...)` to add the directories

As far as I can tell this is how other things in

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D122820#3423712 , @thakis wrote:

> Looks like this breaks tests on macOS: 
> http://45.33.8.238/macm1/31733/step_7.txt
>
> Please take a look, and revert for now if it takes a while to fix.

That is sad, it seems that llvm-cxxfilt isn't working right on some of the 
machines :/


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122820

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

[PATCH] D121637: [PowerPC] Fix EmitPPCBuiltinExpr to emit arguments once

[Amy Kwan via Phabricator via cfe-commits]

amyk accepted this revision as: amyk.
amyk added a comment.
This revision is now accepted and ready to land.

Thanks Quinn. i think this overall LGTM.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121637

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

[clang-tools-extra] 72ae6cc - [pseudo] respect CLANG_INCLUDE_TESTS

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-04-04T15:30:11+02:00
New Revision: 72ae6cc3a689869dcc15cfa8d0abcdd35a35a484

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

LOG: [pseudo] respect CLANG_INCLUDE_TESTS

Added: 


Modified: 
clang-tools-extra/pseudo/CMakeLists.txt

Removed: 




diff  --git a/clang-tools-extra/pseudo/CMakeLists.txt 
b/clang-tools-extra/pseudo/CMakeLists.txt
index a10884ad406b9..fe7f7c63fb75e 100644
--- a/clang-tools-extra/pseudo/CMakeLists.txt
+++ b/clang-tools-extra/pseudo/CMakeLists.txt
@@ -2,5 +2,7 @@ include_directories(include)
 include_directories(${CMAKE_CURRENT_BINARY_DIR}/include)
 add_subdirectory(lib)
 add_subdirectory(tool)
-add_subdirectory(unittests)
-add_subdirectory(test)
+if(CLANG_INCLUDE_TESTS)
+  add_subdirectory(unittests)
+  add_subdirectory(test)
+endif()



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

[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D121233#3425793 , @bjope wrote:

> Have had problems with failing stage 2 builds since this patch:

Should be fixed in 72ae6cc3a689869dcc15cfa8d0abcdd35a35a484 
, please 
let me know if not.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121233

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

[PATCH] D119609: [Clang][Sema] Prohibit expr statement in the default argument

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D119609#3424434 , @junaire wrote:

> Hi @aaron.ballman, do you think it's time to consider reviewing this?
>
> I don't why there's no response or progress in GCC, but I think I can submit 
> a patch for them if they think this issue is low prior.

I think I'm ok with rejecting both cases now, and doing this review.  Though 
I'm still concerned with the 'skip-until' losing some diagnostics, so please 
add another test for that as I requested before.




Comment at: clang/test/Sema/err-expr-stmt-in-default-arg.cpp:3
+
+void foo() {
+  void fn(int i, int = ({ 1; })); // expected-error {{expression statement not 
permitted in default argument}}

Do we also prohibit in a template argument?



Comment at: clang/test/Sema/err-expr-stmt-in-default-arg.cpp:19
+  return bar(l);
+}

erichkeane wrote:
> junaire wrote:
> > erichkeane wrote:
> > > For completeness, I'd like to see an 
> > > in-function-defined-struct-member-function test here as well.
> > > 
> > > As for the above question about the recovery, something like:
> > > 
> > > `void fn(int i, int j = ({ {},{},{,} }), int k = "");` I think checks all 
> > > the issues I can think of.  We want to make sure that 'k' still diagnoses 
> > > its error correctly (and that we have just skipped all of the expression 
> > > statement stuff).
> > Note that clang is already rejected the code: 
> > https://godbolt.org/z/57c4qaaPo
> > 
> I more mean an example like this one:
> https://godbolt.org/z/nf7W1zznh
> 
> I see that we already reject it, however you are doing a 'skip until' here, 
> and I want to make sure that the error on 'k' diagnoses correctly still. 
> 
> With your change, I would expect the 1st two errors there to go away and be 
> replaced by your new error.  BUT the 'k' type would still be there.
> 
> The point of this is to make sure that your error doesn't leave the parser in 
> a 'bad' state.
I don't see the test I requested here, I'd still like to see that to make sure 
the parser still diagnoses' k'.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119609

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

[clang] d1205bb - Reapply"[GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs"

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2022-04-04T06:41:47-07:00
New Revision: d1205bb37d8c37c6e427c339d704e707147989d2

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

LOG: Reapply"[GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed 
structs"

AND the followups that fixed builds.

I attempted to get 'cute' and use llvm-cxxfilt to make the test look
nicer, but apparently some of the bots have a version of llvm-cxxfilt
that is not the in-tree one, so it fails to properly demangle the stuff.
I've disabled this "RUN" line.

This reverts commit 50186b63d1807d389f31c515377d94185795ab44.

Added: 
clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp

Modified: 
clang/lib/AST/ItaniumMangle.cpp

Removed: 




diff  --git a/clang/lib/AST/ItaniumMangle.cpp b/clang/lib/AST/ItaniumMangle.cpp
index fb76fa7b896fc..50e110ec1f57e 100644
--- a/clang/lib/AST/ItaniumMangle.cpp
+++ b/clang/lib/AST/ItaniumMangle.cpp
@@ -5545,6 +5545,47 @@ static QualType getLValueType(ASTContext &Ctx, const 
APValue &LV) {
   return T;
 }
 
+static IdentifierInfo *getUnionInitName(SourceLocation UnionLoc,
+DiagnosticsEngine &Diags,
+const FieldDecl *FD) {
+  // According to:
+  // http://itanium-cxx-abi.github.io/cxx-abi/abi.html#mangling.anonymous
+  // For the purposes of mangling, the name of an anonymous union is considered
+  // to be the name of the first named data member found by a pre-order,
+  // depth-first, declaration-order walk of the data members of the anonymous
+  // union.
+
+  if (FD->getIdentifier())
+return FD->getIdentifier();
+
+  // The only cases where the identifer of a FieldDecl would be blank is if the
+  // field represents an anonymous record type or if it is an unnamed bitfield.
+  // There is no type to descend into in the case of a bitfield, so we can just
+  // return nullptr in that case.
+  if (FD->isBitField())
+return nullptr;
+  const CXXRecordDecl *RD = FD->getType()->getAsCXXRecordDecl();
+
+  // Consider only the fields in declaration order, searched depth-first.  We
+  // don't care about the active member of the union, as all we are doing is
+  // looking for a valid name. We also don't check bases, due to guidance from
+  // the Itanium ABI folks.
+  for (const FieldDecl *RDField : RD->fields()) {
+if (IdentifierInfo *II = getUnionInitName(UnionLoc, Diags, RDField))
+  return II;
+  }
+
+  // According to the Itanium ABI: If there is no such data member (i.e., if 
all
+  // of the data members in the union are unnamed), then there is no way for a
+  // program to refer to the anonymous union, and there is therefore no need to
+  // mangle its name. However, we should diagnose this anyway.
+  unsigned DiagID = Diags.getCustomDiagID(
+  DiagnosticsEngine::Error, "cannot mangle this unnamed union NTTP yet");
+  Diags.Report(UnionLoc, DiagID);
+
+  return nullptr;
+}
+
 void CXXNameMangler::mangleValueInTemplateArg(QualType T, const APValue &V,
   bool TopLevel,
   bool NeedExactType) {
@@ -5628,7 +5669,10 @@ void CXXNameMangler::mangleValueInTemplateArg(QualType 
T, const APValue &V,
 mangleType(T);
 if (!isZeroInitialized(T, V)) {
   Out << "di";
-  mangleSourceName(FD->getIdentifier());
+  IdentifierInfo *II = (getUnionInitName(
+  T->getAsCXXRecordDecl()->getLocation(), Context.getDiags(), FD));
+  if (II)
+mangleSourceName(II);
   mangleValueInTemplateArg(FD->getType(), V.getUnionValue(), false);
 }
 Out << 'E';

diff  --git a/clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp 
b/clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp
new file mode 100644
index 0..4382d1406cfec
--- /dev/null
+++ b/clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp
@@ -0,0 +1,117 @@
+// RUN: %clang_cc1 -std=c++20 -emit-llvm %s -o - -triple=x86_64-linux-gnu | 
FileCheck %s
+
+// FIXME: In the future we could possibly do this to get nicer looking output
+// for this, but it seems that some of the bots don't have a recent enough
+// version of llvm-cxxfilt to demangle these.
+// %clang_cc1 -std=c++20 -emit-llvm %s -o - -triple=x86_64-linux-gnu | 
llvm-cxxfilt | FileCheck %s --check-prefix DEMANGLED
+
+template
+struct wrapper1 {
+  union {
+struct {
+  T RightName;
+};
+  };
+};
+
+template
+struct wrapper2 {
+  union {
+struct {
+  T RightName;
+};
+T WrongName;
+  };
+};
+
+struct Base {
+  int WrongName;
+};
+
+template 
+struct wrapper3 {
+  union {
+struct : Base {
+  T RightName; };
+T WrongName;
+  };
+};
+
+template 
+struct wrapper4 {
+  union {
+int RightName;
+struct {
+  T WrongName;
+};
+

[PATCH] D122885: [clang] Draft: Implement P1703R1

[Timm Bäder via Phabricator via cfe-commits]

tbaeder added a comment.

In D122885#3426070 , @urnathan wrote:

> Ah, I'd had a thinko about what 1703 was.  that's superseded by 
> p1757wg21.link/p1857.  That's what is needed.  (your patch 
> makes more sense in the 1703 context)

Ha, okay. Yes, that makes more sense. Both papers are linked in 
https://clang.llvm.org/cxx_status.html. Sorry for the confusion, I'll have a 
look at 1857R3 instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122885

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

[PATCH] D122952: [clang] NFC: Extend comdat validation in target multiversion function tests.

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/test/CodeGen/attr-target-mv.c:95-111
+// WINDOWS: $foo_used = comdat any
+// WINDOWS: $foo_used2.avx_sse4.2 = comdat any
+// WINDOWS: $pr50025.resolver = comdat any
+// WINDOWS: $pr50025c.resolver = comdat any
+// WINDOWS: $foo_inline.sse4.2 = comdat any
+// WINDOWS: $foo_inline.arch_ivybridge = comdat any
+// WINDOWS: $foo_inline = comdat any

tahonermann wrote:
> The non-inline non-resolver cases here are surprising to me; they aren't 
> COMDAT on Linux. Is this intentional?
I don't think so, we perhaps messed that up.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122952

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

[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

[Bjorn Pettersson via Phabricator via cfe-commits]

bjope added a comment.

In D121233#3426155 , @sammccall wrote:

> In D121233#3425793 , @bjope wrote:
>
>> Have had problems with failing stage 2 builds since this patch:
>
> Should be fixed in 72ae6cc3a689869dcc15cfa8d0abcdd35a35a484 
> , please 
> let me know if not.

Yes, you are right. Fantastic finding without me even providing the cmake 
command.
But it was true that for the failing stage we had disabled tests using 
`-DLLVM_INCLUDE_TESTS=OFF` which result in CLANG_INCLUDE_TESTS being OFF as 
well. So your fix to respect the CLANG_INCLUDE_TESTS setting does indeed solve 
the problem. Thanks alot for the help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121233

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

[PATCH] D122954: [clang] Extend target_clones tests to exercise declarations that are not definitions.

[Erich Keane via Phabricator via cfe-commits]

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

FWIW, I dislike this idea of doing tests in separate commits from the patch 
itself, it makes the review of the patch more difficult, and makes looking 
through history more difficult.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122954

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

[PATCH] D122155: Add warning when eval-method is set in the presence of value unsafe floating-point calculations.

[Zahira Ammarguellat via Phabricator via cfe-commits]

zahiraam updated this revision to Diff 420168.

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

https://reviews.llvm.org/D122155

Files:
  clang/include/clang/Basic/DiagnosticFrontendKinds.td
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/lib/Sema/SemaAttr.cpp
  clang/test/CodeGen/X86/32bit-behavior.c
  clang/test/Driver/eval-method-with-unsafe-math.c
  clang/test/Sema/eval-method-with-unsafe-math.c

Index: clang/test/Sema/eval-method-with-unsafe-math.c
===
--- /dev/null
+++ clang/test/Sema/eval-method-with-unsafe-math.c
@@ -0,0 +1,56 @@
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu  \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu  -freciprocal-math \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-RECPR,CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu  -mreassociate \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-ASSOC,CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu  -fapprox-func \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-FUNC,CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -freciprocal-math -mreassociate -verify \
+// RUN: %s 2>&1 | FileCheck %s --check-prefixes=CHECK-ASSOC,CHECK-RECPR,CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -freciprocal-math -mreassociate -fapprox-func \
+// RUN: -verify %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=CHECK-FUNC,CHECK-ASSOC,CHECK-RECPR,CHECK-PRGM
+
+// RUN: not %clang_cc1 -fexperimental-strict-floating-point \
+// RUN: -triple x86_64-linux-gnu -ffp-eval-method=source \
+// RUN: -verify %s 2>&1 | FileCheck %s --check-prefixes=CHECK-FFP-OPT,CHECK-PRGM
+
+// expected-no-diagnostics
+
+float f1(float a, float b, float c) {
+  a = b + c;
+  return a * b + c;
+}
+
+float f2(float a, float b, float c) {
+  // CHECK-FFP-OPT: option 'ffp-eval-method' cannot be used with '#pragma clang fp reassociate'
+#pragma clang fp reassociate(on)
+  return (a + b) + c;
+}
+
+float f3(float a, float b, float c) {
+#pragma clang fp reassociate(off)
+  return (a - b) - c;
+}
+
+float f4(float a, float b, float c) {
+#pragma clang fp eval_method(double)
+  // CHECK-FUNC: '#pragma clang fp eval_method' cannot be used with option 'fapprox-func'
+  // CHECK-ASSOC: '#pragma clang fp eval_method' cannot be used with option 'mreassociate'
+  // CHECK-RECPR: '#pragma clang fp eval_method' cannot be used with option 'freciprocal'
+  // CHECK-PRGM: '#pragma clang fp eval_method' cannot be used with '#pragma clang fp reassociate'
+#pragma clang fp reassociate(on)
+  return (a * c) - (b * c);
+}
Index: clang/test/Driver/eval-method-with-unsafe-math.c
===
--- /dev/null
+++ clang/test/Driver/eval-method-with-unsafe-math.c
@@ -0,0 +1,29 @@
+// RUN: not %clang -Xclang -fexperimental-strict-floating-point \
+// RUN: -Xclang -triple -Xclang x86_64-linux-gnu -fapprox-func \
+// RUN: -Xclang -verify -ffp-eval-method=source %s 2>&1  \
+// RUN: | FileCheck %s --check-prefixes=CHECK-FUNC
+
+// RUN: not %clang -Xclang -fexperimental-strict-floating-point \
+// RUN: -Xclang -triple -Xclang x86_64-linux-gnu -Xclang -mreassociate \
+// RUN: -ffp-eval-method=source -Xclang -verify %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=CHECK-ASSOC
+
+// RUN: not %clang -Xclang -fexperimental-strict-floating-point \
+// RUN: -Xclang -triple -Xclang x86_64-linux-gnu -Xclang -freciprocal-math \
+// RUN: -ffp-eval-method=source -Xclang -verify %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=CHECK-RECPR
+
+// RUN: not %clang -Xclang -fexperimental-strict-floating-point \
+// RUN: -Xclang -triple -Xclang x86_64-linux-gnu -Xclang -freciprocal-math \
+// RUN: -Xclang -mreassociate -ffp-eval-method=source -Xclang -verify %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=CHECK-ASSOC,CHECK-RECPR
+
+// RUN: not %clang -Xclang -fexperimental-strict-floating-point \
+// RUN: -Xclang -triple -Xclang x86_64-linux-gnu -Xclang -freciprocal-math \
+// RUN: -Xclang -mreassociate -fapprox-func -ffp-eval-method=source \
+// RUN: -Xclang -verify %s 2>&1 \
+// RUN: | FileCheck %s --check-prefixes=CHECK-ASSOC,CHECK-RECPR,CHECK-FUNC
+
+// CHECK-FUNC: (frontend): option 'ffp-eval-method' cannot be used with option 'fapprox-func'
+// CHECK-ASSOC: (frontend): option 'ffp-eval-method' cannot be used with option 'mreassociate'
+// CHECK-RECPR: (frontend): option 'ffp-eval-method' cannot be used with option 'freciprocal'
Index: clang/test/CodeGen/X86/32bit-behavior.c

[PATCH] D119609: [Clang][Sema] Prohibit expr statement in the default argument

[Jun Zhang via Phabricator via cfe-commits]

junaire added inline comments.



Comment at: clang/test/Sema/err-expr-stmt-in-default-arg.cpp:10
+
+  void fn(int i, int j = ({{}, {}, {,}}), int k = ""); // expected-error 
{{expression statement not permitted in default argument}} expected-error 
{{cannot initialize a parameter of type 'int' with an lvalue of type 'const 
char[1]'}} expected-note {{passing argument to parameter 'k' here}}
+}

@erichkeane Do you mean this one? Or I get you wrong?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119609

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

[PATCH] D122955: [clang] NFC: Enhance comments in CodeGen for multiversion function support.

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/include/clang/Basic/Attr.td:2730
+// used for the "default" variant be the largest mangling ID in the
+// variant set. Note that, if duplicate variants are present in
+// 'featuresStrs', that each instance will be assigned its own unique

this comma doesn't read right to me, I don't think it is required.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122955

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

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

In D122820#3426132 , @erichkeane 
wrote:

> In D122820#3423712 , @thakis wrote:
>
>> Looks like this breaks tests on macOS: 
>> http://45.33.8.238/macm1/31733/step_7.txt
>>
>> Please take a look, and revert for now if it takes a while to fix.
>
> That is sad, it seems that llvm-cxxfilt isn't working right on some of the 
> machines :/

Oh hm, if it's due to the wrong llvm-cxxfilt being called, maybe we just need 
to update clang/test/lit.cfg.py to add llvm-cxxfilt to `tools`  to make sure it 
uses the just-built one instead of the one on PATH (? not sure).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122820

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

[PATCH] D121233: [pseudo] Move pseudoparser from clang to clang-tools-extra

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

In D121233#3426189 , @bjope wrote:

> In D121233#3426155 , @sammccall 
> wrote:
>
>> In D121233#3425793 , @bjope wrote:
>>
>>> Have had problems with failing stage 2 builds since this patch:
>>
>> Should be fixed in 72ae6cc3a689869dcc15cfa8d0abcdd35a35a484 
>> , 
>> please let me know if not.
>
> Yes, you are right. Fantastic finding without me even providing the cmake 
> command.
> But it was true that for the failing stage we had disabled tests using 
> `-DLLVM_INCLUDE_TESTS=OFF` which result in CLANG_INCLUDE_TESTS being OFF as 
> well. So your fix to respect the CLANG_INCLUDE_TESTS setting does indeed 
> solve the problem. Thanks alot for the help!

No worries, sorry for the break!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121233

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

[PATCH] D122958: [clang] Corrections for target_clones multiversion functions.

[Erich Keane via Phabricator via cfe-commits]

erichkeane accepted this revision.
erichkeane added inline comments.
This revision is now accepted and ready to land.



Comment at: clang/lib/CodeGen/CodeGenModule.cpp:3492
 
-if (TI.supportsIFunc() || FD->isTargetMultiVersion()) {
-  ResolverFunc = cast(
-  GetGlobalValue((getMangledName(GD) + ".resolver").str()));
-  ResolverFunc->setLinkage(getMultiversionLinkage(*this, GD));
-} else {
-  ResolverFunc = cast(GetGlobalValue(getMangledName(GD)));
-}
+ResolverFunc->setLinkage(getMultiversionLinkage(*this, GD));
 

tahonermann wrote:
> `setLinkage()` was previously only called in the ifunc case. I don't know why 
> that would be, so I "fixed" it here.
Likely that was the only place we 'noticed' it was a problem?  


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122958

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

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

[Nico Weber via Phabricator via cfe-commits]

thakis added a comment.

> Oh hm, if it's due to the wrong llvm-cxxfilt being called, maybe we just need 
> to update clang/test/lit.cfg.py to add llvm-cxxfilt to `tools`  to make sure 
> it uses the just-built one instead of the one on PATH (? not sure).

Actually, I consistently saw this failing on all our mac bots, and as far as I 
can tell they _don't_ have llvm-cxxfilt on PATH. Maybe it's due to macOS 
prepending a `_` by default and `llvm-cxxfilt` insisting on host underscoriness?

  % out/gn/bin/llvm-cxxfilt _Z4funcPci
  _Z4funcPci
  % out/gn/bin/llvm-cxxfilt __Z4funcPcia
  func(char*, int, signed char)

Yes, looks like it. Since you're always passing a linux triple (which doesn't 
add the extra underscore), I think it'll work if you pass `-n` to llvm-cxxfilt 
to force that mode:

  % out/gn/bin/llvm-cxxfilt -n _Z4funcPcia
  func(char*, int, signed char)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122820

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

[PATCH] D122698: [clangd] Add support to extract method for ExtractFunction Tweak

[Sam McCall via Phabricator via cfe-commits]

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

Thanks, this looks great! Just a couple of nits left.
Appreciate you fixing the out-of-line `ns::f()` case too. It's easier to 
understand the fixed logic than the broken logic.

I guess you don't have commit access yet, I can land this for you if you like. 
Let me know your preferred name/email for attribution.




Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:765
+
+  ExtractedFunc.ForwardDeclarationSyntacticDC = ExtractedFunc.SemanticDC =
+  ExtractedFunc.SyntacticDC = ExtZone.EnclosingFunction->getDeclContext();

You're setting/resetting these in lots of different places, but no need for 
that:

SyntacticDC = EnclosingFunction->getLexicalDeclContext()
SemanticDC = getDeclContext();

and set ForwardDeclarationSyntacticDC in captureMethodInfo(), leave it null if 
this isn't a method.



Comment at: clang-tools-extra/clangd/refactor/tweaks/ExtractFunction.cpp:770
+
+  if (isa(ExtZone.EnclosingFunction)) {
+const auto *Method =

this does one check, and is idiomatic in LLVM:

```
if (const auto *Method = llvm::dyn_cast<...>(...))
  captureMethodInfo(...);
```


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

https://reviews.llvm.org/D122698

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

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev created this revision.
kbobyrev added a reviewer: sammccall.
Herald added subscribers: usaxena95, kadircet, arphaman.
Herald added a project: All.
kbobyrev requested review of this revision.
Herald added subscribers: cfe-commits, MaskRay, ilya-biryukov.
Herald added a project: clang-tools-extra.

Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123031

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp


Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -68,9 +68,12 @@
 return false;
   // FIXME(ioeric): resolve the header and store actual file path. For now,
   // we simply assume the written header is suitable to be #included.
-  Includes->addMapping(PP.getSourceManager().getFilename(Range.getBegin()),
-   isLiteralInclude(Text) ? Text.str()
-  : ("\"" + Text + 
"\"").str());
+  auto &SM = PP.getSourceManager();
+  auto Filename =
+  SM.getFileEntryForID(SM.getFileID(Range.getBegin()))->getName();
+  Includes->addMapping(Filename, isLiteralInclude(Text)
+ ? Text.str()
+ : ("\"" + Text + "\"").str());
   return false;
 }
 


Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -68,9 +68,12 @@
 return false;
   // FIXME(ioeric): resolve the header and store actual file path. For now,
   // we simply assume the written header is suitable to be #included.
-  Includes->addMapping(PP.getSourceManager().getFilename(Range.getBegin()),
-   isLiteralInclude(Text) ? Text.str()
-  : ("\"" + Text + "\"").str());
+  auto &SM = PP.getSourceManager();
+  auto Filename =
+  SM.getFileEntryForID(SM.getFileID(Range.getBegin()))->getName();
+  Includes->addMapping(Filename, isLiteralInclude(Text)
+ ? Text.str()
+ : ("\"" + Text + "\"").str());
   return false;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] a97e309 - fix comment typos to cycle bots

[Nico Weber via cfe-commits]

Author: Nico Weber
Date: 2022-04-04T10:18:08-04:00
New Revision: a97e3097cfdfea5a20d3b867571c42ffa0666b01

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

LOG: fix comment typos to cycle bots

Added: 


Modified: 
clang/include/clang/AST/DeclTemplate.h
clang/include/clang/AST/NestedNameSpecifier.h
clang/include/clang/AST/Stmt.h
clang/include/clang/CrossTU/CrossTranslationUnit.h
clang/include/clang/Index/IndexingOptions.h

Removed: 




diff  --git a/clang/include/clang/AST/DeclTemplate.h 
b/clang/include/clang/AST/DeclTemplate.h
index 0459ee8fb6164..ca3a8030ffc61 100644
--- a/clang/include/clang/AST/DeclTemplate.h
+++ b/clang/include/clang/AST/DeclTemplate.h
@@ -1109,7 +1109,7 @@ class FunctionTemplateDecl : public 
RedeclarableTemplateDecl {
   bool isAbbreviated() const {
 // Since the invented template parameters generated from 'auto' parameters
 // are either appended to the end of the explicit template parameter list 
or
-// form a new template paramter list, we can simply observe the last
+// form a new template parameter list, we can simply observe the last
 // parameter to determine if such a thing happened.
 const TemplateParameterList *TPL = getTemplateParameters();
 return TPL->getParam(TPL->size() - 1)->isImplicit();

diff  --git a/clang/include/clang/AST/NestedNameSpecifier.h 
b/clang/include/clang/AST/NestedNameSpecifier.h
index eb01780598a7f..3b6cf97211850 100644
--- a/clang/include/clang/AST/NestedNameSpecifier.h
+++ b/clang/include/clang/AST/NestedNameSpecifier.h
@@ -162,7 +162,7 @@ class NestedNameSpecifier : public llvm::FoldingSetNode {
   /// Return the prefix of this nested name specifier.
   ///
   /// The prefix contains all of the parts of the nested name
-  /// specifier that preced this current specifier. For example, for a
+  /// specifier that precede this current specifier. For example, for a
   /// nested name specifier that represents "foo::bar::", the current
   /// specifier will contain "bar::" and the prefix will contain
   /// "foo::".

diff  --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
index adde424c938f7..1135981319c1a 100644
--- a/clang/include/clang/AST/Stmt.h
+++ b/clang/include/clang/AST/Stmt.h
@@ -594,7 +594,7 @@ class alignas(void *) Stmt {
 unsigned : NumExprBits;
 
 /// The kind of source location builtin represented by the SourceLocExpr.
-/// Ex. __builtin_LINE, __builtin_FUNCTION, ect.
+/// Ex. __builtin_LINE, __builtin_FUNCTION, etc.
 unsigned Kind : 3;
   };
 
@@ -1244,7 +1244,7 @@ class alignas(void *) Stmt {
   }
 
   /// Child Iterators: All subclasses must implement 'children'
-  /// to permit easy iteration over the substatements/subexpessions of an
+  /// to permit easy iteration over the substatements/subexpressions of an
   /// AST node.  This permits easy iteration over all nodes in the AST.
   using child_iterator = StmtIterator;
   using const_child_iterator = ConstStmtIterator;

diff  --git a/clang/include/clang/CrossTU/CrossTranslationUnit.h 
b/clang/include/clang/CrossTU/CrossTranslationUnit.h
index a41c32d4068b2..f94f246e6e32a 100644
--- a/clang/include/clang/CrossTU/CrossTranslationUnit.h
+++ b/clang/include/clang/CrossTU/CrossTranslationUnit.h
@@ -228,7 +228,7 @@ class CrossTranslationUnitContext {
   StringRef InvocationListFilePath);
 
 /// Load the ASTUnit by its identifier found in the index file. If the
-/// indentifier is suffixed with '.ast' it is considered a dump. Otherwise
+/// identifier is suffixed with '.ast' it is considered a dump. Otherwise
 /// it is treated as source-file, and on-demand parsed. Relative paths are
 /// prefixed with CTUDir.
 LoadResultTy load(StringRef Identifier);

diff  --git a/clang/include/clang/Index/IndexingOptions.h 
b/clang/include/clang/Index/IndexingOptions.h
index d19653848d59f..97847dd7d5d88 100644
--- a/clang/include/clang/Index/IndexingOptions.h
+++ b/clang/include/clang/Index/IndexingOptions.h
@@ -29,9 +29,9 @@ struct IndexingOptions {
   bool IndexFunctionLocals = false;
   bool IndexImplicitInstantiation = false;
   bool IndexMacros = true;
-  // Whether to index macro definitions in the Preprocesor when preprocessor
+  // Whether to index macro definitions in the Preprocessor when preprocessor
   // callback is not available (e.g. after parsing has finished). Note that
-  // macro references are not available in Proprocessor.
+  // macro references are not available in Preprocessor.
   bool IndexMacrosInPreprocessor = false;
   // Has no effect if IndexFunctionLocals are false.
   bool IndexParametersInDeclarations = false;



___
cfe-commits mailing list
cfe-commits@lists.llvm.org
h

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Erich Keane via Phabricator via cfe-commits]

erichkeane added subscribers: MattDevereau, david-arm, paulwalker-arm, 
sdesmalen.
erichkeane added a comment.

I don't have any real comments (the changes are quite trivial), and I really 
like the approach here, I think it is more than generous.  I see the transition 
of 
C89/C99: Warn
C11/C17: Warn-as-default-error
C2x: Hard error

as a really gentle and explainable behavior, its just a shame it took us this 
long to do so.

I DO see that there are a ton of AArch64 Intrin tests that seem to have the 
same problems here (in Pre-commit-CI), that will need this change too, though 
they are questionably written.  I might suggest just adding the -std=c99 flag 
to all of those.  Attn: @paulwalker-arm @MattDevereau @david-arm @sdesmalen




Comment at: clang/docs/ReleaseNotes.rst:114
+- The ``-Wimplicit-function-declaration`` warning diagnostic now defaults to
+  emitting an error (which can be downgraded with ``-Wno-error``) in C11 and
+  C17 mode. This is because the feature was removed in C99 and cannot be

xbolva00 wrote:
> I would mention explicitly  -Wno-error= implicit-function-declaration as 
> -Wno-error is a big hammer.
I think this is a valuable change, as suggested by @xbolva00 


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

https://reviews.llvm.org/D122983

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

[PATCH] D123032: [clang][dataflow] Exclude protobuf types from modeling in the environment.

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: xazax.hun, sgatev.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

Google's protobufs are often quite large and their internal state is never worth
modeling in the environment. Exclude them during value construction.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123032

Files:
  clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
  clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/DataflowEnvironmentTest.cpp
@@ -20,8 +20,8 @@
 
 using namespace clang;
 using namespace dataflow;
-using ::testing::ElementsAre;
-using ::testing::Pair;
+using ::testing::IsNull;
+using ::testing::NotNull;
 
 class EnvironmentTest : public ::testing::Test {
   DataflowAnalysisContext Context;
@@ -98,4 +98,70 @@
   EXPECT_NE(PV, nullptr);
 }
 
+TEST_F(EnvironmentTest, ExcludeProtobufTypes) {
+  using namespace ast_matchers;
+
+  std::string Code = R"cc(
+namespace proto2 {
+struct Message {};
+}
+
+namespace google {
+namespace protobuf {
+struct Message {};
+}
+}
+
+namespace other {
+namespace google {
+namespace protobuf {
+struct Message {};
+}
+}
+}
+
+struct Foo : public proto2::Message {
+  bool Field;
+};
+
+struct Bar : public google::protobuf::Message {
+  bool Field;
+};
+
+// Not a protobuf, but looks like it. Verify that it is *not* excluded.
+struct Zab : public other::google::protobuf::Message {
+  bool Field;
+};
+
+void target() {
+  Foo F;
+  Bar B;
+  Zab Z;
+  (void)0;
+  /*[[check]]*/
+}
+  )cc";
+
+  auto Unit =
+  tooling::buildASTFromCodeWithArgs(Code, {"-fsyntax-only", "-std=c++11"});
+  auto &Context = Unit->getASTContext();
+
+  ASSERT_EQ(Context.getDiagnostics().getClient()->getNumErrors(), 0U);
+
+  auto Results = match(varDecl(eachOf(varDecl(hasName("F")).bind("varF"),
+  varDecl(hasName("B")).bind("varB"),
+  varDecl(hasName("Z")).bind("varZ"))),
+   Context);
+  const auto *F = selectFirst("varF", Results);
+  ASSERT_TRUE(F != nullptr);
+  const auto *B = selectFirst("varB", Results);
+  ASSERT_TRUE(B != nullptr);
+  const auto *Z = selectFirst("varZ", Results);
+  ASSERT_TRUE(Z != nullptr);
+
+  EXPECT_THAT(Env.createValue(F->getType()), IsNull());
+  EXPECT_THAT(Env.createValue(B->getType()), IsNull());
+  EXPECT_THAT(Env.createValue(Z->getType()), NotNull());
+}
+
 } // namespace
Index: clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
===
--- clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
+++ clang/lib/Analysis/FlowSensitive/DataflowEnvironment.cpp
@@ -493,6 +493,41 @@
   return Val;
 }
 
+// Hard code certain types to exclude from modeling. Currently, we limit to
+// Google protobufs, since they can be very large, and have no value in
+// modeling.
+static bool isExcludedRecordType(const RecordType *Ty) {
+  const auto *CD = dyn_cast(Ty->getDecl());
+  if (CD == nullptr || !CD->hasDefinition() || CD->getNumBases() != 1)
+return false;
+
+  QualType BQ = CD->bases_begin()->getType();
+  if (BQ.isNull())
+return false;
+
+  const RecordType *BaseTy = BQ->getAs();
+  if (BaseTy == nullptr)
+return false;
+
+  const RecordDecl *RD = BaseTy->getDecl();
+  if (RD->getIdentifier() == nullptr || RD->getName() != "Message")
+return false;
+
+  const auto *ND = dyn_cast(RD->getDeclContext());
+  if (ND == nullptr || ND->getIdentifier() == nullptr)
+return false;
+  if (ND->getName() == "proto2")
+return true;
+
+  // Check for `::google::protobuf`:
+  if (ND->getName() != "protobuf")
+return false;
+
+  ND = dyn_cast(ND->getDeclContext());
+  return ND != nullptr && ND->getParent()->isTranslationUnit() &&
+ ND->getIdentifier() != nullptr && ND->getName() == "google";
+}
+
 Value *Environment::createValueUnlessSelfReferential(
 QualType Type, llvm::DenseSet &Visited, int Depth,
 int &CreatedValuesCount) {
@@ -549,7 +584,8 @@
 return &takeOwnership(std::make_unique(PointeeLoc));
   }
 
-  if (Type->isStructureOrClassType()) {
+  if (Type->isStructureOrClassType() &&
+  !isExcludedRecordType(Type->getAs())) {
 CreatedValuesCount++;
 // FIXME: Initialize only fields that are accessed in the context that is
 // being analyzed.
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev updated this revision to Diff 420175.
kbobyrev added a comment.

Fix the behavior: get Real Path instead.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123031

Files:
  clang-tools-extra/clangd/index/CanonicalIncludes.cpp


Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -68,9 +68,12 @@
 return false;
   // FIXME(ioeric): resolve the header and store actual file path. For now,
   // we simply assume the written header is suitable to be #included.
-  Includes->addMapping(PP.getSourceManager().getFilename(Range.getBegin()),
-   isLiteralInclude(Text) ? Text.str()
-  : ("\"" + Text + 
"\"").str());
+  auto &SM = PP.getSourceManager();
+  auto Filename = SM.getFileEntryForID(SM.getFileID(Range.getBegin()))
+  ->tryGetRealPathName();
+  Includes->addMapping(Filename, isLiteralInclude(Text)
+ ? Text.str()
+ : ("\"" + Text + "\"").str());
   return false;
 }
 


Index: clang-tools-extra/clangd/index/CanonicalIncludes.cpp
===
--- clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -68,9 +68,12 @@
 return false;
   // FIXME(ioeric): resolve the header and store actual file path. For now,
   // we simply assume the written header is suitable to be #included.
-  Includes->addMapping(PP.getSourceManager().getFilename(Range.getBegin()),
-   isLiteralInclude(Text) ? Text.str()
-  : ("\"" + Text + "\"").str());
+  auto &SM = PP.getSourceManager();
+  auto Filename = SM.getFileEntryForID(SM.getFileID(Range.getBegin()))
+  ->tryGetRealPathName();
+  Includes->addMapping(Filename, isLiteralInclude(Text)
+ ? Text.str()
+ : ("\"" + Text + "\"").str());
   return false;
 }
 
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D119609: [Clang][Sema] Prohibit expr statement in the default argument

[Erich Keane via Phabricator via cfe-commits]

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

I think LGTM on technical perspective, Please don't commit until @aaron.ballman 
confirms he is OK with the direction, or would like to wait longer for GCC.




Comment at: clang/test/Sema/err-expr-stmt-in-default-arg.cpp:3
+
+void foo() {
+  void fn(int i, int = ({ 1; })); // expected-error {{expression statement not 
permitted in default argument}}

erichkeane wrote:
> Do we also prohibit in a template argument?
Template args were already disallowed, right?  Presumably there is a test for 
that?



Comment at: clang/test/Sema/err-expr-stmt-in-default-arg.cpp:10
+
+  void fn(int i, int j = ({{}, {}, {,}}), int k = ""); // expected-error 
{{expression statement not permitted in default argument}} expected-error 
{{cannot initialize a parameter of type 'int' with an lvalue of type 'const 
char[1]'}} expected-note {{passing argument to parameter 'k' here}}
+}

junaire wrote:
> @erichkeane Do you mean this one? Or I get you wrong?
Ah, missed that, thank you!  This is sufficient for my concerns.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119609

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

[PATCH] D122935: [AST] Dont invalidate a ref-type var decl if it has no initializer.

[Sam McCall via Phabricator via cfe-commits]

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

I'd be surprised if this *doesn't* break some fragile assumption somewhere, but 
it's a good change.

For the record, the setInvalid was added in 
e6565625f456123ad6eb33c84c8100d8f9411192. The other setInvalids from that patch:

- are for variables with incomplete/abstract types ==> these are correct I think
- are for init-sequence failures ==> these were removed by you in 
9657385960350150b77ed652175b4c3801abd7fa 
 and a 
forked case in 89d9912cbf45068770ac8c1e2ef97b74c3b662ab 


So this feels like tying up a loose end consistent with prior work 👍


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122935

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

[PATCH] D119609: [Clang][Sema] Prohibit expr statement in the default argument

[Jun Zhang via Phabricator via cfe-commits]

junaire updated this revision to Diff 420178.
junaire retitled this revision from " [Clang][Sema] Prohibit expr statement in 
the default argument" to "[Clang][Sema] Prohibit expr statement in the default 
argument".
junaire edited the summary of this revision.
junaire added a comment.

Add tests for template arguments.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119609

Files:
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Parse/ParseDecl.cpp
  clang/lib/Parse/ParseTemplate.cpp
  clang/test/Sema/err-expr-stmt-in-default-arg.cpp
  clang/test/SemaTemplate/dependent-expr.cpp

Index: clang/test/SemaTemplate/dependent-expr.cpp
===
--- clang/test/SemaTemplate/dependent-expr.cpp
+++ clang/test/SemaTemplate/dependent-expr.cpp
@@ -141,15 +141,7 @@
   using U = float; // expected-error {{different types ('float' vs 'decltype(g())' (aka 'void'))}}
 
   void h(auto a, decltype(g())*) {} // expected-note {{previous}}
-  void h(auto a, void*) {} // expected-error {{redefinition}}
-
-  void i(auto a) {
-[](auto a, int = ({decltype(a) i; i * 2;})){}(a); // expected-error {{invalid operands to binary expression ('decltype(a)' (aka 'void *') and 'int')}} expected-note {{in instantiation of}}
-  }
-  void use_i() {
-i(0);
-i((void*)0); // expected-note {{instantiation of}}
-  }
+  void h(auto a, void *) {} // expected-error {{redefinition}}
 }
 
 namespace BindingInStmtExpr {
Index: clang/test/Sema/err-expr-stmt-in-default-arg.cpp
===
--- /dev/null
+++ clang/test/Sema/err-expr-stmt-in-default-arg.cpp
@@ -0,0 +1,29 @@
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++20
+
+void foo() {
+  void fn(int i, int = ({ 1; })); // expected-error {{expression statement not permitted in default argument}}
+
+  auto a = [](int = ({ 1; })) {}; // expected-error {{expression statement not permitted in default argument}}
+
+  auto b = [](){}; // expected-error {{expression statement not permitted in default argument}}
+
+  void fn(int i, int j = ({{}, {}, {,}}), int k = ""); // expected-error {{expression statement not permitted in default argument}} expected-error {{cannot initialize a parameter of type 'int' with an lvalue of type 'const char[1]'}} expected-note {{passing argument to parameter 'k' here}}
+}
+
+template // expected-error {{expression statement not permitted in default argument}}
+void f() {}
+
+template // expected-error {{expression statement not permitted in default argument}}
+class S{};
+
+template 
+int bar(Callable &&Call) {
+  return Call();
+}
+
+int baz() {
+  auto l = [](int a = ({ int x = 12; x; })) { // expected-error {{expression statement not permitted in default argument}}
+return 1;
+  };
+  return bar(l);
+}
Index: clang/lib/Parse/ParseTemplate.cpp
===
--- clang/lib/Parse/ParseTemplate.cpp
+++ clang/lib/Parse/ParseTemplate.cpp
@@ -19,6 +19,7 @@
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
+#include "clang/Sema/SemaDiagnostic.h"
 #include "llvm/Support/TimeProfiler.h"
 using namespace clang;
 
@@ -1007,18 +1008,23 @@
   SourceLocation EqualLoc;
   ExprResult DefaultArg;
   if (TryConsumeToken(tok::equal, EqualLoc)) {
-// C++ [temp.param]p15:
-//   When parsing a default template-argument for a non-type
-//   template-parameter, the first non-nested > is taken as the
-//   end of the template-parameter-list rather than a greater-than
-//   operator.
-GreaterThanIsOperatorScope G(GreaterThanIsOperator, false);
-EnterExpressionEvaluationContext ConstantEvaluated(
-Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
-
-DefaultArg = Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression());
-if (DefaultArg.isInvalid())
+if (Tok.is(tok::l_paren) && NextToken().is(tok::l_brace)) {
+  Diag(Tok.getLocation(), diag::err_expr_statement_in_default_arg);
   SkipUntil(tok::comma, tok::greater, StopAtSemi | StopBeforeMatch);
+} else {
+  // C++ [temp.param]p15:
+  //   When parsing a default template-argument for a non-type
+  //   template-parameter, the first non-nested > is taken as the
+  //   end of the template-parameter-list rather than a greater-than
+  //   operator.
+  GreaterThanIsOperatorScope G(GreaterThanIsOperator, false);
+  EnterExpressionEvaluationContext ConstantEvaluated(
+  Actions, Sema::ExpressionEvaluationContext::ConstantEvaluated);
+  DefaultArg =
+  Actions.CorrectDelayedTyposInExpr(ParseAssignmentExpression());
+  if (DefaultArg.isInvalid())
+SkipUntil(tok::comma, tok::greater, StopAtSemi | StopBeforeMatch);
+}
   }
 
   // Create the parameter.
Index: clang/lib/Parse/ParseDecl.cpp
==

[PATCH] D122820: [GH54588]Fix ItaniumMangler for NTTP unnamed unions w/ unnamed structs

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D122820#3426211 , @thakis wrote:

>> Oh hm, if it's due to the wrong llvm-cxxfilt being called, maybe we just 
>> need to update clang/test/lit.cfg.py to add llvm-cxxfilt to `tools`  to make 
>> sure it uses the just-built one instead of the one on PATH (? not sure).
>
> Actually, I consistently saw this failing on all our mac bots, and as far as 
> I can tell they _don't_ have llvm-cxxfilt on PATH. Maybe it's due to macOS 
> prepending a `_` by default and `llvm-cxxfilt` insisting on host 
> underscoriness?
>
>   % out/gn/bin/llvm-cxxfilt _Z4funcPci
>   _Z4funcPci
>   % out/gn/bin/llvm-cxxfilt __Z4funcPcia
>   func(char*, int, signed char)
>
> Yes, looks like it. Since you're always passing a linux triple (which doesn't 
> add the extra underscore), I think it'll work if you pass `-n` to 
> llvm-cxxfilt to force that mode:
>
>   % out/gn/bin/llvm-cxxfilt -n _Z4funcPcia
>   func(char*, int, signed char)

Ah, great, thank you for looking into that!  I already re-committed with the 
test line disabled and a FIXME(since we're testing the 'mangled' names 
already!), but I'll try a commit with this -n to re-enable the line to see if 
this fixes it (and be quick about reverting it myself if I see the errors come 
across.

Thank you again for your help!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122820

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

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

This is required for D120306  which was 
landed but later reverted because of the failing Windows tests (different slash 
types for filenames requested during preamble parsing and in the main file).


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123031

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

[clang] b1ed286 - Attempt to re-enable demangle test in mangle-nttp-anon-union

[Erich Keane via cfe-commits]

Author: Erich Keane
Date: 2022-04-04T07:28:29-07:00
New Revision: b1ed28685766e6632938ea47d1d60c3028ebcfcf

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

LOG: Attempt to re-enable demangle test in mangle-nttp-anon-union

@thakis believes the problem was the lack of -n on my llvm-cxxfilt call,
so hopefully this is the only problem. Committing to see if this makes
all the buildbots happy.

Added: 


Modified: 
clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp

Removed: 




diff  --git a/clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp 
b/clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp
index 4382d1406cfec..7577ef8f2561f 100644
--- a/clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp
+++ b/clang/test/CodeGenCXX/mangle-nttp-anon-union.cpp
@@ -1,9 +1,5 @@
 // RUN: %clang_cc1 -std=c++20 -emit-llvm %s -o - -triple=x86_64-linux-gnu | 
FileCheck %s
-
-// FIXME: In the future we could possibly do this to get nicer looking output
-// for this, but it seems that some of the bots don't have a recent enough
-// version of llvm-cxxfilt to demangle these.
-// %clang_cc1 -std=c++20 -emit-llvm %s -o - -triple=x86_64-linux-gnu | 
llvm-cxxfilt | FileCheck %s --check-prefix DEMANGLED
+// RUN: %clang_cc1 -std=c++20 -emit-llvm %s -o - -triple=x86_64-linux-gnu | 
llvm-cxxfilt -n | FileCheck %s --check-prefix DEMANGLED
 
 template
 struct wrapper1 {



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

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

note that you seem to be using `auto PublicHeader = 
CanonIncludes.mapHeader(Entry->getName());` (i.e. `getName`) in D120306 
 and not `tryGetRealPathName`. it might be 
nice to have a test case here, at least one that only fails/runs on windows


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123031

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

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D123031#3426303 , @kadircet wrote:

> note that you seem to be using `auto PublicHeader = 
> CanonIncludes.mapHeader(Entry->getName());` (i.e. `getName`) in D120306 
>  and not `tryGetRealPathName`. it might be 
> nice to have a test case here, at least one that only fails/runs on windows

Right! I think I also need to do that in `SymbolCollector.cpp`.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123031

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

[PATCH] D121599: [AST] Better recovery on an expression refers to an invalid decl.

[Sam McCall via Phabricator via cfe-commits]

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

Diagnostic changes look great!




Comment at: clang/lib/Sema/SemaDecl.cpp:1230
   LookupResult Result(*this, Name, NameLoc, LookupOrdinaryName);
   return BuildDeclarationNameExpr(SS, Result, /*ADL=*/true);
 }

wonder if if the results of setting AcceptInvalidDecl here would be good/bad?
(happy with in this patch/separate one/not at all, just curious)

Also possible candidates are the calls in:
 - Sema::ActOnIdExpression
 - Sema::BuildQualifiedDeclarationNameExpr
 - Sema::BuildPossibleImplicitMemberExpr
 - BuildRecoveryCallExpr in SemaOverload




Comment at: clang/lib/Sema/SemaExpr.cpp:3156
   if (D->isInvalidDecl())
-return true;
+return !AcceptInvalid;
 

if AcceptInvalid is true here, why do we skip all the rest of the checks?

Seems like in that case there's not much point calling the function at all...

My suspicion is that if we resolve to e.g. an invalid typedef then the 
diagnostic is good.



Comment at: clang/lib/Sema/SemaExpr.cpp:3473
+   /*FIXME: TemplateKWLoc*/ SourceLocation(), 
TemplateArgs);
+  // Many clang AST consumers assume a DeclRefExpr refers to a valid decl. We
+  // wrap a DeclRefExpr referring to an invalid decl with a dependent-type

Maybe drop the "many", which kind of implies they're incorrect to do so?



Comment at: clang/lib/Sema/SemaExpr.cpp:3478
+  if (VD->isInvalidDecl() && E)
+return CreateRecoveryExpr(E->getBeginLoc(), E->getEndLoc(), {E});
+  return E;

This means we're changing the AST returned for existing 
`AcceptInvalidDecl=true` callers, right?

I think this is just attemptRecovery() in SemaCXX.cpp, which is part of typo 
correction. Previously we might transforming typos into DeclRefExpr to invalid, 
but now we're transforming them to RecoveryExpr wrapping DeclExpr to invalid.

This seems sensible, but I'm a little concerned there are no test changes for 
it. Is it possible to construct one?

I kind of expected this to work:
```
m *foo;
void z() {
  goo;
}
```
but in fact we produce an opaque RecoveryExpr (with no DeclRefExpr) inside 
today.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D121599

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

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

[Kadir Cetinkaya via Phabricator via cfe-commits]

kadircet added a comment.

> Right! I think I also need to do that in SymbolCollector.cpp.

Now that you mentioned this too, it feels like the issue is not about being 
consistent, as I think all of the places that we have today are actually making 
use of `FileEntry::getName` as keys to 
`CanonicalIncludes::{addMapping,mapHeader}`. Are we sure we're not relying on 
some sort of canonization of file path (AFAICT, that's not the case the only 
canonization that's happening is dropping `..`) when you use 
`tryGetRealPathName` instead of `getName`?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123031

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

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added a comment.

In D123031#3426382 , @kadircet wrote:

>> Right! I think I also need to do that in SymbolCollector.cpp.
>
> Now that you mentioned this too, it feels like the issue is not about being 
> consistent, as I think all of the places that we have today are actually 
> making use of `FileEntry::getName` as keys to 
> `CanonicalIncludes::{addMapping,mapHeader}`. Are we sure we're not relying on 
> some sort of canonization of file path (AFAICT, that's not the case the only 
> canonization that's happening is dropping `..`) when you use 
> `tryGetRealPathName` instead of `getName`?

Right, I think the name of the patch is misleading. "Consistent" means 
"consistency across different SM/FMs" rather than consistency across the call 
sites.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123031

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

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

[Sam McCall via Phabricator via cfe-commits]

sammccall added a comment.

Changing the strings here makes sense, but I'm confused by the use of 
tryGetRealPathName.
Also, we ended up dropping the filename in favor of UniqueID in 
IncludeStructure, so that's an alternative here.

Yes, I think we need a test here with a scope wide enough that it tests that 
the callsites of addMapping/getMapping line up.




Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:73
+  auto Filename = SM.getFileEntryForID(SM.getFileID(Range.getBegin()))
+  ->tryGetRealPathName();
+  Includes->addMapping(Filename, isLiteralInclude(Text)

offline we had discussed getName rather than tryGetRealPathName instead.

The comment in HeaderStructure says specifically that we avoid 
tryGetRealPathName because it's not preserved in the preamble.
I suspect that this will give incorrect results in cases involving non-preamble 
`#includes`.

In any case we should have a comment somewhere on CanonicalIncludes about how 
we're identifying files, and why it's stable enough. The lack of such a comment 
is really what led to the bug on friday :-)



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:73
+  auto Filename = SM.getFileEntryForID(SM.getFileID(Range.getBegin()))
+  ->tryGetRealPathName();
+  Includes->addMapping(Filename, isLiteralInclude(Text)

sammccall wrote:
> offline we had discussed getName rather than tryGetRealPathName instead.
> 
> The comment in HeaderStructure says specifically that we avoid 
> tryGetRealPathName because it's not preserved in the preamble.
> I suspect that this will give incorrect results in cases involving 
> non-preamble `#includes`.
> 
> In any case we should have a comment somewhere on CanonicalIncludes about how 
> we're identifying files, and why it's stable enough. The lack of such a 
> comment is really what led to the bug on friday :-)
tryGetRealPathName can be the empty string, what do we do in such cases?

(In practice I think this is going to happen in the case where you have a 
non-preamble `#include` of a header-guarded file which was also part of the 
preamble) 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123031

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

[PATCH] D122965: Corrected A Command

[Priyansh Singh via Phabricator via cfe-commits]

ps-19 marked 2 inline comments as done.
ps-19 added a comment.

Can anyone please review the patch?




Comment at: clang/test/CodeGen/c-unicode.c:5
 int \uaccess = 0;
 // ALLOWED: "곎ss":
 // ALLOWED-NOT: "\uaccess":

smeenai wrote:
> ps-19 wrote:
> > Comment Line No: 5 is // ALLOWED: "곎ss": i think "곎ss" in istake here, 
> > according to google it is written in korean and i can not understand it's 
> > meaning, but it should be made more concise and readable for more user.
> Given that the test is called `c-unicode.c`, I imagine it's intentional.
Yes i too was thinking the same.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122965

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

[PATCH] D123031: [clangd] Use consistent header paths in CanonicalIncludes mappings

[Kirill Bobyrev via Phabricator via cfe-commits]

kbobyrev added inline comments.



Comment at: clang-tools-extra/clangd/index/CanonicalIncludes.cpp:73
+  auto Filename = SM.getFileEntryForID(SM.getFileID(Range.getBegin()))
+  ->tryGetRealPathName();
+  Includes->addMapping(Filename, isLiteralInclude(Text)

sammccall wrote:
> sammccall wrote:
> > offline we had discussed getName rather than tryGetRealPathName instead.
> > 
> > The comment in HeaderStructure says specifically that we avoid 
> > tryGetRealPathName because it's not preserved in the preamble.
> > I suspect that this will give incorrect results in cases involving 
> > non-preamble `#includes`.
> > 
> > In any case we should have a comment somewhere on CanonicalIncludes about 
> > how we're identifying files, and why it's stable enough. The lack of such a 
> > comment is really what led to the bug on friday :-)
> tryGetRealPathName can be the empty string, what do we do in such cases?
> 
> (In practice I think this is going to happen in the case where you have a 
> non-preamble `#include` of a header-guarded file which was also part of the 
> preamble) 
Oh, okay, I thought `tryGetRealPathName` would not be empty here :(

The problem with `getName` is that it turned out to produce the same results 
that we already get right now through SourceManager.

That leaves us with other ways of canonicalization: either through conversions 
to the native path or `FileManager::getCanonicalName()`. Both are expensive and 
I think the problem is that though the mapping here would be OK to generate in 
an expensive manner, the callsites would not be very happy with that but I 
guess it's acceptable.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123031

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

[PATCH] D122965: Corrected A Command

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

In D122965#3426403 , @ps-19 wrote:

> Can anyone please review the patch?

FYI: please only ping the patch after about a week has gone by with no review 
(https://llvm.org/docs/CodeReview.html#lgtm-how-a-patch-is-accepted). Reviewers 
get to reviews as we're able to, but it's quite common for there to be a few 
days between reviews.

That said, the changes here LGTM (though I can't fathom why this was a CodeGen 
test to begin with -- it looks like it should have been a Parser or Sema test 
and using `-verify`)


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122965

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

[PATCH] D122965: Corrected A Command

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122965#3426412 , @aaron.ballman 
wrote:

> In D122965#3426403 , @ps-19 wrote:
>
>> Can anyone please review the patch?
>
> FYI: please only ping the patch after about a week has gone by with no review 
> (https://llvm.org/docs/CodeReview.html#lgtm-how-a-patch-is-accepted). 
> Reviewers get to reviews as we're able to, but it's quite common for there to 
> be a few days between reviews.
>
> That said, the changes here LGTM (though I can't fathom why this was a 
> CodeGen test to begin with -- it looks like it should have been a Parser or 
> Sema test and using `-verify`)

Oops, forgot to ask -- what name and email address would you like for patch 
attribution?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122965

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

[PATCH] D122965: Corrected A Command

[Priyansh Singh via Phabricator via cfe-commits]

ps-19 marked an inline comment as done.
ps-19 added a comment.

Priyansh Singh
priyansh.singh...@gmail.com


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122965

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked 2 inline comments as done.
aaron.ballman added a comment.

In D122983#3426261 , @erichkeane 
wrote:

> I don't have any real comments (the changes are quite trivial), and I really 
> like the approach here, I think it is more than generous.  I see the 
> transition of 
> C89/C99: Warn
> C11/C17: Warn-as-default-error
> C2x: Hard error
>
> as a really gentle and explainable behavior, its just a shame it took us this 
> long to do so.

Thanks!

> I DO see that there are a ton of AArch64 Intrin tests that seem to have the 
> same problems here (in Pre-commit-CI), that will need this change too, though 
> they are questionably written.  I might suggest just adding the -std=c99 flag 
> to all of those.  Attn: @paulwalker-arm @MattDevereau @david-arm @sdesmalen

Yeah, those tests seem to be overly-constraining. There's no reason to be 
validating whether there's an implicit function declaration warning in a 
*codegen* test. I will change all of those AAarch64 tests to require -std=c99 
explicitly whenever possible, remove the `-verify` flag because there's no 
reason for a codegen test to verify diagnostic behavior that isn't generated by 
the CodeGen library, and remove the `// expected-warning` comments. I plan to 
do that as an NFC change that I'll land outside of this patch, unless any of 
the AArch64 folks speak up pretty quickly.




Comment at: clang/docs/ReleaseNotes.rst:114
+- The ``-Wimplicit-function-declaration`` warning diagnostic now defaults to
+  emitting an error (which can be downgraded with ``-Wno-error``) in C11 and
+  C17 mode. This is because the feature was removed in C99 and cannot be

erichkeane wrote:
> xbolva00 wrote:
> > I would mention explicitly  -Wno-error= implicit-function-declaration as 
> > -Wno-error is a big hammer.
> I think this is a valuable change, as suggested by @xbolva00 
Agreed!


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

https://reviews.llvm.org/D122983

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

[clang] aa19500 - Correct a typo in a RUN line

[Aaron Ballman via cfe-commits]

Author: Priyansh Singh
Date: 2022-04-04T11:32:05-04:00
New Revision: aa19500a4c3a66058078f88b3a7dd7a9ad30be12

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

LOG: Correct a typo in a RUN line

Added: 


Modified: 
clang/test/CodeGen/c-unicode.c

Removed: 




diff  --git a/clang/test/CodeGen/c-unicode.c b/clang/test/CodeGen/c-unicode.c
index 13d4bbfb71e82..3a0c561908c7d 100644
--- a/clang/test/CodeGen/c-unicode.c
+++ b/clang/test/CodeGen/c-unicode.c
@@ -1,5 +1,5 @@
 // REQUIRES: x86-registered-target
-// RUN: %clang --target=x86_64--linug-gnu -S %s -o - | FileCheck %s 
-check-prefix=ALLOWED
+// RUN: %clang --target=x86_64--linux-gnu -S %s -o - | FileCheck %s 
-check-prefix=ALLOWED
 // RUN: not %clang --target=x86_64--linux-gnu -std=c89 -S %s -o - 2>&1 | 
FileCheck %s -check-prefix=DENIED
 int \uaccess = 0;
 // ALLOWED: "곎ss":



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

[PATCH] D123037: [clang][dataflow] Support integral casts

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel created this revision.
ymandel added reviewers: xazax.hun, sgatev.
Herald added subscribers: tschuett, steakhal, rnkovacs.
Herald added a project: All.
ymandel requested review of this revision.
Herald added a project: clang.

Adds support for implicit casts `CK_IntegralCast` and `CK_IntegralToBoolean`.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123037

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1900,11 +1900,87 @@
 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
 ASSERT_THAT(BarDecl, NotNull());
 
-const auto *FooVal =
-cast(Env.getValue(*FooDecl, SkipPast::None));
-const auto *BarVal =
-cast(Env.getValue(*BarDecl, SkipPast::None));
-EXPECT_EQ(FooVal, BarVal);
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, IntegralCast) {
+  std::string Code = R"(
+void target(int Foo) {
+  long Bar = Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, IntegraltoBooleanCast) {
+  std::string Code = R"(
+void target(int Foo) {
+  bool Bar = Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+EXPECT_NE(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, IntegraltoBooleanCastFromBool) {
+  std::string Code = R"(
+void target(bool Foo) {
+  int Zab = Foo;
+  bool Bar = Zab;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
   });
 }
 
@@ -2394,6 +2470,35 @@
   });
 }
 
+// `__builtin_expect` takes and returns a `long` argument, so other types
+// involve casts. This verifies that we identify the input and output in that
+// case.
+TEST_F(TransferTest, BuiltinExpectBoolArg) {
+  std::string Code = R"(
+void target(bool Foo) {
+  bool Bar = __builtin_expect(Foo, true);
+  /*[[p]]*/
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const auto &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+

[PATCH] D122965: Corrected A Command

[Aaron Ballman via Phabricator via cfe-commits]

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

In D122965#3426446 , @ps-19 wrote:

> Priyansh Singh
> priyansh.singh...@gmail.com

Thanks! I've landed on your behalf in aa19500a4c3a66058078f88b3a7dd7a9ad30be12 
.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122965

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

[PATCH] D123037: [clang][dataflow] Support integral casts

[Yitzhak Mandelbaum via Phabricator via cfe-commits]

ymandel updated this revision to Diff 420196.
ymandel added a comment.

fix typo


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123037

Files:
  clang/lib/Analysis/FlowSensitive/Transfer.cpp
  clang/unittests/Analysis/FlowSensitive/TransferTest.cpp

Index: clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
===
--- clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
+++ clang/unittests/Analysis/FlowSensitive/TransferTest.cpp
@@ -1900,11 +1900,87 @@
 const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
 ASSERT_THAT(BarDecl, NotNull());
 
-const auto *FooVal =
-cast(Env.getValue(*FooDecl, SkipPast::None));
-const auto *BarVal =
-cast(Env.getValue(*BarDecl, SkipPast::None));
-EXPECT_EQ(FooVal, BarVal);
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, IntegralCast) {
+  std::string Code = R"(
+void target(int Foo) {
+  long Bar = Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, IntegraltoBooleanCast) {
+  std::string Code = R"(
+void target(int Foo) {
+  bool Bar = Foo;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+EXPECT_NE(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
+TEST_F(TransferTest, IntegralToBooleanCastFromBool) {
+  std::string Code = R"(
+void target(bool Foo) {
+  int Zab = Foo;
+  bool Bar = Zab;
+  // [[p]]
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const Environment &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
   });
 }
 
@@ -2394,6 +2470,35 @@
   });
 }
 
+// `__builtin_expect` takes and returns a `long` argument, so other types
+// involve casts. This verifies that we identify the input and output in that
+// case.
+TEST_F(TransferTest, BuiltinExpectBoolArg) {
+  std::string Code = R"(
+void target(bool Foo) {
+  bool Bar = __builtin_expect(Foo, true);
+  /*[[p]]*/
+}
+  )";
+  runDataflow(Code,
+  [](llvm::ArrayRef<
+ std::pair>>
+ Results,
+ ASTContext &ASTCtx) {
+ASSERT_THAT(Results, ElementsAre(Pair("p", _)));
+const auto &Env = Results[0].second.Env;
+
+const ValueDecl *FooDecl = findValueDecl(ASTCtx, "Foo");
+ASSERT_THAT(FooDecl, NotNull());
+
+const ValueDecl *BarDecl = findValueDecl(ASTCtx, "Bar");
+ASSERT_THAT(BarDecl, NotNull());
+
+EXPECT_EQ(Env.getValue(*FooDecl, SkipPast::None),
+  Env.getValue(*BarDecl, SkipPast::None));
+  });
+}
+
 TEST_F(TransferTest, BuiltinUnreachable) {
   std::string Code = R"(
 void target(bool Foo) {
Index: clang/lib/Analysis/FlowSensitive/Transfer.cpp
=

[PATCH] D119017: [clang] roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre added a comment.

In D119017#3400347 , 
@devin.jeanpierre wrote:

> I don't have easy access to a Windows machine at work so am struggling with 
> the reproduction instructions.

Well, after much procrastinating, and an hour of fiddling with installing 
Visual Studio, I tried on Linux with the following RUN line:

  // RUN: %clang_cc1 -fsyntax-only -verify %s -std=c++11 -triple i686-pc-win32

No joy. So I guess I really do need to reproduce this on Windows!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119017

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

Please consider this "AArch64 folks speaking up".  What are your plans here 
exactly? I have no issue with adding `-std=c99` to the RUN lines, but "remove 
the // expected-warning comments" sounds like a significant loss of test 
coverage.


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

https://reviews.llvm.org/D122983

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman marked an inline comment as done.
aaron.ballman added a comment.

In D122983#3426534 , @paulwalker-arm 
wrote:

> Please consider this "AArch64 folks speaking up".  What are your plans here 
> exactly? I have no issue with adding `-std=c99` to the RUN lines, but "remove 
> the // expected-warning comments" sounds like a significant loss of test 
> coverage.

CodeGen tests don't typically add `-verify` unless the diagnostic is generated 
by the CodeGen library, so if removing the `expected-warning` lines loses test 
coverage, that speaks to a lack of test coverage in Sema. But what, exactly, 
are these tests trying to validate? We have Sema tests which test the behavior 
of "do we want to diagnose this as an implicit declaration or not", so I don't 
see these lines adding any value to the tests.


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

https://reviews.llvm.org/D122983

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

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

Can you guys please help me review this patch?  😁


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

The tests verify a set of builtins do not exist when the associated feature 
flag is not present.  They sit within CodeGen because the tests were plentiful 
and it did not seem worth duplicating them.


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

https://reviews.llvm.org/D122983

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

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

I don't particularly get what the 'fix' is here to Richard's issue.  
Additionally, I think this is showing me the absolute desperate need we have 
for some sort of 'text-checking' tests for this feature, checking vs the IR is 
too unreadable.




Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
LValue RecordLV, CharUnits Align,
-   llvm::FunctionCallee Func, int Lvl) {
+   bool dumpTypeName, llvm::FunctionCallee Func,
+   int Lvl) {

Not a fan of bool parameters, they make calls harder to read.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2132
+
+  // If current field is a record type, we should not dump the type name in
+  // recursive dumpRecord call.

This comment isn't clear to me, can you clarify?  I thought the issue that 
@rsmith reported was that we dumped too types rarely, but it looks like you're 
taking cases where we already dumped the typename and stopped doing so?



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2141
+  TmpRes =
+  dumpRecord(CGF, CanonicalType, FieldLV, Align, false, Func, Lvl + 1);
   Res = CGF.Builder.CreateAdd(TmpRes, Res);





Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2705
 LValue RecordLV = MakeAddrLValue(RecordPtr, Arg0Type, Arg0Align);
-Value *Res = dumpRecord(*this, Arg0Type, RecordLV, Arg0Align,
+Value *Res = dumpRecord(*this, Arg0Type, RecordLV, Arg0Align, true,
 {LLVMFuncType, Func}, 0);




Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D122983#3426450 , @aaron.ballman 
wrote:

> Yeah, those tests seem to be overly-constraining. There's no reason to be 
> validating whether there's an implicit function declaration warning in a 
> *codegen* test. I will change all of those AAarch64 tests to require -std=c99 
> explicitly whenever possible, remove the `-verify` flag because there's no 
> reason for a codegen test to verify diagnostic behavior that isn't generated 
> by the CodeGen library, and remove the `// expected-warning` comments. I plan 
> to do that as an NFC change that I'll land outside of this patch, unless any 
> of the AArch64 folks speak up pretty quickly.

Maybe a smaller hammer can be used here, e.g., 
`-Wno-error=implicit-function-declaration`? The use of the `-verify` flag in a 
CodeGen test to validate that the test is written as "cleanly" as intended is 
something that I am sympathetic to.


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

https://reviews.llvm.org/D122983

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Hubert Tong via Phabricator via cfe-commits]

hubert.reinterpretcast added a comment.

In D122983#3426541 , @paulwalker-arm 
wrote:

> The tests verify a set of builtins do not exist when the associated feature 
> flag is not present.  They sit within CodeGen because the tests were 
> plentiful and it did not seem worth duplicating them.

This sounds like a situation where the test files should have more comments 
near the RUN lines (if the situation is not resolved via a different solution).


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

https://reviews.llvm.org/D122983

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122983#3426541 , @paulwalker-arm 
wrote:

> The tests verify a set of builtins do not exist when the associated feature 
> flag is not present.  They sit within CodeGen because the tests were 
> plentiful and it did not seem worth duplicating them.

You don't need to duplicate hundreds of tests to have appropriate Sema tests 
for this functionality. You could have added a single test file to test/Sema 
that tests each of the builtins you care about the semantic diagnostic behavior 
of. Doing this from codegen on a hundreds of individual tests that don't run 
everywhrere is disruptive because it makes modifying diagnostic behavior 
considerably harder than it should be. We separate codegen and sema tests for a 
reason.

I'll leave the test coverage but add `-std=c99` to the verify lines because 
that will unblock this patch. However, I'd still appreciate if the AArch64 
maintainers would clean up these tests in a more appropriate fashion, because 
adding the -std= line also loses test coverage (especially when we switch the 
default language mode). This will need to happen sooner rather than later 
(relatively speaking; nothing is on fire requiring immediate attention) -- I 
expect we'll bump clang's default from C17 to C23 sometime in the next few 
years, at which point all of these tests will fail to compile because there 
will be no support for implicit function declarations whatsoever.


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

https://reviews.llvm.org/D122983

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

[PATCH] D123045: [clang][extract-api] Fix small issues with SymbolGraphSerializer

[Daniel Grumberg via Phabricator via cfe-commits]

dang created this revision.
dang added reviewers: zixuw, ributzka, QuietMisdreavus.
Herald added a project: All.
dang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This includes:

- replacing "relationhips" with "relationships"
- emitting the "pathComponents" property on symbols
- emitting the "accessLevel" property on symbols


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123045

Files:
  clang/include/clang/ExtractAPI/Serialization/SymbolGraphSerializer.h
  clang/lib/ExtractAPI/Serialization/SymbolGraphSerializer.cpp
  clang/test/ExtractAPI/enum.c
  clang/test/ExtractAPI/global_record.c
  clang/test/ExtractAPI/global_record_multifile.c
  clang/test/ExtractAPI/language.c
  clang/test/ExtractAPI/macro_undefined.c
  clang/test/ExtractAPI/macros.c
  clang/test/ExtractAPI/objc_interface.m
  clang/test/ExtractAPI/objc_protocol.m
  clang/test/ExtractAPI/struct.c

Index: clang/test/ExtractAPI/struct.c
===
--- clang/test/ExtractAPI/struct.c
+++ clang/test/ExtractAPI/struct.c
@@ -48,7 +48,7 @@
   "vendor": "apple"
 }
   },
-  "relationhips": [
+  "relationships": [
 {
   "kind": "memberOf",
   "source": "c:@S@Color@FI@Red",
@@ -72,6 +72,7 @@
   ],
   "symbols": [
 {
+  "accessLevel": "public",
   "declarationFragments": [
 {
   "kind": "keyword",
@@ -112,8 +113,10 @@
 "identifier": "c.struct"
   },
   "location": {
-"character": 8,
-"line": 2,
+"position": {
+  "character": 8,
+  "line": 2
+},
 "uri": "file://INPUT_DIR/input.h"
   },
   "names": {
@@ -124,9 +127,13 @@
   }
 ],
 "title": "Color"
-  }
+  },
+  "pathComponents": [
+"Color"
+  ]
 },
 {
+  "accessLevel": "public",
   "declarationFragments": [
 {
   "kind": "typeIdentifier",
@@ -151,8 +158,10 @@
 "identifier": "c.property"
   },
   "location": {
-"character": 12,
-"line": 3,
+"position": {
+  "character": 12,
+  "line": 3
+},
 "uri": "file://INPUT_DIR/input.h"
   },
   "names": {
@@ -163,9 +172,14 @@
   }
 ],
 "title": "Red"
-  }
+  },
+  "pathComponents": [
+"Color",
+"Red"
+  ]
 },
 {
+  "accessLevel": "public",
   "declarationFragments": [
 {
   "kind": "typeIdentifier",
@@ -190,8 +204,10 @@
 "identifier": "c.property"
   },
   "location": {
-"character": 12,
-"line": 4,
+"position": {
+  "character": 12,
+  "line": 4
+},
 "uri": "file://INPUT_DIR/input.h"
   },
   "names": {
@@ -202,9 +218,14 @@
   }
 ],
 "title": "Green"
-  }
+  },
+  "pathComponents": [
+"Color",
+"Green"
+  ]
 },
 {
+  "accessLevel": "public",
   "declarationFragments": [
 {
   "kind": "typeIdentifier",
@@ -229,8 +250,10 @@
 "identifier": "c.property"
   },
   "location": {
-"character": 12,
-"line": 5,
+"position": {
+  "character": 12,
+  "line": 5
+},
 "uri": "file://INPUT_DIR/input.h"
   },
   "names": {
@@ -241,9 +264,14 @@
   }
 ],
 "title": "Blue"
-  }
+  },
+  "pathComponents": [
+"Color",
+"Blue"
+  ]
 },
 {
+  "accessLevel": "public",
   "declarationFragments": [
 {
   "kind": "typeIdentifier",
@@ -285,8 +313,10 @@
 "identifier": "c.property"
   },
   "location": {
-"character": 12,
-"line": 7,
+"position": {
+  "character": 12,
+  "line": 7
+},
 "uri": "file://INPUT_DIR/input.h"
   },
   "names": {
@@ -297,7 +327,11 @@
   }
 ],
 "title": "Alpha"
-  }
+  },
+  "pathComponents": [
+"Color",
+"Alpha"
+  ]
 }
   ]
 }
Index: clang/test/ExtractAPI/objc_protocol.m
===
--- clang/test/ExtractAPI/objc_protocol.m
+++ clang/test/ExtractAPI/objc_protocol.m
@@ -45,7 +45,7 @@
   "vendor": "apple"
 }
   },
-  "relationhips": [
+  "relationships": [
 {
   "kind": "conformsTo",
   "source": "c:objc(pl)AnotherProtocol",
@@ -54,6 +54,7 @@
   ],
   "symbols": [
 {
+  "accessLevel": "public",
   "declarationFragments": [
 {
   "kind": "keyword",
@@ -77,8 +78,10 @@
 "identifier": "objective-c.protocol"
   },
   "location": {
-"character": 11,
-"line": 1,
+"position": {
+  "character": 11,
+  "line": 1
+},
 "uri": "file://INPUT_DIR/in

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

Thanks for this.  I can see about cleaning up the tests but I'm still not sure 
what the advantage is.  The affected RUN lines are already `-fsyntax-only` 
tests.  Is it about where the test files live or is there something else I 
should be considering?  The benefit of the current tests is that it's easy to 
spot holes as a single function tests both requirements.  My fear is that 
separating the tests based on Sema/CodeGen could mean we'll miss something and 
never know.


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

https://reviews.llvm.org/D122983

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Erich Keane via Phabricator via cfe-commits]

erichkeane added a comment.

In D122983#3426661 , @paulwalker-arm 
wrote:

> Thanks for this.  I can see about cleaning up the tests but I'm still not 
> sure what the advantage is.  The affected RUN lines are already 
> `-fsyntax-only` tests.  Is it about where the test files live or is there 
> something else I should be considering?  The benefit of the current tests is 
> that it's easy to spot holes as a single function tests both requirements.  
> My fear is that separating the tests based on Sema/CodeGen could mean we'll 
> miss something and never know.

There are two big problems with the way these are split up.  First, the 'list' 
of these functions that you expect to not be declared under a flag don't exist 
all in 1 place, and end up being difficult to follow/track down as they are 
spread out as much as they are.  Second, they are in tests that have a 
"REQUIRES" clause in it, which causes them to not be caught in many 
configurations.  We typically avoid doing -verify in CodeGen (unless the 
diagnostic is ACTUALLY in CodeGen) as a matter of business.  The diagnostic 
you're using to validate these is likely a poor choice (as it is a non-standard 
extension based warning) though, which is what makes this so difficult on folks 
trying to modernize Clang to follow standards.  IMO, if these had been C++ 
tests (which would have 'not found' failures), or validated the existence of 
these in some other mechanism, we likely would have never noticed other than 
via an audit.


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

https://reviews.llvm.org/D122983

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

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

In D122920#3426556 , @erichkeane 
wrote:

> I don't particularly get what the 'fix' is here to Richard's issue.  
> Additionally, I think this is showing me the absolute desperate need we have 
> for some sort of 'text-checking' tests for this feature, checking vs the IR 
> is too unreadable.

Thanks for review!  The issue that @rsmith reported was when the struct field 
is a record type, the __builtin_dumpr_struct no longer prints the field name. 
Eg:
The code:

  #include 
  
  struct A {
  int x;
  };
  
  struct B {
  struct A a;
  };
  
  
  int main() {
  struct B x = {0};
  __builtin_dump_struct(&x, &printf);
  }

The clang trunk:(This missing field name 'a' for type struct A), our expected 
output is 'struct A a = {')

  struct B {
  struct A {
  int x = 0
  }
  }

After this patch:

  struct B {
  struct A a = {
  int x = 0
  }
  }

I try to improve this comment. And do you think it would be better to name this 
boolean 'includeTypeName'?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
LValue RecordLV, CharUnits Align,
-   llvm::FunctionCallee Func, int Lvl) {
+   bool dumpTypeName, llvm::FunctionCallee Func,
+   int Lvl) {

erichkeane wrote:
> Not a fan of bool parameters, they make calls harder to read.
I can add comments for this parameter or this function, what do you think, or 
do you have a better idea? I would like to improve it.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2141
+  TmpRes =
+  dumpRecord(CGF, CanonicalType, FieldLV, Align, false, Func, Lvl + 1);
   Res = CGF.Builder.CreateAdd(TmpRes, Res);

erichkeane wrote:
> 
Agree!I will modify it later



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2705
 LValue RecordLV = MakeAddrLValue(RecordPtr, Arg0Type, Arg0Align);
-Value *Res = dumpRecord(*this, Arg0Type, RecordLV, Arg0Align,
+Value *Res = dumpRecord(*this, Arg0Type, RecordLV, Arg0Align, true,
 {LLVMFuncType, Func}, 0);

erichkeane wrote:
> 
Agree！


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Paul Walker via Phabricator via cfe-commits]

paulwalker-arm added a comment.

Ok, message understood.  I'll try and get these cleaned up, I cannot say 
precisely when but will push for before we branch for 15 if that sounds 
sensible.


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

https://reviews.llvm.org/D122983

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

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Erich Keane via Phabricator via cfe-commits]

erichkeane added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
LValue RecordLV, CharUnits Align,
-   llvm::FunctionCallee Func, int Lvl) {
+   bool dumpTypeName, llvm::FunctionCallee Func,
+   int Lvl) {

yihanaa wrote:
> erichkeane wrote:
> > Not a fan of bool parameters, they make calls harder to read.
> I can add comments for this parameter or this function, what do you think, or 
> do you have a better idea? I would like to improve it.
I prefer using an 'enum' for these sorts of things, we do that elsewhere.  It 
seems that what you ACTUALLY care about here is whether this is a 'top level', 
and thus has a FieldName to print, right? So you actually want to 
dumpFieldName?  Or am I still misunderstanding something?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122983#3426661 , @paulwalker-arm 
wrote:

> Thanks for this.

Any time, thank you for speaking up when I was about to drop test coverage by 
accident!

In D122983#3426716 , @erichkeane 
wrote:

> In D122983#3426661 , 
> @paulwalker-arm wrote:
>
>> Thanks for this.  I can see about cleaning up the tests but I'm still not 
>> sure what the advantage is.  The affected RUN lines are already 
>> `-fsyntax-only` tests.  Is it about where the test files live or is there 
>> something else I should be considering?  The benefit of the current tests is 
>> that it's easy to spot holes as a single function tests both requirements.  
>> My fear is that separating the tests based on Sema/CodeGen could mean we'll 
>> miss something and never know.
>
> There are two big problems with the way these are split up.  First, the 
> 'list' of these functions that you expect to not be declared under a flag 
> don't exist all in 1 place, and end up being difficult to follow/track down 
> as they are spread out as much as they are.  Second, they are in tests that 
> have a "REQUIRES" clause in it, which causes them to not be caught in many 
> configurations.  We typically avoid doing -verify in CodeGen (unless the 
> diagnostic is ACTUALLY in CodeGen) as a matter of business.  The diagnostic 
> you're using to validate these is likely a poor choice (as it is a 
> non-standard extension based warning) though, which is what makes this so 
> difficult on folks trying to modernize Clang to follow standards.  IMO, if 
> these had been C++ tests (which would have 'not found' failures), or 
> validated the existence of these in some other mechanism, we likely would 
> have never noticed other than via an audit.

+1 to this.

It's also about expectations -- the test directories are set up to be largely 
reflective of the libraries which comprise Clang and the contents of those 
directories are intended to test that particular component and not others. 
Mixing codegen and sema tests winds up being a surprise quite often when 
working on diagnostics because running the *whole* test suite is expensive 
while running just the Sema tests is quite quick. Then we can run the whole 
test suite once locally when we're finished changing diagnostics, just in case 
we missed stuff. So already, codegen tests with diagnostic checks come as a 
surprise from that workflow. But as Erich points out, these particular tests 
don't run everywhere to begin with, so it came as a rather unpleasant surprise 
that there's ~200 more tests impacted by changing the diagnostic. It would have 
been far less painful if these were in a single file in Sema -- we'd only have 
to fix it once instead of ~200 times and we'd catch the diagnostic regardless 
of what targets are registered.


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

https://reviews.llvm.org/D122983

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

[PATCH] D120662: [clang-offload-bundler] add -input/-output options

[Siu Chi Chan via Phabricator via cfe-commits]

scchan updated this revision to Diff 420218.
scchan added a comment.

- fixed the hip-link-bundle-archive.hip test issue on Windows
- fixed a couple of formatting issues


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D120662

Files:
  clang/lib/Driver/ToolChains/Clang.cpp
  clang/lib/Driver/ToolChains/CommonArgs.cpp
  clang/lib/Driver/ToolChains/HIPUtility.cpp
  clang/test/Driver/clang-offload-bundler-asserts-on.c
  clang/test/Driver/clang-offload-bundler.c
  clang/test/Driver/fat-archive-unbundle-ext.c
  clang/test/Driver/fat_archive_amdgpu.cpp
  clang/test/Driver/fat_archive_nvptx.cpp
  clang/test/Driver/hip-device-compile.hip
  clang/test/Driver/hip-link-bundle-archive.hip
  clang/test/Driver/hip-link-save-temps.hip
  clang/test/Driver/hip-output-file-name.hip
  clang/test/Driver/hip-rdc-device-only.hip
  clang/test/Driver/hip-save-temps.hip
  clang/test/Driver/hip-toolchain-device-only.hip
  clang/test/Driver/hip-toolchain-no-rdc.hip
  clang/test/Driver/hip-toolchain-rdc-separate.hip
  clang/test/Driver/hip-toolchain-rdc-static-lib.hip
  clang/test/Driver/hip-toolchain-rdc.hip
  clang/test/Driver/hip-unbundle-preproc.hip
  clang/test/Driver/hipspv-toolchain-rdc.hip
  clang/test/Driver/hipspv-toolchain.hip
  clang/test/Driver/openmp-offload-gpu.c
  clang/test/Driver/openmp-offload.c
  clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp

Index: clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
===
--- clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
+++ clang/tools/clang-offload-bundler/ClangOffloadBundler.cpp
@@ -62,15 +62,26 @@
 // and -help) will be hidden.
 static cl::OptionCategory
 ClangOffloadBundlerCategory("clang-offload-bundler options");
-
 static cl::list
-InputFileNames("inputs", cl::CommaSeparated, cl::OneOrMore,
-   cl::desc("[,...]"),
+InputFileNames("input", cl::ZeroOrMore,
+   cl::desc("Input file."
+" Can be specified multiple times "
+"for multiple input files."),
cl::cat(ClangOffloadBundlerCategory));
 static cl::list
-OutputFileNames("outputs", cl::CommaSeparated,
-cl::desc("[,...]"),
+InputFileNamesDeprecatedOpt("inputs", cl::CommaSeparated, cl::ZeroOrMore,
+cl::desc("[,...] (deprecated)"),
+cl::cat(ClangOffloadBundlerCategory));
+static cl::list
+OutputFileNames("output", cl::ZeroOrMore,
+cl::desc("Output file."
+ " Can be specified multiple times "
+ "for multiple output files."),
 cl::cat(ClangOffloadBundlerCategory));
+static cl::list
+OutputFileNamesDeprecatedOpt("outputs", cl::CommaSeparated, cl::ZeroOrMore,
+ cl::desc("[,...] (deprecated)"),
+ cl::cat(ClangOffloadBundlerCategory));
 static cl::list
 TargetNames("targets", cl::CommaSeparated,
 cl::desc("[-,...]"),
@@ -1384,6 +1395,38 @@
 }
   };
 
+  auto warningOS = [argv]() -> raw_ostream & {
+return WithColor::warning(errs(), StringRef(argv[0]));
+  };
+
+  if (InputFileNames.getNumOccurrences() != 0 &&
+  InputFileNamesDeprecatedOpt.getNumOccurrences() != 0) {
+reportError(createStringError(
+errc::invalid_argument,
+"-inputs and -input cannot be used together, use only -input instead"));
+  }
+  if (InputFileNamesDeprecatedOpt.size()) {
+warningOS() << "-inputs is deprecated, use -input instead\n";
+// temporary hack to support -inputs
+std::vector &s = InputFileNames;
+s.insert(s.end(), InputFileNamesDeprecatedOpt.begin(),
+ InputFileNamesDeprecatedOpt.end());
+  }
+
+  if (OutputFileNames.getNumOccurrences() != 0 &&
+  OutputFileNamesDeprecatedOpt.getNumOccurrences() != 0) {
+reportError(createStringError(errc::invalid_argument,
+  "-outputs and -output cannot be used "
+  "together, use only -output instead"));
+  }
+  if (OutputFileNamesDeprecatedOpt.size()) {
+warningOS() << "-outputs is deprecated, use -output instead\n";
+// temporary hack to support -outputs
+std::vector &s = OutputFileNames;
+s.insert(s.end(), OutputFileNamesDeprecatedOpt.begin(),
+ OutputFileNamesDeprecatedOpt.end());
+  }
+
   if (ListBundleIDs) {
 if (Unbundle) {
   reportError(
@@ -1408,9 +1451,8 @@
   }
 
   if (OutputFileNames.getNumOccurrences() == 0) {
-reportError(createStringError(
-errc::invalid_argument,
-"for the --outputs option: must be specified at least once!"));
+reportError(
+createStringError(errc::invalid_argument, "no output file specified!"));
   }
   if (TargetNam

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122983#3426739 , @paulwalker-arm 
wrote:

> Ok, message understood.  I'll try and get these cleaned up, I cannot say 
> precisely when but will push for before we branch for 15 if that sounds 
> sensible.

Oops, sorry, I didn't see this came in while I was also responding. :-) That 
would be fantastic and sounds like a very reasonable timeframe. I don't expect 
this to be a Real Issue until we go to change to C23 by default, which is still 
likely to be a few years away. Thank you for looking into it, and thanks again 
for speaking up about the potential loss of test coverage!


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

https://reviews.llvm.org/D122983

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

[PATCH] D123049: Emit OpenCL metadata when targeting SPIR-V

[Shangwu Yao via Phabricator via cfe-commits]

shangwuyao created this revision.
shangwuyao added reviewers: jlebar, yaxunl, tra.
Herald added subscribers: ldrumm, ThomasRaoux, Anastasia.
Herald added a project: All.
shangwuyao requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This is required for converting function calls such as get_global_id()
into SPIR-V builtins.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123049

Files:
  clang/lib/CodeGen/CodeGenModule.cpp
  clang/lib/Frontend/CompilerInvocation.cpp
  clang/test/CodeGenCUDASPIRV/kernel-cc.cu


Index: clang/test/CodeGenCUDASPIRV/kernel-cc.cu
===
--- clang/test/CodeGenCUDASPIRV/kernel-cc.cu
+++ clang/test/CodeGenCUDASPIRV/kernel-cc.cu
@@ -7,3 +7,6 @@
 // CHECK: define spir_kernel void @_Z6kernelv()
 
 __attribute__((global)) void kernel() { return; }
+
+// CHECK: !opencl.ocl.version = !{[[OCL:![0-9]+]]}
+// CHECK: [[OCL]] = !{i32 2, i32 0}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3312,6 +3312,10 @@
 // whereas respecting contract flag in backend.
 Opts.setDefaultFPContractMode(LangOptions::FPM_FastHonorPragmas);
   } else if (Opts.CUDA) {
+if (T.isSPIRV()) {
+  // Emit OpenCL version metadata in LLVM IR when targeting SPIR-V.
+  Opts.OpenCLVersion = 200;
+}
 // Allow fuse across statements disregarding pragmas.
 Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
   }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -784,7 +784,7 @@
   LangOpts.OpenMP);
 
   // Emit OpenCL specific module metadata: OpenCL/SPIR version.
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL || (LangOpts.CUDAIsDevice && getTriple().isSPIRV())) {
 EmitOpenCLMetadata();
 // Emit SPIR version.
 if (getTriple().isSPIR()) {


Index: clang/test/CodeGenCUDASPIRV/kernel-cc.cu
===
--- clang/test/CodeGenCUDASPIRV/kernel-cc.cu
+++ clang/test/CodeGenCUDASPIRV/kernel-cc.cu
@@ -7,3 +7,6 @@
 // CHECK: define spir_kernel void @_Z6kernelv()
 
 __attribute__((global)) void kernel() { return; }
+
+// CHECK: !opencl.ocl.version = !{[[OCL:![0-9]+]]}
+// CHECK: [[OCL]] = !{i32 2, i32 0}
Index: clang/lib/Frontend/CompilerInvocation.cpp
===
--- clang/lib/Frontend/CompilerInvocation.cpp
+++ clang/lib/Frontend/CompilerInvocation.cpp
@@ -3312,6 +3312,10 @@
 // whereas respecting contract flag in backend.
 Opts.setDefaultFPContractMode(LangOptions::FPM_FastHonorPragmas);
   } else if (Opts.CUDA) {
+if (T.isSPIRV()) {
+  // Emit OpenCL version metadata in LLVM IR when targeting SPIR-V.
+  Opts.OpenCLVersion = 200;
+}
 // Allow fuse across statements disregarding pragmas.
 Opts.setDefaultFPContractMode(LangOptions::FPM_Fast);
   }
Index: clang/lib/CodeGen/CodeGenModule.cpp
===
--- clang/lib/CodeGen/CodeGenModule.cpp
+++ clang/lib/CodeGen/CodeGenModule.cpp
@@ -784,7 +784,7 @@
   LangOpts.OpenMP);
 
   // Emit OpenCL specific module metadata: OpenCL/SPIR version.
-  if (LangOpts.OpenCL) {
+  if (LangOpts.OpenCL || (LangOpts.CUDAIsDevice && getTriple().isSPIRV())) {
 EmitOpenCLMetadata();
 // Emit SPIR version.
 if (getTriple().isSPIR()) {
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
LValue RecordLV, CharUnits Align,
-   llvm::FunctionCallee Func, int Lvl) {
+   bool dumpTypeName, llvm::FunctionCallee Func,
+   int Lvl) {

erichkeane wrote:
> yihanaa wrote:
> > erichkeane wrote:
> > > Not a fan of bool parameters, they make calls harder to read.
> > I can add comments for this parameter or this function, what do you think, 
> > or do you have a better idea? I would like to improve it.
> I prefer using an 'enum' for these sorts of things, we do that elsewhere.  It 
> seems that what you ACTUALLY care about here is whether this is a 'top 
> level', and thus has a FieldName to print, right? So you actually want to 
> dumpFieldName?  Or am I still misunderstanding something?
> I prefer using an 'enum' for these sorts of things, we do that elsewhere.  It 
> seems that what you ACTUALLY care about here is whether this is a 'top 
> level', and thus has a FieldName to print, right? So you actually want to 
> dumpFieldName?  Or am I still misunderstanding something?

Yeah, you are right. A little  different is that this patch will DO NOT dump 
the typename, field name and '=' in recursive dumpRecord call. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added a comment.

I'll just add that in the recursive call, for record type, it will just print 
what's between {...}
Eg:

  int printf(const char *, ...);
  
  struct A {
  int x;
  };
  
  struct B {
  struct A a;
  struct C {
int b;
union X {
  int i;
  int j;
} z;
  } bar;
  };
  
  
  int main() {
  struct B x = {0};
  __builtin_dump_struct(&x, &printf);
  }

output:

  struct B {
  struct A a = {
  int x = 0
  }
  struct C bar = {
  int b = 0
  union X z = {
  int i = 0
  int j = 0
  }
  }
  }


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D122008: [flang][driver] Add support for generating executables

[Andrzej Warzynski via Phabricator via cfe-commits]

awarzynski added a comment.

We discussed this patch in our community call today and some people expressed 
their reservations about merging this just now. As I mentioned, I would like to 
have this merged to unblock the CMake PR 
. As promised, 
here are the options that we've considered:

1. merge this patch "today",
2. merge this patch "today", but add a flag so that by default `flang-new 
file.f90` wouldn't generate an executable (we would remove this flag at a later 
time),
3. wait until the upstreaming of fir-dev is complete (based on today's update, 
I think that that would be in June/July the earliest).

Could you tell me what your preference is?

Please keep in mind that even once CMake support is available, it's going to 
land in the "latest" version of CMake. Folks will have to download the latest 
binaries from https://github.com/Kitware/CMake/releases in order to benefit 
from this.  With time, CMake shipped with various OSes will catch-up and become 
"modern" enough. But there's going to be a delay and hence it makes sense to 
get the CMake support sooner rather than later.

Please comment if I missed or misinterpreted anything. Also, I will add more 
reviewers - basically everyone who discussed this in the call today. Please add 
anyone that I've missed!

Thanks for taking a look!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122008

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

[PATCH] D123049: Emit OpenCL metadata when targeting SPIR-V

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

Is this because your HIP threadIdx etc are implemented using OpenCL builtins so 
that the emitted LLVM IR contains calls of OpenCL builtins?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123049

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

[PATCH] D123049: Emit OpenCL metadata when targeting SPIR-V

[Shangwu Yao via Phabricator via cfe-commits]

shangwuyao added a comment.

In D123049#3426849 , @yaxunl wrote:

> Is this because your HIP threadIdx etc are implemented using OpenCL builtins 
> so that the emitted LLVM IR contains calls of OpenCL builtins?

Yes, that's correct.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123049

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

[PATCH] D123049: Emit OpenCL metadata when targeting SPIR-V

[Yaxun Liu via Phabricator via cfe-commits]

yaxunl added a comment.

LGTM since currently there is only one HIP/SPIRV implementation. If in the 
future there is another HIP/SPIRV implementation that does not need this, it 
could disable it by triple.


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D123049

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

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

I pushed 5d90004874c7b040cd43597826aabb34757d25ab 
 to 
hopefully address the lion's share of the currently failing precommit CI tests. 
I'll address the remaining few as an update to this patch.


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

https://reviews.llvm.org/D122983

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

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Wang Yihan via Phabricator via cfe-commits]

yihanaa added inline comments.



Comment at: clang/lib/CodeGen/CGBuiltin.cpp:2048
LValue RecordLV, CharUnits Align,
-   llvm::FunctionCallee Func, int Lvl) {
+   bool dumpTypeName, llvm::FunctionCallee Func,
+   int Lvl) {

yihanaa wrote:
> erichkeane wrote:
> > yihanaa wrote:
> > > erichkeane wrote:
> > > > Not a fan of bool parameters, they make calls harder to read.
> > > I can add comments for this parameter or this function, what do you 
> > > think, or do you have a better idea? I would like to improve it.
> > I prefer using an 'enum' for these sorts of things, we do that elsewhere.  
> > It seems that what you ACTUALLY care about here is whether this is a 'top 
> > level', and thus has a FieldName to print, right? So you actually want to 
> > dumpFieldName?  Or am I still misunderstanding something?
> > I prefer using an 'enum' for these sorts of things, we do that elsewhere.  
> > It seems that what you ACTUALLY care about here is whether this is a 'top 
> > level', and thus has a FieldName to print, right? So you actually want to 
> > dumpFieldName?  Or am I still misunderstanding something?
> 
> Yeah, you are right. A little  different is that this patch will DO NOT dump 
> the typename, field name and '=' in recursive dumpRecord call. 
Yeah, you are right. 


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

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

[PATCH] D122798: [clang][extract-api][NFC] Add documentation

[Zixu Wang via Phabricator via cfe-commits]

zixuw added a comment.

Has this landed yet?


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122798

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

[PATCH] D123056: [clang][extract-api] Undefining macros should not result in a crash

[Daniel Grumberg via Phabricator via cfe-commits]

dang created this revision.
dang added reviewers: zixuw, ributzka, QuietMisdreavus.
Herald added a project: All.
dang requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This fixes the situation where a undefining a not previously defined
macro resulted in a crash. Before trying to remove a definition from
PendingMacros we first check to see if the macro did indeed have a
previous definition.


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123056

Files:
  clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
  clang/test/ExtractAPI/macro_undefined.c


Index: clang/test/ExtractAPI/macro_undefined.c
===
--- clang/test/ExtractAPI/macro_undefined.c
+++ clang/test/ExtractAPI/macro_undefined.c
@@ -19,6 +19,8 @@
 FUNC_GEN(foo)
 FUNC_GEN(bar, const int *, unsigned);
 #undef FUNC_GEN
+// Undefining a not previously defined macro should not result in a crash.
+#undef FOO
 
 //--- reference.output.json.in
 {
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -534,6 +534,11 @@
   // macro definition for it.
   void MacroUndefined(const Token &MacroNameToken, const MacroDefinition &MD,
   const MacroDirective *Undef) override {
+// If this macro wasn't previously defined we don't need to do anything
+// here.
+if (!Undef)
+  return;
+
 llvm::erase_if(PendingMacros, [&MD](const PendingMacro &PM) {
   return MD.getMacroInfo()->getDefinitionLoc() ==
  PM.MD->getMacroInfo()->getDefinitionLoc();


Index: clang/test/ExtractAPI/macro_undefined.c
===
--- clang/test/ExtractAPI/macro_undefined.c
+++ clang/test/ExtractAPI/macro_undefined.c
@@ -19,6 +19,8 @@
 FUNC_GEN(foo)
 FUNC_GEN(bar, const int *, unsigned);
 #undef FUNC_GEN
+// Undefining a not previously defined macro should not result in a crash.
+#undef FOO
 
 //--- reference.output.json.in
 {
Index: clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
===
--- clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
+++ clang/lib/ExtractAPI/ExtractAPIConsumer.cpp
@@ -534,6 +534,11 @@
   // macro definition for it.
   void MacroUndefined(const Token &MacroNameToken, const MacroDefinition &MD,
   const MacroDirective *Undef) override {
+// If this macro wasn't previously defined we don't need to do anything
+// here.
+if (!Undef)
+  return;
+
 llvm::erase_if(PendingMacros, [&MD](const PendingMacro &PM) {
   return MD.getMacroInfo()->getDefinitionLoc() ==
  PM.MD->getMacroInfo()->getDefinitionLoc();
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122983: [C11/C2x] Change the behavior of the implicit function declaration warning

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman updated this revision to Diff 420242.
aaron.ballman added a comment.
Herald added subscribers: kbarton, nemanjai.

Updated the release note and hopefully addressed the remaining test failures 
found by precommit CI.


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

https://reviews.llvm.org/D122983

Files:
  clang-tools-extra/clangd/IncludeFixer.cpp
  clang-tools-extra/clangd/ParsedAST.cpp
  
clang-tools-extra/test/clang-tidy/checkers/bugprone-easily-swappable-parameters-implicits.c
  clang/docs/ReleaseNotes.rst
  clang/include/clang/Basic/DiagnosticSemaKinds.td
  clang/lib/Sema/SemaDecl.cpp
  clang/lib/Sema/SemaExpr.cpp
  clang/test/ARCMT/objcmt-arc-cf-annotations.m
  clang/test/ARCMT/objcmt-arc-cf-annotations.m.result
  clang/test/Analysis/OSAtomic_mac.c
  clang/test/Analysis/ObjCProperties.m
  clang/test/Analysis/PR49642.c
  clang/test/Analysis/diagnostics/no-store-func-path-notes.c
  clang/test/Analysis/misc-ps-region-store.m
  clang/test/Analysis/novoidtypecrash.c
  clang/test/Analysis/plist-macros-with-expansion.c
  clang/test/CodeGen/2002-07-14-MiscTests3.c
  clang/test/CodeGen/2002-07-31-SubregFailure.c
  clang/test/CodeGen/2003-08-18-SigSetJmp.c
  clang/test/CodeGen/2004-11-27-StaticFunctionRedeclare.c
  clang/test/CodeGen/2005-01-02-ConstantInits.c
  clang/test/CodeGen/2005-01-02-VAArgError-ICE.c
  clang/test/CodeGen/2006-01-13-StackSave.c
  clang/test/CodeGen/2006-03-03-MissingInitializer.c
  clang/test/CodeGen/2008-05-12-TempUsedBeforeDef.c
  clang/test/CodeGen/2008-07-30-redef-of-bitcasted-decl.c
  clang/test/CodeGen/2008-08-19-cast-of-typedef.c
  clang/test/CodeGen/2008-10-13-FrontendCrash.c
  clang/test/CodeGen/PowerPC/builtins-ppc-p8vector.c
  clang/test/CodeGen/X86/bmi2-builtins.c
  clang/test/CodeGen/aarch64-mops.c
  clang/test/CodeGen/aarch64-neon-sm4-sm3.c
  clang/test/CodeGen/arm_acle.c
  clang/test/CodeGen/attribute_constructor.c
  clang/test/CodeGen/builtins-arm-microsoft.c
  clang/test/CodeGen/builtins-arm-msvc-compat-only.c
  clang/test/CodeGen/cast-emit.c
  clang/test/CodeGen/debug-info-block-vars.c
  clang/test/CodeGen/debug-info-crash.c
  clang/test/CodeGen/decl.c
  clang/test/CodeGen/init-with-member-expr.c
  clang/test/CodeGen/misaligned-param.c
  clang/test/CodeGen/neon-crypto.c
  clang/test/CodeGen/struct-comma.c
  clang/test/CodeGen/variable-array.c
  clang/test/Frontend/warning-mapping-2.c
  clang/test/Headers/arm-cmse-header-ns.c
  clang/test/Import/objc-arc/test-cleanup-object.m
  clang/test/Modules/config_macros.m
  clang/test/Modules/modulemap-locations.m
  clang/test/OpenMP/declare_mapper_messages.c
  clang/test/PCH/chain-macro-override.c
  clang/test/Rewriter/rewrite-foreach-2.m
  clang/test/Rewriter/rewrite-try-catch.m
  clang/test/Sema/__try.c
  clang/test/Sema/aarch64-tme-errors.c
  clang/test/Sema/arm-no-fp16.c
  clang/test/Sema/bitfield.c
  clang/test/Sema/builtin-setjmp.c
  clang/test/Sema/builtins.c
  clang/test/Sema/cxx-as-c.c
  clang/test/Sema/implicit-builtin-decl.c
  clang/test/Sema/implicit-decl.c
  clang/test/Sema/implicit-ms-builtin-decl.c
  clang/test/Sema/typo-correction.c
  clang/test/Sema/vla.c
  clang/test/Sema/warn-strict-prototypes.c
  clang/test/VFS/module_missing_vfs.m

Index: clang/test/VFS/module_missing_vfs.m
===
--- clang/test/VFS/module_missing_vfs.m
+++ clang/test/VFS/module_missing_vfs.m
@@ -1,12 +1,12 @@
 // RUN: rm -rf %t && mkdir -p %t
 // RUN: echo "void funcA(void);" >> %t/a.h
 
-// RUN: not %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
+// RUN: not %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml 2>&1 | FileCheck %s -check-prefix=ERROR
 // ERROR: virtual filesystem overlay file '{{.*}}' not found
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 // RUN: sed -e "s@INPUT_DIR@%{/S:regex_replacement}/Inputs@g" -e "s@OUT_DIR@%{/t:regex_replacement}@g" %S/Inputs/MissingVFS/vfsoverlay.yaml > %t/vfs.yaml
-// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
+// RUN: %clang_cc1 -std=c99 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t/mcp -I %S/Inputs/MissingVFS %s -fsyntax-only -ivfsoverlay %t/vfs.yaml
 // RUN: find %t/mcp -name "A-*.pcm" | count 1
 
 @import A;
Index: clang/test/Sema/warn-strict-prototypes.c
===
--- clang/test/Sema/warn-strict-prototypes.c
+++ clang/test/Sema/warn-strict-prototypes.c
@@ -1,5 +1,5 @@
 // RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -Wno-implicit-function-declaration -verify %s
-// RUN: %clang_cc1 -triple i386-pc-unknown -fsyntax-only -Wstrict-prototypes -fdiagnostics-parseable-fixits %s 2>&1 | Fi

[PATCH] D122798: [clang][extract-api][NFC] Add documentation

[Daniel Grumberg via Phabricator via cfe-commits]

dang added a comment.

In D122798#3427004 , @zixuw wrote:

> Has this landed yet?

Doing it now!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122798

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

[clang] 422d05e - [clang][extract-api][NFC] Add documentation

[Daniel Grumberg via cfe-commits]

Author: Daniel Grumberg
Date: 2022-04-04T18:59:44+01:00
New Revision: 422d05e792dbd6a97f5afd4cdd5e8aa677e97444

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

LOG: [clang][extract-api][NFC] Add documentation

Add struct level documentation for MacroDefinitionRecord.

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

Added: 


Modified: 
clang/include/clang/ExtractAPI/API.h

Removed: 




diff  --git a/clang/include/clang/ExtractAPI/API.h 
b/clang/include/clang/ExtractAPI/API.h
index 142dd7a45de47..62e30b1d4c57b 100644
--- a/clang/include/clang/ExtractAPI/API.h
+++ b/clang/include/clang/ExtractAPI/API.h
@@ -380,6 +380,7 @@ struct ObjCProtocolRecord : ObjCContainerRecord {
   virtual void anchor();
 };
 
+/// This holds information associated with macro definitions.
 struct MacroDefinitionRecord : APIRecord {
   MacroDefinitionRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
 DeclarationFragments Declaration,



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

[PATCH] D122798: [clang][extract-api][NFC] Add documentation

[Daniel Grumberg via Phabricator via cfe-commits]

This revision was landed with ongoing or failed builds.
This revision was automatically updated to reflect the committed changes.
Closed by commit rG422d05e792db: [clang][extract-api][NFC] Add documentation 
(authored by dang).

Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122798

Files:
  clang/include/clang/ExtractAPI/API.h


Index: clang/include/clang/ExtractAPI/API.h
===
--- clang/include/clang/ExtractAPI/API.h
+++ clang/include/clang/ExtractAPI/API.h
@@ -380,6 +380,7 @@
   virtual void anchor();
 };
 
+/// This holds information associated with macro definitions.
 struct MacroDefinitionRecord : APIRecord {
   MacroDefinitionRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
 DeclarationFragments Declaration,


Index: clang/include/clang/ExtractAPI/API.h
===
--- clang/include/clang/ExtractAPI/API.h
+++ clang/include/clang/ExtractAPI/API.h
@@ -380,6 +380,7 @@
   virtual void anchor();
 };
 
+/// This holds information associated with macro definitions.
 struct MacroDefinitionRecord : APIRecord {
   MacroDefinitionRecord(StringRef Name, StringRef USR, PresumedLoc Loc,
 DeclarationFragments Declaration,
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[PATCH] D122920: [Clang][CodeGen]Fix __builtin_dump_struct missing record type field name

[Wang Yihan via Phabricator via cfe-commits]

yihanaa updated this revision to Diff 420254.
yihanaa added a comment.

Improve comment


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122920

Files:
  clang/lib/CodeGen/CGBuiltin.cpp
  clang/test/CodeGen/dump-struct-builtin.c

Index: clang/test/CodeGen/dump-struct-builtin.c
===
--- clang/test/CodeGen/dump-struct-builtin.c
+++ clang/test/CodeGen/dump-struct-builtin.c
@@ -5,95 +5,122 @@
 
 // CHECK: @__const.unit1.a = private unnamed_addr constant %struct.U1A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U1:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U1A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"short a = %hd\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U1:@[0-9]+]] = private unnamed_addr constant [15 x i8] c"short a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U1:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hd\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U1:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit2.a = private unnamed_addr constant %struct.U2A { i16 12 }, align 2
 // CHECK-NEXT: [[STRUCT_STR_U2:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U2A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [28 x i8] c"unsigned short a = %hu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U2:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"unsigned short a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U2:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%hu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U2:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit3.a = private unnamed_addr constant %struct.U3A { i32 12 }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U3:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U3A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [16 x i8] c"int a = %d\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U3:@[0-9]+]] = private unnamed_addr constant [13 x i8] c"int a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U3:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%d\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U3:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit4.a = private unnamed_addr constant %struct.U4A { i32 12 }, align 4
 // CHECK-NEXT: [[STRUCT_STR_U4:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U4A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [25 x i8] c"unsigned int a = %u\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U4:@[0-9]+]] = private unnamed_addr constant [22 x i8] c"unsigned int a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U4:@[0-9]+]] = private unnamed_addr constant [4 x i8] c"%u\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U4:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit5.a = private unnamed_addr constant %struct.U5A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U5A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [18 x i8] c"long a = %ld\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U5:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"long a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U5:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%ld\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U5:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit6.a = private unnamed_addr constant %struct.U6A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U6:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U6A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [27 x i8] c"unsigned long a = %lu\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U6:@[0-9]+]] = private unnamed_addr constant [23 x i8] c"unsigned long a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U6:@[0-9]+]] = private unnamed_addr constant [5 x i8] c"%lu\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U6:@[0-9]+]] = private unnamed_addr constant [3 x i8] c"}\0A\00", align 1
 
 // CHECK: @__const.unit7.a = private unnamed_addr constant %struct.U7A { i64 12 }, align 8
 // CHECK-NEXT: [[STRUCT_STR_U7:@[0-9]+]] = private unnamed_addr constant [14 x i8] c"struct U7A {\0A\00", align 1
-// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [24 x i8] c"long long a = %lld\0A\00", align 1
+// CHECK-NEXT: [[FIELD_U7:@[0-9]+]] = private unnamed_addr constant [19 x i8] c"long long a = \00", align 1
+// CHECK-NEXT: [[FORMAT_U7:@[0-9]+]] = private unnamed_addr constant [6 x i8] c"%lld\0A\00", align 1
 // CHECK-NEXT: [[END_STRUCT_U7:@[0-9]+]] = private unn

[PATCH] D119609: [Clang][Sema] Prohibit expr statement in the default argument

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D119609#3426279 , @erichkeane 
wrote:

> I think LGTM on technical perspective, Please don't commit until 
> @aaron.ballman confirms he is OK with the direction, or would like to wait 
> longer for GCC.

I spotted some technical issues with the diagnostic wording. As for waiting for 
GCC, my concern there is largely with the fact that people use statement 
expressions in macros so that they can define local variables that don't impact 
the outer scope where the macro is expanded. I could see such a macro being 
used as a default argument, so I worry a bit about breaking code in that case. 
However, C doesn't have default arguments, so this isn't an issue there. And 
C++ has lambdas which can be called in a default argument, so users have some 
ability to fix their code in the event of an error so long as they're in C++11 
or later. Because of that, I would be okay moving forward with this, but 
cautiously in case someone reports code that breaks and can't easily be fixed 
(or significant breakage in an important 3rd party header).




Comment at: clang/include/clang/Basic/DiagnosticSemaKinds.td:4383-4384
   "%select{default|copy|move}0 constructor">;
+def err_expr_statement_in_default_arg : Error<
+  "expression statement not permitted in default argument">;
 

I get mixed up every time, but I double-checked -- these are statement 
expressions, not expression statements. I also think the diagnostic should be 
more clear about default argument vs default non-type template argument.



Comment at: clang/lib/Parse/ParseDecl.cpp:7073
+  Actions.ActOnParamDefaultArgumentError(Param, EqualLoc);
+  // Skip the expression statement and continue parsing
+  SkipUntil(tok::comma, StopBeforeMatch);





Comment at: clang/test/Sema/err-expr-stmt-in-default-arg.cpp:1
+// RUN: %clang_cc1 %s -fsyntax-only -verify -std=c++20
+

You should rename this test to `stmt-expr-in-default-arg.cpp`


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D119609

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

[PATCH] D122155: Add warning when eval-method is set in the presence of value unsafe floating-point calculations.

[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, thanks!


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

https://reviews.llvm.org/D122155

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

[PATCH] D122808: [clang] Fix warnings when `-Wdeprecated-enum-enum-conversion` is enabled

[Aaron Ballman via Phabricator via cfe-commits]

aaron.ballman added a comment.

In D122808#3425703 , 
@antoniofrighetto wrote:

> Looks definitely better! How about this slightly changed version protecting 
> the interface?
>
>   /// Helper which adds two underlying types of enumeration type.
>   template  typename EnumTy2,
> typename UT1 = std::enable_if_t, 
> std::underlying_type_t>,
> typename UT2 = std::enable_if_t, 
> std::underlying_type_t>>
>   constexpr auto addEnumValues(EnumTy1 LHS, EnumTy2 RHS) {
> return static_cast(LHS) + static_cast(RHS);
>   }
>
> I feel like this is something we may wish to readopt in the future for other 
> similar cases as well  (e.g., `-Wdeprecated-anon-enum-enum-conversion` in 
> `Comment.h`).

I think that's a great interface for it!


Repository:
  rG LLVM Github Monorepo

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

https://reviews.llvm.org/D122808

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

[clang-tools-extra] 6f3f1e9 - [clangd] Remove trivial uses of FileEntry::getName

[Sam McCall via cfe-commits]

Author: Sam McCall
Date: 2022-04-04T20:59:51+02:00
New Revision: 6f3f1e98686575004bef4257cbc6c825de5382af

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

LOG: [clangd] Remove trivial uses of FileEntry::getName

It's deprecated; migrate to FileEntryRef::getName where it doesn't matter.
Also change one subtle case of implicit FileEntry::getName to be explicit.

After this patch, all the remaining FileEntry::getName calls are subtle
cases where we may be relying on exactly which filename variant is returned
(for indexing, IWYU directive handling, etc).

Added: 


Modified: 
clang-tools-extra/clangd/Diagnostics.cpp
clang-tools-extra/clangd/IncludeCleaner.cpp
clang-tools-extra/clangd/Preamble.cpp
clang-tools-extra/clangd/index/CanonicalIncludes.cpp
clang-tools-extra/clangd/index/FileIndex.cpp
clang-tools-extra/clangd/index/SymbolCollector.cpp
clang-tools-extra/clangd/refactor/Tweak.cpp
clang-tools-extra/clangd/refactor/tweaks/AddUsing.cpp
clang-tools-extra/clangd/unittests/IndexActionTests.cpp

Removed: 




diff  --git a/clang-tools-extra/clangd/Diagnostics.cpp 
b/clang-tools-extra/clangd/Diagnostics.cpp
index 762906615f323..46a16a9f2eeb3 100644
--- a/clang-tools-extra/clangd/Diagnostics.cpp
+++ b/clang-tools-extra/clangd/Diagnostics.cpp
@@ -196,16 +196,16 @@ bool tryMoveToMainFile(Diag &D, FullSourceLoc DiagLoc) {
 return false;
 
   // Add a note that will point to real diagnostic.
-  const auto *FE = SM.getFileEntryForID(SM.getFileID(DiagLoc));
+  auto FE = SM.getFileEntryRefForID(SM.getFileID(DiagLoc)).getValue();
   D.Notes.emplace(D.Notes.begin());
   Note &N = D.Notes.front();
-  N.AbsFile = std::string(FE->tryGetRealPathName());
-  N.File = std::string(FE->getName());
+  N.AbsFile = std::string(FE.getFileEntry().tryGetRealPathName());
+  N.File = std::string(FE.getName());
   N.Message = "error occurred here";
   N.Range = D.Range;
 
   // Update diag to point at include inside main file.
-  D.File = SM.getFileEntryForID(SM.getMainFileID())->getName().str();
+  D.File = SM.getFileEntryRefForID(SM.getMainFileID())->getName().str();
   D.Range = std::move(R);
   D.InsideMainFile = true;
   // Update message to mention original file.

diff  --git a/clang-tools-extra/clangd/IncludeCleaner.cpp 
b/clang-tools-extra/clangd/IncludeCleaner.cpp
index 04dbf12410cf7..44acd34086af9 100644
--- a/clang-tools-extra/clangd/IncludeCleaner.cpp
+++ b/clang-tools-extra/clangd/IncludeCleaner.cpp
@@ -246,14 +246,14 @@ static bool mayConsiderUnused(const Inclusion &Inc, 
ParsedAST &AST) {
   // Headers without include guards have side effects and are not
   // self-contained, skip them.
   assert(Inc.HeaderID);
-  auto FE = AST.getSourceManager().getFileManager().getFile(
+  auto FE = AST.getSourceManager().getFileManager().getFileRef(
   AST.getIncludeStructure().getRealPath(
   static_cast(*Inc.HeaderID)));
   assert(FE);
   if 
(!AST.getPreprocessor().getHeaderSearchInfo().isFileMultipleIncludeGuarded(
-  *FE)) {
+  &FE->getFileEntry())) {
 dlog("{0} doesn't have header guard and will not be considered unused",
- (*FE)->getName());
+ FE->getName());
 return false;
   }
   return true;
@@ -418,7 +418,7 @@ std::vector issueUnusedIncludesDiagnostics(ParsedAST 
&AST,
   std::vector Result;
   std::string FileName =
   AST.getSourceManager()
-  .getFileEntryForID(AST.getSourceManager().getMainFileID())
+  .getFileEntryRefForID(AST.getSourceManager().getMainFileID())
   ->getName()
   .str();
   for (const auto *Inc : computeUnusedIncludes(AST)) {

diff  --git a/clang-tools-extra/clangd/Preamble.cpp 
b/clang-tools-extra/clangd/Preamble.cpp
index aab6dc78092f7..b3a3aec985ba0 100644
--- a/clang-tools-extra/clangd/Preamble.cpp
+++ b/clang-tools-extra/clangd/Preamble.cpp
@@ -677,7 +677,7 @@ PreamblePatch PreamblePatch::unmodified(const PreambleData 
&Preamble) {
 SourceLocation translatePreamblePatchLocation(SourceLocation Loc,
   const SourceManager &SM) {
   auto DefFile = SM.getFileID(Loc);
-  if (auto *FE = SM.getFileEntryForID(DefFile)) {
+  if (auto FE = SM.getFileEntryRefForID(DefFile)) {
 auto IncludeLoc = SM.getIncludeLoc(DefFile);
 // Preamble patch is included inside the builtin file.
 if (IncludeLoc.isValid() && SM.isWrittenInBuiltinFile(IncludeLoc) &&

diff  --git a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp 
b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
index 7067f1771b94f..ce02f97bb7d8c 100644
--- a/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
+++ b/clang-tools-extra/clangd/index/CanonicalIncludes.cpp
@@ -66,11 +66,12 @@ collectIWYUHeaderMaps(CanonicalIncludes *Includes) {
   

[PATCH] D123059: re-roll-forward "[clang] Mark `trivial_abi` types as "trivially relocatable"".""

[Devin Jeanpierre via Phabricator via cfe-commits]

devin.jeanpierre created this revision.
devin.jeanpierre added a reviewer: gribozavr2.
Herald added subscribers: dexonsmith, pengfei.
Herald added a reviewer: aaron.ballman.
Herald added a project: All.
devin.jeanpierre requested review of this revision.
Herald added a project: clang.
Herald added a subscriber: cfe-commits.

This reverts commit b0bc93da926a943cdc2d8b04f8dcbe23a774520c 
.

Changes: `s/_WIN32/_WIN64/g`.

The calling convention kind is specific to 64-bit windows.
It's even in the name: `CCK_MicrosoftWin64`.

After this, the test passes with both `-triple i686-pc-win32` and `-triple 
x86_64-pc-win32`. Phew!


Repository:
  rG LLVM Github Monorepo

https://reviews.llvm.org/D123059

Files:
  clang/docs/LanguageExtensions.rst
  clang/include/clang/AST/Type.h
  clang/include/clang/Basic/AttrDocs.td
  clang/include/clang/Basic/TokenKinds.def
  clang/lib/AST/Type.cpp
  clang/lib/Sema/SemaExprCXX.cpp
  clang/test/SemaCXX/attr-trivial-abi.cpp
  clang/test/SemaCXX/type-traits.cpp
  clang/test/SemaObjCXX/arc-type-traits.mm
  clang/test/SemaObjCXX/objc-weak-type-traits.mm

Index: clang/test/SemaObjCXX/objc-weak-type-traits.mm
===
--- clang/test/SemaObjCXX/objc-weak-type-traits.mm
+++ clang/test/SemaObjCXX/objc-weak-type-traits.mm
@@ -8,7 +8,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) static_assert(!Trait(Type), "")
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) static_assert(Trait(Type1, Type2), "")
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) static_assert(!Trait(Type1, Type2), "")
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -208,3 +208,12 @@
 TRAIT_IS_FALSE_2(__is_trivially_constructible, HasWeak, HasWeak&&);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
+
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaObjCXX/arc-type-traits.mm
===
--- clang/test/SemaObjCXX/arc-type-traits.mm
+++ clang/test/SemaObjCXX/arc-type-traits.mm
@@ -12,7 +12,7 @@
 #define TRAIT_IS_FALSE(Trait, Type) char JOIN2(Trait,__LINE__)[Trait(Type)? -1 : 1]
 #define TRAIT_IS_TRUE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? 1 : -1]
 #define TRAIT_IS_FALSE_2(Trait, Type1, Type2) char JOIN2(Trait,__LINE__)[Trait(Type1, Type2)? -1 : 1]
-  
+
 struct HasStrong { id obj; };
 struct HasWeak { __weak id obj; };
 struct HasUnsafeUnretained { __unsafe_unretained id obj; };
@@ -213,3 +213,11 @@
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained);
 TRAIT_IS_TRUE_2(__is_trivially_constructible, HasUnsafeUnretained, HasUnsafeUnretained&&);
 
+// __is_trivially_relocatable
+TRAIT_IS_TRUE(__is_trivially_relocatable, __strong id);
+TRAIT_IS_FALSE(__is_trivially_relocatable, __weak id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __autoreleasing id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, __unsafe_unretained id);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasStrong);
+TRAIT_IS_FALSE(__is_trivially_relocatable, HasWeak);
+TRAIT_IS_TRUE(__is_trivially_relocatable, HasUnsafeUnretained);
Index: clang/test/SemaCXX/type-traits.cpp
===
--- clang/test/SemaCXX/type-traits.cpp
+++ clang/test/SemaCXX/type-traits.cpp
@@ -2854,3 +2854,64 @@
 #undef T16384
 #undef T32768
 } // namespace type_trait_expr_numargs_overflow
+
+namespace is_trivially_relocatable {
+
+static_assert(!__is_trivially_relocatable(void), "");
+static_assert(__is_trivially_relocatable(int), "");
+static_assert(__is_trivially_relocatable(int[]), "");
+
+enum Enum {};
+static_assert(__is_trivially_relocatable(Enum), "");
+static_assert(__is_trivially_relocatable(Enum[]), "");
+
+union Union {int x;};
+static_assert(__is_trivially_relocatable(Union), "");
+static_assert(__is_trivially_relocatable(Union[]), "");
+
+struct Trivial {};
+static_assert(__is_trivially_relocatable(Trivial), "");
+static_assert(__is_trivially_relocatable(Trivial[]), "");
+
+struct Incomplete; // expected-note {{forward declaration of 'is_trivially_relocatable::Incomplete'}}
+bool unused = __is_trivially_relocatable(Incomplete); // expected-error {{incomplete type}}
+
+struct NontrivialDtor {
+  ~NontrivialDtor() {}
+};
+static_assert(!__is_trivially_relocatable(Non