[clang] [clang][Sema] Guard diagnose_if value-dependent conditions (PR #197647)

[Aaron Ballman via cfe-commits]

================
@@ -7801,8 +7801,12 @@ bool Sema::diagnoseArgDependentDiagnoseIfAttrs(const FunctionDecl *Function,
         // It's sane to use the same Args for any redecl of this function, since
         // EvaluateWithSubstitution only cares about the position of each
         // argument in the arg list, not the ParmVarDecl* it maps to.
-        if (!DIA->getCond()->EvaluateWithSubstitution(
-                Result, Context, cast<FunctionDecl>(DIA->getParent()), Args, ThisArg))
+        // FIXME: This doesn't consider value-dependent cases, because doing so
+        // is very difficult. Ideally, we should handle them more gracefully.
+        if (DIA->getCond()->isValueDependent() ||
----------------
AaronBallman wrote:

I was trying to figure out whether type dependence matters here or not, so I looked at this a bit more.

We're already doing that two-step dance here. e.g., we don't get into `Sema::diagnoseArgDependentDiagnoseIfAttrs()` for the primary template, only when instantiating it.

The issue boils down to use of `-ferror-limit`; that results in `TreeTransform` having nodes which *remain* dependent because of the error dependency bit. IIRC that's theoretically fine because we can still end up with dependencies in non-error cases (like partial specializations), but I wonder if we're going to have other similar issues with other constructs -- we don't test with low error limits in general. That said, it's not clear to me whether this change is correct -- should we be looking for error dependence specifically? Or can we devise a test case that fails without error dependence?

CC @erichkeane @mizvekov for opinions.


https://github.com/llvm/llvm-project/pull/197647