fodinabor updated this revision to Diff 290460.
fodinabor marked 9 inline comments as done.
fodinabor added a comment.
Incorporating review comments:
- renaming option to -Wno-error=unknown and adding warning in description
- emit warnings instead of fully ignoring the issues
Documentation and unit tests will follow
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D86137/new/
https://reviews.llvm.org/D86137
Files:
.clang-format
clang/include/clang/Format/Format.h
clang/lib/Format/Format.cpp
clang/tools/clang-format/ClangFormat.cpp
llvm/include/llvm/Support/YAMLParser.h
llvm/include/llvm/Support/YAMLTraits.h
llvm/lib/Support/YAMLParser.cpp
llvm/lib/Support/YAMLTraits.cpp
Index: llvm/lib/Support/YAMLTraits.cpp
===================================================================
--- llvm/lib/Support/YAMLTraits.cpp
+++ llvm/lib/Support/YAMLTraits.cpp
@@ -48,6 +48,10 @@
Ctxt = Context;
}
+void IO::setAllowUnknownKeys(bool Allow) {
+ llvm_unreachable("Only supported for Input");
+}
+
//===----------------------------------------------------------------------===//
// Input
//===----------------------------------------------------------------------===//
@@ -197,8 +201,13 @@
return;
for (const auto &NN : MN->Mapping) {
if (!is_contained(MN->ValidKeys, NN.first())) {
- setError(NN.second.get(), Twine("unknown key '") + NN.first() + "'");
- break;
+ auto ReportNode = NN.second.get();
+ auto ReportMessage = Twine("unknown key '") + NN.first() + "'";
+ if (!AllowUnknownKeys) {
+ setError(ReportNode, ReportMessage);
+ break;
+ } else
+ reportWarning(ReportNode, ReportMessage);
}
}
}
@@ -370,6 +379,15 @@
EC = make_error_code(errc::invalid_argument);
}
+void Input::reportWarning(HNode *hnode, const Twine &message) {
+ assert(hnode && "HNode must not be NULL");
+ reportWarning(hnode->_node, message);
+}
+
+void Input::reportWarning(Node *node, const Twine &message) {
+ Strm->printError(node, message, SourceMgr::DK_Warning);
+}
+
std::unique_ptr<Input::HNode> Input::createHNodes(Node *N) {
SmallString<128> StringStorage;
if (ScalarNode *SN = dyn_cast<ScalarNode>(N)) {
@@ -428,6 +446,8 @@
setError(CurrentNode, Message);
}
+void Input::setAllowUnknownKeys(bool Allow) { AllowUnknownKeys = Allow; }
+
bool Input::canElideEmptySequence() {
return false;
}
Index: llvm/lib/Support/YAMLParser.cpp
===================================================================
--- llvm/lib/Support/YAMLParser.cpp
+++ llvm/lib/Support/YAMLParser.cpp
@@ -1775,12 +1775,9 @@
bool Stream::failed() { return scanner->failed(); }
-void Stream::printError(Node *N, const Twine &Msg) {
+void Stream::printError(Node *N, const Twine &Msg, SourceMgr::DiagKind Kind) {
SMRange Range = N ? N->getSourceRange() : SMRange();
- scanner->printError( Range.Start
- , SourceMgr::DK_Error
- , Msg
- , Range);
+ scanner->printError(Range.Start, Kind, Msg, Range);
}
document_iterator Stream::begin() {
Index: llvm/include/llvm/Support/YAMLTraits.h
===================================================================
--- llvm/include/llvm/Support/YAMLTraits.h
+++ llvm/include/llvm/Support/YAMLTraits.h
@@ -789,6 +789,7 @@
virtual NodeKind getNodeKind() = 0;
virtual void setError(const Twine &) = 0;
+ virtual void setAllowUnknownKeys(bool Allow);
template <typename T>
void enumCase(T &Val, const char* Str, const T ConstVal) {
@@ -1495,6 +1496,9 @@
void setError(HNode *hnode, const Twine &message);
void setError(Node *node, const Twine &message);
+ void reportWarning(HNode *hnode, const Twine &message);
+ void reportWarning(Node *hnode, const Twine &message);
+
public:
// These are only used by operator>>. They could be private
// if those templated things could be made friends.
@@ -1504,6 +1508,8 @@
/// Returns the current node that's being parsed by the YAML Parser.
const Node *getCurrentNode() const;
+ void setAllowUnknownKeys(bool Allow) override;
+
private:
SourceMgr SrcMgr; // must be before Strm
std::unique_ptr<llvm::yaml::Stream> Strm;
@@ -1514,6 +1520,7 @@
std::vector<bool> BitValuesUsed;
HNode *CurrentNode = nullptr;
bool ScalarMatchFound = false;
+ bool AllowUnknownKeys = false;
};
///
Index: llvm/include/llvm/Support/YAMLParser.h
===================================================================
--- llvm/include/llvm/Support/YAMLParser.h
+++ llvm/include/llvm/Support/YAMLParser.h
@@ -40,6 +40,7 @@
#include "llvm/ADT/StringRef.h"
#include "llvm/Support/Allocator.h"
#include "llvm/Support/SMLoc.h"
+#include "llvm/Support/SourceMgr.h"
#include <cassert>
#include <cstddef>
#include <iterator>
@@ -51,7 +52,6 @@
namespace llvm {
class MemoryBufferRef;
-class SourceMgr;
class raw_ostream;
class Twine;
@@ -100,7 +100,8 @@
return !failed();
}
- void printError(Node *N, const Twine &Msg);
+ void printError(Node *N, const Twine &Msg,
+ SourceMgr::DiagKind Kind = SourceMgr::DK_Error);
private:
friend class Document;
Index: clang/tools/clang-format/ClangFormat.cpp
===================================================================
--- clang/tools/clang-format/ClangFormat.cpp
+++ clang/tools/clang-format/ClangFormat.cpp
@@ -104,6 +104,16 @@
"SortIncludes style flag"),
cl::cat(ClangFormatCategory));
+// using the full param name as Wno-error probably won't be a common use case in
+// clang-format
+static cl::opt<bool> AllowUnknownOptions(
+ "Wno-error=unknown",
+ cl::desc("If set, unknown format options are only warned about.\n"
+ "Use with caution, as this might lead to dramatically "
+ "differing format depending on an option being "
+ "supported or not."),
+ cl::init(false), cl::cat(ClangFormatCategory));
+
static cl::opt<bool>
Verbose("verbose", cl::desc("If set, shows the list of processed files"),
cl::cat(ClangFormatCategory));
@@ -378,7 +388,8 @@
}
llvm::Expected<FormatStyle> FormatStyle =
- getStyle(Style, AssumedFileName, FallbackStyle, Code->getBuffer());
+ getStyle(Style, AssumedFileName, FallbackStyle, Code->getBuffer(),
+ nullptr, AllowUnknownOptions.getValue());
if (!FormatStyle) {
llvm::errs() << llvm::toString(FormatStyle.takeError()) << "\n";
return true;
Index: clang/lib/Format/Format.cpp
===================================================================
--- clang/lib/Format/Format.cpp
+++ clang/lib/Format/Format.cpp
@@ -1288,7 +1288,8 @@
return true;
}
-std::error_code parseConfiguration(StringRef Text, FormatStyle *Style) {
+std::error_code parseConfiguration(StringRef Text, FormatStyle *Style,
+ bool AllowUnknownOptions) {
assert(Style);
FormatStyle::LanguageKind Language = Style->Language;
assert(Language != FormatStyle::LK_None);
@@ -1302,6 +1303,7 @@
// Mapping also uses the context to get the language to find the correct
// base style.
Input.setContext(Style);
+ Input.setAllowUnknownKeys(AllowUnknownOptions);
Input >> Styles;
if (Input.error())
return Input.error();
@@ -2800,8 +2802,8 @@
llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
StringRef FallbackStyleName,
- StringRef Code,
- llvm::vfs::FileSystem *FS) {
+ StringRef Code, llvm::vfs::FileSystem *FS,
+ bool AllowUnknownOptions) {
if (!FS) {
FS = llvm::vfs::getRealFileSystem().get();
}
@@ -2813,7 +2815,8 @@
if (StyleName.startswith("{")) {
// Parse YAML/JSON style from the command line.
- if (std::error_code ec = parseConfiguration(StyleName, &Style))
+ if (std::error_code ec =
+ parseConfiguration(StyleName, &Style, AllowUnknownOptions))
return make_string_error("Error parsing -style: " + ec.message());
return Style;
}
@@ -2857,8 +2860,8 @@
FS->getBufferForFile(ConfigFile.str());
if (std::error_code EC = Text.getError())
return make_string_error(EC.message());
- if (std::error_code ec =
- parseConfiguration(Text.get()->getBuffer(), &Style)) {
+ if (std::error_code ec = parseConfiguration(
+ Text.get()->getBuffer(), &Style, AllowUnknownOptions)) {
if (ec == ParseError::Unsuitable) {
if (!UnsuitableConfigFiles.empty())
UnsuitableConfigFiles.append(", ");
Index: clang/include/clang/Format/Format.h
===================================================================
--- clang/include/clang/Format/Format.h
+++ clang/include/clang/Format/Format.h
@@ -2478,7 +2478,8 @@
private:
FormatStyleSet StyleSet;
- friend std::error_code parseConfiguration(StringRef Text, FormatStyle *Style);
+ friend std::error_code parseConfiguration(StringRef Text, FormatStyle *Style,
+ bool AllowUnknownOptions);
};
/// Returns a format style complying with the LLVM coding standards:
@@ -2533,7 +2534,11 @@
///
/// When ``BasedOnStyle`` is not present, options not present in the YAML
/// document, are retained in \p Style.
-std::error_code parseConfiguration(StringRef Text, FormatStyle *Style);
+///
+/// If AllowUnknownOptions is true, no errors are emitted if unknown
+/// format options are occured.
+std::error_code parseConfiguration(StringRef Text, FormatStyle *Style,
+ bool AllowUnknownOptions = false);
/// Gets configuration in a YAML string.
std::string configurationAsText(const FormatStyle &Style);
@@ -2670,6 +2675,9 @@
/// language if the filename isn't sufficient.
/// \param[in] FS The underlying file system, in which the file resides. By
/// default, the file system is the real file system.
+/// \param[in] AllowUnknownOptions If true, unknown format options only
+/// emit a warning. If false, errors are emitted on unknown format
+/// options.
///
/// \returns FormatStyle as specified by ``StyleName``. If ``StyleName`` is
/// "file" and no file is found, returns ``FallbackStyle``. If no style could be
@@ -2677,7 +2685,8 @@
llvm::Expected<FormatStyle> getStyle(StringRef StyleName, StringRef FileName,
StringRef FallbackStyle,
StringRef Code = "",
- llvm::vfs::FileSystem *FS = nullptr);
+ llvm::vfs::FileSystem *FS = nullptr,
+ bool AllowUnknownOptions = false);
// Guesses the language from the ``FileName`` and ``Code`` to be formatted.
// Defaults to FormatStyle::LK_Cpp.
Index: .clang-format
===================================================================
--- .clang-format
+++ .clang-format
@@ -1 +1,2 @@
BasedOnStyle: LLVM
+testbshit: ste
\ No newline at end of file
_______________________________________________
cfe-commits mailing list
cfe-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/cfe-commits
