[clang-tblgen] Fix non-determinism in generating AttrSubMatchRulesParserStringSwitches.inc

[ikudrin]

Summary

llvm::MapVector, compared to std::map, guarantees the same iteration order in different runs.

[aaron.ballman]

I'd like to understand the motivation behind the change a bit more. What problems stem from generating the parsing helpers in a different order? Why do we not need the same changes when generating documentation (see EmitClangAttrDocs())?

[ikudrin]

We've noticed binary differences in clang in several builds on our local build bots, and the patch fixed that.

As for generating the documentation, the issue can be seen, for example, in Clang-12 and Clang-13 documentation, where the section "Nullable Attributes" is placed differently. Maybe it'll be better to sort the categories alphabetically. Anyway, that is a similar but still a different issue.

[aaron.ballman]

In D113168#3114626, @ikudrin wrote:

We've noticed binary differences in clang in several builds on our local build bots, and the patch fixed that.

As for generating the documentation, the issue can be seen, for example, in Clang-12 and Clang-13 documentation, where the section "Nullable Attributes" is placed differently. Maybe it'll be better to sort the categories alphabetically. Anyway, that is a similar but still a different issue.

FWIW, I consider the documentation issue to be more of a problem than the one being fixed here. The fix for this means code we generate will be generated in the same order every time, which has nice properties in terms of doing incremental builds, but is largely unimportant as far as users are concerned. The documentation is user facing, so there's more of a chance to notice positional changes there.

I'm happy enough with the changes, but please fix the other nondeterminism at the same time so that we don't have to play whack-a-mole in this file.

[ikudrin]

The compiler has to be deterministic and one way to check that is to recompile a relatively large project several times. For clang, that large project is usually clang itself, so it has to be compiled in a stable way.

Anyway, I've prepared a fix for generating the documentation, D113477. Please, take a look.

[aaron.ballman]

LGTM, thanks for the fix!

clang/utils/TableGen/ClangAttrEmitter.cpp

 //===----------------------------------------------------------------------===//
 
 #include "TableGenBackends.h"
 #include "ASTTableGen.h"
 
 #include "llvm/ADT/ArrayRef.h"
 #include "llvm/ADT/DenseMap.h"
 #include "llvm/ADT/DenseSet.h"
+#include "llvm/ADT/MapVector.h"
 #include "llvm/ADT/STLExtras.h"
 #include "llvm/ADT/SmallString.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/ADT/StringRef.h"
 #include "llvm/ADT/StringSet.h"
 #include "llvm/ADT/StringSwitch.h"
 #include "llvm/ADT/iterator_range.h"
 #include "llvm/Support/ErrorHandling.h"

 
 void PragmaClangAttributeSupport::generateParsingHelpers(raw_ostream &OS) {
   // Generate routines that check the names of sub-rules.
   OS << "Optional<attr::SubjectMatchRule> "
         "defaultIsAttributeSubjectMatchSubRuleFor(StringRef, bool) {\n";
   OS << "  return None;\n";
   OS << "}\n\n";
 
-  std::map<const Record *, std::vector<AttributeSubjectMatchRule>>
+  llvm::MapVector<const Record *, std::vector<AttributeSubjectMatchRule>>
       SubMatchRules;
   for (const auto &Rule : Rules) {
     if (!Rule.isSubRule())
       continue;
     SubMatchRules[Rule.MetaSubject].push_back(Rule);
   }
 
   for (const auto &SubMatchRule : SubMatchRules) {