[clang] [CIR] Call code gen; create empty cir.func op (PR #113483)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes approved this pull request.

Thanks David! 

https://github.com/llvm/llvm-project/pull/113483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Call code gen; create empty cir.func op (PR #113483)

[Bruno Cardoso Lopes via cfe-commits]

bcardosolopes wrote:

> Again though, a `FIXME: at one point we should capture macro expansions here 
> so we can diagnose expansions more thoroughly` is perhaps not a bad addition 
> here.

+1

https://github.com/llvm/llvm-project/pull/113483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Call code gen; create empty cir.func op (PR #113483)

[Bruno Cardoso Lopes via cfe-commits]

@@ -52,10 +62,33 @@ class CIRGenModule : public CIRGenTypeCache {
   /// A "module" matches a c/cpp source file: containing a list of functions.
   mlir::ModuleOp theModule;
 
+  clang::DiagnosticsEngine &diags;
+
   const clang::TargetInfo ⌖
 
 public:
+  mlir::ModuleOp getModule() const { return theModule; }
+
+  /// Helpers to convert Clang's SourceLocation to an MLIR Location.

bcardosolopes wrote:

What we get in this case is:
```
module ...  {
  cir.global  external dsolocal @y = #cir.int<22> : !s32i {alignment = 4 : i64} 
loc(#loc3)
} loc(#loc)
#loc = loc("a.cpp":0:0)
#loc1 = loc("a.cpp":3:1)
#loc2 = loc("a.cpp":3:9)
#loc3 = loc(fused[#loc1, #loc2])
```

> which location does MLIR expect to work with?

Whatever we tell it should hold. We usually take the presumed (expanded) loc 
(from `getSourceRange` or begin/end):

```
mlir::Location CIRGenFunction::getLoc(SourceLocation SLoc) {
  // Some AST nodes might contain invalid source locations (e.g.
  // CXXDefaultArgExpr), workaround that to still get something out.
  if (SLoc.isValid()) {
const SourceManager &SM = getContext().getSourceManager();
PresumedLoc PLoc = SM.getPresumedLoc(SLoc);
StringRef Filename = PLoc.getFilename();
return mlir::FileLineColLoc::get(builder.getStringAttr(Filename),
 PLoc.getLine(), PLoc.getColumn());
  } else {
// Do our best...
assert(currSrcLoc && "expected to inherit some source location");
return *currSrcLoc;
  }
}
```
My take is that this is good enough for CIRGen purposes and can/should probably 
be improved when we start working on emitting debug information (where we can 
go for non-presumed locs if it makes sense), before that it feels designing 
prematurely to me.

https://github.com/llvm/llvm-project/pull/113483
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes approved this pull request.

Seems like all issues have been addressed? Anything remaining @AaronBallman ?

Thanks everyone for all the reviews!

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

bcardosolopes wrote:

Thanks @AaronBallman 

> But again, the important thing is to at least be self-consistent. e.g.,

It's a good point to reinforce, updated.

> I just don't think it benefits anyone to do so other than to make it easier 
> to land the initial patches, which doesn't seem like a compelling reason to 
> stick with auto.

Fair. I moved the usage of `auto` for `mlir::Location` for things outside 
"CIRGen/FrontendAction" but allowed as part of `lib/CIR/{Transforms,*}`, where 
the context makes it way more easier to understand (e.g. it can only be one 
thing, no Clang's source locations expected at that point)

> I think that's something we can live with, but like above, this makes it 
> harder for the community in general while making it easier for folks already 
> involved with MLIR.

Ok, I'll take the "we can live with" for this one.

Doc updated accordingly: 
https://llvm.github.io/clangir/GettingStarted/coding-guideline.html

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

bcardosolopes wrote:

I encoded the discussion in our clangir.org page, with some speculation on 
Aaron's pending replies, happy to change and fix anything. In the future we'll 
upstream that too, but for the time being it's here: 
https://llvm.github.io/clangir/GettingStarted/coding-guideline.html

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

bcardosolopes wrote:

> What is obvious in the MLIR space is not necessarily what's obvious in Clang;

Sure.

>  I have no idea whether that returns a `SourceLocation`, a `PresumedLoc`, a 
> `FullSourceLoc`, etc, so I don't think that is a use of `auto` I would want 
> to have to reason about as a reviewer. That said, if the only use of `loc` 
> is: `Diag(loc, diag::err_something);` then the type really doesn't matter and 
> use of `auto` is probably fine. It's a judgment call.

For location, `getLoc` always return `mlir::Location`, it's not related to 
Clang's source location. CIRGen use of `loc` is mostly to pass it down to MLIR 
APIs, because MLIR source locations are required whenever one is building or 
rewriting an operation. We usually convert Clang's location into MLIR and pass 
those around as expected. Has that changed your opinion for this specific case?

> Again, I have no idea what the type of that is and I have no way to verify 
> that it actually _uses_ value semantics because the pointer and qualifiers 
> can be inferred and references can be dropped. That makes it harder to review 
> the code; I would spell out the type in this case as well.

Makes sense. For some background: MLIR "instructions", usually called 
"operations", usually have pretty default return types when querying operands, 
`mlir::Value` for SSA values or when an `mlir::Attribute` is returned, the 
getter/setter is suffixed with `Attr`. Would it be reasonable to suggest a 
guideline where in CIRGen we get more strict on not using `auto` but for other 
passes and transformations it should be fine to use auto on those? My rationale 
here is that post CIRGen it's very likely the reviwer audience is going to be 
more among MLIR experts than Clang experts and the former are more familiar 
with the idiom.

> The big things for reviewers are: don't use `auto` if the type isn't 
> incredibly obvious (spelled in the initialization or extremely obvious based 
> on immediately surrounding code) and don't make us infer important parts the 
> declarator (if it's a `const Foo *` and `auto` makes sense for some reason, 
> spell it `const auto *` rather than `auto`).

Agreed. Btw, I'm not advocating for plain `auto` here/everywhere. This is more 
about `auto` usage big picture.

> And if a reviewer asks for something to be converted from `auto` to an 
> explicit type name, assume that means the code isn't as readable as it could 
> be and switch to a concrete type (we should not be arguing "I think this is 
> clear therefore you should also think it's clear" during review, that just 
> makes the review process painful for everyone involved).

Agreed. If it makes the reviewer job easier we should just abide.

> My thinking is: 1) (most important) the convention should be consistent with 
> other nearby code, whatever that convention happens to be.

Ok, for camelBack, it seems like `clang/lib/CodeGen` is already adopting it in 
multiple places (unsure if it's intentional or not?), and looks incomplete 
because probably no one took the effort to make it uniform? The signal this 
gives me is that we should just do it (use camelBack) for CIRGen.

2) if there's not already a convention to follow and if it's code that lives in 
`clang/include/clang`, it should generally follow the LLVM style (it's an 
"external" interface) because those are the APIs that folks outside of CIR will 
be calling into. If it's code that lives in `clang/lib/`, following the MLIR 
style is less of a concern.

Great & agreed, thanks for the clarification.

> ... changing to the LLVM style is generally preferred over changing to the 
> MLIR style. One caveat to this is: avoid busywork but use your best judgement 
> on how we eventually get to a consistent use of the LLVM style. ... it's 
> probably best to just bite the bullet and switch to LLVM style even though 
> MLIR would be less effort. Does this all seem reasonable?

Yep, thank you @AaronBallman for putting the time here to help us set the right 
approach looking fwd. To be more specific, I'd suggest:

- `clang/include` -> LLVM
- `lib/Driver/*` -> LLVM
- `lib/FrontendTool` -> LLVM
- `lib/CIR/FrontendAction` -> LLVM
- `lib/CIR/CodeGen` -> MLIR
- `lib/CIR/*` -> MLIR

What do you think?

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

bcardosolopes wrote:

Thanks for everyone's input so far. Let me try to summarize two discussions in 
this PR so we can set on an approach and give advice to our CIR community (and 
encode on our webpage) on how to move forward in upcoming patches. Useful 
resources: 
- [LLVM Coding Standard](https://llvm.org/docs/CodingStandards.html)
- [MLIR Style 
Guide](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide)

CC'ing more people who might care: @seven-mile, @Lancern.

## Use of `auto`

As @joker-eph mentioned, MLIR isn't meant to differ from LLVM/Clang, though 
they encode the differences as part of their guidelines. The `auto` 
[guidance](https://llvm.org/docs/CodingStandards.html#use-auto-type-deduction-to-make-code-more-readable)
 is still in favor of us keeping it for all `isa`, `dyn_cast` and `cast`, which 
is used in CIR, so it probably doesn't affect most of what we currently care 
about. Here's my suggestion for the entire `lib/CIR`:

1. Use it for function templates such as `isa`, `dyn_cast`, `cast`, `create` 
and `rewriteOp.*` variants.
2. Use it for things considered obvious/common in MLIR space, examples:
  - `auto loc = op->getLoc()`.
  - Getting operands and results from operations (they obey Value Semantics), 
e.g.: `DepthwiseConv2DNhwcHwcmOp op; ...; auto stride = op.getStrides();`
  - Other examples we are happy to provide upon reviewer feedback if it makes 
sense.

Using the logic above, all `auto`s in this current PR have to be changed (since 
none apply).

## Namings: CamelCase vs camelBack

>From this discussion, seems like @AaronBallman and @erichkeane are fine with 
>us using camelBack and all the other differences from [MLIR Style 
>Guide](https://mlir.llvm.org/getting_started/DeveloperGuide/#style-guide) in 
>CIRGen and the rest of CIR. Is that right? This is based on the comment:

> The mixed naming conventions in the header should be fixed (preference is to 
> follow LLVM style if we're changing code around, but if the local preference 
> is for MLIR, that's fine so long as it's consistent).

However, @AaronBallman also wrote:

> Also, the fact that the naming keeps being inconsistent is a pretty strong 
> reason to change to use the LLVM naming convention, at least for external 
> interfaces.

Should we ignore this in light of your first comment? If not, can you clarify 
what do you mean by external interfaces? Just want to make sure we get it right 
looking fwd.

Does this makes sense? Thoughts?

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance &CI) {
   StringRef Action("unknown");
   (void)Action;
 
+  auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;

bcardosolopes wrote:

Right, it's a different discussion than naming. @joker-eph: what's the `auto` 
policy for MLIR, is it different from LLVM/Clang? I'm also having trouble to 
find the previous discussions, let me search a bit more.

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,61 @@
+//===--- CIRGenModule.h - Per-Module state for CIR gen --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is the internal per-translation-unit state used for CIR translation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+#define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+
+#include "CIRGenTypeCache.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/MLIRContext.h"
+
+using namespace clang;
+namespace cir {
+
+/// This class organizes the cross-function state that is used while generating
+/// CIR code.
+class CIRGenModule : public CIRGenTypeCache {
+  CIRGenModule(CIRGenModule &) = delete;
+  CIRGenModule &operator=(CIRGenModule &) = delete;
+
+public:
+  CIRGenModule(mlir::MLIRContext &context, clang::ASTContext &astctx,
+   const clang::CodeGenOptions &CGO,
+   clang::DiagnosticsEngine &Diags);
+
+  ~CIRGenModule();
+
+private:
+  /// Hold Clang AST information.
+  clang::ASTContext &astCtx;
+
+  const clang::LangOptions &langOpts;

bcardosolopes wrote:

Agreed, we've been using a mix so we didn't had to go the extra length for 
something the community could ask us to change as part of upstreaming, so 
indeed this is the right time to make it happen.

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,88 @@
+//===--- CIRGenAction.cpp - LLVM Code generation Frontend Action -===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#include "clang/CIRFrontendAction/CIRGenAction.h"
+#include "clang/CIR/CIRGenerator.h"
+#include "clang/Frontend/CompilerInstance.h"
+
+#include "mlir/IR/MLIRContext.h"
+#include "mlir/IR/OwningOpRef.h"
+
+using namespace cir;
+using namespace clang;
+
+namespace cir {
+
+class CIRGenConsumer : public clang::ASTConsumer {
+
+  virtual void anchor();
+
+  [[maybe_unused]] CIRGenAction::OutputType action;

bcardosolopes wrote:

To silence warnings for things we are about to introduce in follow up commits. 
Sometimes we prefetch the skeleton in NFC fashion, in order to make 
functionality changes have a smaller / more relevant diff. But we could surely 
remove them too. 

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,61 @@
+//=== CIRGenAction.h - CIR Code Generation Frontend Action -*- C++ 
-*--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_CIR_CIRGENACTION_H
+#define LLVM_CLANG_CIR_CIRGENACTION_H
+
+#include "clang/Frontend/FrontendAction.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/OwningOpRef.h"
+
+namespace mlir {
+class MLIRContext;
+class ModuleOp;
+} // namespace mlir
+
+namespace cir {
+class CIRGenConsumer;
+
+class CIRGenAction : public clang::ASTFrontendAction {
+public:
+  enum class OutputType {
+EmitCIR,

bcardosolopes wrote:

This is a bit more complicated that it looks. The rationale is that we need to 
duplicate all actions in order to use clangir transparently in all the 
pipeline, see the current code here: 
https://github.com/llvm/clangir/blob/main/clang/lib/CIR/FrontendAction/CIRGenAction.cpp

So we antecipate all existing available ones: 
https://github.com/llvm/clangir/blob/f52e99e0bd566a4a3bfe73d1991d1ae692407d61/clang/include/clang/CIRFrontendAction/CIRGenAction.h#L33

If you have better ideas we're happy to abide.

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes edited 
https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,61 @@
+//===--- CIRGenModule.h - Per-Module state for CIR gen --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is the internal per-translation-unit state used for CIR translation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+#define LLVM_CLANG_LIB_CIR_CODEGEN_CIRGENMODULE_H
+
+#include "CIRGenTypeCache.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+
+#include "mlir/IR/BuiltinOps.h"
+#include "mlir/IR/MLIRContext.h"
+
+using namespace clang;
+namespace cir {
+
+/// This class organizes the cross-function state that is used while generating
+/// CIR code.
+class CIRGenModule : public CIRGenTypeCache {
+  CIRGenModule(CIRGenModule &) = delete;
+  CIRGenModule &operator=(CIRGenModule &) = delete;
+
+public:
+  CIRGenModule(mlir::MLIRContext &context, clang::ASTContext &astctx,
+   const clang::CodeGenOptions &CGO,
+   clang::DiagnosticsEngine &Diags);
+
+  ~CIRGenModule();
+
+private:
+  /// Hold Clang AST information.
+  clang::ASTContext &astCtx;
+
+  const clang::LangOptions &langOpts;

bcardosolopes wrote:

As long as we can keep this naming (e.g. `langOpts `) in `lib/CIR` and 
`include/clang/CIR` should be fine for us. 

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

@@ -42,6 +47,14 @@ CreateFrontendBaseAction(CompilerInstance &CI) {
   StringRef Action("unknown");
   (void)Action;
 
+  auto UseCIR = CI.getFrontendOpts().UseClangIRPipeline;

bcardosolopes wrote:

Should this be the case for actual lib/CIR/CodeGen too? MLIR uses `auto` 
extensively (perhaps because value semantics is common land there) and we have 
been doing that as well for CIRGen and passes - it's going to be a massive 
change for us, so we it'd be good to know if that should apply everywhere or 
only into the areas of Clang isolated from MLIR.

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Build out AST consumer patterns to reach the entry point into CIRGen (PR #91007)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,4 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s

bcardosolopes wrote:

We don't have any IR to test just yet. @lanza you could use `FileCheck` with 
`--allow-empty` here for now. You could also rename this test `emit-cir.c` 
since the purpose for now is to not crash (as it would before this PR).

https://github.com/llvm/llvm-project/pull/91007
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Add CIRGenerator and plug it via CIRGenAction (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

@@ -2900,7 +2900,7 @@ defm clangir : BoolFOption<"clangir",
   PosFlag,
   NegFlag LLVM 
pipeline to compile">,
   BothFlags<[], [ClangOption, CC1Option], "">>;
-def emit_cir : Flag<["-"], "emit-cir">, Visibility<[CC1Option]>,
+def emit_cir : Flag<["-"], "emit-cir">, Visibility<[ClangOption, CC1Option]>,

bcardosolopes wrote:

Hmmm, this is content for Aaron, who was opposed to my take.

https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Add CIRGenerator and plug it via CIRGenAction (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,5 @@
+// RUN: %clang_cc1 -triple x86_64-unknown-linux-gnu -fclangir -emit-cir %s | 
FileCheck %s
+
+// CHECK: CIRGenModule::buildTopLevelDecl
+
+void foo() {}

bcardosolopes wrote:

Oh, I see what you are doing. I was thinking even more simple, no code, just an 
empty file without invoking FileCheck. So you can remove the printing hack. 
Before this PR, calling into `-emit-cir` would crash, so the idea of the simple 
test is to just make sure it doesn't anymore.

https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Add CIRGenerator and plug it via CIRGenAction (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes requested changes to this pull request.


https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Add CIRGenerator and plug it via CIRGenAction (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes edited 
https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Add CIRGenerator and plug it via CIRGenAction (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,28 @@
+//===--- CIRGenTypeCache.h - Commonly used LLVM types and info -*- C++ 
--*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This structure provides a set of common types useful during CIR emission.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CODEGENTYPECACHE_H
+#define LLVM_CLANG_LIB_CIR_CODEGENTYPECACHE_H
+
+namespace cir {
+
+/// This structure provides a set of types that are commonly used
+/// during IR emission. It's initialized once in CodeGenModule's
+/// constructor and then copied around into new CIRGenFunction's.
+struct CIRGenTypeCache {
+  CIRGenTypeCache() {}
+

bcardosolopes wrote:

Remove the extra newline

https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [CIR] Add CIRGenerator and plug it via CIRGenAction (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes edited 
https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cirgenmodule buildtopleveldecl husk (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes commented:

Perhaps add a `-emit-cir` testcase that doesn't test anything but just proves 
the wiring works. More comments inline.

https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cirgenmodule buildtopleveldecl husk (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,28 @@
+//===--- CIRGenTypeCache.h - Commonly used LLVM types and info -*- C++ 
--*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This structure provides a set of common types useful during CIR emission.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CIR_CODEGENTYPECACHE_H
+#define LLVM_CLANG_LIB_CIR_CODEGENTYPECACHE_H
+
+namespace cir {
+
+/// This structure provides a set of types that are commonly used
+/// during IR emission. It's initialized once in CodeGenModule's
+/// constructor and then copied around into new CIRGenFunction's.
+struct CIRGenTypeCache {
+  CIRGenTypeCache() {}
+

bcardosolopes wrote:

Tie this up?

https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cirgenmodule buildtopleveldecl husk (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,62 @@
+//=== CIRGenAction.h - CIR Code Generation Frontend Action -*- C++ 
-*--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_CIR_CIRGENACTION_H
+#define LLVM_CLANG_CIR_CIRGENACTION_H
+
+#include "clang/Frontend/FrontendAction.h"
+
+namespace mlir {
+class MLIRContext;
+class ModuleOp;
+template  class OwningOpRef;
+} // namespace mlir
+
+namespace cir {
+
+class CIRGenAction : public clang::ASTFrontendAction {
+public:
+  enum class OutputType {
+EmitCIR,
+  };
+
+private:
+  friend class CIRGenConsumer;
+
+  // TODO: this is redundant but just using the OwningModuleRef requires more 
of
+  // clang against MLIR. Hide this somewhere else.
+  std::unique_ptr> mlirModule;

bcardosolopes wrote:

The original comment is from 2 years ago, any idea why is still relevant?

https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cirgenmodule buildtopleveldecl husk (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,42 @@
+//===--- CIRGenModule.h - Per-Module state for CIR gen --*- C++ 
-*-===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+//
+// This is the internal per-translation-unit state used for CIR translation.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_LIB_CODEGEN_CIRGENMODULE_H
+#define LLVM_CLANG_LIB_CODEGEN_CIRGENMODULE_H
+
+#include "CIRGenTypeCache.h"
+
+#include "clang/AST/ASTContext.h"
+#include "clang/Basic/CodeGenOptions.h"
+#include "clang/Basic/Diagnostic.h"
+#include "mlir/IR/MLIRContext.h"
+
+using namespace clang;
+namespace cir {
+/// Implementation of a CIR/MLIR emission from Clang AST.
+///
+/// This will emit operations that are specific to C(++)/ObjC(++) language,
+/// preserving the semantics of the language and (hopefully) allow to perform

bcardosolopes wrote:

perhaps remove the "hopefully" and just have a more solid statement

https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cirgenmodule buildtopleveldecl husk (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,62 @@
+//=== CIRGenAction.h - CIR Code Generation Frontend Action -*- C++ 
-*--===//
+//
+// Part of the LLVM Project, under the Apache License v2.0 with LLVM 
Exceptions.
+// See https://llvm.org/LICENSE.txt for license information.
+// SPDX-License-Identifier: Apache-2.0 WITH LLVM-exception
+//
+//===--===//
+
+#ifndef LLVM_CLANG_CIR_CIRGENACTION_H
+#define LLVM_CLANG_CIR_CIRGENACTION_H
+
+#include "clang/Frontend/FrontendAction.h"
+
+namespace mlir {
+class MLIRContext;
+class ModuleOp;
+template  class OwningOpRef;
+} // namespace mlir
+
+namespace cir {
+
+class CIRGenAction : public clang::ASTFrontendAction {
+public:
+  enum class OutputType {
+EmitCIR,
+  };
+
+private:
+  friend class CIRGenConsumer;
+
+  // TODO: this is redundant but just using the OwningModuleRef requires more 
of
+  // clang against MLIR. Hide this somewhere else.
+  std::unique_ptr> mlirModule;
+
+  mlir::MLIRContext *mlirContext;
+
+protected:
+  CIRGenAction(OutputType action, mlir::MLIRContext *mlirContext = nullptr);
+
+  void foo() {

bcardosolopes wrote:

What is this about?

https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] cirgenmodule buildtopleveldecl husk (PR #90831)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes edited 
https://github.com/llvm/llvm-project/pull/90831
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Fix finding instantiated decls for class template specializations during instantiation (PR #72346)

[Bruno Cardoso Lopes via cfe-commits]

bcardosolopes wrote:

I'm not 100% confident here but the fix makes sense and seems good (nice 
testcase!).

https://github.com/llvm/llvm-project/pull/72346
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang-tools-extra] [clang-apply-replacements] Apply format only if --format is specified (PR #70801)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes commented:

Can you please add a testcase? 

https://github.com/llvm/llvm-project/pull/70801
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[llvm] [libc] [clang-tools-extra] [lld] [libcxx] [clang] [flang] [lldb] [OpenACC] Initial commits to support OpenACC (PR #70234)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,14 @@
+// RUN: %clang -S -### -fopenacc %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-DRIVER
+// CHECK-DRIVER: "-cc1" {{.*}} "-fopenacc"

bcardosolopes wrote:

Thanks for the explanation!

https://github.com/llvm/llvm-project/pull/70234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [clang-tools-extra] [llvm] [RFC] Perform lifetime bound checks for arguments to coroutine (PR #69360)

[Bruno Cardoso Lopes via cfe-commits]

@@ -7584,11 +7584,22 @@ static void 
visitLifetimeBoundArguments(IndirectLocalPath &Path, Expr *Call,
 
   if (ObjectArg && implicitObjectParamIsLifetimeBound(Callee))
 VisitLifetimeBoundArg(Callee, ObjectArg);
-
+  bool checkCoroCall = false;
+  if (const auto *RD = Callee->getReturnType()->getAsRecordDecl()) {
+for (const auto &attr :
+ RD->getUnderlyingDecl()->specific_attrs()) {
+  // Only for demonstration: Get feedback and add a clang annotation as an
+  // extension.
+  if (attr->getAnnotation() == "coro_type") {

bcardosolopes wrote:

I don't think you need this to find if a record is a "coroutine type", there 
are other ways, for example:
```
if (rec->getDeclName().isIdentifier() && rec->getName() == "promise_type") {

```

https://github.com/llvm/llvm-project/pull/69360
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [libc] [flang] [lldb] [libcxx] [llvm] [lld] [clang-tools-extra] [OpenACC] Initial commits to support OpenACC (PR #70234)

[Bruno Cardoso Lopes via cfe-commits]

@@ -605,6 +605,17 @@ static void InitializeStandardPredefinedMacros(const 
TargetInfo &TI,
   Builder.defineMacro("HIP_API_PER_THREAD_DEFAULT_STREAM");
 }
   }
+
+  if (LangOpts.OpenACC) {
+// FIXME: When we have full support for OpenACC, we should set this to the
+// version we support. Until then, set as '1' by default, but provide a
+// temporary mechanism for users to override this so real-world examples 
can
+// be tested against.
+if (!LangOpts.OpenACCMacroOverride.empty())
+  Builder.defineMacro("_OPENACC", LangOpts.OpenACCMacroOverride);
+else
+  Builder.defineMacro("_OPENACC", "1");

bcardosolopes wrote:

This probably deserves a test akin to `-E -dM` and FileCheck for the macro.

https://github.com/llvm/llvm-project/pull/70234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[flang] [clang] [llvm] [libc] [lld] [libcxx] [clang-tools-extra] [lldb] [OpenACC] Initial commits to support OpenACC (PR #70234)

[Bruno Cardoso Lopes via cfe-commits]

@@ -0,0 +1,14 @@
+// RUN: %clang -S -### -fopenacc %s 2>&1 | FileCheck %s 
--check-prefix=CHECK-DRIVER
+// CHECK-DRIVER: "-cc1" {{.*}} "-fopenacc"

bcardosolopes wrote:

How does the interaction between using both `-fopenmp` and `-fopenacc` at the 
same time should look like? One takes precedence? Or should it be rejected?

https://github.com/llvm/llvm-project/pull/70234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] [lld] [flang] [clang] [llvm] [lldb] [clang-tools-extra] [libc] [OpenACC] Initial commits to support OpenACC (PR #70234)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes commented:

The changes in this patch looks pretty straightforward! Left some inline 
comments.

https://github.com/llvm/llvm-project/pull/70234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libcxx] [llvm] [lld] [clang] [libc] [flang] [lldb] [clang-tools-extra] [OpenACC] Initial commits to support OpenACC (PR #70234)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes edited 
https://github.com/llvm/llvm-project/pull/70234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[libc] [clang] [lld] [llvm] [lldb] [libcxx] [clang-tools-extra] [flang] [OpenACC] Initial commits to support OpenACC (PR #70234)

[Bruno Cardoso Lopes via cfe-commits]

@@ -1342,6 +1342,15 @@ def err_opencl_logical_exclusive_or : Error<
 def err_openclcxx_virtual_function : Error<
   "virtual functions are not supported in C++ for OpenCL">;
 
+// OpenACC Support.
+def warn_pragma_acc_ignored : Warning<
+  "unexpected '#pragma acc ...' in program">, InGroup, 
DefaultIgnore;
+def err_acc_unexpected_directive : Error<
+  "unexpected OpenACC directive %select{|'#pragma acc %1'}0">;
+def warn_pragma_acc_unimplemented
+: Warning<"OpenACC Directives not yet implemented, pragma ignored">,

bcardosolopes wrote:

Does `Directives` need to be capitalized?

https://github.com/llvm/llvm-project/pull/70234
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes approved this pull request.

LGTM. @ChuanqiXu9 should give the final blessing though.

https://github.com/llvm/llvm-project/pull/70973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)

[Bruno Cardoso Lopes via cfe-commits]

@@ -1965,9 +1965,15 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation 
Loc) {
 if (PD->getType()->isDependentType())
   continue;
 
+// Preserve the referenced state for unused parameter diagnostics.
+bool DeclReferenced = PD->isReferenced();
+
 ExprResult PDRefExpr =
 BuildDeclRefExpr(PD, PD->getType().getNonReferenceType(),
  ExprValueKind::VK_LValue, Loc); // FIXME: scope?
+
+PD->setReferenced(DeclReferenced);

bcardosolopes wrote:

As you mentioned offline, it's because creating the expression will mark it 
referenced, gotcha.

https://github.com/llvm/llvm-project/pull/70973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes edited 
https://github.com/llvm/llvm-project/pull/70973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes edited 
https://github.com/llvm/llvm-project/pull/70973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)

[Bruno Cardoso Lopes via cfe-commits]

@@ -1965,9 +1965,15 @@ bool Sema::buildCoroutineParameterMoves(SourceLocation 
Loc) {
 if (PD->getType()->isDependentType())
   continue;
 
+// Preserve the referenced state for unused parameter diagnostics.
+bool DeclReferenced = PD->isReferenced();
+
 ExprResult PDRefExpr =
 BuildDeclRefExpr(PD, PD->getType().getNonReferenceType(),
  ExprValueKind::VK_LValue, Loc); // FIXME: scope?
+
+PD->setReferenced(DeclReferenced);

bcardosolopes wrote:

What happens in `BuildDeclRefExpr` such that `PD` loses track of whether it is 
referenced? If something is changing `PD` in a bad way, perhaps at that point 
might be better to set the appropriated method instead. 

https://github.com/llvm/llvm-project/pull/70973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang] Preserve coroutine parameter referenced state (PR #70973)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes commented:

Thanks for improving this! I haven't looked at your previous review of this PR, 
but I like the simplicity of this one. 

https://github.com/llvm/llvm-project/pull/70973
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][LLVM][Coroutines] Prevent __coro_gro from outliving __promise (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes closed 
https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][LLVM][Coroutines] Prevent __coro_gro from outliving __promise (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

bcardosolopes wrote:

Applied suggested changes and updated PR.

https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][LLVM][Coroutines] Prevent __coro_gro from outliving __promise (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes resolved 
https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][LLVM][Coroutines] Prevent __coro_gro from outliving __promise (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

@@ -104,3 +105,5 @@ invoker g() {
   // CHECK: call void 
@_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
   co_return;
 }
+
+// CHECK: ![[OutFrameMetadata]] = !{}

bcardosolopes wrote:

Done

https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][LLVM][Coroutines] Prevent __coro_gro from outliving __promise (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes resolved 
https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][LLVM][Coroutines] Prevent __coro_gro from outliving __promise (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

@@ -6691,6 +6691,22 @@ sections that the user does not want removed after 
linking.
   ...
   !0 = !{}
 
+'``coro.outside.frame``' Metadata
+^^
+
+``coro.outside.frame`` metadata may be attached to an alloca instruction to
+to signify that it shouldn't be promoted to the coroutine frame, useful for
+filtering allocas out by the frontend when emitting internal control 
mechanisms.
+Additionally, this metadata is only used as a flag, so the associated
+node must be empty.
+
+.. code-block:: text
+
+  %__coro_gro = alloca %struct.GroType, align 1, !coro.outside.frame !0
+
+  ...
+  !0 = !{}
+

bcardosolopes wrote:

Done!

https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][LLVM][Coroutines] Prevent __coro_gro from outliving __promise (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes updated 
https://github.com/llvm/llvm-project/pull/66706

>From 6312dd56ed3a2f47e7291ae32ca044622a317259 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes 
Date: Wed, 20 Sep 2023 15:00:06 -0700
Subject: [PATCH 1/2] [Clang][LLVM][Coroutines] Prevent __coro_gro from
 outliving __promise

When dealing with short-circuiting coroutines (e.g. expected), the deferred
calls that resolve the get_return_object are currently being emitted after we
delete the coroutine frame.

This was caught by ASAN when using optimizations -O1 and above: optimizations
after inlining would place the __coro_gro in the heap, and subsequent delete of
the coroframe followed by the conversion -> BOOM.

This patch forbids the GRO to be placed in the coroutine frame, by adding a new
metadata node that can be attached to `alloca` instructions.

This fixes #49843.
---
 clang/lib/CodeGen/CGCoroutine.cpp|  5 +
 clang/test/CodeGenCoroutines/coro-gro.cpp|  5 -
 llvm/docs/LangRef.rst| 16 
 llvm/include/llvm/IR/FixedMetadataKinds.def  |  1 +
 llvm/lib/Transforms/Coroutines/CoroFrame.cpp |  5 +
 5 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 448943084acedf3..58310216ecff1f5 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -535,6 +535,11 @@ struct GetReturnObjectManager {
 Builder.CreateStore(Builder.getFalse(), GroActiveFlag);
 
 GroEmission = CGF.EmitAutoVarAlloca(*GroVarDecl);
+auto *GroAlloca = dyn_cast_or_null(
+GroEmission.getOriginalAllocatedAddress().getPointer());
+assert(GroAlloca && "expected alloca to be emitted");
+GroAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
+   llvm::MDNode::get(CGF.CGM.getLLVMContext(), {}));
 
 // Remember the top of EHStack before emitting the cleanup.
 auto old_top = CGF.EHStack.stable_begin();
diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp 
b/clang/test/CodeGenCoroutines/coro-gro.cpp
index b48b769950ae871..ba872e39f4e3de8 100644
--- a/clang/test/CodeGenCoroutines/coro-gro.cpp
+++ b/clang/test/CodeGenCoroutines/coro-gro.cpp
@@ -30,12 +30,13 @@ void doSomething() noexcept;
 int f() {
   // CHECK: %[[RetVal:.+]] = alloca i32
   // CHECK: %[[GroActive:.+]] = alloca i1
+  // CHECK: %[[CoroGro:.+]] = alloca %struct.GroType, {{.*}} 
!coro.outside.frame ![[OutFrameMetadata:.+]]
 
   // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64()
   // CHECK: call noalias noundef nonnull ptr @_Znwm(i64 noundef %[[Size]])
   // CHECK: store i1 false, ptr %[[GroActive]]
   // CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_typeC1Ev(
-  // CHECK: call void 
@_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv(
+  // CHECK: call void 
@_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv({{.*}} 
%[[CoroGro]]
   // CHECK: store i1 true, ptr %[[GroActive]]
 
   Cleanup cleanup;
@@ -104,3 +105,5 @@ invoker g() {
   // CHECK: call void 
@_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
   co_return;
 }
+
+// CHECK: ![[OutFrameMetadata]] = !{}
\ No newline at end of file
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index f542e70bcfee810..9e3f8962ce1b52e 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -6691,6 +6691,22 @@ sections that the user does not want removed after 
linking.
   ...
   !0 = !{}
 
+'``coro.outside.frame``' Metadata
+^^
+
+``coro.outside.frame`` metadata may be attached to an alloca instruction to
+to signify that it shouldn't be promoted to the coroutine frame, useful for
+filtering allocas out by the frontend when emitting internal control 
mechanisms.
+Additionally, this metadata is only used as a flag, so the associated
+node must be empty.
+
+.. code-block:: text
+
+  %__coro_gro = alloca %struct.GroType, align 1, !coro.outside.frame !0
+
+  ...
+  !0 = !{}
+
 '``unpredictable``' Metadata
 
 
diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def 
b/llvm/include/llvm/IR/FixedMetadataKinds.def
index 8723bf2a0680c77..b375d0f0912060f 100644
--- a/llvm/include/llvm/IR/FixedMetadataKinds.def
+++ b/llvm/include/llvm/IR/FixedMetadataKinds.def
@@ -50,3 +50,4 @@ LLVM_FIXED_MD_KIND(MD_callsite, "callsite", 35)
 LLVM_FIXED_MD_KIND(MD_kcfi_type, "kcfi_type", 36)
 LLVM_FIXED_MD_KIND(MD_pcsections, "pcsections", 37)
 LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38)
+LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39)
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp 
b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index a12dd710174f3f8..9c1ee322ce0b177 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -2804,6 +2804,11 @@ static void collectFrameAlloca(AllocaInst *AI, 
coro::Shape &Shape,
   if 

[clang] [Clang][LLVM][Coroutines] Prevent __coro_gro from outliving __promise (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

bcardosolopes wrote:

Thanks for the clarifications

> By forcing the GRO not living on the coroutine frames, it shouldn't be a 
> problem if the lifetime of `__coro_gro` outlives the `__promise`. The only 
> limit is that the initialization of `__coro_gro` should be in the lifetime of 
> `__promise`.

Applied your suggestion based on #49843, and edited description + title.

https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][LLVM][Coroutines] Prevent __coro_gro from outliving __promise (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes edited 
https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Coroutines] Improve GRO handling to better fit scopes and cleanups (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes edited 
https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Coroutines] Improve GRO handling to better fit scopes and cleanups (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes updated 
https://github.com/llvm/llvm-project/pull/66706

>From 6312dd56ed3a2f47e7291ae32ca044622a317259 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes 
Date: Wed, 20 Sep 2023 15:00:06 -0700
Subject: [PATCH] [Clang][LLVM][Coroutines] Prevent __coro_gro from outliving
 __promise

When dealing with short-circuiting coroutines (e.g. expected), the deferred
calls that resolve the get_return_object are currently being emitted after we
delete the coroutine frame.

This was caught by ASAN when using optimizations -O1 and above: optimizations
after inlining would place the __coro_gro in the heap, and subsequent delete of
the coroframe followed by the conversion -> BOOM.

This patch forbids the GRO to be placed in the coroutine frame, by adding a new
metadata node that can be attached to `alloca` instructions.

This fixes #49843.
---
 clang/lib/CodeGen/CGCoroutine.cpp|  5 +
 clang/test/CodeGenCoroutines/coro-gro.cpp|  5 -
 llvm/docs/LangRef.rst| 16 
 llvm/include/llvm/IR/FixedMetadataKinds.def  |  1 +
 llvm/lib/Transforms/Coroutines/CoroFrame.cpp |  5 +
 5 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 448943084acedf3..58310216ecff1f5 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -535,6 +535,11 @@ struct GetReturnObjectManager {
 Builder.CreateStore(Builder.getFalse(), GroActiveFlag);
 
 GroEmission = CGF.EmitAutoVarAlloca(*GroVarDecl);
+auto *GroAlloca = dyn_cast_or_null(
+GroEmission.getOriginalAllocatedAddress().getPointer());
+assert(GroAlloca && "expected alloca to be emitted");
+GroAlloca->setMetadata(llvm::LLVMContext::MD_coro_outside_frame,
+   llvm::MDNode::get(CGF.CGM.getLLVMContext(), {}));
 
 // Remember the top of EHStack before emitting the cleanup.
 auto old_top = CGF.EHStack.stable_begin();
diff --git a/clang/test/CodeGenCoroutines/coro-gro.cpp 
b/clang/test/CodeGenCoroutines/coro-gro.cpp
index b48b769950ae871..ba872e39f4e3de8 100644
--- a/clang/test/CodeGenCoroutines/coro-gro.cpp
+++ b/clang/test/CodeGenCoroutines/coro-gro.cpp
@@ -30,12 +30,13 @@ void doSomething() noexcept;
 int f() {
   // CHECK: %[[RetVal:.+]] = alloca i32
   // CHECK: %[[GroActive:.+]] = alloca i1
+  // CHECK: %[[CoroGro:.+]] = alloca %struct.GroType, {{.*}} 
!coro.outside.frame ![[OutFrameMetadata:.+]]
 
   // CHECK: %[[Size:.+]] = call i64 @llvm.coro.size.i64()
   // CHECK: call noalias noundef nonnull ptr @_Znwm(i64 noundef %[[Size]])
   // CHECK: store i1 false, ptr %[[GroActive]]
   // CHECK: call void @_ZNSt16coroutine_traitsIiJEE12promise_typeC1Ev(
-  // CHECK: call void 
@_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv(
+  // CHECK: call void 
@_ZNSt16coroutine_traitsIiJEE12promise_type17get_return_objectEv({{.*}} 
%[[CoroGro]]
   // CHECK: store i1 true, ptr %[[GroActive]]
 
   Cleanup cleanup;
@@ -104,3 +105,5 @@ invoker g() {
   // CHECK: call void 
@_ZN7invoker15invoker_promise17get_return_objectEv({{.*}} %[[AggRes]]
   co_return;
 }
+
+// CHECK: ![[OutFrameMetadata]] = !{}
\ No newline at end of file
diff --git a/llvm/docs/LangRef.rst b/llvm/docs/LangRef.rst
index f542e70bcfee810..9e3f8962ce1b52e 100644
--- a/llvm/docs/LangRef.rst
+++ b/llvm/docs/LangRef.rst
@@ -6691,6 +6691,22 @@ sections that the user does not want removed after 
linking.
   ...
   !0 = !{}
 
+'``coro.outside.frame``' Metadata
+^^
+
+``coro.outside.frame`` metadata may be attached to an alloca instruction to
+to signify that it shouldn't be promoted to the coroutine frame, useful for
+filtering allocas out by the frontend when emitting internal control 
mechanisms.
+Additionally, this metadata is only used as a flag, so the associated
+node must be empty.
+
+.. code-block:: text
+
+  %__coro_gro = alloca %struct.GroType, align 1, !coro.outside.frame !0
+
+  ...
+  !0 = !{}
+
 '``unpredictable``' Metadata
 
 
diff --git a/llvm/include/llvm/IR/FixedMetadataKinds.def 
b/llvm/include/llvm/IR/FixedMetadataKinds.def
index 8723bf2a0680c77..b375d0f0912060f 100644
--- a/llvm/include/llvm/IR/FixedMetadataKinds.def
+++ b/llvm/include/llvm/IR/FixedMetadataKinds.def
@@ -50,3 +50,4 @@ LLVM_FIXED_MD_KIND(MD_callsite, "callsite", 35)
 LLVM_FIXED_MD_KIND(MD_kcfi_type, "kcfi_type", 36)
 LLVM_FIXED_MD_KIND(MD_pcsections, "pcsections", 37)
 LLVM_FIXED_MD_KIND(MD_DIAssignID, "DIAssignID", 38)
+LLVM_FIXED_MD_KIND(MD_coro_outside_frame, "coro.outside.frame", 39)
diff --git a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp 
b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
index a12dd710174f3f8..9c1ee322ce0b177 100644
--- a/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
+++ b/llvm/lib/Transforms/Coroutines/CoroFrame.cpp
@@ -2804,6 +2804,11 @@ static void collectFrameAlloca(AllocaInst *AI, 
coro::Shape &Shape,
   if (AI 

[clang] [Clang][Coroutines] Improve GRO handling to better fit scopes and cleanups (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

bcardosolopes wrote:

Updated the patch to account for failing libcxx tests, and to not change 
current codegen when GRO allocas are not involved.

https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Coroutines] Improve GRO handling to better fit scopes and cleanups (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes updated 
https://github.com/llvm/llvm-project/pull/66706

>From f9d54b81d4c3c10c3dd26193d9bef52785826f21 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes 
Date: Fri, 15 Sep 2023 15:40:20 -0700
Subject: [PATCH 1/3] [Clang][Coroutines] Improve GRO handling to better fit
 scopes and cleanups

When dealing with short-circuiting coroutines (e.g. `expected`), the deferred
calls that resolve the `get_return_object` are currently being emitted *after*
we delete the coroutine frame.

This was caught by ASAN when using optimizations `-O1` and above:
optimizations after inlining would place the `__coro_gro` in the heap, and
subsequent delete of the coroframe followed by the conversion -> BOOM.

This patch fixes GRO placement into scopes and cleanups such that:

1. Emit the return block while within `CallCoroDelete` cleanup scope. This
makes the conversion call to happen right after `coro.end` but before deleting
the coroutine frame.

2. Change gro allocation to happen *after* `__promise` is allocated. The effect
is to bound `__coro_gro` within the lifetime of `__promise`, which should make
destruction of the conversion result to happen before we also delete the
coroframe.
---
 clang/lib/CodeGen/CGCoroutine.cpp |  38 +--
 .../test/CodeGenCoroutines/coro-dest-slot.cpp |  18 +-
 .../test/CodeGenCoroutines/coro-expected.cpp  | 223 ++
 clang/test/CodeGenCoroutines/coro-gro.cpp |  30 ++-
 4 files changed, 280 insertions(+), 29 deletions(-)
 create mode 100644 clang/test/CodeGenCoroutines/coro-expected.cpp

diff --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 448943084acedf3..741a8f9990c1ed7 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -657,8 +657,6 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt &S) {
   CurCoro.Data->CoroBegin = CoroBegin;
 
   GetReturnObjectManager GroManager(*this, S);
-  GroManager.EmitGroAlloca();
-
   CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB);
   {
 CGDebugInfo *DI = getDebugInfo();
@@ -696,6 +694,12 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt &S) {
 // promise local variable was not emitted yet.
 CoroId->setArgOperand(1, PromiseAddrVoidPtr);
 
+// Emit the alloca for ` __coro_gro`  *after* it's done for the promise.
+// This guarantees that while looking at deferred results from calls to
+// `get_return_object`, the lifetime of ` __coro_gro` is enclosed by the
+// `__promise` lifetime, and cleanup order is properly respected.
+GroManager.EmitGroAlloca();
+
 // Now we have the promise, initialize the GRO
 GroManager.EmitGroInit();
 
@@ -752,22 +756,22 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt &S) {
   // We don't need FinalBB. Emit it to make sure the block is deleted.
   EmitBlock(FinalBB, /*IsFinished=*/true);
 }
-  }
 
-  EmitBlock(RetBB);
-  // Emit coro.end before getReturnStmt (and parameter destructors), since
-  // resume and destroy parts of the coroutine should not include them.
-  llvm::Function *CoroEnd = CGM.getIntrinsic(llvm::Intrinsic::coro_end);
-  Builder.CreateCall(CoroEnd,
- {NullPtr, Builder.getFalse(),
-  llvm::ConstantTokenNone::get(CoroEnd->getContext())});
-
-  if (Stmt *Ret = S.getReturnStmt()) {
-// Since we already emitted the return value above, so we shouldn't
-// emit it again here.
-if (GroManager.DirectEmit)
-  cast(Ret)->setRetValue(nullptr);
-EmitStmt(Ret);
+EmitBlock(RetBB);
+// Emit coro.end before getReturnStmt (and parameter destructors), since
+// resume and destroy parts of the coroutine should not include them.
+llvm::Function *CoroEnd = CGM.getIntrinsic(llvm::Intrinsic::coro_end);
+Builder.CreateCall(CoroEnd,
+   {NullPtr, Builder.getFalse(),
+llvm::ConstantTokenNone::get(CoroEnd->getContext())});
+
+if (Stmt *Ret = S.getReturnStmt()) {
+  // Since we already emitted the return value above, so we shouldn't
+  // emit it again here.
+  if (GroManager.DirectEmit)
+cast(Ret)->setRetValue(nullptr);
+  EmitStmt(Ret);
+}
   }
 
   // LLVM require the frontend to mark the coroutine.
diff --git a/clang/test/CodeGenCoroutines/coro-dest-slot.cpp 
b/clang/test/CodeGenCoroutines/coro-dest-slot.cpp
index d794c74cd52d61a..0d2416e7913da0d 100644
--- a/clang/test/CodeGenCoroutines/coro-dest-slot.cpp
+++ b/clang/test/CodeGenCoroutines/coro-dest-slot.cpp
@@ -17,6 +17,7 @@ struct coro {
 extern "C" coro f(int) { co_return; }
 // Verify that cleanup.dest.slot is eliminated in a coroutine.
 // CHECK-LABEL: f(
+// CHECK: %[[PROMISE:.+]] = alloca %"struct.coro::promise_type"
 // CHECK: %[[INIT_SUSPEND:.+]] = call i8 @llvm.coro.suspend(
 // CHECK-NEXT: switch i8 %[[INIT_SUSPEND]], label
 // CHECK-NEXT:   i8 0, label %[[INIT_READY:.+]]
@@ -32,9 +

[clang] [Clang][Coroutines] Improve GRO handling to better fit scopes and cleanups (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes updated 
https://github.com/llvm/llvm-project/pull/66706

>From f9d54b81d4c3c10c3dd26193d9bef52785826f21 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes 
Date: Fri, 15 Sep 2023 15:40:20 -0700
Subject: [PATCH 1/2] [Clang][Coroutines] Improve GRO handling to better fit
 scopes and cleanups

When dealing with short-circuiting coroutines (e.g. `expected`), the deferred
calls that resolve the `get_return_object` are currently being emitted *after*
we delete the coroutine frame.

This was caught by ASAN when using optimizations `-O1` and above:
optimizations after inlining would place the `__coro_gro` in the heap, and
subsequent delete of the coroframe followed by the conversion -> BOOM.

This patch fixes GRO placement into scopes and cleanups such that:

1. Emit the return block while within `CallCoroDelete` cleanup scope. This
makes the conversion call to happen right after `coro.end` but before deleting
the coroutine frame.

2. Change gro allocation to happen *after* `__promise` is allocated. The effect
is to bound `__coro_gro` within the lifetime of `__promise`, which should make
destruction of the conversion result to happen before we also delete the
coroframe.
---
 clang/lib/CodeGen/CGCoroutine.cpp |  38 +--
 .../test/CodeGenCoroutines/coro-dest-slot.cpp |  18 +-
 .../test/CodeGenCoroutines/coro-expected.cpp  | 223 ++
 clang/test/CodeGenCoroutines/coro-gro.cpp |  30 ++-
 4 files changed, 280 insertions(+), 29 deletions(-)
 create mode 100644 clang/test/CodeGenCoroutines/coro-expected.cpp

diff --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 448943084acedf3..741a8f9990c1ed7 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -657,8 +657,6 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt &S) {
   CurCoro.Data->CoroBegin = CoroBegin;
 
   GetReturnObjectManager GroManager(*this, S);
-  GroManager.EmitGroAlloca();
-
   CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB);
   {
 CGDebugInfo *DI = getDebugInfo();
@@ -696,6 +694,12 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt &S) {
 // promise local variable was not emitted yet.
 CoroId->setArgOperand(1, PromiseAddrVoidPtr);
 
+// Emit the alloca for ` __coro_gro`  *after* it's done for the promise.
+// This guarantees that while looking at deferred results from calls to
+// `get_return_object`, the lifetime of ` __coro_gro` is enclosed by the
+// `__promise` lifetime, and cleanup order is properly respected.
+GroManager.EmitGroAlloca();
+
 // Now we have the promise, initialize the GRO
 GroManager.EmitGroInit();
 
@@ -752,22 +756,22 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt &S) {
   // We don't need FinalBB. Emit it to make sure the block is deleted.
   EmitBlock(FinalBB, /*IsFinished=*/true);
 }
-  }
 
-  EmitBlock(RetBB);
-  // Emit coro.end before getReturnStmt (and parameter destructors), since
-  // resume and destroy parts of the coroutine should not include them.
-  llvm::Function *CoroEnd = CGM.getIntrinsic(llvm::Intrinsic::coro_end);
-  Builder.CreateCall(CoroEnd,
- {NullPtr, Builder.getFalse(),
-  llvm::ConstantTokenNone::get(CoroEnd->getContext())});
-
-  if (Stmt *Ret = S.getReturnStmt()) {
-// Since we already emitted the return value above, so we shouldn't
-// emit it again here.
-if (GroManager.DirectEmit)
-  cast(Ret)->setRetValue(nullptr);
-EmitStmt(Ret);
+EmitBlock(RetBB);
+// Emit coro.end before getReturnStmt (and parameter destructors), since
+// resume and destroy parts of the coroutine should not include them.
+llvm::Function *CoroEnd = CGM.getIntrinsic(llvm::Intrinsic::coro_end);
+Builder.CreateCall(CoroEnd,
+   {NullPtr, Builder.getFalse(),
+llvm::ConstantTokenNone::get(CoroEnd->getContext())});
+
+if (Stmt *Ret = S.getReturnStmt()) {
+  // Since we already emitted the return value above, so we shouldn't
+  // emit it again here.
+  if (GroManager.DirectEmit)
+cast(Ret)->setRetValue(nullptr);
+  EmitStmt(Ret);
+}
   }
 
   // LLVM require the frontend to mark the coroutine.
diff --git a/clang/test/CodeGenCoroutines/coro-dest-slot.cpp 
b/clang/test/CodeGenCoroutines/coro-dest-slot.cpp
index d794c74cd52d61a..0d2416e7913da0d 100644
--- a/clang/test/CodeGenCoroutines/coro-dest-slot.cpp
+++ b/clang/test/CodeGenCoroutines/coro-dest-slot.cpp
@@ -17,6 +17,7 @@ struct coro {
 extern "C" coro f(int) { co_return; }
 // Verify that cleanup.dest.slot is eliminated in a coroutine.
 // CHECK-LABEL: f(
+// CHECK: %[[PROMISE:.+]] = alloca %"struct.coro::promise_type"
 // CHECK: %[[INIT_SUSPEND:.+]] = call i8 @llvm.coro.suspend(
 // CHECK-NEXT: switch i8 %[[INIT_SUSPEND]], label
 // CHECK-NEXT:   i8 0, label %[[INIT_READY:.+]]
@@ -32,9 +

[clang] [Clang][Coroutines] Improve GRO handling to better fit scopes and cleanups (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

bcardosolopes wrote:

Thanks for the fast reply @ChuanqiXu9 

> I remember that there is a defect that we may place the GRO on the coroutine 
> frame.

Can you point me to such defect? I had no luck searching for it.

> And my instinct reaction is that would this patch be covered by forcing GRO 
> to not live on the coroutine frame?

How do you suggest we do so? Even if we teach the optimizers to not touch the 
GRO (e.g. `-O0`) there's still a correctness issue. The generated code is still 
wrong, given that currently, the lifetime of `__coro_gro` outlives the 
`__promise`, but accessing the proxy for delayed conversion might still depend 
on the promise.

Also, what do you mean by "patch be covered"? I'm confused.

https://github.com/llvm/llvm-project/pull/66706
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

[clang] [Clang][Coroutines] Improve GRO handling to better fit scopes and cleanups (PR #66706)

[Bruno Cardoso Lopes via cfe-commits]

https://github.com/bcardosolopes created 
https://github.com/llvm/llvm-project/pull/66706

When dealing with short-circuiting coroutines (e.g. `expected`), the deferred 
calls that resolve the `get_return_object` are currently being emitted *after* 
we delete the coroutine frame.

This was caught by ASAN when using optimizations `-O1` and above: optimizations 
after inlining would place the `__coro_gro` in the heap, and subsequent delete 
of the coroframe followed by the conversion -> BOOM.

This patch fixes GRO placement into scopes and cleanups such that:

1. Emit the return block while within `CallCoroDelete` cleanup scope. This 
makes the conversion call to happen right after `coro.end` but before deleting 
the coroutine frame.

2. Change gro allocation to happen *after* `__promise` is allocated. The effect 
is to bound `__coro_gro` within the lifetime of `__promise`, which should make 
destruction of the conversion result to happen before we also delete the 
coroframe.

>From f9d54b81d4c3c10c3dd26193d9bef52785826f21 Mon Sep 17 00:00:00 2001
From: Bruno Cardoso Lopes 
Date: Fri, 15 Sep 2023 15:40:20 -0700
Subject: [PATCH] [Clang][Coroutines] Improve GRO handling to better fit scopes
 and cleanups

When dealing with short-circuiting coroutines (e.g. `expected`), the deferred
calls that resolve the `get_return_object` are currently being emitted *after*
we delete the coroutine frame.

This was caught by ASAN when using optimizations `-O1` and above:
optimizations after inlining would place the `__coro_gro` in the heap, and
subsequent delete of the coroframe followed by the conversion -> BOOM.

This patch fixes GRO placement into scopes and cleanups such that:

1. Emit the return block while within `CallCoroDelete` cleanup scope. This
makes the conversion call to happen right after `coro.end` but before deleting
the coroutine frame.

2. Change gro allocation to happen *after* `__promise` is allocated. The effect
is to bound `__coro_gro` within the lifetime of `__promise`, which should make
destruction of the conversion result to happen before we also delete the
coroframe.
---
 clang/lib/CodeGen/CGCoroutine.cpp |  38 +--
 .../test/CodeGenCoroutines/coro-dest-slot.cpp |  18 +-
 .../test/CodeGenCoroutines/coro-expected.cpp  | 223 ++
 clang/test/CodeGenCoroutines/coro-gro.cpp |  30 ++-
 4 files changed, 280 insertions(+), 29 deletions(-)
 create mode 100644 clang/test/CodeGenCoroutines/coro-expected.cpp

diff --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 448943084acedf3..741a8f9990c1ed7 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -657,8 +657,6 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt &S) {
   CurCoro.Data->CoroBegin = CoroBegin;
 
   GetReturnObjectManager GroManager(*this, S);
-  GroManager.EmitGroAlloca();
-
   CurCoro.Data->CleanupJD = getJumpDestInCurrentScope(RetBB);
   {
 CGDebugInfo *DI = getDebugInfo();
@@ -696,6 +694,12 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt &S) {
 // promise local variable was not emitted yet.
 CoroId->setArgOperand(1, PromiseAddrVoidPtr);
 
+// Emit the alloca for ` __coro_gro`  *after* it's done for the promise.
+// This guarantees that while looking at deferred results from calls to
+// `get_return_object`, the lifetime of ` __coro_gro` is enclosed by the
+// `__promise` lifetime, and cleanup order is properly respected.
+GroManager.EmitGroAlloca();
+
 // Now we have the promise, initialize the GRO
 GroManager.EmitGroInit();
 
@@ -752,22 +756,22 @@ void CodeGenFunction::EmitCoroutineBody(const 
CoroutineBodyStmt &S) {
   // We don't need FinalBB. Emit it to make sure the block is deleted.
   EmitBlock(FinalBB, /*IsFinished=*/true);
 }
-  }
 
-  EmitBlock(RetBB);
-  // Emit coro.end before getReturnStmt (and parameter destructors), since
-  // resume and destroy parts of the coroutine should not include them.
-  llvm::Function *CoroEnd = CGM.getIntrinsic(llvm::Intrinsic::coro_end);
-  Builder.CreateCall(CoroEnd,
- {NullPtr, Builder.getFalse(),
-  llvm::ConstantTokenNone::get(CoroEnd->getContext())});
-
-  if (Stmt *Ret = S.getReturnStmt()) {
-// Since we already emitted the return value above, so we shouldn't
-// emit it again here.
-if (GroManager.DirectEmit)
-  cast(Ret)->setRetValue(nullptr);
-EmitStmt(Ret);
+EmitBlock(RetBB);
+// Emit coro.end before getReturnStmt (and parameter destructors), since
+// resume and destroy parts of the coroutine should not include them.
+llvm::Function *CoroEnd = CGM.getIntrinsic(llvm::Intrinsic::coro_end);
+Builder.CreateCall(CoroEnd,
+   {NullPtr, Builder.getFalse(),
+llvm::ConstantTokenNone::get(CoroEnd->getContext())});
+
+if (Stmt *Ret = S.getReturnStmt()) {
+  // Since we already emitted 

[clang] 07ef7b1 - [Builtins] Add __builtin_assume_separate_storage

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2023-03-23T16:35:30-07:00
New Revision: 07ef7b1ff21e8e3faaf8279b8ec6a7f0ac252fad

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

LOG: [Builtins] Add __builtin_assume_separate_storage

Plumbing from the language level to the assume intrinsics with
separate_storage operand bundles.

Patch by David Goldblatt (davidtgoldblatt)

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

Added: 
clang/test/CodeGen/builtin-assume-separate-storage.c
clang/test/Sema/builtin-assume-separate-storage.c

Modified: 
clang/docs/LanguageExtensions.rst
clang/docs/ReleaseNotes.rst
clang/include/clang/Basic/Builtins.def
clang/lib/CodeGen/CGBuiltin.cpp

Removed: 




diff  --git a/clang/docs/LanguageExtensions.rst 
b/clang/docs/LanguageExtensions.rst
index f8c83d4d6d162..a9bdc83c53e7a 100644
--- a/clang/docs/LanguageExtensions.rst
+++ b/clang/docs/LanguageExtensions.rst
@@ -2358,6 +2358,46 @@ evaluated, so any side effects of the expression will be 
discarded.
 
 Query for this feature with ``__has_builtin(__builtin_assume)``.
 
+.. _langext-__builtin_assume_separate_storage:
+
+``__builtin_assume_separate_storage``
+
+
+``__builtin_assume_separate_storage`` is used to provide the optimizer with the
+knowledge that its two arguments point to separately allocated objects.
+
+**Syntax**:
+
+.. code-block:: c++
+
+__builtin_assume_separate_storage(const volatile void *, const volatile 
void *)
+
+**Example of Use**:
+
+.. code-block:: c++
+
+  int foo(int *x, int *y) {
+  __builtin_assume_separate_storage(x, y);
+  *x = 0;
+  *y = 1;
+  // The optimizer may optimize this to return 0 without reloading from *x.
+  return *x;
+  }
+
+**Description**:
+
+The arguments to this function are assumed to point into separately allocated
+storage (either 
diff erent variable definitions or 
diff erent dynamic storage
+allocations). The optimizer may use this fact to aid in alias analysis. If the
+arguments point into the same storage, the behavior is undefined. Note that the
+definition of "storage" here refers to the outermost enclosing allocation of 
any
+particular object (so for example, it's never correct to call this function
+passing the addresses of fields in the same struct, elements of the same array,
+etc.).
+
+Query for this feature with 
``__has_builtin(__builtin_assume_separate_storage)``.
+
+
 ``__builtin_offsetof``
 --
 

diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index faac3b17b223f..29e3f516c06e5 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -124,6 +124,8 @@ Non-comprehensive list of changes in this release
 - Clang now supports ``__builtin_FILE_NAME()`` which returns the same
   information as the ``__FILE_NAME__`` macro (the presumed file name
   from the invocation point, with no path components included).
+- Clang now supports ``__builtin_assume_separate_storage`` that indicates that
+  its arguments point to objects in separate storage allocations.
 
 New Compiler Flags
 --

diff  --git a/clang/include/clang/Basic/Builtins.def 
b/clang/include/clang/Basic/Builtins.def
index 957375eccb84a..dea806099efbf 100644
--- a/clang/include/clang/Basic/Builtins.def
+++ b/clang/include/clang/Basic/Builtins.def
@@ -1591,6 +1591,7 @@ BUILTIN(__builtin_annotation, "v.", "tn")
 
 // Invariants
 BUILTIN(__builtin_assume, "vb", "nE")
+BUILTIN(__builtin_assume_separate_storage, "vvCD*vCD*", "nE")
 
 // Multiprecision Arithmetic Builtins.
 BUILTIN(__builtin_addcb, "UcUcCUcCUcCUc*", "n")

diff  --git a/clang/lib/CodeGen/CGBuiltin.cpp b/clang/lib/CodeGen/CGBuiltin.cpp
index 6381d68c161c6..b3aea13878c1c 100644
--- a/clang/lib/CodeGen/CGBuiltin.cpp
+++ b/clang/lib/CodeGen/CGBuiltin.cpp
@@ -2856,6 +2856,18 @@ RValue CodeGenFunction::EmitBuiltinExpr(const GlobalDecl 
GD, unsigned BuiltinID,
 Builder.CreateCall(FnAssume, ArgValue);
 return RValue::get(nullptr);
   }
+  case Builtin::BI__builtin_assume_separate_storage: {
+const Expr *Arg0 = E->getArg(0);
+const Expr *Arg1 = E->getArg(1);
+
+Value *Value0 = EmitScalarExpr(Arg0);
+Value *Value1 = EmitScalarExpr(Arg1);
+
+Value *Values[] = {Value0, Value1};
+OperandBundleDefT OBD("separate_storage", Values);
+Builder.CreateAssumption(ConstantInt::getTrue(getLLVMContext()), {OBD});
+return RValue::get(nullptr);
+  }
   case Builtin::BI__arithmetic_fence: {
 // Create the builtin call if FastMath is selected, and the target
 // supports the builtin, otherwise just return the argument.

diff  --git a/clang/test/CodeGen/builtin-assume-separate-storage.c 
b/clang/test/CodeGen/builtin-assume-separate-storage.c
new file mode 100644
index 000

[clang] fa0d4e1 - [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain conditions

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2023-03-21T21:42:31-07:00
New Revision: fa0d4e1f12a3f69dd0afb07c0928c867ab921537

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

LOG: [Coroutines] Implement fix for cwg2563 issue and enable RVO under certain 
conditions

- The cwg2563 issue is fixed by delaying GRO initialization only when the types
  mismatch between GRO and function return.
- When the types match directly initialize, which indirectly enables RVO to
  kick in, partially restores behavior introduced in
  https://reviews.llvm.org/D117087.
- Add entry to release notes.

Background:
https://github.com/llvm/llvm-project/issues/56532
https://cplusplus.github.io/CWG/issues/2563.html
https://github.com/cplusplus/papers/issues/1414

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

Added: 


Modified: 
clang/docs/ReleaseNotes.rst
clang/include/clang/AST/StmtCXX.h
clang/lib/CodeGen/CGCoroutine.cpp
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/TreeTransform.h
clang/test/CodeGenCoroutines/coro-gro.cpp
clang/test/SemaCXX/coroutine-no-move-ctor.cpp

Removed: 




diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
index c0162cf506cbc..005bf99a62457 100644
--- a/clang/docs/ReleaseNotes.rst
+++ b/clang/docs/ReleaseNotes.rst
@@ -219,6 +219,12 @@ Bug Fixes in This Version
 - Fix crash when using ``[[clang::always_inline]]`` or ``[[clang::noinline]]``
   statement attributes on a call to a template function in the body of a
   template function.
+- Fix coroutines issue where ``get_return_object()`` result was always eargerly
+  converted to the return type. Eager initialization (allowing RVO) is now only
+  perfomed when these types match, otherwise deferred initialization is used,
+  enabling short-circuiting coroutines use cases. This fixes
+  (`#56532 `_) in
+  antecipation of `CWG2563 
_`.
 
 Bug Fixes to Compiler Builtins
 ^^

diff  --git a/clang/include/clang/AST/StmtCXX.h 
b/clang/include/clang/AST/StmtCXX.h
index 05dfac2b50c3f..8ba667c02fc09 100644
--- a/clang/include/clang/AST/StmtCXX.h
+++ b/clang/include/clang/AST/StmtCXX.h
@@ -411,9 +411,8 @@ class CoroutineBodyStmt final
 return cast(getStoredStmts()[SubStmt::ReturnValue]);
   }
   Expr *getReturnValue() const {
-assert(getReturnStmt());
-auto *RS = cast(getReturnStmt());
-return RS->getRetValue();
+auto *RS = dyn_cast_or_null(getReturnStmt());
+return RS ? RS->getRetValue() : nullptr;
   }
   Stmt *getReturnStmt() const { return getStoredStmts()[SubStmt::ReturnStmt]; }
   Stmt *getReturnStmtOnAllocFailure() const {

diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 38167cc74a7f3..da3da5e600104 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -472,13 +472,41 @@ struct GetReturnObjectManager {
   CodeGenFunction &CGF;
   CGBuilderTy &Builder;
   const CoroutineBodyStmt &S;
+  // When true, performs RVO for the return object.
+  bool DirectEmit = false;
 
   Address GroActiveFlag;
   CodeGenFunction::AutoVarEmission GroEmission;
 
   GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S)
   : CGF(CGF), Builder(CGF.Builder), S(S), 
GroActiveFlag(Address::invalid()),
-GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {}
+GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {
+// The call to get_­return_­object is sequenced before the call to
+// initial_­suspend and is invoked at most once, but there are caveats
+// regarding on whether the prvalue result object may be initialized
+// directly/eager or delayed, depending on the types involved.
+//
+// More info at https://github.com/cplusplus/papers/issues/1414
+//
+// The general cases:
+// 1. Same type of get_return_object and coroutine return type (direct
+// emission):
+//  - Constructed in the return slot.
+// 2. Different types (delayed emission):
+//  - Constructed temporary object prior to initial suspend initialized 
with
+//  a call to get_return_object()
+//  - When coroutine needs to to return to the caller and needs to 
construct
+//  return value for the coroutine it is initialized with expiring value of
+//  the temporary obtained above.
+//
+// Direct emission for void returning coroutines or GROs.
+DirectEmit = [&]() {
+  auto *RVI = S.getReturnValueInit();
+  assert(RVI && "expected RVI");
+  auto GroType = RVI->getType();
+  return CGF.getContext().hasSameType(GroType, CGF.FnRetTy);
+}();
+  }
 
   // The gro variable has to outlive coro

[clang] 43f5085 - [Coroutines] Fix premature conversion of return object

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2023-03-21T21:42:25-07:00
New Revision: 43f5085fa80f716acf93870618b1d93ec85c1d01

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

LOG: [Coroutines] Fix premature conversion of return object

Fix https://github.com/llvm/llvm-project/issues/56532

Effectively, this reverts behavior introduced in 
https://reviews.llvm.org/D117087,
which did two things:

1. Change delayed to early conversion of return object.
2. Introduced RVO possibilities because of early conversion.

This patches fixes (1) and removes (2). I already worked on a follow up for (2)
in a separated patch. I believe it's important to split these two because if 
the RVO
causes any problems we can explore reverting (2) while maintaining (1).

Notes on some testcase changes:
- `pr59221.cpp` changed to `-O1` so we can check that the front-end honors
  the value checked for. Sounds like `-O3` without RVO is more likely
  to work with LLVM optimizations...
- Comment out delete members `coroutine-no-move-ctor.cpp` since behavior
  now requires copies again.

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

Added: 


Modified: 
clang/include/clang/AST/StmtCXX.h
clang/lib/AST/StmtCXX.cpp
clang/lib/CodeGen/CGCoroutine.cpp
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/TreeTransform.h
clang/test/CodeGenCoroutines/coro-gro.cpp
clang/test/CodeGenCoroutines/pr59221.cpp
clang/test/SemaCXX/coroutine-no-move-ctor.cpp
clang/test/SemaCXX/coroutines.cpp
clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp

Removed: 




diff  --git a/clang/include/clang/AST/StmtCXX.h 
b/clang/include/clang/AST/StmtCXX.h
index 2c71f86768963..05dfac2b50c3f 100644
--- a/clang/include/clang/AST/StmtCXX.h
+++ b/clang/include/clang/AST/StmtCXX.h
@@ -326,6 +326,7 @@ class CoroutineBodyStmt final
 OnFallthrough, ///< Handler for control flow falling off the body.
 Allocate,  ///< Coroutine frame memory allocation.
 Deallocate,///< Coroutine frame memory deallocation.
+ResultDecl,///< Declaration holding the result of get_return_object.
 ReturnValue,   ///< Return value for thunk function: p.get_return_object().
 ReturnStmt,///< Return statement for the thunk function.
 ReturnStmtOnAllocFailure, ///< Return statement if allocation failed.
@@ -352,6 +353,7 @@ class CoroutineBodyStmt final
 Stmt *OnFallthrough = nullptr;
 Expr *Allocate = nullptr;
 Expr *Deallocate = nullptr;
+Stmt *ResultDecl = nullptr;
 Expr *ReturnValue = nullptr;
 Stmt *ReturnStmt = nullptr;
 Stmt *ReturnStmtOnAllocFailure = nullptr;
@@ -404,6 +406,7 @@ class CoroutineBodyStmt final
   Expr *getDeallocate() const {
 return cast_or_null(getStoredStmts()[SubStmt::Deallocate]);
   }
+  Stmt *getResultDecl() const { return getStoredStmts()[SubStmt::ResultDecl]; }
   Expr *getReturnValueInit() const {
 return cast(getStoredStmts()[SubStmt::ReturnValue]);
   }

diff  --git a/clang/lib/AST/StmtCXX.cpp b/clang/lib/AST/StmtCXX.cpp
index 33b0421ad1016..a3ae5392f54bc 100644
--- a/clang/lib/AST/StmtCXX.cpp
+++ b/clang/lib/AST/StmtCXX.cpp
@@ -117,6 +117,7 @@ 
CoroutineBodyStmt::CoroutineBodyStmt(CoroutineBodyStmt::CtorArgs const &Args)
   SubStmts[CoroutineBodyStmt::OnFallthrough] = Args.OnFallthrough;
   SubStmts[CoroutineBodyStmt::Allocate] = Args.Allocate;
   SubStmts[CoroutineBodyStmt::Deallocate] = Args.Deallocate;
+  SubStmts[CoroutineBodyStmt::ResultDecl] = Args.ResultDecl;
   SubStmts[CoroutineBodyStmt::ReturnValue] = Args.ReturnValue;
   SubStmts[CoroutineBodyStmt::ReturnStmt] = Args.ReturnStmt;
   SubStmts[CoroutineBodyStmt::ReturnStmtOnAllocFailure] =

diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 9b233c1807cf1..38167cc74a7f3 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -467,6 +467,71 @@ struct CallCoroDelete final : public EHScopeStack::Cleanup 
{
 };
 }
 
+namespace {
+struct GetReturnObjectManager {
+  CodeGenFunction &CGF;
+  CGBuilderTy &Builder;
+  const CoroutineBodyStmt &S;
+
+  Address GroActiveFlag;
+  CodeGenFunction::AutoVarEmission GroEmission;
+
+  GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S)
+  : CGF(CGF), Builder(CGF.Builder), S(S), 
GroActiveFlag(Address::invalid()),
+GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {}
+
+  // The gro variable has to outlive coroutine frame and coroutine promise, 
but,
+  // it can only be initialized after coroutine promise was created, thus, we
+  // split its emission in two parts. EmitGroAlloca emits an alloca and sets up
+  // cleanups. Later when coroutine promise is available we initialize the gro
+  // and sets the flag that the cleanup is now act

[clang] 54225c4 - [Coroutines] Fix premature conversion of return object

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2023-03-09T14:18:26-08:00
New Revision: 54225c457a336b1609c6d064b2b606a9238a28b9

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

LOG: [Coroutines] Fix premature conversion of return object

Fix https://github.com/llvm/llvm-project/issues/56532

Effectively, this reverts behavior introduced in 
https://reviews.llvm.org/D117087,
which did two things:

1. Change delayed to early conversion of return object.
2. Introduced RVO possibilities because of early conversion.

This patches fixes (1) and removes (2). I already worked on a follow up for (2)
in a separated patch. I believe it's important to split these two because if 
the RVO
causes any problems we can explore reverting (2) while maintaining (1).

Notes on some testcase changes:
- `pr59221.cpp` changed to `-O1` so we can check that the front-end honors
  the value checked for. Sounds like `-O3` without RVO is more likely
  to work with LLVM optimizations...
- Comment out delete members `coroutine-no-move-ctor.cpp` since behavior
  now requires copies again.

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

Added: 


Modified: 
clang/include/clang/AST/StmtCXX.h
clang/lib/AST/StmtCXX.cpp
clang/lib/CodeGen/CGCoroutine.cpp
clang/lib/Sema/SemaCoroutine.cpp
clang/lib/Sema/TreeTransform.h
clang/test/CodeGenCoroutines/coro-gro.cpp
clang/test/CodeGenCoroutines/pr59221.cpp
clang/test/SemaCXX/coroutine-no-move-ctor.cpp
clang/test/SemaCXX/coroutines.cpp
clang/test/SemaCXX/warn-throw-out-noexcept-coro.cpp

Removed: 




diff  --git a/clang/include/clang/AST/StmtCXX.h 
b/clang/include/clang/AST/StmtCXX.h
index 2c71f86768963..05dfac2b50c3f 100644
--- a/clang/include/clang/AST/StmtCXX.h
+++ b/clang/include/clang/AST/StmtCXX.h
@@ -326,6 +326,7 @@ class CoroutineBodyStmt final
 OnFallthrough, ///< Handler for control flow falling off the body.
 Allocate,  ///< Coroutine frame memory allocation.
 Deallocate,///< Coroutine frame memory deallocation.
+ResultDecl,///< Declaration holding the result of get_return_object.
 ReturnValue,   ///< Return value for thunk function: p.get_return_object().
 ReturnStmt,///< Return statement for the thunk function.
 ReturnStmtOnAllocFailure, ///< Return statement if allocation failed.
@@ -352,6 +353,7 @@ class CoroutineBodyStmt final
 Stmt *OnFallthrough = nullptr;
 Expr *Allocate = nullptr;
 Expr *Deallocate = nullptr;
+Stmt *ResultDecl = nullptr;
 Expr *ReturnValue = nullptr;
 Stmt *ReturnStmt = nullptr;
 Stmt *ReturnStmtOnAllocFailure = nullptr;
@@ -404,6 +406,7 @@ class CoroutineBodyStmt final
   Expr *getDeallocate() const {
 return cast_or_null(getStoredStmts()[SubStmt::Deallocate]);
   }
+  Stmt *getResultDecl() const { return getStoredStmts()[SubStmt::ResultDecl]; }
   Expr *getReturnValueInit() const {
 return cast(getStoredStmts()[SubStmt::ReturnValue]);
   }

diff  --git a/clang/lib/AST/StmtCXX.cpp b/clang/lib/AST/StmtCXX.cpp
index 33b0421ad1016..a3ae5392f54bc 100644
--- a/clang/lib/AST/StmtCXX.cpp
+++ b/clang/lib/AST/StmtCXX.cpp
@@ -117,6 +117,7 @@ 
CoroutineBodyStmt::CoroutineBodyStmt(CoroutineBodyStmt::CtorArgs const &Args)
   SubStmts[CoroutineBodyStmt::OnFallthrough] = Args.OnFallthrough;
   SubStmts[CoroutineBodyStmt::Allocate] = Args.Allocate;
   SubStmts[CoroutineBodyStmt::Deallocate] = Args.Deallocate;
+  SubStmts[CoroutineBodyStmt::ResultDecl] = Args.ResultDecl;
   SubStmts[CoroutineBodyStmt::ReturnValue] = Args.ReturnValue;
   SubStmts[CoroutineBodyStmt::ReturnStmt] = Args.ReturnStmt;
   SubStmts[CoroutineBodyStmt::ReturnStmtOnAllocFailure] =

diff  --git a/clang/lib/CodeGen/CGCoroutine.cpp 
b/clang/lib/CodeGen/CGCoroutine.cpp
index 9b233c1807cf1..38167cc74a7f3 100644
--- a/clang/lib/CodeGen/CGCoroutine.cpp
+++ b/clang/lib/CodeGen/CGCoroutine.cpp
@@ -467,6 +467,71 @@ struct CallCoroDelete final : public EHScopeStack::Cleanup 
{
 };
 }
 
+namespace {
+struct GetReturnObjectManager {
+  CodeGenFunction &CGF;
+  CGBuilderTy &Builder;
+  const CoroutineBodyStmt &S;
+
+  Address GroActiveFlag;
+  CodeGenFunction::AutoVarEmission GroEmission;
+
+  GetReturnObjectManager(CodeGenFunction &CGF, const CoroutineBodyStmt &S)
+  : CGF(CGF), Builder(CGF.Builder), S(S), 
GroActiveFlag(Address::invalid()),
+GroEmission(CodeGenFunction::AutoVarEmission::invalid()) {}
+
+  // The gro variable has to outlive coroutine frame and coroutine promise, 
but,
+  // it can only be initialized after coroutine promise was created, thus, we
+  // split its emission in two parts. EmitGroAlloca emits an alloca and sets up
+  // cleanups. Later when coroutine promise is available we initialize the gro
+  // and sets the flag that the cleanup is now act

Re: [clang] 6ed67cc - [Coroutines] Remove -fcoroutines-ts

[Bruno Cardoso Lopes via cfe-commits]

Makes total sense, thank you for the clarification.

On Mon, Feb 27, 2023 at 6:24 PM chuanqi.xcq  wrote:
>
> Hi Bruno,
>
> We talked about removing `-fcoroutines-ts` in 
> https://github.com/llvm/llvm-project/issues/59110
> and https://reviews.llvm.org/D108697. I raised the example you used here. And 
> the conclusion
> is that this is the clang's policy for `-f*-ts` options. The same thing 
> happens for -fconcepts-ts and
> -fmodules-ts. From my understanding, this is because we (clang) want less 
> dialects and we want to
> focus on the form `-std=*`.
>
> > If my understanding now is right, `-std=c++17 -fcoroutines` should 
> work, right?
>
> `clang -std=c++17 -fcoroutines` shouldn't work. Since it is the same with 
> `-fcoroutines-ts`.
> But if you want or you can't update to `-std=c++20` quickly, you can use
> `clang -std=c++17 -Xclang -fcoroutines` for the period of transition. But the 
> `Xclang` options are
> for developers instead of the users. So we (the users) should update
> to `-std=c++20` or higher to use coroutines finally.
>
> Thanks,
> Chuanqi
>
>
> --
> From:Bruno Cardoso Lopes 
> Send Time:2023年2月28日(星期二) 06:51
> To:Chuanqi Xu ; Chuanqi Xu 
> Cc:cfe-commits 
> Subject:Re: [clang] 6ed67cc - [Coroutines] Remove -fcoroutines-ts
>
> > I understand that the name "coroutines-ts" isn't meaningful these
> > days, but it also sounds like this commit does more than remove the
> > flag, it caps useful functionality. How are users supposed to use
> > c++17 with coroutines now? It's very common in our codebase and we
> > have users relying on it.
>
> Looks like I misread your patch, sorry! If my understanding now is
> right, `-std=c++17 -fcoroutines` should work, right? We should
> probably add an extra test in `clang/test/Driver/coroutines.cpp` that
> just test that (we currently just test when it's not).
>
> Cheers,
>
> --
> Bruno Cardoso Lopes
> http://www.brunocardoso.cc
>
>


-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang] 6ed67cc - [Coroutines] Remove -fcoroutines-ts

[Bruno Cardoso Lopes via cfe-commits]

> I understand that the name "coroutines-ts" isn't meaningful these
> days, but it also sounds like this commit does more than remove the
> flag, it caps useful functionality. How are users supposed to use
> c++17 with coroutines now? It's very common in our codebase and we
> have users relying on it.

Looks like I misread your patch, sorry! If my understanding now is
right, `-std=c++17 -fcoroutines` should work, right? We should
probably add an extra test in `clang/test/Driver/coroutines.cpp` that
just test that (we currently just test when it's not).

Cheers,

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

Re: [clang] 6ed67cc - [Coroutines] Remove -fcoroutines-ts

[Bruno Cardoso Lopes via cfe-commits]

Hi Chuanqi,

I know the warning mentions it to be removed in clang-17, but a heads
up "landing in a week" or so would have been great :)

I understand that the name "coroutines-ts" isn't meaningful these
days, but it also sounds like this commit does more than remove the
flag, it caps useful functionality. How are users supposed to use
c++17 with coroutines now? It's very common in our codebase and we
have users relying on it.

Thanks,

On Wed, Feb 22, 2023 at 10:44 PM Chuanqi Xu via cfe-commits
 wrote:
>
>
> Author: Chuanqi Xu
> Date: 2023-02-23T14:40:58+08:00
> New Revision: 6ed67ccba7e4699e9e42302f2f9b7653444258ba
>
> URL: 
> https://github.com/llvm/llvm-project/commit/6ed67ccba7e4699e9e42302f2f9b7653444258ba
> DIFF: 
> https://github.com/llvm/llvm-project/commit/6ed67ccba7e4699e9e42302f2f9b7653444258ba.diff
>
> LOG: [Coroutines] Remove -fcoroutines-ts
>
> Since we decided to remove the support for `-fcoroutines-ts` in
> clang/llvm17 and the clang16/llvm16 is branched. So we're going to
> remove the `-fcoroutines-ts` option.
>
> Added:
> clang/test/Parser/cxx20-coroutines.cpp
>
> Modified:
> clang/docs/ReleaseNotes.rst
> clang/include/clang/AST/Stmt.h
> clang/include/clang/Basic/DiagnosticDriverKinds.td
> clang/include/clang/Basic/DiagnosticGroups.td
> clang/include/clang/Basic/StmtNodes.td
> clang/include/clang/Basic/TokenKinds.def
> clang/include/clang/Driver/Options.td
> clang/include/clang/Sema/Sema.h
> clang/lib/AST/StmtPrinter.cpp
> clang/lib/Driver/ToolChains/Clang.cpp
> clang/lib/Sema/TreeTransform.h
> clang/test/AST/Inputs/std-coroutine-exp-namespace.h
> clang/test/AST/Inputs/std-coroutine.h
> clang/test/CodeGen/no-skipped-passes-O0-opt-bisect.c
> clang/test/CodeGenCoroutines/coro-builtins-err.c
> clang/test/CodeGenCoroutines/coro-builtins.c
> clang/test/CodeGenCoroutines/coro-gro2.cpp
> clang/test/CodeGenCoroutines/coro-params.cpp
> clang/test/CoverageMapping/coroutine.cpp
> clang/test/Driver/coroutines.c
> clang/test/Driver/coroutines.cpp
> clang/test/Index/coroutines.cpp
> clang/test/Lexer/coroutines.cpp
> clang/test/Lexer/cxx-features.cpp
> clang/test/Modules/requires-coroutines.mm
> clang/test/PCH/coroutines.cpp
> clang/test/SemaCXX/coroutine-builtins.cpp
> clang/test/SemaCXX/thread-safety-coro.cpp
>
> Removed:
> clang/test/Parser/cxx1z-coroutines.cpp
> clang/test/SemaCXX/Inputs/std-coroutine-exp-namespace.h
>
>
> 
> diff  --git a/clang/docs/ReleaseNotes.rst b/clang/docs/ReleaseNotes.rst
> index 9c89bbc0d1786..be3ea5ff63cde 100644
> --- a/clang/docs/ReleaseNotes.rst
> +++ b/clang/docs/ReleaseNotes.rst
> @@ -115,6 +115,8 @@ Removed Compiler Flags
>  -
>  - The deprecated flag `-fmodules-ts` is removed. Please use ``-std=c++20``
>or higher to use standard C++ modules instead.
> +- The deprecated flag `-fcoroutines-ts` is removed. Please use ``-std=c++20``
> +  or higher to use standard C++ coroutines instead.
>
>  Attribute Changes in Clang
>  --
>
> diff  --git a/clang/include/clang/AST/Stmt.h b/clang/include/clang/AST/Stmt.h
> index b70cf3aec5d6c..45010f19b69b2 100644
> --- a/clang/include/clang/AST/Stmt.h
> +++ b/clang/include/clang/AST/Stmt.h
> @@ -978,7 +978,7 @@ class alignas(void *) Stmt {
>  SourceLocation RequiresKWLoc;
>};
>
> -  //===--- C++ Coroutines TS bitfields classes ---===//
> +  //===--- C++ Coroutines bitfields classes ---===//
>
>class CoawaitExprBitfields {
>  friend class CoawaitExpr;
> @@ -1082,7 +1082,7 @@ class alignas(void *) Stmt {
>  LambdaExprBitfields LambdaExprBits;
>  RequiresExprBitfields RequiresExprBits;
>
> -// C++ Coroutines TS expressions
> +// C++ Coroutines expressions
>  CoawaitExprBitfields CoawaitBits;
>
>  // Obj-C Expressions
>
> diff  --git a/clang/include/clang/Basic/DiagnosticDriverKinds.td 
> b/clang/include/clang/Basic/DiagnosticDriverKinds.td
> index 77fb1e00585a0..4c922650e100f 100644
> --- a/clang/include/clang/Basic/DiagnosticDriverKinds.td
> +++ b/clang/include/clang/Basic/DiagnosticDriverKinds.td
> @@ -637,11 +637,6 @@ def warn_drv_libstdcxx_not_found : Warning<
>"command line to use the libc++ standard library instead">,
>InGroup>;
>
> -def warn_deperecated_fcoroutines_ts_flag : Warning<
> -  "the '-fcoroutines-ts' flag is deprecated and it will be removed in Clang 
> 17; "
> -  "use '-std=c++20' or higher to use standard C++ coroutines instead">,
> -  InGroup;
> -
>  def err_drv_cannot_mix_options : Error<"cannot specify '%1' along with 
> '%0'">;
>
>  def err_drv_invalid_object_mode : Error<
>
> diff  --git a/clang/include/clang/Basic/DiagnosticGroups.td 
> b/clang/include/clang/Basic/DiagnosticGroups.td
> index 17fdcffa2d427..d56aba34ac0a3 100644
> --- a/clang/include/clang/Basic/DiagnosticGroups.td
> +++ b/clang/include/clang/Basic/

[clang] 0b8daee - [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2023-02-03T15:37:29-08:00
New Revision: 0b8daee028a87ab8a6f8fe54d2eb2d5b5c2babd4

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

LOG: [Clang][SemaCXX][Coroutines] Fix misleading diagnostics with -Wunsequenced

D115187 exposed CoroutineSuspendExpr's operand, which makes some nodes to show
up twice during the traversal, confusing the check for unsequenced operations.
Skip the operand since it's already handled as part of the common expression
and get rid of the misleading warnings.

https://github.com/llvm/llvm-project/issues/56768

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

Added: 
clang/test/SemaCXX/warn-unsequenced-coro.cpp

Modified: 
clang/lib/Sema/SemaChecking.cpp
clang/test/SemaCXX/Inputs/std-coroutine.h
clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp
clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp

Removed: 




diff  --git a/clang/lib/Sema/SemaChecking.cpp b/clang/lib/Sema/SemaChecking.cpp
index efba0a8871c6c..d7830432099ad 100644
--- a/clang/lib/Sema/SemaChecking.cpp
+++ b/clang/lib/Sema/SemaChecking.cpp
@@ -15241,6 +15241,23 @@ class SequenceChecker : public 
ConstEvaluatedExprVisitor {
 Base::VisitStmt(E);
   }
 
+  void VisitCoroutineSuspendExpr(const CoroutineSuspendExpr *CSE) {
+for (auto *Sub : CSE->children()) {
+  const Expr *ChildExpr = dyn_cast_or_null(Sub);
+  if (!ChildExpr)
+continue;
+
+  if (ChildExpr == CSE->getOperand())
+// Do not recurse over a CoroutineSuspendExpr's operand.
+// The operand is also a subexpression of getCommonExpr(), and
+// recursing into it directly could confuse object management
+// for the sake of sequence tracking.
+continue;
+
+  Visit(Sub);
+}
+  }
+
   void VisitCastExpr(const CastExpr *E) {
 Object O = Object();
 if (E->getCastKind() == CK_LValueToRValue)

diff  --git a/clang/test/SemaCXX/Inputs/std-coroutine.h 
b/clang/test/SemaCXX/Inputs/std-coroutine.h
index 832ae7746f68b..0bc459d48ccc6 100644
--- a/clang/test/SemaCXX/Inputs/std-coroutine.h
+++ b/clang/test/SemaCXX/Inputs/std-coroutine.h
@@ -4,12 +4,23 @@
 
 namespace std {
 
+template struct remove_reference   { typedef T type; };
+template struct remove_reference  { typedef T type; };
+template struct remove_reference { typedef T type; };
+
+template
+typename remove_reference::type &&move(T &&t) noexcept;
+
+struct input_iterator_tag {};
+struct forward_iterator_tag : public input_iterator_tag {};
+
 template 
 struct coroutine_traits { using promise_type = typename Ret::promise_type; };
 
 template 
 struct coroutine_handle {
   static coroutine_handle from_address(void *) noexcept;
+  static coroutine_handle from_promise(Promise &promise);
   constexpr void* address() const noexcept;
 };
 template <>

diff  --git a/clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp 
b/clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp
index ae67f3aa819a8..97b4e687ed870 100644
--- a/clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp
+++ b/clang/test/SemaCXX/coroutine-mixed-exp-namespace.cpp
@@ -26,5 +26,5 @@ void test() {
   co_return; // expected-error {{mixed use of std and std::experimental 
namespaces for coroutine components}}
   // expected-warning@-1{{support for 'std::experimental::coroutine_traits' 
will be removed}}
   // expected-note@Inputs/std-coroutine-exp-namespace.h:8 {{'coroutine_traits' 
declared here}}
-  // expected-note@Inputs/std-coroutine.h:8 {{'coroutine_traits' declared 
here}}
+  // expected-note@Inputs/std-coroutine.h:18 {{'coroutine_traits' declared 
here}}
 }

diff  --git a/clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp 
b/clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
index a4f228a227838..7fec83fdd7c28 100644
--- a/clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
+++ b/clang/test/SemaCXX/coroutine-mixed2-exp-namespace.cpp
@@ -27,5 +27,5 @@ void test() {
   co_return; // expected-error {{mixed use of std and std::experimental 
namespaces for coroutine components}}
   // expected-warning@-1{{support for 'std::experimental::coroutine_traits' 
will be removed}}
   // expected-note@Inputs/std-coroutine-exp-namespace.h:8 {{'coroutine_traits' 
declared here}}
-  // expected-note@Inputs/std-coroutine.h:8 {{'coroutine_traits' declared 
here}}
+  // expected-note@Inputs/std-coroutine.h:18 {{'coroutine_traits' declared 
here}}
 }

diff  --git a/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp 
b/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp
index 49e79b868b0f5..b09482d5b426b 100644
--- a/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp
+++ b/clang/test/SemaCXX/coroutine-mixed4-exp-namespace.cpp
@@ -28,5 +28,

[clang] f4a13c9 - [Clang][ScanDeps] Change multiple-commands.c test to use -fmodules-cache-path on implicit builds

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2022-09-09T16:20:04-07:00
New Revision: f4a13c9c0a049d40e0365477c58c2a5369eda6dc

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

LOG: [Clang][ScanDeps] Change multiple-commands.c test to use 
-fmodules-cache-path on implicit builds

The module cache escapes the test output dirs in this test. Since its default 
location maybe
composed of system and user related path this can cause problems in some 
builders (e.g. not
accessible paths inherited in a chroot environment).

Clean the test a bit by passing `-fmodules-cache-path` inside the test output 
dirs.

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

Added: 


Modified: 
clang/test/ClangScanDeps/multiple-commands.c

Removed: 




diff  --git a/clang/test/ClangScanDeps/multiple-commands.c 
b/clang/test/ClangScanDeps/multiple-commands.c
index 6640d40e6e7c6..bb169ea10995a 100644
--- a/clang/test/ClangScanDeps/multiple-commands.c
+++ b/clang/test/ClangScanDeps/multiple-commands.c
@@ -150,7 +150,7 @@
   },
   {
 "directory": "DIR"
-"command": "clang_tool -target x86_64-apple-darwin -c 
DIR/tu_save_temps_module.c -save-temps=obj -o DIR/tu_save_temps_module.o 
-fmodules -fimplicit-modules -fimplicit-module-maps"
+"command": "clang_tool -target x86_64-apple-darwin -c 
DIR/tu_save_temps_module.c -save-temps=obj -o DIR/tu_save_temps_module.o 
-fmodules -fimplicit-modules -fimplicit-module-maps 
-fmodules-cache-path=DIR/cache"
 "file": "DIR/tu_save_temps_module.c"
   }
 ]



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

[clang] e6a76a4 - [Clang][CoverageMapping] Fix compile time explosions by adjusting only appropriated skipped ranges

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2022-06-08T23:13:39-07:00
New Revision: e6a76a49356efd11f5f36690181f0f60cecb2e01

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

LOG: [Clang][CoverageMapping] Fix compile time explosions by adjusting only 
appropriated skipped ranges

D83592 added comments to be part of skipped regions, and as part of that, it
also shrinks a skipped range if it spans a line that contains a non-comment
token. This is done by `adjustSkippedRange`.

The `adjustSkippedRange` currently runs on skipped regions that are not
comments, causing a 5min regression while building a big C++ files without any
comments.

Fix the compile time introduced in D83592 by tagging SkippedRange with kind
information and use that to decide what needs additional processing.

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

Added: 


Modified: 
clang/lib/CodeGen/CoverageMappingGen.cpp
clang/lib/CodeGen/CoverageMappingGen.h

Removed: 




diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 8952125eeefcb..d1cbe109a6cf8 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -60,26 +60,27 @@ 
CoverageMappingModuleGen::setUpCoverageCallbacks(Preprocessor &PP) {
   return CoverageInfo;
 }
 
-void CoverageSourceInfo::AddSkippedRange(SourceRange Range) {
+void CoverageSourceInfo::AddSkippedRange(SourceRange Range,
+ SkippedRange::Kind RangeKind) {
   if (EmptyLineCommentCoverage && !SkippedRanges.empty() &&
   PrevTokLoc == SkippedRanges.back().PrevTokLoc &&
   SourceMgr.isWrittenInSameFile(SkippedRanges.back().Range.getEnd(),
 Range.getBegin()))
 SkippedRanges.back().Range.setEnd(Range.getEnd());
   else
-SkippedRanges.push_back({Range, PrevTokLoc});
+SkippedRanges.push_back({Range, RangeKind, PrevTokLoc});
 }
 
 void CoverageSourceInfo::SourceRangeSkipped(SourceRange Range, SourceLocation) 
{
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::PPIfElse);
 }
 
 void CoverageSourceInfo::HandleEmptyline(SourceRange Range) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::EmptyLine);
 }
 
 bool CoverageSourceInfo::HandleComment(Preprocessor &PP, SourceRange Range) {
-  AddSkippedRange(Range);
+  AddSkippedRange(Range, SkippedRange::Comment);
   return false;
 }
 
@@ -335,6 +336,8 @@ class CoverageMappingBuilder {
   /// This shrinks the skipped range if it spans a line that contains a
   /// non-comment token. If shrinking the skipped range would make it empty,
   /// this returns None.
+  /// Note this function can potentially be expensive because
+  /// getSpellingLineNumber uses getLineNumber, which is expensive.
   Optional adjustSkippedRange(SourceManager &SM,
   SourceLocation LocStart,
   SourceLocation LocEnd,
@@ -382,8 +385,13 @@ class CoverageMappingBuilder {
   auto CovFileID = getCoverageFileID(LocStart);
   if (!CovFileID)
 continue;
-  Optional SR =
-  adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc, I.NextTokLoc);
+  Optional SR;
+  if (I.isComment())
+SR = adjustSkippedRange(SM, LocStart, LocEnd, I.PrevTokLoc,
+I.NextTokLoc);
+  else if (I.isPPIfElse() || I.isEmptyLine())
+SR = {SM, LocStart, LocEnd};
+
   if (!SR.hasValue())
 continue;
   auto Region = CounterMappingRegion::makeSkipped(

diff  --git a/clang/lib/CodeGen/CoverageMappingGen.h 
b/clang/lib/CodeGen/CoverageMappingGen.h
index ae4f435d4ff34..f5282601b6406 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.h
+++ b/clang/lib/CodeGen/CoverageMappingGen.h
@@ -31,15 +31,29 @@ class Decl;
 class Stmt;
 
 struct SkippedRange {
+  enum Kind {
+PPIfElse, // Preprocessor #if/#else ...
+EmptyLine,
+Comment,
+  };
+
   SourceRange Range;
   // The location of token before the skipped source range.
   SourceLocation PrevTokLoc;
   // The location of token after the skipped source range.
   SourceLocation NextTokLoc;
+  // The nature of this skipped range
+  Kind RangeKind;
+
+  bool isComment() { return RangeKind == Comment; }
+  bool isEmptyLine() { return RangeKind == EmptyLine; }
+  bool isPPIfElse() { return RangeKind == PPIfElse; }
 
-  SkippedRange(SourceRange Range, SourceLocation PrevTokLoc = SourceLocation(),
+  SkippedRange(SourceRange Range, Kind K,
+   SourceLocation PrevTokLoc = SourceLocation(),
SourceLocation NextTokLoc = SourceLocation())
-  : Range(Range), PrevTokLoc(PrevTokLoc), NextTokLoc(NextTokLoc) {}
+  : Range(Range), PrevTokLoc(P

[clang] ce54b22 - [Clang][CoverageMapping] Fix switch counter codegen compile time explosion

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2022-05-26T11:05:15-07:00
New Revision: ce54b22657f01d1c40de4941ceb6e7119848aecf

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

LOG: [Clang][CoverageMapping] Fix switch counter codegen compile time explosion

C++ generated code with huge amount of switch cases chokes badly while emitting
coverage mapping, in our specific testcase (~72k cases), it won't stop after 
hours.
After this change, the frontend job now finishes in 4.5s and shrinks down 
`@__covrec_`
by 288k when compared to disabling simplification altogether.

There's probably no good way to create a testcase for this, but it's easy to
reproduce, just add thousands of cases in the below switch, and build with
`-fprofile-instr-generate -fcoverage-mapping`.

```
enum type : int {
 FEATURE_INVALID = 0,
 FEATURE_A = 1,
 ...
};

const char *to_string(type e) {
  switch (e) {
  case type::FEATURE_INVALID: return "FEATURE_INVALID";
  case type::FEATURE_A: return "FEATURE_A";}
  ...
  }

```

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

Added: 


Modified: 
clang/lib/CodeGen/CoverageMappingGen.cpp
llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
llvm/lib/ProfileData/Coverage/CoverageMapping.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CoverageMappingGen.cpp 
b/clang/lib/CodeGen/CoverageMappingGen.cpp
index 9b81c8a670f59..8952125eeefcb 100644
--- a/clang/lib/CodeGen/CoverageMappingGen.cpp
+++ b/clang/lib/CodeGen/CoverageMappingGen.cpp
@@ -550,17 +550,18 @@ struct CounterCoverageMappingBuilder
   Counter GapRegionCounter;
 
   /// Return a counter for the subtraction of \c RHS from \c LHS
-  Counter subtractCounters(Counter LHS, Counter RHS) {
-return Builder.subtract(LHS, RHS);
+  Counter subtractCounters(Counter LHS, Counter RHS, bool Simplify = true) {
+return Builder.subtract(LHS, RHS, Simplify);
   }
 
   /// Return a counter for the sum of \c LHS and \c RHS.
-  Counter addCounters(Counter LHS, Counter RHS) {
-return Builder.add(LHS, RHS);
+  Counter addCounters(Counter LHS, Counter RHS, bool Simplify = true) {
+return Builder.add(LHS, RHS, Simplify);
   }
 
-  Counter addCounters(Counter C1, Counter C2, Counter C3) {
-return addCounters(addCounters(C1, C2), C3);
+  Counter addCounters(Counter C1, Counter C2, Counter C3,
+  bool Simplify = true) {
+return addCounters(addCounters(C1, C2, Simplify), C3, Simplify);
   }
 
   /// Return the region counter for the given statement.
@@ -1317,11 +1318,16 @@ struct CounterCoverageMappingBuilder
 const SwitchCase *Case = S->getSwitchCaseList();
 for (; Case; Case = Case->getNextSwitchCase()) {
   HasDefaultCase = HasDefaultCase || isa(Case);
-  CaseCountSum = addCounters(CaseCountSum, getRegionCounter(Case));
+  CaseCountSum =
+  addCounters(CaseCountSum, getRegionCounter(Case), 
/*Simplify=*/false);
   createSwitchCaseRegion(
   Case, getRegionCounter(Case),
   subtractCounters(ParentCount, getRegionCounter(Case)));
 }
+// Simplify is skipped while building the counters above: it can get really
+// slow on top of switches with thousands of cases. Instead, trigger
+// simplification by adding zero to the last counter.
+CaseCountSum = addCounters(CaseCountSum, Counter::getZero());
 
 // If no explicit default case exists, create a branch region to represent
 // the hidden branch, which will be added later by the CodeGen. This region

diff  --git a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h 
b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
index e1f45019b1a92..e35751512245d 100644
--- a/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
+++ b/llvm/include/llvm/ProfileData/Coverage/CoverageMapping.h
@@ -195,11 +195,11 @@ class CounterExpressionBuilder {
   ArrayRef getExpressions() const { return Expressions; }
 
   /// Return a counter that represents the expression that adds LHS and RHS.
-  Counter add(Counter LHS, Counter RHS);
+  Counter add(Counter LHS, Counter RHS, bool Simplify = true);
 
   /// Return a counter that represents the expression that subtracts RHS from
   /// LHS.
-  Counter subtract(Counter LHS, Counter RHS);
+  Counter subtract(Counter LHS, Counter RHS, bool Simplify = true);
 };
 
 using LineColPair = std::pair;

diff  --git a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp 
b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
index 94c2bee3590cb..f9e58fd6afa50 100644
--- a/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
+++ b/llvm/lib/ProfileData/Coverage/CoverageMapping.cpp
@@ -123,13 +123,15 @@ Counter CounterExpressionBuilder::simplify(Counter 
ExpressionTree) {
   return C;
 }
 
-Counter CounterExpressionBuilder::add(Counter LHS, Counter 

[clang] c9aaf34 - [SemaCXX] Handle lack of TypeSourceInfo on special member functions in templated lambdas

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2021-06-22T17:26:05-07:00
New Revision: c9aaf34b8db884faa3d3ced4d2fb88fd45697408

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

LOG: [SemaCXX] Handle lack of TypeSourceInfo on special member functions in 
templated lambdas

During template instantiation involving templated lambdas, clang
could hit an assertion in `TemplateDeclInstantiator::SubstFunctionType`
since the functions are not associated with any `TypeSourceInfo`:

`assert(OldTInfo && "substituting function without type source info");`

This path is triggered when using templated lambdas like the one added as
a test to this patch. To fix this:

- Create `TypeSourceInfo`s for special members and make sure the template
instantiator can get through all patterns.
- Introduce a `SpecialMemberTypeInfoRebuilder` tree transform to rewrite
such member function arguments. Without this, we get errors like:

`error: only special member functions and comparison operators may be defaulted`

since `getDefaultedFunctionKind` can't properly recognize these functions
as special members as part of `SetDeclDefaulted`.

Fixes PR45828 and PR44848

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

Added: 
clang/test/SemaCXX/lambdas-implicit-explicit-template.cpp

Modified: 
clang/lib/Sema/SemaDeclCXX.cpp
clang/lib/Sema/SemaTemplateInstantiate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/lib/Sema/TreeTransform.h

Removed: 




diff  --git a/clang/lib/Sema/SemaDeclCXX.cpp b/clang/lib/Sema/SemaDeclCXX.cpp
index 6d2710f1774ce..a68a06eb4d270 100644
--- a/clang/lib/Sema/SemaDeclCXX.cpp
+++ b/clang/lib/Sema/SemaDeclCXX.cpp
@@ -13202,6 +13202,16 @@ void 
Sema::setupImplicitSpecialMemberType(CXXMethodDecl *SpecialMem,
 
   auto QT = Context.getFunctionType(ResultTy, Args, EPI);
   SpecialMem->setType(QT);
+
+  // During template instantiation of implicit special member functions we need
+  // a reliable TypeSourceInfo for the function prototype in order to allow
+  // functions to be substituted.
+  if (inTemplateInstantiation() &&
+  cast(SpecialMem->getParent())->isLambda()) {
+TypeSourceInfo *TSI =
+Context.getTrivialTypeSourceInfo(SpecialMem->getType());
+SpecialMem->setTypeSourceInfo(TSI);
+  }
 }
 
 CXXConstructorDecl *Sema::DeclareImplicitDefaultConstructor(
@@ -14880,12 +14890,18 @@ CXXConstructorDecl 
*Sema::DeclareImplicitCopyConstructor(
 
   setupImplicitSpecialMemberType(CopyConstructor, Context.VoidTy, ArgType);
 
+  // During template instantiation of special member functions we need a
+  // reliable TypeSourceInfo for the parameter types in order to allow 
functions
+  // to be substituted.
+  TypeSourceInfo *TSI = nullptr;
+  if (inTemplateInstantiation() && ClassDecl->isLambda())
+TSI = Context.getTrivialTypeSourceInfo(ArgType);
+
   // Add the parameter to the constructor.
-  ParmVarDecl *FromParam = ParmVarDecl::Create(Context, CopyConstructor,
-   ClassLoc, ClassLoc,
-   /*IdentifierInfo=*/nullptr,
-   ArgType, /*TInfo=*/nullptr,
-   SC_None, nullptr);
+  ParmVarDecl *FromParam =
+  ParmVarDecl::Create(Context, CopyConstructor, ClassLoc, ClassLoc,
+  /*IdentifierInfo=*/nullptr, ArgType,
+  /*TInfo=*/TSI, SC_None, nullptr);
   CopyConstructor->setParams(FromParam);
 
   CopyConstructor->setTrivial(

diff  --git a/clang/lib/Sema/SemaTemplateInstantiate.cpp 
b/clang/lib/Sema/SemaTemplateInstantiate.cpp
index f9d40a2ed4b7b..f18f77d3442a1 100644
--- a/clang/lib/Sema/SemaTemplateInstantiate.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiate.cpp
@@ -2817,7 +2817,8 @@ Sema::InstantiateClass(SourceLocation 
PointOfInstantiation,
 
   if (!Instantiation->isInvalidDecl()) {
 // Perform any dependent diagnostics from the pattern.
-PerformDependentDiagnostics(Pattern, TemplateArgs);
+if (Pattern->isDependentContext())
+  PerformDependentDiagnostics(Pattern, TemplateArgs);
 
 // Instantiate any out-of-line class template partial
 // specializations now.

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 0fa42c5494213..be4c519307898 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -9,6 +9,7 @@
 //
 //===--===/
 
+#include "TreeTransform.h"
 #include "clang/AST/ASTConsumer.h"
 #include "clang/AST/ASTContext.h"
 #include "clang/AST/ASTMutationListener.h"
@@ -1825,9 +1826,16 @@ Decl 
*TemplateDeclInstantiator::Visi

[clang] 819e0d1 - [CGAtomic] Lift strong requirement for remaining compare_exchange combinations

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2021-05-06T21:05:20-07:00
New Revision: 819e0d105e84c6081cfcfa0e38fd257b6124553a

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

LOG: [CGAtomic] Lift strong requirement for remaining compare_exchange 
combinations

Follow up on 431e3138a and complete the other possible combinations.

Besides enforcing the new behavior, it also mitigates TSAN false positives when
combining orders that used to be stronger.

Added: 


Modified: 
clang/lib/CodeGen/CGAtomic.cpp
clang/test/CodeGen/atomic-ops.c
clang/test/CodeGenOpenCL/atomic-ops.cl
llvm/lib/AsmParser/LLParser.cpp
llvm/lib/IR/Instructions.cpp
llvm/lib/IR/Verifier.cpp

Removed: 




diff  --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index ef016cf24f134..cc85375f08ddf 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -451,47 +451,38 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction 
&CGF, AtomicExpr *E,
   }
 
   // Create all the relevant BB's
-  llvm::BasicBlock *MonotonicBB = nullptr, *AcquireBB = nullptr,
-   *SeqCstBB = nullptr;
-  MonotonicBB = CGF.createBasicBlock("monotonic_fail", CGF.CurFn);
-  if (SuccessOrder != llvm::AtomicOrdering::Monotonic)
-AcquireBB = CGF.createBasicBlock("acquire_fail", CGF.CurFn);
-  if (SuccessOrder == llvm::AtomicOrdering::SequentiallyConsistent)
-SeqCstBB = CGF.createBasicBlock("seqcst_fail", CGF.CurFn);
-
-  llvm::BasicBlock *ContBB = CGF.createBasicBlock("atomic.continue", 
CGF.CurFn);
-
-  llvm::SwitchInst *SI = CGF.Builder.CreateSwitch(FailureOrderVal, 
MonotonicBB);
-
-  // Emit all the 
diff erent atomics
+  auto *MonotonicBB = CGF.createBasicBlock("monotonic_fail", CGF.CurFn);
+  auto *AcquireBB = CGF.createBasicBlock("acquire_fail", CGF.CurFn);
+  auto *SeqCstBB = CGF.createBasicBlock("seqcst_fail", CGF.CurFn);
+  auto *ContBB = CGF.createBasicBlock("atomic.continue", CGF.CurFn);
 
   // MonotonicBB is arbitrarily chosen as the default case; in practice, this
   // doesn't matter unless someone is crazy enough to use something that
   // doesn't fold to a constant for the ordering.
+  llvm::SwitchInst *SI = CGF.Builder.CreateSwitch(FailureOrderVal, 
MonotonicBB);
+  // Implemented as acquire, since it's the closest in LLVM.
+  SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
+  AcquireBB);
+  SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::acquire),
+  AcquireBB);
+  SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::seq_cst),
+  SeqCstBB);
+
+  // Emit all the 
diff erent atomics
   CGF.Builder.SetInsertPoint(MonotonicBB);
   emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
 Size, SuccessOrder, llvm::AtomicOrdering::Monotonic, 
Scope);
   CGF.Builder.CreateBr(ContBB);
 
-  if (AcquireBB) {
-CGF.Builder.SetInsertPoint(AcquireBB);
-emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
-  Size, SuccessOrder, llvm::AtomicOrdering::Acquire, 
Scope);
-CGF.Builder.CreateBr(ContBB);
-if (SuccessOrder != llvm::AtomicOrdering::Release)
-  SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
-  AcquireBB);
-SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::acquire),
-AcquireBB);
-  }
-  if (SeqCstBB) {
-CGF.Builder.SetInsertPoint(SeqCstBB);
-emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, 
SuccessOrder,
-  llvm::AtomicOrdering::SequentiallyConsistent, Scope);
-CGF.Builder.CreateBr(ContBB);
-SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::seq_cst),
-SeqCstBB);
-  }
+  CGF.Builder.SetInsertPoint(AcquireBB);
+  emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, SuccessOrder,
+llvm::AtomicOrdering::Acquire, Scope);
+  CGF.Builder.CreateBr(ContBB);
+
+  CGF.Builder.SetInsertPoint(SeqCstBB);
+  emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, SuccessOrder,
+llvm::AtomicOrdering::SequentiallyConsistent, Scope);
+  CGF.Builder.CreateBr(ContBB);
 
   CGF.Builder.SetInsertPoint(ContBB);
 }

diff  --git a/clang/test/CodeGen/atomic-ops.c b/clang/test/CodeGen/atomic-ops.c
index 269406fc3c50f..02edec19bca93 100644
--- a/clang/test/CodeGen/atomic-ops.c
+++ b/clang/test/CodeGen/atomic-ops.c
@@ -490,29 +490,36 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, 
int success, int fail) {
 
   // CHECK: [[MONOTONIC]]
   // CHECK: switch {{.*}}, label %[[MONOTONIC_MONOTONIC:[0-9a-zA-Z._]+]] [
+  // CHECK-NEXT: i32 1, label %[[MONOTONIC_ACQUIRE:[0-9a-zA-Z._]+]]
+  // CHECK-NEXT: i32 2, label %[

[clang] 431e313 - [CGAtomic] Lift stronger requirements on cmpxch and support acquire failure mode

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2021-03-23T16:45:37-07:00
New Revision: 431e3138a1f3fd2bb7b25e1f4766d935058136f8

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

LOG: [CGAtomic] Lift stronger requirements on cmpxch and support acquire 
failure mode

- Fix `emitAtomicCmpXchgFailureSet` to support release/acquire (succ/fail) 
memory order.
- Remove stronger checks for cmpxch.

Effectively, this addresses http://wg21.link/p0418

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

Added: 


Modified: 
clang/lib/CodeGen/CGAtomic.cpp
clang/test/CodeGen/atomic-ops.c
clang/test/CodeGenOpenCL/atomic-ops.cl
llvm/docs/LangRef.rst

Removed: 




diff  --git a/clang/lib/CodeGen/CGAtomic.cpp b/clang/lib/CodeGen/CGAtomic.cpp
index c7256e240a31..8ac36e4a6b64 100644
--- a/clang/lib/CodeGen/CGAtomic.cpp
+++ b/clang/lib/CodeGen/CGAtomic.cpp
@@ -427,6 +427,8 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction 
&CGF, AtomicExpr *E,
 else
   switch ((llvm::AtomicOrderingCABI)FOS) {
   case llvm::AtomicOrderingCABI::relaxed:
+  // 31.7.2.18: "The failure argument shall not be memory_order_release
+  // nor memory_order_acq_rel". Fallback to monotonic.
   case llvm::AtomicOrderingCABI::release:
   case llvm::AtomicOrderingCABI::acq_rel:
 FailureOrder = llvm::AtomicOrdering::Monotonic;
@@ -439,12 +441,10 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction 
&CGF, AtomicExpr *E,
 FailureOrder = llvm::AtomicOrdering::SequentiallyConsistent;
 break;
   }
-if (isStrongerThan(FailureOrder, SuccessOrder)) {
-  // Don't assert on undefined behavior "failure argument shall be no
-  // stronger than the success argument".
-  FailureOrder =
-  llvm::AtomicCmpXchgInst::getStrongestFailureOrdering(SuccessOrder);
-}
+// Prior to c++17, "the failure argument shall be no stronger than the
+// success argument". This condition has been lifted and the only
+// precondition is 31.7.2.18. Effectively treat this as a DR and skip
+// language version checks.
 emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2, Size, 
SuccessOrder,
   FailureOrder, Scope);
 return;
@@ -454,8 +454,7 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction 
&CGF, AtomicExpr *E,
   llvm::BasicBlock *MonotonicBB = nullptr, *AcquireBB = nullptr,
*SeqCstBB = nullptr;
   MonotonicBB = CGF.createBasicBlock("monotonic_fail", CGF.CurFn);
-  if (SuccessOrder != llvm::AtomicOrdering::Monotonic &&
-  SuccessOrder != llvm::AtomicOrdering::Release)
+  if (SuccessOrder != llvm::AtomicOrdering::Monotonic)
 AcquireBB = CGF.createBasicBlock("acquire_fail", CGF.CurFn);
   if (SuccessOrder == llvm::AtomicOrdering::SequentiallyConsistent)
 SeqCstBB = CGF.createBasicBlock("seqcst_fail", CGF.CurFn);
@@ -479,8 +478,9 @@ static void emitAtomicCmpXchgFailureSet(CodeGenFunction 
&CGF, AtomicExpr *E,
 emitAtomicCmpXchg(CGF, E, IsWeak, Dest, Ptr, Val1, Val2,
   Size, SuccessOrder, llvm::AtomicOrdering::Acquire, 
Scope);
 CGF.Builder.CreateBr(ContBB);
-SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
-AcquireBB);
+if (SuccessOrder != llvm::AtomicOrdering::Release)
+  SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::consume),
+  AcquireBB);
 SI->addCase(CGF.Builder.getInt32((int)llvm::AtomicOrderingCABI::acquire),
 AcquireBB);
   }

diff  --git a/clang/test/CodeGen/atomic-ops.c b/clang/test/CodeGen/atomic-ops.c
index 4deb1322e0ff..269406fc3c50 100644
--- a/clang/test/CodeGen/atomic-ops.c
+++ b/clang/test/CodeGen/atomic-ops.c
@@ -500,6 +500,7 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int 
success, int fail) {
 
   // CHECK: [[RELEASE]]
   // CHECK: switch {{.*}}, label %[[RELEASE_MONOTONIC:[0-9a-zA-Z._]+]] [
+  // CHECK-NEXT: i32 2, label %[[RELEASE_ACQUIRE:[0-9a-zA-Z._]+]]
   // CHECK-NEXT: ]
 
   // CHECK: [[ACQREL]]
@@ -527,6 +528,14 @@ void generalFailureOrder(_Atomic(int) *ptr, int *ptr2, int 
success, int fail) {
   // CHECK: cmpxchg {{.*}} acquire acquire, align
   // CHECK: br
 
+  // CHECK: [[RELEASE_MONOTONIC]]
+  // CHECK: cmpxchg {{.*}} release monotonic, align
+  // CHECK: br
+
+  // CHECK: [[RELEASE_ACQUIRE]]
+  // CHECK: cmpxchg {{.*}} release acquire, align
+  // CHECK: br
+
   // CHECK: [[ACQREL_MONOTONIC]]
   // CHECK: cmpxchg {{.*}} acq_rel monotonic, align
   // CHECK: br
@@ -562,6 +571,20 @@ void generalWeakness(int *ptr, int *ptr2, _Bool weak) {
   // CHECK-NOT: br
   // CHECK: cmpxchg weak {{.*}} seq_cst seq_cst, align
   // CHECK: br
+
+  __atomic_compare_exchange_n(ptr, ptr2, 42, weak, memory_order_release, 

[clang] cffb0dd - [SemaTemplate] Stop passing insertion position around during VarTemplate instantiation

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2020-10-12T16:48:50-07:00
New Revision: cffb0dd54d41d8e249d2009467c4beb5b681ba26

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

LOG: [SemaTemplate] Stop passing insertion position around during VarTemplate 
instantiation

They can get stale at use time because of updates from other recursive
specializations. Instead, rely on the existence of previous declarations to add
the specialization.

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

Added: 


Modified: 
clang/include/clang/Sema/Sema.h
clang/include/clang/Sema/Template.h
clang/lib/Sema/SemaTemplate.cpp
clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
clang/test/SemaTemplate/instantiate-var-template.cpp

Removed: 




diff  --git a/clang/include/clang/Sema/Sema.h b/clang/include/clang/Sema/Sema.h
index ca1f87cfdb2b..b5f0c08300bf 100644
--- a/clang/include/clang/Sema/Sema.h
+++ b/clang/include/clang/Sema/Sema.h
@@ -9171,7 +9171,7 @@ class Sema final {
   const TemplateArgumentList &TemplateArgList,
   const TemplateArgumentListInfo &TemplateArgsInfo,
   SmallVectorImpl &Converted,
-  SourceLocation PointOfInstantiation, void *InsertPos,
+  SourceLocation PointOfInstantiation,
   LateInstantiatedAttrVec *LateAttrs = nullptr,
   LocalInstantiationScope *StartingScope = nullptr);
   VarTemplateSpecializationDecl *CompleteVarTemplateSpecializationDecl(

diff  --git a/clang/include/clang/Sema/Template.h 
b/clang/include/clang/Sema/Template.h
index 91d175fdd050..0dcaf565591b 100644
--- a/clang/include/clang/Sema/Template.h
+++ b/clang/include/clang/Sema/Template.h
@@ -600,7 +600,7 @@ enum class TemplateSubstitutionKind : char {
 TagDecl *NewDecl);
 
 Decl *VisitVarTemplateSpecializationDecl(
-VarTemplateDecl *VarTemplate, VarDecl *FromVar, void *InsertPos,
+VarTemplateDecl *VarTemplate, VarDecl *FromVar,
 const TemplateArgumentListInfo &TemplateArgsInfo,
 ArrayRef Converted,
 VarTemplateSpecializationDecl *PrevDecl = nullptr);

diff  --git a/clang/lib/Sema/SemaTemplate.cpp b/clang/lib/Sema/SemaTemplate.cpp
index 8baf5b96fbf8..4ecae8faad66 100644
--- a/clang/lib/Sema/SemaTemplate.cpp
+++ b/clang/lib/Sema/SemaTemplate.cpp
@@ -4584,7 +4584,7 @@ Sema::CheckVarTemplateId(VarTemplateDecl *Template, 
SourceLocation TemplateLoc,
   // FIXME: LateAttrs et al.?
   VarTemplateSpecializationDecl *Decl = BuildVarTemplateInstantiation(
   Template, InstantiationPattern, *InstantiationArgs, TemplateArgs,
-  Converted, TemplateNameLoc, InsertPos /*, LateAttrs, StartingScope*/);
+  Converted, TemplateNameLoc /*, LateAttrs, StartingScope*/);
   if (!Decl)
 return true;
 

diff  --git a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp 
b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
index 9420bd04b7a9..7200dc72825d 100644
--- a/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
+++ b/clang/lib/Sema/SemaTemplateInstantiateDecl.cpp
@@ -3628,11 +3628,11 @@ Decl 
*TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
 return nullptr;
 
   return VisitVarTemplateSpecializationDecl(
-  InstVarTemplate, D, InsertPos, VarTemplateArgsInfo, Converted, PrevDecl);
+  InstVarTemplate, D, VarTemplateArgsInfo, Converted, PrevDecl);
 }
 
 Decl *TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
-VarTemplateDecl *VarTemplate, VarDecl *D, void *InsertPos,
+VarTemplateDecl *VarTemplate, VarDecl *D,
 const TemplateArgumentListInfo &TemplateArgsInfo,
 ArrayRef Converted,
 VarTemplateSpecializationDecl *PrevDecl) {
@@ -3655,8 +3655,11 @@ Decl 
*TemplateDeclInstantiator::VisitVarTemplateSpecializationDecl(
   SemaRef.Context, Owner, D->getInnerLocStart(), D->getLocation(),
   VarTemplate, DI->getType(), DI, D->getStorageClass(), Converted);
   Var->setTemplateArgsInfo(TemplateArgsInfo);
-  if (InsertPos)
+  if (!PrevDecl) {
+void *InsertPos = nullptr;
+VarTemplate->findSpecialization(Converted, InsertPos);
 VarTemplate->AddSpecialization(Var, InsertPos);
+  }
 
   if (SemaRef.getLangOpts().OpenCL)
 SemaRef.deduceOpenCLAddressSpace(Var);
@@ -4865,7 +4868,7 @@ VarTemplateSpecializationDecl 
*Sema::BuildVarTemplateInstantiation(
 const TemplateArgumentList &TemplateArgList,
 const TemplateArgumentListInfo &TemplateArgsInfo,
 SmallVectorImpl &Converted,
-SourceLocation PointOfInstantiation, void *InsertPos,
+SourceLocation PointOfInstantiation,
 LateInstantiatedAttrVec *LateAttrs,
 LocalInstantiationScope *StartingScope) {
   if (FromVar->isInvalidDecl())
@@ -4904,7 +4907,7 @@ VarTemplateSpecializationDecl 
*Sema::BuildVarTemplateInstantiation(
 
   return cast_or_null(
   Instantiator.VisitVarTemplateSpecial

[clang] d892eec - Reapply: Make header inclusion order from umbrella dirs deterministic

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2020-04-21T15:45:54-07:00
New Revision: d892eec710caae84099f38fdb89d32ca15a23c1a

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

LOG: Reapply: Make header inclusion order from umbrella dirs deterministic

Sort the headers by name before adding the includes in
collectModuleHeaderIncludes. This makes the include order for building
umbrellas deterministic across different filesystems and also guarantees
that the ASTWriter always dump top headers in the same order.

There's currently no good way to test for this behavior.

This was first introduced in r289478 and reverted few times because of
ASANifed test failures on open source bots (both from LLVM and Swift).

Finally reproduced the problem in a Linux machine and use std::sort as a
fix, since we are not dealing with POD-like types.

rdar://problem/28116411

Added: 


Modified: 
clang/lib/Frontend/FrontendAction.cpp

Removed: 




diff  --git a/clang/lib/Frontend/FrontendAction.cpp 
b/clang/lib/Frontend/FrontendAction.cpp
index 6168057d115d..dc361b2fdd24 100644
--- a/clang/lib/Frontend/FrontendAction.cpp
+++ b/clang/lib/Frontend/FrontendAction.cpp
@@ -364,6 +364,7 @@ static std::error_code collectModuleHeaderIncludes(
 llvm::sys::path::native(UmbrellaDir.Entry->getName(), DirNative);
 
 llvm::vfs::FileSystem &FS = FileMgr.getVirtualFileSystem();
+SmallVector, 8> Headers;
 for (llvm::vfs::recursive_directory_iterator Dir(FS, DirNative, EC), End;
  Dir != End && !EC; Dir.increment(EC)) {
   // Check whether this entry has an extension typically associated with
@@ -394,13 +395,25 @@ static std::error_code collectModuleHeaderIncludes(
++It)
 llvm::sys::path::append(RelativeHeader, *It);
 
-  // Include this header as part of the umbrella directory.
-  Module->addTopHeader(*Header);
-  addHeaderInclude(RelativeHeader, Includes, LangOpts, Module->IsExternC);
+  std::string RelName = RelativeHeader.c_str();
+  Headers.push_back(std::make_pair(RelName, *Header));
 }
 
 if (EC)
   return EC;
+
+// Sort header paths and make the header inclusion order deterministic
+// across 
diff erent OSs and filesystems.
+llvm::sort(Headers.begin(), Headers.end(), [](
+  const std::pair &LHS,
+  const std::pair &RHS) {
+return LHS.first < RHS.first;
+});
+for (auto &H : Headers) {
+  // Include this header as part of the umbrella directory.
+  Module->addTopHeader(H.second);
+  addHeaderInclude(H.first, Includes, LangOpts, Module->IsExternC);
+}
   }
 
   // Recurse into submodules.



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

[clang] 90f58ea - [ODRHash] Factor out functionality for CXXRecord ODR diagnostics (NFCI)

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2020-01-22T13:33:17-08:00
New Revision: 90f58eaeff5f1d5017e7b689fac79180cdfa0160

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

LOG: [ODRHash] Factor out functionality for CXXRecord ODR diagnostics (NFCI)

There's going to be a lot of common code between RecordDecl and
CXXRecordDecl, factor out some of the logic in preparation for
adding the RecordDecl side.

Added: 


Modified: 
clang/lib/Serialization/ASTReader.cpp

Removed: 




diff  --git a/clang/lib/Serialization/ASTReader.cpp 
b/clang/lib/Serialization/ASTReader.cpp
index 21f6a36565ef..5351ac31c179 100644
--- a/clang/lib/Serialization/ASTReader.cpp
+++ b/clang/lib/Serialization/ASTReader.cpp
@@ -9451,6 +9451,446 @@ void ASTReader::diagnoseOdrViolations() {
 return Hash.CalculateHash();
   };
 
+  // Used with err_module_odr_violation_mismatch_decl and
+  // note_module_odr_violation_mismatch_decl
+  // This list should be the same Decl's as in ODRHash::isWhiteListedDecl
+  enum ODRMismatchDecl {
+EndOfClass,
+PublicSpecifer,
+PrivateSpecifer,
+ProtectedSpecifer,
+StaticAssert,
+Field,
+CXXMethod,
+TypeAlias,
+TypeDef,
+Var,
+Friend,
+FunctionTemplate,
+Other
+  };
+
+  // Used with err_module_odr_violation_mismatch_decl_
diff  and
+  // note_module_odr_violation_mismatch_decl_
diff 
+  enum ODRMismatchDeclDifference {
+StaticAssertCondition,
+StaticAssertMessage,
+StaticAssertOnlyMessage,
+FieldName,
+FieldTypeName,
+FieldSingleBitField,
+FieldDifferentWidthBitField,
+FieldSingleMutable,
+FieldSingleInitializer,
+FieldDifferentInitializers,
+MethodName,
+MethodDeleted,
+MethodDefaulted,
+MethodVirtual,
+MethodStatic,
+MethodVolatile,
+MethodConst,
+MethodInline,
+MethodNumberParameters,
+MethodParameterType,
+MethodParameterName,
+MethodParameterSingleDefaultArgument,
+MethodParameterDifferentDefaultArgument,
+MethodNoTemplateArguments,
+MethodDifferentNumberTemplateArguments,
+MethodDifferentTemplateArgument,
+MethodSingleBody,
+MethodDifferentBody,
+TypedefName,
+TypedefType,
+VarName,
+VarType,
+VarSingleInitializer,
+VarDifferentInitializer,
+VarConstexpr,
+FriendTypeFunction,
+FriendType,
+FriendFunction,
+FunctionTemplateDifferentNumberParameters,
+FunctionTemplateParameterDifferentKind,
+FunctionTemplateParameterName,
+FunctionTemplateParameterSingleDefaultArgument,
+FunctionTemplateParameterDifferentDefaultArgument,
+FunctionTemplateParameterDifferentType,
+FunctionTemplatePackParameter,
+  };
+
+  // These lambdas have the common portions of the ODR diagnostics.  This
+  // has the same return as Diag(), so addition parameters can be passed
+  // in with operator<<
+  auto ODRDiagDeclError = [this](NamedDecl *FirstRecord, StringRef FirstModule,
+ SourceLocation Loc, SourceRange Range,
+ ODRMismatchDeclDifference DiffType) {
+return Diag(Loc, diag::err_module_odr_violation_mismatch_decl_
diff )
+   << FirstRecord << FirstModule.empty() << FirstModule << Range
+   << DiffType;
+  };
+  auto ODRDiagDeclNote = [this](StringRef SecondModule, SourceLocation Loc,
+SourceRange Range, ODRMismatchDeclDifference 
DiffType) {
+return Diag(Loc, diag::note_module_odr_violation_mismatch_decl_
diff )
+   << SecondModule << Range << DiffType;
+  };
+
+  auto ODRDiagField = [this, &ODRDiagDeclError, &ODRDiagDeclNote,
+   &ComputeQualTypeODRHash, &ComputeODRHash](
+  NamedDecl *FirstRecord, StringRef FirstModule,
+  StringRef SecondModule, FieldDecl *FirstField,
+  FieldDecl *SecondField) {
+IdentifierInfo *FirstII = FirstField->getIdentifier();
+IdentifierInfo *SecondII = SecondField->getIdentifier();
+if (FirstII->getName() != SecondII->getName()) {
+  ODRDiagDeclError(FirstRecord, FirstModule, FirstField->getLocation(),
+   FirstField->getSourceRange(), FieldName)
+  << FirstII;
+  ODRDiagDeclNote(SecondModule, SecondField->getLocation(),
+  SecondField->getSourceRange(), FieldName)
+  << SecondII;
+
+  return true;
+}
+
+assert(getContext().hasSameType(FirstField->getType(),
+SecondField->getType()));
+
+QualType FirstType = FirstField->getType();
+QualType SecondType = SecondField->getType();
+if (ComputeQualTypeODRHash(FirstType) !=
+ComputeQualTypeODRHash(SecondType)) {
+  ODRDiagDeclErr

[clang] 3466ceb - Add a test to cover structural match for recursive data types

[Bruno Cardoso Lopes via cfe-commits]

Author: Bruno Cardoso Lopes
Date: 2019-11-14T18:32:27-08:00
New Revision: 3466cebe94ba461b296bb1314e76118cc2823dfb

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

LOG: Add a test to cover structural match for recursive data types

This didn't use to work prior to r370639, now that this is supported
add a testcase to prevent regressions.

rdar://problem/53602368

Added: 
clang/test/Modules/Inputs/rec-types/a.h
clang/test/Modules/Inputs/rec-types/b.h
clang/test/Modules/Inputs/rec-types/c.h
clang/test/Modules/Inputs/rec-types/module.modulemap
clang/test/Modules/structural-equivalent-recursive-types.c

Modified: 


Removed: 




diff  --git a/clang/test/Modules/Inputs/rec-types/a.h 
b/clang/test/Modules/Inputs/rec-types/a.h
new file mode 100644
index ..8e88ea7c5042
--- /dev/null
+++ b/clang/test/Modules/Inputs/rec-types/a.h
@@ -0,0 +1,2 @@
+// rec-types/a.h
+#include "b.h"

diff  --git a/clang/test/Modules/Inputs/rec-types/b.h 
b/clang/test/Modules/Inputs/rec-types/b.h
new file mode 100644
index ..05bbd99dd580
--- /dev/null
+++ b/clang/test/Modules/Inputs/rec-types/b.h
@@ -0,0 +1,2 @@
+// rec-types/b.h
+#include "c.h"

diff  --git a/clang/test/Modules/Inputs/rec-types/c.h 
b/clang/test/Modules/Inputs/rec-types/c.h
new file mode 100644
index ..fd2eb5a259b0
--- /dev/null
+++ b/clang/test/Modules/Inputs/rec-types/c.h
@@ -0,0 +1,7 @@
+struct some_descriptor
+{
+  // commenting line above make this struct work
+  void *(*thunk)(struct some_descriptor *);
+  unsigned long key;
+};
+

diff  --git a/clang/test/Modules/Inputs/rec-types/module.modulemap 
b/clang/test/Modules/Inputs/rec-types/module.modulemap
new file mode 100644
index ..680ed71ac9f2
--- /dev/null
+++ b/clang/test/Modules/Inputs/rec-types/module.modulemap
@@ -0,0 +1,9 @@
+module a {
+  header "a.h"
+  // Hide content by not re-exporting module b.
+}
+
+module b {
+  header "b.h"
+  export *
+}

diff  --git a/clang/test/Modules/structural-equivalent-recursive-types.c 
b/clang/test/Modules/structural-equivalent-recursive-types.c
new file mode 100644
index ..bb3ec7b499bb
--- /dev/null
+++ b/clang/test/Modules/structural-equivalent-recursive-types.c
@@ -0,0 +1,7 @@
+// RUN: rm -rf %t
+// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -fmodules-cache-path=%t 
-I%S/Inputs/rec-types -fsyntax-only %s -verify
+#include "a.h"
+#include "c.h"
+void foo(struct some_descriptor *st) { (void)st->thunk; }
+
+// expected-no-diagnostics



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

r374895 - Reapply: [Modules][PCH] Hash input files content

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Tue Oct 15 07:23:55 2019
New Revision: 374895

URL: http://llvm.org/viewvc/llvm-project?rev=374895&view=rev
Log:
Reapply: [Modules][PCH] Hash input files content

Summary:
When files often get touched during builds, the mtime based validation
leads to different problems in implicit modules builds, even when the
content doesn't actually change:

- Modules only: module invalidation due to out of date files. Usually causing 
rebuild traffic.
- Modules + PCH: build failures because clang cannot rebuild a module if it 
comes from building a PCH.
- PCH: build failures because clang cannot rebuild a PCH in case one of the 
input headers has different mtime.

This patch proposes hashing the content of input files (headers and
module maps), which is performed during serialization time. When looking
at input files for validation, clang only computes the hash in case
there's a mtime mismatch.

I've tested a couple of different hash algorithms availble in LLVM in
face of building modules+pch for `#import `:
- `hash_code`: performace diff within the noise, total module cache increased 
by 0.07%.
- `SHA1`: 5% slowdown. Haven't done real size measurements, but it'd be 
BLOCK_ID+20 bytes per input file, instead of BLOCK_ID+8 bytes from `hash_code`.
- `MD5`: 3% slowdown. Like above, but BLOCK_ID+16 bytes per input file.

Given the numbers above, the patch uses `hash_code`. The patch also
improves invalidation error msgs to point out which type of problem the
user is facing: "mtime", "size" or "content".

rdar://problem/29320105

Reviewers: dexonsmith, arphaman, rsmith, aprantl

Subscribers: jkorous, cfe-commits, ributzka

Tags: #clang

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

llvm-svn: 374841

Added:
cfe/trunk/test/Modules/validate-file-content.m
cfe/trunk/test/PCH/validate-file-content.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=374895&r1=374894&r2=374895&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Tue Oct 15 
07:23:55 2019
@@ -18,13 +18,16 @@ def err_fe_pch_malformed : Error<
 def err_fe_pch_malformed_block : Error<
 "malformed block record in PCH file: '%0'">, DefaultFatal;
 def err_fe_pch_file_modified : Error<
-"file '%0' has been modified since the precompiled header '%1' was built">,
+"file '%0' has been modified since the precompiled header '%1' was built"
+": %select{size|mtime|content}2 changed">,
 DefaultFatal;
 def err_fe_module_file_modified : Error<
-"file '%0' has been modified since the module file '%1' was built">,
+"file '%0' has been modified since the module file '%1' was built"
+": %select{size|mtime|content}2 changed">,
 DefaultFatal;
 def err_fe_ast_file_modified : Error<
-"file '%0' has been modified since the AST file '%1' was built">,
+"file '%0' has been modified since the AST file '%1' was built"
+": %select{size|mtime|content}2 changed">,
 DefaultFatal;
 def err_fe_pch_file_overridden : Error<
 "file '%0' from the precompiled header has been overridden">;
@@ -399,6 +402,8 @@ def warn_module_uses_date_time : Warning
 def err_module_no_size_mtime_for_header : Error<
   "cannot emit module %0: %select{size|mtime}1 must be explicitly specified "
   "for missing header file \"%2\"">;
+def err_module_unable_to_hash_content : Error<
+  "failed to hash content for '%0' because memory buffer cannot be retrieved">;
 } // let CategoryName
 } // let Component
 

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=374895&r1=374894&r2=374895&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Tue Oct 15 07:23:55 2019
@@ -1368,6 +1368,28 @@ def fmodules_validate_system_headers : F
   HelpText<"Validate the system headers that a module depends on when loading 
the module">;
 def fno_modules_validate_system_headers : Flag<["-"], 
"fno-modules-validate-system-headers">,
   Group, Flags<[DriverOption]>;
+
+def fvalidate_ast_input_files_content:
+  Flag <["-"],

Re: r374841 - [Modules][PCH] Hash input files content

[Bruno Cardoso Lopes via cfe-commits]

Thanks Eric!

On Mon, Oct 14, 2019 at 8:12 PM Eric Christopher  wrote:
>
> This was breaking a few bots and I couldn't find you on irc so I've
> reverted it thusly:
>
> echristo@jhereg ~/s/llvm-project> git llvm push
> Pushing 1 commit:
>   175b1b856ea Temporarily Revert [Modules][PCH] Hash input files
> content as it's breaking a few bots.
> Sendingcfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
> Sendingcfe/trunk/include/clang/Driver/Options.td
> Sendingcfe/trunk/include/clang/Lex/HeaderSearchOptions.h
> Sendingcfe/trunk/include/clang/Serialization/ASTBitCodes.h
> Sendingcfe/trunk/include/clang/Serialization/ASTReader.h
> Sendingcfe/trunk/lib/Driver/ToolChains/Clang.cpp
> Sendingcfe/trunk/lib/Frontend/CompilerInstance.cpp
> Sendingcfe/trunk/lib/Frontend/CompilerInvocation.cpp
> Sendingcfe/trunk/lib/Serialization/ASTReader.cpp
> Sendingcfe/trunk/lib/Serialization/ASTWriter.cpp
> Deleting   cfe/trunk/test/Modules/validate-file-content.m
> Deleting   cfe/trunk/test/PCH/validate-file-content.m
> Transmitting file data ..done
> Committing transaction...
> Committed revision 374842.
> Committed 175b1b856ea to svn.
>
> Sorry for the inconvenience!
>
> -eric
>
> On Mon, Oct 14, 2019 at 3:59 PM Bruno Cardoso Lopes via cfe-commits
>  wrote:
> >
> > Author: bruno
> > Date: Mon Oct 14 16:02:03 2019
> > New Revision: 374841
> >
> > URL: http://llvm.org/viewvc/llvm-project?rev=374841&view=rev
> > Log:
> > [Modules][PCH] Hash input files content
> >
> > Summary:
> > When files often get touched during builds, the mtime based validation
> > leads to different problems in implicit modules builds, even when the
> > content doesn't actually change:
> >
> > - Modules only: module invalidation due to out of date files. Usually 
> > causing rebuild traffic.
> > - Modules + PCH: build failures because clang cannot rebuild a module if it 
> > comes from building a PCH.
> > - PCH: build failures because clang cannot rebuild a PCH in case one of the 
> > input headers has different mtime.
> >
> > This patch proposes hashing the content of input files (headers and
> > module maps), which is performed during serialization time. When looking
> > at input files for validation, clang only computes the hash in case
> > there's a mtime mismatch.
> >
> > I've tested a couple of different hash algorithms availble in LLVM in
> > face of building modules+pch for `#import `:
> > - `hash_code`: performace diff within the noise, total module cache 
> > increased by 0.07%.
> > - `SHA1`: 5% slowdown. Haven't done real size measurements, but it'd be 
> > BLOCK_ID+20 bytes per input file, instead of BLOCK_ID+8 bytes from 
> > `hash_code`.
> > - `MD5`: 3% slowdown. Like above, but BLOCK_ID+16 bytes per input file.
> >
> > Given the numbers above, the patch uses `hash_code`. The patch also
> > improves invalidation error msgs to point out which type of problem the
> > user is facing: "mtime", "size" or "content".
> >
> > rdar://problem/29320105
> >
> > Reviewers: dexonsmith, arphaman, rsmith, aprantl
> >
> > Subscribers: jkorous, cfe-commits, ributzka
> >
> > Tags: #clang
> >
> > Differential Revision: https://reviews.llvm.org/D67249
> >
> > Added:
> > cfe/trunk/test/Modules/validate-file-content.m
> > cfe/trunk/test/PCH/validate-file-content.m
> > Modified:
> > cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
> > cfe/trunk/include/clang/Driver/Options.td
> > cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
> > cfe/trunk/include/clang/Serialization/ASTBitCodes.h
> > cfe/trunk/include/clang/Serialization/ASTReader.h
> > cfe/trunk/lib/Driver/ToolChains/Clang.cpp
> > cfe/trunk/lib/Frontend/CompilerInstance.cpp
> > cfe/trunk/lib/Frontend/CompilerInvocation.cpp
> > cfe/trunk/lib/Serialization/ASTReader.cpp
> > cfe/trunk/lib/Serialization/ASTWriter.cpp
> >
> > Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
> > URL: 
> > http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=374841&r1=374840&r2=374841&view=diff
> > ==
> > --- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
> > +++ cfe/trunk/include/clang/Basic/DiagnosticSerial

r374841 - [Modules][PCH] Hash input files content

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Mon Oct 14 16:02:03 2019
New Revision: 374841

URL: http://llvm.org/viewvc/llvm-project?rev=374841&view=rev
Log:
[Modules][PCH] Hash input files content

Summary:
When files often get touched during builds, the mtime based validation
leads to different problems in implicit modules builds, even when the
content doesn't actually change:

- Modules only: module invalidation due to out of date files. Usually causing 
rebuild traffic.
- Modules + PCH: build failures because clang cannot rebuild a module if it 
comes from building a PCH.
- PCH: build failures because clang cannot rebuild a PCH in case one of the 
input headers has different mtime.

This patch proposes hashing the content of input files (headers and
module maps), which is performed during serialization time. When looking
at input files for validation, clang only computes the hash in case
there's a mtime mismatch.

I've tested a couple of different hash algorithms availble in LLVM in
face of building modules+pch for `#import `:
- `hash_code`: performace diff within the noise, total module cache increased 
by 0.07%.
- `SHA1`: 5% slowdown. Haven't done real size measurements, but it'd be 
BLOCK_ID+20 bytes per input file, instead of BLOCK_ID+8 bytes from `hash_code`.
- `MD5`: 3% slowdown. Like above, but BLOCK_ID+16 bytes per input file.

Given the numbers above, the patch uses `hash_code`. The patch also
improves invalidation error msgs to point out which type of problem the
user is facing: "mtime", "size" or "content".

rdar://problem/29320105

Reviewers: dexonsmith, arphaman, rsmith, aprantl

Subscribers: jkorous, cfe-commits, ributzka

Tags: #clang

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

Added:
cfe/trunk/test/Modules/validate-file-content.m
cfe/trunk/test/PCH/validate-file-content.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
cfe/trunk/include/clang/Driver/Options.td
cfe/trunk/include/clang/Lex/HeaderSearchOptions.h
cfe/trunk/include/clang/Serialization/ASTBitCodes.h
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Driver/ToolChains/Clang.cpp
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Frontend/CompilerInvocation.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/lib/Serialization/ASTWriter.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=374841&r1=374840&r2=374841&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Mon Oct 14 
16:02:03 2019
@@ -18,13 +18,16 @@ def err_fe_pch_malformed : Error<
 def err_fe_pch_malformed_block : Error<
 "malformed block record in PCH file: '%0'">, DefaultFatal;
 def err_fe_pch_file_modified : Error<
-"file '%0' has been modified since the precompiled header '%1' was built">,
+"file '%0' has been modified since the precompiled header '%1' was built"
+": %select{size|mtime|content}2 changed">,
 DefaultFatal;
 def err_fe_module_file_modified : Error<
-"file '%0' has been modified since the module file '%1' was built">,
+"file '%0' has been modified since the module file '%1' was built"
+": %select{size|mtime|content}2 changed">,
 DefaultFatal;
 def err_fe_ast_file_modified : Error<
-"file '%0' has been modified since the AST file '%1' was built">,
+"file '%0' has been modified since the AST file '%1' was built"
+": %select{size|mtime|content}2 changed">,
 DefaultFatal;
 def err_fe_pch_file_overridden : Error<
 "file '%0' from the precompiled header has been overridden">;
@@ -399,6 +402,8 @@ def warn_module_uses_date_time : Warning
 def err_module_no_size_mtime_for_header : Error<
   "cannot emit module %0: %select{size|mtime}1 must be explicitly specified "
   "for missing header file \"%2\"">;
+def err_module_unable_to_hash_content : Error<
+  "failed to hash content for '%0' because memory buffer cannot be retrieved">;
 } // let CategoryName
 } // let Component
 

Modified: cfe/trunk/include/clang/Driver/Options.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Driver/Options.td?rev=374841&r1=374840&r2=374841&view=diff
==
--- cfe/trunk/include/clang/Driver/Options.td (original)
+++ cfe/trunk/include/clang/Driver/Options.td Mon Oct 14 16:02:03 2019
@@ -1368,6 +1368,28 @@ def fmodules_validate_system_headers : F
   HelpText<"Validate the system headers that a module depends on when loading 
the module">;
 def fno_modules_validate_system_headers : Flag<["-"], 
"fno-modules-validate-system-headers">,
   Group, Flags<[DriverOption]>;
+
+def fvalidate_ast_input_files_content:
+  Flag <["-"], "fvalidate-ast-input-files

r372039 - [Modules][Objective-C] Use complete decl from module when diagnosing missing import

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Mon Sep 16 15:00:29 2019
New Revision: 372039

URL: http://llvm.org/viewvc/llvm-project?rev=372039&view=rev
Log:
[Modules][Objective-C] Use complete decl from module when diagnosing missing 
import

Summary:
Otherwise the definition (first found) for ObjCInterfaceDecl's might
precede the module one, which will eventually lead to crash, since
diagnoseMissingImport needs one coming from a module.

This behavior changed after Richard's r342018, which started to look
into the definition of ObjCInterfaceDecls.

rdar://problem/49237144

Reviewers: rsmith, arphaman

Subscribers: jkorous, dexonsmith, ributzka, cfe-commits

Tags: #clang

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

Added:
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/

cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/

cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/

cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/Bar.h

cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/Foo.h

cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Modules/

cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Modules/module.modulemap

cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/PrivateHeaders/

cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/PrivateHeaders/RandoPriv.h
cfe/trunk/test/Modules/interface-diagnose-missing-import.m
Modified:
cfe/trunk/lib/Sema/SemaLookup.cpp

Modified: cfe/trunk/lib/Sema/SemaLookup.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaLookup.cpp?rev=372039&r1=372038&r2=372039&view=diff
==
--- cfe/trunk/lib/Sema/SemaLookup.cpp (original)
+++ cfe/trunk/lib/Sema/SemaLookup.cpp Mon Sep 16 15:00:29 2019
@@ -5273,8 +5273,11 @@ static NamedDecl *getDefinitionToImport(
 return FD->getDefinition();
   if (TagDecl *TD = dyn_cast(D))
 return TD->getDefinition();
+  // The first definition for this ObjCInterfaceDecl might be in the TU
+  // and not associated with any module. Use the one we know to be complete
+  // and have just seen in a module.
   if (ObjCInterfaceDecl *ID = dyn_cast(D))
-return ID->getDefinition();
+return ID;
   if (ObjCProtocolDecl *PD = dyn_cast(D))
 return PD->getDefinition();
   if (TemplateDecl *TD = dyn_cast(D))

Added: 
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/Bar.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/Bar.h?rev=372039&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/Bar.h
 (added)
+++ 
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/Bar.h
 Mon Sep 16 15:00:29 2019
@@ -0,0 +1 @@
+// interface-diagnose-missing-import/Foo.framework/Headers/Bar.h

Added: 
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/Foo.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/Foo.h?rev=372039&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/Foo.h
 (added)
+++ 
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Headers/Foo.h
 Mon Sep 16 15:00:29 2019
@@ -0,0 +1,2 @@
+#import 
+#import 

Added: 
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Modules/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Modules/module.modulemap?rev=372039&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Modules/module.modulemap
 (added)
+++ 
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/Modules/module.modulemap
 Mon Sep 16 15:00:29 2019
@@ -0,0 +1,6 @@
+// interface-diagnose-missing-import/Foo.framework/Modules/module.modulemap
+framework module Foo {
+  umbrella header "Foo.h"
+  export *
+  module * { export * }
+}

Added: 
cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/PrivateHeaders/RandoPriv.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/interface-diagnose-missing-import/Foo.framework/PrivateHeaders/RandoPriv.h?rev=372039&view=auto
==
--- 
cfe/t

r370422 - [Modules] Make ReadModuleMapFileBlock errors reliable

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Thu Aug 29 16:14:08 2019
New Revision: 370422

URL: http://llvm.org/viewvc/llvm-project?rev=370422&view=rev
Log:
[Modules] Make ReadModuleMapFileBlock errors reliable

This prevents a crash when an error should be emitted instead.

During implicit module builds, there are cases where ReadASTCore is called with
ImportedBy set to nullptr, which breaks expectations in ReadModuleMapFileBlock,
leading to crashes.

Fix this by improving ReadModuleMapFileBlock to handle ImportedBy correctly.
This only happens non deterministically in the wild, when the underlying file
system changes while concurrent compiler invocations use implicit modules,
forcing rebuilds which see an inconsistent filesystem state. That said, there's
no much to do w.r.t. writing tests here.

rdar://problem/48828801

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td?rev=370422&r1=370421&r2=370422&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSerializationKinds.td Thu Aug 29 
16:14:08 2019
@@ -77,13 +77,13 @@ def remark_module_import : Remark<
   InGroup;
 
 def err_imported_module_not_found : Error<
-"module '%0' in AST file '%1' (imported by AST file '%2') "
+"module '%0' in AST file '%1' %select{(imported by AST file '%2') |}4"
 "is not defined in any loaded module map file; "
 "maybe you need to load '%3'?">, DefaultFatal;
 def note_imported_by_pch_module_not_found : Note<
 "consider adding '%0' to the header search path">;
 def err_imported_module_modmap_changed : Error<
-"module '%0' imported by AST file '%1' found in a different module map 
file"
+"module '%0' %select{in|imported by}4 AST file '%1' found in a different 
module map file"
 " (%2) than when the importing AST file was built (%3)">, DefaultFatal;
 def err_imported_module_relocated : Error<
 "module '%0' was built in directory '%1' but now resides in "

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=370422&r1=370421&r2=370422&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Thu Aug 29 16:14:08 2019
@@ -3823,7 +3823,6 @@ ASTReader::ReadModuleMapFileBlock(Record
 const FileEntry *ModMap = M ? Map.getModuleMapFileForUniquing(M) : nullptr;
 // Don't emit module relocation error if we have -fno-validate-pch
 if (!PP.getPreprocessorOpts().DisablePCHValidation && !ModMap) {
-  assert(ImportedBy && "top-level import should be verified");
   if ((ClientLoadCapabilities & ARR_OutOfDate) == 0) {
 if (auto *ASTFE = M ? M->getASTFile() : nullptr) {
   // This module was defined by an imported (explicit) module.
@@ -3832,12 +3831,13 @@ ASTReader::ReadModuleMapFileBlock(Record
 } else {
   // This module was built with a different module map.
   Diag(diag::err_imported_module_not_found)
-  << F.ModuleName << F.FileName << ImportedBy->FileName
-  << F.ModuleMapPath;
+  << F.ModuleName << F.FileName
+  << (ImportedBy ? ImportedBy->FileName : "") << F.ModuleMapPath
+  << !ImportedBy;
   // In case it was imported by a PCH, there's a chance the user is
   // just missing to include the search path to the directory 
containing
   // the modulemap.
-  if (ImportedBy->Kind == MK_PCH)
+  if (ImportedBy && ImportedBy->Kind == MK_PCH)
 Diag(diag::note_imported_by_pch_module_not_found)
 << llvm::sys::path::parent_path(F.ModuleMapPath);
 }
@@ -3851,11 +3851,13 @@ ASTReader::ReadModuleMapFileBlock(Record
 auto StoredModMap = FileMgr.getFile(F.ModuleMapPath);
 if (!StoredModMap || *StoredModMap != ModMap) {
   assert(ModMap && "found module is missing module map file");
-  assert(ImportedBy && "top-level import should be verified");
+  assert((ImportedBy || F.Kind == MK_ImplicitModule) &&
+ "top-level import should be verified");
+  bool NotImported = F.Kind == MK_ImplicitModule && !ImportedBy;
   if ((ClientLoadCapabilities & ARR_OutOfDate) == 0)
 Diag(diag::err_imported_module_modmap_changed)
-  << F.ModuleName << ImportedBy->FileName
-  << ModMap->getName() << F.ModuleMapPath;
+<< F.ModuleName << (NotImported ? F.FileName : 
ImportedBy->FileName)
+<< ModMap->getName() << F.ModuleMapPath << NotImported;
   return OutOfDate;
 }

r348789 - [constexpr][c++2a] Try-catch blocks in constexpr functions

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Mon Dec 10 11:03:12 2018
New Revision: 348789

URL: http://llvm.org/viewvc/llvm-project?rev=348789&view=rev
Log:
[constexpr][c++2a] Try-catch blocks in constexpr functions

Implement support for try-catch blocks in constexpr functions, as
proposed in http://wg21.link/P1002 and voted in San Diego for c++20.

The idea is that we can still never throw inside constexpr, so the catch
block is never entered. A try-catch block like this:

try { f(); } catch (...) { }

is then morally equivalent to just

{ f(); }

Same idea should apply for function/constructor try blocks.

rdar://problem/45530773

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

Modified:
cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
cfe/trunk/lib/AST/ExprConstant.cpp
cfe/trunk/lib/Sema/SemaDeclCXX.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p3.cpp
cfe/trunk/test/CXX/dcl.dcl/dcl.spec/dcl.constexpr/p4.cpp
cfe/trunk/test/CXX/drs/dr6xx.cpp
cfe/trunk/www/cxx_status.html

Modified: cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td?rev=348789&r1=348788&r2=348789&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticSemaKinds.td Mon Dec 10 11:03:12 
2018
@@ -2357,6 +2357,13 @@ def warn_cxx11_compat_constexpr_body_inv
   "use of this statement in a constexpr %select{function|constructor}0 "
   "is incompatible with C++ standards before C++14">,
   InGroup, DefaultIgnore;
+def ext_constexpr_body_invalid_stmt_cxx2a : ExtWarn<
+  "use of this statement in a constexpr %select{function|constructor}0 "
+  "is a C++2a extension">, InGroup;
+def warn_cxx17_compat_constexpr_body_invalid_stmt : Warning<
+  "use of this statement in a constexpr %select{function|constructor}0 "
+  "is incompatible with C++ standards before C++2a">,
+  InGroup, DefaultIgnore;
 def ext_constexpr_type_definition : ExtWarn<
   "type definition in a constexpr %select{function|constructor}0 "
   "is a C++14 extension">, InGroup;
@@ -2409,6 +2416,16 @@ def note_constexpr_body_previous_return
   "previous return statement is here">;
 def err_constexpr_function_try_block : Error<
   "function try block not allowed in constexpr 
%select{function|constructor}0">;
+
+// c++2a function try blocks in constexpr
+def ext_constexpr_function_try_block_cxx2a : ExtWarn<
+  "function try block in constexpr %select{function|constructor}0 is "
+  "a C++2a extension">, InGroup;
+def warn_cxx17_compat_constexpr_function_try_block : Warning<
+  "function try block in constexpr %select{function|constructor}0 is "
+  "incompatible with C++ standards before C++2a">,
+  InGroup, DefaultIgnore;
+
 def err_constexpr_union_ctor_no_init : Error<
   "constexpr union constructor does not initialize any member">;
 def err_constexpr_ctor_missing_init : Error<

Modified: cfe/trunk/lib/AST/ExprConstant.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/AST/ExprConstant.cpp?rev=348789&r1=348788&r2=348789&view=diff
==
--- cfe/trunk/lib/AST/ExprConstant.cpp (original)
+++ cfe/trunk/lib/AST/ExprConstant.cpp Mon Dec 10 11:03:12 2018
@@ -4279,6 +4279,9 @@ static EvalStmtResult EvaluateStmt(StmtR
   case Stmt::CaseStmtClass:
   case Stmt::DefaultStmtClass:
 return EvaluateStmt(Result, Info, cast(S)->getSubStmt(), Case);
+  case Stmt::CXXTryStmtClass:
+// Evaluate try blocks by evaluating all sub statements.
+return EvaluateStmt(Result, Info, cast(S)->getTryBlock(), 
Case);
   }
 }
 

Modified: cfe/trunk/lib/Sema/SemaDeclCXX.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclCXX.cpp?rev=348789&r1=348788&r2=348789&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclCXX.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclCXX.cpp Mon Dec 10 11:03:12 2018
@@ -1803,7 +1803,7 @@ static void CheckConstexprCtorInitialize
 static bool
 CheckConstexprFunctionStmt(Sema &SemaRef, const FunctionDecl *Dcl, Stmt *S,
SmallVectorImpl &ReturnStmts,
-   SourceLocation &Cxx1yLoc) {
+   SourceLocation &Cxx1yLoc, SourceLocation &Cxx2aLoc) 
{
   // - its function-body shall be [...] a compound-statement that contains only
   switch (S->getStmtClass()) {
   case Stmt::NullStmtClass:
@@ -1840,7 +1840,7 @@ CheckConstexprFunctionStmt(Sema &SemaRef
 CompoundStmt *CompStmt = cast(S);
 for (auto *BodyIt : CompStmt->body()) {
   if (!CheckConstexprFunctionStmt(SemaRef, Dcl, BodyIt, ReturnStmts,
-  Cxx1yLoc))
+  Cxx1yLoc, Cxx2aLoc))
 return false;
 }
 return true;
@@ -1858,11 +1858,11 @@ CheckConstex

r342499 - [Modules] Add platform and environment features to requires clause

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Tue Sep 18 10:11:13 2018
New Revision: 342499

URL: http://llvm.org/viewvc/llvm-project?rev=342499&view=rev
Log:
[Modules] Add platform and environment features to requires clause

Allows module map writers to add build requirements based on
platform/os. This helps when target features and language dialects
aren't enough to conditionalize building a module, among other things,
it allow module maps for different platforms to live in the same file.

rdar://problem/43909745

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

Added:
cfe/trunk/test/Modules/target-platform-features.m
Modified:
cfe/trunk/docs/Modules.rst
cfe/trunk/lib/Basic/Module.cpp

Modified: cfe/trunk/docs/Modules.rst
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/docs/Modules.rst?rev=342499&r1=342498&r2=342499&view=diff
==
--- cfe/trunk/docs/Modules.rst (original)
+++ cfe/trunk/docs/Modules.rst Tue Sep 18 10:11:13 2018
@@ -411,7 +411,7 @@ A *requires-declaration* specifies the r
   *feature*:
 ``!``:sub:`opt` *identifier*
 
-The requirements clause allows specific modules or submodules to specify that 
they are only accessible with certain language dialects or on certain 
platforms. The feature list is a set of identifiers, defined below. If any of 
the features is not available in a given translation unit, that translation 
unit shall not import the module. When building a module for use by a 
compilation, submodules requiring unavailable features are ignored. The 
optional ``!`` indicates that a feature is incompatible with the module.
+The requirements clause allows specific modules or submodules to specify that 
they are only accessible with certain language dialects, platforms, 
environments and target specific features. The feature list is a set of 
identifiers, defined below. If any of the features is not available in a given 
translation unit, that translation unit shall not import the module. When 
building a module for use by a compilation, submodules requiring unavailable 
features are ignored. The optional ``!`` indicates that a feature is 
incompatible with the module.
 
 The following features are defined:
 
@@ -466,6 +466,11 @@ tls
 *target feature*
   A specific target feature (e.g., ``sse4``, ``avx``, ``neon``) is available.
 
+*platform/os*
+  A os/platform variant (e.g. ``freebsd``, ``win32``, ``windows``, ``linux``, 
``ios``, ``macos``, ``iossimulator``) is available.
+
+*environment*
+  A environment variant (e.g. ``gnu``, ``gnueabi``, ``android``, ``msvc``) is 
available.
 
 **Example:** The ``std`` module can be extended to also include C++ and C++11 
headers using a *requires-declaration*:
 

Modified: cfe/trunk/lib/Basic/Module.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/Module.cpp?rev=342499&r1=342498&r2=342499&view=diff
==
--- cfe/trunk/lib/Basic/Module.cpp (original)
+++ cfe/trunk/lib/Basic/Module.cpp Tue Sep 18 10:11:13 2018
@@ -71,6 +71,37 @@ Module::~Module() {
   }
 }
 
+static bool isPlatformEnvironment(const TargetInfo &Target, StringRef Feature) 
{
+  StringRef Platform = Target.getPlatformName();
+  StringRef Env = Target.getTriple().getEnvironmentName();
+
+  // Attempt to match platform and environment.
+  if (Platform == Feature || Target.getTriple().getOSName() == Feature ||
+  Env == Feature)
+return true;
+
+  auto CmpPlatformEnv = [](StringRef LHS, StringRef RHS) {
+auto Pos = LHS.find("-");
+if (Pos == StringRef::npos)
+  return false;
+SmallString<128> NewLHS = LHS.slice(0, Pos);
+NewLHS += LHS.slice(Pos+1, LHS.size());
+return NewLHS == RHS;
+  };
+
+  SmallString<128> PlatformEnv = Target.getTriple().getOSAndEnvironmentName();
+  // Darwin has different but equivalent variants for simulators, example:
+  //   1. x86_64-apple-ios-simulator
+  //   2. x86_64-apple-iossimulator
+  // where both are valid examples of the same platform+environment but in the
+  // variant (2) the simulator is hardcoded as part of the platform name. Both
+  // forms above should match for "iossimulator" requirement.
+  if (Target.getTriple().isOSDarwin() && PlatformEnv.endswith("simulator"))
+return PlatformEnv == Feature || CmpPlatformEnv(PlatformEnv, Feature);
+
+  return PlatformEnv == Feature;
+}
+
 /// Determine whether a translation unit built using the current
 /// language options has the given feature.
 static bool hasFeature(StringRef Feature, const LangOptions &LangOpts,
@@ -93,7 +124,8 @@ static bool hasFeature(StringRef Feature
 .Case("opencl", LangOpts.OpenCL)
 .Case("tls", Target.isTLSSupported())
 .Case("zvector", LangOpts.ZVector)
-.Default(Target.hasFeature(Feature));
+.Default(Target.hasFeature(Feature) ||
+   

r341902 - [Modules] Add imported modules to the output of -module-file-info

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Mon Sep 10 22:17:13 2018
New Revision: 341902

URL: http://llvm.org/viewvc/llvm-project?rev=341902&view=rev
Log:
[Modules] Add imported modules to the output of -module-file-info

Fix a bug in the deserialization of IMPORTS section and allow for
imported modules to also be printed with -module-file-info.

rdar://problem/43867753

Modified:
cfe/trunk/include/clang/Serialization/ASTReader.h
cfe/trunk/lib/Frontend/FrontendActions.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp
cfe/trunk/test/Modules/module_file_info.m

Modified: cfe/trunk/include/clang/Serialization/ASTReader.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Serialization/ASTReader.h?rev=341902&r1=341901&r2=341902&view=diff
==
--- cfe/trunk/include/clang/Serialization/ASTReader.h (original)
+++ cfe/trunk/include/clang/Serialization/ASTReader.h Mon Sep 10 22:17:13 2018
@@ -232,7 +232,7 @@ public:
 
   /// If needsImportVisitation returns \c true, this is called for each
   /// AST file imported by this AST file.
-  virtual void visitImport(StringRef Filename) {}
+  virtual void visitImport(StringRef ModuleName, StringRef Filename) {}
 
   /// Indicates that a particular module file extension has been read.
   virtual void readModuleFileExtension(

Modified: cfe/trunk/lib/Frontend/FrontendActions.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendActions.cpp?rev=341902&r1=341901&r2=341902&view=diff
==
--- cfe/trunk/lib/Frontend/FrontendActions.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendActions.cpp Mon Sep 10 22:17:13 2018
@@ -601,6 +601,17 @@ namespace {
 
   return true;
 }
+
+/// Returns true if this \c ASTReaderListener wants to receive the
+/// imports of the AST file via \c visitImport, false otherwise.
+bool needsImportVisitation() const override { return true; }
+
+/// If needsImportVisitation returns \c true, this is called for each
+/// AST file imported by this AST file.
+void visitImport(StringRef ModuleName, StringRef Filename) override {
+  Out.indent(2) << "Imports module '" << ModuleName
+<< "': " << Filename.str() << "\n";
+}
 #undef DUMP_BOOLEAN
   };
 }

Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Serialization/ASTReader.cpp?rev=341902&r1=341901&r2=341902&view=diff
==
--- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
+++ cfe/trunk/lib/Serialization/ASTReader.cpp Mon Sep 10 22:17:13 2018
@@ -4868,11 +4868,11 @@ bool ASTReader::readASTFileControlBlock(
   unsigned Idx = 0, N = Record.size();
   while (Idx < N) {
 // Read information about the AST file.
-Idx += 5; // ImportLoc, Size, ModTime, Signature
-SkipString(Record, Idx); // Module name; FIXME: pass to listener?
+Idx += 1+1+1+1+5; // Kind, ImportLoc, Size, ModTime, Signature
+std::string ModuleName = ReadString(Record, Idx);
 std::string Filename = ReadString(Record, Idx);
 ResolveImportedPath(Filename, ModuleDir);
-Listener.visitImport(Filename);
+Listener.visitImport(ModuleName, Filename);
   }
   break;
 }

Modified: cfe/trunk/test/Modules/module_file_info.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module_file_info.m?rev=341902&r1=341901&r2=341902&view=diff
==
--- cfe/trunk/test/Modules/module_file_info.m (original)
+++ cfe/trunk/test/Modules/module_file_info.m Mon Sep 10 22:17:13 2018
@@ -16,6 +16,7 @@
 
 // CHECK: Module name: DependsOnModule
 // CHECK: Module map file: {{.*}}DependsOnModule.framework{{[/\\]}}module.map
+// CHECK: Imports module 'Module': {{.*}}Module.pcm
 
 // CHECK: Language options:
 // CHECK:   C99: Yes


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

Re: r340114 - [analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker

[Bruno Cardoso Lopes via cfe-commits]

Hi George,

This broke the modules build, reverted in r340117 for now. I can help
you figure out any module map change if necessary next week.

Thanks,
On Fri, Aug 17, 2018 at 6:46 PM George Karpenkov via cfe-commits
 wrote:
>
> Author: george.karpenkov
> Date: Fri Aug 17 18:45:50 2018
> New Revision: 340114
>
> URL: http://llvm.org/viewvc/llvm-project?rev=340114&view=rev
> Log:
> [analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker
>
> ARCMigrator is using code from RetainCountChecker, which is a layering
> violation (and it also does it badly, by using a different header, and
> then relying on implementation being present in a header file).
>
> This change splits up RetainSummaryManager into a separate library in
> lib/Analysis, which can be used independently of a checker.
>
> Differential Revision: https://reviews.llvm.org/D50934
>
> Added:
> cfe/trunk/include/clang/Analysis/RetainSummaryManager.h
> cfe/trunk/include/clang/StaticAnalyzer/Checkers/SelectorExtras.h
> cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
> Removed:
> cfe/trunk/include/clang/Analysis/ObjCRetainCount.h
> 
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp
> 
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h
> cfe/trunk/lib/StaticAnalyzer/Checkers/SelectorExtras.h
> Modified:
> cfe/trunk/lib/ARCMigrate/CMakeLists.txt
> cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
> cfe/trunk/lib/Analysis/CMakeLists.txt
> cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
> cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
> cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp
> 
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp
> 
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h
> 
> cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
> cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp
>
> Removed: cfe/trunk/include/clang/Analysis/ObjCRetainCount.h
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ObjCRetainCount.h?rev=340113&view=auto
> ==
> --- cfe/trunk/include/clang/Analysis/ObjCRetainCount.h (original)
> +++ cfe/trunk/include/clang/Analysis/ObjCRetainCount.h (removed)
> @@ -1,231 +0,0 @@
> -//==-- ObjCRetainCount.h - Retain count summaries for Cocoa ---*- C++ 
> -*--//
> -//
> -// The LLVM Compiler Infrastructure
> -//
> -// This file is distributed under the University of Illinois Open Source
> -// License. See LICENSE.TXT for details.
> -//
> -//===--===//
> -//
> -//  This file defines the core data structures for retain count "summaries"
> -//  for Objective-C and Core Foundation APIs.  These summaries are used
> -//  by the static analyzer to summarize the retain/release effects of
> -//  function and method calls.  This drives a path-sensitive typestate
> -//  analysis in the static analyzer, but can also potentially be used by
> -//  other clients.
> -//
> -//===--===//
> -
> -#ifndef LLVM_CLANG_STATICANALYZER_CHECKERS_OBJCRETAINCOUNT_H
> -#define LLVM_CLANG_STATICANALYZER_CHECKERS_OBJCRETAINCOUNT_H
> -
> -#include "clang/Basic/LLVM.h"
> -#include "llvm/ADT/ArrayRef.h"
> -#include "llvm/ADT/SmallVector.h"
> -
> -namespace clang {
> -class FunctionDecl;
> -class ObjCMethodDecl;
> -
> -namespace ento { namespace objc_retain {
> -
> -/// An ArgEffect summarizes the retain count behavior on an argument or 
> receiver
> -/// to a function or method.
> -enum ArgEffect {
> -  /// There is no effect.
> -  DoNothing,
> -
> -  /// The argument is treated as if an -autorelease message had been sent to
> -  /// the referenced object.
> -  Autorelease,
> -
> -  /// The argument is treated as if an -dealloc message had been sent to
> -  /// the referenced object.
> -  Dealloc,
> -
> -  /// The argument has its reference count decreased by 1.  This is as
> -  /// if CFRelease has been called on the argument.
> -  DecRef,
> -
> -  /// The argument has its reference count decreased by 1.  This is as
> -  /// if a -release message has been sent to the argument.  This differs
> -  /// in behavior from DecRef when GC is enabled.
> -  DecRefMsg,
> -
> -  /// The argument has its reference count decreased by 1 to model
> -  /// a transferred bridge cast under ARC.
> -  DecRefBridgedTransferred,
> -
> -  /// The argument has its reference count increased by 1.  This is as
> -  /// if a -retain message has been sent to the argument.  This differs
> -  /// in behavior from IncRef when GC is enabled.
> -  IncRefMsg,
> -
> -  /// The argument has its reference count increased by 1.  This is as
> -  /// if CFRetain has been call

r340117 - Revert "[analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker"

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Fri Aug 17 20:22:11 2018
New Revision: 340117

URL: http://llvm.org/viewvc/llvm-project?rev=340117&view=rev
Log:
Revert "[analyzer] [NFC] Split up RetainSummaryManager from RetainCountChecker"

This reverts commit a786521fa66c72edd308baff0c08961b6d964fb1.

Bots haven't caught up yet, but broke modules build with:

../tools/clang/include/clang/StaticAnalyzer/Checkers/MPIFunctionClassifier.h:18:10:
fatal error: cyclic dependency in module 'Clang_StaticAnalyzer_Core':
Clang_StaticAnalyzer_Core -> Clang_Analysis ->
Clang_StaticAnalyzer_Checkers -> Clang_StaticAnalyzer_Core
 ^

Added:
cfe/trunk/include/clang/Analysis/ObjCRetainCount.h

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.cpp
  - copied, changed from r340114, 
cfe/trunk/lib/Analysis/RetainSummaryManager.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountSummaries.h
  - copied, changed from r340114, 
cfe/trunk/include/clang/Analysis/RetainSummaryManager.h
cfe/trunk/lib/StaticAnalyzer/Checkers/SelectorExtras.h
  - copied, changed from r340114, 
cfe/trunk/include/clang/StaticAnalyzer/Checkers/SelectorExtras.h
Removed:
cfe/trunk/include/clang/Analysis/RetainSummaryManager.h
cfe/trunk/include/clang/StaticAnalyzer/Checkers/SelectorExtras.h
cfe/trunk/lib/Analysis/RetainSummaryManager.cpp
Modified:
cfe/trunk/lib/ARCMigrate/CMakeLists.txt
cfe/trunk/lib/ARCMigrate/ObjCMT.cpp
cfe/trunk/lib/Analysis/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/BasicObjCFoundationChecks.cpp
cfe/trunk/lib/StaticAnalyzer/Checkers/CMakeLists.txt
cfe/trunk/lib/StaticAnalyzer/Checkers/NoReturnFunctionChecker.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.cpp

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountChecker.h

cfe/trunk/lib/StaticAnalyzer/Checkers/RetainCountChecker/RetainCountDiagnostics.h
cfe/trunk/lib/StaticAnalyzer/Checkers/TrustNonnullChecker.cpp

Added: cfe/trunk/include/clang/Analysis/ObjCRetainCount.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Analysis/ObjCRetainCount.h?rev=340117&view=auto
==
--- cfe/trunk/include/clang/Analysis/ObjCRetainCount.h (added)
+++ cfe/trunk/include/clang/Analysis/ObjCRetainCount.h Fri Aug 17 20:22:11 2018
@@ -0,0 +1,231 @@
+//==-- ObjCRetainCount.h - Retain count summaries for Cocoa ---*- C++ 
-*--//
+//
+// The LLVM Compiler Infrastructure
+//
+// This file is distributed under the University of Illinois Open Source
+// License. See LICENSE.TXT for details.
+//
+//===--===//
+//
+//  This file defines the core data structures for retain count "summaries"
+//  for Objective-C and Core Foundation APIs.  These summaries are used
+//  by the static analyzer to summarize the retain/release effects of
+//  function and method calls.  This drives a path-sensitive typestate
+//  analysis in the static analyzer, but can also potentially be used by
+//  other clients.
+//
+//===--===//
+
+#ifndef LLVM_CLANG_STATICANALYZER_CHECKERS_OBJCRETAINCOUNT_H
+#define LLVM_CLANG_STATICANALYZER_CHECKERS_OBJCRETAINCOUNT_H
+
+#include "clang/Basic/LLVM.h"
+#include "llvm/ADT/ArrayRef.h"
+#include "llvm/ADT/SmallVector.h"
+
+namespace clang {
+class FunctionDecl;
+class ObjCMethodDecl;
+
+namespace ento { namespace objc_retain {
+
+/// An ArgEffect summarizes the retain count behavior on an argument or 
receiver
+/// to a function or method.
+enum ArgEffect {
+  /// There is no effect.
+  DoNothing,
+
+  /// The argument is treated as if an -autorelease message had been sent to
+  /// the referenced object.
+  Autorelease,
+
+  /// The argument is treated as if an -dealloc message had been sent to
+  /// the referenced object.
+  Dealloc,
+
+  /// The argument has its reference count decreased by 1.  This is as
+  /// if CFRelease has been called on the argument.
+  DecRef,
+
+  /// The argument has its reference count decreased by 1.  This is as
+  /// if a -release message has been sent to the argument.  This differs
+  /// in behavior from DecRef when GC is enabled.
+  DecRefMsg,
+
+  /// The argument has its reference count decreased by 1 to model
+  /// a transferred bridge cast under ARC.
+  DecRefBridgedTransferred,
+
+  /// The argument has its reference count increased by 1.  This is as
+  /// if a -retain message has been sent to the argument.  This differs
+  /// in behavior from IncRef when GC is enabled.
+  IncRefMsg,
+
+  /// The argument has its reference count increased by 1.  This is as
+  /// if CFRetain has been called on the argument.
+  IncRef,
+
+  /// Used to mark an argument as collectible in GC mode, currently a noop.
+  MakeCollectable,
+
+  /// The argument is a poin

r337555 - [www] Add CodeCompass and CodeChecker to Clang Related Projects page

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Fri Jul 20 07:46:10 2018
New Revision: 337555

URL: http://llvm.org/viewvc/llvm-project?rev=337555&view=rev
Log:
[www] Add CodeCompass and CodeChecker to Clang Related Projects page

Modified:
cfe/trunk/www/related.html

Modified: cfe/trunk/www/related.html
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/www/related.html?rev=337555&r1=337554&r2=337555&view=diff
==
--- cfe/trunk/www/related.html (original)
+++ cfe/trunk/www/related.html Fri Jul 20 07:46:10 2018
@@ -93,6 +93,28 @@
 
   
 
+  CodeCompass
+  
+
+  Site:
+http://github.com/Ericsson/CodeCompass";>http://github.com/Ericsson/CodeCompass
+
+
+CodeCompass is an open-source, extensible code comprehension framework 
which uses LLVM/Clang to analyze and visualize C and C++ projects. It also 
supports both regex-based text search, discovering complex C/C++ language 
elements, with advanced navigation and visualisation.
+
+  
+
+  CodeChecker
+  
+
+  Site:
+http://github.com/Ericsson/CodeChecker";>http://github.com/Ericsson/CodeChecker
+
+
+CodeChecker is a static analysis infrastructure built on the 
LLVM/Clang Static Analyzer toolchain. It provides a user interface to execute 
analysis of C/C++ projects with Clang SA and Clang-Tidy, which outputs are then 
stored into a database navigable via a web application. This web application 
and a corresponding command-line tool supports a variety of report management 
and issue triaging options, such as difference view between analyses, automatic 
incremental analysis, marking and commenting on individual reports.
+
+  
+
 
   
 


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

r337447 - [PCH+Modules] Load -fmodule-map-file content before including PCHs

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Thu Jul 19 05:32:06 2018
New Revision: 337447

URL: http://llvm.org/viewvc/llvm-project?rev=337447&view=rev
Log:
[PCH+Modules] Load -fmodule-map-file content before including PCHs

Consider:
1) Generate PCH with -fmodules and -fmodule-map-file
2) Use PCH with -fmodules and the same -fmodule-map-file

If we don't load -fmodule-map-file content before including PCHs,
the modules that are dependencies in PCHs cannot get loaded,
since there's no matching module map file when reading back the AST.

rdar://problem/40852867

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

Added:
cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m
Modified:
cfe/trunk/lib/Frontend/FrontendAction.cpp

Modified: cfe/trunk/lib/Frontend/FrontendAction.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/FrontendAction.cpp?rev=337447&r1=337446&r2=337447&view=diff
==
--- cfe/trunk/lib/Frontend/FrontendAction.cpp (original)
+++ cfe/trunk/lib/Frontend/FrontendAction.cpp Thu Jul 19 05:32:06 2018
@@ -766,6 +766,22 @@ bool FrontendAction::BeginSourceFile(Com
   if (!BeginSourceFileAction(CI))
 goto failure;
 
+  // If we were asked to load any module map files, do so now.
+  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
+if (auto *File = CI.getFileManager().getFile(Filename))
+  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
+  File, /*IsSystem*/false);
+else
+  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
+  }
+
+  // Add a module declaration scope so that modules from -fmodule-map-file
+  // arguments may shadow modules found implicitly in search paths.
+  CI.getPreprocessor()
+  .getHeaderSearchInfo()
+  .getModuleMap()
+  .finishModuleDeclarationScope();
+
   // Create the AST context and consumer unless this is a preprocessor only
   // action.
   if (!usesPreprocessorOnly()) {
@@ -855,22 +871,6 @@ bool FrontendAction::BeginSourceFile(Com
"doesn't support modules");
   }
 
-  // If we were asked to load any module map files, do so now.
-  for (const auto &Filename : CI.getFrontendOpts().ModuleMapFiles) {
-if (auto *File = CI.getFileManager().getFile(Filename))
-  CI.getPreprocessor().getHeaderSearchInfo().loadModuleMapFile(
-  File, /*IsSystem*/false);
-else
-  CI.getDiagnostics().Report(diag::err_module_map_not_found) << Filename;
-  }
-
-  // Add a module declaration scope so that modules from -fmodule-map-file
-  // arguments may shadow modules found implicitly in search paths.
-  CI.getPreprocessor()
-  .getHeaderSearchInfo()
-  .getModuleMap()
-  .finishModuleDeclarationScope();
-
   // If we were asked to load any module files, do so now.
   for (const auto &ModuleFile : CI.getFrontendOpts().ModuleFiles)
 if (!CI.loadModuleFile(ModuleFile))

Added: cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m?rev=337447&view=auto
==
--- cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m (added)
+++ cfe/trunk/test/Modules/module-imported-by-pch-with-modulemap.m Thu Jul 19 
05:32:06 2018
@@ -0,0 +1,16 @@
+// RUN: rm -rf %t.dst %t.cache
+// RUN: mkdir -p %t.dst/folder-with-modulemap %t.dst/pch-folder
+// RUN: echo '#import "folder-with-modulemap/included.h"' > %t.dst/header.h
+// RUN: echo 'extern int MyModuleVersion;' > 
%t.dst/folder-with-modulemap/MyModule.h
+// RUN: echo '@import MyModule;' > %t.dst/folder-with-modulemap/included.h
+// RUN: echo 'module MyModule { header "MyModule.h" }' > 
%t.dst/folder-with-modulemap/MyModule.modulemap
+
+// RUN: %clang_cc1 -emit-pch -o %t.dst/pch-folder/header.pch 
-fmodule-map-file=%t.dst/folder-with-modulemap/MyModule.modulemap -x 
objective-c-header -fmodules-cache-path=%t.cache -fmodules 
-fimplicit-module-maps %t.dst/header.h
+// RUN: %clang_cc1 -fsyntax-only 
-fmodule-map-file=%t.dst/folder-with-modulemap/MyModule.modulemap 
-fmodules-cache-path=%t.cache -fmodules -fimplicit-module-maps %s -include-pch 
%t.dst/pch-folder/header.pch -verify
+
+// expected-no-diagnostics
+
+void test() {
+  (void)MyModuleVersion; // should be found by implicit import
+}
+


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

r337430 - Reapply r336660: [Modules] Autoload subdirectory modulemaps with specific LangOpts

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Wed Jul 18 16:21:19 2018
New Revision: 337430

URL: http://llvm.org/viewvc/llvm-project?rev=337430&view=rev
Log:
Reapply r336660: [Modules] Autoload subdirectory modulemaps with specific 
LangOpts

Summary:
Reproducer and errors:
https://bugs.llvm.org/show_bug.cgi?id=37878

lookupModule was falling back to loadSubdirectoryModuleMaps when it couldn't
find ModuleName in (proper) search paths. This was causing iteration over all
files in the search path subdirectories for example "/usr/include/foobar" in
bugzilla case.

Users don't expect Clang to load modulemaps in subdirectories implicitly, and
also the disk access is not cheap.

if (AllowExtraModuleMapSearch) true with ObjC with @import ModuleName.

Reviewers: rsmith, aprantl, bruno

Subscribers: cfe-commits, teemperor, v.g.vassilev

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

Added:
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/a.h
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/b.h
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/c.h
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/include/module.modulemap
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/module.modulemap
cfe/trunk/test/Modules/autoload-subdirectory.cpp
Modified:
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=337430&r1=337429&r2=337430&view=diff
==
--- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h Wed Jul 18 16:21:19 2018
@@ -529,8 +529,12 @@ public:
   /// search directories to produce a module definition. If not, this lookup
   /// will only return an already-known module.
   ///
+  /// \param AllowExtraModuleMapSearch Whether we allow to search modulemaps
+  /// in subdirectories.
+  ///
   /// \returns The module with the given name.
-  Module *lookupModule(StringRef ModuleName, bool AllowSearch = true);
+  Module *lookupModule(StringRef ModuleName, bool AllowSearch = true,
+   bool AllowExtraModuleMapSearch = false);
 
   /// Try to find a module map file in the given directory, returning
   /// \c nullptr if none is found.
@@ -595,8 +599,12 @@ private:
   /// but for compatibility with some buggy frameworks, additional attempts
   /// may be made to find the module under a related-but-different search-name.
   ///
+  /// \param AllowExtraModuleMapSearch Whether we allow to search modulemaps
+  /// in subdirectories.
+  ///
   /// \returns The module named ModuleName.
-  Module *lookupModule(StringRef ModuleName, StringRef SearchName);
+  Module *lookupModule(StringRef ModuleName, StringRef SearchName,
+   bool AllowExtraModuleMapSearch = false);
 
   /// Retrieve a module with the given name, which may be part of the
   /// given framework.

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=337430&r1=337429&r2=337430&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Wed Jul 18 16:21:19 2018
@@ -1653,8 +1653,10 @@ CompilerInstance::loadModule(SourceLocat
 // Retrieve the cached top-level module.
 Module = Known->second;
   } else if (ModuleName == getLangOpts().CurrentModule) {
-// This is the module we're building. 
-Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
+// This is the module we're building.
+Module = PP->getHeaderSearchInfo().lookupModule(
+ModuleName, /*AllowSearch*/ true,
+/*AllowExtraModuleMapSearch*/ !IsInclusionDirective);
 /// FIXME: perhaps we should (a) look for a module using the module name
 //  to file map (PrebuiltModuleFiles) and (b) diagnose if still not found?
 //if (Module == nullptr) {
@@ -1666,7 +1668,8 @@ CompilerInstance::loadModule(SourceLocat
 Known = KnownModules.insert(std::make_pair(Path[0].first, Module)).first;
   } else {
 // Search for a module with the given name.
-Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
+Module = PP->getHeaderSearchInfo().lookupModule(ModuleName, true,
+!IsInclusionDirective);
 HeaderSearchOptions &HSOpts =
 PP->getHeaderSearchInfo().getHeaderSearchOpts();
 
@@ -1743,7 +1746,8 @@ CompilerInstance::loadModule(SourceLocat
ImportLoc, ARRFlags)) {
 case ASTReader::Success: {
   if (Source != ModuleCache && !Module) {
-Module = PP->getHeaderSearchInfo().lookupModu

r336920 - Revert "[modules] Fix 37878; Autoload subdirectory modulemaps with specific LangOpts"

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Thu Jul 12 10:38:48 2018
New Revision: 336920

URL: http://llvm.org/viewvc/llvm-project?rev=336920&view=rev
Log:
Revert "[modules] Fix 37878; Autoload subdirectory modulemaps with specific 
LangOpts"

This reverts commit f40124d4f05ecf4f880cf4e8f26922d861f705f3 / r336660.

This change shouldn't be affecting `@import` behavior, but turns out it is:
https://ci.swift.org/view/swift-master-next/job/oss-swift-incremental-RA-osx-master-next/2800/consoleFull#-12570166563122a513-f36a-4c87-8ed7-cbc36a1ec144

Working on a reduced testcase for this, reverting in the meantime.

rdar://problem/4210

Removed:
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/a.h
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/b.h
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/c.h
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/include/module.modulemap
cfe/trunk/test/Modules/Inputs/autoload-subdirectory/module.modulemap
cfe/trunk/test/Modules/autoload-subdirectory.cpp
Modified:
cfe/trunk/include/clang/Lex/HeaderSearch.h
cfe/trunk/lib/Frontend/CompilerInstance.cpp
cfe/trunk/lib/Lex/HeaderSearch.cpp
cfe/trunk/lib/Serialization/ASTReader.cpp

Modified: cfe/trunk/include/clang/Lex/HeaderSearch.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Lex/HeaderSearch.h?rev=336920&r1=336919&r2=336920&view=diff
==
--- cfe/trunk/include/clang/Lex/HeaderSearch.h (original)
+++ cfe/trunk/include/clang/Lex/HeaderSearch.h Thu Jul 12 10:38:48 2018
@@ -529,12 +529,8 @@ public:
   /// search directories to produce a module definition. If not, this lookup
   /// will only return an already-known module.
   ///
-  /// \param AllowExtraModuleMapSearch Whether we allow to search modulemaps
-  /// in subdirectories.
-  ///
   /// \returns The module with the given name.
-  Module *lookupModule(StringRef ModuleName, bool AllowSearch = true,
-   bool AllowExtraModuleMapSearch = false);
+  Module *lookupModule(StringRef ModuleName, bool AllowSearch = true);
 
   /// Try to find a module map file in the given directory, returning
   /// \c nullptr if none is found.
@@ -599,12 +595,8 @@ private:
   /// but for compatibility with some buggy frameworks, additional attempts
   /// may be made to find the module under a related-but-different search-name.
   ///
-  /// \param AllowExtraModuleMapSearch Whether we allow to search modulemaps
-  /// in subdirectories.
-  ///
   /// \returns The module named ModuleName.
-  Module *lookupModule(StringRef ModuleName, StringRef SearchName,
-   bool AllowExtraModuleMapSearch = false);
+  Module *lookupModule(StringRef ModuleName, StringRef SearchName);
 
   /// Retrieve a module with the given name, which may be part of the
   /// given framework.

Modified: cfe/trunk/lib/Frontend/CompilerInstance.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Frontend/CompilerInstance.cpp?rev=336920&r1=336919&r2=336920&view=diff
==
--- cfe/trunk/lib/Frontend/CompilerInstance.cpp (original)
+++ cfe/trunk/lib/Frontend/CompilerInstance.cpp Thu Jul 12 10:38:48 2018
@@ -1653,10 +1653,8 @@ CompilerInstance::loadModule(SourceLocat
 // Retrieve the cached top-level module.
 Module = Known->second;
   } else if (ModuleName == getLangOpts().CurrentModule) {
-// This is the module we're building.
-Module = PP->getHeaderSearchInfo().lookupModule(
-ModuleName, /*AllowSearch*/ true,
-/*AllowExtraModuleMapSearch*/ !IsInclusionDirective);
+// This is the module we're building. 
+Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
 /// FIXME: perhaps we should (a) look for a module using the module name
 //  to file map (PrebuiltModuleFiles) and (b) diagnose if still not found?
 //if (Module == nullptr) {
@@ -1668,8 +1666,7 @@ CompilerInstance::loadModule(SourceLocat
 Known = KnownModules.insert(std::make_pair(Path[0].first, Module)).first;
   } else {
 // Search for a module with the given name.
-Module = PP->getHeaderSearchInfo().lookupModule(ModuleName, true,
-!IsInclusionDirective);
+Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
 HeaderSearchOptions &HSOpts =
 PP->getHeaderSearchInfo().getHeaderSearchOpts();
 
@@ -1746,8 +1743,7 @@ CompilerInstance::loadModule(SourceLocat
ImportLoc, ARRFlags)) {
 case ASTReader::Success: {
   if (Source != ModuleCache && !Module) {
-Module = PP->getHeaderSearchInfo().lookupModule(ModuleName, true,
-!IsInclusionDirective);
+Module = PP->getHeaderSearchInfo().lookupModule(ModuleName);
 if (!Module || !Module->getASTFile() ||
 FileMgr->getFile(

r336031 - Add protocol redefinition to the current scope/context

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Fri Jun 29 17:49:27 2018
New Revision: 336031

URL: http://llvm.org/viewvc/llvm-project?rev=336031&view=rev
Log:
Add protocol redefinition to the current scope/context

Not doing so causes the AST writter to assert since the decl in question
never gets emitted. This is fine when modules is not used, but otherwise
we need to serialize something other than garbage.

rdar://problem/39844933

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

Added:
cfe/trunk/test/Modules/Inputs/protocol-redefinition/
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/

cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/

cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/

cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/

cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
cfe/trunk/test/Modules/protocol-redefinition.m
Modified:
cfe/trunk/lib/Sema/SemaDeclObjC.cpp

Modified: cfe/trunk/lib/Sema/SemaDeclObjC.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Sema/SemaDeclObjC.cpp?rev=336031&r1=336030&r2=336031&view=diff
==
--- cfe/trunk/lib/Sema/SemaDeclObjC.cpp (original)
+++ cfe/trunk/lib/Sema/SemaDeclObjC.cpp Fri Jun 29 17:49:27 2018
@@ -1210,6 +1210,11 @@ Sema::ActOnStartProtocolInterface(Source
 PDecl = ObjCProtocolDecl::Create(Context, CurContext, ProtocolName,
  ProtocolLoc, AtProtoInterfaceLoc,
  /*PrevDecl=*/nullptr);
+
+// If we are using modules, add the decl to the context in order to
+// serialize something meaningful.
+if (getLangOpts().Modules)
+  PushOnScopeChains(PDecl, TUScope);
 PDecl->startDefinition();
   } else {
 if (PrevDecl) {

Added: 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h?rev=336031&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
 (added)
+++ 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Headers/Base.h
 Fri Jun 29 17:49:27 2018
@@ -0,0 +1,3 @@
+@protocol Foo
+- (void)someMethodOnFoo;
+@end

Added: 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap?rev=336031&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
 (added)
+++ 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Base.framework/Modules/module.modulemap
 Fri Jun 29 17:49:27 2018
@@ -0,0 +1,4 @@
+framework module Base {
+  header "Base.h"
+  export *
+}
\ No newline at end of file

Added: 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h?rev=336031&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h 
(added)
+++ 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Headers/Kit.h 
Fri Jun 29 17:49:27 2018
@@ -0,0 +1,6 @@
+#import 
+
+// REDECLARATION
+@protocol Foo
+- (void)someMethodOnFoo;
+@end

Added: 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap?rev=336031&view=auto
==
--- 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
 (added)
+++ 
cfe/trunk/test/Modules/Inputs/protocol-redefinition/Kit.framework/Modules/module.modulemap
 Fri Jun 29 17:49:27 2018
@@ -0,0 +1,4 @@
+framework module Kit {
+  header "Kit.h"
+  export *
+}
\ No newline at end of file

Added: cfe/trunk/test/Modules/protocol-redefinition.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/protocol-red

r335780 - [Modules][ObjC] Warn on the use of '@import' in framework headers

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Wed Jun 27 13:29:36 2018
New Revision: 335780

URL: http://llvm.org/viewvc/llvm-project?rev=335780&view=rev
Log:
[Modules][ObjC] Warn on the use of '@import' in framework headers

Using @import in framework headers inhibit the use of such headers
when not using modules, this is specially bad for headers that end
up in the SDK (or any other system framework). Add a warning to give
users some indication that this is discouraged.

rdar://problem/39192894

Added:
cfe/trunk/test/Modules/Inputs/at-import-in-framework-header/
cfe/trunk/test/Modules/Inputs/at-import-in-framework-header/A.framework/

cfe/trunk/test/Modules/Inputs/at-import-in-framework-header/A.framework/Headers/

cfe/trunk/test/Modules/Inputs/at-import-in-framework-header/A.framework/Headers/A.h

cfe/trunk/test/Modules/Inputs/at-import-in-framework-header/A.framework/Modules/

cfe/trunk/test/Modules/Inputs/at-import-in-framework-header/A.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/at-import-in-framework-header/module.modulemap
cfe/trunk/test/Modules/at-import-in-framework-header.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
cfe/trunk/lib/Parse/Parser.cpp
cfe/trunk/test/VFS/umbrella-mismatch.m

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=335780&r1=335779&r2=335780&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Wed Jun 27 13:29:36 2018
@@ -34,6 +34,7 @@ def AutoImport : DiagGroup<"auto-import"
 def FrameworkHdrQuotedInclude : 
DiagGroup<"quoted-include-in-framework-header">;
 def FrameworkIncludePrivateFromPublic :
   DiagGroup<"framework-include-private-from-public">;
+def FrameworkHdrAtImport : DiagGroup<"atimport-in-framework-header">;
 def CXX14BinaryLiteral : DiagGroup<"c++14-binary-literal">;
 def CXXPre14CompatBinaryLiteral : 
DiagGroup<"c++98-c++11-compat-binary-literal">;
 def GNUBinaryLiteral : DiagGroup<"gnu-binary-literal">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td?rev=335780&r1=335779&r2=335780&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticParseKinds.td Wed Jun 27 13:29:36 
2018
@@ -248,6 +248,11 @@ def err_unexpected_at : Error<"unexpecte
 def err_atimport : Error<
 "use of '@import' when modules are disabled">;
 
+def warn_atimport_in_framework_header : Warning<
+  "use of '@import' in framework header is discouraged, "
+  "including this header requires -fmodules">,
+  InGroup;
+
 def err_invalid_reference_qualifier_application : Error<
   "'%0' qualifier may not be applied to a reference">;
 def err_illegal_decl_reference_to_reference : Error<

Modified: cfe/trunk/lib/Parse/Parser.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Parse/Parser.cpp?rev=335780&r1=335779&r2=335780&view=diff
==
--- cfe/trunk/lib/Parse/Parser.cpp (original)
+++ cfe/trunk/lib/Parse/Parser.cpp Wed Jun 27 13:29:36 2018
@@ -20,6 +20,7 @@
 #include "clang/Sema/DeclSpec.h"
 #include "clang/Sema/ParsedTemplate.h"
 #include "clang/Sema/Scope.h"
+#include "llvm/Support/Path.h"
 using namespace clang;
 
 
@@ -2123,6 +2124,7 @@ Decl *Parser::ParseModuleImport(SourceLo
   assert((AtLoc.isInvalid() ? Tok.is(tok::kw_import)
 : Tok.isObjCAtKeyword(tok::objc_import)) &&
  "Improper start to module import");
+  bool IsObjCAtImport = Tok.isObjCAtKeyword(tok::objc_import);
   SourceLocation ImportLoc = ConsumeToken();
   SourceLocation StartLoc = AtLoc.isInvalid() ? ImportLoc : AtLoc;
   
@@ -2146,6 +2148,16 @@ Decl *Parser::ParseModuleImport(SourceLo
   if (Import.isInvalid())
 return nullptr;
 
+  // Using '@import' in framework headers requires modules to be enabled so 
that
+  // the header is parseable. Emit a warning to make the user aware.
+  if (IsObjCAtImport && AtLoc.isValid()) {
+auto &SrcMgr = PP.getSourceManager();
+auto *FE = SrcMgr.getFileEntryForID(SrcMgr.getFileID(AtLoc));
+if (FE && llvm::sys::path::parent_path(FE->getDir()->getName())
+  .endswith(".framework"))
+  Diags.Report(AtLoc, diag::warn_atimport_in_framework_header);
+  }
+
   return Import.get();
 }
 

Added: 
cfe/trunk/test/Modules/Inputs/at-import-in-framework-header/A.framework/Headers/A.h
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/Inputs/at-import-in-framework-header/A.framework/Headers/A.h?rev=335780&view=auto
=

r335543 - Fix tests from r335542 to use %hmaptool

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Mon Jun 25 15:25:48 2018
New Revision: 335543

URL: http://llvm.org/viewvc/llvm-project?rev=335543&view=rev
Log:
Fix tests from r335542 to use %hmaptool

Modified:
cfe/trunk/test/Modules/framework-public-includes-private.m

Modified: cfe/trunk/test/Modules/framework-public-includes-private.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/framework-public-includes-private.m?rev=335543&r1=335542&r2=335543&view=diff
==
--- cfe/trunk/test/Modules/framework-public-includes-private.m (original)
+++ cfe/trunk/test/Modules/framework-public-includes-private.m Mon Jun 25 
15:25:48 2018
@@ -3,8 +3,8 @@
 // RUN: rm -rf %t
 // RUN: mkdir %t
 
-// RUN: hmaptool write %S/Inputs/framework-public-includes-private/a.hmap.json 
%t/a.hmap
-// RUN: hmaptool write %S/Inputs/framework-public-includes-private/z.hmap.json 
%t/z.hmap
+// RUN: %hmaptool write 
%S/Inputs/framework-public-includes-private/a.hmap.json %t/a.hmap
+// RUN: %hmaptool write 
%S/Inputs/framework-public-includes-private/z.hmap.json %t/z.hmap
 
 // RUN: sed -e "s:TEST_DIR:%S/Inputs/framework-public-includes-private:g" \
 // RUN:   %S/Inputs/framework-public-includes-private/z.yaml > %t/z.yaml


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

r335542 - Warning for framework include violation from Headers to PrivateHeaders

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Mon Jun 25 15:24:17 2018
New Revision: 335542

URL: http://llvm.org/viewvc/llvm-project?rev=335542&view=rev
Log:
Warning for framework include violation from Headers to PrivateHeaders

Framework vendors usually layout their framework headers in the
following way:

Foo.framework/Headers -> "public" headers
Foo.framework/PrivateHeader -> "private" headers

Since both headers in both directories can be found with #import
, it's easy to make mistakes and include headers in
Foo.framework/PrivateHeader from headers in Foo.framework/Headers, which
usually configures a layering violation on Darwin ecosystems. One of the
problem this causes is dep cycles when modules are used, since it's very
common for "private" modules to include from the "public" ones; adding
an edge the other way around will trigger cycles.

Add a warning to catch those cases such that:

./A.framework/Headers/A.h:1:10: warning: public framework header includes 
private framework header 'A/APriv.h'
#include 
 ^

rdar://problem/38712182

Added:
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Headers/A.h

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.modulemap

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/Modules/module.private.modulemap

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv.h

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/A.framework/PrivateHeaders/APriv2.h
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/a.hmap.json

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.h

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.modulemap

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/Z.private.modulemap

cfe/trunk/test/Modules/Inputs/framework-public-includes-private/flat-header-path/ZPriv.h
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.hmap.json
cfe/trunk/test/Modules/Inputs/framework-public-includes-private/z.yaml
cfe/trunk/test/Modules/framework-public-includes-private.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/lib/Lex/HeaderSearch.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=335542&r1=335541&r2=335542&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Mon Jun 25 15:24:17 2018
@@ -32,6 +32,8 @@ def Availability : DiagGroup<"availabili
 def Section : DiagGroup<"section">;
 def AutoImport : DiagGroup<"auto-import">;
 def FrameworkHdrQuotedInclude : 
DiagGroup<"quoted-include-in-framework-header">;
+def FrameworkIncludePrivateFromPublic :
+  DiagGroup<"framework-include-private-from-public">;
 def CXX14BinaryLiteral : DiagGroup<"c++14-binary-literal">;
 def CXXPre14CompatBinaryLiteral : 
DiagGroup<"c++98-c++11-compat-binary-literal">;
 def GNUBinaryLiteral : DiagGroup<"gnu-binary-literal">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=335542&r1=335541&r2=335542&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Mon Jun 25 15:24:17 2018
@@ -718,6 +718,9 @@ def warn_quoted_include_in_framework_hea
   "double-quoted include \"%0\" in framework header, "
   "expected angle-bracketed instead"
   >, InGroup, DefaultIgnore;
+def warn_framework_include_private_from_public : Warning<
+  "public framework header includes private framework header '%0'"
+  >, InGroup;
 
 def warn_auto_module_import : Warning<
   "treating #%select{include|import|include_next|__include_macros}0 as an "

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=335542&r1=335541&r2=335542&view=diff
==
--- cfe/trunk/lib/

Re: r335375 - Re-apply: Warning for framework headers using double quote includes

[Bruno Cardoso Lopes via cfe-commits]

Hi Nico,

> Why not enable it by default, or put it in -Wall at least? We don't like 
> off-by-default warnings :-)

Thanks for double checking. There's an explanation/discussion in the
review: https://reviews.llvm.org/D47157, let me know if you have
additional questions.

Cheers,

-- 
Bruno Cardoso Lopes
http://www.brunocardoso.cc
___
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits

r335375 - Re-apply: Warning for framework headers using double quote includes

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Fri Jun 22 11:05:17 2018
New Revision: 335375

URL: http://llvm.org/viewvc/llvm-project?rev=335375&view=rev
Log:
Re-apply: Warning for framework headers using double quote includes

Introduce -Wquoted-include-in-framework-header, which should fire a warning
whenever a quote include appears in a framework header and suggest a fix-it.
For instance, for header A.h added in the tests, this is how the warning looks
like:

./A.framework/Headers/A.h:2:10: warning: double-quoted include "A0.h" in 
framework header, expected angle-bracketed instead 
[-Wquoted-include-in-framework-header]
#include "A0.h"
 ^~
 
./A.framework/Headers/A.h:3:10: warning: double-quoted include "B.h" in 
framework header, expected angle-bracketed instead 
[-Wquoted-include-in-framework-header]
#include "B.h"
 ^
 

This helps users to prevent frameworks from using local headers when in fact
they should be targetting system level ones.

The warning is off by default.

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

rdar://problem/37077034

Added:
cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A.h
cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Headers/A0.h

cfe/trunk/test/Modules/Inputs/double-quotes/A.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/double-quotes/B.h
cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Headers/X.h

cfe/trunk/test/Modules/Inputs/double-quotes/X.framework/Modules/module.modulemap
cfe/trunk/test/Modules/Inputs/double-quotes/a.hmap.json
cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.h
cfe/trunk/test/Modules/Inputs/double-quotes/flat-header-path/Z.modulemap
cfe/trunk/test/Modules/Inputs/double-quotes/x.hmap.json
cfe/trunk/test/Modules/Inputs/double-quotes/z.yaml
cfe/trunk/test/Modules/double-quotes.m
Modified:
cfe/trunk/include/clang/Basic/DiagnosticGroups.td
cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
cfe/trunk/lib/Lex/HeaderSearch.cpp

Modified: cfe/trunk/include/clang/Basic/DiagnosticGroups.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticGroups.td?rev=335375&r1=335374&r2=335375&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticGroups.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticGroups.td Fri Jun 22 11:05:17 2018
@@ -31,6 +31,7 @@ def AutoDisableVptrSanitizer : DiagGroup
 def Availability : DiagGroup<"availability">;
 def Section : DiagGroup<"section">;
 def AutoImport : DiagGroup<"auto-import">;
+def FrameworkHdrQuotedInclude : 
DiagGroup<"quoted-include-in-framework-header">;
 def CXX14BinaryLiteral : DiagGroup<"c++14-binary-literal">;
 def CXXPre14CompatBinaryLiteral : 
DiagGroup<"c++98-c++11-compat-binary-literal">;
 def GNUBinaryLiteral : DiagGroup<"gnu-binary-literal">;

Modified: cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td?rev=335375&r1=335374&r2=335375&view=diff
==
--- cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td (original)
+++ cfe/trunk/include/clang/Basic/DiagnosticLexKinds.td Fri Jun 22 11:05:17 2018
@@ -714,6 +714,11 @@ def warn_mmap_redundant_export_as : Warn
 def err_mmap_submodule_export_as : Error<
   "only top-level modules can be re-exported as public">;
 
+def warn_quoted_include_in_framework_header : Warning<
+  "double-quoted include \"%0\" in framework header, "
+  "expected angle-bracketed instead"
+  >, InGroup, DefaultIgnore;
+
 def warn_auto_module_import : Warning<
   "treating #%select{include|import|include_next|__include_macros}0 as an "
   "import of module '%1'">, InGroup, DefaultIgnore;

Modified: cfe/trunk/lib/Lex/HeaderSearch.cpp
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Lex/HeaderSearch.cpp?rev=335375&r1=335374&r2=335375&view=diff
==
--- cfe/trunk/lib/Lex/HeaderSearch.cpp (original)
+++ cfe/trunk/lib/Lex/HeaderSearch.cpp Fri Jun 22 11:05:17 2018
@@ -621,6 +621,59 @@ static const char *copyString(StringRef
   return CopyStr;
 }
 
+static bool isFrameworkStylePath(StringRef Path,
+ SmallVectorImpl &FrameworkName) {
+  using namespace llvm::sys;
+  path::const_iterator I = path::begin(Path);
+  path::const_iterator E = path::end(Path);
+
+  // Detect different types of framework style paths:
+  //
+  //   ...Foo.framework/{Headers,PrivateHeaders}
+  //   ...Foo.framework/Versions/{A,Current}/{Headers,PrivateHeaders}
+  //   ...Foo.framework/Frameworks/Nested.framework/{Headers,PrivateHeaders}
+  //   ...
+  //
+  // and some other variations among these lines.
+  int FoundComp = 0;
+  while (I != E) {
+if (I->endswith(".framework")) {
+  Framewo

Re: r335330 - [hmaptool] Turn %hmaptool into a proper substitution

[Bruno Cardoso Lopes via cfe-commits]

Thanks Ben!
On Fri, Jun 22, 2018 at 2:51 AM Benjamin Kramer via cfe-commits
 wrote:
>
> Author: d0k
> Date: Fri Jun 22 02:46:40 2018
> New Revision: 335330
>
> URL: http://llvm.org/viewvc/llvm-project?rev=335330&view=rev
> Log:
> [hmaptool] Turn %hmaptool into a proper substitution
>
> This is still super ugly, but at least it doesn't require working
> directories to just line up perfectly for python to find the tool.
>
> Modified:
> cfe/trunk/test/Modules/crash-vfs-headermaps.m
> cfe/trunk/test/Preprocessor/headermap-rel.c
> cfe/trunk/test/Preprocessor/headermap-rel2.c
> cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c
> cfe/trunk/test/lit.cfg.py
>
> Modified: cfe/trunk/test/Modules/crash-vfs-headermaps.m
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/crash-vfs-headermaps.m?rev=335330&r1=335329&r2=335330&view=diff
> ==
> --- cfe/trunk/test/Modules/crash-vfs-headermaps.m (original)
> +++ cfe/trunk/test/Modules/crash-vfs-headermaps.m Fri Jun 22 02:46:40 2018
> @@ -3,7 +3,7 @@
>  // RUN: rm -rf %t
>  // RUN: mkdir -p %t/m %t/i/Foo.framework/Headers
>  // RUN: echo '// Foo.h' > %t/i/Foo.framework/Headers/Foo.h
> -// RUN: '%python' hmaptool write 
> %S/../Preprocessor/Inputs/headermap-rel/foo.hmap.json %t/i/foo.hmap
> +// RUN: %hmaptool write 
> %S/../Preprocessor/Inputs/headermap-rel/foo.hmap.json %t/i/foo.hmap
>
>  // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
>  // RUN: %clang -fsyntax-only -fmodules -fmodules-cache-path=%t/m %s \
>
> Modified: cfe/trunk/test/Preprocessor/headermap-rel.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/headermap-rel.c?rev=335330&r1=335329&r2=335330&view=diff
> ==
> --- cfe/trunk/test/Preprocessor/headermap-rel.c (original)
> +++ cfe/trunk/test/Preprocessor/headermap-rel.c Fri Jun 22 02:46:40 2018
> @@ -1,5 +1,5 @@
>  // RUN: rm -f %t.hmap
> -// RUN: '%python' hmaptool write %S/Inputs/headermap-rel/foo.hmap.json 
> %t.hmap
> +// RUN: %hmaptool write %S/Inputs/headermap-rel/foo.hmap.json %t.hmap
>  // RUN: %clang_cc1 -E %s -o %t.i -I %t.hmap -F %S/Inputs/headermap-rel
>  // RUN: FileCheck %s -input-file %t.i
>
>
> Modified: cfe/trunk/test/Preprocessor/headermap-rel2.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/headermap-rel2.c?rev=335330&r1=335329&r2=335330&view=diff
> ==
> --- cfe/trunk/test/Preprocessor/headermap-rel2.c (original)
> +++ cfe/trunk/test/Preprocessor/headermap-rel2.c Fri Jun 22 02:46:40 2018
> @@ -1,5 +1,5 @@
>  // RUN: rm -f %t.hmap
> -// RUN: '%python' hmaptool write 
> %S/Inputs/headermap-rel2/project-headers.hmap.json %t.hmap
> +// RUN: %hmaptool write %S/Inputs/headermap-rel2/project-headers.hmap.json 
> %t.hmap
>  // RUN: %clang_cc1 -v -fsyntax-only %s -iquote %t.hmap -isystem 
> %S/Inputs/headermap-rel2/system/usr/include -I %S/Inputs/headermap-rel2 -H
>  // RUN: %clang_cc1 -fsyntax-only %s -iquote %t.hmap -isystem 
> %S/Inputs/headermap-rel2/system/usr/include -I %S/Inputs/headermap-rel2 -H 2> 
> %t.out
>  // RUN: FileCheck %s -input-file %t.out
>
> Modified: cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c?rev=335330&r1=335329&r2=335330&view=diff
> ==
> --- cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c (original)
> +++ cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c Fri Jun 22 
> 02:46:40 2018
> @@ -1,5 +1,5 @@
>  // RUN: rm -f %t.hmap
> -// RUN: '%python' hmaptool write %S/Inputs/nonportable-hmaps/foo.hmap.json 
> %t.hmap
> +// RUN: %hmaptool write %S/Inputs/nonportable-hmaps/foo.hmap.json %t.hmap
>  // RUN: %clang_cc1 -Eonly\
>  // RUN:   -I%t.hmap \
>  // RUN:   -I%S/Inputs/nonportable-hmaps  \
>
> Modified: cfe/trunk/test/lit.cfg.py
> URL: 
> http://llvm.org/viewvc/llvm-project/cfe/trunk/test/lit.cfg.py?rev=335330&r1=335329&r2=335330&view=diff
> ==
> --- cfe/trunk/test/lit.cfg.py (original)
> +++ cfe/trunk/test/lit.cfg.py Fri Jun 22 02:46:40 2018
> @@ -58,7 +58,7 @@ tool_dirs = [config.clang_tools_dir, con
>
>  tools = [
>  'c-index-test', 'clang-check', 'clang-diff', 'clang-format', 
> 'clang-tblgen',
> -'opt', 'hmaptool',
> +'opt',
>  ToolSubst('%clang_func_map', command=FindTool(
>  'clang-func-mapping'), unresolved='ignore'),
>  ]
> @@ -69,6 +69,10 @@ if config.clang_examples:
>
>  llvm_config.add_tool_substitutions(tools, tool_dirs)
>
> +config.substitutions.append(
> +('%hmaptool', '%s %s' % (config.python_executable,
> +  

r335295 - Re-apply: Add python tool to dump and construct header maps

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Thu Jun 21 14:45:24 2018
New Revision: 335295

URL: http://llvm.org/viewvc/llvm-project?rev=335295&view=rev
Log:
Re-apply: Add python tool to dump and construct header maps

Header maps are binary files used by Xcode, which are used to map
header names or paths to other locations. Clang has support for
those since its inception, but there's not a lot of header map
testing around.

Since it's a binary format, testing becomes pretty much brittle
and its hard to even know what's inside if you don't have the
appropriate tools.

Add a python based tool that allows creating and dumping header
maps based on a json description of those. While here, rewrite
tests to use the tool and remove the binary files from the tree.

This tool was initially written by Daniel Dunbar.

Thanks to Stella Stamenova for helping make this work on Windows.

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

rdar://problem/39994722

Added:
cfe/trunk/test/Preprocessor/Inputs/headermap-rel/foo.hmap.json
cfe/trunk/test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap.json
cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap.json
cfe/trunk/utils/hmaptool/CMakeLists.txt
cfe/trunk/utils/hmaptool/hmaptool   (with props)
Removed:
cfe/trunk/test/Preprocessor/Inputs/headermap-rel/foo.hmap
cfe/trunk/test/Preprocessor/Inputs/headermap-rel2/project-headers.hmap
cfe/trunk/test/Preprocessor/Inputs/nonportable-hmaps/foo.hmap
Modified:
cfe/trunk/CMakeLists.txt
cfe/trunk/test/CMakeLists.txt
cfe/trunk/test/Modules/crash-vfs-headermaps.m
cfe/trunk/test/Preprocessor/headermap-rel.c
cfe/trunk/test/Preprocessor/headermap-rel2.c
cfe/trunk/test/Preprocessor/nonportable-include-with-hmap.c
cfe/trunk/test/lit.cfg.py

Modified: cfe/trunk/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/CMakeLists.txt?rev=335295&r1=335294&r2=335295&view=diff
==
--- cfe/trunk/CMakeLists.txt (original)
+++ cfe/trunk/CMakeLists.txt Thu Jun 21 14:45:24 2018
@@ -753,6 +753,7 @@ endif()
 if (LLVM_ADD_NATIVE_VISUALIZERS_TO_SOLUTION)
   add_subdirectory(utils/ClangVisualizers)
 endif()
+add_subdirectory(utils/hmaptool)
 
 configure_file(
   ${CLANG_SOURCE_DIR}/include/clang/Config/config.h.cmake

Modified: cfe/trunk/test/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/CMakeLists.txt?rev=335295&r1=335294&r2=335295&view=diff
==
--- cfe/trunk/test/CMakeLists.txt (original)
+++ cfe/trunk/test/CMakeLists.txt Thu Jun 21 14:45:24 2018
@@ -54,6 +54,7 @@ list(APPEND CLANG_TEST_DEPS
   clang-rename
   clang-refactor
   clang-diff
+  hmaptool
   )
   
 if(CLANG_ENABLE_STATIC_ANALYZER)

Modified: cfe/trunk/test/Modules/crash-vfs-headermaps.m
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Modules/crash-vfs-headermaps.m?rev=335295&r1=335294&r2=335295&view=diff
==
--- cfe/trunk/test/Modules/crash-vfs-headermaps.m (original)
+++ cfe/trunk/test/Modules/crash-vfs-headermaps.m Thu Jun 21 14:45:24 2018
@@ -1,15 +1,9 @@
 // REQUIRES: crash-recovery, shell, system-darwin
 
-// This uses a headermap with this entry:
-//   Foo.h -> Foo/Foo.h
-
-// Copy out the headermap from test/Preprocessor/Inputs/headermap-rel and avoid
-// adding another binary format to the repository.
-
 // RUN: rm -rf %t
-// RUN: mkdir -p %t/m
-// RUN: cp -a %S/../Preprocessor/Inputs/headermap-rel %t/i
+// RUN: mkdir -p %t/m %t/i/Foo.framework/Headers
 // RUN: echo '// Foo.h' > %t/i/Foo.framework/Headers/Foo.h
+// RUN: '%python' hmaptool write 
%S/../Preprocessor/Inputs/headermap-rel/foo.hmap.json %t/i/foo.hmap
 
 // RUN: not env FORCE_CLANG_DIAGNOSTICS_CRASH= TMPDIR=%t TEMP=%t TMP=%t \
 // RUN: %clang -fsyntax-only -fmodules -fmodules-cache-path=%t/m %s \

Removed: cfe/trunk/test/Preprocessor/Inputs/headermap-rel/foo.hmap
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/Inputs/headermap-rel/foo.hmap?rev=335294&view=auto
==
Binary files cfe/trunk/test/Preprocessor/Inputs/headermap-rel/foo.hmap 
(original) and cfe/trunk/test/Preprocessor/Inputs/headermap-rel/foo.hmap 
(removed) differ

Added: cfe/trunk/test/Preprocessor/Inputs/headermap-rel/foo.hmap.json
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/test/Preprocessor/Inputs/headermap-rel/foo.hmap.json?rev=335295&view=auto
==
--- cfe/trunk/test/Preprocessor/Inputs/headermap-rel/foo.hmap.json (added)
+++ cfe/trunk/test/Preprocessor/Inputs/headermap-rel/foo.hmap.json Thu Jun 21 
14:45:24 2018
@@ -0,0 +1,6 @@
+{
+  "mappings" :
+{
+ "Foo.h" : "Foo/Foo.h"
+}
+}

Removed: cfe/trunk/test/Preprocessor/Inputs/headermap-rel2/project-heade

r335194 - Revert "Fix hmaptool cmake file to work on Windows"

[Bruno Cardoso Lopes via cfe-commits]

Author: bruno
Date: Wed Jun 20 18:23:42 2018
New Revision: 335194

URL: http://llvm.org/viewvc/llvm-project?rev=335194&view=rev
Log:
Revert "Fix hmaptool cmake file to work on Windows"

This reverts commit 63711c3cd337a0d22617579a904af07481139611, due to
breaking bots:

http://lab.llvm.org:8011/builders/clang-x64-ninja-win7/builds/11315
http://lab.llvm.org:8011/builders/llvm-clang-x86_64-expensive-checks-win/builds/10411/steps/test-check-all/logs/stdio

Modified:
cfe/trunk/utils/hmaptool/CMakeLists.txt

Modified: cfe/trunk/utils/hmaptool/CMakeLists.txt
URL: 
http://llvm.org/viewvc/llvm-project/cfe/trunk/utils/hmaptool/CMakeLists.txt?rev=335194&r1=335193&r2=335194&view=diff
==
--- cfe/trunk/utils/hmaptool/CMakeLists.txt (original)
+++ cfe/trunk/utils/hmaptool/CMakeLists.txt Wed Jun 20 18:23:42 2018
@@ -1,14 +1,14 @@
 set(CLANG_HMAPTOOL hmaptool)
 
-add_custom_command(OUTPUT 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL}
+add_custom_command(OUTPUT ${CMAKE_BINARY_DIR}/bin/${CLANG_HMAPTOOL}
COMMAND ${CMAKE_COMMAND} -E make_directory
- ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin
+ ${CMAKE_BINARY_DIR}/bin
COMMAND ${CMAKE_COMMAND} -E copy
  ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL}
- ${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/
+ ${CMAKE_BINARY_DIR}/bin/
DEPENDS ${CMAKE_CURRENT_SOURCE_DIR}/${CLANG_HMAPTOOL})
 
-list(APPEND Depends 
${CMAKE_BINARY_DIR}/${CMAKE_CFG_INTDIR}/bin/${CLANG_HMAPTOOL})
+list(APPEND Depends ${CMAKE_BINARY_DIR}/bin/${CLANG_HMAPTOOL})
 install(PROGRAMS ${CLANG_HMAPTOOL} DESTINATION bin)
 
 add_custom_target(hmaptool ALL DEPENDS ${Depends})


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