r293123 - Remove and replace DiagStatePoint tracking and lookup data structure.
Duncan P. N. Exon Smith via cfe-commits
cfe-commits at lists.llvm.org
Sat Jan 28 20:16:34 PST 2017
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?
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
More information about the cfe-commits
mailing list