Thanks for adding this!

In D139069#3999732, @int3 wrote:

Doesn't look like I have permissions to see that link. Could you share it?

We have been running into some issues too that blames to this, mostly around Swift-related stuff. Don't have a small repro yet though, so a small failing test would be helpful

I guess we should revert this for now

FYI, this change caused an iOS/arm64 test in Chromium to crash (https://bugs.chromium.org/p/chromium/issues/detail?id=1400716). Will try to investigate more, though.

ping @efriedma, do you mind looking at this again? thanks!

Looks good to me!

Address comments

add to test case, modify name mangling, change fn info opts enum type

In D137872#3925723, @efriedma wrote:

I'm not quite sure I understand what's happening here. Does this actually avoid generating two copies of the function body if both the call operator and the conversion are used?

cleanup

Clean up existing code and add code to make the call operator also call the new function.

In D136998#3926368, @efriedma wrote:

In D136998#3926321, @rnk wrote:

In D136998#3906874, @efriedma wrote:

Should we try to use this codepath for variadic lambdas as well?

Yes!

Do we want to try to unify our cloning code? CodeGenFunction::GenerateVarArgsThunk has code doing something similar. (It's at least worth comparing to see if you're doing something significantly different...)

Good idea

In D136998#3906881, @efriedma wrote:

Might also be worth considering if we can avoid cloning here. It should be possible to emit the lambda body into a separate function with a calling convention of your choice, and make both the call operator and the static invoker call it.

This would be nice. I wasn't able to provide guidance on how to do that, and I think Amy was struggling to synthesize a new method (__invoke, __impl) at the AST layer.

I think you wouldn't actually synthesize it at all in the AST.

Basically, the follow code changes:

1. Change the mangling of the function that contains the actual lambda body.
2. Make that function use an alternate calling convention that doesn't involve inalloca etc.
3. Synthesize thunks to represent the actual call operator and static invoker.

This is similar to the way virtual overriding works: there's an actual function, but there are also alternate entry points that end up in the vtables.

Fix calling convention of cloned function.

In D136998#3906881, @efriedma wrote:

Might also be worth considering if we can avoid cloning here. It should be possible to emit the lambda body into a separate function with a calling convention of your choice, and make both the call operator and the static invoker call it.

Move cloning code into a function.

moved some stuff around

update

update

update

Add another debug info strategy to the description

Move ctor homing info to fstandalone-debug.

Actually, maybe I should add some of this info to the -fstandalone-debug section.

Remove documentation paragraph.

In D136188#3866266, @dblaikie wrote:

(not sure whether to discuss this here or on the bug)

Should we remove the documentation? Part of the principles of "cc1" flags is that their implementation details, not to be publicly used and maybe not to be publicly documented either?

In D133457#3832590, @mstorsjo wrote:

@akhuang I noticed that https://github.com/llvm/llvm-project/blob/main/clang/lib/Driver/ToolChains/MSVC.cpp#L199-L200 has got an explicit check for Args.hasArg(options::OPT__SLASH_MD, options::OPT__SLASH_MDd) - I think that this would need to be amended to handle these cases too - or is that inferred from here somehow?

Fix flag name in test case

Change name of flag, and alias /Zl to this flag.

In D133959#3793283, @hans wrote:

Could the new flag be marked as an Alias in the tablegen, so that we don't need to do any code changes for it?

Commands like clang -funwind-tables -fsanitize=thread t.cc now result in an unused command line argument warning. Is this an intended behavior change?

ran clang-format

more comments

Address comments, more cleanup

Clean up test, add doc brief to new flag, try to put the flag logic in a separate function

We're seeing this warning in code with global constants, e.g.

In D81950#3692893, @nikic wrote:

The test-columns.pdb file added here is 6MB large. Is it possible to reduce its size while preserving the spirit of the test?

Looks good, thanks!

In D110450#3653404, @aganea wrote:

Hello @akhuang! Do you think I can land this on your behalf? I've already rebased & tested locally.

Hi, this caused a crash in chromium builds - here's a c-reduced repro

int a;
void b() {
  int c = 16 >> a;
  short *d;
  long e;
  short *f = e = 0;
  for (; e < c; e++)
    *f++ = d[0];
}

This also fixes the crashes in the chromium build, thanks!

I saw another crash on chromium builds - not sure if this is related to the previous mentioned crash but here's a creduced repro:

struct f {
  float g;
  float h;
};
struct j {
  j() = default;
  j(float k, float l) : c(k), d(l) {}
  j operator*(j k) const { return j(k.a + c, b + k.d); }
  float a = 1.0f;
  float b = 0.0f;
  float c = 0.0f;
  float d = 1.0f;
  float e = 0.0f;
};
struct m {
  j n() const;
  f o;
  j p;
};
j m::n() const {
  if (o.g || o.h)
    return j();
  j a;
  return p * a;
}

build with

clang -cc1 -O2 -vectorize-slp -emit-llvm -fno-delete-null-pointer-checks t.cpp

Oh, I see, looks good then!

I think this looks good, although I don't entirely remember how these work-- what's the situation where the "current line offset is actually for next range"?

some more ctx.config -> config->

config. to config->

further cleanup

cleanup and fix stuff that got messed during rebase

Address comments

@aganea Ah, right, I can take a look at the comments today

Hi, this change caused clang to crash in chromium arm builds; could you take a look?

Add std=c++17 to the test.

In D116256#3215801, @thakis wrote:

Thanks for the patch! This looks roughly right to me.

Maybe the list of ESTs that are allowed to be mismatched should be opt-in instead of opt-out? (i.e. instead of checking for "not EST_BasicNoexcept / EST_DependentNoexcept", check for EST_NoThrow (I think?) Not sure which way is better.

(Looks like the old code was added for https://llvm.org/PR25265)

Could you check that we still emit the warning on line 5 in https://godbolt.org/z/bxfx8jsjd ? The test is mostly from https://docs.microsoft.com/en-us/cpp/build/reference/zc-noexcepttypes?view=msvc-170 -- it feels like we might want to have different behavior in c++17 (and later) and c++14 (and earlier) for some of the diags, possibly. Looks like MSVC also has an error on line 15 by default (with /std:c++17, it seems to accept it with /std:c++14), while we only warn.

(I'm a bit surprised the noexcept bit doesn't make it into the mangled name in the ms abi, but we're consistent with msvc about this so that's all good.)

Fix warning behavior

lgtm!

lgtm!

I think on Windows the RemoveFileOnSignal path doesn't always work when the process is killed, which causes temp files to be left sometimes (see https://reviews.llvm.org/D102736)

This also appears to fix another llvm bug filed on a windows ram drive issue (https://bugs.llvm.org/show_bug.cgi?id=52080), which is cool.

add test