jasonliu accepted this revision.
jasonliu added a comment.
LGTM with minor nits.
================
Comment at: clang/lib/Sema/SemaAttr.cpp:74
return;
- // 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
+ // The #pragma align/pack affected a record in an included file, so Clang
+ // should warn when that pragma was written in a file that included the
----------------
================
Comment at: clang/lib/Sema/SemaAttr.cpp:347-349
+ AlignmentVal = CurVal.getPackNumber();
+ if (!CurVal.IsPackSet())
AlignmentVal = 8;
----------------
nit
================
Comment at: clang/lib/Sema/SemaAttr.cpp:372
+ // XL pragma pack does not support identifier syntax.
+ if (IsXLPragma && !SlotLabel.empty()) {
+ Diag(PragmaLoc, diag::err_pragma_pack_identifer_not_supported);
----------------
Could we move this to the top of the function so that we could have a quick
exit when we sees this?
If we are going to emit an error, the `AlignPackStack.Act(PragmaLoc, Action,
StringRef(), Info);` is not necessary any more.
================
Comment at: clang/lib/Sema/SemaAttr.cpp:555
+ });
+ // If we found the label so pop from there.
+ if (I != Stack.rend()) {
----------------
================
Comment at: clang/lib/Sema/SemaAttr.cpp:581
+ } else if (!Stack.empty()) {
+ // xl '#pragma align' sets the base line,
+ // and pragma pack cannot pop over the base line.
----------------
================
Comment at: clang/lib/Sema/SemaAttr.cpp:582
+ // xl '#pragma align' sets the base line,
+ // and pragma pack cannot pop over the base line.
+ if (Value.IsXLStack() && Value.IsPackAttr() &&
CurrentValue.IsAlignAttr())
----------------
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D87702/new/
https://reviews.llvm.org/D87702
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
