[analyzer][UninitializedObjectChecker] New flag to ignore guarded uninitialized fields

[Szelethus]

Summary

This patch is an implementation of the ideas discussed on the mailing list. It didn't cause any crashes on the already existing test (I checked it by adding the flag to them and run lit), but then again, I wasn't able to check it on larger code bases just yet, and LLVM tends to have a couple tricks up it's sleeve, so be beware of that.

[Szelethus]

@george.karpenkov @xazax.hun, is this how you imagined checking whether an access is guarded?

[george.karpenkov]

Thanks! The usual question: would it be easier to implement using AST matchers?

[Szelethus]

I played around with it, and this seemed a bit cleaner. Since I would only match against the definition of the record (and the definition of its methods), the matcher would need to look like this: stmt(hasDescendant(blahBlah(...))), and I'd only be able to retrieve (with my limited knowledge) the entire method definition, not the actual FieldDecl in the case of a match.
I didn'd do a whole lot of AST based analysis, so feel free to correct me on this one ^-^

[NoQ]

You can retrieve any node from the ASTMatcher by .bind()ing sub-matchers to string keys and later retrieving them from the MatchResult dictionary by those keys. It's like a regex capturing groups (and, well, ASTMatchers also support backreferences to such groups, i.e. equalsBoundNode()).

[Szelethus]

Reimplemented with AST matchers.
@george.karpenkov I know you wanted to test this feature for a while, but sadly I've been busy with the macro expansion related projects, I hope it's still relevant.

[xazax.hun]

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

501 ↗

(On Diff #168592)

I would consider any access that is guarded by if, ternary operator, switch as a guard.

For example I would consider field guarded in the following method:

int *method() {
  return cond ? field : nullptr;
}

But unguarded in this:

int *method() {
  return field ? field2 : nullptr;
}

Also, knowing that a variable is guarded or not might be a useful heuristic for other checks too. So if you do plan to cover all those cases. I would recommend to implement it outside of this check so other checks might also leverage this functionality.

[xazax.hun]

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

519 ↗

(On Diff #168592)

I am not sure if this is a reliable way to check if the access is before the guard.

Consider:

switch(x): {
   case 2: guard; access; break;
   case 1: access break;
}

Here, we have no particular ordering between the access in case 1 and the guard in case 2 at runtime. But relying on the source locations we might come to the false conclusion that there is. Loops, gotos can cause similar problems.
I do understand that this might not be too easy to solve without traversing the cfg and we might not want to do that but I think we should at least add a test/todo.

[Szelethus]

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

519 ↗

(On Diff #168592)

I am not sure if this is a reliable way to check if the access is before the guard.

I'm 100% sure it isn't. Using the CFG instead of matchers sounds like a great and difficult to implement (at least to me, as I never touched them) idea. It should get rid of the false negatives, at least in part.

[...] I think we should at least add a test/todo.

There are some :)

[george.karpenkov]

Checking for asserts makes sense, but as a rough estimate will suppress roughly zero false positives I have seen.
In general, LLVM codebase is exceptional in its use of asserts, and most projects, unfortunately, don't really know how to use them.

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

519 ↗

(On Diff #168592)

Using the CFG instead of matchers sounds like a great and difficult to implement

That would also require building a dataflow framework, which we do not have (yet)

[Szelethus]

Added new guards as recommended by @xazax.hun

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

519 ↗

(On Diff #168592)

Too bad :/ Makes me understand though why it would be valuable to implement it.

[Szelethus]

Ping

[NoQ]

Code looks good to me, but I'd wait for @george.karpenkov because it was his idea.

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

502 ↗

(On Diff #168885)

In a lot of standard libraries assert() is implemented as a macro. You might want to catch the corresponding builtin or look at macro names (not sure if we have a matcher for the latter but it should be easy to add).

[NoQ]

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

515 ↗

(On Diff #168885)

I feel that it's a good practice to assert(FirstAccess) here (same for FirstGuard and generally after every getNodeAs). The assertion will hold because "access" is always bound to on every possible match and is always a member-expression, and it'd be great to check that after the matcher is changed.

[Szelethus]

Hmm, I'll investigate the assert issue @NoQ mentioned before moving forward.

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

502 ↗

(On Diff #168885)

Hmm, maybe go for [[noreturn]] functions?

[Szelethus]

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

502 ↗

(On Diff #168885)

Unfortunately, macros are not a part of the AST, so I'm struggling a lot with this patch, but I'm making progress.

[Szelethus]

@george.karpenkov Matching macros is a very non-trivial job, how would you feel if we shipped this patch as-is, and maybe leave a TODO about adding macro asserts down the line?

[Szelethus]

@george.karpenkov Matching macros is a very non-trivial job, how would you feel if we shipped this patch as-is, and maybe leave a TODO about adding macro asserts down the line?

The only solution I saw in clang-tidy was to match binary expressions as a heuristic, and check whether they are inside a macro named assert. That is hacky at best, any objection against the current state of this patch?

I'm planning to evaluate the checker in a variety of projects after this one, see how things are looking, and push it out of alpha.

[Szelethus]

Changes are no longer planned for the reasons explained in my earlier comments.

[NoQ]

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

502 ↗

(On Diff #168885)

Maybe [[noreturn]] or just catch specific builtins such as __assert_fail - see what NoReturnFunctionChecker reacts to.

[Szelethus]

Let's try that again then! :)

[Szelethus]

• Rebased to master.
• Adding functions calls with the noreturn attribute to the guards.
• Shamelessly stole all sorts of assert-like function names from NoReturnFunctionChecker.
• Added new test cases accordingly.

This should be it! Sorry for being so slow with this -- I genuinely struggled a lot with macros before ultimately giving up.

[Szelethus]

Marking this patch as non-accepted.

[Szelethus]

Ping.

[Szelethus]

Ping^2

[Szelethus]

Ping^3. @NoQ @george.karpenkov Could you please take a look? @xazax.hun Any other objections?

[NoQ]

I have no objections. George, this was your idea, does it look good to you?

lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

577–578 ↗

(On Diff #177080)

That's quite unreliable, but in a good way. Like, in if (x) foo(); else bar();, in terms of source locations bar() goes after foo(), but in practice bar() never gets called after foo() (unless the whole thing is also in a loop). But when we're trying to heuristically suppress the specific false positive pattern in which statements definitely go in that order, it's not the end of the world when the heuristic suppresses a few other patterns.

So i think it's worth pointing out to the reader that this approach is a bit wonky, but not necessarily worth improving upon. That said, it should be possible to do this sort of stuff with a CFG-based analysis, probably even with one of the existing analyses in lib/Analysis.

[Szelethus]

If you don't mind, I'd prefer to finally get over this patch. I'll commit on the 31st, but will wait for any potential feedback 'til then.

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

 //
 //   - "CheckPointeeInitialization" (boolean). If set to false, the checker will
 //     not analyze the pointee of pointer/reference fields, and will only check
 //     whether the object itself is initialized. Defaults to false.
 //
 //     `-analyzer-config \
 //         alpha.cplusplus.UninitializedObject:CheckPointeeInitialization=true`.
 //
+//     TODO: With some clever heuristics, some pointers should be dereferenced
+//     by default. For example, if the pointee is constructed within the
+//     constructor call, it's reasonable to say that no external object
+//     references it, and we wouldn't generate multiple report on the same
+//     pointee.
+//
 //   - "IgnoreRecordsWithField" (string). If supplied, the checker will not
 //     analyze structures that have a field with a name or type name that
 //     matches the given pattern. Defaults to "".
 //
 //     `-analyzer-config \
 // alpha.cplusplus.UninitializedObject:IgnoreRecordsWithField="[Tt]ag|[Kk]ind"`.
 //
-//     TODO: With some clever heuristics, some pointers should be dereferenced
-//     by default. For example, if the pointee is constructed within the
-//     constructor call, it's reasonable to say that no external object
-//     references it, and we wouldn't generate multiple report on the same
-//     pointee.
+//   - "IgnoreGuardedFields" (boolean). If set to true, the checker will analyze
+//     _syntactically_ whether the found uninitialized object is used without a
+//     preceding assert call. Defaults to false.
+//
+//     `-analyzer-config \
+//         alpha.cplusplus.UninitializedObject:IgnoreGuardedFields=true`.
 //
 // Most of the following methods as well as the checker itself is defined in
 // UninitializedObjectChecker.cpp.
 //
 // Some methods are implemented in UninitializedPointee.cpp, to reduce the
 // complexity of the main checker file.
 //
 //===----------------------------------------------------------------------===//
 
 #ifndef LLVM_CLANG_STATICANALYZER_UNINITIALIZEDOBJECT_H
 #define LLVM_CLANG_STATICANALYZER_UNINITIALIZEDOBJECT_H
 
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 
 namespace clang {
 namespace ento {
 
 struct UninitObjCheckerOptions {
   bool IsPedantic = false;
   bool ShouldConvertNotesToWarnings = false;
   bool CheckPointeeInitialization = false;
   std::string IgnoredRecordsWithFieldPattern;
+  bool IgnoreGuardedFields = false;
 };
 
 /// A lightweight polymorphic wrapper around FieldRegion *. We'll use this
 /// interface to store addinitional information about fields. As described
 /// later, a list of these objects (i.e. "fieldchain") will be constructed and
 /// used for printing note messages should an uninitialized value be found.
 class FieldNode {
 protected:

cfe/trunk/lib/StaticAnalyzer/Checkers/UninitializedObject/UninitializedObjectChecker.cpp

 //
 // Some of the logic is implemented in UninitializedPointee.cpp, to reduce the
 // complexity of this file.
 //
 //===----------------------------------------------------------------------===//
 
 #include "clang/StaticAnalyzer/Checkers/BuiltinCheckerRegistration.h"
 #include "UninitializedObject.h"
+#include "clang/ASTMatchers/ASTMatchFinder.h"
 #include "clang/StaticAnalyzer/Core/BugReporter/BugType.h"
 #include "clang/StaticAnalyzer/Core/Checker.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/CheckerContext.h"
 #include "clang/StaticAnalyzer/Core/PathSensitive/DynamicTypeMap.h"
 
 using namespace clang;
 using namespace clang::ento;
+using namespace clang::ast_matchers;
 
 /// We'll mark fields (and pointee of fields) that are confirmed to be
 /// uninitialized as already analyzed.
 REGISTER_SET_WITH_PROGRAMSTATE(AnalyzedRegions, const MemRegion *)
 
 namespace {
 
 class UninitializedObjectChecker

 /// it multiple times).
 static bool willObjectBeAnalyzedLater(const CXXConstructorDecl *Ctor,
                                       CheckerContext &Context);
 
 /// Checks whether RD contains a field with a name or type name that matches
 /// \p Pattern.
 static bool shouldIgnoreRecord(const RecordDecl *RD, StringRef Pattern);
 
+/// Checks _syntactically_ whether it is possible to access FD from the record
+/// that contains it without a preceding assert (even if that access happens
+/// inside a method). This is mainly used for records that act like unions, like
+/// having multiple bit fields, with only a fraction being properly initialized.
+/// If these fields are properly guarded with asserts, this method returns
+/// false.
+///
+/// Since this check is done syntactically, this method could be inaccurate.
+static bool hasUnguardedAccess(const FieldDecl *FD, ProgramStateRef State);
+
 //===----------------------------------------------------------------------===//
 //                  Methods for UninitializedObjectChecker.
 //===----------------------------------------------------------------------===//
 
 void UninitializedObjectChecker::checkEndFunction(
     const ReturnStmt *RS, CheckerContext &Context) const {
 
   const auto *CtorDecl = dyn_cast_or_null<CXXConstructorDecl>(

 bool FindUninitializedFields::addFieldToUninits(FieldChainInfo Chain,
                                                 const MemRegion *PointeeR) {
   const FieldRegion *FR = Chain.getUninitRegion();
 
   assert((PointeeR || !isDereferencableType(FR->getDecl()->getType())) &&
          "One must also pass the pointee region as a parameter for "
          "dereferenceable fields!");
 
+  if (State->getStateManager().getContext().getSourceManager().isInSystemHeader(
+          FR->getDecl()->getLocation()))
+    return false;
+
+  if (Opts.IgnoreGuardedFields && !hasUnguardedAccess(FR->getDecl(), State))
+    return false;
+
   if (State->contains<AnalyzedRegions>(FR))
     return false;
 
   if (PointeeR) {
     if (State->contains<AnalyzedRegions>(PointeeR)) {
       return false;
     }
     State = State->add<AnalyzedRegions>(PointeeR);
   }
 
   State = State->add<AnalyzedRegions>(FR);
 
-  if (State->getStateManager().getContext().getSourceManager().isInSystemHeader(
-          FR->getDecl()->getLocation()))
-    return false;
-
   UninitFieldMap::mapped_type NoteMsgBuf;
   llvm::raw_svector_ostream OS(NoteMsgBuf);
   Chain.printNoteMsg(OS);
+
   return UninitFields.insert({FR, std::move(NoteMsgBuf)}).second;
 }
 
 bool FindUninitializedFields::isNonUnionUninit(const TypedValueRegion *R,
                                                FieldChainInfo LocalChain) {
   assert(R->getValueType()->isRecordType() &&
          !R->getValueType()->isUnionType() &&
          "This method only checks non-union record objects!");

 //===----------------------------------------------------------------------===//
 //                           Utility functions.
 //===----------------------------------------------------------------------===//
 
 static const TypedValueRegion *
 getConstructedRegion(const CXXConstructorDecl *CtorDecl,
                      CheckerContext &Context) {
 
-  Loc ThisLoc = Context.getSValBuilder().getCXXThis(CtorDecl,
-                                                    Context.getStackFrame());
+  Loc ThisLoc =
+      Context.getSValBuilder().getCXXThis(CtorDecl, Context.getStackFrame());
 
   SVal ObjectV = Context.getState()->getSVal(ThisLoc);
 
   auto *R = ObjectV.getAsRegion()->getAs<TypedValueRegion>();
   if (R && !R->getValueType()->getAsCXXRecordDecl())
     return nullptr;
 
   return R;
if (R.match(FD->getType().getAsString()))
       return true;
     if (R.match(FD->getName()))
       return true;
   }
 
   return false;
 }
 
+static const Stmt *getMethodBody(const CXXMethodDecl *M) {
+  if (isa<CXXConstructorDecl>(M))
+    return nullptr;
+
+  if (!M->isDefined())
+    return nullptr;
+
+  return M->getDefinition()->getBody();
+}
+
+static bool hasUnguardedAccess(const FieldDecl *FD, ProgramStateRef State) {
+
+  if (FD->getAccess() == AccessSpecifier::AS_public)
+    return true;
+
+  const auto *Parent = dyn_cast<CXXRecordDecl>(FD->getParent());
+
+  if (!Parent)
+    return true;
+
+  Parent = Parent->getDefinition();
+  assert(Parent && "The record's definition must be avaible if an uninitialized"
+                   " field of it was found!");
+
+  ASTContext &AC = State->getStateManager().getContext();
+
+  auto FieldAccessM = memberExpr(hasDeclaration(equalsNode(FD))).bind("access");
+
+  auto AssertLikeM = callExpr(callee(functionDecl(
+      anyOf(hasName("exit"), hasName("panic"), hasName("error"),
+            hasName("Assert"), hasName("assert"), hasName("ziperr"),
+            hasName("assfail"), hasName("db_error"), hasName("__assert"),
+            hasName("__assert2"), hasName("_wassert"), hasName("__assert_rtn"),
+            hasName("__assert_fail"), hasName("dtrace_assfail"),
+            hasName("yy_fatal_error"), hasName("_XCAssertionFailureHandler"),
+            hasName("_DTAssertionFailureHandler"),
+            hasName("_TSAssertionFailureHandler")))));
+
+  auto NoReturnFuncM = callExpr(callee(functionDecl(isNoReturn())));
+
+  auto GuardM =
+      stmt(anyOf(ifStmt(), switchStmt(), conditionalOperator(), AssertLikeM,
+            NoReturnFuncM))
+          .bind("guard");
+
+  for (const CXXMethodDecl *M : Parent->methods()) {
+    const Stmt *MethodBody = getMethodBody(M);
+    if (!MethodBody)
+      continue;
+
+    auto Accesses = match(stmt(hasDescendant(FieldAccessM)), *MethodBody, AC);
+    if (Accesses.empty())
+      continue;
+    const auto *FirstAccess = Accesses[0].getNodeAs<MemberExpr>("access");
+    assert(FirstAccess);
+
+    auto Guards = match(stmt(hasDescendant(GuardM)), *MethodBody, AC);
+    if (Guards.empty())
+      return true;
+    const auto *FirstGuard = Guards[0].getNodeAs<Stmt>("guard");
+    assert(FirstGuard);
+
+    if (FirstAccess->getBeginLoc() < FirstGuard->getBeginLoc())
+      return true;
+  }
+
+  return false;
+}
+
 std::string clang::ento::getVariableName(const FieldDecl *Field) {
   // If Field is a captured lambda variable, Field->getName() will return with
   // an empty string. We can however acquire it's name from the lambda's
   // captures.
   const auto *CXXParent = dyn_cast<CXXRecordDecl>(Field->getParent());
 
   if (CXXParent && CXXParent->isLambda()) {
     assert(CXXParent->captures_begin());
void ento::registerUninitializedObjectChecker(CheckerManager &Mgr) {
   auto Chk = Mgr.registerChecker<UninitializedObjectChecker>();
 
   AnalyzerOptions &AnOpts = Mgr.getAnalyzerOptions();
   UninitObjCheckerOptions &ChOpts = Chk->Opts;
 
   ChOpts.IsPedantic =
       AnOpts.getCheckerBooleanOption("Pedantic", /*DefaultVal*/ false, Chk);
   ChOpts.ShouldConvertNotesToWarnings =
-      AnOpts.getCheckerBooleanOption("NotesAsWarnings", /*DefaultVal*/ false, Chk);
+      AnOpts.getCheckerBooleanOption("NotesAsWarnings",
+                                     /*DefaultVal*/ false, Chk);
   ChOpts.CheckPointeeInitialization = AnOpts.getCheckerBooleanOption(
       "CheckPointeeInitialization", /*DefaultVal*/ false, Chk);
   ChOpts.IgnoredRecordsWithFieldPattern =
       AnOpts.getCheckerStringOption("IgnoreRecordsWithField",
                                     /*DefaultVal*/ "", Chk);
+  ChOpts.IgnoreGuardedFields =
+      AnOpts.getCheckerBooleanOption("IgnoreGuardedFields",
+                                     /*DefaultVal*/ false, Chk);
 }
 
 bool ento::shouldRegisterUninitializedObjectChecker(const LangOptions &LO) {
   return true;
 }

cfe/trunk/test/Analysis/cxx-uninitialized-object-unguarded-access.cpp

+// RUN: %clang_analyze_cc1 -analyzer-checker=core,alpha.cplusplus.UninitializedObject \
+// RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:Pedantic=true -DPEDANTIC \
+// RUN:   -analyzer-config alpha.cplusplus.UninitializedObject:IgnoreGuardedFields=true \
+// RUN:   -std=c++11 -verify  %s
+
+//===----------------------------------------------------------------------===//
+// Helper functions for tests.
+//===----------------------------------------------------------------------===//
+
+[[noreturn]] void halt();
+
+void assert(int b) {
+  if (!b)
+    halt();
+}
+
+int rand();
+
+//===----------------------------------------------------------------------===//
+// Tests for fields properly guarded by asserts.
+//===----------------------------------------------------------------------===//
+
+class NoUnguardedFieldsTest {
+public:
+  enum Kind {
+    V,
+    A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  NoUnguardedFieldsTest(Kind K) : K(K) {
+    switch (K) {
+    case V:
+      Volume = 0;
+      break;
+    case A:
+      Area = 0;
+      break;
+    }
+  }
+
+  void operator-() {
+    assert(K == Kind::A);
+    (void)Area;
+  }
+
+  void operator+() {
+    assert(K == Kind::V);
+    (void)Volume;
+  }
+};
+
+void fNoUnguardedFieldsTest() {
+  NoUnguardedFieldsTest T1(NoUnguardedFieldsTest::Kind::A);
+  NoUnguardedFieldsTest T2(NoUnguardedFieldsTest::Kind::V);
+}
+
+class NoUngardedFieldsNoReturnFuncCalledTest {
+public:
+  enum Kind {
+    V,
+    A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  NoUngardedFieldsNoReturnFuncCalledTest(Kind K) : K(K) {
+    switch (K) {
+    case V:
+      Volume = 0;
+      break;
+    case A:
+      Area = 0;
+      break;
+    }
+  }
+
+  void operator-() {
+    halt();
+    (void)Area;
+  }
+
+  void operator+() {
+    halt();
+    (void)Volume;
+  }
+};
+
+void fNoUngardedFieldsNoReturnFuncCalledTest() {
+  NoUngardedFieldsNoReturnFuncCalledTest
+    T1(NoUngardedFieldsNoReturnFuncCalledTest::Kind::A);
+  NoUngardedFieldsNoReturnFuncCalledTest
+    T2(NoUngardedFieldsNoReturnFuncCalledTest::Kind::V);
+}
+
+class NoUnguardedFieldsWithUndefMethodTest {
+public:
+  enum Kind {
+    V,
+    A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  NoUnguardedFieldsWithUndefMethodTest(Kind K) : K(K) {
+    switch (K) {
+    case V:
+      Volume = 0;
+      break;
+    case A:
+      Area = 0;
+      break;
+    }
+  }
+
+  void operator-() {
+    assert(K == Kind::A);
+    (void)Area;
+  }
+
+  void operator+() {
+    assert(K == Kind::V);
+    (void)Volume;
+  }
+
+  // We're checking method definitions for guards, so this is a no-crash test
+  // whether we handle methods without definitions.
+  void methodWithoutDefinition();
+};
+
+void fNoUnguardedFieldsWithUndefMethodTest() {
+  NoUnguardedFieldsWithUndefMethodTest
+      T1(NoUnguardedFieldsWithUndefMethodTest::Kind::A);
+  NoUnguardedFieldsWithUndefMethodTest
+      T2(NoUnguardedFieldsWithUndefMethodTest::Kind::V);
+}
+
+class UnguardedFieldThroughMethodTest {
+public:
+  enum Kind {
+    V,
+    A
+  };
+
+private:
+  int Volume, Area; // expected-note {{uninitialized field 'this->Volume'}}
+  Kind K;
+
+public:
+  UnguardedFieldThroughMethodTest(Kind K) : K(K) {
+    switch (K) {
+    case V:
+      Volume = 0;
+      break;
+    case A:
+      Area = 0; // expected-warning {{1 uninitialized field}}
+      break;
+    }
+  }
+
+  void operator-() {
+    assert(K == Kind::A);
+    (void)Area;
+  }
+
+  void operator+() {
+    (void)Volume;
+  }
+};
+
+void fUnguardedFieldThroughMethodTest() {
+  UnguardedFieldThroughMethodTest T1(UnguardedFieldThroughMethodTest::Kind::A);
+}
+
+class UnguardedPublicFieldsTest {
+public:
+  enum Kind {
+    V,
+    A
+  };
+
+public:
+  // Note that fields are public.
+  int Volume, Area; // expected-note {{uninitialized field 'this->Volume'}}
+  Kind K;
+
+public:
+  UnguardedPublicFieldsTest(Kind K) : K(K) {
+    switch (K) {
+    case V:
+      Volume = 0;
+      break;
+    case A:
+      Area = 0; // expected-warning {{1 uninitialized field}}
+      break;
+    }
+  }
+
+  void operator-() {
+    assert(K == Kind::A);
+    (void)Area;
+  }
+
+  void operator+() {
+    assert(K == Kind::V);
+    (void)Volume;
+  }
+};
+
+void fUnguardedPublicFieldsTest() {
+  UnguardedPublicFieldsTest T1(UnguardedPublicFieldsTest::Kind::A);
+}
+
+//===----------------------------------------------------------------------===//
+// Highlights of some false negatives due to syntactic checking.
+//===----------------------------------------------------------------------===//
+
+class UnguardedFalseNegativeTest1 {
+public:
+  enum Kind {
+    V,
+    A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  UnguardedFalseNegativeTest1(Kind K) : K(K) {
+    switch (K) {
+    case V:
+      Volume = 0;
+      break;
+    case A:
+      Area = 0;
+      break;
+    }
+  }
+
+  void operator-() {
+    if (rand())
+      assert(K == Kind::A);
+    (void)Area;
+  }
+
+  void operator+() {
+    if (rand())
+      assert(K == Kind::V);
+    (void)Volume;
+  }
+};
+
+void fUnguardedFalseNegativeTest1() {
+  UnguardedFalseNegativeTest1 T1(UnguardedFalseNegativeTest1::Kind::A);
+}
+
+class UnguardedFalseNegativeTest2 {
+public:
+  enum Kind {
+    V,
+    A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  UnguardedFalseNegativeTest2(Kind K) : K(K) {
+    switch (K) {
+    case V:
+      Volume = 0;
+      break;
+    case A:
+      Area = 0;
+      break;
+    }
+  }
+
+  void operator-() {
+    assert(rand());
+    (void)Area;
+  }
+
+  void operator+() {
+    assert(rand());
+    (void)Volume;
+  }
+};
+
+void fUnguardedFalseNegativeTest2() {
+  UnguardedFalseNegativeTest2 T1(UnguardedFalseNegativeTest2::Kind::A);
+}
+
+//===----------------------------------------------------------------------===//
+// Tests for other guards. These won't be as thorough, as other guards are
+// matched the same way as asserts, so if they are recognized, they are expected
+// to work as well as asserts do.
+//
+// None of these tests expect warnings, since the flag works correctly if these
+// fields are regarded properly guarded.
+//===----------------------------------------------------------------------===//
+
+class IfGuardedFieldsTest {
+public:
+  enum Kind {
+    V,
+    A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  IfGuardedFieldsTest(Kind K) : K(K) {
+    switch (K) {
+    case V:
+      Volume = 0;
+      break;
+    case A:
+      Area = 0;
+      break;
+    }
+  }
+
+  void operator-() {
+    if (K != Kind::A)
+      return;
+    (void)Area;
+  }
+
+  void operator+() {
+    if (K != Kind::V)
+      return;
+    (void)Volume;
+  }
+};
+
+void fIfGuardedFieldsTest() {
+  IfGuardedFieldsTest T1(IfGuardedFieldsTest::Kind::A);
+  IfGuardedFieldsTest T2(IfGuardedFieldsTest::Kind::V);
+}
+
+class SwitchGuardedFieldsTest {
+public:
+  enum Kind {
+    V,
+    A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  SwitchGuardedFieldsTest(Kind K) : K(K) {
+    switch (K) {
+    case V:
+      Volume = 0;
+      break;
+    case A:
+      Area = 0;
+      break;
+    }
+  }
+
+  int operator-() {
+    switch (K) {
+    case Kind::A:
+      return Area;
+    case Kind::V:
+      return -1;
+    }
+  }
+
+  int operator+() {
+    switch (K) {
+    case Kind::A:
+      return Area;
+    case Kind::V:
+      return -1;
+    }
+  }
+};
+
+void fSwitchGuardedFieldsTest() {
+  SwitchGuardedFieldsTest T1(SwitchGuardedFieldsTest::Kind::A);
+  SwitchGuardedFieldsTest T2(SwitchGuardedFieldsTest::Kind::V);
+}
+
+class ConditionalOperatorGuardedFieldsTest {
+public:
+  enum Kind {
+    V,
+    A
+  };
+
+private:
+  int Volume, Area;
+  Kind K;
+
+public:
+  ConditionalOperatorGuardedFieldsTest(Kind K) : K(K) {
+    switch (K) {
+    case V:
+      Volume = 0;
+      break;
+    case A:
+      Area = 0;
+      break;
+    }
+  }
+
+  int operator-() {
+    return K == Kind::A ? Area : -1;
+  }
+
+  int operator+() {
+    return K == Kind::V ? Volume : -1;
+  }
+};
+
+void fConditionalOperatorGuardedFieldsTest() {
+  ConditionalOperatorGuardedFieldsTest
+      T1(ConditionalOperatorGuardedFieldsTest::Kind::A);
+  ConditionalOperatorGuardedFieldsTest
+      T2(ConditionalOperatorGuardedFieldsTest::Kind::V);
+}