This revision was automatically updated to reflect the committed changes.
Closed by commit rGe97071d79520: [NFC] Renaming PackStack to AlignPackStack
(authored by Xiangling_L).
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D93901/new/
https://reviews.llvm.org/D93901
Files:
clang/include/clang/Sema/Sema.h
clang/include/clang/Serialization/ASTBitCodes.h
clang/include/clang/Serialization/ASTReader.h
clang/lib/Sema/Sema.cpp
clang/lib/Sema/SemaAttr.cpp
clang/lib/Serialization/ASTReader.cpp
clang/lib/Serialization/ASTWriter.cpp
clang/test/Sema/Inputs/pragma-align-pack1.h
clang/test/Sema/misleading-pragma-align-pack-diagnostics.c
Index: clang/test/Sema/misleading-pragma-align-pack-diagnostics.c
===================================================================
--- /dev/null
+++ clang/test/Sema/misleading-pragma-align-pack-diagnostics.c
@@ -0,0 +1,19 @@
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -Wpragma-pack-suspicious-include -I %S/Inputs -DALIGN_SET_HERE -verify %s
+// RUN: %clang_cc1 -triple i686-apple-darwin9 -fsyntax-only -Wpragma-pack-suspicious-include -I %S/Inputs -DRECORD_ALIGN -verify %s
+
+#ifdef ALIGN_SET_HERE
+#pragma align = natural // expected-warning {{unterminated '#pragma pack (push, ...)' at end of file}}
+// expected-warning@+9 {{the current #pragma pack alignment value is modified in the included file}}
+#endif
+
+#ifdef RECORD_ALIGN
+#pragma align = mac68k
+// expected-note@-1 {{previous '#pragma pack' directive that modifies alignment is here}}
+// expected-warning@+3 {{non-default #pragma pack value changes the alignment of struct or union members in the included file}}
+#endif
+
+#include "pragma-align-pack1.h"
+
+#ifdef RECORD_ALIGN
+#pragma align = reset
+#endif
Index: clang/test/Sema/Inputs/pragma-align-pack1.h
===================================================================
--- /dev/null
+++ clang/test/Sema/Inputs/pragma-align-pack1.h
@@ -0,0 +1,11 @@
+#ifdef ALIGN_SET_HERE
+#pragma align = mac68k
+// expected-note@-1 {{previous '#pragma pack' directive that modifies alignment is here}}
+// expected-warning@-2 {{unterminated '#pragma pack (push, ...)' at end of file}}
+#endif
+
+#ifdef RECORD_ALIGN
+struct S {
+ int x;
+};
+#endif
Index: clang/lib/Serialization/ASTWriter.cpp
===================================================================
--- clang/lib/Serialization/ASTWriter.cpp
+++ clang/lib/Serialization/ASTWriter.cpp
@@ -4151,24 +4151,24 @@
Stream.EmitRecord(POINTERS_TO_MEMBERS_PRAGMA_OPTIONS, Record);
}
-/// Write the state of 'pragma pack' at the end of the module.
+/// Write the state of 'pragma align/pack' at the end of the module.
void ASTWriter::WritePackPragmaOptions(Sema &SemaRef) {
- // Don't serialize pragma pack state for modules, since it should only take
- // effect on a per-submodule basis.
+ // Don't serialize pragma align/pack state for modules, since it should only
+ // take effect on a per-submodule basis.
if (WritingModule)
return;
RecordData Record;
- Record.push_back(SemaRef.PackStack.CurrentValue);
- AddSourceLocation(SemaRef.PackStack.CurrentPragmaLocation, Record);
- Record.push_back(SemaRef.PackStack.Stack.size());
- for (const auto &StackEntry : SemaRef.PackStack.Stack) {
+ Record.push_back(SemaRef.AlignPackStack.CurrentValue);
+ AddSourceLocation(SemaRef.AlignPackStack.CurrentPragmaLocation, Record);
+ Record.push_back(SemaRef.AlignPackStack.Stack.size());
+ for (const auto &StackEntry : SemaRef.AlignPackStack.Stack) {
Record.push_back(StackEntry.Value);
AddSourceLocation(StackEntry.PragmaLocation, Record);
AddSourceLocation(StackEntry.PragmaPushLocation, Record);
AddString(StackEntry.StackSlotLabel, Record);
}
- Stream.EmitRecord(PACK_PRAGMA_OPTIONS, Record);
+ Stream.EmitRecord(ALIGN_PACK_PRAGMA_OPTIONS, Record);
}
/// Write the state of 'pragma float_control' at the end of the module.
Index: clang/lib/Serialization/ASTReader.cpp
===================================================================
--- clang/lib/Serialization/ASTReader.cpp
+++ clang/lib/Serialization/ASTReader.cpp
@@ -3742,25 +3742,25 @@
ForceCUDAHostDeviceDepth = Record[0];
break;
- case PACK_PRAGMA_OPTIONS: {
+ case ALIGN_PACK_PRAGMA_OPTIONS: {
if (Record.size() < 3) {
Error("invalid pragma pack record");
return Failure;
}
- PragmaPackCurrentValue = Record[0];
- PragmaPackCurrentLocation = ReadSourceLocation(F, Record[1]);
+ PragmaAlignPackCurrentValue = Record[0];
+ PragmaAlignPackCurrentLocation = ReadSourceLocation(F, Record[1]);
unsigned NumStackEntries = Record[2];
unsigned Idx = 3;
// Reset the stack when importing a new module.
- PragmaPackStack.clear();
+ PragmaAlignPackStack.clear();
for (unsigned I = 0; I < NumStackEntries; ++I) {
- PragmaPackStackEntry Entry;
+ PragmaAlignPackStackEntry Entry;
Entry.Value = Record[Idx++];
Entry.Location = ReadSourceLocation(F, Record[Idx++]);
Entry.PushLocation = ReadSourceLocation(F, Record[Idx++]);
- PragmaPackStrings.push_back(ReadString(Record, Idx));
- Entry.SlotLabel = PragmaPackStrings.back();
- PragmaPackStack.push_back(Entry);
+ PragmaAlignPackStrings.push_back(ReadString(Record, Idx));
+ Entry.SlotLabel = PragmaAlignPackStrings.back();
+ PragmaAlignPackStack.push_back(Entry);
}
break;
}
@@ -7875,32 +7875,36 @@
}
SemaObj->ForceCUDAHostDeviceDepth = ForceCUDAHostDeviceDepth;
- if (PragmaPackCurrentValue) {
+ if (PragmaAlignPackCurrentValue) {
// The bottom of the stack might have a default value. It must be adjusted
// to the current value to ensure that the packing state is preserved after
// popping entries that were included/imported from a PCH/module.
bool DropFirst = false;
- if (!PragmaPackStack.empty() &&
- PragmaPackStack.front().Location.isInvalid()) {
- assert(PragmaPackStack.front().Value == SemaObj->PackStack.DefaultValue &&
+ if (!PragmaAlignPackStack.empty() &&
+ PragmaAlignPackStack.front().Location.isInvalid()) {
+ assert(PragmaAlignPackStack.front().Value ==
+ SemaObj->AlignPackStack.DefaultValue &&
"Expected a default alignment value");
- SemaObj->PackStack.Stack.emplace_back(
- PragmaPackStack.front().SlotLabel, SemaObj->PackStack.CurrentValue,
- SemaObj->PackStack.CurrentPragmaLocation,
- PragmaPackStack.front().PushLocation);
+ SemaObj->AlignPackStack.Stack.emplace_back(
+ PragmaAlignPackStack.front().SlotLabel,
+ SemaObj->AlignPackStack.CurrentValue,
+ SemaObj->AlignPackStack.CurrentPragmaLocation,
+ PragmaAlignPackStack.front().PushLocation);
DropFirst = true;
}
for (const auto &Entry :
- llvm::makeArrayRef(PragmaPackStack).drop_front(DropFirst ? 1 : 0))
- SemaObj->PackStack.Stack.emplace_back(Entry.SlotLabel, Entry.Value,
- Entry.Location, Entry.PushLocation);
- if (PragmaPackCurrentLocation.isInvalid()) {
- assert(*PragmaPackCurrentValue == SemaObj->PackStack.DefaultValue &&
+ llvm::makeArrayRef(PragmaAlignPackStack).drop_front(DropFirst ? 1 : 0))
+ SemaObj->AlignPackStack.Stack.emplace_back(
+ Entry.SlotLabel, Entry.Value, Entry.Location, Entry.PushLocation);
+ if (PragmaAlignPackCurrentLocation.isInvalid()) {
+ assert(*PragmaAlignPackCurrentValue ==
+ SemaObj->AlignPackStack.DefaultValue &&
"Expected a default alignment value");
// Keep the current values.
} else {
- SemaObj->PackStack.CurrentValue = *PragmaPackCurrentValue;
- SemaObj->PackStack.CurrentPragmaLocation = PragmaPackCurrentLocation;
+ SemaObj->AlignPackStack.CurrentValue = *PragmaAlignPackCurrentValue;
+ SemaObj->AlignPackStack.CurrentPragmaLocation =
+ PragmaAlignPackCurrentLocation;
}
}
if (FpPragmaCurrentValue) {
Index: clang/lib/Sema/SemaAttr.cpp
===================================================================
--- clang/lib/Sema/SemaAttr.cpp
+++ clang/lib/Sema/SemaAttr.cpp
@@ -49,27 +49,28 @@
void Sema::AddAlignmentAttributesForRecord(RecordDecl *RD) {
// If there is no pack value, we don't need any attributes.
- if (!PackStack.CurrentValue)
+ if (!AlignPackStack.CurrentValue)
return;
// Otherwise, check to see if we need a max field alignment attribute.
- if (unsigned Alignment = PackStack.CurrentValue) {
+ if (unsigned Alignment = AlignPackStack.CurrentValue) {
if (Alignment == Sema::kMac68kAlignmentSentinel)
RD->addAttr(AlignMac68kAttr::CreateImplicit(Context));
else
RD->addAttr(MaxFieldAlignmentAttr::CreateImplicit(Context,
Alignment * 8));
}
- if (PackIncludeStack.empty())
+ if (AlignPackIncludeStack.empty())
return;
- // The #pragma pack affected a record in an included file, so Clang should
- // warn when that pragma was written in a file that included the included
- // file.
- for (auto &PackedInclude : llvm::reverse(PackIncludeStack)) {
- if (PackedInclude.CurrentPragmaLocation != PackStack.CurrentPragmaLocation)
+ // The #pragma align/pack affected a record in an included file, so Clang
+ // should warn when that the pragma was written in a file that included the
+ // included file.
+ for (auto &AlignPackedInclude : llvm::reverse(AlignPackIncludeStack)) {
+ if (AlignPackedInclude.CurrentPragmaLocation !=
+ AlignPackStack.CurrentPragmaLocation)
break;
- if (PackedInclude.HasNonDefaultValue)
- PackedInclude.ShouldWarnOnInclude = true;
+ if (AlignPackedInclude.HasNonDefaultValue)
+ AlignPackedInclude.ShouldWarnOnInclude = true;
}
}
@@ -238,8 +239,8 @@
// Reset just pops the top of the stack, or resets the current alignment to
// default.
Action = Sema::PSK_Pop;
- if (PackStack.Stack.empty()) {
- if (PackStack.CurrentValue) {
+ if (AlignPackStack.Stack.empty()) {
+ if (AlignPackStack.CurrentValue) {
Action = Sema::PSK_Reset;
} else {
Diag(PragmaLoc, diag::warn_pragma_options_align_reset_failed)
@@ -250,7 +251,7 @@
break;
}
- PackStack.Act(PragmaLoc, Action, StringRef(), Alignment);
+ AlignPackStack.Act(PragmaLoc, Action, StringRef(), Alignment);
}
void Sema::ActOnPragmaClangSection(SourceLocation PragmaLoc, PragmaClangSectionAction Action,
@@ -317,7 +318,7 @@
// Show the current alignment, making sure to show the right value
// for the default.
// FIXME: This should come from the target.
- AlignmentVal = PackStack.CurrentValue;
+ AlignmentVal = AlignPackStack.CurrentValue;
if (AlignmentVal == 0)
AlignmentVal = 8;
if (AlignmentVal == Sema::kMac68kAlignmentSentinel)
@@ -330,61 +331,72 @@
if (Action & Sema::PSK_Pop) {
if (Alignment && !SlotLabel.empty())
Diag(PragmaLoc, diag::warn_pragma_pack_pop_identifier_and_alignment);
- if (PackStack.Stack.empty())
+ if (AlignPackStack.Stack.empty())
Diag(PragmaLoc, diag::warn_pragma_pop_failed) << "pack" << "stack empty";
}
- PackStack.Act(PragmaLoc, Action, SlotLabel, AlignmentVal);
+ AlignPackStack.Act(PragmaLoc, Action, SlotLabel, AlignmentVal);
}
-void Sema::DiagnoseNonDefaultPragmaPack(PragmaPackDiagnoseKind Kind,
- SourceLocation IncludeLoc) {
- if (Kind == PragmaPackDiagnoseKind::NonDefaultStateAtInclude) {
- SourceLocation PrevLocation = PackStack.CurrentPragmaLocation;
+void Sema::DiagnoseNonDefaultPragmaAlignPack(PragmaAlignPackDiagnoseKind Kind,
+ SourceLocation IncludeLoc) {
+ if (Kind == PragmaAlignPackDiagnoseKind::NonDefaultStateAtInclude) {
+ SourceLocation PrevLocation = AlignPackStack.CurrentPragmaLocation;
// Warn about non-default alignment at #includes (without redundant
// warnings for the same directive in nested includes).
// The warning is delayed until the end of the file to avoid warnings
// for files that don't have any records that are affected by the modified
// alignment.
bool HasNonDefaultValue =
- PackStack.hasValue() &&
- (PackIncludeStack.empty() ||
- PackIncludeStack.back().CurrentPragmaLocation != PrevLocation);
- PackIncludeStack.push_back(
- {PackStack.CurrentValue,
- PackStack.hasValue() ? PrevLocation : SourceLocation(),
+ AlignPackStack.hasValue() &&
+ (AlignPackIncludeStack.empty() ||
+ AlignPackIncludeStack.back().CurrentPragmaLocation != PrevLocation);
+ AlignPackIncludeStack.push_back(
+ {AlignPackStack.CurrentValue,
+ AlignPackStack.hasValue() ? PrevLocation : SourceLocation(),
HasNonDefaultValue, /*ShouldWarnOnInclude*/ false});
return;
}
- assert(Kind == PragmaPackDiagnoseKind::ChangedStateAtExit && "invalid kind");
- PackIncludeState PrevPackState = PackIncludeStack.pop_back_val();
- if (PrevPackState.ShouldWarnOnInclude) {
+ assert(Kind == PragmaAlignPackDiagnoseKind::ChangedStateAtExit &&
+ "invalid kind");
+ AlignPackIncludeState PrevAlignPackState =
+ AlignPackIncludeStack.pop_back_val();
+ // FIXME: AlignPackStack may contain both #pragma align and #pragma pack
+ // information, diagnostics below might not be accurate if we have mixed
+ // pragmas.
+ if (PrevAlignPackState.ShouldWarnOnInclude) {
// Emit the delayed non-default alignment at #include warning.
Diag(IncludeLoc, diag::warn_pragma_pack_non_default_at_include);
- Diag(PrevPackState.CurrentPragmaLocation, diag::note_pragma_pack_here);
+ Diag(PrevAlignPackState.CurrentPragmaLocation, diag::note_pragma_pack_here);
}
// Warn about modified alignment after #includes.
- if (PrevPackState.CurrentValue != PackStack.CurrentValue) {
+ if (PrevAlignPackState.CurrentValue != AlignPackStack.CurrentValue) {
Diag(IncludeLoc, diag::warn_pragma_pack_modified_after_include);
- Diag(PackStack.CurrentPragmaLocation, diag::note_pragma_pack_here);
+ Diag(AlignPackStack.CurrentPragmaLocation, diag::note_pragma_pack_here);
}
}
-void Sema::DiagnoseUnterminatedPragmaPack() {
- if (PackStack.Stack.empty())
+void Sema::DiagnoseUnterminatedPragmaAlignPack() {
+ if (AlignPackStack.Stack.empty())
return;
bool IsInnermost = true;
- for (const auto &StackSlot : llvm::reverse(PackStack.Stack)) {
+
+ // FIXME: AlignPackStack may contain both #pragma align and #pragma pack
+ // information, diagnostics below might not be accurate if we have mixed
+ // pragmas.
+ for (const auto &StackSlot : llvm::reverse(AlignPackStack.Stack)) {
Diag(StackSlot.PragmaPushLocation, diag::warn_pragma_pack_no_pop_eof);
// The user might have already reset the alignment, so suggest replacing
// the reset with a pop.
- if (IsInnermost && PackStack.CurrentValue == PackStack.DefaultValue) {
- auto DB = Diag(PackStack.CurrentPragmaLocation,
+ if (IsInnermost &&
+ AlignPackStack.CurrentValue == AlignPackStack.DefaultValue) {
+ auto DB = Diag(AlignPackStack.CurrentPragmaLocation,
diag::note_pragma_pack_pop_instead_reset);
- SourceLocation FixItLoc = Lexer::findLocationAfterToken(
- PackStack.CurrentPragmaLocation, tok::l_paren, SourceMgr, LangOpts,
- /*SkipTrailing=*/false);
+ SourceLocation FixItLoc =
+ Lexer::findLocationAfterToken(AlignPackStack.CurrentPragmaLocation,
+ tok::l_paren, SourceMgr, LangOpts,
+ /*SkipTrailing=*/false);
if (FixItLoc.isValid())
DB << FixItHint::CreateInsertion(FixItLoc, "pop");
}
Index: clang/lib/Sema/Sema.cpp
===================================================================
--- clang/lib/Sema/Sema.cpp
+++ clang/lib/Sema/Sema.cpp
@@ -120,8 +120,9 @@
}
IncludeStack.push_back(IncludeLoc);
- S->DiagnoseNonDefaultPragmaPack(
- Sema::PragmaPackDiagnoseKind::NonDefaultStateAtInclude, IncludeLoc);
+ S->DiagnoseNonDefaultPragmaAlignPack(
+ Sema::PragmaAlignPackDiagnoseKind::NonDefaultStateAtInclude,
+ IncludeLoc);
}
break;
}
@@ -130,8 +131,8 @@
if (llvm::timeTraceProfilerEnabled())
llvm::timeTraceProfilerEnd();
- S->DiagnoseNonDefaultPragmaPack(
- Sema::PragmaPackDiagnoseKind::ChangedStateAtExit,
+ S->DiagnoseNonDefaultPragmaAlignPack(
+ Sema::PragmaAlignPackDiagnoseKind::ChangedStateAtExit,
IncludeStack.pop_back_val());
}
break;
@@ -157,7 +158,7 @@
OriginalLexicalContext(nullptr), MSStructPragmaOn(false),
MSPointerToMemberRepresentationMethod(
LangOpts.getMSPointerToMemberRepresentationMethod()),
- VtorDispStack(LangOpts.getVtorDispMode()), PackStack(0),
+ VtorDispStack(LangOpts.getVtorDispMode()), AlignPackStack(0),
DataSegStack(nullptr), BSSSegStack(nullptr), ConstSegStack(nullptr),
CodeSegStack(nullptr), FpPragmaStack(FPOptionsOverride()),
CurInitSeg(nullptr), VisContext(nullptr),
@@ -1038,7 +1039,7 @@
}
}
- DiagnoseUnterminatedPragmaPack();
+ DiagnoseUnterminatedPragmaAlignPack();
DiagnoseUnterminatedPragmaAttribute();
// All delayed member exception specs should be checked or we end up accepting
Index: clang/include/clang/Serialization/ASTReader.h
===================================================================
--- clang/include/clang/Serialization/ASTReader.h
+++ clang/include/clang/Serialization/ASTReader.h
@@ -869,17 +869,17 @@
llvm::SmallVector<FpPragmaStackEntry, 2> FpPragmaStack;
llvm::SmallVector<std::string, 2> FpPragmaStrings;
- /// The pragma pack state.
- Optional<unsigned> PragmaPackCurrentValue;
- SourceLocation PragmaPackCurrentLocation;
- struct PragmaPackStackEntry {
+ /// The pragma align/pack state.
+ Optional<unsigned> PragmaAlignPackCurrentValue;
+ SourceLocation PragmaAlignPackCurrentLocation;
+ struct PragmaAlignPackStackEntry {
unsigned Value;
SourceLocation Location;
SourceLocation PushLocation;
StringRef SlotLabel;
};
- llvm::SmallVector<PragmaPackStackEntry, 2> PragmaPackStack;
- llvm::SmallVector<std::string, 2> PragmaPackStrings;
+ llvm::SmallVector<PragmaAlignPackStackEntry, 2> PragmaAlignPackStack;
+ llvm::SmallVector<std::string, 2> PragmaAlignPackStrings;
/// The OpenCL extension settings.
OpenCLOptions OpenCLExtensions;
Index: clang/include/clang/Serialization/ASTBitCodes.h
===================================================================
--- clang/include/clang/Serialization/ASTBitCodes.h
+++ clang/include/clang/Serialization/ASTBitCodes.h
@@ -681,8 +681,8 @@
MODULAR_CODEGEN_DECLS = 60,
- /// Record code for \#pragma pack options.
- PACK_PRAGMA_OPTIONS = 61,
+ /// Record code for \#pragma align/pack options.
+ ALIGN_PACK_PRAGMA_OPTIONS = 61,
/// The stack of open #ifs/#ifdefs recorded in a preamble.
PP_CONDITIONAL_STACK = 62,
Index: clang/include/clang/Sema/Sema.h
===================================================================
--- clang/include/clang/Sema/Sema.h
+++ clang/include/clang/Sema/Sema.h
@@ -568,14 +568,14 @@
// #pragma pack.
// Sentinel to represent when the stack is set to mac68k alignment.
static const unsigned kMac68kAlignmentSentinel = ~0U;
- PragmaStack<unsigned> PackStack;
- // The current #pragma pack values and locations at each #include.
- struct PackIncludeState {
+ PragmaStack<unsigned> AlignPackStack;
+ // The current #pragma align/pack values and locations at each #include.
+ struct AlignPackIncludeState {
unsigned CurrentValue;
SourceLocation CurrentPragmaLocation;
bool HasNonDefaultValue, ShouldWarnOnInclude;
};
- SmallVector<PackIncludeState, 8> PackIncludeStack;
+ SmallVector<AlignPackIncludeState, 8> AlignPackIncludeStack;
// Segment #pragmas.
PragmaStack<StringLiteral *> DataSegStack;
PragmaStack<StringLiteral *> BSSSegStack;
@@ -9721,14 +9721,14 @@
void ActOnPragmaPack(SourceLocation PragmaLoc, PragmaMsStackAction Action,
StringRef SlotLabel, Expr *Alignment);
- enum class PragmaPackDiagnoseKind {
+ enum class PragmaAlignPackDiagnoseKind {
NonDefaultStateAtInclude,
ChangedStateAtExit
};
- void DiagnoseNonDefaultPragmaPack(PragmaPackDiagnoseKind Kind,
- SourceLocation IncludeLoc);
- void DiagnoseUnterminatedPragmaPack();
+ void DiagnoseNonDefaultPragmaAlignPack(PragmaAlignPackDiagnoseKind Kind,
+ SourceLocation IncludeLoc);
+ void DiagnoseUnterminatedPragmaAlignPack();
/// ActOnPragmaMSStruct - Called on well formed \#pragma ms_struct [on|off].
void ActOnPragmaMSStruct(PragmaMSStructKind Kind);
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
