Thanks, let's go with this for now.

I think we need to take this to the C++ committee. For example, consider a case like:

I think this is the wrong solution; I think the assertion is correct and is observing a problem elsewhere. Imagine if we had something like:

This will diagnose unused values in unreachable code in constant-evaluated contexts; that doesn't seem quite right. For example, in:

Looks good to me.

I think this is worth doing -- "rvalue" is at least ambiguous and, in C++-specific cases, confusing and wrong. Saying "prvalue" in C-specific parts of clang may also be a bit surprising, but it's unambiguous and still meaningful.

@ldionne How should we go about establishing whether libc++ would be prepared to officially support CUDA? Right now, Clang's CUDA support is patching in attributes onto libc++ functions from the outside, which doesn't seem like a sustainable model.

Should we also warn about inline variables in anonymous namespaces?

In D102689#2778011, @rjmccall wrote:

The C++ standard does not appear to have similar wording. On the other hand, the C++ standard says that e.g. "The result of the expression (T) cast-expression is of type T", and similarly for the other casts, which is clearly just wrong if T is a reference type; the wording clarifies that the expression is an l-value or x-value if the type is a reference but doesn't remove the reference-ness of the expression type as it must, unless that's done by some other clause at a distance.

In D102567#2767539, @rriddle wrote:

I think we'll still need to revert though.

I don't think out-of-tree experiments on possibilities for move semantics are especially motivating for this, one way or the other, but I do think it would be nice to make some kind of change here.

Thanks, nice catch!

The undef problem seems pretty fundamental here, and I don't think there's an easy fix. Let's consider that separately from this patch.

In D102367#2755587, @nikic wrote:

Can you explain why UndefValue needs to be excluded? It's not mentioned in the summary, and the test case does not cover it.

(Reopening, earlier commit was accidental.)

(No blocking concerns here.)

Generally-speaking, we have a plan that I'm happy for us to work towards, and I'm happy for our progress towards that plan to be incremental. Even though this might not be fully in that direction right now, I think that's OK.

Thanks, this looks good to me!

Thanks, I'm broadly very happy with this. My remaining comments are all very minor.

This change is not correct. The language rules only apply to particular syntactic forms, so we need to check for those exact syntactic forms here.

In D101598#2729865, @ldionne wrote:

Just for posterity, what we discussed is that since there is a list of addressable functions in the standard, we should explore adding a warning to Clang that fires whenever somebody takes the address of a function in namespace std, except if it's an addressable function. That list would either be maintained explicitly in Clang, or, preferably, we'd have an attribute that we can mark addressable functions with so that the compiler doesn't flag those specific functions.

When this change returns, I'd like to see a different implementation strategy. Running a recursive AST visitation after the fact is generally not the right way to look for this kind of issue; adding an extra pass to speculatively hunt for each kind of warning we might want to issue is too expensive. Instead, I think we should do something simpler and cheaper, such as tracking, for each variable in scope, the number of potentially-evaluated non-assignment references to that variable. For example:

Note that, per https://clang.llvm.org/docs/InternalsManual.html#the-format-string,

In D100465#2695691, @rsmith wrote:

Then remove the workarounds as (hopefully now entirely uncontroversial) follow-up steps.

From a process standpoint, I think the best thing would be to first send out a patch adding a release note saying we no longer support libstdc++ < 4.8.3 and updating the part of docs/Toolchain.rst that Aaron linked to, and send a brief note to cfe-dev drawing people's attention to that patch and asking for objections -- I don't think you need to wait for more than a couple of days for such objections, but as this is effectively a policy change we should probably give people a little time to respond. Then remove the workarounds as (hopefully now entirely uncontroversial) follow-up steps.

I think we should decide what version of libstdc++ we're prepared to support and document that somewhere, and it would make sense to remove the various libstdc++ compatibility changes that target older versions. Google still has builds that use Clang trunk, recent glibc, and libstdc++ 4.9, in >=C++14 mode, though (which I think is the combination that needs this workaround), so I'm not sure that the world is ready for Clang's minimum to be increased to 6.3 or later. (Of course Google could locally patch their libstdc++, but if there's one organization building in this mode still, it seems likely that there are others, and in particular version 4.9 was the last version prior to the introduction of the dual ABI, so there may be reasons people want to stop there in particular. I think at least the need to support libstdc++ 4.2 for Apple targets has disappeared now, though, so it's possible that we could drop support for 4.8 and earlier?)

In D99517#2693418, @thakis wrote:

Looks like this breaks tests on mac/arm: http://45.33.8.238/macm1/7552/step_7.txt

Functionally this looks good to me. I've suggested some minor cleanups and I understand you're doing some wordsmithing on the diagnostics; I think once those are complete this will be ready to land. Thank you!

Exposing this information seems fine to me. I think it'd be more useful to expose it in a way the preprocessor can inspect, but it's hard to see how to do that without requiring us to invent identifier-shaped names for all encodings, and perhaps we shouldn't be in the business of doing that. If this string literal approach addresses the use cases you have, then I think that's OK.

In D99790#2681487, @lebedev.ri wrote:

I think it should be fixed in CodeGenVTables::maybeEmitThunk(), likely in arrangeGlobalDeclaration().
Or is this wrong on the AST level, too?

Thanks, I think this makes sense as a generalization of the existing "idiomatic aggregate initialization" rule. Some suggested cleanups but otherwise LGTM.

LGTM

Sorry for the late review.

This is somewhat related to the following RFC by @rjmccall:
https://lists.llvm.org/pipermail/llvm-dev/2016-January/094012.html

In D99893#2672563, @cor3ntin wrote:

This would solve some of the issues (perfect code-gen / no body instantiation), but the declarations are still instantiated of course.
And I have a few folks asking me whether I could get rid of it. Unfortunately, I think the goals of preserving the AST as is today and getting rid of these template instantiations are contradictory.