[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

aaron.ballman closed this revision. aaron.ballman added a comment. I've commit on your behalf in b0de656bdf0ee3f4e51d04ae29160dab99819e8e (clang), b0de656bdf0ee3f4e51d04ae29160dab99819e8e

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

ddpagan updated this revision to Diff 385692. ddpagan added a comment. Fixed build problem (found during build of flang). Added align clause to simple clause check. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112577/new/ https://reviews.llvm.org/D112577 Files:

RE: [PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

] Initial parsing/sema for 'align' clause aaron.ballman added a comment. Before I land this, it looks like precommit CI is failing with what possibly is a related failure: [9094/9117] Linking CXX executable tools/flang/unittests/Frontend/FlangFrontendTests FAILED: tools/flang/unittests/Frontend

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

aaron.ballman added a comment. Before I land this, it looks like precommit CI is failing with what possibly is a related failure: [9094/9117] Linking CXX executable tools/flang/unittests/Frontend/FlangFrontendTests FAILED: tools/flang/unittests/Frontend/FlangFrontendTests : &&

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

ddpagan updated this revision to Diff 385098. ddpagan added a comment. Successfully rebased, built, and tested. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112577/new/ https://reviews.llvm.org/D112577 Files: clang/include/clang/AST/OpenMPClause.h

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

aaron.ballman added a comment. Ah, thank you for the explanations! Then this continues to LGTM. Do you need someone to commit on your behalf? If so, what name and email address would you like me to use for patch attribution in git? CHANGES SINCE LAST ACTION

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

ddpagan added a comment. Thanks for reviewing the code, Aaron. Comment at: clang/lib/Serialization/ASTWriter.cpp:5018 Record.AddStmt(A->getAllocator()); +Record.AddStmt(A->getAlignment()); Record.AddSourceRange(A->getRange());

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

aaron.ballman accepted this revision. aaron.ballman added a comment. LGTM aside from some possible nits. Comment at: clang/lib/Serialization/ASTWriter.cpp:5018 Record.AddStmt(A->getAllocator()); +Record.AddStmt(A->getAlignment());

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

ddpagan added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:362-364 + /// Sets the location of '('. + void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; } + aaron.ballman wrote: > ABataev wrote: > > Also worth to make it private > I

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

ddpagan updated this revision to Diff 383428. ddpagan added a comment. Thanks for the review, Aaron. CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112577/new/ https://reviews.llvm.org/D112577 Files: clang/include/clang/AST/OpenMPClause.h

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

aaron.ballman added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:362-364 + /// Sets the location of '('. + void setLParenLoc(SourceLocation Loc) { LParenLoc = Loc; } + ABataev wrote: > Also worth to make it private I agree, I think these

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

ABataev accepted this revision. ABataev added a comment. This revision is now accepted and ready to land. LG CHANGES SINCE LAST ACTION https://reviews.llvm.org/D112577/new/ https://reviews.llvm.org/D112577 ___ cfe-commits mailing list

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

ddpagan updated this revision to Diff 382701. ddpagan added a comment. Thanks for the review, Alexey. Made the requested changes except for modifying "used_children". After looking over other clause classes, for example, OMPNumThreadsClause, we're being consistent in how we're declaring these.

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

ABataev added inline comments. Comment at: clang/include/clang/AST/OpenMPClause.h:334 +/// value of 8. +class OMPAlignClause : public OMPClause { + friend class OMPClauseReader; `final` Comment at:

[PATCH] D112577: [clang][OpenMP] Initial parsing/sema for 'align' clause

ddpagan created this revision. ddpagan added reviewers: ABataev, jdoerfert. Herald added subscribers: arphaman, guansong, yaxunl. Herald added a reviewer: aaron.ballman. ddpagan requested review of this revision. Herald added subscribers: llvm-commits, cfe-commits, sstefan1. Herald added projects: