r293123 - Remove and replace DiagStatePoint tracking and lookup data structure.
Richard Smith via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 28 20:43:07 PST 2017
On 28 Jan 2017 8:16 pm, "Duncan P. N. Exon Smith via cfe-commits" <
cfe-commits at lists.llvm.org> wrote:
Hi Richard,
This commit makes one of the tests fail in:
https://reviews.llvm.org/D27689
The implicit modules model expects to be able to "upgrade" an implicit PCM
with more -Werror flags *without* affecting the signature. However, with
your commit, the command-line diagnostic flags (e.g., -Wconversion vs
-Wno-conversion) now change the AST, and thus change the ASTFileSignature.
One possible hammer would be to disable this pragma-diagnostic-stuff
somehow for implicit modules. Do you have any other suggestions?
I think that upgrade model is incorrect in the fully general case. For
instance, in C++, enabling more warning flags in the definition of a
template can cause behaviour changes in users of the template. Likewise, in
C, as I recall we'll sometimes look at the warning state at the point of
definition of a macro to determine whether to emit a warning in a macro
expansion.
That said, for implicit modules we can probably assume that the effects of
changing the starting state can be simulated. Right now ,the implicit /
explicit module decision is not made until the point of import, so we would
need to commit to that earlier to avoid emitting the information at all in
that case. Another alternative would be to exclude the diagnostic settings
in the initial state from the hash, either by never emitting it and
recomputing it from the diagnostic flags as needed, or by emitting it into
an unhashed portion of the file.
Thanks,
Duncan
> On 2017-Jan-25, at 17:01, Richard Smith via cfe-commits <
cfe-commits at lists.llvm.org> wrote:
>
> Author: rsmith
> Date: Wed Jan 25 19:01:01 2017
> New Revision: 293123
>
> URL: http://llvm.org/viewvc/llvm-project?rev=293123&view=rev
> Log:
> Remove and replace DiagStatePoint tracking and lookup data structure.
>
> Rather than storing a single flat list of SourceLocations where the
diagnostic
> state changes (in source order), we now store a separate list for each
FileID
> in which there is a diagnostic state transition. (State for other files is
> built and cached lazily, on demand.) This has two consequences:
>
> 1) We can now sensibly support modules, and properly track the diagnostic
state
> for modular headers (this matters when, for instance, triggering
instantiation
> of a template defined within a module triggers diagnostics).
>
> 2) It's much faster than the old approach, since we can now just do a
binary
> search on the offsets within the FileID rather than needing to call
> isBeforeInTranslationUnit to determine source order (which is surprisingly
> slow). For some pathological (but real world) files, this reduces total
> compilation time by more than 10%.
>
> For now, the diagnostic state points for modules are loaded eagerly. It
seems
> feasible to defer this until diagnostic state information for one of the
> module's files is needed, but that's not part of this patch.
>
> Added:
> cfe/trunk/test/Modules/diag-pragma.cpp
> Modified:
> cfe/trunk/include/clang/Basic/Diagnostic.h
> cfe/trunk/lib/Basic/Diagnostic.cpp
> cfe/trunk/lib/Basic/DiagnosticIDs.cpp
> cfe/trunk/lib/Serialization/ASTReader.cpp
> cfe/trunk/lib/Serialization/ASTWriter.cpp
> cfe/trunk/test/Modules/Inputs/diag_pragma.h
>
> Modified: cfe/trunk/include/clang/Basic/Diagnostic.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/include/
clang/Basic/Diagnostic.h?rev=293123&r1=293122&r2=293123&view=diff
> ============================================================
==================
> --- cfe/trunk/include/clang/Basic/Diagnostic.h (original)
> +++ cfe/trunk/include/clang/Basic/Diagnostic.h Wed Jan 25 19:01:01 2017
> @@ -29,6 +29,7 @@
> #include <cassert>
> #include <cstdint>
> #include <list>
> +#include <map>
> #include <memory>
> #include <string>
> #include <type_traits>
> @@ -232,40 +233,97 @@ private:
> /// \brief Keeps and automatically disposes all DiagStates that we
create.
> std::list<DiagState> DiagStates;
>
> - /// \brief Represents a point in source where the diagnostic state was
> - /// modified because of a pragma.
> - ///
> - /// 'Loc' can be null if the point represents the diagnostic state
> - /// modifications done through the command-line.
> - struct DiagStatePoint {
> - DiagState *State;
> - SourceLocation Loc;
> - DiagStatePoint(DiagState *State, SourceLocation Loc)
> - : State(State), Loc(Loc) { }
> + /// A mapping from files to the diagnostic states for those files.
Lazily
> + /// built on demand for files in which the diagnostic state has not
changed.
> + class DiagStateMap {
> + public:
> + /// Add an initial diagnostic state.
> + void appendFirst(DiagState *State);
> + /// Add a new latest state point.
> + void append(SourceManager &SrcMgr, SourceLocation Loc, DiagState
*State);
> + /// Look up the diagnostic state at a given source location.
> + DiagState *lookup(SourceManager &SrcMgr, SourceLocation Loc) const;
> + /// Determine whether this map is empty.
> + bool empty() const { return Files.empty(); }
> + /// Clear out this map.
> + void clear() {
> + Files.clear();
> + FirstDiagState = CurDiagState = nullptr;
> + CurDiagStateLoc = SourceLocation();
> + }
> +
> + /// Grab the most-recently-added state point.
> + DiagState *getCurDiagState() const { return CurDiagState; }
> + /// Get the location at which a diagnostic state was last added.
> + SourceLocation getCurDiagStateLoc() const { return CurDiagStateLoc; }
> +
> + private:
> + /// \brief Represents a point in source where the diagnostic state
was
> + /// modified because of a pragma.
> + ///
> + /// 'Loc' can be null if the point represents the diagnostic state
> + /// modifications done through the command-line.
> + struct DiagStatePoint {
> + DiagState *State;
> + unsigned Offset;
> + DiagStatePoint(DiagState *State, unsigned Offset)
> + : State(State), Offset(Offset) { }
> + };
> +
> + /// Description of the diagnostic states and state transitions for a
> + /// particular FileID.
> + struct File {
> + /// The diagnostic state for the parent file. This is strictly
redundant,
> + /// as looking up the DecomposedIncludedLoc for the FileID in the
Files
> + /// map would give us this, but we cache it here for performance.
> + File *Parent = nullptr;
> + /// The offset of this file within its parent.
> + unsigned ParentOffset = 0;
> + /// Whether this file has any local (not imported from an AST file)
> + /// diagnostic state transitions.
> + bool HasLocalTransitions = false;
> + /// The points within the file where the state changes. There will
always
> + /// be at least one of these (the state on entry to the file).
> + llvm::SmallVector<DiagStatePoint, 4> StateTransitions;
> +
> + DiagState *lookup(unsigned Offset) const;
> + };
> +
> + /// The diagnostic states for each file.
> + mutable std::map<FileID, File> Files;
> +
> + /// The initial diagnostic state.
> + DiagState *FirstDiagState;
> + /// The current diagnostic state.
> + DiagState *CurDiagState;
> + /// The location at which the current diagnostic state was
established.
> + SourceLocation CurDiagStateLoc;
> +
> + /// Get the diagnostic state information for a file.
> + File *getFile(SourceManager &SrcMgr, FileID ID) const;
> +
> + friend class ASTReader;
> + friend class ASTWriter;
> };
>
> - /// \brief A sorted vector of all DiagStatePoints representing changes
in
> - /// diagnostic state due to diagnostic pragmas.
> - ///
> - /// The vector is always sorted according to the SourceLocation of the
> - /// DiagStatePoint.
> - typedef std::vector<DiagStatePoint> DiagStatePointsTy;
> - mutable DiagStatePointsTy DiagStatePoints;
> + DiagStateMap DiagStatesByLoc;
>
> /// \brief Keeps the DiagState that was active during each diagnostic
'push'
> /// so we can get back at it when we 'pop'.
> std::vector<DiagState *> DiagStateOnPushStack;
>
> DiagState *GetCurDiagState() const {
> - assert(!DiagStatePoints.empty());
> - return DiagStatePoints.back().State;
> + return DiagStatesByLoc.getCurDiagState();
> }
>
> void PushDiagStatePoint(DiagState *State, SourceLocation L);
>
> /// \brief Finds the DiagStatePoint that contains the diagnostic state
of
> /// the given source location.
> - DiagStatePointsTy::iterator GetDiagStatePointForLoc(SourceLocation
Loc) const;
> + DiagState *GetDiagStateForLoc(SourceLocation Loc) const {
> + return SourceMgr ? DiagStatesByLoc.lookup(*SourceMgr, Loc)
> + : DiagStatesByLoc.getCurDiagState();
> + }
>
> /// \brief Sticky flag set to \c true when an error is emitted.
> bool ErrorOccurred;
> @@ -372,7 +430,7 @@ public:
> return *SourceMgr;
> }
> void setSourceManager(SourceManager *SrcMgr) {
> - assert(DiagStatePoints.size() == 1 && DiagStatePoints[0].Loc.isInvalid()
&&
> + assert(DiagStatesByLoc.empty() &&
> "Leftover diag state from a different SourceManager.");
> SourceMgr = SrcMgr;
> }
>
> Modified: cfe/trunk/lib/Basic/Diagnostic.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/
Diagnostic.cpp?rev=293123&r1=293122&r2=293123&view=diff
> ============================================================
==================
> --- cfe/trunk/lib/Basic/Diagnostic.cpp (original)
> +++ cfe/trunk/lib/Basic/Diagnostic.cpp Wed Jan 25 19:01:01 2017
> @@ -132,13 +132,13 @@ void DiagnosticsEngine::Reset() {
>
> // Clear state related to #pragma diagnostic.
> DiagStates.clear();
> - DiagStatePoints.clear();
> + DiagStatesByLoc.clear();
> DiagStateOnPushStack.clear();
>
> // Create a DiagState and DiagStatePoint representing diagnostic changes
> // through command-line.
> DiagStates.emplace_back();
> - DiagStatePoints.emplace_back(&DiagStates.back(), SourceLocation());
> + DiagStatesByLoc.appendFirst(&DiagStates.back());
> }
>
> void DiagnosticsEngine::SetDelayedDiagnostic(unsigned DiagID, StringRef
Arg1,
> @@ -158,49 +158,94 @@ void DiagnosticsEngine::ReportDelayed()
> DelayedDiagArg2.clear();
> }
>
> +void DiagnosticsEngine::DiagStateMap::appendFirst(
> + DiagState *State) {
> + assert(Files.empty() && "not first");
> + FirstDiagState = CurDiagState = State;
> + CurDiagStateLoc = SourceLocation();
> +}
> +
> +void DiagnosticsEngine::DiagStateMap::append(SourceManager &SrcMgr,
> + SourceLocation Loc,
> + DiagState *State) {
> + CurDiagState = State;
> + CurDiagStateLoc = Loc;
> +
> + std::pair<FileID, unsigned> Decomp = SrcMgr.getDecomposedLoc(Loc);
> + unsigned Offset = Decomp.second;
> + for (File *F = getFile(SrcMgr, Decomp.first); F;
> + Offset = F->ParentOffset, F = F->Parent) {
> + F->HasLocalTransitions = true;
> + auto &Last = F->StateTransitions.back();
> + assert(Last.Offset <= Offset && "state transitions added out of
order");
> +
> + if (Last.Offset == Offset) {
> + if (Last.State == State)
> + break;
> + Last.State = State;
> + continue;
> + }
> +
> + F->StateTransitions.push_back({State, Offset});
> + }
> +}
> +
> +DiagnosticsEngine::DiagState *
> +DiagnosticsEngine::DiagStateMap::lookup(SourceManager &SrcMgr,
> + SourceLocation Loc) const {
> + // Common case: we have not seen any diagnostic pragmas.
> + if (Files.empty())
> + return FirstDiagState;
> +
> + std::pair<FileID, unsigned> Decomp = SrcMgr.getDecomposedLoc(Loc);
> + const File *F = getFile(SrcMgr, Decomp.first);
> + return F->lookup(Decomp.second);
> +}
> +
> +DiagnosticsEngine::DiagState *
> +DiagnosticsEngine::DiagStateMap::File::lookup(unsigned Offset) const {
> + auto OnePastIt = std::upper_bound(
> + StateTransitions.begin(), StateTransitions.end(), Offset,
> + [](unsigned Offset, const DiagStatePoint &P) {
> + return Offset < P.Offset;
> + });
> + assert(OnePastIt != StateTransitions.begin() && "missing initial
state");
> + return OnePastIt[-1].State;
> +}
> +
> +DiagnosticsEngine::DiagStateMap::File *
> +DiagnosticsEngine::DiagStateMap::getFile(SourceManager &SrcMgr,
> + FileID ID) const {
> + // Get or insert the File for this ID.
> + auto Range = Files.equal_range(ID);
> + if (Range.first != Range.second)
> + return &Range.first->second;
> + auto &F = Files.insert(Range.first, std::make_pair(ID,
File()))->second;
> +
> + // We created a new File; look up the diagnostic state at the start of
it and
> + // initialize it.
> + if (ID.isValid()) {
> + std::pair<FileID, unsigned> Decomp = SrcMgr.
getDecomposedIncludedLoc(ID);
> + F.Parent = getFile(SrcMgr, Decomp.first);
> + F.ParentOffset = Decomp.second;
> + F.StateTransitions.push_back({F.Parent->lookup(Decomp.second), 0});
> + } else {
> + // This is the (imaginary) root file into which we pretend all
top-level
> + // files are included; it descends from the initial state.
> + //
> + // FIXME: This doesn't guarantee that we use the same ordering as
> + // isBeforeInTranslationUnit in the cases where someone invented
another
> + // top-level file and added diagnostic pragmas to it. See the code
at the
> + // end of isBeforeInTranslationUnit for the quirks it deals with.
> + F.StateTransitions.push_back({FirstDiagState, 0});
> + }
> + return &F;
> +}
> +
> void DiagnosticsEngine::PushDiagStatePoint(DiagState *State,
> SourceLocation Loc) {
> - // Make sure that DiagStatePoints is always sorted according to Loc.
> assert(Loc.isValid() && "Adding invalid loc point");
> - assert(!DiagStatePoints.empty() &&
> - (DiagStatePoints.back().Loc.isInvalid() ||
> - getSourceManager().isBeforeInTranslationUnit(
> - DiagStatePoints.back().Loc, Loc)) &&
> - "Previous point loc comes after or is the same as new one");
> - DiagStatePoints.push_back(DiagStatePoint(State, Loc));
> -}
> -
> -DiagnosticsEngine::DiagStatePointsTy::iterator
> -DiagnosticsEngine::GetDiagStatePointForLoc(SourceLocation L) const {
> - assert(!DiagStatePoints.empty());
> - assert(DiagStatePoints.front().Loc.isInvalid() &&
> - "Should have created a DiagStatePoint for command-line");
> -
> - if (!SourceMgr)
> - return DiagStatePoints.end() - 1;
> -
> - FullSourceLoc Loc(L, *SourceMgr);
> - if (Loc.isInvalid())
> - return DiagStatePoints.end() - 1;
> -
> - DiagStatePointsTy::iterator Pos = DiagStatePoints.end();
> - SourceLocation LastStateChangePos = DiagStatePoints.back().Loc;
> - if (LastStateChangePos.isValid() &&
> - Loc.isBeforeInTranslationUnitThan(LastStateChangePos))
> - Pos = std::upper_bound(
> - DiagStatePoints.begin(), DiagStatePoints.end(),
> - DiagStatePoint(nullptr, Loc),
> - [this](const DiagStatePoint &LHS, const DiagStatePoint &RHS) {
> - // If Loc is invalid it means it came from <command-line>, in
which
> - // case we regard it as coming before any valid source
location.
> - if (RHS.Loc.isInvalid())
> - return false;
> - if (LHS.Loc.isInvalid())
> - return true;
> - return SourceMgr->isBeforeInTranslationUnit(LHS.Loc, RHS.Loc);
> - });
> - --Pos;
> - return Pos;
> + DiagStatesByLoc.append(*SourceMgr, Loc, State);
> }
>
> void DiagnosticsEngine::setSeverity(diag::kind Diag, diag::Severity Map,
> @@ -210,11 +255,8 @@ void DiagnosticsEngine::setSeverity(diag
> assert((Diags->isBuiltinWarningOrExtension(Diag) ||
> (Map == diag::Severity::Fatal || Map == diag::Severity::Error))
&&
> "Cannot map errors into warnings!");
> - assert(!DiagStatePoints.empty());
> assert((L.isInvalid() || SourceMgr) && "No SourceMgr for valid
location");
>
> - SourceLocation Loc = SourceMgr ? L : SourceLocation();
> - SourceLocation LastStateChangePos = DiagStatePoints.back().Loc;
> // Don't allow a mapping to a warning override an error/fatal mapping.
> if (Map == diag::Severity::Warning) {
> DiagnosticMapping &Info = GetCurDiagState()->getOrAddMapping(Diag);
> @@ -225,49 +267,22 @@ void DiagnosticsEngine::setSeverity(diag
> DiagnosticMapping Mapping = makeUserMapping(Map, L);
>
> // Common case; setting all the diagnostics of a group in one place.
> - if (Loc.isInvalid() || Loc == LastStateChangePos) {
> - GetCurDiagState()->setMapping(Diag, Mapping);
> - return;
> - }
> -
> - // Another common case; modifying diagnostic state in a source location
> - // after the previous one.
> - if ((Loc.isValid() && LastStateChangePos.isInvalid()) ||
> - SourceMgr->isBeforeInTranslationUnit(LastStateChangePos, Loc)) {
> - // A diagnostic pragma occurred, create a new DiagState initialized
with
> - // the current one and a new DiagStatePoint to record at which
location
> - // the new state became active.
> - DiagStates.push_back(*GetCurDiagState());
> - PushDiagStatePoint(&DiagStates.back(), Loc);
> - GetCurDiagState()->setMapping(Diag, Mapping);
> - return;
> - }
> -
> - // We allow setting the diagnostic state in random source order for
> - // completeness but it should not be actually happening in normal
practice.
> -
> - DiagStatePointsTy::iterator Pos = GetDiagStatePointForLoc(Loc);
> - assert(Pos != DiagStatePoints.end());
> -
> - // Update all diagnostic states that are active after the given
location.
> - for (DiagStatePointsTy::iterator
> - I = Pos+1, E = DiagStatePoints.end(); I != E; ++I) {
> - I->State->setMapping(Diag, Mapping);
> - }
> -
> - // If the location corresponds to an existing point, just update its
state.
> - if (Pos->Loc == Loc) {
> - Pos->State->setMapping(Diag, Mapping);
> + if ((L.isInvalid() || L == DiagStatesByLoc.getCurDiagStateLoc()) &&
> + DiagStatesByLoc.getCurDiagState()) {
> + // FIXME: This is theoretically wrong: if the current state is
shared with
> + // some other location (via push/pop) we will change the state for
that
> + // other location as well. This cannot currently happen, as we can't
update
> + // the diagnostic state at the same location at which we pop.
> + DiagStatesByLoc.getCurDiagState()->setMapping(Diag, Mapping);
> return;
> }
>
> - // Create a new state/point and fit it into the vector of
DiagStatePoints
> - // so that the vector is always ordered according to location.
> - assert(SourceMgr->isBeforeInTranslationUnit(Pos->Loc, Loc));
> - DiagStates.push_back(*Pos->State);
> - DiagState *NewState = &DiagStates.back();
> - NewState->setMapping(Diag, Mapping);
> - DiagStatePoints.insert(Pos + 1, DiagStatePoint(NewState, Loc));
> + // A diagnostic pragma occurred, create a new DiagState initialized
with
> + // the current one and a new DiagStatePoint to record at which location
> + // the new state became active.
> + DiagStates.push_back(*GetCurDiagState());
> + DiagStates.back().setMapping(Diag, Mapping);
> + PushDiagStatePoint(&DiagStates.back(), L);
> }
>
> bool DiagnosticsEngine::setSeverityForGroup(diag::Flavor Flavor,
>
> Modified: cfe/trunk/lib/Basic/DiagnosticIDs.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/Basic/
DiagnosticIDs.cpp?rev=293123&r1=293122&r2=293123&view=diff
> ============================================================
==================
> --- cfe/trunk/lib/Basic/DiagnosticIDs.cpp (original)
> +++ cfe/trunk/lib/Basic/DiagnosticIDs.cpp Wed Jan 25 19:01:01 2017
> @@ -411,11 +411,8 @@ DiagnosticIDs::getDiagnosticSeverity(uns
> // to error. Errors can only be mapped to fatal.
> diag::Severity Result = diag::Severity::Fatal;
>
> - DiagnosticsEngine::DiagStatePointsTy::iterator
> - Pos = Diag.GetDiagStatePointForLoc(Loc);
> - DiagnosticsEngine::DiagState *State = Pos->State;
> -
> // Get the mapping information, or compute it lazily.
> + DiagnosticsEngine::DiagState *State = Diag.GetDiagStateForLoc(Loc);
> DiagnosticMapping &Mapping = State->getOrAddMapping((diag::kind)DiagID);
>
> // TODO: Can a null severity really get here?
>
> Modified: cfe/trunk/lib/Serialization/ASTReader.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/
Serialization/ASTReader.cpp?rev=293123&r1=293122&r2=293123&view=diff
> ============================================================
==================
> --- cfe/trunk/lib/Serialization/ASTReader.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTReader.cpp Wed Jan 25 19:01:01 2017
> @@ -5297,48 +5297,84 @@ HeaderFileInfo ASTReader::GetHeaderFileI
> }
>
> void ASTReader::ReadPragmaDiagnosticMappings(DiagnosticsEngine &Diag) {
> - // FIXME: Make it work properly with modules.
> - SmallVector<DiagnosticsEngine::DiagState *, 32> DiagStates;
> + using DiagState = DiagnosticsEngine::DiagState;
> + SmallVector<DiagState *, 32> DiagStates;
> +
> for (ModuleIterator I = ModuleMgr.begin(), E = ModuleMgr.end(); I != E;
++I) {
> ModuleFile &F = *(*I);
> unsigned Idx = 0;
> + auto &Record = F.PragmaDiagMappings;
> + if (Record.empty())
> + continue;
> +
> DiagStates.clear();
> - assert(!Diag.DiagStates.empty());
> - DiagStates.push_back(&Diag.DiagStates.front()); // the command-line
one.
> - while (Idx < F.PragmaDiagMappings.size()) {
> - SourceLocation Loc = ReadSourceLocation(F,
F.PragmaDiagMappings[Idx++]);
> - unsigned DiagStateID = F.PragmaDiagMappings[Idx++];
> - if (DiagStateID != 0) {
> - Diag.DiagStatePoints.push_back(
> - DiagnosticsEngine::DiagStatePoint(DiagStates[
DiagStateID-1],
> - FullSourceLoc(Loc, SourceMgr)));
> - continue;
> - }
>
> - assert(DiagStateID == 0);
> + auto ReadDiagState =
> + [&](const DiagState &BasedOn, SourceLocation Loc,
> + bool IncludeNonPragmaStates) -> DiagnosticsEngine::DiagState
* {
> + unsigned BackrefID = Record[Idx++];
> + if (BackrefID != 0)
> + return DiagStates[BackrefID - 1];
> +
> // A new DiagState was created here.
> - Diag.DiagStates.push_back(*Diag.GetCurDiagState());
> - DiagnosticsEngine::DiagState *NewState = &Diag.DiagStates.back();
> + Diag.DiagStates.push_back(BasedOn);
> + DiagState *NewState = &Diag.DiagStates.back();
> DiagStates.push_back(NewState);
> - Diag.DiagStatePoints.push_back(
> - DiagnosticsEngine::DiagStatePoint(NewState,
> - FullSourceLoc(Loc,
SourceMgr)));
> - while (true) {
> - assert(Idx < F.PragmaDiagMappings.size() &&
> - "Invalid data, didn't find '-1' marking end of diag/map
pairs");
> - if (Idx >= F.PragmaDiagMappings.size()) {
> - break; // Something is messed up but at least avoid infinite
loop in
> - // release build.
> - }
> - unsigned DiagID = F.PragmaDiagMappings[Idx++];
> - if (DiagID == (unsigned)-1) {
> - break; // no more diag/map pairs for this location.
> - }
> - diag::Severity Map = (diag::Severity)F.PragmaDiagMappings[Idx++];
> + while (Idx + 1 < Record.size() && Record[Idx] != unsigned(-1)) {
> + unsigned DiagID = Record[Idx++];
> + diag::Severity Map = (diag::Severity)Record[Idx++];
> DiagnosticMapping Mapping = Diag.makeUserMapping(Map, Loc);
> - Diag.GetCurDiagState()->setMapping(DiagID, Mapping);
> + if (Mapping.isPragma() || IncludeNonPragmaStates)
> + NewState->setMapping(DiagID, Mapping);
> + }
> + assert(Idx != Record.size() && Record[Idx] == unsigned(-1) &&
> + "Invalid data, didn't find '-1' marking end of diag/map
pairs");
> + ++Idx;
> + return NewState;
> + };
> +
> + auto *FirstState = ReadDiagState(
> + F.isModule() ? DiagState() : *Diag.DiagStatesByLoc.CurDiagState,
> + SourceLocation(), F.isModule());
> + SourceLocation CurStateLoc =
> + ReadSourceLocation(F, F.PragmaDiagMappings[Idx++]);
> + auto *CurState = ReadDiagState(*FirstState, CurStateLoc, false);
> +
> + if (!F.isModule()) {
> + Diag.DiagStatesByLoc.CurDiagState = CurState;
> + Diag.DiagStatesByLoc.CurDiagStateLoc = CurStateLoc;
> +
> + // Preserve the property that the imaginary root file describes the
> + // current state.
> + auto &T = Diag.DiagStatesByLoc.Files[FileID()].StateTransitions;
> + if (T.empty())
> + T.push_back({CurState, 0});
> + else
> + T[0].State = CurState;
> + }
> +
> + while (Idx < Record.size()) {
> + SourceLocation Loc = ReadSourceLocation(F, Record[Idx++]);
> + auto IDAndOffset = SourceMgr.getDecomposedLoc(Loc);
> + assert(IDAndOffset.second == 0 && "not a start location for a
FileID");
> + unsigned Transitions = Record[Idx++];
> +
> + // Note that we don't need to set up Parent/ParentOffset here,
because
> + // we won't be changing the diagnostic state within imported
FileIDs
> + // (other than perhaps appending to the main source file, which
has no
> + // parent).
> + auto &F = Diag.DiagStatesByLoc.Files[IDAndOffset.first];
> + F.StateTransitions.reserve(F.StateTransitions.size() +
Transitions);
> + for (unsigned I = 0; I != Transitions; ++I) {
> + unsigned Offset = Record[Idx++];
> + auto *State =
> + ReadDiagState(*FirstState, Loc.getLocWithOffset(Offset),
false);
> + F.StateTransitions.push_back({State, Offset});
> }
> }
> +
> + // Don't try to read these mappings again.
> + Record.clear();
> }
> }
>
>
> Modified: cfe/trunk/lib/Serialization/ASTWriter.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/lib/
Serialization/ASTWriter.cpp?rev=293123&r1=293122&r2=293123&view=diff
> ============================================================
==================
> --- cfe/trunk/lib/Serialization/ASTWriter.cpp (original)
> +++ cfe/trunk/lib/Serialization/ASTWriter.cpp Wed Jan 25 19:01:01 2017
> @@ -2802,38 +2802,43 @@ ASTWriter::inferSubmoduleIDFromLocation(
>
> void ASTWriter::WritePragmaDiagnosticMappings(const DiagnosticsEngine
&Diag,
> bool isModule) {
> - // Make sure set diagnostic pragmas don't affect the translation unit
that
> - // imports the module.
> - // FIXME: Make diagnostic pragma sections work properly with modules.
> - if (isModule)
> - return;
> -
> llvm::SmallDenseMap<const DiagnosticsEngine::DiagState *, unsigned, 64>
> DiagStateIDMap;
> unsigned CurrID = 0;
> - DiagStateIDMap[&Diag.DiagStates.front()] = ++CurrID; // the
command-line one.
> RecordData Record;
> - for (DiagnosticsEngine::DiagStatePointsTy::const_iterator
> - I = Diag.DiagStatePoints.begin(), E =
Diag.DiagStatePoints.end();
> - I != E; ++I) {
> - const DiagnosticsEngine::DiagStatePoint &point = *I;
> - if (point.Loc.isInvalid())
> - continue;
>
> - AddSourceLocation(point.Loc, Record);
> - unsigned &DiagStateID = DiagStateIDMap[point.State];
> + auto AddDiagState = [&](const DiagnosticsEngine::DiagState *State,
> + bool IncludeNonPragmaStates) {
> + unsigned &DiagStateID = DiagStateIDMap[State];
> Record.push_back(DiagStateID);
> -
> +
> if (DiagStateID == 0) {
> DiagStateID = ++CurrID;
> - for (const auto &I : *(point.State)) {
> - if (I.second.isPragma()) {
> + for (const auto &I : *State) {
> + if (I.second.isPragma() || IncludeNonPragmaStates) {
> Record.push_back(I.first);
> Record.push_back((unsigned)I.second.getSeverity());
> }
> }
> - Record.push_back(-1); // mark the end of the diag/map pairs for
this
> - // location.
> + // Add a sentinel to mark the end of the diag IDs.
> + Record.push_back(unsigned(-1));
> + }
> + };
> +
> + AddDiagState(Diag.DiagStatesByLoc.FirstDiagState, isModule);
> + AddSourceLocation(Diag.DiagStatesByLoc.CurDiagStateLoc, Record);
> + AddDiagState(Diag.DiagStatesByLoc.CurDiagState, false);
> +
> + for (auto &FileIDAndFile : Diag.DiagStatesByLoc.Files) {
> + if (!FileIDAndFile.first.isValid() ||
> + !FileIDAndFile.second.HasLocalTransitions)
> + continue;
> + AddSourceLocation(Diag.SourceMgr->getLocForStartOfFile(
FileIDAndFile.first),
> + Record);
> + Record.push_back(FileIDAndFile.second.StateTransitions.size());
> + for (auto &StatePoint : FileIDAndFile.second.StateTransitions) {
> + Record.push_back(StatePoint.Offset);
> + AddDiagState(StatePoint.State, false);
> }
> }
>
>
> Modified: cfe/trunk/test/Modules/Inputs/diag_pragma.h
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
Modules/Inputs/diag_pragma.h?rev=293123&r1=293122&r2=293123&view=diff
> ============================================================
==================
> --- cfe/trunk/test/Modules/Inputs/diag_pragma.h (original)
> +++ cfe/trunk/test/Modules/Inputs/diag_pragma.h Wed Jan 25 19:01:01 2017
> @@ -1,3 +1,13 @@
> #define DIAG_PRAGMA_MACRO 1
>
> #pragma clang diagnostic ignored "-Wparentheses"
> +
> +#ifdef __cplusplus
> +template<typename T> const char *f(T t) {
> + return "foo" + t;
> +}
> +#pragma clang diagnostic ignored "-Wstring-plus-int"
> +template<typename T> const char *g(T t) {
> + return "foo" + t;
> +}
> +#endif
>
> Added: cfe/trunk/test/Modules/diag-pragma.cpp
> URL: http://llvm.org/viewvc/llvm-project/cfe/trunk/test/
Modules/diag-pragma.cpp?rev=293123&view=auto
> ============================================================
==================
> --- cfe/trunk/test/Modules/diag-pragma.cpp (added)
> +++ cfe/trunk/test/Modules/diag-pragma.cpp Wed Jan 25 19:01:01 2017
> @@ -0,0 +1,47 @@
> +// RUN: rm -rf %t
> +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -emit-module
-fmodules-cache-path=%t -fmodule-name=diag_pragma -x c++
%S/Inputs/module.map -fmodules-ts
> +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -verify
-fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts
> +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -emit-module
-fmodule-name=diag_pragma -x c++ %S/Inputs/module.map -fmodules-ts -o
%t/explicit.pcm -Werror=string-plus-int
> +// RUN: %clang_cc1 -fmodules -fimplicit-module-maps -verify
-fmodules-cache-path=%t -I %S/Inputs %s -fmodules-ts -DEXPLICIT_FLAG
-fmodule-file=%t/explicit.pcm
> +
> +import diag_pragma;
> +
> +int foo(int x) {
> + // Diagnostics from templates in the module follow the diagnostic
state from
> + // when the module was built.
> +#ifdef EXPLICIT_FLAG
> + // expected-error at diag_pragma.h:7 {{adding 'int' to a string}}
> +#else
> + // expected-warning at diag_pragma.h:7 {{adding 'int' to a string}}
> +#endif
> + // expected-note at diag_pragma.h:7 {{use array indexing}}
> + f(0); // expected-note {{instantiation of}}
> +
> + g(0); // ok, warning was ignored when building module
> +
> + // Diagnostics from this source file ignore the diagnostic state from
the
> + // module.
> + void("foo" + x); // expected-warning {{adding 'int' to a string}}
> + // expected-note at -1 {{use array indexing}}
> +
> +#pragma clang diagnostic ignored "-Wstring-plus-int"
> +
> + // Diagnostics from the module ignore diagnostic state changes from
this
> + // source file.
> +#ifdef EXPLICIT_FLAG
> + // expected-error at diag_pragma.h:7 {{adding 'long' to a string}}
> +#else
> + // expected-warning at diag_pragma.h:7 {{adding 'long' to a string}}
> +#endif
> + // expected-note at diag_pragma.h:7 {{use array indexing}}
> + f(0L); // expected-note {{instantiation of}}
> +
> + g(0L);
> +
> + void("bar" + x);
> +
> + if (x = DIAG_PRAGMA_MACRO) // expected-warning {{using the result of
an assignment as a condition without parentheses}} \
> + // expected-note {{place parentheses}}
expected-note {{use '=='}}
> + return 0;
> + return 1;
> +}
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at lists.llvm.org
> http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
_______________________________________________
cfe-commits mailing list
cfe-commits at lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20170128/feb93cb7/attachment-0001.html>
More information about the cfe-commits
mailing list