aaron.ballman created this revision.
aaron.ballman added reviewers: rsmith, dblaikie, echristo.
Herald added subscribers: jsji, kbarton, nemanjai.
While working on https://reviews.llvm.org/D54349, it was noted that the
`SourceRange` returned from the preprocessor callbacks was bogus. It was
expected to pass the source range for the condition to a `#if` or `#elif`
directive, but what was actually passed was a much larger range that could even
include all of the conditionally skipped tokens.
This patch adjusts the source range passed in to the callbacks to only include
the condition range itself by tracking that information a bit better in the
preprocessor.
https://reviews.llvm.org/D54450
Files:
include/clang/Lex/Preprocessor.h
lib/Lex/PPDirectives.cpp
lib/Lex/PPExpressions.cpp
unittests/Lex/PPCallbacksTest.cpp
Index: unittests/Lex/PPCallbacksTest.cpp
===================================================================
--- unittests/Lex/PPCallbacksTest.cpp
+++ unittests/Lex/PPCallbacksTest.cpp
@@ -65,6 +65,29 @@
SrcMgr::CharacteristicKind FileType;
};
+class CondDirectiveCallbacks : public PPCallbacks {
+public:
+ struct Result {
+ SourceRange ConditionRange;
+ ConditionValueKind ConditionValue;
+
+ Result(SourceRange R, ConditionValueKind K)
+ : ConditionRange(R), ConditionValue(K) {}
+ };
+
+ std::vector<Result> Results;
+
+ void If(SourceLocation Loc, SourceRange ConditionRange,
+ ConditionValueKind ConditionValue) override {
+ Results.emplace_back(ConditionRange, ConditionValue);
+ }
+
+ void Elif(SourceLocation Loc, SourceRange ConditionRange,
+ ConditionValueKind ConditionValue, SourceLocation IfLoc) override {
+ Results.emplace_back(ConditionRange, ConditionValue);
+ }
+};
+
// Stub to collect data from PragmaOpenCLExtension callbacks.
class PragmaOpenCLExtensionCallbacks : public PPCallbacks {
public:
@@ -137,6 +160,15 @@
return StringRef(B, E - B);
}
+ StringRef GetSourceStringToEnd(CharSourceRange Range) {
+ const char *B = SourceMgr.getCharacterData(Range.getBegin());
+ const char *E = SourceMgr.getCharacterData(Range.getEnd());
+
+ return StringRef(
+ B,
+ E - B + Lexer::MeasureTokenLength(Range.getEnd(), SourceMgr, LangOpts));
+ }
+
// Run lexer over SourceText and collect FilenameRange from
// the InclusionDirective callback.
CharSourceRange InclusionDirectiveFilenameRange(const char *SourceText,
@@ -199,6 +231,36 @@
return Callbacks;
}
+ std::vector<CondDirectiveCallbacks::Result>
+ DirectiveExprRange(StringRef SourceText) {
+ TrivialModuleLoader ModLoader;
+ MemoryBufferCache PCMCache;
+ std::unique_ptr<llvm::MemoryBuffer> Buf =
+ llvm::MemoryBuffer::getMemBuffer(SourceText);
+ SourceMgr.setMainFileID(SourceMgr.createFileID(std::move(Buf)));
+ HeaderSearch HeaderInfo(std::make_shared<HeaderSearchOptions>(), SourceMgr,
+ Diags, LangOpts, Target.get());
+ Preprocessor PP(std::make_shared<PreprocessorOptions>(), Diags, LangOpts,
+ SourceMgr, PCMCache, HeaderInfo, ModLoader,
+ /*IILookup =*/nullptr,
+ /*OwnsHeaderSearch =*/false);
+ PP.Initialize(*Target);
+ auto *Callbacks = new CondDirectiveCallbacks;
+ PP.addPPCallbacks(std::unique_ptr<PPCallbacks>(Callbacks));
+
+ // Lex source text.
+ PP.EnterMainSourceFile();
+
+ while (true) {
+ Token Tok;
+ PP.Lex(Tok);
+ if (Tok.is(tok::eof))
+ break;
+ }
+
+ return Callbacks->Results;
+ }
+
PragmaOpenCLExtensionCallbacks::CallbackParameters
PragmaOpenCLExtensionCall(const char *SourceText) {
LangOptions OpenCLLangOpts;
@@ -368,4 +430,42 @@
ASSERT_EQ(ExpectedState, Parameters.State);
}
-} // anonoymous namespace
+TEST_F(PPCallbacksTest, DirectiveExprRanges) {
+ const auto &Results1 = DirectiveExprRange("#if FLUZZY_FLOOF\n#endif\n");
+ EXPECT_EQ(Results1.size(), 1);
+ EXPECT_EQ(
+ GetSourceStringToEnd(CharSourceRange(Results1[0].ConditionRange, false)),
+ "FLUZZY_FLOOF");
+
+ const auto &Results2 = DirectiveExprRange("#if 1 + 4 < 7\n#endif\n");
+ EXPECT_EQ(Results2.size(), 1);
+ EXPECT_EQ(
+ GetSourceStringToEnd(CharSourceRange(Results2[0].ConditionRange, false)),
+ "1 + 4 < 7");
+
+ const auto &Results3 = DirectiveExprRange("#if 1 + \\\n 2\n#endif\n");
+ EXPECT_EQ(Results3.size(), 1);
+ EXPECT_EQ(
+ GetSourceStringToEnd(CharSourceRange(Results3[0].ConditionRange, false)),
+ "1 + \\\n 2");
+
+ const auto &Results4 = DirectiveExprRange("#if 0\n#elif FLOOFY\n#endif\n");
+ EXPECT_EQ(Results4.size(), 2);
+ EXPECT_EQ(
+ GetSourceStringToEnd(CharSourceRange(Results4[0].ConditionRange, false)),
+ "0");
+ EXPECT_EQ(
+ GetSourceStringToEnd(CharSourceRange(Results4[1].ConditionRange, false)),
+ "FLOOFY");
+
+ const auto &Results5 = DirectiveExprRange("#if 1\n#elif FLOOFY\n#endif\n");
+ EXPECT_EQ(Results5.size(), 2);
+ EXPECT_EQ(
+ GetSourceStringToEnd(CharSourceRange(Results5[0].ConditionRange, false)),
+ "1");
+ EXPECT_EQ(
+ GetSourceStringToEnd(CharSourceRange(Results5[1].ConditionRange, false)),
+ "FLOOFY");
+}
+
+} // namespace
Index: lib/Lex/PPExpressions.cpp
===================================================================
--- lib/Lex/PPExpressions.cpp
+++ lib/Lex/PPExpressions.cpp
@@ -863,7 +863,7 @@
// Restore 'DisableMacroExpansion'.
DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
- return {ResVal.Val != 0, DT.IncludedUndefinedIds};
+ return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()};
}
// Otherwise, we must have a binary operator (e.g. "#if 1 < 2"), so parse the
@@ -876,7 +876,7 @@
// Restore 'DisableMacroExpansion'.
DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
- return {false, DT.IncludedUndefinedIds};
+ return {false, DT.IncludedUndefinedIds, ResVal.getRange()};
}
// If we aren't at the tok::eod token, something bad happened, like an extra
@@ -888,5 +888,5 @@
// Restore 'DisableMacroExpansion'.
DisableMacroExpansion = DisableMacroExpansionAtStartOfDirective;
- return {ResVal.Val != 0, DT.IncludedUndefinedIds};
+ return {ResVal.Val != 0, DT.IncludedUndefinedIds, ResVal.getRange()};
}
Index: lib/Lex/PPDirectives.cpp
===================================================================
--- lib/Lex/PPDirectives.cpp
+++ lib/Lex/PPDirectives.cpp
@@ -80,12 +80,18 @@
/// Read and discard all tokens remaining on the current line until
/// the tok::eod token is found.
-void Preprocessor::DiscardUntilEndOfDirective() {
+SourceRange Preprocessor::DiscardUntilEndOfDirective() {
Token Tmp;
- do {
- LexUnexpandedToken(Tmp);
+ SourceRange Res;
+
+ LexUnexpandedToken(Tmp);
+ Res.setBegin(Tmp.getLocation());
+ while (Tmp.isNot(tok::eod)) {
assert(Tmp.isNot(tok::eof) && "EOF seen while discarding directive tokens");
- } while (Tmp.isNot(tok::eod));
+ LexUnexpandedToken(Tmp);
+ }
+ Res.setEnd(Tmp.getLocation());
+ return Res;
}
/// Enumerates possible cases of #define/#undef a reserved identifier.
@@ -550,13 +556,15 @@
assert(CurPPLexer->LexingRawMode && "We have to be skipping here!");
CurPPLexer->LexingRawMode = false;
IdentifierInfo *IfNDefMacro = nullptr;
- const bool CondValue = EvaluateDirectiveExpression(IfNDefMacro).Conditional;
+ DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);
+ const bool CondValue = DER.Conditional;
CurPPLexer->LexingRawMode = true;
if (Callbacks) {
const SourceLocation CondEnd = CurPPLexer->getSourceLocation();
- Callbacks->Elif(Tok.getLocation(),
- SourceRange(CondBegin, CondEnd),
- (CondValue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False), CondInfo.IfLoc);
+ Callbacks->Elif(
+ Tok.getLocation(), DER.ExprRange,
+ (CondValue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False),
+ CondInfo.IfLoc);
}
// If this condition is true, enter it!
if (CondValue) {
@@ -1199,19 +1207,24 @@
; // ok
else if (StrTok.isNot(tok::string_literal)) {
Diag(StrTok, diag::err_pp_line_invalid_filename);
- return DiscardUntilEndOfDirective();
+ DiscardUntilEndOfDirective();
+ return;
} else if (StrTok.hasUDSuffix()) {
Diag(StrTok, diag::err_invalid_string_udl);
- return DiscardUntilEndOfDirective();
+ DiscardUntilEndOfDirective();
+ return;
} else {
// Parse and validate the string, converting it into a unique ID.
StringLiteralParser Literal(StrTok, *this);
assert(Literal.isAscii() && "Didn't allow wide strings in");
- if (Literal.hadError)
- return DiscardUntilEndOfDirective();
+ if (Literal.hadError) {
+ DiscardUntilEndOfDirective();
+ return;
+ }
if (Literal.Pascal) {
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
- return DiscardUntilEndOfDirective();
+ DiscardUntilEndOfDirective();
+ return;
}
FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
@@ -1344,19 +1357,24 @@
FileKind = SourceMgr.getFileCharacteristic(DigitTok.getLocation());
} else if (StrTok.isNot(tok::string_literal)) {
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
- return DiscardUntilEndOfDirective();
+ DiscardUntilEndOfDirective();
+ return;
} else if (StrTok.hasUDSuffix()) {
Diag(StrTok, diag::err_invalid_string_udl);
- return DiscardUntilEndOfDirective();
+ DiscardUntilEndOfDirective();
+ return;
} else {
// Parse and validate the string, converting it into a unique ID.
StringLiteralParser Literal(StrTok, *this);
assert(Literal.isAscii() && "Didn't allow wide strings in");
- if (Literal.hadError)
- return DiscardUntilEndOfDirective();
+ if (Literal.hadError) {
+ DiscardUntilEndOfDirective();
+ return;
+ }
if (Literal.Pascal) {
Diag(StrTok, diag::err_pp_linemarker_invalid_filename);
- return DiscardUntilEndOfDirective();
+ DiscardUntilEndOfDirective();
+ return;
}
FilenameID = SourceMgr.getLineTableFilenameID(Literal.GetString());
@@ -1430,7 +1448,8 @@
if (StrTok.hasUDSuffix()) {
Diag(StrTok, diag::err_invalid_string_udl);
- return DiscardUntilEndOfDirective();
+ DiscardUntilEndOfDirective();
+ return;
}
// Verify that there is nothing after the string, other than EOD.
@@ -2872,10 +2891,8 @@
// Parse and evaluate the conditional expression.
IdentifierInfo *IfNDefMacro = nullptr;
- const SourceLocation ConditionalBegin = CurPPLexer->getSourceLocation();
const DirectiveEvalResult DER = EvaluateDirectiveExpression(IfNDefMacro);
const bool ConditionalTrue = DER.Conditional;
- const SourceLocation ConditionalEnd = CurPPLexer->getSourceLocation();
// If this condition is equivalent to #ifndef X, and if this is the first
// directive seen, handle it for the multiple-include optimization.
@@ -2888,9 +2905,9 @@
}
if (Callbacks)
- Callbacks->If(IfToken.getLocation(),
- SourceRange(ConditionalBegin, ConditionalEnd),
- (ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False));
+ Callbacks->If(
+ IfToken.getLocation(), DER.ExprRange,
+ (ConditionalTrue ? PPCallbacks::CVK_True : PPCallbacks::CVK_False));
// Should we include the stuff contained by this directive?
if (PPOpts->SingleFileParseMode && DER.IncludedUndefinedIds) {
@@ -2983,9 +3000,7 @@
// #elif directive in a non-skipping conditional... start skipping.
// We don't care what the condition is, because we will always skip it (since
// the block immediately before it was included).
- const SourceLocation ConditionalBegin = CurPPLexer->getSourceLocation();
- DiscardUntilEndOfDirective();
- const SourceLocation ConditionalEnd = CurPPLexer->getSourceLocation();
+ SourceRange ConditionRange = DiscardUntilEndOfDirective();
PPConditionalInfo CI;
if (CurPPLexer->popConditionalLevel(CI)) {
@@ -3001,8 +3016,7 @@
if (CI.FoundElse) Diag(ElifToken, diag::pp_err_elif_after_else);
if (Callbacks)
- Callbacks->Elif(ElifToken.getLocation(),
- SourceRange(ConditionalBegin, ConditionalEnd),
+ Callbacks->Elif(ElifToken.getLocation(), ConditionRange,
PPCallbacks::CVK_NotEvaluated, CI.IfLoc);
if (PPOpts->SingleFileParseMode && !CI.FoundNonSkip) {
Index: include/clang/Lex/Preprocessor.h
===================================================================
--- include/clang/Lex/Preprocessor.h
+++ include/clang/Lex/Preprocessor.h
@@ -1832,8 +1832,8 @@
void CheckEndOfDirective(const char *DirType, bool EnableMacros = false);
/// Read and discard all tokens remaining on the current line until
- /// the tok::eod token is found.
- void DiscardUntilEndOfDirective();
+ /// the tok::eod token is found. Returns the range of the skipped tokens.
+ SourceRange DiscardUntilEndOfDirective();
/// Returns true if the preprocessor has seen a use of
/// __DATE__ or __TIME__ in the file so far.
@@ -2003,6 +2003,9 @@
/// True if the expression contained identifiers that were undefined.
bool IncludedUndefinedIds;
+
+ /// The source range for the expression.
+ SourceRange ExprRange;
};
/// Evaluate an integer constant expression that may occur after a
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
