aaron.ballman added inline comments.
================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:716 +def warn_pragma_pack_non_default_at_include : Warning< + "non-default #pragma pack value might change the alignment of records in the " + "included file">, ---------------- We don't use "record" in the text for diagnostics. Perhaps replace record with "struct or union members"? ================ Comment at: include/clang/Basic/DiagnosticSemaKinds.td:718 + "included file">, + InGroup<PragmaPack>; +def warn_pragma_pack_modified_after_include : Warning< ---------------- This can move up a line. ================ Comment at: include/clang/Sema/Sema.h:1162 + sema::SemaPPCallbacks *SemaPPCallbackHandler; + ---------------- Please group this with the other private Sema members and give it a \brief comment. ================ Comment at: lib/Sema/Sema.cpp:95 + if (IncludeLoc.isValid()) { + // Entering an include. + IncludeStack.push_back(IncludeLoc); ---------------- This comment doesn't add much value. ================ Comment at: lib/Sema/SemaAttr.cpp:235-236 + return; + for (auto I = PackStack.Stack.rbegin(), E = PackStack.Stack.rend(); I != E; + ++I) + Diag(I->PragmaPushLocation, diag::warn_pragma_pack_no_pop_eof); ---------------- Can you use `for (const auto &StackSlot : llvm::reverse(PackStack.Stack))` instead? ================ Comment at: lib/Sema/SemaAttr.cpp:287-288 if (Action & PSK_Push) - Stack.push_back(Slot(StackSlotLabel, CurrentValue, CurrentPragmaLocation)); + Stack.push_back(Slot(StackSlotLabel, CurrentValue, CurrentPragmaLocation, + PragmaLocation)); else if (Action & PSK_Pop) { ---------------- This would be better written using `emplace_back()` rather than constructing and using `push_back()`. Repository: rL LLVM https://reviews.llvm.org/D35484 _______________________________________________ cfe-commits mailing list cfe-commits@lists.llvm.org http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits